From 2e36fca1dcf4c2218a1a136465ee37dfa33e5641 Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Mon, 11 Mar 2019 08:40:13 -0400 Subject: [PATCH 1/3] tests/resource/aws_s3_bucket: Prevent CheckDestroy errors due to eventual consistency Previously, these errors could be returned since bucket deletion may take a few seconds to propagate: ``` testing.go:599: Error destroying resource! WARNING: Dangling resources may exist. The full state and error is shown below. Error: Check failed: AWS S3 Bucket still exists: tf-test-bucket-865512847222774644 ``` Output from acceptance testing: ``` --- PASS: TestAccAWSS3Bucket_basic (26.11s) ``` --- aws/resource_aws_s3_bucket_test.go | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/aws/resource_aws_s3_bucket_test.go b/aws/resource_aws_s3_bucket_test.go index 87fb0809e02..325560e34fc 100644 --- a/aws/resource_aws_s3_bucket_test.go +++ b/aws/resource_aws_s3_bucket_test.go @@ -1598,11 +1598,31 @@ func testAccCheckAWSS3BucketDestroyWithProvider(s *terraform.State, provider *sc continue } - _, err := conn.HeadBucket(&s3.HeadBucketInput{ + input := &s3.HeadBucketInput{ Bucket: aws.String(rs.Primary.ID), + } + + // Retry for S3 eventual consistency + err := resource.Retry(1*time.Minute, func() *resource.RetryError { + _, err := conn.HeadBucket(input) + + if isAWSErr(err, s3.ErrCodeNoSuchBucket, "") || isAWSErr(err, "NotFound", "") { + return nil + } + + if err != nil { + return resource.NonRetryableError(err) + } + + return resource.RetryableError(fmt.Errorf("AWS S3 Bucket still exists: %s", rs.Primary.ID)) }) - if err == nil { - return fmt.Errorf("AWS S3 Bucket still exists: %s", rs.Primary.ID) + + if isResourceTimeoutError(err) { + _, err = conn.HeadBucket(input) + } + + if err != nil { + return err } } return nil From 30e646bce60b9e00f41ebc7baee7aac041f7da25 Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Mon, 11 Mar 2019 08:53:12 -0400 Subject: [PATCH 2/3] tests/service/s3: Add mybucket and mylogs prefixes to sweeping These two test bucket name prefixes were piling up. We are still keeping the prefix based checks with S3 Bucket names to allow internal users to migrate off their existing buckets. Previous output from acceptance testing: ``` testing.go:538: Step 0 error: Error applying: 1 error occurred: * aws_s3_bucket.destination: 1 error occurred: * aws_s3_bucket.destination: Error creating S3 bucket: TooManyBuckets: You have attempted to create more buckets than allowed ``` Output from test sweeper: ``` $ go test ./aws -v -sweep=us-west-2 -sweep-run=aws_s3_bucket -timeout 10h ... 2019/03/11 08:50:53 [INFO] Deleting S3 Bucket: mybucket.1023155772881419661 2019/03/11 08:50:54 [INFO] Deleting S3 Bucket: mybucket.1577116853589958138 2019/03/11 08:50:54 [INFO] Deleting S3 Bucket: mybucket.1626923880857291096 2019/03/11 08:50:55 [INFO] Deleting S3 Bucket: mybucket.1918546658572021590 2019/03/11 08:50:56 [INFO] Deleting S3 Bucket: mybucket.2529084512248573413 2019/03/11 08:50:57 [INFO] Deleting S3 Bucket: mybucket.3093679243342824574 2019/03/11 08:50:57 [INFO] Deleting S3 Bucket: mybucket.3585723648527080440 2019/03/11 08:50:58 [INFO] Deleting S3 Bucket: mybucket.3947240404810737197 2019/03/11 08:50:59 [INFO] Deleting S3 Bucket: mybucket.4249057747579529260 2019/03/11 08:51:00 [INFO] Deleting S3 Bucket: mybucket.4313397459697999834 2019/03/11 08:51:00 [INFO] Deleting S3 Bucket: mybucket.4525762606472232559 2019/03/11 08:51:01 [INFO] Deleting S3 Bucket: mybucket.4648730065658883877 2019/03/11 08:51:01 [INFO] Deleting S3 Bucket: mybucket.4823172894294433332 2019/03/11 08:51:03 [INFO] Deleting S3 Bucket: mybucket.5586690765808533355 2019/03/11 08:51:04 [INFO] Deleting S3 Bucket: mybucket.5862186744102500925 2019/03/11 08:51:05 [INFO] Deleting S3 Bucket: mybucket.6558243541687708420 2019/03/11 08:51:05 [INFO] Deleting S3 Bucket: mybucket.8348088465902968848 2019/03/11 08:51:06 [INFO] Deleting S3 Bucket: mybucket.8790461889364761310 2019/03/11 08:51:07 [INFO] Deleting S3 Bucket: mybucket.942914357360372416 2019/03/11 08:51:08 [INFO] Deleting S3 Bucket: mylogs.1023155772881419661 2019/03/11 08:51:09 [INFO] Deleting S3 Bucket: mylogs.1577116853589958138 2019/03/11 08:51:09 [INFO] Deleting S3 Bucket: mylogs.1626923880857291096 2019/03/11 08:51:10 [INFO] Deleting S3 Bucket: mylogs.1918546658572021590 2019/03/11 08:51:11 [INFO] Deleting S3 Bucket: mylogs.2831805700791414627 2019/03/11 08:51:13 [INFO] Deleting S3 Bucket: mylogs.3093679243342824574 2019/03/11 08:51:14 [INFO] Deleting S3 Bucket: mylogs.3102934574929067207 2019/03/11 08:51:15 [INFO] Deleting S3 Bucket: mylogs.320626190810004195 2019/03/11 08:51:17 [INFO] Deleting S3 Bucket: mylogs.3770914409175143465 2019/03/11 08:51:17 [INFO] Deleting S3 Bucket: mylogs.3947240404810737197 2019/03/11 08:51:18 [INFO] Deleting S3 Bucket: mylogs.4823172894294433332 2019/03/11 08:51:19 [INFO] Deleting S3 Bucket: mylogs.5107202730033269573 2019/03/11 08:51:19 [INFO] Deleting S3 Bucket: mylogs.5586690765808533355 2019/03/11 08:51:20 [INFO] Deleting S3 Bucket: mylogs.5788936017557252310 2019/03/11 08:51:21 [INFO] Deleting S3 Bucket: mylogs.5862186744102500925 2019/03/11 08:51:21 [INFO] Deleting S3 Bucket: mylogs.6558243541687708420 2019/03/11 08:51:22 [INFO] Deleting S3 Bucket: mylogs.7781653724497201590 2019/03/11 08:51:23 [INFO] Deleting S3 Bucket: mylogs.8348088465902968848 2019/03/11 08:51:24 [INFO] Deleting S3 Bucket: mylogs.8790461889364761310 2019/03/11 08:51:24 [INFO] Deleting S3 Bucket: mylogs.942914357360372416 ... 2019/03/11 08:51:25 Sweeper Tests ran: - aws_s3_bucket_object - aws_s3_bucket ok github.com/terraform-providers/terraform-provider-aws/aws 71.008s ``` --- aws/resource_aws_s3_bucket_object_test.go | 12 +++++++++++- aws/resource_aws_s3_bucket_test.go | 12 +++++++++++- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/aws/resource_aws_s3_bucket_object_test.go b/aws/resource_aws_s3_bucket_object_test.go index a037b17f7c5..338a7ce9210 100644 --- a/aws/resource_aws_s3_bucket_object_test.go +++ b/aws/resource_aws_s3_bucket_object_test.go @@ -53,7 +53,17 @@ func testSweepS3BucketObjects(region string) error { for _, bucket := range output.Buckets { bucketName := aws.StringValue(bucket.Name) - if !strings.HasPrefix(bucketName, "tf-acc") && !strings.HasPrefix(bucketName, "tf-object-test") && !strings.HasPrefix(bucketName, "tf-test") { + hasPrefix := false + prefixes := []string{"mybucket.", "mylogs.", "tf-acc", "tf-object-test", "tf-test"} + + for _, prefix := range prefixes { + if strings.HasPrefix(bucketName, prefix) { + hasPrefix = true + break + } + } + + if !hasPrefix { log.Printf("[INFO] Skipping S3 Bucket: %s", bucketName) continue } diff --git a/aws/resource_aws_s3_bucket_test.go b/aws/resource_aws_s3_bucket_test.go index 325560e34fc..e793404e8ff 100644 --- a/aws/resource_aws_s3_bucket_test.go +++ b/aws/resource_aws_s3_bucket_test.go @@ -59,7 +59,17 @@ func testSweepS3Buckets(region string) error { for _, bucket := range output.Buckets { name := aws.StringValue(bucket.Name) - if !strings.HasPrefix(name, "tf-acc") && !strings.HasPrefix(name, "tf-object-test") && !strings.HasPrefix(name, "tf-test") { + hasPrefix := false + prefixes := []string{"mybucket.", "mylogs.", "tf-acc", "tf-object-test", "tf-test"} + + for _, prefix := range prefixes { + if strings.HasPrefix(name, prefix) { + hasPrefix = true + break + } + } + + if !hasPrefix { log.Printf("[INFO] Skipping S3 Bucket: %s", name) continue } From e20466747caf9aaea7059b5f2f465bf7df99f448 Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Mon, 11 Mar 2019 09:10:32 -0400 Subject: [PATCH 3/3] resource/aws_s3_bucket: Prevent panic with empty replication_configuration rules filter References: * https://github.com/terraform-providers/terraform-provider-aws/issues/7427 * https://github.com/terraform-providers/terraform-provider-aws/issues/6600 This is a best effort fix given this may be caused by a manual console update and we do not have a replicating Terraform configuration. The previous code was missing a `nil` check before type assertion. Output from acceptance testing: ``` --- PASS: TestAccAWSS3Bucket_shouldFailNotFound (17.44s) --- PASS: TestAccAWSS3Bucket_Cors_EmptyOrigin (29.29s) --- PASS: TestAccAWSS3Bucket_enableDefaultEncryption_whenAES256IsUsed (29.94s) --- PASS: TestAccAWSS3Bucket_Cors_Delete (31.64s) --- PASS: TestAccAWSS3Bucket_importBasic (33.49s) --- PASS: TestAccAWSS3Bucket_ReplicationExpectVersioningValidationError (41.70s) --- PASS: TestAccAWSS3Bucket_Logging (47.80s) --- PASS: TestAccAWSS3Bucket_LifecycleExpireMarkerOnly (52.04s) --- PASS: TestAccAWSS3Bucket_disableDefaultEncryption_whenDefaultEncryptionIsEnabled (52.96s) --- PASS: TestAccAWSS3Bucket_Cors_Update (53.36s) --- PASS: TestAccAWSS3Bucket_objectLock (53.81s) --- PASS: TestAccAWSS3Bucket_enableDefaultEncryption_whenTypical (58.86s) --- PASS: TestAccAWSS3Bucket_WebsiteRoutingRules (48.75s) --- PASS: TestAccAWSS3Bucket_ReplicationWithoutStorageClass (70.75s) --- PASS: TestAccAWSS3Bucket_Policy (70.96s) --- PASS: TestAccAWSS3Bucket_ReplicationWithoutPrefix (72.03s) --- PASS: TestAccAWSS3Bucket_Versioning (72.13s) --- PASS: TestAccAWSS3Bucket_Lifecycle (73.01s) --- PASS: TestAccAWSS3Bucket_RequestPayer (47.45s) --- PASS: TestAccAWSS3Bucket_basic (24.91s) --- PASS: TestAccAWSS3Bucket_namePrefix (24.41s) --- PASS: TestAccAWSS3Bucket_region (31.70s) --- PASS: TestAccAWSS3Bucket_generatedName (26.78s) --- PASS: TestAccAWSS3Bucket_acceleration (49.96s) --- PASS: TestAccAWSS3Bucket_importWithPolicy (35.90s) --- PASS: TestAccAWSS3Bucket_UpdateAcl (48.55s) --- PASS: TestAccAWSS3Bucket_WebsiteRedirect (70.34s) --- PASS: TestAccAWSS3Bucket_Website_Simple (70.38s) --- PASS: TestAccAWSS3Bucket_ReplicationConfiguration_Rule_Destination_AccessControlTranslation (148.13s) --- PASS: TestAccAWSS3Bucket_ReplicationSchemaV2 (216.56s) --- PASS: TestAccAWSS3Bucket_Replication (225.08s) ``` --- aws/resource_aws_s3_bucket.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aws/resource_aws_s3_bucket.go b/aws/resource_aws_s3_bucket.go index ae935d56baa..df0a9438e63 100644 --- a/aws/resource_aws_s3_bucket.go +++ b/aws/resource_aws_s3_bucket.go @@ -1931,7 +1931,7 @@ func resourceAwsS3BucketReplicationConfigurationUpdate(s3conn *s3.S3, d *schema. rcRule.SourceSelectionCriteria = ruleSsc } - if f, ok := rr["filter"].([]interface{}); ok && len(f) > 0 { + if f, ok := rr["filter"].([]interface{}); ok && len(f) > 0 && f[0] != nil { // XML schema V2. rcRule.Priority = aws.Int64(int64(rr["priority"].(int))) rcRule.Filter = &s3.ReplicationRuleFilter{}