-
Notifications
You must be signed in to change notification settings - Fork 2
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
Conversation
There was a problem hiding this 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!
docs/resources/waf_policy.md
Outdated
name = "Terraform Ruleset" | ||
description = "This is a test ruleset through Terraform" |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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", |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
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. |
…_id in build settings
WAF policy resource
Managed WAF rules data source