search

hashicorp / terraform-provider-aws

The AWS Provider enables Terraform to manage AWS resources.
https://registry.terraform.io/providers/hashicorp/aws
Mozilla Public License 2.0
9.82k stars 9.16k forks source link

WAF aws_waf_rule resource does not allow underscores in metric_name #12080

Closed scalp42 closed 4 years ago

scalp42 commented 4 years ago

Assuming the following:

resource "aws_waf_rule" "filter_country_vietnam" {
  depends_on  = ["aws_waf_geo_match_set.vietnam"]
  name        = "filter_country_vietnam"
  metric_name = "filter_country_vietnam"

  predicates {
    data_id = aws_waf_geo_match_set.vietnam.id
    negated = false
    type    = "GeoMatch"
  }
}

The valid regex is: ^[\w#:\.\-/]+$

Screen Shot 2020-02-18 at 16 34 08

Tested by hand and we can pass underscrores in the rule (gets passed to metric_name):

Screen Shot 2020-02-18 at 16 27 56

I also tested with the JSON editor:

Screen Shot 2020-02-18 at 16 33 19

Related to https://github.com/terraform-providers/terraform-provider-aws/issues/8197.

Y-Tian commented 4 years ago

+1. This is very useful, can we get someone to look over this?

ewbankkit commented 4 years ago

The aws_waf_rule resource (but interestingly not the aws_wafregional_rule) resource validates the metric_name attribute with validateWafMetricName():

https://github.com/terraform-providers/terraform-provider-aws/blob/768341474a743c3db30610c6b434fed598d98cd8/aws/validators.go#L1781-L1789

The applicable API references are:

I think that the AWS Console is defaulting to the new WAF v2 API:

MetricName A friendly name of the CloudWatch metric. The name can contain only alphanumeric characters (A-Z, a-z, 0-9), with length from one to 128 characters. It can't contain whitespace or metric names reserved for AWS WAF, for example "All" and "Default_Action." You can't change a MetricName after you create a VisibilityConfig.

Type: String

Length Constraints: Minimum length of 1. Maximum length of 255.

Pattern: ^[\w#:.-/]+$

Required: Yes

https://github.com/terraform-providers/terraform-provider-aws/issues/11176 tracks implementing the WAFv2 Web ACL resource.

scalp42 commented 4 years ago

@ewbankkit thanks for the feedback but it doesn't seem to match what I see:

Error: Only alphanumeric characters allowed in "metric_name": "filter_countries"

  on waf_regional/groups.tf line 1, in resource "aws_wafregional_rule_group" "filter_countries":
   1: resource "aws_wafregional_rule_group" "filter_countries" {

aws_wafregional_rule_group does appear to still check for it for example while aws_waf_rule is fine with underscores.

Y-Tian commented 4 years ago

@ewbankkit I opened a PR that fixes this the way you described it above. Could you please take a look and let me know if there's any other work on this? Cheers

ewbankkit commented 4 years ago

@scalp42 It's all rather inconsistent right now. For WAF resources with a metric_name attribute:

ewbankkit commented 4 years ago

@scalp42 If I try creating an aws_wafregional_rate_based_rule resource with metric_name = "filter_country_vietnam", incorporating the change from @Y-Tian, then the API returns an exception:

Error creating WAF Regional Rate Based Rule (): WAFDisallowedNameException: The specified name is not permitted.

You should be able to click on Switch to AWS WAF Classic in the AWS WAF section of the WAF & Shield console to verify that in classic mode underscores aren't allowed in metrics names. Please upvote the linked WAF v2 issue 😄.

scalp42 commented 4 years ago

Gotcha, this is pretty confusing. Thanks for taking the time @ewbankkit we'll focus on #11176 then.

ewbankkit commented 4 years ago

@scalp42 Can you please close this issue if you're happy with continuing any discussion in #11176? Thanks.

ghost commented 4 years ago

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!