diff --git a/.changelog/33216.txt b/.changelog/33216.txt new file mode 100644 index 00000000000..91640fd7d4d --- /dev/null +++ b/.changelog/33216.txt @@ -0,0 +1,3 @@ +```release-note:bug +resource/aws_wafv2_web_acl: Prevent deletion of the AWS-managed `ShieldMitigationRuleGroup` rule on resource Update +``` \ No newline at end of file diff --git a/internal/service/wafv2/exports_test.go b/internal/service/wafv2/exports_test.go new file mode 100644 index 00000000000..c5eda66392e --- /dev/null +++ b/internal/service/wafv2/exports_test.go @@ -0,0 +1,10 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package wafv2 + +// Exports for use in tests only. +var ( + ListRuleGroupsPages = listRuleGroupsPages + ListWebACLsPages = listWebACLsPages +) diff --git a/internal/service/wafv2/web_acl.go b/internal/service/wafv2/web_acl.go index fc3fa915ebd..c1b88a3aa5b 100644 --- a/internal/service/wafv2/web_acl.go +++ b/internal/service/wafv2/web_acl.go @@ -264,6 +264,17 @@ func resourceWebACLUpdate(ctx context.Context, d *schema.ResourceData, meta inte conn := meta.(*conns.AWSClient).WAFV2Conn(ctx) if d.HasChangesExcept("tags", "tags_all") { + // Find the AWS managed ShieldMitigationRuleGroup group rule if existent and add it into the set of rules to update + // so that the provider will not remove the Shield rule when changes are applied to the WebACL. + rules := expandWebACLRules(d.Get("rule").(*schema.Set).List()) + if sr := findShieldRule(rules); len(sr) == 0 { + output, err := FindWebACLByThreePartKey(ctx, conn, d.Id(), d.Get("name").(string), d.Get("scope").(string)) + if err != nil { + return diag.Errorf("reading WAFv2 WebACL (%s): %s", d.Id(), err) + } + rules = append(rules, findShieldRule(output.WebACL.Rules)...) + } + input := &wafv2.UpdateWebACLInput{ AssociationConfig: expandAssociationConfig(d.Get("association_config").([]interface{})), CaptchaConfig: expandCaptchaConfig(d.Get("captcha_config").([]interface{})), @@ -271,7 +282,7 @@ func resourceWebACLUpdate(ctx context.Context, d *schema.ResourceData, meta inte Id: aws.String(d.Id()), LockToken: aws.String(d.Get("lock_token").(string)), Name: aws.String(d.Get("name").(string)), - Rules: expandWebACLRules(d.Get("rule").(*schema.Set).List()), + Rules: rules, Scope: aws.String(d.Get("scope").(string)), VisibilityConfig: expandVisibilityConfig(d.Get("visibility_config").([]interface{})), } @@ -364,9 +375,14 @@ func FindWebACLByThreePartKey(ctx context.Context, conn *wafv2.WAFV2, id, name, // See https://docs.aws.amazon.com/waf/latest/developerguide/ddos-automatic-app-layer-response-rg.html func filterWebACLRules(rules, configRules []*wafv2.Rule) []*wafv2.Rule { var fr []*wafv2.Rule - pattern := `^ShieldMitigationRuleGroup_\d{12}_[0-9A-Fa-f]{8}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{12}_.*` + sr := findShieldRule(rules) + + if len(sr) == 0 { + return rules + } + for _, r := range rules { - if regexache.MustCompile(pattern).MatchString(aws.StringValue(r.Name)) { + if aws.StringValue(r.Name) == aws.StringValue(sr[0].Name) { filter := true for _, cr := range configRules { if aws.StringValue(cr.Name) == aws.StringValue(r.Name) { @@ -383,3 +399,14 @@ func filterWebACLRules(rules, configRules []*wafv2.Rule) []*wafv2.Rule { } return fr } + +func findShieldRule(rules []*wafv2.Rule) []*wafv2.Rule { + pattern := `^ShieldMitigationRuleGroup_\d{12}_[0-9A-Fa-f]{8}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{12}_.*` + var sr []*wafv2.Rule + for _, r := range rules { + if regexache.MustCompile(pattern).MatchString(aws.StringValue(r.Name)) { + sr = append(sr, r) + } + } + return sr +} diff --git a/internal/service/wafv2/web_acl_test.go b/internal/service/wafv2/web_acl_test.go index edc3b59714a..d314a38b03d 100644 --- a/internal/service/wafv2/web_acl_test.go +++ b/internal/service/wafv2/web_acl_test.go @@ -1736,11 +1736,10 @@ func TestAccWAFV2WebACL_RuleGroupReference_shieldMitigation(t *testing.T) { CheckDestroy: testAccCheckWebACLDestroy(ctx), Steps: []resource.TestStep{ { - Config: testAccWebACLConfig_ruleGroupForShieldMitigation(webACLName), + Config: testAccWebACLConfig_ruleGroupForShieldMitigation(webACLName, "desc1"), Check: resource.ComposeTestCheckFunc( testAccCheckWebACLExists(ctx, resourceName, &v), - acctest.MatchResourceAttrRegionalARN(resourceName, "arn", "wafv2", regexache.MustCompile(`regional/webacl/.+$`)), - resource.TestCheckResourceAttr(resourceName, "name", webACLName), + resource.TestCheckResourceAttr(resourceName, "description", "desc1"), resource.TestCheckResourceAttr(resourceName, "rule.#", "0"), ), }, @@ -1758,7 +1757,7 @@ func TestAccWAFV2WebACL_RuleGroupReference_shieldMitigation(t *testing.T) { aclID := "" lockToken := "" - err := webACLsPages(ctx, conn, input, func(page *wafv2.ListWebACLsOutput, lastPage bool) bool { + err := tfwafv2.ListWebACLsPages(ctx, conn, input, func(page *wafv2.ListWebACLsOutput, lastPage bool) bool { if page == nil { return !lastPage } @@ -1789,7 +1788,7 @@ func TestAccWAFV2WebACL_RuleGroupReference_shieldMitigation(t *testing.T) { rgARN := "" - err = ruleGroupPages(ctx, conn, in, func(page *wafv2.ListRuleGroupsOutput, lastPage bool) bool { + err = tfwafv2.ListRuleGroupsPages(ctx, conn, in, func(page *wafv2.ListRuleGroupsOutput, lastPage bool) bool { if page == nil { return !lastPage } @@ -1859,11 +1858,18 @@ func TestAccWAFV2WebACL_RuleGroupReference_shieldMitigation(t *testing.T) { t.Fatalf("out-of-band added rule (%s) not found, cannot test handling of rule", webACLName) } }, - Config: testAccWebACLConfig_ruleGroupForShieldMitigation(webACLName), + Config: testAccWebACLConfig_ruleGroupForShieldMitigation(webACLName, "desc1"), Check: resource.ComposeTestCheckFunc( testAccCheckWebACLExists(ctx, resourceName, &v), - acctest.MatchResourceAttrRegionalARN(resourceName, "arn", "wafv2", regexache.MustCompile(`regional/webacl/.+$`)), - resource.TestCheckResourceAttr(resourceName, "name", webACLName), + resource.TestCheckResourceAttr(resourceName, "description", "desc1"), + resource.TestCheckResourceAttr(resourceName, "rule.#", "0"), + ), + }, + { + Config: testAccWebACLConfig_ruleGroupForShieldMitigation(webACLName, "desc2"), + Check: resource.ComposeTestCheckFunc( + testAccCheckWebACLExists(ctx, resourceName, &v), + resource.TestCheckResourceAttr(resourceName, "description", "desc2"), resource.TestCheckResourceAttr(resourceName, "rule.#", "0"), ), }, @@ -1871,40 +1877,6 @@ func TestAccWAFV2WebACL_RuleGroupReference_shieldMitigation(t *testing.T) { }) } -func webACLsPages(ctx context.Context, conn *wafv2.WAFV2, input *wafv2.ListWebACLsInput, fn func(*wafv2.ListWebACLsOutput, bool) bool) error { - for { - output, err := conn.ListWebACLsWithContext(ctx, input) - if err != nil { - return err - } - - lastPage := aws.StringValue(output.NextMarker) == "" - if !fn(output, lastPage) || lastPage { - break - } - - input.NextMarker = output.NextMarker - } - return nil -} - -func ruleGroupPages(ctx context.Context, conn *wafv2.WAFV2, input *wafv2.ListRuleGroupsInput, fn func(*wafv2.ListRuleGroupsOutput, bool) bool) error { - for { - output, err := conn.ListRuleGroupsWithContext(ctx, input) - if err != nil { - return err - } - - lastPage := aws.StringValue(output.NextMarker) == "" - if !fn(output, lastPage) || lastPage { - break - } - - input.NextMarker = output.NextMarker - } - return nil -} - // Ensure magically-added (i.e., AWS-added) rule for Shield with CF distribution DDoS auto // mitigation does not cause diff and provider doesn't attempt to remove. // See https://github.com/hashicorp/terraform-provider-aws/issues/22869 @@ -1921,16 +1893,27 @@ func TestAccWAFV2WebACL_RuleGroupReference_manageShieldMitigationRule(t *testing CheckDestroy: testAccCheckWebACLDestroy(ctx), Steps: []resource.TestStep{ { - Config: testAccWebACLConfig_ruleGroupShieldMitigation(webACLName), + Config: testAccWebACLConfig_ruleGroupShieldMitigation(webACLName, "desc1"), Check: resource.ComposeTestCheckFunc( testAccCheckWebACLExists(ctx, resourceName, &v), - acctest.MatchResourceAttrRegionalARN(resourceName, "arn", "wafv2", regexache.MustCompile(`regional/webacl/.+$`)), - resource.TestCheckResourceAttr(resourceName, "name", webACLName), + resource.TestCheckResourceAttr(resourceName, "description", "desc1"), resource.TestCheckResourceAttr(resourceName, "rule.#", "1"), ), }, { - Config: testAccWebACLConfig_ruleGroupShieldMitigation(webACLName), + Config: testAccWebACLConfig_ruleGroupShieldMitigation(webACLName, "desc1"), + PlanOnly: true, + }, + { + Config: testAccWebACLConfig_ruleGroupShieldMitigation(webACLName, "desc2"), + Check: resource.ComposeTestCheckFunc( + testAccCheckWebACLExists(ctx, resourceName, &v), + resource.TestCheckResourceAttr(resourceName, "description", "desc2"), + resource.TestCheckResourceAttr(resourceName, "rule.#", "1"), + ), + }, + { + Config: testAccWebACLConfig_ruleGroupShieldMitigation(webACLName, "desc2"), PlanOnly: true, }, }, @@ -4682,7 +4665,7 @@ resource "aws_wafv2_web_acl" "test" { `, rName) } -func testAccWebACLConfig_ruleGroupShieldMitigation(rName string) string { +func testAccWebACLConfig_ruleGroupShieldMitigation(rName, description string) string { return fmt.Sprintf(` resource "aws_wafv2_rule_group" "test" { capacity = 10 @@ -4706,6 +4689,8 @@ resource "aws_wafv2_web_acl" "test" { block {} } + description = %[2]q + rule { name = "ShieldMitigationRuleGroup_${data.aws_caller_identity.current.account_id}_5e665b1c-1641-4b7a-8db1-567871a18b2a_uniqueid" priority = 11 @@ -4733,10 +4718,10 @@ resource "aws_wafv2_web_acl" "test" { sampled_requests_enabled = false } } -`, rName) +`, rName, description) } -func testAccWebACLConfig_ruleGroupForShieldMitigation(rName string) string { +func testAccWebACLConfig_ruleGroupForShieldMitigation(rName, description string) string { return fmt.Sprintf(` resource "aws_wafv2_rule_group" "test" { capacity = 10 @@ -4758,13 +4743,15 @@ resource "aws_wafv2_web_acl" "test" { block {} } + description = %[2]q + visibility_config { cloudwatch_metrics_enabled = true metric_name = "friendly-metric-name" sampled_requests_enabled = false } } -`, rName) +`, rName, description) } func testAccWebACLConfig_tokenDomains(rName, domain1, domain2 string) string {