Skip to content

Conversation

@ramonsnir
Copy link
Contributor

@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
Contributor 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
Contributor 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
Contributor 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
Contributor 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
Contributor 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