From d2b21b9c1d26b905ab64c7d61f5d23eae323a857 Mon Sep 17 00:00:00 2001
From: Brian Flad <bflad417@gmail.com>
Date: Wed, 13 Nov 2019 19:32:53 -0500
Subject: [PATCH] resource/aws_s3_bucket: Retry GetBucketTagging API calls on
 NoSuchBucket errors due to eventual consistency

Reference: https://github.com/terraform-providers/terraform-provider-aws/issues/10068

Previously, our acceptance testing would report eventual consistency issues such as:

```
--- FAIL: TestAccAWSS3Bucket_tagsWithSystemTags (11.28s)
    testing.go:615: Step 0 error: errors during apply:

        Error: error getting S3 bucket tags: NoSuchBucket: The specified bucket does not exist

--- FAIL: TestAccAWSCodeBuildProject_Artifacts_ArtifactIdentifier (7.39s)
    testing.go:615: Step 0 error: errors during apply:

        Error: error getting S3 bucket tags: NoSuchBucket: The specified bucket does not exist
```

The `aws_s3_bucket` resource tends to call our `retryOnAwsCode()` helper function to deal with this type of eventual consistency issue so we also add that handling to this API call as well.

Output from acceptance testing (after retrying any failing tests due to other read-after-write eventual consistency issues):

```
--- PASS: TestAccAWSS3Bucket_acceleration (57.29s)
--- PASS: TestAccAWSS3Bucket_basic (32.63s)
--- PASS: TestAccAWSS3Bucket_Bucket_EmptyString (30.76s)
--- PASS: TestAccAWSS3Bucket_Cors_Delete (29.45s)
--- PASS: TestAccAWSS3Bucket_Cors_EmptyOrigin (32.09s)
--- PASS: TestAccAWSS3Bucket_Cors_Update (55.25s)
--- PASS: TestAccAWSS3Bucket_disableDefaultEncryption_whenDefaultEncryptionIsEnabled (52.43s)
--- PASS: TestAccAWSS3Bucket_enableDefaultEncryption_whenAES256IsUsed (30.94s)
--- PASS: TestAccAWSS3Bucket_enableDefaultEncryption_whenTypical (61.25s)
--- PASS: TestAccAWSS3Bucket_forceDestroy (26.03s)
--- PASS: TestAccAWSS3Bucket_forceDestroyWithEmptyPrefixes (26.81s)
--- PASS: TestAccAWSS3Bucket_forceDestroyWithObjectLockEnabled (33.29s)
--- PASS: TestAccAWSS3Bucket_generatedName (31.31s)
--- PASS: TestAccAWSS3Bucket_LifecycleBasic (78.75s)
--- PASS: TestAccAWSS3Bucket_LifecycleExpireMarkerOnly (52.02s)
--- PASS: TestAccAWSS3Bucket_Logging (49.71s)
--- PASS: TestAccAWSS3Bucket_namePrefix (30.67s)
--- PASS: TestAccAWSS3Bucket_objectLock (52.31s)
--- PASS: TestAccAWSS3Bucket_Policy (70.45s)
--- PASS: TestAccAWSS3Bucket_region (29.50s)
--- PASS: TestAccAWSS3Bucket_Replication (223.64s)
--- PASS: TestAccAWSS3Bucket_ReplicationConfiguration_Rule_Destination_AccessControlTranslation (141.38s)
--- PASS: TestAccAWSS3Bucket_ReplicationExpectVersioningValidationError (35.00s)
--- PASS: TestAccAWSS3Bucket_ReplicationSchemaV2 (231.22s)
--- PASS: TestAccAWSS3Bucket_ReplicationWithoutPrefix (67.87s)
--- PASS: TestAccAWSS3Bucket_RequestPayer (53.47s)
--- PASS: TestAccAWSS3Bucket_shouldFailNotFound (13.40s)
--- PASS: TestAccAWSS3Bucket_tagsWithNoSystemTags (94.76s)
--- PASS: TestAccAWSS3Bucket_tagsWithSystemTags (144.59s)
--- PASS: TestAccAWSS3Bucket_UpdateAcl (53.37s)
--- PASS: TestAccAWSS3Bucket_WebsiteRedirect (77.20s)
--- PASS: TestAccAWSS3Bucket_WebsiteRoutingRules (57.40s)
```
---
 aws/s3_tags.go | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/aws/s3_tags.go b/aws/s3_tags.go
index 3f99c16982d7..5e8376e84e1b 100644
--- a/aws/s3_tags.go
+++ b/aws/s3_tags.go
@@ -14,17 +14,32 @@ import (
 // s3.GetBucketTagging, except returns an empty slice instead of an error when
 // there are no tags.
 func getTagSetS3Bucket(conn *s3.S3, bucket string) ([]*s3.Tag, error) {
-	resp, err := conn.GetBucketTagging(&s3.GetBucketTaggingInput{
+	input := &s3.GetBucketTaggingInput{
 		Bucket: aws.String(bucket),
+	}
+
+	// Retry due to S3 eventual consistency
+	outputRaw, err := retryOnAwsCode(s3.ErrCodeNoSuchBucket, func() (interface{}, error) {
+		return conn.GetBucketTagging(input)
 	})
+
+	// S3 API Reference (https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketTagging.html)
+	// lists the special error as NoSuchTagSetError, however the existing logic used NoSuchTagSet
+	// and the AWS Go SDK has neither as a constant.
+	if isAWSErr(err, "NoSuchTagSet", "") {
+		return nil, nil
+	}
+
 	if err != nil {
-		if isAWSErr(err, "NoSuchTagSet", "") {
-			return nil, nil
-		}
 		return nil, err
 	}
 
-	return resp.TagSet, nil
+	var tagSet []*s3.Tag
+	if output, ok := outputRaw.(*s3.GetBucketTaggingOutput); ok {
+		tagSet = output.TagSet
+	}
+
+	return tagSet, nil
 }
 
 // setTags is a helper to set the tags for a resource. It expects the