Add tags to WAF WebACL, Rule & Rule Group Resources #10408

DrFaust92 · 2019-10-07T14:43:52Z

Community Note

Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
Please do not leave "+1" comments, they generate extra noise for pull request followers and do not help prioritize the request

Relates to #9289

Release note for CHANGELOG:

- resource/resource_aws_waf_rule: Add `tags` argument.
- resource/resource_aws_waf_web_acl: Add `tags` argument.
- resource/resource_aws_waf_rule_group: Add `tags` argument.

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccAWSWaFWebAcl_'

=== RUN   TestAccAWSWafWebAcl_basic
=== PAUSE TestAccAWSWafWebAcl_basic
=== CONT  TestAccAWSWafWebAcl_basic
--- PASS: TestAccAWSWafWebAcl_basic (54.26s)
=== RUN   TestAccAWSWafWebAcl_changeNameForceNew
=== PAUSE TestAccAWSWafWebAcl_changeNameForceNew
=== CONT  TestAccAWSWafWebAcl_changeNameForceNew
--- PASS: TestAccAWSWafWebAcl_changeNameForceNew (76.04s)
=== RUN   TestAccAWSWafWebAcl_DefaultAction
=== PAUSE TestAccAWSWafWebAcl_DefaultAction
=== CONT  TestAccAWSWafWebAcl_DefaultAction
--- PASS: TestAccAWSWafWebAcl_DefaultAction (95.53s)
=== RUN   TestAccAWSWafWebAcl_Rules
=== PAUSE TestAccAWSWafWebAcl_Rules
=== CONT  TestAccAWSWafWebAcl_Rules
--- FAIL: TestAccAWSWafWebAcl_Rules (145.75s)
    testing.go:569: Step 2 error: errors during apply:
        
        Error: Error deleting WAF Rule: WAFReferencedItemException: This entity is still referenced by other entities.
        	status code: 400, request id: 04e760a2-ec28-11e9-92e8-8baa677d2d68
        
        
=== RUN   TestAccAWSWafWebAcl_LoggingConfiguration
=== PAUSE TestAccAWSWafWebAcl_LoggingConfiguration
=== CONT  TestAccAWSWafWebAcl_LoggingConfiguration
--- PASS: TestAccAWSWafWebAcl_LoggingConfiguration (197.73s)
=== RUN   TestAccAWSWafWebAcl_disappears
=== PAUSE TestAccAWSWafWebAcl_disappears
=== CONT  TestAccAWSWafWebAcl_disappears
--- PASS: TestAccAWSWafWebAcl_disappears (40.43s)
=== RUN   TestAccAWSWafWebAcl_Tags
=== PAUSE TestAccAWSWafWebAcl_Tags
=== CONT  TestAccAWSWafWebAcl_Tags
--- PASS: TestAccAWSWafWebAcl_Tags (105.80s)
...

$ make testacc TESTARGS='-run=TestAccAWSWaFRuleGroup_'

=== RUN   TestAccAWSWafRuleGroup_basic
=== PAUSE TestAccAWSWafRuleGroup_basic
=== CONT  TestAccAWSWafRuleGroup_basic
--- PASS: TestAccAWSWafRuleGroup_basic (67.32s)
=== RUN   TestAccAWSWafRuleGroup_changeNameForceNew
=== PAUSE TestAccAWSWafRuleGroup_changeNameForceNew
=== CONT  TestAccAWSWafRuleGroup_changeNameForceNew
--- PASS: TestAccAWSWafRuleGroup_changeNameForceNew (89.21s)
=== RUN   TestAccAWSWafRuleGroup_disappears
=== PAUSE TestAccAWSWafRuleGroup_disappears
=== CONT  TestAccAWSWafRuleGroup_disappears
--- PASS: TestAccAWSWafRuleGroup_disappears (55.54s)
=== RUN   TestAccAWSWafRuleGroup_changeActivatedRules
=== PAUSE TestAccAWSWafRuleGroup_changeActivatedRules
=== CONT  TestAccAWSWafRuleGroup_changeActivatedRules
--- PASS: TestAccAWSWafRuleGroup_changeActivatedRules (96.20s)
=== RUN   TestAccAWSWafRuleGroup_Tags
=== PAUSE TestAccAWSWafRuleGroup_Tags
=== CONT  TestAccAWSWafRuleGroup_Tags
--- PASS: TestAccAWSWafRuleGroup_Tags (96.91s)
=== RUN   TestAccAWSWafRuleGroup_noActivatedRules
=== PAUSE TestAccAWSWafRuleGroup_noActivatedRules
=== CONT  TestAccAWSWafRuleGroup_noActivatedRules
--- PASS: TestAccAWSWafRuleGroup_noActivatedRules (38.98s)

$ make testacc TESTARGS='-run=TestAccAWSWaFRule_'

=== RUN   TestAccAWSWafRule_basic
=== PAUSE TestAccAWSWafRule_basic
=== CONT  TestAccAWSWafRule_basic
=== RUN   TestAccAWSWafRule_changeNameForceNew
=== PAUSE TestAccAWSWafRule_changeNameForceNew
=== RUN   TestAccAWSWafRule_disappears
=== PAUSE TestAccAWSWafRule_disappears
=== CONT  TestAccAWSWafRule_disappears
=== RUN   TestAccAWSWafRule_changePredicates
=== PAUSE TestAccAWSWafRule_changePredicates
=== RUN   TestAccAWSWafRule_geoMatchSetPredicate
=== PAUSE TestAccAWSWafRule_geoMatchSetPredicate
=== CONT  TestAccAWSWafRule_geoMatchSetPredicate
=== RUN   TestAccAWSWafRule_noPredicates
=== PAUSE TestAccAWSWafRule_noPredicates
=== CONT  TestAccAWSWafRule_noPredicates
--- PASS: TestAccAWSWafRule_noPredicates (37.86s)
=== RUN   TestAccAWSWafRule_Tags
=== PAUSE TestAccAWSWafRule_Tags
=== CONT  TestAccAWSWafRule_Tags
--- PASS: TestAccAWSWafRule_basic (65.84s)
=== CONT  TestAccAWSWafRule_changePredicates
=== CONT  TestAccAWSWafRule_changeNameForceNew
--- PASS: TestAccAWSWafRule_geoMatchSetPredicate (69.30s)
--- PASS: TestAccAWSWafRule_disappears (53.59s)
--- PASS: TestAccAWSWafRule_Tags (108.77s)
--- PASS: TestAccAWSWafRule_changePredicates (80.81s)
--- PASS: TestAccAWSWafRule_changeNameForceNew (89.17s)

merge changes

…der-aws into waf-tags

aws_waf_rate_based_rule aws_waf_rule aws_waf_rule_group and add create logic for these resources

aws_waf_rate_based_rule aws_waf_rule aws_waf_rule_group

aws_wafregional_rate_based_rule aws_wafregional_rule aws_wafregional_rule_group aws_wafregional_web_acl

bflad

Thanks again for submitting this, @DrFaust92 -- please reach out if you have any questions or do not have time to implement the feedback items. 😄

aws/tagsWAF.go

aws/resource_aws_waf_web_acl.go

aws/resource_aws_waf_web_acl_test.go

DrFaust92 · 2019-10-10T07:05:22Z

hey @bflad,
im new to golang and after the proposed change i get this message:

any idea what needs to change in the project settings? (i guess that is the issue and not the package itself)

bflad · 2019-10-10T14:32:30Z

@DrFaust92 hmm, since the internal package is within this Go module, it shouldn't be complaining. That seems like an issue with your editor. Here's some Go documentation on internal packages for reference.

Also please note that if these tag additions are done in separate PRs per-resource, we can merge them immediately once the resource code, acceptance testing, and documentation is ready for that one resource (rather than waiting for everything to be done). 😄

* aws_waf_rate_based_rule * aws_waf_rule_group * aws_waf_rule refactored tests a bit to not repeat resource name

DrFaust92 · 2019-10-11T13:09:46Z

@bflad,
TestAccAWSWafWebAcl_Rules test seems to fail with the following error:

 Error: Error deleting WAF Rule: WAFReferencedItemException: This entity is still referenced by other entities.
        	status code: 400, request id: 04e760a2-ec28-11e9-92e8-8baa677d2d68

this is unrelated to the change im adding as it fails on master as well.

…ed_rule aws_wafregional_rule aws_wafregional_rule_group aws_wafregional_web_acl" This reverts commit 34a3e53

bflad · 2019-10-11T15:24:29Z

@DrFaust92 thanks for bringing that up. Yeah, that one is indeed flakey/failing on master. You can ignore that in your testing. 👍

This reverts commit 00176bf

This reverts commit 0de5648

…af_rule_group" This reverts commit 4d3e23a

…aws_waf_rule aws_waf_rule_group" This reverts commit 84733a4

This reverts commit 4597385

DrFaust92 · 2019-10-11T17:15:33Z

Also please note that if these tag additions are done in separate PRs per-resource, we can merge them immediately once the resource code, acceptance testing, and documentation is ready for that one resource (rather than waiting for everything to be done). 😄

@bflad, i realised my mistake too late. descoping this to only a few resources and i'll open other PRs for other resources (or related tasks)

i think this is ready now. thanks for the support on this!

bflad

Looks great, @DrFaust92, thanks so much for your work here! 🚀

--- PASS: TestAccAWSWafRuleGroup_noActivatedRules (15.91s)
--- PASS: TestAccAWSWafRuleGroup_Tags (39.80s)
--- PASS: TestAccAWSWafRule_Tags (70.48s)
--- PASS: TestAccAWSWafRule_changeNameForceNew (74.99s)
--- PASS: TestAccAWSWafRule_noPredicates (76.48s)
--- PASS: TestAccAWSWafWebAcl_DefaultAction (91.12s)
--- PASS: TestAccAWSWafWebAcl_Tags (93.10s)
--- PASS: TestAccAWSWafRuleGroup_basic (108.74s)
--- PASS: TestAccAWSWafWebAcl_disappears (109.73s)
--- PASS: TestAccAWSWafWebAcl_LoggingConfiguration (127.80s)
--- PASS: TestAccAWSWafWebAcl_basic (129.29s)
--- PASS: TestAccAWSWafRule_basic (141.68s)
--- PASS: TestAccAWSWafRule_geoMatchSetPredicate (142.90s)
--- PASS: TestAccAWSWafRuleGroup_changeActivatedRules (151.86s)
--- PASS: TestAccAWSWafRule_changePredicates (161.65s)
--- PASS: TestAccAWSWafWebAcl_changeNameForceNew (174.97s)
--- PASS: TestAccAWSWafRuleGroup_changeNameForceNew (181.00s)
--- PASS: TestAccAWSWafRule_disappears (192.69s)
--- PASS: TestAccAWSWafRuleGroup_disappears (205.69s)

bflad · 2019-10-11T17:55:49Z

aws/resource_aws_waf_rule.go

@@ -109,6 +117,23 @@ func resourceAwsWafRuleRead(d *schema.ResourceData, meta interface{}) error {
 		predicates = append(predicates, predicate)
 	}

+	arn := arn.ARN{


Nit: For the future, it might be nice to expose the ARN as an attribute as well if folks need it for other reasons (and we can also use d.Get("arn").(string) in the Update function as well) 👍

In the resource code:

// Resource schema "arn": { Type: schema.TypeString, Computed: true, } // Read function (or Create function if needed in there first) arn := arn.ARN{/* ... */}.String() d.Set("arn", arn) // Update function can access via d.Get("arn").(string)

Acceptance testing:

// In the _basic test, one of the following // Depending on global/regional ARN and known resource value (Check) or regex (Match) testAccCheckResourceAttrGlobalARN(resourceName, "arn", "iam", fmt.Sprintf("policy/%s", rName)), testAccMatchResourceAttrGlobalARN(resourceName, "arn", "organizations", regexp.MustCompile(`organization/o-.+`)), testAccCheckResourceAttrRegionalARN(resourceName, "arn", "athena", fmt.Sprintf("workgroup/%s", rName)), testAccMatchResourceAttrRegionalARN(resourceName, "arn", "acm", regexp.MustCompile(`certificate/.+`)),

Resource documentation (attributes section):

* `arn` - Amazon Resource Name (ARN)

ghost · 2020-03-29T17:46:15Z

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!

add tags to waf web acl

6e005ce

DrFaust92 requested a review from a team October 7, 2019 14:43

ghost added size/M Managed by automation to categorize the size of a PR. service/waf Issues and PRs that pertain to the waf service. labels Oct 7, 2019

WAF Tags tests

7694241

ghost added size/L Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. and removed size/M Managed by automation to categorize the size of a PR. labels Oct 7, 2019

WAF Tags tests

5009e79

DrFaust92 closed this Oct 7, 2019

DrFaust92 reopened this Oct 7, 2019

DrFaust92 and others added 3 commits October 8, 2019 11:56

Merge pull request #1 from terraform-providers/master

31f9449

merge changes

waf tags condintion if tags attribute is not empty

55b08c2

Merge branch 'master' of https://github.com/DrFaust92/terraform-provi…

fdbf9f5

…der-aws into waf-tags

ghost added size/XL Managed by automation to categorize the size of a PR. and removed size/L Managed by automation to categorize the size of a PR. labels Oct 8, 2019

DrFaust92 and others added 3 commits October 8, 2019 12:28

fix dependency

e3c1e27

add tags to waf web acl resource doc

3ed4b01

waf tag test

036a91c

ghost added the documentation Introduces or discusses updates to documentation. label Oct 9, 2019

DrFaust92 added 3 commits October 9, 2019 21:18

add tags parms to:

4d3e23a

aws_waf_rate_based_rule aws_waf_rule aws_waf_rule_group and add create logic for these resources

add tags param to documentation for:

84733a4

aws_waf_rate_based_rule aws_waf_rule aws_waf_rule_group

add tags param to documentation for:

34a3e53

aws_wafregional_rate_based_rule aws_wafregional_rule aws_wafregional_rule_group aws_wafregional_web_acl

bflad suggested changes Oct 10, 2019

View reviewed changes

bflad added enhancement Requests to existing resources that expand the functionality or scope. waiting-response Maintainers are waiting on response from community or contributor. labels Oct 10, 2019

DrFaust92 added 3 commits October 10, 2019 09:42

add test to aws_waf_web_acl

94a3a9b

use keyvaluesmap package

0de5648

use keyvaluetags

00176bf

wrtie acceptence tests for:

4597385

* aws_waf_rate_based_rule * aws_waf_rule_group * aws_waf_rule refactored tests a bit to not repeat resource name

ghost added size/XL Managed by automation to categorize the size of a PR. and removed size/L Managed by automation to categorize the size of a PR. labels Oct 10, 2019

DrFaust92 added 2 commits October 10, 2019 19:11

remove trailing new lines

4103c55

remove trailing new lines

a6592c2

DrFaust92 added 2 commits October 11, 2019 16:15

Revert "add tags param to documentation for: aws_wafregional_rate_bas…

64fbf6d

…ed_rule aws_wafregional_rule aws_wafregional_rule_group aws_wafregional_web_acl" This reverts commit 34a3e53

tags from waf_rule

fbd5610

DrFaust92 added 7 commits October 11, 2019 19:51

tags from waf_rule_group

7e44d9e

Revert "use keyvaluetags"

23a696c

This reverts commit 00176bf

Revert "use keyvaluesmap package"

2840089

This reverts commit 0de5648

Revert "add tags parms to: aws_waf_rate_based_rule aws_waf_rule aws_w…

c605e7f

…af_rule_group" This reverts commit 4d3e23a

Revert "add tags param to documentation for: aws_waf_rate_based_rule …

d19b1f9

…aws_waf_rule aws_waf_rule_group" This reverts commit 84733a4

Revert "wrtie acceptence tests for:"

24bdfb4

This reverts commit 4597385

tags from waf_rule_group

8a9fb29

DrFaust92 changed the title ~~[WIP] Add tags to waf resources~~ Add tags to waf resources Oct 11, 2019

DrFaust92 changed the title ~~Add tags to waf resources~~ Add tags to WAF WebACL, Rule & Rule Group Resources Oct 11, 2019

bflad added this to the v2.33.0 milestone Oct 11, 2019

bflad approved these changes Oct 11, 2019

View reviewed changes

bflad merged commit d562f95 into hashicorp:master Oct 11, 2019

bflad added a commit that referenced this pull request Oct 11, 2019

Update CHANGELOG for #10408

cc2974d

DrFaust92 deleted the waf-tags branch October 20, 2019 20:57

This was referenced Nov 8, 2019

r/waf_rule- refactor to use keyvaluetags #10798

Merged

r/waf_rule_group - refactor to use keyvaluetags #10799

Merged

ewbankkit mentioned this pull request Nov 8, 2019

AWS WAF tagging #9289

Closed

ghost locked and limited conversation to collaborators Mar 29, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add tags to WAF WebACL, Rule & Rule Group Resources #10408

Add tags to WAF WebACL, Rule & Rule Group Resources #10408

DrFaust92 commented Oct 7, 2019 •

edited

Loading

bflad left a comment

DrFaust92 commented Oct 10, 2019

bflad commented Oct 10, 2019

DrFaust92 commented Oct 11, 2019

bflad commented Oct 11, 2019

DrFaust92 commented Oct 11, 2019

bflad left a comment

bflad Oct 11, 2019

ghost commented Mar 29, 2020

Add tags to WAF WebACL, Rule & Rule Group Resources #10408

Add tags to WAF WebACL, Rule & Rule Group Resources #10408

Conversation

DrFaust92 commented Oct 7, 2019 • edited Loading

Community Note

bflad left a comment

Choose a reason for hiding this comment

DrFaust92 commented Oct 10, 2019

bflad commented Oct 10, 2019

DrFaust92 commented Oct 11, 2019

bflad commented Oct 11, 2019

DrFaust92 commented Oct 11, 2019

bflad left a comment

Choose a reason for hiding this comment

bflad Oct 11, 2019

Choose a reason for hiding this comment

ghost commented Mar 29, 2020

DrFaust92 commented Oct 7, 2019 •

edited

Loading