From 1291515aa07083f11855961ecb8e07695158d324 Mon Sep 17 00:00:00 2001 From: Angie Pinilla Date: Tue, 2 Mar 2021 01:58:02 -0500 Subject: [PATCH 1/2] catch in-use exception when removing WAF rule; linting --- aws/resource_aws_waf_rule.go | 55 ++++++--- aws/resource_aws_waf_rule_test.go | 56 ++++----- aws/resource_aws_waf_web_acl.go | 58 ++++----- aws/resource_aws_waf_web_acl_test.go | 174 ++++++++++++--------------- 4 files changed, 171 insertions(+), 172 deletions(-) diff --git a/aws/resource_aws_waf_rule.go b/aws/resource_aws_waf_rule.go index 7802bf0e00ed..fcecd5a02640 100644 --- a/aws/resource_aws_waf_rule.go +++ b/aws/resource_aws_waf_rule.go @@ -6,8 +6,8 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/arn" - "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/service/waf" + "github.com/hashicorp/aws-sdk-go-base/tfawserr" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" "github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags" @@ -95,7 +95,7 @@ func resourceAwsWafRuleCreate(d *schema.ResourceData, meta interface{}) error { noPredicates := []interface{}{} err := updateWafRuleResource(d.Id(), noPredicates, newPredicates, conn) if err != nil { - return fmt.Errorf("Error Updating WAF Rule: %s", err) + return fmt.Errorf("error updating WAF Rule (%s): %w", d.Id(), err) } } @@ -111,14 +111,24 @@ func resourceAwsWafRuleRead(d *schema.ResourceData, meta interface{}) error { } resp, err := conn.GetRule(params) + if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, waf.ErrCodeNonexistentItemException) { + log.Printf("[WARN] WAF Rule (%s) not found, removing from state", d.Id()) + d.SetId("") + return nil + } + if err != nil { - if awsErr, ok := err.(awserr.Error); ok && awsErr.Code() == waf.ErrCodeNonexistentItemException { - log.Printf("[WARN] WAF Rule (%s) not found, removing from state", d.Id()) - d.SetId("") - return nil + return fmt.Errorf("error getting WAF Rule (%s): %w", d.Id(), err) + } + + if resp == nil || resp.Rule == nil { + if d.IsNewResource() { + return fmt.Errorf("error getting WAF Rule (%s): not found", d.Id()) } - return err + log.Printf("[WARN] WAF Rule (%s) not found, removing from state", d.Id()) + d.SetId("") + return nil } var predicates []map[string]interface{} @@ -143,11 +153,11 @@ func resourceAwsWafRuleRead(d *schema.ResourceData, meta interface{}) error { tags, err := keyvaluetags.WafListTags(conn, arn) if err != nil { - return fmt.Errorf("error listing tags for WAF Rule (%s): %s", arn, err) + return fmt.Errorf("error listing tags for WAF Rule (%s): %w", d.Id(), err) } if err := d.Set("tags", tags.IgnoreAws().IgnoreConfig(ignoreTagsConfig).Map()); err != nil { - return fmt.Errorf("error setting tags: %s", err) + return fmt.Errorf("error setting tags: %w", err) } d.Set("predicates", predicates) @@ -166,7 +176,7 @@ func resourceAwsWafRuleUpdate(d *schema.ResourceData, meta interface{}) error { err := updateWafRuleResource(d.Id(), oldP, newP, conn) if err != nil { - return fmt.Errorf("Error Updating WAF Rule: %s", err) + return fmt.Errorf("error updating WAF Rule (%s): %w", d.Id(), err) } } @@ -174,7 +184,7 @@ func resourceAwsWafRuleUpdate(d *schema.ResourceData, meta interface{}) error { o, n := d.GetChange("tags") if err := keyvaluetags.WafUpdateTags(conn, d.Get("arn").(string), o, n); err != nil { - return fmt.Errorf("error updating tags: %s", err) + return fmt.Errorf("error updating WAF Rule (%s) tags: %w", d.Id(), err) } } @@ -189,7 +199,7 @@ func resourceAwsWafRuleDelete(d *schema.ResourceData, meta interface{}) error { noPredicates := []interface{}{} err := updateWafRuleResource(d.Id(), oldPredicates, noPredicates, conn) if err != nil { - return fmt.Errorf("Error updating WAF Rule Predicates: %s", err) + return fmt.Errorf("error updating WAF Rule (%s) predicates: %w", d.Id(), err) } } @@ -199,11 +209,24 @@ func resourceAwsWafRuleDelete(d *schema.ResourceData, meta interface{}) error { ChangeToken: token, RuleId: aws.String(d.Id()), } - log.Printf("[INFO] Deleting WAF Rule") - return conn.DeleteRule(req) + + output, err := conn.DeleteRule(req) + + // Deleting a WAF Rule after being removed from a WAF WebACL + // can return a WAFReferencedItemException when attempted in quick succession; + // thus, we catch the error here and re-attempt + if tfawserr.ErrCodeEquals(err, waf.ErrCodeReferencedItemException) { + return output, nil + } + + return output, err }) + if err != nil { - return fmt.Errorf("Error deleting WAF Rule: %s", err) + if tfawserr.ErrCodeEquals(err, waf.ErrCodeNonexistentItemException) { + return nil + } + return fmt.Errorf("error deleting WAF Rule (%s): %w", d.Id(), err) } return nil @@ -221,7 +244,7 @@ func updateWafRuleResource(id string, oldP, newP []interface{}, conn *waf.WAF) e return conn.UpdateRule(req) }) if err != nil { - return fmt.Errorf("Error Updating WAF Rule: %s", err) + return fmt.Errorf("error updating WAF Rule (%s): %w", id, err) } return nil diff --git a/aws/resource_aws_waf_rule_test.go b/aws/resource_aws_waf_rule_test.go index 167192237d3c..b0e27ae347e7 100644 --- a/aws/resource_aws_waf_rule_test.go +++ b/aws/resource_aws_waf_rule_test.go @@ -505,7 +505,7 @@ func testAccPreCheckAWSWaf(t *testing.T) { func testAccAWSWafRuleConfig(name string) string { return fmt.Sprintf(` resource "aws_waf_ipset" "ipset" { - name = "%s" + name = %[1]q ip_set_descriptors { type = "IPV4" @@ -515,8 +515,8 @@ resource "aws_waf_ipset" "ipset" { resource "aws_waf_rule" "wafrule" { depends_on = [aws_waf_ipset.ipset] - name = "%s" - metric_name = "%s" + name = %[1]q + metric_name = %[1]q predicates { data_id = aws_waf_ipset.ipset.id @@ -524,13 +524,13 @@ resource "aws_waf_rule" "wafrule" { type = "IPMatch" } } -`, name, name, name) +`, name) } func testAccAWSWafRuleConfigChangeName(name string) string { return fmt.Sprintf(` resource "aws_waf_ipset" "ipset" { - name = "%s" + name = %[1]q ip_set_descriptors { type = "IPV4" @@ -540,8 +540,8 @@ resource "aws_waf_ipset" "ipset" { resource "aws_waf_rule" "wafrule" { depends_on = [aws_waf_ipset.ipset] - name = "%s" - metric_name = "%s" + name = %[1]q + metric_name = %[1]q predicates { data_id = aws_waf_ipset.ipset.id @@ -549,13 +549,13 @@ resource "aws_waf_rule" "wafrule" { type = "IPMatch" } } -`, name, name, name) +`, name) } func testAccAWSWafRuleConfig_changePredicates(name string) string { return fmt.Sprintf(` resource "aws_waf_ipset" "ipset" { - name = "%s" + name = %[1]q ip_set_descriptors { type = "IPV4" @@ -564,7 +564,7 @@ resource "aws_waf_ipset" "ipset" { } resource "aws_waf_byte_match_set" "set" { - name = "%s" + name = %[1]q byte_match_tuples { text_transformation = "NONE" @@ -579,8 +579,8 @@ resource "aws_waf_byte_match_set" "set" { } resource "aws_waf_rule" "wafrule" { - name = "%s" - metric_name = "%s" + name = %[1]q + metric_name = %[1]q predicates { data_id = aws_waf_byte_match_set.set.id @@ -588,22 +588,22 @@ resource "aws_waf_rule" "wafrule" { type = "ByteMatch" } } -`, name, name, name, name) +`, name) } func testAccAWSWafRuleConfig_noPredicates(name string) string { return fmt.Sprintf(` resource "aws_waf_rule" "wafrule" { - name = "%s" - metric_name = "%s" + name = %[1]q + metric_name = %[1]q } -`, name, name) +`, name) } func testAccAWSWafRuleConfig_geoMatchSetPredicate(name string) string { return fmt.Sprintf(` resource "aws_waf_geo_match_set" "geo_match_set" { - name = "%s" + name = %[1]q geo_match_constraint { type = "Country" @@ -612,8 +612,8 @@ resource "aws_waf_geo_match_set" "geo_match_set" { } resource "aws_waf_rule" "wafrule" { - name = "%s" - metric_name = "%s" + name = %[1]q + metric_name = %[1]q predicates { data_id = aws_waf_geo_match_set.geo_match_set.id @@ -621,13 +621,13 @@ resource "aws_waf_rule" "wafrule" { type = "GeoMatch" } } -`, name, name, name) +`, name) } func testAccAWSWafRuleConfigTags1(rName, tag1Key, tag1Value string) string { return fmt.Sprintf(` resource "aws_waf_ipset" "ipset" { - name = "%s" + name = %[1]q ip_set_descriptors { type = "IPV4" @@ -637,8 +637,8 @@ resource "aws_waf_ipset" "ipset" { resource "aws_waf_rule" "wafrule" { depends_on = [aws_waf_ipset.ipset] - name = "%s" - metric_name = "%s" + name = %[1]q + metric_name = %[1]q predicates { data_id = aws_waf_ipset.ipset.id @@ -650,13 +650,13 @@ resource "aws_waf_rule" "wafrule" { %q = %q } } -`, rName, rName, rName, tag1Key, tag1Value) +`, rName, tag1Key, tag1Value) } func testAccAWSWafRuleConfigTags2(rName, tag1Key, tag1Value, tag2Key, tag2Value string) string { return fmt.Sprintf(` resource "aws_waf_ipset" "ipset" { - name = "%s" + name = %[1]q ip_set_descriptors { type = "IPV4" @@ -666,8 +666,8 @@ resource "aws_waf_ipset" "ipset" { resource "aws_waf_rule" "wafrule" { depends_on = [aws_waf_ipset.ipset] - name = "%s" - metric_name = "%s" + name = %[1]q + metric_name = %[1]q predicates { data_id = aws_waf_ipset.ipset.id @@ -680,5 +680,5 @@ resource "aws_waf_rule" "wafrule" { %q = %q } } -`, rName, rName, rName, tag1Key, tag1Value, tag2Key, tag2Value) +`, rName, tag1Key, tag1Value, tag2Key, tag2Value) } diff --git a/aws/resource_aws_waf_web_acl.go b/aws/resource_aws_waf_web_acl.go index e9e46f02fa38..d0c9ff91793d 100644 --- a/aws/resource_aws_waf_web_acl.go +++ b/aws/resource_aws_waf_web_acl.go @@ -7,6 +7,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/arn" "github.com/aws/aws-sdk-go/service/waf" + "github.com/hashicorp/aws-sdk-go-base/tfawserr" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" "github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags" @@ -184,9 +185,8 @@ func resourceAwsWafWebAclCreate(d *schema.ResourceData, meta interface{}) error LoggingConfiguration: expandWAFLoggingConfiguration(loggingConfiguration, arn), } - log.Printf("[DEBUG] Updating WAF Web ACL (%s) Logging Configuration: %s", d.Id(), input) if _, err := conn.PutLoggingConfiguration(input); err != nil { - return fmt.Errorf("error updating WAF Web ACL (%s) Logging Configuration: %s", d.Id(), err) + return fmt.Errorf("error putting WAF Web ACL (%s) Logging Configuration: %s", d.Id(), err) } } @@ -203,7 +203,7 @@ func resourceAwsWafWebAclCreate(d *schema.ResourceData, meta interface{}) error return conn.UpdateWebACL(req) }) if err != nil { - return fmt.Errorf("Error Updating WAF ACL: %s", err) + return fmt.Errorf("error updating WAF Web ACL (%s): %w", d.Id(), err) } } @@ -219,53 +219,56 @@ func resourceAwsWafWebAclRead(d *schema.ResourceData, meta interface{}) error { } resp, err := conn.GetWebACL(params) - if err != nil { - if isAWSErr(err, waf.ErrCodeNonexistentItemException, "") { - log.Printf("[WARN] WAF ACL (%s) not found, removing from state", d.Id()) - d.SetId("") - return nil - } + if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, waf.ErrCodeNonexistentItemException) { + log.Printf("[WARN] WAF Web ACL (%s) not found, removing from state", d.Id()) + d.SetId("") + return nil + } - return err + if err != nil { + return fmt.Errorf("error getting WAF Web ACL (%s): %w", d.Id(), err) } if resp == nil || resp.WebACL == nil { - log.Printf("[WARN] WAF ACL (%s) not found, removing from state", d.Id()) + if d.IsNewResource() { + return fmt.Errorf("error getting WAF Web ACL (%s): not found", d.Id()) + } + + log.Printf("[WARN] WAF Web ACL (%s) not found, removing from state", d.Id()) d.SetId("") return nil } d.Set("arn", resp.WebACL.WebACLArn) - arn := *resp.WebACL.WebACLArn + arn := aws.StringValue(resp.WebACL.WebACLArn) if err := d.Set("default_action", flattenWafAction(resp.WebACL.DefaultAction)); err != nil { - return fmt.Errorf("error setting default_action: %s", err) + return fmt.Errorf("error setting default_action: %w", err) } d.Set("name", resp.WebACL.Name) d.Set("metric_name", resp.WebACL.MetricName) tags, err := keyvaluetags.WafListTags(conn, arn) if err != nil { - return fmt.Errorf("error listing tags for WAF ACL (%s): %s", arn, err) + return fmt.Errorf("error listing tags for WAF Web ACL (%s): %w", d.Id(), err) } if err := d.Set("tags", tags.IgnoreAws().IgnoreConfig(ignoreTagsConfig).Map()); err != nil { - return fmt.Errorf("error setting tags: %s", err) + return fmt.Errorf("error setting tags: %w", err) } if err := d.Set("rules", flattenWafWebAclRules(resp.WebACL.Rules)); err != nil { - return fmt.Errorf("error setting rules: %s", err) + return fmt.Errorf("error setting rules: %w", err) } getLoggingConfigurationInput := &waf.GetLoggingConfigurationInput{ - ResourceArn: aws.String(d.Get("arn").(string)), + ResourceArn: aws.String(arn), } loggingConfiguration := []interface{}{} - log.Printf("[DEBUG] Getting WAF Web ACL (%s) Logging Configuration: %s", d.Id(), getLoggingConfigurationInput) getLoggingConfigurationOutput, err := conn.GetLoggingConfiguration(getLoggingConfigurationInput) if err != nil && !isAWSErr(err, waf.ErrCodeNonexistentItemException, "") { - return fmt.Errorf("error getting WAF Web ACL (%s) Logging Configuration: %s", d.Id(), err) + return fmt.Errorf("error getting WAF Web ACL (%s) Logging Configuration: %w", d.Id(), err) } if getLoggingConfigurationOutput != nil { @@ -273,7 +276,7 @@ func resourceAwsWafWebAclRead(d *schema.ResourceData, meta interface{}) error { } if err := d.Set("logging_configuration", loggingConfiguration); err != nil { - return fmt.Errorf("error setting logging_configuration: %s", err) + return fmt.Errorf("error setting logging_configuration: %w", err) } return nil @@ -297,7 +300,7 @@ func resourceAwsWafWebAclUpdate(d *schema.ResourceData, meta interface{}) error return conn.UpdateWebACL(req) }) if err != nil { - return fmt.Errorf("Error Updating WAF ACL: %s", err) + return fmt.Errorf("error updating WAF Web ACL (%s): %w", d.Id(), err) } } @@ -309,18 +312,16 @@ func resourceAwsWafWebAclUpdate(d *schema.ResourceData, meta interface{}) error LoggingConfiguration: expandWAFLoggingConfiguration(loggingConfiguration, d.Get("arn").(string)), } - log.Printf("[DEBUG] Updating WAF Web ACL (%s) Logging Configuration: %s", d.Id(), input) if _, err := conn.PutLoggingConfiguration(input); err != nil { - return fmt.Errorf("error updating WAF Web ACL (%s) Logging Configuration: %s", d.Id(), err) + return fmt.Errorf("error updating WAF Web ACL (%s) Logging Configuration: %w", d.Id(), err) } } else { input := &waf.DeleteLoggingConfigurationInput{ ResourceArn: aws.String(d.Get("arn").(string)), } - log.Printf("[DEBUG] Deleting WAF Web ACL (%s) Logging Configuration: %s", d.Id(), input) if _, err := conn.DeleteLoggingConfiguration(input); err != nil { - return fmt.Errorf("error deleting WAF Web ACL (%s) Logging Configuration: %s", d.Id(), err) + return fmt.Errorf("error deleting WAF Web ACL (%s) Logging Configuration: %w", d.Id(), err) } } @@ -330,7 +331,7 @@ func resourceAwsWafWebAclUpdate(d *schema.ResourceData, meta interface{}) error o, n := d.GetChange("tags") if err := keyvaluetags.WafUpdateTags(conn, d.Get("arn").(string), o, n); err != nil { - return fmt.Errorf("error updating tags: %s", err) + return fmt.Errorf("error updating WAF Web ACL (%s) tags: %w", d.Id(), err) } } @@ -354,7 +355,7 @@ func resourceAwsWafWebAclDelete(d *schema.ResourceData, meta interface{}) error return conn.UpdateWebACL(req) }) if err != nil { - return fmt.Errorf("Error Removing WAF Regional ACL Rules: %s", err) + return fmt.Errorf("error removing WAF Web ACL (%s) rules: %w", d.Id(), err) } } @@ -365,11 +366,10 @@ func resourceAwsWafWebAclDelete(d *schema.ResourceData, meta interface{}) error WebACLId: aws.String(d.Id()), } - log.Printf("[INFO] Deleting WAF ACL") return conn.DeleteWebACL(req) }) if err != nil { - return fmt.Errorf("Error Deleting WAF ACL: %s", err) + return fmt.Errorf("error deleting WAF Web ACL (%s): %w", d.Id(), err) } return nil } diff --git a/aws/resource_aws_waf_web_acl_test.go b/aws/resource_aws_waf_web_acl_test.go index fa82a5c91e76..20132473f210 100644 --- a/aws/resource_aws_waf_web_acl_test.go +++ b/aws/resource_aws_waf_web_acl_test.go @@ -8,9 +8,11 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/waf" + "github.com/hashicorp/go-multierror" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/waf/lister" ) func init() { @@ -23,94 +25,68 @@ func init() { func testSweepWafWebAcls(region string) error { client, err := sharedClientForRegion(region) if err != nil { - return fmt.Errorf("error getting client: %s", err) + return fmt.Errorf("error getting client: %w", err) } conn := client.(*AWSClient).wafconn - input := &waf.ListWebACLsInput{} - - for { - output, err := conn.ListWebACLs(input) + var sweeperErrs *multierror.Error - if testSweepSkipSweepError(err) { - log.Printf("[WARN] Skipping WAF Regional Web ACL sweep for %s: %s", region, err) - return nil - } + input := &waf.ListWebACLsInput{} - if err != nil { - return fmt.Errorf("error listing WAF Regional Web ACLs: %s", err) + err = lister.ListWebACLsPages(conn, input, func(page *waf.ListWebACLsOutput, lastPage bool) bool { + if page == nil { + return !lastPage } - for _, webACL := range output.WebACLs { - deleteInput := &waf.DeleteWebACLInput{ - WebACLId: webACL.WebACLId, + for _, webACL := range page.WebACLs { + if webACL == nil { + continue } + id := aws.StringValue(webACL.WebACLId) - wr := newWafRetryer(conn) - _, err := wr.RetryWithToken(func(token *string) (interface{}, error) { - deleteInput.ChangeToken = token - log.Printf("[INFO] Deleting WAF Regional Web ACL: %s", id) - return conn.DeleteWebACL(deleteInput) - }) + r := resourceAwsWafWebAcl() + d := r.Data(nil) + d.SetId(id) + + // Need to Read first to fill in rules argument + err := r.Read(d, client) + + if err != nil { + sweeperErr := fmt.Errorf("error reading WAF Web ACL (%s): %w", id, err) + log.Printf("[ERROR] %s", sweeperErr) + sweeperErrs = multierror.Append(sweeperErrs, sweeperErr) + continue + } - if isAWSErr(err, waf.ErrCodeNonEmptyEntityException, "") { - getWebACLInput := &waf.GetWebACLInput{ - WebACLId: webACL.WebACLId, - } - - getWebACLOutput, getWebACLErr := conn.GetWebACL(getWebACLInput) - - if getWebACLErr != nil { - return fmt.Errorf("error getting WAF Regional Web ACL (%s): %s", id, getWebACLErr) - } - - var updates []*waf.WebACLUpdate - updateWebACLInput := &waf.UpdateWebACLInput{ - DefaultAction: getWebACLOutput.WebACL.DefaultAction, - Updates: updates, - WebACLId: webACL.WebACLId, - } - - for _, rule := range getWebACLOutput.WebACL.Rules { - update := &waf.WebACLUpdate{ - Action: aws.String(waf.ChangeActionDelete), - ActivatedRule: rule, - } - - updateWebACLInput.Updates = append(updateWebACLInput.Updates, update) - } - - _, updateWebACLErr := wr.RetryWithToken(func(token *string) (interface{}, error) { - updateWebACLInput.ChangeToken = token - log.Printf("[INFO] Removing Rules from WAF Regional Web ACL: %s", id) - return conn.UpdateWebACL(updateWebACLInput) - }) - - if updateWebACLErr != nil { - return fmt.Errorf("error removing rules from WAF Regional Web ACL (%s): %s", id, updateWebACLErr) - } - - _, err = wr.RetryWithToken(func(token *string) (interface{}, error) { - deleteInput.ChangeToken = token - log.Printf("[INFO] Deleting WAF Regional Web ACL: %s", id) - return conn.DeleteWebACL(deleteInput) - }) + // In case it was already deleted + if d.Id() == "" { + continue } + err = r.Delete(d, client) + if err != nil { - return fmt.Errorf("error deleting WAF Regional Web ACL (%s): %s", id, err) + sweeperErr := fmt.Errorf("error deleting WAF Web ACL (%s): %w", id, err) + log.Printf("[ERROR] %s", sweeperErr) + sweeperErrs = multierror.Append(sweeperErrs, sweeperErr) + continue } } - if aws.StringValue(output.NextMarker) == "" { - break - } + return !lastPage + }) - input.NextMarker = output.NextMarker + if testSweepSkipSweepError(err) { + log.Printf("[WARN] Skipping WAF Web ACL sweep for %s: %s", region, err) + return sweeperErrs.ErrorOrNil() // In case we have completed some pages, but had errors } - return nil + if err != nil { + sweeperErrs = multierror.Append(sweeperErrs, fmt.Errorf("error describing WAF Web ACLs: %w", err)) + } + + return sweeperErrs.ErrorOrNil() } func TestAccAWSWafWebAcl_basic(t *testing.T) { @@ -481,33 +457,33 @@ func testAccCheckAWSWafWebAclExists(n string, v *waf.WebACL) resource.TestCheckF func testAccAWSWafWebAclConfig_Required(rName string) string { return fmt.Sprintf(` resource "aws_waf_web_acl" "test" { - metric_name = %q - name = %q + metric_name = %[1]q + name = %[1]q default_action { type = "ALLOW" } } -`, rName, rName) +`, rName) } func testAccAWSWafWebAclConfig_DefaultAction(rName, defaultAction string) string { return fmt.Sprintf(` resource "aws_waf_web_acl" "test" { - metric_name = %q - name = %q + metric_name = %[1]q + name = %[1]q default_action { type = %q } } -`, rName, rName, defaultAction) +`, rName, defaultAction) } func testAccAWSWafWebAclConfig_Rules_Single_Rule(rName string) string { return fmt.Sprintf(` resource "aws_waf_ipset" "test" { - name = %q + name = %[1]q ip_set_descriptors { type = "IPV4" @@ -516,8 +492,8 @@ resource "aws_waf_ipset" "test" { } resource "aws_waf_rule" "test" { - metric_name = %q - name = %q + metric_name = %[1]q + name = %[1]q predicates { data_id = aws_waf_ipset.test.id @@ -527,8 +503,8 @@ resource "aws_waf_rule" "test" { } resource "aws_waf_web_acl" "test" { - metric_name = %q - name = %q + metric_name = %[1]q + name = %[1]q default_action { type = "ALLOW" @@ -543,19 +519,19 @@ resource "aws_waf_web_acl" "test" { } } } -`, rName, rName, rName, rName, rName) +`, rName) } func testAccAWSWafWebAclConfig_Rules_Single_RuleGroup(rName string) string { return fmt.Sprintf(` resource "aws_waf_rule_group" "test" { - metric_name = %q - name = %q + metric_name = %[1]q + name = %[1]q } resource "aws_waf_web_acl" "test" { - metric_name = %q - name = %q + metric_name = %[1]q + name = %[1]q default_action { type = "ALLOW" @@ -571,13 +547,13 @@ resource "aws_waf_web_acl" "test" { } } } -`, rName, rName, rName, rName) +`, rName) } func testAccAWSWafWebAclConfig_Rules_Multiple(rName string) string { return fmt.Sprintf(` resource "aws_waf_ipset" "test" { - name = %q + name = %[1]q ip_set_descriptors { type = "IPV4" @@ -586,8 +562,8 @@ resource "aws_waf_ipset" "test" { } resource "aws_waf_rule" "test" { - metric_name = %q - name = %q + metric_name = %[1]q + name = %[1]q predicates { data_id = aws_waf_ipset.test.id @@ -597,13 +573,13 @@ resource "aws_waf_rule" "test" { } resource "aws_waf_rule_group" "test" { - metric_name = %q - name = %q + metric_name = %[1]q + name = %[1]q } resource "aws_waf_web_acl" "test" { - metric_name = %q - name = %q + metric_name = %[1]q + name = %[1]q default_action { type = "ALLOW" @@ -628,7 +604,7 @@ resource "aws_waf_web_acl" "test" { } } } -`, rName, rName, rName, rName, rName, rName, rName) +`, rName) } func testAccAWSWafWebAclConfig_Logging(rName string) string { @@ -772,8 +748,8 @@ resource "aws_kinesis_firehose_delivery_stream" "test" { func testAccAWSWafWebAclConfigTags1(rName, tag1Key, tag1Value string) string { return fmt.Sprintf(` resource "aws_waf_web_acl" "test" { - metric_name = %q - name = %q + metric_name = %[1]q + name = %[1]q default_action { type = "ALLOW" @@ -783,14 +759,14 @@ resource "aws_waf_web_acl" "test" { %q = %q } } -`, rName, rName, tag1Key, tag1Value) +`, rName, tag1Key, tag1Value) } func testAccAWSWafWebAclConfigTags2(rName, tag1Key, tag1Value, tag2Key, tag2Value string) string { return fmt.Sprintf(` resource "aws_waf_web_acl" "test" { - metric_name = %q - name = %q + metric_name = %[1]q + name = %[1]q default_action { type = "ALLOW" @@ -801,5 +777,5 @@ resource "aws_waf_web_acl" "test" { %q = %q } } -`, rName, rName, tag1Key, tag1Value, tag2Key, tag2Value) +`, rName, tag1Key, tag1Value, tag2Key, tag2Value) } From 62adf9a24cab20196349e9661ccae6317ee45f68 Mon Sep 17 00:00:00 2001 From: Angie Pinilla Date: Tue, 16 Mar 2021 01:05:12 -0400 Subject: [PATCH 2/2] add AWS SDK enumerations and simplify validations; test linting --- .changelog/17876.txt | 3 + aws/resource_aws_waf_rule.go | 65 ++++++--- aws/resource_aws_waf_rule_test.go | 205 ++++++++++++--------------- aws/resource_aws_waf_web_acl.go | 37 ++--- aws/resource_aws_waf_web_acl_test.go | 38 ++--- 5 files changed, 166 insertions(+), 182 deletions(-) create mode 100644 .changelog/17876.txt diff --git a/.changelog/17876.txt b/.changelog/17876.txt new file mode 100644 index 000000000000..5c7ab8bd6028 --- /dev/null +++ b/.changelog/17876.txt @@ -0,0 +1,3 @@ +```release-note:bug +resource/aws_waf_rule: Fix rule deletion when still referenced by a WebACL +``` diff --git a/aws/resource_aws_waf_rule.go b/aws/resource_aws_waf_rule.go index fcecd5a02640..964cbb945c25 100644 --- a/aws/resource_aws_waf_rule.go +++ b/aws/resource_aws_waf_rule.go @@ -3,14 +3,22 @@ package aws import ( "fmt" "log" + "regexp" + "time" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/arn" "github.com/aws/aws-sdk-go/service/waf" "github.com/hashicorp/aws-sdk-go-base/tfawserr" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" "github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource" +) + +const ( + WafRuleDeleteTimeout = 5 * time.Minute ) func resourceAwsWafRule() *schema.Resource { @@ -33,7 +41,7 @@ func resourceAwsWafRule() *schema.Resource { Type: schema.TypeString, Required: true, ForceNew: true, - ValidateFunc: validateWafMetricName, + ValidateFunc: validation.StringMatch(regexp.MustCompile(`^[0-9A-Za-z]+$`), "must contain only alphanumeric characters"), }, "predicates": { Type: schema.TypeSet, @@ -52,7 +60,7 @@ func resourceAwsWafRule() *schema.Resource { "type": { Type: schema.TypeString, Required: true, - ValidateFunc: validateWafPredicatesType(), + ValidateFunc: validation.StringInSlice(waf.PredicateType_Values(), false), }, }, }, @@ -84,9 +92,11 @@ func resourceAwsWafRuleCreate(d *schema.ResourceData, meta interface{}) error { return conn.CreateRule(params) }) + if err != nil { - return err + return fmt.Errorf("error creating WAF Rule (%s): %w", d.Get("name").(string), err) } + resp := out.(*waf.CreateRuleOutput) d.SetId(aws.StringValue(resp.Rule.RuleId)) @@ -118,12 +128,12 @@ func resourceAwsWafRuleRead(d *schema.ResourceData, meta interface{}) error { } if err != nil { - return fmt.Errorf("error getting WAF Rule (%s): %w", d.Id(), err) + return fmt.Errorf("error reading WAF Rule (%s): %w", d.Id(), err) } if resp == nil || resp.Rule == nil { if d.IsNewResource() { - return fmt.Errorf("error getting WAF Rule (%s): not found", d.Id()) + return fmt.Errorf("error reading WAF Rule (%s): not found", d.Id()) } log.Printf("[WARN] WAF Rule (%s) not found, removing from state", d.Id()) @@ -153,7 +163,7 @@ func resourceAwsWafRuleRead(d *schema.ResourceData, meta interface{}) error { tags, err := keyvaluetags.WafListTags(conn, arn) if err != nil { - return fmt.Errorf("error listing tags for WAF Rule (%s): %w", d.Id(), err) + return fmt.Errorf("error listing tags for WAF Rule (%s): %w", arn, err) } if err := d.Set("tags", tags.IgnoreAws().IgnoreConfig(ignoreTagsConfig).Map()); err != nil { @@ -204,24 +214,38 @@ func resourceAwsWafRuleDelete(d *schema.ResourceData, meta interface{}) error { } wr := newWafRetryer(conn) - _, err := wr.RetryWithToken(func(token *string) (interface{}, error) { - req := &waf.DeleteRuleInput{ - ChangeToken: token, - RuleId: aws.String(d.Id()), - } + err := resource.Retry(WafRuleDeleteTimeout, func() *resource.RetryError { + var err error + _, err = wr.RetryWithToken(func(token *string) (interface{}, error) { + req := &waf.DeleteRuleInput{ + ChangeToken: token, + RuleId: aws.String(d.Id()), + } - output, err := conn.DeleteRule(req) + return conn.DeleteRule(req) + }) - // Deleting a WAF Rule after being removed from a WAF WebACL - // can return a WAFReferencedItemException when attempted in quick succession; - // thus, we catch the error here and re-attempt - if tfawserr.ErrCodeEquals(err, waf.ErrCodeReferencedItemException) { - return output, nil + if err != nil { + if tfawserr.ErrCodeEquals(err, waf.ErrCodeReferencedItemException) { + return resource.RetryableError(err) + } + return resource.NonRetryableError(err) } - return output, err + return nil }) + if tfresource.TimedOut(err) { + _, err = wr.RetryWithToken(func(token *string) (interface{}, error) { + req := &waf.DeleteRuleInput{ + ChangeToken: token, + RuleId: aws.String(d.Id()), + } + + return conn.DeleteRule(req) + }) + } + if err != nil { if tfawserr.ErrCodeEquals(err, waf.ErrCodeNonexistentItemException) { return nil @@ -243,9 +267,6 @@ func updateWafRuleResource(id string, oldP, newP []interface{}, conn *waf.WAF) e return conn.UpdateRule(req) }) - if err != nil { - return fmt.Errorf("error updating WAF Rule (%s): %w", id, err) - } - return nil + return err } diff --git a/aws/resource_aws_waf_rule_test.go b/aws/resource_aws_waf_rule_test.go index b0e27ae347e7..1ab79ee6f3ec 100644 --- a/aws/resource_aws_waf_rule_test.go +++ b/aws/resource_aws_waf_rule_test.go @@ -7,12 +7,11 @@ import ( "testing" "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/service/waf" + "github.com/hashicorp/aws-sdk-go-base/tfawserr" "github.com/hashicorp/go-multierror" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/waf/lister" ) @@ -45,6 +44,10 @@ func testSweepWafRules(region string) error { } for _, rule := range page.Rules { + if rule == nil { + continue + } + id := aws.StringValue(rule.RuleId) r := resourceAwsWafRule() @@ -166,7 +169,7 @@ func TestAccAWSWafRule_disappears(t *testing.T) { Config: testAccAWSWafRuleConfig(wafRuleName), Check: resource.ComposeTestCheckFunc( testAccCheckAWSWafRuleExists(resourceName, &v), - testAccCheckAWSWafRuleDisappears(&v), + testAccCheckResourceDisappears(testAccProvider, resourceAwsWafRule(), resourceName), ), ExpectNonEmptyPlan: true, }, @@ -179,7 +182,6 @@ func TestAccAWSWafRule_changePredicates(t *testing.T) { var byteMatchSet waf.ByteMatchSet var before, after waf.Rule - var idx int ruleName := fmt.Sprintf("wafrule%s", acctest.RandString(5)) resourceName := "aws_waf_rule.wafrule" @@ -195,7 +197,6 @@ func TestAccAWSWafRule_changePredicates(t *testing.T) { testAccCheckAWSWafRuleExists(resourceName, &before), resource.TestCheckResourceAttr(resourceName, "name", ruleName), resource.TestCheckResourceAttr(resourceName, "predicates.#", "1"), - computeWafRulePredicateWithIpSet(&ipset, false, "IPMatch", &idx), resource.TestCheckTypeSetElemNestedAttrs(resourceName, "predicates.*", map[string]string{ "negated": "false", "type": "IPMatch", @@ -209,7 +210,6 @@ func TestAccAWSWafRule_changePredicates(t *testing.T) { testAccCheckAWSWafRuleExists(resourceName, &after), resource.TestCheckResourceAttr(resourceName, "name", ruleName), resource.TestCheckResourceAttr(resourceName, "predicates.#", "1"), - computeWafRulePredicateWithByteMatchSet(&byteMatchSet, true, "ByteMatch", &idx), resource.TestCheckTypeSetElemNestedAttrs(resourceName, "predicates.*", map[string]string{ "negated": "true", "type": "ByteMatch", @@ -224,7 +224,6 @@ func TestAccAWSWafRule_geoMatchSetPredicate(t *testing.T) { var geoMatchSet waf.GeoMatchSet var v waf.Rule - var idx int ruleName := fmt.Sprintf("wafrule%s", acctest.RandString(5)) resourceName := "aws_waf_rule.wafrule" @@ -240,7 +239,6 @@ func TestAccAWSWafRule_geoMatchSetPredicate(t *testing.T) { testAccCheckAWSWafRuleExists(resourceName, &v), resource.TestCheckResourceAttr(resourceName, "name", ruleName), resource.TestCheckResourceAttr(resourceName, "predicates.#", "1"), - computeWafRulePredicateWithGeoMatchSet(&geoMatchSet, true, "GeoMatch", &idx), resource.TestCheckTypeSetElemNestedAttrs(resourceName, "predicates.*", map[string]string{ "negated": "true", "type": "GeoMatch", @@ -251,61 +249,29 @@ func TestAccAWSWafRule_geoMatchSetPredicate(t *testing.T) { }) } -// computeWafRulePredicateWithIpSet calculates index -// which isn't static because dataId is generated as part of the test -func computeWafRulePredicateWithIpSet(ipSet *waf.IPSet, negated bool, pType string, idx *int) resource.TestCheckFunc { - return func(s *terraform.State) error { - predicateResource := resourceAwsWafRule().Schema["predicates"].Elem.(*schema.Resource) - - m := map[string]interface{}{ - "data_id": *ipSet.IPSetId, - "negated": negated, - "type": pType, - } - - f := schema.HashResource(predicateResource) - *idx = f(m) - - return nil - } -} - -// computeWafRulePredicateWithByteMatchSet calculates index -// which isn't static because dataId is generated as part of the test -func computeWafRulePredicateWithByteMatchSet(set *waf.ByteMatchSet, negated bool, pType string, idx *int) resource.TestCheckFunc { - return func(s *terraform.State) error { - predicateResource := resourceAwsWafRule().Schema["predicates"].Elem.(*schema.Resource) - - m := map[string]interface{}{ - "data_id": *set.ByteMatchSetId, - "negated": negated, - "type": pType, - } - - f := schema.HashResource(predicateResource) - *idx = f(m) - - return nil - } -} - -// computeWafRulePredicateWithGeoMatchSet calculates index -// which isn't static because dataId is generated as part of the test -func computeWafRulePredicateWithGeoMatchSet(set *waf.GeoMatchSet, negated bool, pType string, idx *int) resource.TestCheckFunc { - return func(s *terraform.State) error { - predicateResource := resourceAwsWafRule().Schema["predicates"].Elem.(*schema.Resource) - - m := map[string]interface{}{ - "data_id": *set.GeoMatchSetId, - "negated": negated, - "type": pType, - } - - f := schema.HashResource(predicateResource) - *idx = f(m) +// TestAccAWSWafRule_webACL validates the resource's +// retry behavior when removed from a WebACL +func TestAccAWSWafRule_webACL(t *testing.T) { + var rule waf.Rule + rName := acctest.RandomWithPrefix("tf-acc-test") + resourceName := "aws_waf_rule.test" - return nil - } + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t); testAccPreCheckAWSWaf(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSWafRuleDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSWafRuleConfig_referencedByWebACL(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSWafRuleExists(resourceName, &rule), + ), + }, + { + Config: testAccAWSWafWebAclConfig_noRules(rName), + }, + }, + }) } func TestAccAWSWafRule_noPredicates(t *testing.T) { @@ -383,49 +349,6 @@ func TestAccAWSWafRule_Tags(t *testing.T) { }) } -func testAccCheckAWSWafRuleDisappears(v *waf.Rule) resource.TestCheckFunc { - return func(s *terraform.State) error { - conn := testAccProvider.Meta().(*AWSClient).wafconn - - wr := newWafRetryer(conn) - _, err := wr.RetryWithToken(func(token *string) (interface{}, error) { - req := &waf.UpdateRuleInput{ - ChangeToken: token, - RuleId: v.RuleId, - } - - for _, Predicate := range v.Predicates { - Predicate := &waf.RuleUpdate{ - Action: aws.String("DELETE"), - Predicate: &waf.Predicate{ - Negated: Predicate.Negated, - Type: Predicate.Type, - DataId: Predicate.DataId, - }, - } - req.Updates = append(req.Updates, Predicate) - } - - return conn.UpdateRule(req) - }) - if err != nil { - return fmt.Errorf("Error Updating WAF Rule: %s", err) - } - - _, err = wr.RetryWithToken(func(token *string) (interface{}, error) { - opts := &waf.DeleteRuleInput{ - ChangeToken: token, - RuleId: v.RuleId, - } - return conn.DeleteRule(opts) - }) - if err != nil { - return fmt.Errorf("Error Deleting WAF Rule: %s", err) - } - return nil - } -} - func testAccCheckAWSWafRuleDestroy(s *terraform.State) error { for _, rs := range s.RootModule().Resources { if rs.Type != "aws_waf_rule" { @@ -438,20 +361,17 @@ func testAccCheckAWSWafRuleDestroy(s *terraform.State) error { RuleId: aws.String(rs.Primary.ID), }) - if err == nil { - if *resp.Rule.RuleId == rs.Primary.ID { - return fmt.Errorf("WAF Rule %s still exists", rs.Primary.ID) - } + if tfawserr.ErrCodeEquals(err, waf.ErrCodeNonexistentItemException) { + continue } - // Return nil if the Rule is already destroyed - if awsErr, ok := err.(awserr.Error); ok { - if awsErr.Code() == waf.ErrCodeNonexistentItemException { - return nil - } + if err != nil { + return fmt.Errorf("error reading WAF Rule (%s): %w", rs.Primary.ID, err) } - return err + if resp != nil && resp.Rule != nil { + return fmt.Errorf("WAF Rule (%s) still exists", rs.Primary.ID) + } } return nil @@ -682,3 +602,58 @@ resource "aws_waf_rule" "wafrule" { } `, rName, tag1Key, tag1Value, tag2Key, tag2Value) } + +func testAccAWSWafRuleConfig_referencedByWebACL(rName string) string { + return fmt.Sprintf(` +resource "aws_waf_ipset" "test" { + name = %[1]q + + ip_set_descriptors { + type = "IPV4" + value = "192.0.7.0/24" + } +} + +resource "aws_waf_rule" "test" { + metric_name = "testrulemetric" + name = %[1]q + + predicates { + data_id = aws_waf_ipset.test.id + negated = false + type = "IPMatch" + } +} + +resource "aws_waf_web_acl" "test" { + metric_name = "testwebaclmetric" + name = %[1]q + + default_action { + type = "ALLOW" + } + + rules { + priority = 1 + rule_id = aws_waf_rule.test.id + + action { + type = "BLOCK" + } + } +} +`, rName) +} + +func testAccAWSWafWebAclConfig_noRules(rName string) string { + return fmt.Sprintf(` +resource "aws_waf_web_acl" "test" { + metric_name = "testwebaclmetric" + name = %[1]q + + default_action { + type = "ALLOW" + } +} +`, rName) +} diff --git a/aws/resource_aws_waf_web_acl.go b/aws/resource_aws_waf_web_acl.go index d0c9ff91793d..7295cfb26f8f 100644 --- a/aws/resource_aws_waf_web_acl.go +++ b/aws/resource_aws_waf_web_acl.go @@ -3,6 +3,7 @@ package aws import ( "fmt" "log" + "regexp" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/arn" @@ -50,7 +51,7 @@ func resourceAwsWafWebAcl() *schema.Resource { Type: schema.TypeString, Required: true, ForceNew: true, - ValidateFunc: validateWafMetricName, + ValidateFunc: validation.StringMatch(regexp.MustCompile(`^[0-9A-Za-z]+$`), "must contain only alphanumeric characters"), }, "logging_configuration": { Type: schema.TypeList, @@ -126,14 +127,10 @@ func resourceAwsWafWebAcl() *schema.Resource { Required: true, }, "type": { - Type: schema.TypeString, - Optional: true, - Default: waf.WafRuleTypeRegular, - ValidateFunc: validation.StringInSlice([]string{ - waf.WafRuleTypeRegular, - waf.WafRuleTypeRateBased, - waf.WafRuleTypeGroup, - }, false), + Type: schema.TypeString, + Optional: true, + Default: waf.WafRuleTypeRegular, + ValidateFunc: validation.StringInSlice(waf.WafRuleType_Values(), false), }, "rule_id": { Type: schema.TypeString, @@ -166,9 +163,11 @@ func resourceAwsWafWebAclCreate(d *schema.ResourceData, meta interface{}) error return conn.CreateWebACL(params) }) + if err != nil { - return err + return fmt.Errorf("error creating WAF Web ACL (%s): %w", d.Get("name").(string), err) } + resp := out.(*waf.CreateWebACLOutput) d.SetId(aws.StringValue(resp.WebACL.WebACLId)) @@ -186,7 +185,7 @@ func resourceAwsWafWebAclCreate(d *schema.ResourceData, meta interface{}) error } if _, err := conn.PutLoggingConfiguration(input); err != nil { - return fmt.Errorf("error putting WAF Web ACL (%s) Logging Configuration: %s", d.Id(), err) + return fmt.Errorf("error putting WAF Web ACL (%s) Logging Configuration: %w", d.Id(), err) } } @@ -202,6 +201,7 @@ func resourceAwsWafWebAclCreate(d *schema.ResourceData, meta interface{}) error } return conn.UpdateWebACL(req) }) + if err != nil { return fmt.Errorf("error updating WAF Web ACL (%s): %w", d.Id(), err) } @@ -226,12 +226,12 @@ func resourceAwsWafWebAclRead(d *schema.ResourceData, meta interface{}) error { } if err != nil { - return fmt.Errorf("error getting WAF Web ACL (%s): %w", d.Id(), err) + return fmt.Errorf("error reading WAF Web ACL (%s): %w", d.Id(), err) } if resp == nil || resp.WebACL == nil { if d.IsNewResource() { - return fmt.Errorf("error getting WAF Web ACL (%s): not found", d.Id()) + return fmt.Errorf("error reading WAF Web ACL (%s): not found", d.Id()) } log.Printf("[WARN] WAF Web ACL (%s) not found, removing from state", d.Id()) @@ -250,7 +250,7 @@ func resourceAwsWafWebAclRead(d *schema.ResourceData, meta interface{}) error { tags, err := keyvaluetags.WafListTags(conn, arn) if err != nil { - return fmt.Errorf("error listing tags for WAF Web ACL (%s): %w", d.Id(), err) + return fmt.Errorf("error listing tags for WAF Web ACL (%s): %w", arn, err) } if err := d.Set("tags", tags.IgnoreAws().IgnoreConfig(ignoreTagsConfig).Map()); err != nil { return fmt.Errorf("error setting tags: %w", err) @@ -267,8 +267,8 @@ func resourceAwsWafWebAclRead(d *schema.ResourceData, meta interface{}) error { getLoggingConfigurationOutput, err := conn.GetLoggingConfiguration(getLoggingConfigurationInput) - if err != nil && !isAWSErr(err, waf.ErrCodeNonexistentItemException, "") { - return fmt.Errorf("error getting WAF Web ACL (%s) Logging Configuration: %w", d.Id(), err) + if err != nil && !tfawserr.ErrCodeEquals(err, waf.ErrCodeNonexistentItemException) { + return fmt.Errorf("error reading WAF Web ACL (%s) Logging Configuration: %w", d.Id(), err) } if getLoggingConfigurationOutput != nil { @@ -368,9 +368,14 @@ func resourceAwsWafWebAclDelete(d *schema.ResourceData, meta interface{}) error return conn.DeleteWebACL(req) }) + if err != nil { + if tfawserr.ErrCodeEquals(err, waf.ErrCodeNonexistentItemException) { + return nil + } return fmt.Errorf("error deleting WAF Web ACL (%s): %w", d.Id(), err) } + return nil } diff --git a/aws/resource_aws_waf_web_acl_test.go b/aws/resource_aws_waf_web_acl_test.go index 20132473f210..4c089b219f6b 100644 --- a/aws/resource_aws_waf_web_acl_test.go +++ b/aws/resource_aws_waf_web_acl_test.go @@ -8,6 +8,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/waf" + "github.com/hashicorp/aws-sdk-go-base/tfawserr" "github.com/hashicorp/go-multierror" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" @@ -309,7 +310,7 @@ func TestAccAWSWafWebAcl_disappears(t *testing.T) { Config: testAccAWSWafWebAclConfig_Required(rName), Check: resource.ComposeTestCheckFunc( testAccCheckAWSWafWebAclExists(resourceName, &webACL), - testAccCheckAWSWafWebAclDisappears(&webACL), + testAccCheckResourceDisappears(testAccProvider, resourceAwsWafWebAcl(), resourceName), ), ExpectNonEmptyPlan: true, }, @@ -376,26 +377,6 @@ func TestAccAWSWafWebAcl_Tags(t *testing.T) { }) } -func testAccCheckAWSWafWebAclDisappears(v *waf.WebACL) resource.TestCheckFunc { - return func(s *terraform.State) error { - conn := testAccProvider.Meta().(*AWSClient).wafconn - - wr := newWafRetryer(conn) - - _, err := wr.RetryWithToken(func(token *string) (interface{}, error) { - opts := &waf.DeleteWebACLInput{ - ChangeToken: token, - WebACLId: v.WebACLId, - } - return conn.DeleteWebACL(opts) - }) - if err != nil { - return fmt.Errorf("Error Deleting WAF ACL: %s", err) - } - return nil - } -} - func testAccCheckAWSWafWebAclDestroy(s *terraform.State) error { for _, rs := range s.RootModule().Resources { if rs.Type != "aws_waf_web_acl" { @@ -408,18 +389,17 @@ func testAccCheckAWSWafWebAclDestroy(s *terraform.State) error { WebACLId: aws.String(rs.Primary.ID), }) - if err == nil { - if *resp.WebACL.WebACLId == rs.Primary.ID { - return fmt.Errorf("WebACL %s still exists", rs.Primary.ID) - } + if tfawserr.ErrCodeEquals(err, waf.ErrCodeNonexistentItemException) { + continue } - // Return nil if the WebACL is already destroyed - if isAWSErr(err, waf.ErrCodeNonexistentItemException, "") { - continue + if err != nil { + return fmt.Errorf("error reading WAF Web ACL (%s): %w", rs.Primary.ID, err) } - return err + if resp != nil && resp.WebACL != nil { + return fmt.Errorf("WAF Web ACL (%s) still exists", rs.Primary.ID) + } } return nil