terraform-provider-aws icon indicating copy to clipboard operation
terraform-provider-aws copied to clipboard

wafv2 rate_based_rule with nested scopedown and/or not working

Open ghost opened this issue 4 years ago • 9 comments

This issue was originally opened by @jpatallah as hashicorp/terraform#26530. It was migrated here as a result of the provider split. The original body of the issue is below.


Terraform Version

terraform version
Terraform v0.13.3

Terraform Configuration Files

  rule {
    name     = "tf-jptest-login"
    priority = 7

    action {
      block {}
    }

    statement {
      rate_based_statement {
        limit              = 100
        aggregate_key_type = "IP"
        scope_down_statement {
          and_statement {
            statement {
              byte_match_statement {
                field_to_match {
                  uri_path {}
                }
                positional_constraint = "CONTAINS"
                search_string = "login"
                text_transformation {
                  priority = 1
                  type     = "LOWERCASE"
                }
              }
            }
            statement {
              not_statement {
                statement {
                  or_statement {
                    statement {
                      ip_set_reference_statement {
                        arn = aws_wafv2_ip_set.tf-jptest-local-ips.arn
                      }
                    }
                    statement {
                      regex_pattern_set_reference_statement {
                        arn = aws_wafv2_regex_pattern_set.tf-jptest-good-bots.arn
                        field_to_match {
                          single_header {
                            name = "user-agent"
                          }
                        }

                        text_transformation {
                          priority = 1
                          type     = "LOWERCASE"
                        }
                      }
                    }
                  }
                }
              }
            }
          }
        }
      }
    }

    visibility_config {
      cloudwatch_metrics_enabled = true
      metric_name                = "tf-jptest-login"
      sampled_requests_enabled   = true
    }
  }

Debug Output

-----------------------------------------------------
2020/10/09 05:23:10 [DEBUG] [aws-sdk-go] {}

Error: Unsupported block type

  on acl.tf line 41, in resource "aws_wafv2_web_acl" "acl":
  41:                   or_statement {

Blocks of type "or_statement" are not expected here.

Expected Behavior

It should create the wafv2 rule

Actual Behavior

Failed with error message: Blocks of type "or_statement" are not expected here.

Additional Context

Works in the aws gui using the json editor:

{
  "Name": "tf-jptest-login",
  "Priority": 7,
  "Statement": {
    "RateBasedStatement": {
      "Limit": 100,
      "AggregateKeyType": "IP",
      "ScopeDownStatement": {
        "AndStatement": {
          "Statements": [
            {
              "ByteMatchStatement": {
                "SearchString": "login",
                "FieldToMatch": {
                  "UriPath": {}
                },
                "TextTransformations": [
                  {
                    "Priority": 1,
                    "Type": "LOWERCASE"
                  }
                ],
                "PositionalConstraint": "CONTAINS"
              }
            },
            {
              "NotStatement": {
                "Statement": {
                  "OrStatement": {
                    "Statements": [
                      {
                        "IPSetReferenceStatement": {
                          "ARN": "arn:aws:wafv2:us-east-1:<redacted>:global/ipset/tf-local-ips/3761c76e-4c42-4d96-96d9-ada46e4e917e"
                        }
                      },
                      {
                        "RegexPatternSetReferenceStatement": {
                          "ARN": "arn:aws:wafv2:us-east-1:<redacted>:global/regexpatternset/tf-good-bots/25663cc9-2ed8-4d4f-b0b7-93ad4b28b150",
                          "FieldToMatch": {
                            "SingleHeader": {
                              "Name": "user-agent"
                            }
                          },
                          "TextTransformations": [
                            {
                              "Priority": 1,
                              "Type": "LOWERCASE"
                            }
                          ]
                        }
                      }
                    ]
                  }
                }
              }
            }
          ]
        }
      }
    }
  },
  "Action": {
    "Block": {}
  },
  "VisibilityConfig": {
    "SampledRequestsEnabled": true,
    "CloudWatchMetricsEnabled": true,
    "MetricName": "tf-jptest-login"
  }
}

ghost avatar Oct 09 '20 19:10 ghost

Some more information: When attempting to import the valid json rule using terraform import I get the error message: Error: Error setting rule: Invalid address to set: []string{"rule", "0", "statement", "0", "rate_based_statement", "0", "scope_down_statement", "0", "and_statement", "0", "statement", "1", "not_statement", "0", "statement", "0", "or_statement"}

jpatallah avatar Oct 10 '20 21:10 jpatallah

Hi @jpatallah, thank you for creating this issue! While we do support up to 4 levels of nested statements at the moment, the not_statement in the example provided only supports 1 of byte_match_statement, geo_match_statement, ip_set_reference_statement, regex_pattern_set_reference_statement, size_constraint_statement, sqli_match_statement, or xss_match_statement as these provide the last level of nesting and do not support any further nested statements like an or_statement; thus the behavior you're experiencing is expected. Looking at our documentation, I believe we can clarify the not_statement section as it currently doesn't make note of this. Please note, adding support for a use-case like yours in the provider is currently blocked by a performance bug in terraform core (https://github.com/hashicorp/terraform/issues/25889).

anGie44 avatar Oct 13 '20 21:10 anGie44

@anGie44 Just got bitten with the not_statement issue as well. But it looks like the performance bug you mentioned has since been patched and merged. What would be the way forwards to get the not_statement working with deeper nestings?

FnTm avatar Oct 21 '20 11:10 FnTm

Just hit this as well. Did anyone find a workaround for this?

ghost avatar Jan 04 '21 14:01 ghost

This just removed all our scope down rules. 👎🏼

estahn avatar May 12 '21 06:05 estahn

We seem to be hitting this same issue. Are there any work arounds for this currently?

strantalis avatar Mar 03 '22 20:03 strantalis

Even though the performance issue https://github.com/hashicorp/terraform/issues/25889 was closed by https://github.com/hashicorp/terraform/pull/26577 (as already mentioned in https://github.com/hashicorp/terraform-provider-aws/issues/15580#issuecomment-713507480), increasing the ruleGroupRootStatementSchemaLevel constant from 3 to 5 still heavily slows acceptance tests down (approximately x7):

# TF_ACC=1 time go test ./internal/service/wafv2 -v -count 1 -parallel 20  -run=TestAccWAFV2RuleGroup_RateBased_maxNested -timeout 10m
=== RUN   TestAccWAFV2RuleGroup_RateBased_maxNested
=== PAUSE TestAccWAFV2RuleGroup_RateBased_maxNested
=== CONT  TestAccWAFV2RuleGroup_RateBased_maxNested
--- PASS: TestAccWAFV2RuleGroup_RateBased_maxNested (43.08s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/wafv2      43.185s
52.63user 3.57system 0:52.36elapsed 107%CPU (0avgtext+0avgdata 2973044maxresident)k
8inputs+580968outputs (72503major+1247135minor)pagefaults 0swaps

# sed -i "s/ruleGroupRootStatementSchemaLevel = 3/ruleGroupRootStatementSchemaLevel = 5/" internal/service/wafv2/consts.go

# git diff
diff --git a/internal/service/wafv2/consts.go b/internal/service/wafv2/consts.go
index 8a8e5ea9d8..53068648cb 100644
--- a/internal/service/wafv2/consts.go
+++ b/internal/service/wafv2/consts.go
@@ -1,6 +1,6 @@
 package wafv2

 const (
-       ruleGroupRootStatementSchemaLevel = 3
+       ruleGroupRootStatementSchemaLevel = 5
        webACLRootStatementSchemaLevel    = 3
 )

# TF_ACC=1 time go test ./internal/service/wafv2 -v -count 1 -parallel 20  -run=TestAccWAFV2RuleGroup_RateBased_maxNested -timeout 10m
=== RUN   TestAccWAFV2RuleGroup_RateBased_maxNested
=== PAUSE TestAccWAFV2RuleGroup_RateBased_maxNested
=== CONT  TestAccWAFV2RuleGroup_RateBased_maxNested
--- PASS: TestAccWAFV2RuleGroup_RateBased_maxNested (298.02s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/wafv2      298.340s
289.03user 10.65system 5:22.94elapsed 92%CPU (0avgtext+0avgdata 2939132maxresident)k
304inputs+588944outputs (72506major+3856847minor)pagefaults 0swaps

Edit: increasing the ruleGroupRootStatementSchemaLevel constant from 3 to 4 still slows acceptance tests down but way less (approximately x1.5):

# sed -i "s/ruleGroupRootStatementSchemaLevel = [0-9]/ruleGroupRootStatementSchemaLevel = 4/" internal/service/wafv2/consts.go

# git diff
diff --git a/internal/service/wafv2/consts.go b/internal/service/wafv2/consts.go
index 8a8e5ea9d8..f10af2e535 100644
--- a/internal/service/wafv2/consts.go
+++ b/internal/service/wafv2/consts.go
@@ -1,6 +1,6 @@
 package wafv2

 const (
-       ruleGroupRootStatementSchemaLevel = 3
+       ruleGroupRootStatementSchemaLevel = 4
        webACLRootStatementSchemaLevel    = 3
 )

# TF_ACC=1 time go test ./internal/service/wafv2 -v -count 1 -parallel 20  -run=TestAccWAFV2RuleGroup_RateBased_maxNested -timeout 10m
=== RUN   TestAccWAFV2RuleGroup_RateBased_maxNested
=== PAUSE TestAccWAFV2RuleGroup_RateBased_maxNested
=== CONT  TestAccWAFV2RuleGroup_RateBased_maxNested
--- PASS: TestAccWAFV2RuleGroup_RateBased_maxNested (65.68s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/wafv2      65.839s
107.60user 6.18system 1:23.19elapsed 136%CPU (0avgtext+0avgdata 2965876maxresident)k
0inputs+604744outputs (72493major+2641879minor)pagefaults 0swaps

A current work-around is to use the aws_cloudformation_stack resource, see:

  • https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/cloudformation_stack
  • https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-wafv2-rulegroup-statement.html

pdecat avatar Nov 15 '22 20:11 pdecat

Until the performance issue with such recursive schemas is actually resolved in terraform core, and the limit can be raised or completely removed, I see two options moving forward:

Allowing configuring this limit at the provider level which, even if slower, may be acceptable for some, e.g.:

# NOT WORKING
provider "aws" {
  region = "us-east-1"
  
  max_wafv2_rule_group_statements_depth = 4 # <--- THIS IS NOT IMPLEMENTED
}

Allowing the definition of the rule's statement in plain JSON, e.g.:

# NOT WORKING
resource "aws_wafv2_rule_group" "example" {
  name     = "example-rule"
  scope    = "REGIONAL"
  capacity = 2

  rule {
    name     = "rule-1"
    priority = 1

    action {
      allow {}
    }

    statement_json = jsonencode({ # <--- THIS IS NOT IMPLEMENTED
      geo_match_statement {
        country_codes = ["US", "NL"]
      }
    })

    visibility_config {
      cloudwatch_metrics_enabled = false
      metric_name                = "friendly-rule-metric-name"
      sampled_requests_enabled   = false
    }
  }

  visibility_config {
    cloudwatch_metrics_enabled = false
    metric_name                = "friendly-metric-name"
    sampled_requests_enabled   = false
  }
}

Note: statement and statement_json would be mutually exclusive.

Work-around

Finally, as mentioned in my previous comment, a current work-around is to use the aws_cloudformation_stack resource, see:

  • https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/cloudformation_stack
  • https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-wafv2-rulegroup-statement.html

pdecat avatar Nov 15 '22 20:11 pdecat

Would highly support a solution that allows use of the aws_wafv2_web_acl resource instead of an aws_cloudformation_stack. Either solution proposed sounds fine to me. I would happily accept a slower apply for a functional resource, but if it's preferable not to make the change on provider level, then statement_json seems fine to me too.

conorevans avatar Nov 29 '22 16:11 conorevans