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.

Sign up for GitHub

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

Jump to bottom

resource/ruleset: add support for HTTP rate limiting #1179

Merged
merged 1 commit into from
Sep 12, 2021
Merged

resource/ruleset: add support for HTTP rate limiting #1179

merged 1 commit into from
Sep 12, 2021

Conversation

jacobbednarz
Copy link
Member

jacobbednarz
jacobbednarz commented Sep 7, 2021
View reviewed changes
website/docs/r/ruleset.html.markdown
@@ -107,6 +133,7 @@ The following arguments are supported:
* `enabled` - (Optional) Whether the rule is active.
* `expression` - (Required) Criteria for an HTTP request to trigger the ruleset rule action. Uses the Firewall Rules expression language based on Wireshark display filters. Refer to the [Firewall Rules language](https://developers.cloudflare.com/firewall/cf-firewall-language) documentation for all available fields, operators, and functions.
* `id` - (Read only) Unique rule identifier.
* `ratelimit` - (Optional) List of parameters that configure HTTP rate limiting behaviour (refer to the [nested schema](#nestedblock--ratelimiting-parameters)).
Copy link
Member Author

Choose a reason for hiding this comment

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

cc @pedrosousa for 👀

@jacobbednarz
Copy link
Member Author

CI is happy (with cloudflare-go shimmed in)

TF_ACC=1 go test $(go list ./...) -v -run "^TestAccCloudflareRuleset_" -count 1 -timeout 120m -parallel 1
?   	github.com/cloudflare/terraform-provider-cloudflare	[no test files]
=== RUN   TestAccCloudflareRuleset_WAFBasic
=== PAUSE TestAccCloudflareRuleset_WAFBasic
=== RUN   TestAccCloudflareRuleset_WAFManagedRuleset
=== PAUSE TestAccCloudflareRuleset_WAFManagedRuleset
=== RUN   TestAccCloudflareRuleset_WAFManagedRulesetOWASP
=== PAUSE TestAccCloudflareRuleset_WAFManagedRulesetOWASP
=== RUN   TestAccCloudflareRuleset_WAFManagedRulesetOWASPBlockXSSWithAnomalyOver60
=== PAUSE TestAccCloudflareRuleset_WAFManagedRulesetOWASPBlockXSSWithAnomalyOver60
=== RUN   TestAccCloudflareRuleset_WAFManagedRulesetOWASPOnlyPL1
=== PAUSE TestAccCloudflareRuleset_WAFManagedRulesetOWASPOnlyPL1
=== RUN   TestAccCloudflareRuleset_WAFManagedRulesetDeployMultiple
=== PAUSE TestAccCloudflareRuleset_WAFManagedRulesetDeployMultiple
=== RUN   TestAccCloudflareRuleset_WAFManagedRulesetDeployMultipleWithSkip
=== PAUSE TestAccCloudflareRuleset_WAFManagedRulesetDeployMultipleWithSkip
=== RUN   TestAccCloudflareRuleset_WAFManagedRulesetDeployMultipleWithTopSkipAndLastSkip
=== PAUSE TestAccCloudflareRuleset_WAFManagedRulesetDeployMultipleWithTopSkipAndLastSkip
=== RUN   TestAccCloudflareRuleset_WAFManagedRulesetWithCategoryBasedOverrides
=== PAUSE TestAccCloudflareRuleset_WAFManagedRulesetWithCategoryBasedOverrides
=== RUN   TestAccCloudflareRuleset_WAFManagedRulesetWithIDBasedOverrides
=== PAUSE TestAccCloudflareRuleset_WAFManagedRulesetWithIDBasedOverrides
=== RUN   TestAccCloudflareRuleset_MagicTransitUpdateWithHigherPriority
    provider_test.go:138: Skipping acceptance test as 0da42c8d2132a9ddaf714f9e7c920711 is not configured for Magic Transit
--- SKIP: TestAccCloudflareRuleset_MagicTransitUpdateWithHigherPriority (0.00s)
=== RUN   TestAccCloudflareRuleset_RateLimit
=== PAUSE TestAccCloudflareRuleset_RateLimit
=== CONT  TestAccCloudflareRuleset_WAFBasic
--- PASS: TestAccCloudflareRuleset_WAFBasic (3.17s)
=== CONT  TestAccCloudflareRuleset_WAFManagedRulesetDeployMultipleWithSkip
--- PASS: TestAccCloudflareRuleset_WAFManagedRulesetDeployMultipleWithSkip (3.34s)
=== CONT  TestAccCloudflareRuleset_RateLimit
--- PASS: TestAccCloudflareRuleset_RateLimit (3.27s)
=== CONT  TestAccCloudflareRuleset_WAFManagedRulesetWithIDBasedOverrides
--- PASS: TestAccCloudflareRuleset_WAFManagedRulesetWithIDBasedOverrides (3.36s)
=== CONT  TestAccCloudflareRuleset_WAFManagedRulesetWithCategoryBasedOverrides
--- PASS: TestAccCloudflareRuleset_WAFManagedRulesetWithCategoryBasedOverrides (3.07s)
=== CONT  TestAccCloudflareRuleset_WAFManagedRulesetDeployMultipleWithTopSkipAndLastSkip
--- PASS: TestAccCloudflareRuleset_WAFManagedRulesetDeployMultipleWithTopSkipAndLastSkip (4.22s)
=== CONT  TestAccCloudflareRuleset_WAFManagedRulesetOWASPBlockXSSWithAnomalyOver60
--- PASS: TestAccCloudflareRuleset_WAFManagedRulesetOWASPBlockXSSWithAnomalyOver60 (3.38s)
=== CONT  TestAccCloudflareRuleset_WAFManagedRulesetDeployMultiple
--- PASS: TestAccCloudflareRuleset_WAFManagedRulesetDeployMultiple (3.38s)
=== CONT  TestAccCloudflareRuleset_WAFManagedRulesetOWASPOnlyPL1
--- PASS: TestAccCloudflareRuleset_WAFManagedRulesetOWASPOnlyPL1 (3.84s)
=== CONT  TestAccCloudflareRuleset_WAFManagedRuleset
--- PASS: TestAccCloudflareRuleset_WAFManagedRuleset (3.02s)
=== CONT  TestAccCloudflareRuleset_WAFManagedRulesetOWASP
--- PASS: TestAccCloudflareRuleset_WAFManagedRulesetOWASP (3.73s)
PASS
ok  	github.com/cloudflare/terraform-provider-cloudflare/cloudflare	38.146s
?   	github.com/cloudflare/terraform-provider-cloudflare/version	[no test files]

pedrosousa
pedrosousa reviewed Sep 7, 2021
View reviewed changes
website/docs/r/ruleset.html.markdown Outdated
<a id="nestedblock--ratelimiting-parameters"></a>
**Nested schema for `ratelimit`**

* `characteristics` - (Optional) List of parameters that define how Cloudflare tracks the request rate for this rule.
Copy link
Contributor

Choose a reason for hiding this comment

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

Quick note about the optional/mandatory fields (sorry for only noticing this now).

For rate limiting rules, only "mitigation_expression" is optional. However, all the fields are marked as optional.

It would probably be more useful to say if the fields are mandatory/optional in the context of the current nested schema. However, all other sections consider the global rule definition context, so they would also have to be changed.

Copy link
Member Author

Choose a reason for hiding this comment

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

The value will come from the Terraform schema which unfortunately doesn't have the same level of context as the API itself so in that aspect, and what we report here, the fields are optional. In the past, we've leant on adequate documentation in the API or developer documentation to provide more insight into available configurations.

website/docs/r/ruleset.html.markdown Outdated Show resolved Hide resolved
website/docs/r/ruleset.html.markdown Outdated Show resolved Hide resolved
pedrosousa
pedrosousa reviewed Sep 7, 2021
View reviewed changes
Copy link
Contributor

@pedrosousa pedrosousa left a comment

Choose a reason for hiding this comment

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

Revert capitalization changes back to "Rate Limiting rules".

website/docs/r/ruleset.html.markdown Outdated Show resolved Hide resolved
website/docs/r/ruleset.html.markdown Outdated Show resolved Hide resolved
@jacobbednarz jacobbednarz merged commit 43c9032 into master Sep 12, 2021
@jacobbednarz jacobbednarz deleted the ruleset-http-ratelimit branch September 12, 2021 21:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pedrosousa pedrosousa pedrosousa left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add support for Rate Limiting rules (ruleset)
2 participants
@jacobbednarz @pedrosousa