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

WAF aws_waf_rule resource does not allow underscores in metric_name #12080

Closed
scalp42 opened this issue Feb 19, 2020 · 9 comments
Closed

WAF aws_waf_rule resource does not allow underscores in metric_name #12080

scalp42 opened this issue Feb 19, 2020 · 9 comments
Labels
service/waf Issues and PRs that pertain to the waf service.

Comments

@scalp42
Copy link
Contributor

scalp42 commented Feb 19, 2020

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 #8197.

@ghost ghost added the service/waf Issues and PRs that pertain to the waf service. label Feb 19, 2020
@github-actions github-actions bot added the needs-triage Waiting for first response or review from a maintainer. label Feb 19, 2020
@Y-Tian
Copy link

Y-Tian commented Feb 19, 2020

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

@ewbankkit
Copy link
Contributor

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:

A friendly name or description for the metrics for this Rule. The name can contain only alphanumeric characters (A-Z, a-z, 0-9), with maximum length 128 and minimum length one. It can't contain whitespace or metric names reserved for AWS WAF, including "All" and "Default_Action." You can't change the name of the metric after you create the Rule.

A friendly name or description for the metrics for this Rule. The name can contain only alphanumeric characters (A-Z, a-z, 0-9), with maximum length 128 and minimum length one. It can't contain whitespace or metric names reserved for AWS WAF, including "All" and "Default_Action." You can't change the name of the metric after you create the Rule.

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

#11176 tracks implementing the WAFv2 Web ACL resource.

@scalp42
Copy link
Contributor Author

scalp42 commented Feb 19, 2020

@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
Copy link

Y-Tian commented Feb 19, 2020

@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
Copy link
Contributor

ewbankkit commented Feb 19, 2020

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

  • aws_waf_rate_based_rule - ValidateFunc: validateWafMetricName
  • aws_waf_rule - ValidateFunc: validateWafMetricName
  • aws_waf_rule_group - ValidateFunc: validateWafMetricName
  • aws_waf_web_acl - ValidateFunc: validateWafMetricName
  • aws_wafregional_rate_based_rule - ValidateFunc: validateWafMetricName
  • aws_wafregional_rule - ValidateFunc: nil
  • aws_wafregional_rule_group - ValidateFunc: validateWafMetricName
  • aws_wafregional_web_acl - ValidateFunc: nil

@ewbankkit
Copy link
Contributor

ewbankkit commented Feb 19, 2020

@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
Copy link
Contributor Author

scalp42 commented Feb 19, 2020

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

@ewbankkit
Copy link
Contributor

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

@scalp42 scalp42 closed this as completed Feb 19, 2020
@ghost
Copy link

ghost commented Mar 27, 2020

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!

@ghost ghost locked and limited conversation to collaborators Mar 27, 2020
@breathingdust breathingdust removed the needs-triage Waiting for first response or review from a maintainer. label Sep 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
service/waf Issues and PRs that pertain to the waf service.
Projects
None yet
4 participants