Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WAF policy resource, managed WAF rulesets data source, and waf_policy_id in build settings #77

Merged
merged 1 commit into from
Nov 14, 2024

Conversation

ramonsnir
Copy link
Member

@ramonsnir ramonsnir commented Nov 12, 2024

Copy link
Member

@rybit rybit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing blocking! I think it will be great to have it!

Comment on lines 18 to 19
name = "Terraform Ruleset"
description = "This is a test ruleset through Terraform"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this is the policy not the rule set. A policy contains a set of rule sets that each in turn contain a set of rules

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're not wrong, but this is what netlify-react-ui sets for all its policies:

    "name": "Baseline Ruleset",
    "description": "Netlify maintained ruleset",

I copied this very early in the development and didn't think about it. I'll change it to say policy.

description = "This is a test ruleset through Terraform"
rule_sets = [
{
managed_id = "crs-basic",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there some const that we can export? I don't know if terraform has those in general

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will think about that. Worst-case, maybe a hard-coded data source? I'll let you know what I find.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see something in the docs, and ChatGPT also recommended a hard-coded data source - which I don't love. I haven't seen other people doing that.

I will note that there is a static validation on this: https://github.com/netlify/terraform-provider-netlify/pull/77/files#diff-020ba6260ff9fbb79426060f367c74580fbcd04ff8b20c35bbd6345d1acb9157R116

It will fail the .tf file before trying to apply, and I think smart IDEs might do a red squiggly if you enter an invalid value.

@@ -0,0 +1,2 @@
# Import a WAF policy by its team ID and the policy ID
terraform import netlify_log_drain.http 6600abcdef1234567890abcd:6600abcdef1234567890abcd
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what this is doing?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should say netlify_waf_policy 🙂 It's an example of how you import it through the CLI. It goes into the generated Markdown docs.

@ramonsnir
Copy link
Member Author

Merging of this PR is waiting on an API change around plan capabilities, coming up in the next few days. The change might require logic change when connecting sites to policies, so I'm waiting for that to be done to test the two together.

@ramonsnir ramonsnir marked this pull request as ready for review November 14, 2024 16:19
@ramonsnir ramonsnir merged commit 2da870c into main Nov 14, 2024
3 checks passed
@ramonsnir ramonsnir deleted the waf branch November 14, 2024 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants