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.

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

wafv2 rate_based_rule with nested scopedown and/or not working #15580

Open
ghost opened this issue Oct 9, 2020 · 10 comments
Open

wafv2 rate_based_rule with nested scopedown and/or not working #15580

ghost opened this issue Oct 9, 2020 · 10 comments
Labels
documentation Introduces or discusses updates to documentation. enhancement Requests to existing resources that expand the functionality or scope. service/wafv2 Issues and PRs that pertain to the wafv2 service.

Comments

@ghost
Copy link

ghost commented Oct 9, 2020

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 ghost added the service/wafv2 Issues and PRs that pertain to the wafv2 service. label Oct 9, 2020
@github-actions github-actions bot added the needs-triage Waiting for first response or review from a maintainer. label Oct 9, 2020
@jpatallah
Copy link

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"}

@anGie44 anGie44 added enhancement Requests to existing resources that expand the functionality or scope. and removed needs-triage Waiting for first response or review from a maintainer. labels Oct 13, 2020
@anGie44
Copy link
Contributor

anGie44 commented Oct 13, 2020

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 (hashicorp/terraform#25889).

@anGie44 anGie44 added the documentation Introduces or discusses updates to documentation. label Oct 13, 2020
@FnTm
Copy link
Contributor

FnTm commented Oct 21, 2020

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

@ghost
Copy link

ghost commented Jan 4, 2021

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

@estahn
Copy link

estahn commented May 12, 2021

This just removed all our scope down rules. 👎🏼

@strantalis
Copy link

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

@pdecat
Copy link
Contributor

pdecat commented Nov 15, 2022

Even though the performance issue hashicorp/terraform#25889 was closed by hashicorp/terraform#26577 (as already mentioned in #15580 (comment)), 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:

@pdecat
Copy link
Contributor

pdecat commented Nov 15, 2022

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:

@conorevans
Copy link

conorevans commented Nov 29, 2022

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.

@calizarr
Copy link

calizarr commented Oct 1, 2024

I have just run into this issue as well. Has there been any movement forwards towards including a direct JSON string or increasing the rule group statement depth?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Introduces or discusses updates to documentation. enhancement Requests to existing resources that expand the functionality or scope. service/wafv2 Issues and PRs that pertain to the wafv2 service.
Projects
None yet
Development

No branches or pull requests

8 participants