-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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: Add support for custom request and responses to rules #19415
WAFv2: Add support for custom request and responses to rules #19415
Conversation
…/block/count actions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Welcome @andyalm 👋
It looks like this is your first Pull Request submission to the Terraform AWS Provider! If you haven’t already done so please make sure you have checked out our CONTRIBUTING guide and FAQ to make sure your contribution is adhering to best practice and has all the necessary elements in place for a successful approval.
Also take a look at our FAQ which details how we prioritize Pull Requests for inclusion.
Thanks again, and welcome to the community! 😃
…onse and got them passing.
8f8c69c
to
1a706da
Compare
I've removed the WIP as I believe I have gone through all of the PR checklist items. This is ready for a full review. |
@anGie44 I could use some help understanding these linting errors. |
Hi @andyalm , sure thing! The linter check is using One quick note on this PR: since the custom request/response attributes being added here are also supported in the RuleGroup resource, I would just update the title to reflect the affected resource and if interested, an additional PR can build on this one (to re-use the shared schemas) to add support w/in the RuleGroup. |
@anGie44 I had not noticed that RuleGroup also supports this, but of course, it makes sense. I would be happy to add that to this PR if you prefer (with added tests). Or would you prefer that be handled as a separate PR? |
@andyalm , yep let's add that to this PR . I was just skimming over the changes, and since the |
Great. I'll get working on that and will expand my acceptance test run to include all Wafv2 tests, not just the WebACL tests. |
Awesome thank you @andyalm ! I would definitely add tests similar to those you've added in the webACL resource to the |
…actions in WAFv2 RuleSet resources.
…ade them `TypeSet` instead of `TypeList`
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @andyalm , overall this PR looks great! Just some updates needed around the conventions around the flatten/expand methods and a reminder to update the .changelog/19415.txt
file with a reference to the wafv2_rule_group
resource and an update to the resource documentation files to reflect the new definitions for the actions updated in the schema.
Output of acceptance tests:
--- PASS: TestAccAwsWafv2RuleGroup_RegexPatternSetReferenceStatement (53.01s)
--- PASS: TestAccAwsWafv2RuleGroup_Minimal (58.29s)
--- PASS: TestAccAwsWafv2RuleGroup_IpSetReferenceStatement (60.71s)
--- PASS: TestAccAwsWafv2RuleGroup_RuleAction_CustomResponse (77.46s)
--- PASS: TestAccAwsWafv2RuleGroup_basic (84.12s)
--- PASS: TestAccAwsWafv2RuleGroup_Disappears (84.10s)
--- PASS: TestAccAwsWafv2RuleGroup_ChangeNameForceNew (118.01s)
--- PASS: TestAccAwsWafv2RuleGroup_SizeConstraintStatement (128.95s)
--- PASS: TestAccAwsWafv2RuleGroup_GeoMatchStatement (132.14s)
--- PASS: TestAccAwsWafv2RuleGroup_ByteMatchStatement (136.60s)
--- PASS: TestAccAwsWafv2RuleGroup_ChangeCapacityForceNew (144.82s)
--- PASS: TestAccAwsWafv2RuleGroup_ChangeMetricNameForceNew (150.11s)
--- PASS: TestAccAwsWafv2RuleGroup_updateRule (158.37s)
--- PASS: TestAccAwsWafv2RuleGroup_RuleAction (165.79s)
--- PASS: TestAccAwsWafv2RuleGroup_RuleAction_CustomRequestHandling (171.87s)
--- PASS: TestAccAwsWafv2RuleGroup_updateRuleProperties (181.21s)
--- PASS: TestAccAwsWafv2RuleGroup_SqliMatchStatement (130.96s)
--- PASS: TestAccAwsWafv2RuleGroup_XssMatchStatement (128.51s)
--- PASS: TestAccAwsWafv2RuleGroup_GeoMatchStatement_ForwardedIPConfig (196.08s)
--- PASS: TestAccAwsWafv2RuleGroup_LogicalRuleStatements (212.66s)
--- PASS: TestAccAwsWafv2WebACLAssociation_basic (137.51s)
--- PASS: TestAccAwsWafv2RuleGroup_Tags (168.95s)
--- PASS: TestAccAwsWafv2WebACLLoggingConfiguration_basic (157.12s)
--- PASS: TestAccAwsWafv2WebACL_basic (75.85s)
--- PASS: TestAccAwsWafv2WebACLAssociation_Disappears (191.03s)
--- PASS: TestAccAwsWafv2WebACLLoggingConfiguration_disappears (154.17s)
--- PASS: TestAccAwsWafv2WebACL_Disappears (82.90s)
--- PASS: TestAccAwsWafv2RuleGroup_IpSetReferenceStatement_IPSetForwardedIPConfig (328.72s)
--- PASS: TestAccAwsWafv2WebACLLoggingConfiguration_updateMethodRedactedField (226.23s)
--- PASS: TestAccAwsWafv2WebACLLoggingConfiguration_emptyRedactedFields (184.19s)
--- PASS: TestAccAwsWafv2WebACLLoggingConfiguration_disappears_WebAcl (175.16s)
--- PASS: TestAccAwsWafv2WebACLLoggingConfiguration_updateUriPathRedactedField (222.89s)
--- PASS: TestAccAwsWafv2WebACL_Minimal (83.65s)
--- PASS: TestAccAwsWafv2WebACLLoggingConfiguration_updateQueryStringRedactedField (247.68s)
--- PASS: TestAccAwsWafv2WebACL_updateRule (167.65s)
--- PASS: TestAccAwsWafv2WebACL_ChangeNameForceNew (165.19s)
--- PASS: TestAccAwsWafv2RuleGroup_ByteMatchStatement_FieldToMatch (418.92s)
--- PASS: TestAccAwsWafv2WebACLLoggingConfiguration_updateSingleHeaderRedactedField (310.45s)
--- PASS: TestAccAwsWafv2WebACL_ManagedRuleGroupStatement (174.56s)
--- PASS: TestAccAwsWafv2WebACLLoggingConfiguration_updateMultipleRedactedFields (306.95s)
--- PASS: TestAccAwsWafv2WebACLLoggingConfiguration_updateEmptyRedactedFields (275.65s)
--- PASS: TestAccAwsWafv2WebACL_IPSetReferenceStatement (101.33s)
--- PASS: TestAccAwsWafv2WebACLLoggingConfiguration_changeLogDestinationConfigsForceNew (315.02s)
--- PASS: TestAccAwsWafv2WebACL_UpdateRuleProperties (264.79s)
--- PASS: TestAccAwsWafv2WebACL_RateBasedStatement (163.90s)
--- PASS: TestAccAwsWafv2WebACL_GeoMatchStatement (161.47s)
--- PASS: TestAccAwsWafv2WebACL_MaxNestedRateBasedStatements (93.44s)
--- PASS: TestAccAwsWafv2WebACL_GeoMatchStatement_ForwardedIPConfig (161.96s)
--- PASS: TestAccAwsWafv2WebACLLoggingConfiguration_changeResourceARNForceNew (344.33s)
--- PASS: TestAccAwsWafv2WebACL_MaxNestedOperatorStatements (80.04s)
--- PASS: TestAccAwsWafv2WebACL_CustomRequestHandling (143.87s)
--- PASS: TestAccAwsWafv2WebACL_RateBasedStatement_ForwardedIPConfig (147.28s)
--- PASS: TestAccAwsWafv2WebACL_RuleGroupReferenceStatement (149.40s)
--- PASS: TestAccAwsWafv2WebACL_CustomResponse (130.19s)
--- PASS: TestAccAwsWafv2WebACL_Tags (141.11s)
--- PASS: TestAccAwsWafv2WebACL_IPSetReferenceStatement_IPSetForwardedIPConfig (175.28s)
--- PASS: TestAccAwsWafv2WebACLLoggingConfiguration_loggingFilter (347.75s)
Co-authored-by: angie pinilla <[email protected]>
f913233
to
eaa2ae7
Compare
…action` section. Per AWS docs, they are not supported. (https://docs.aws.amazon.com/waf/latest/developerguide/waf-custom-request-response.html)
… docs for `wafv2_web_acl` and `wafv2_rule_group` resources.
…nt` actions to always at least return an empty object. By the time these `expand` functions are called, we know we at least want an empty object to indicate which action we are to take.
@anGie44 I have made all of the requested changes, with the exception of the |
@anGie44 that test failure doesn't seem related to my changes. Any idea how to clear that up? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes looks great @andyalm 👍 Just a small suggestion to the changelog entries as the WebACL resource has both a default_action and rule actions. Looks like the go test
CI failure might be related to a transient error, i'll trigger a re-run but it shouldn't hold up merging this in. Update: CI checks ✅
Output of acceptance tests:
--- PASS: TestAccAwsWafv2RuleGroup_RegexPatternSetReferenceStatement (54.72s)
--- PASS: TestAccAwsWafv2RuleGroup_Disappears (57.48s)
--- PASS: TestAccAwsWafv2RuleGroup_IpSetReferenceStatement (65.18s)
--- PASS: TestAccAwsWafv2RuleGroup_RuleAction_CustomResponse (76.12s)
--- PASS: TestAccAwsWafv2RuleGroup_ChangeMetricNameForceNew (98.84s)
--- PASS: TestAccAwsWafv2RuleGroup_ChangeNameForceNew (99.15s)
--- PASS: TestAccAwsWafv2RuleGroup_Minimal (100.53s)
--- PASS: TestAccAwsWafv2RuleGroup_basic (107.20s)
--- PASS: TestAccAwsWafv2RuleGroup_updateRule (121.84s)
--- PASS: TestAccAwsWafv2RuleGroup_GeoMatchStatement (136.85s)
--- PASS: TestAccAwsWafv2RuleGroup_RuleAction_CustomRequestHandling (137.20s)
--- PASS: TestAccAwsWafv2RuleGroup_ChangeCapacityForceNew (145.23s)
--- PASS: TestAccAwsWafv2RuleGroup_updateRuleProperties (165.01s)
--- PASS: TestAccAwsWafv2RuleGroup_SizeConstraintStatement (168.60s)
--- PASS: TestAccAwsWafv2RuleGroup_SqliMatchStatement (121.14s)
--- PASS: TestAccAwsWafv2RuleGroup_GeoMatchStatement_ForwardedIPConfig (177.87s)
--- PASS: TestAccAwsWafv2RuleGroup_ByteMatchStatement (181.21s)
--- PASS: TestAccAwsWafv2RuleGroup_XssMatchStatement (122.03s)
--- PASS: TestAccAwsWafv2RuleGroup_RuleAction (201.70s)
--- PASS: TestAccAwsWafv2WebACLAssociation_basic (129.42s)
--- PASS: TestAccAwsWafv2RuleGroup_Tags (161.55s)
--- PASS: TestAccAwsWafv2RuleGroup_LogicalRuleStatements (224.04s)
--- PASS: TestAccAwsWafv2RuleGroup_IpSetReferenceStatement_IPSetForwardedIPConfig (237.55s)
--- PASS: TestAccAwsWafv2WebACLLoggingConfiguration_basic (147.04s)
--- PASS: TestAccAwsWafv2WebACL_basic (91.68s)
--- PASS: TestAccAwsWafv2WebACLAssociation_Disappears (202.23s)
--- PASS: TestAccAwsWafv2WebACL_Disappears (81.10s)
--- PASS: TestAccAwsWafv2WebACLLoggingConfiguration_disappears (174.67s)
--- PASS: TestAccAwsWafv2WebACLLoggingConfiguration_disappears_WebAcl (163.66s)
--- PASS: TestAccAwsWafv2WebACLLoggingConfiguration_updateUriPathRedactedField (212.68s)
--- PASS: TestAccAwsWafv2WebACLLoggingConfiguration_emptyRedactedFields (173.43s)
--- PASS: TestAccAwsWafv2WebACLLoggingConfiguration_updateMethodRedactedField (247.38s)
--- PASS: TestAccAwsWafv2WebACLLoggingConfiguration_updateQueryStringRedactedField (247.42s)
--- PASS: TestAccAwsWafv2WebACL_Minimal (75.97s)
--- PASS: TestAccAwsWafv2WebACL_updateRule (168.06s)
--- PASS: TestAccAwsWafv2WebACL_ChangeNameForceNew (154.95s)
--- PASS: TestAccAwsWafv2WebACLLoggingConfiguration_updateSingleHeaderRedactedField (298.01s)
--- PASS: TestAccAwsWafv2WebACLLoggingConfiguration_updateMultipleRedactedFields (271.81s)
--- PASS: TestAccAwsWafv2WebACL_ManagedRuleGroupStatement (170.59s)
--- PASS: TestAccAwsWafv2WebACLLoggingConfiguration_updateEmptyRedactedFields (245.80s)
--- PASS: TestAccAwsWafv2RuleGroup_ByteMatchStatement_FieldToMatch (431.13s)
--- PASS: TestAccAwsWafv2WebACL_IPSetReferenceStatement (93.64s)
--- PASS: TestAccAwsWafv2WebACL_RateBasedStatement (155.82s)
--- PASS: TestAccAwsWafv2WebACL_MaxNestedRateBasedStatements (91.03s)
--- PASS: TestAccAwsWafv2WebACL_GeoMatchStatement (152.83s)
--- PASS: TestAccAwsWafv2WebACL_UpdateRuleProperties (255.10s)
--- PASS: TestAccAwsWafv2WebACL_MaxNestedOperatorStatements (81.98s)
--- PASS: TestAccAwsWafv2WebACL_GeoMatchStatement_ForwardedIPConfig (144.05s)
--- PASS: TestAccAwsWafv2WebACL_RateBasedStatement_ForwardedIPConfig (139.06s)
--- PASS: TestAccAwsWafv2WebACL_CustomResponse (124.67s)
--- PASS: TestAccAwsWafv2WebACL_RuleGroupReferenceStatement (141.35s)
--- PASS: TestAccAwsWafv2WebACL_CustomRequestHandling (126.72s)
--- PASS: TestAccAwsWafv2WebACL_Tags (133.40s)
--- PASS: TestAccAwsWafv2WebACL_IPSetReferenceStatement_IPSetForwardedIPConfig (168.53s)
--- PASS: TestAccAwsWafv2WebACLLoggingConfiguration_changeResourceARNForceNew (389.20s)
--- PASS: TestAccAwsWafv2WebACLLoggingConfiguration_changeLogDestinationConfigsForceNew (376.71s)
--- PASS: TestAccAwsWafv2WebACLLoggingConfiguration_loggingFilter (373.59s)
Co-authored-by: angie pinilla <[email protected]>
Thanks for walking me through the process @anGie44 ! That was a really great experience. |
Happy to hear it @andyalm ! Thanks again for your contributions, hope to see more soon 😄 |
@andyalm thanks so much for doing this! |
This has been released in version 3.43.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks! |
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. |
This adds support for
custom_request_handling
on allow and count actions, andcustom_response
on block actions.Notes:
custom_request_handling
and one forcustom_response
.Community Note
Closes #18754
Output from acceptance testing: