Merge pull request #7887 from terraform-providers/b-aws_s3_bucket-pre…

…vent-empty-filter-crash resource/aws_s3_bucket: Prevent empty replication_configuration rules filter crash
hashicorp · Mar 14, 2019 · 1281fae · 1281fae
2 parents 84f1465 + e204667
commit 1281fae
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 6 deletions.
diff --git 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{}

diff --git 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
@@ -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
 		}
@@ -1598,11 +1608,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