umotif-public / terraform-aws-waf-webaclv2

Terraform module to configure WAF V2 Web ACL with managed rules for Application Load Balancer
https://registry.terraform.io/modules/umotif-public/waf-webaclv2/aws
Other
137 stars 124 forks source link

Invalid index when scope_down_statement not defined in rate_based_statement #117

Closed splichy closed 1 year ago

splichy commented 1 year ago

What is the current behavior?

╷
│ Error: Invalid index
│ 
│   on ../modules/aws-waf-webaclv2/main.tf line 2554, in resource "aws_wafv2_web_acl" "main":
│ 2554:               for_each = (contains(keys(rate_based_statement.value), "scope_down_statement")) && rate_based_statement.value["scope_down_statement"] != null ? [lookup(rate_based_statement.value, "scope_down_statement", {})] : []
│     ├────────────────
│     │ rate_based_statement.value is object with 2 attributes
│ 
│ The given key does not identify an element in this collection value.
╵

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem. scope_down_statement is optional: documentation

{
      name     = "ip-rate-limit"
      priority = "8"
      action   = "block"

      rate_based_statement = {
        limit              = 2000
        aggregate_key_type = "IP"
      }

      visibility_config = {
        cloudwatch_metrics_enabled = false
        sampled_requests_enabled   = false
      }
}

What is the expected behavior? Create rule without scope_down_statement

Software versions? Terraform v1.5.3 on linux_amd64

splichy commented 1 year ago

Change was introduced in https://github.com/umotif-public/terraform-aws-waf-webaclv2/commit/ee76f542346b9702a331ceee0a7b296804b3dd1d -> https://github.com/umotif-public/terraform-aws-waf-webaclv2/pull/102/files I don't understand how the initial issue on lines 6111 & 6112 relates with line 2554?

splichy commented 1 year ago

I believe that the previous for_each was correct, so adding a fix + discovered the same on https://github.com/umotif-public/terraform-aws-waf-webaclv2/pull/118/files#diff-dc46acf24afd63ef8c556b77c126ccc6e578bc87e3aa09a931f33d9bf2532fbbL2820

splichy commented 1 year ago

Also adding PR https://github.com/umotif-public/terraform-aws-waf-webaclv2/pull/119 with fixed tests

splichy commented 1 year ago

Hi @Ohid25, can you have a look at mentioned PRs?

Ohid25 commented 1 year ago

Thank you for the contribution @splichy - this is now released as a part of 5.1.2.