From c854df433163f3d2808adfa6aa8dffcc4b3ae36d Mon Sep 17 00:00:00 2001 From: James Armes Date: Mon, 28 Oct 2024 15:26:18 -0400 Subject: [PATCH 1/8] feat: Added rate limiting configuration. --- main.tf | 54 ++++++++++++++++++++++++++++++++++------------------ variables.tf | 11 +++++++++++ 2 files changed, 47 insertions(+), 18 deletions(-) diff --git a/main.tf b/main.tf index 92213cf..0f56135 100644 --- a/main.tf +++ b/main.tf @@ -127,33 +127,51 @@ resource "aws_wafv2_web_acl" "waf" { visibility_config { cloudwatch_metrics_enabled = true - metric_name = "${local.prefix}-${rule.key}" + metric_name = "${local.prefix}-ip-${rule.key}" sampled_requests_enabled = true } } } - # TODO: Make rate-limiting configurable. - rule { - name = "AWS-RateBasedRule-IP-300" - priority = 100 + # For each rate-limiting rule, create a rule with the appropriate action. + dynamic "rule" { + for_each = var.rate_limit_rules + content { + name = rule.value.name != "" ? rule.value.name : "${local.prefix}-rate-${rule.key}" + priority = rule.value.priority != null + ? rule.value.priority + : index(var.ip_set_rules, rule.key) + length(var.ip_set_rules) - action { - count {} - } + action { + dynamic "allow" { + for_each = rule.value.action == "allow" ? [true] : [] + content {} + } - statement { - rate_based_statement { - aggregate_key_type = "IP" - evaluation_window_sec = 300 - limit = 300 + dynamic "block" { + for_each = rule.value.action == "block" ? [true] : [] + content {} + } + + dynamic "count" { + for_each = rule.value.action == "count" ? [true] : [] + content {} + } } - } - visibility_config { - cloudwatch_metrics_enabled = true - metric_name = "${local.prefix}-waf-rate-limit" - sampled_requests_enabled = true + statement { + rate_based_statement { + aggregate_key_type = "IP" + evaluation_window_sec = rule.value.window + limit = rule.value.limit + } + } + + visibility_config { + cloudwatch_metrics_enabled = true + metric_name = "${local.prefix}-rate-${rule.key}" + sampled_requests_enabled = true + } } } diff --git a/variables.tf b/variables.tf index c5291b0..3401542 100644 --- a/variables.tf +++ b/variables.tf @@ -47,6 +47,17 @@ variable "ip_set_rules" { default = {} } +variable "rate_limit_rules" { + type = map(object({ + name = optional(string, "") + window = optional(number, 300) + action = optional(string, "allow") + limit = optional(number, 300) + })) + description = "Rate limiting configuration for the WAF." + default = {} +} + variable "subdomain" { type = string description = "Subdomain used for this deployment. Defaults to the environment." From 5aa378e4cc569531e03d24b1e874297cc72f68cf Mon Sep 17 00:00:00 2001 From: James Armes Date: Mon, 28 Oct 2024 15:36:04 -0400 Subject: [PATCH 2/8] fix: Priority formatting --- main.tf | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/main.tf b/main.tf index 0f56135..68a09da 100644 --- a/main.tf +++ b/main.tf @@ -138,9 +138,7 @@ resource "aws_wafv2_web_acl" "waf" { for_each = var.rate_limit_rules content { name = rule.value.name != "" ? rule.value.name : "${local.prefix}-rate-${rule.key}" - priority = rule.value.priority != null - ? rule.value.priority - : index(var.ip_set_rules, rule.key) + length(var.ip_set_rules) + priority = rule.value.priority != null ? rule.value.priority : index(var.ip_set_rules, rule.key) + length(var.ip_set_rules) action { dynamic "allow" { From d3f450dc36dcf8f2606e03235ba6546926f0eb4b Mon Sep 17 00:00:00 2001 From: James Armes Date: Mon, 28 Oct 2024 15:39:26 -0400 Subject: [PATCH 3/8] fix: Missing object property. --- main.tf | 4 +++- variables.tf | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/main.tf b/main.tf index 68a09da..0f56135 100644 --- a/main.tf +++ b/main.tf @@ -138,7 +138,9 @@ resource "aws_wafv2_web_acl" "waf" { for_each = var.rate_limit_rules content { name = rule.value.name != "" ? rule.value.name : "${local.prefix}-rate-${rule.key}" - priority = rule.value.priority != null ? rule.value.priority : index(var.ip_set_rules, rule.key) + length(var.ip_set_rules) + priority = rule.value.priority != null + ? rule.value.priority + : index(var.ip_set_rules, rule.key) + length(var.ip_set_rules) action { dynamic "allow" { diff --git a/variables.tf b/variables.tf index 3401542..58644d7 100644 --- a/variables.tf +++ b/variables.tf @@ -50,9 +50,10 @@ variable "ip_set_rules" { variable "rate_limit_rules" { type = map(object({ name = optional(string, "") - window = optional(number, 300) action = optional(string, "allow") limit = optional(number, 300) + window = optional(number, 300) + priority = optional(number, null) })) description = "Rate limiting configuration for the WAF." default = {} From 0a633c9c67696281b06bdd5d2b280bb7beb544a1 Mon Sep 17 00:00:00 2001 From: James Armes Date: Mon, 28 Oct 2024 15:40:50 -0400 Subject: [PATCH 4/8] fix: Formatting. --- main.tf | 20 +++++++++----------- testing.tfvars | 4 ++-- variables.tf | 8 ++++---- 3 files changed, 15 insertions(+), 17 deletions(-) diff --git a/main.tf b/main.tf index 0f56135..d19327d 100644 --- a/main.tf +++ b/main.tf @@ -138,9 +138,7 @@ resource "aws_wafv2_web_acl" "waf" { for_each = var.rate_limit_rules content { name = rule.value.name != "" ? rule.value.name : "${local.prefix}-rate-${rule.key}" - priority = rule.value.priority != null - ? rule.value.priority - : index(var.ip_set_rules, rule.key) + length(var.ip_set_rules) + priority = rule.value.priority != null ? rule.value.priority : index(var.ip_set_rules, rule.key) + length(var.ip_set_rules) action { dynamic "allow" { @@ -182,12 +180,12 @@ resource "aws_wafv2_web_acl" "waf" { override_action { dynamic "none" { for_each = var.passive ? [] : [true] - content { } + content {} } dynamic "count" { for_each = var.passive ? [true] : [] - content { } + content {} } } @@ -212,12 +210,12 @@ resource "aws_wafv2_web_acl" "waf" { override_action { dynamic "none" { for_each = var.passive ? [] : [true] - content { } + content {} } dynamic "count" { for_each = var.passive ? [true] : [] - content { } + content {} } } @@ -242,12 +240,12 @@ resource "aws_wafv2_web_acl" "waf" { override_action { dynamic "none" { for_each = var.passive ? [] : [true] - content { } + content {} } dynamic "count" { for_each = var.passive ? [true] : [] - content { } + content {} } } @@ -272,12 +270,12 @@ resource "aws_wafv2_web_acl" "waf" { override_action { dynamic "none" { for_each = var.passive ? [] : [true] - content { } + content {} } dynamic "count" { for_each = var.passive ? [true] : [] - content { } + content {} } } diff --git a/testing.tfvars b/testing.tfvars index 1830800..679a5a0 100644 --- a/testing.tfvars +++ b/testing.tfvars @@ -1,3 +1,3 @@ log_bucket = "testing-bucket.s3.amazonaws.com" -domain = "example.com" -project = "example" +domain = "example.com" +project = "example" diff --git a/variables.tf b/variables.tf index 58644d7..c12bda2 100644 --- a/variables.tf +++ b/variables.tf @@ -49,10 +49,10 @@ variable "ip_set_rules" { variable "rate_limit_rules" { type = map(object({ - name = optional(string, "") - action = optional(string, "allow") - limit = optional(number, 300) - window = optional(number, 300) + name = optional(string, "") + action = optional(string, "allow") + limit = optional(number, 300) + window = optional(number, 300) priority = optional(number, null) })) description = "Rate limiting configuration for the WAF." From 5e7ccc2aa4184b2d83493048e776fd8be8f9bfed Mon Sep 17 00:00:00 2001 From: James Armes Date: Mon, 28 Oct 2024 16:55:45 -0400 Subject: [PATCH 5/8] fix: Updated metric names. --- main.tf | 4 ++-- variables.tf | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/main.tf b/main.tf index d19327d..5512343 100644 --- a/main.tf +++ b/main.tf @@ -127,7 +127,7 @@ resource "aws_wafv2_web_acl" "waf" { visibility_config { cloudwatch_metrics_enabled = true - metric_name = "${local.prefix}-ip-${rule.key}" + metric_name = "${local.prefix}-waf-ip-${rule.key}" sampled_requests_enabled = true } } @@ -167,7 +167,7 @@ resource "aws_wafv2_web_acl" "waf" { visibility_config { cloudwatch_metrics_enabled = true - metric_name = "${local.prefix}-rate-${rule.key}" + metric_name = "${local.prefix}-waf-rate-${rule.key}" sampled_requests_enabled = true } } diff --git a/variables.tf b/variables.tf index c12bda2..6a81471 100644 --- a/variables.tf +++ b/variables.tf @@ -43,7 +43,7 @@ variable "ip_set_rules" { priority = optional(number, null) arn = string })) - description = "Custom WAF rules to apply to the CloudFront distribution." + description = "Custom IP Set rules for the WAF." default = {} } @@ -61,7 +61,7 @@ variable "rate_limit_rules" { variable "subdomain" { type = string - description = "Subdomain used for this deployment. Defaults to the environment." + description = "Subdomain for the distribution. Defaults to the environment." default = "" } From d1cb7f004027762b6e93bf7c5de45444aac4dfa7 Mon Sep 17 00:00:00 2001 From: James Armes Date: Tue, 29 Oct 2024 12:07:12 -0400 Subject: [PATCH 6/8] docs: Added documentation for `ip_set_rules` and `rate_limit_rules`. --- README.md | 74 +++++++++++++++++++++++++++++++++++++++++++--------- variables.tf | 6 ++--- 2 files changed, 65 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index 62ae5bf..e63897e 100644 --- a/README.md +++ b/README.md @@ -49,18 +49,22 @@ these rules are spaced out to allow for custom rules to be inserted between. ## Inputs -| Name | Description | Type | Default | Required | -|----------------|-----------------------------------------------------------------------------------------------------|----------------|---------|----------| -| domain | Primary domain for the distribution. The hosted zone for this domain should be in the same account. | `string` | n/a | yes | -| log_bucket | Domain name of the S3 bucket to send logs to. | `string` | n/a | yes | -| project | The name of the project. | `string` | n/a | yes | -| environment | The environment for the project. | `string` | `"dev"` | no | -| [ip_set_rules] | The environment for the project. | `map(object)` | `"dev"` | no | -| log_group | CloudWatch log group to send WAF logs to. | `list(string)` | `[]` | no | -| origin_domain | Fully qualified domain name for the origin. Defaults to `origin.${subdomain}.${domain}`. | `string` | n/a | no | -| passive | Enable passive mode for the WAF, counting all requests rather than blocking. | `bool` | `false` | no | -| subdomain | Subdomain for the distribution. Defaults to the environment. | `string` | n/a | no | -| tags | Optional tags to be applied to all resources. | `list` | `[]` | no | +> [!WARNING] +> **This is an early release, and the API is subject to change until `v1.0.0`.** + +| Name | Description | Type | Default | Required | +|--------------------|-----------------------------------------------------------------------------------------------------|---------------|---------|----------| +| domain | Primary domain for the distribution. The hosted zone for this domain should be in the same account. | `string` | n/a | yes | +| log_bucket | Domain name of the S3 bucket to send logs to. | `string` | n/a | yes | +| log_group | CloudWatch log group to send WAF logs to. | `string` | n/a | yes | +| project | Project that these resources are supporting. | `string` | n/a | yes | +| environment | The environment for the deployment. | `string` | `"dev"` | no | +| [ip_set_rules] | Custom IP Set rules for the WAF | `map(object)` | `{}` | no | +| [rate_limit_rules] | Rate limiting configuration for the WAF. | `map(object)` | `{}` | no | +| origin_domain | Fully qualified domain name for the origin. Defaults to `origin.${subdomain}.${domain}`. | `string` | n/a | no | +| passive | Enable passive mode for the WAF, counting all requests rather than blocking. | `bool` | `false` | no | +| subdomain | Subdomain for the distribution. Defaults to the environment. | `string` | n/a | no | +| tags | Optional tags to be applied to all resources. | `map(string)` | `{}` | no | ### ip_set_rules @@ -102,10 +106,56 @@ module "cloudfront_waf" { } ``` +| Name | Description | Type | Default | Required | +|----------|-------------------------------------------------------------------------------|----------|-----------|----------| +| action | The action to perform. | `string` | `"allow"` | no | +| arn | ARN of the IP set to match on. | `string` | n/a | yes | +| name | Name for this rule. Defaults to `${project}-${environment}-rate-${rule.key}`. | `string` | `""` | no | +| priority | Rule priority. Defaults to the rule's position in the map. | `number` | `nil` | no | + +### rate_limit_rules + +To rate limit traffic based on IP address, you can specify a map of rate limit +rules to create. The rate limit rules are applied in the order they are defined, +or though the `priority` field. + +> _Note: Rate limit rules are added after all IP set rules by default. Use +> `priority` to order your rules if you need more control._ + +For example, to rate limit requests to 300 over a 5-minute period: + +```hcl +module "cloudfront_waf" { + source = "github.com/codeforamerica/tofu-modules-aws-cloudfront-waf?ref=1.1.0" + + project = "my-project" + environment = "staging" + domain = "my-project.org" + log_bucket = module.logging.bucket + + rate_limit_rules = { + limit = { + name = "my-project-staging-rate-limit" + action = "block" + limit = 500 + window = 500 + } + } +} +``` + +| Name | Description | Type | Default | Required | +|----------|-----------------------------------------------------------------------------------------|----------|-----------|----------| +| action | The action to perform. | `string` | `"block"` | no | +| name | Name for this rule. Defaults to `${project}-${environment}-rate-${rule.key}`. | `string` | `""` | no | +| limit | The number of requests allowed within the window. Minimum value of 10. | `number` | `10` | no | +| priority | Rule priority. Defaults to the rule's position in the map + the number of IP set rules. | `number` | `nil` | no | +| window | Number of seconds to limit requests in. Options are: 60, 120, 300, 600 | `number` | `60` | no | [distribution]: https://docs.aws.amazon.com/AmazonCloudFront/latest/DeveloperGuide/distribution-working-with.html [ip-rules]: https://docs.aws.amazon.com/waf/latest/developerguide/waf-rule-statement-type-ipset-match.html [ip_set_rules]: #ip_set_rules +[rate_limit_rules]: #rate_limit_rules [rules-common]: https://docs.aws.amazon.com/waf/latest/developerguide/aws-managed-rule-groups-baseline.html#aws-managed-rule-groups-baseline-crs [rules-inputs]: https://docs.aws.amazon.com/waf/latest/developerguide/aws-managed-rule-groups-baseline.html#aws-managed-rule-groups-baseline-known-bad-inputs [rules-ip-rep]: https://docs.aws.amazon.com/waf/latest/developerguide/aws-managed-rule-groups-ip-rep.html#aws-managed-rule-groups-ip-rep-amazon diff --git a/variables.tf b/variables.tf index 6a81471..3c8e4b2 100644 --- a/variables.tf +++ b/variables.tf @@ -50,9 +50,9 @@ variable "ip_set_rules" { variable "rate_limit_rules" { type = map(object({ name = optional(string, "") - action = optional(string, "allow") - limit = optional(number, 300) - window = optional(number, 300) + action = optional(string, "block") + limit = optional(number, 10) + window = optional(number, 60) priority = optional(number, null) })) description = "Rate limiting configuration for the WAF." From 0d4d881669340076d11069668ecaa86287c6430e Mon Sep 17 00:00:00 2001 From: James Armes Date: Tue, 29 Oct 2024 12:10:57 -0400 Subject: [PATCH 7/8] docs: Updated note format. --- README.md | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index e63897e..780f834 100644 --- a/README.md +++ b/README.md @@ -49,8 +49,6 @@ these rules are spaced out to allow for custom rules to be inserted between. ## Inputs -> [!WARNING] -> **This is an early release, and the API is subject to change until `v1.0.0`.** | Name | Description | Type | Default | Required | |--------------------|-----------------------------------------------------------------------------------------------------|---------------|---------|----------| @@ -119,8 +117,9 @@ To rate limit traffic based on IP address, you can specify a map of rate limit rules to create. The rate limit rules are applied in the order they are defined, or though the `priority` field. -> _Note: Rate limit rules are added after all IP set rules by default. Use -> `priority` to order your rules if you need more control._ +> [!NOTE] +> Rate limit rules are added after all IP set rules by default. Use `priority` +> to order your rules if you need more control. For example, to rate limit requests to 300 over a 5-minute period: From bb52510155202e2f5978929e8f0af6cf8b8f2a8e Mon Sep 17 00:00:00 2001 From: James Armes Date: Tue, 29 Oct 2024 12:59:18 -0400 Subject: [PATCH 8/8] docs: Pull in latest from template. --- .github/workflows/release.yaml | 2 +- CONTRIBUTING.md | 46 ++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 CONTRIBUTING.md diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index dd9cb8c..1fb4223 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -16,7 +16,7 @@ jobs: - name: Checkout source code uses: actions/checkout@v4 with: - fetch-tags: true + fetch-depth: 0 - name: Bump version and create changelog id: bump uses: commitizen-tools/commitizen-action@master diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..347cab4 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,46 @@ +# Contributing + +## Commit message format + +All commit messages should follow the [Conventional Commits][commits] format. +This format allows us to automatically generate changelogs and version numbers +based on the commit messages. + +Common commit types include: + +* `fix`: A bug fix +* `feat`: A new feature +* `ci`: Changes to CI/CD +* `docs`: Changes to documentation + +adding `!` after the type indicates a breaking change. For example, `feat!` +would indicate a new feature that breaks existing functionality, and would +therefore require a major version bump. + +`bump` is a special type used to indicate a version bump. This is used by the +automated release process, and should be avoided in normal commits. + +## Coding standards + +Code should follow the [OpenTofu style conventions][style]. This ensures that +all code is consistent and easy to read and maintain. + +To make resources easier to find, you may group them together in a single file +within your module. For example, while `main.tf` handles the main configuration, +you may create a `dns.tf` file to handle all DNS-related resources. + +Additionally, the following should be grouped together within their own files: + +* `data.tf` for data sources +* `local.tf` for local values +* `output.tf` for outputs + +## Code reviews + +All code should be contributing in the form of a pull request. Pull requests +should have an approval from _at least_ one required reviewer as defined in the +`CODEOWNERS` file. Additional reviews are welcome, and may be requested by +either the submitter or the required reviewer. + +[commits]: https://www.conventionalcommits.org/en/v1.0.0/ +[style]: https://opentofu.org/docs/language/syntax/style/