From 39a98e033eb8468369171b02c6e3478dad88754b Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Thu, 11 Feb 2021 19:27:26 -0500 Subject: [PATCH] resource/s3_bucket_object: Add retry --- aws/internal/keyvaluetags/s3_tags.go | 22 ++++- aws/resource_aws_s3_bucket_object_test.go | 97 ++++++++++++++--------- 2 files changed, 80 insertions(+), 39 deletions(-) diff --git a/aws/internal/keyvaluetags/s3_tags.go b/aws/internal/keyvaluetags/s3_tags.go index 90182d759f0..995547b1c29 100644 --- a/aws/internal/keyvaluetags/s3_tags.go +++ b/aws/internal/keyvaluetags/s3_tags.go @@ -4,10 +4,13 @@ package keyvaluetags import ( "fmt" + "time" "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/service/s3" "github.com/hashicorp/aws-sdk-go-base/tfawserr" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" tfs3 "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/s3" ) @@ -86,7 +89,24 @@ func S3ObjectListTags(conn *s3.S3, bucket, key string) (KeyValueTags, error) { Key: aws.String(key), } - output, err := conn.GetObjectTagging(input) + var output *s3.GetObjectTaggingOutput + + err := resource.Retry(1*time.Minute, func() *resource.RetryError { + var err error + output, err = conn.GetObjectTagging(input) + if awsErr, ok := err.(awserr.Error); ok { + if awsErr.Code() == "NoSuchKey" { + return resource.RetryableError( + fmt.Errorf("getting object tagging %s, retrying: %w", bucket, err), + ) + } + } + if err != nil { + return resource.NonRetryableError(err) + } + + return nil + }) if tfawserr.ErrCodeEquals(err, tfs3.ErrCodeNoSuchTagSet) { return New(nil), nil diff --git a/aws/resource_aws_s3_bucket_object_test.go b/aws/resource_aws_s3_bucket_object_test.go index 6af10d88478..de759a0d1d7 100644 --- a/aws/resource_aws_s3_bucket_object_test.go +++ b/aws/resource_aws_s3_bucket_object_test.go @@ -14,6 +14,7 @@ import ( "time" "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/service/s3" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" @@ -640,8 +641,8 @@ func TestAccAWSS3BucketObject_storageClass(t *testing.T) { func TestAccAWSS3BucketObject_tags(t *testing.T) { var obj1, obj2, obj3, obj4 s3.GetObjectOutput + rName := acctest.RandomWithPrefix("tf-acc-test") resourceName := "aws_s3_bucket_object.object" - rInt := acctest.RandInt() key := "test-key" resource.ParallelTest(t, resource.TestCase{ @@ -651,7 +652,7 @@ func TestAccAWSS3BucketObject_tags(t *testing.T) { Steps: []resource.TestStep{ { PreConfig: func() {}, - Config: testAccAWSS3BucketObjectConfig_withTags(rInt, key, "stuff"), + Config: testAccAWSS3BucketObjectConfig_withTags(rName, key, "stuff"), Check: resource.ComposeTestCheckFunc( testAccCheckAWSS3BucketObjectExists(resourceName, &obj1), testAccCheckAWSS3BucketObjectBody(&obj1, "stuff"), @@ -663,7 +664,7 @@ func TestAccAWSS3BucketObject_tags(t *testing.T) { }, { PreConfig: func() {}, - Config: testAccAWSS3BucketObjectConfig_withUpdatedTags(rInt, key, "stuff"), + Config: testAccAWSS3BucketObjectConfig_withUpdatedTags(rName, key, "stuff"), Check: resource.ComposeTestCheckFunc( testAccCheckAWSS3BucketObjectExists(resourceName, &obj2), testAccCheckAWSS3BucketObjectVersionIdEquals(&obj2, &obj1), @@ -677,7 +678,7 @@ func TestAccAWSS3BucketObject_tags(t *testing.T) { }, { PreConfig: func() {}, - Config: testAccAWSS3BucketObjectConfig_withNoTags(rInt, key, "stuff"), + Config: testAccAWSS3BucketObjectConfig_withNoTags(rName, key, "stuff"), Check: resource.ComposeTestCheckFunc( testAccCheckAWSS3BucketObjectExists(resourceName, &obj3), testAccCheckAWSS3BucketObjectVersionIdEquals(&obj3, &obj2), @@ -687,7 +688,7 @@ func TestAccAWSS3BucketObject_tags(t *testing.T) { }, { PreConfig: func() {}, - Config: testAccAWSS3BucketObjectConfig_withTags(rInt, key, "changed stuff"), + Config: testAccAWSS3BucketObjectConfig_withTags(rName, key, "changed stuff"), Check: resource.ComposeTestCheckFunc( testAccCheckAWSS3BucketObjectExists(resourceName, &obj4), testAccCheckAWSS3BucketObjectVersionIdDiffers(&obj4, &obj3), @@ -704,8 +705,8 @@ func TestAccAWSS3BucketObject_tags(t *testing.T) { func TestAccAWSS3BucketObject_tagsLeadingSingleSlash(t *testing.T) { var obj1, obj2, obj3, obj4 s3.GetObjectOutput + rName := acctest.RandomWithPrefix("tf-acc-test") resourceName := "aws_s3_bucket_object.object" - rInt := acctest.RandInt() key := "/test-key" resource.ParallelTest(t, resource.TestCase{ @@ -715,7 +716,7 @@ func TestAccAWSS3BucketObject_tagsLeadingSingleSlash(t *testing.T) { Steps: []resource.TestStep{ { PreConfig: func() {}, - Config: testAccAWSS3BucketObjectConfig_withTags(rInt, key, "stuff"), + Config: testAccAWSS3BucketObjectConfig_withTags(rName, key, "stuff"), Check: resource.ComposeTestCheckFunc( testAccCheckAWSS3BucketObjectExists(resourceName, &obj1), testAccCheckAWSS3BucketObjectBody(&obj1, "stuff"), @@ -727,7 +728,7 @@ func TestAccAWSS3BucketObject_tagsLeadingSingleSlash(t *testing.T) { }, { PreConfig: func() {}, - Config: testAccAWSS3BucketObjectConfig_withUpdatedTags(rInt, key, "stuff"), + Config: testAccAWSS3BucketObjectConfig_withUpdatedTags(rName, key, "stuff"), Check: resource.ComposeTestCheckFunc( testAccCheckAWSS3BucketObjectExists(resourceName, &obj2), testAccCheckAWSS3BucketObjectVersionIdEquals(&obj2, &obj1), @@ -741,7 +742,7 @@ func TestAccAWSS3BucketObject_tagsLeadingSingleSlash(t *testing.T) { }, { PreConfig: func() {}, - Config: testAccAWSS3BucketObjectConfig_withNoTags(rInt, key, "stuff"), + Config: testAccAWSS3BucketObjectConfig_withNoTags(rName, key, "stuff"), Check: resource.ComposeTestCheckFunc( testAccCheckAWSS3BucketObjectExists(resourceName, &obj3), testAccCheckAWSS3BucketObjectVersionIdEquals(&obj3, &obj2), @@ -751,7 +752,7 @@ func TestAccAWSS3BucketObject_tagsLeadingSingleSlash(t *testing.T) { }, { PreConfig: func() {}, - Config: testAccAWSS3BucketObjectConfig_withTags(rInt, key, "changed stuff"), + Config: testAccAWSS3BucketObjectConfig_withTags(rName, key, "changed stuff"), Check: resource.ComposeTestCheckFunc( testAccCheckAWSS3BucketObjectExists(resourceName, &obj4), testAccCheckAWSS3BucketObjectVersionIdDiffers(&obj4, &obj3), @@ -768,8 +769,8 @@ func TestAccAWSS3BucketObject_tagsLeadingSingleSlash(t *testing.T) { func TestAccAWSS3BucketObject_tagsLeadingMultipleSlashes(t *testing.T) { var obj1, obj2, obj3, obj4 s3.GetObjectOutput + rName := acctest.RandomWithPrefix("tf-acc-test") resourceName := "aws_s3_bucket_object.object" - rInt := acctest.RandInt() key := "/////test-key" resource.ParallelTest(t, resource.TestCase{ @@ -779,7 +780,7 @@ func TestAccAWSS3BucketObject_tagsLeadingMultipleSlashes(t *testing.T) { Steps: []resource.TestStep{ { PreConfig: func() {}, - Config: testAccAWSS3BucketObjectConfig_withTags(rInt, key, "stuff"), + Config: testAccAWSS3BucketObjectConfig_withTags(rName, key, "stuff"), Check: resource.ComposeTestCheckFunc( testAccCheckAWSS3BucketObjectExists(resourceName, &obj1), testAccCheckAWSS3BucketObjectBody(&obj1, "stuff"), @@ -791,7 +792,7 @@ func TestAccAWSS3BucketObject_tagsLeadingMultipleSlashes(t *testing.T) { }, { PreConfig: func() {}, - Config: testAccAWSS3BucketObjectConfig_withUpdatedTags(rInt, key, "stuff"), + Config: testAccAWSS3BucketObjectConfig_withUpdatedTags(rName, key, "stuff"), Check: resource.ComposeTestCheckFunc( testAccCheckAWSS3BucketObjectExists(resourceName, &obj2), testAccCheckAWSS3BucketObjectVersionIdEquals(&obj2, &obj1), @@ -805,7 +806,7 @@ func TestAccAWSS3BucketObject_tagsLeadingMultipleSlashes(t *testing.T) { }, { PreConfig: func() {}, - Config: testAccAWSS3BucketObjectConfig_withNoTags(rInt, key, "stuff"), + Config: testAccAWSS3BucketObjectConfig_withNoTags(rName, key, "stuff"), Check: resource.ComposeTestCheckFunc( testAccCheckAWSS3BucketObjectExists(resourceName, &obj3), testAccCheckAWSS3BucketObjectVersionIdEquals(&obj3, &obj2), @@ -815,7 +816,7 @@ func TestAccAWSS3BucketObject_tagsLeadingMultipleSlashes(t *testing.T) { }, { PreConfig: func() {}, - Config: testAccAWSS3BucketObjectConfig_withTags(rInt, key, "changed stuff"), + Config: testAccAWSS3BucketObjectConfig_withTags(rName, key, "changed stuff"), Check: resource.ComposeTestCheckFunc( testAccCheckAWSS3BucketObjectExists(resourceName, &obj4), testAccCheckAWSS3BucketObjectVersionIdDiffers(&obj4, &obj3), @@ -832,8 +833,8 @@ func TestAccAWSS3BucketObject_tagsLeadingMultipleSlashes(t *testing.T) { func TestAccAWSS3BucketObject_tagsMultipleSlashes(t *testing.T) { var obj1, obj2, obj3, obj4 s3.GetObjectOutput + rName := acctest.RandomWithPrefix("tf-acc-test") resourceName := "aws_s3_bucket_object.object" - rInt := acctest.RandInt() key := "first//second///third//" resource.ParallelTest(t, resource.TestCase{ @@ -843,7 +844,7 @@ func TestAccAWSS3BucketObject_tagsMultipleSlashes(t *testing.T) { Steps: []resource.TestStep{ { PreConfig: func() {}, - Config: testAccAWSS3BucketObjectConfig_withTags(rInt, key, "stuff"), + Config: testAccAWSS3BucketObjectConfig_withTags(rName, key, "stuff"), Check: resource.ComposeTestCheckFunc( testAccCheckAWSS3BucketObjectExists(resourceName, &obj1), testAccCheckAWSS3BucketObjectBody(&obj1, "stuff"), @@ -855,7 +856,7 @@ func TestAccAWSS3BucketObject_tagsMultipleSlashes(t *testing.T) { }, { PreConfig: func() {}, - Config: testAccAWSS3BucketObjectConfig_withUpdatedTags(rInt, key, "stuff"), + Config: testAccAWSS3BucketObjectConfig_withUpdatedTags(rName, key, "stuff"), Check: resource.ComposeTestCheckFunc( testAccCheckAWSS3BucketObjectExists(resourceName, &obj2), testAccCheckAWSS3BucketObjectVersionIdEquals(&obj2, &obj1), @@ -869,7 +870,7 @@ func TestAccAWSS3BucketObject_tagsMultipleSlashes(t *testing.T) { }, { PreConfig: func() {}, - Config: testAccAWSS3BucketObjectConfig_withNoTags(rInt, key, "stuff"), + Config: testAccAWSS3BucketObjectConfig_withNoTags(rName, key, "stuff"), Check: resource.ComposeTestCheckFunc( testAccCheckAWSS3BucketObjectExists(resourceName, &obj3), testAccCheckAWSS3BucketObjectVersionIdEquals(&obj3, &obj2), @@ -879,7 +880,7 @@ func TestAccAWSS3BucketObject_tagsMultipleSlashes(t *testing.T) { }, { PreConfig: func() {}, - Config: testAccAWSS3BucketObjectConfig_withTags(rInt, key, "changed stuff"), + Config: testAccAWSS3BucketObjectConfig_withTags(rName, key, "changed stuff"), Check: resource.ComposeTestCheckFunc( testAccCheckAWSS3BucketObjectExists(resourceName, &obj4), testAccCheckAWSS3BucketObjectVersionIdDiffers(&obj4, &obj3), @@ -1107,8 +1108,8 @@ func TestAccAWSS3BucketObject_defaultBucketSSE(t *testing.T) { func TestAccAWSS3BucketObject_ignoreTags(t *testing.T) { var obj s3.GetObjectOutput + rName := acctest.RandomWithPrefix("tf-acc-test") resourceName := "aws_s3_bucket_object.object" - rInt := acctest.RandInt() key := "test-key" resource.ParallelTest(t, resource.TestCase{ @@ -1120,7 +1121,7 @@ func TestAccAWSS3BucketObject_ignoreTags(t *testing.T) { PreConfig: func() {}, Config: composeConfig( testAccProviderConfigIgnoreTagsKeyPrefixes1("ignorekey"), - testAccAWSS3BucketObjectConfig_withNoTags(rInt, key, "stuff")), + testAccAWSS3BucketObjectConfig_withNoTags(rName, key, "stuff")), Check: resource.ComposeTestCheckFunc( testAccCheckAWSS3BucketObjectExists(resourceName, &obj), testAccCheckAWSS3BucketObjectBody(&obj, "stuff"), @@ -1135,7 +1136,7 @@ func TestAccAWSS3BucketObject_ignoreTags(t *testing.T) { PreConfig: func() {}, Config: composeConfig( testAccProviderConfigIgnoreTagsKeyPrefixes1("ignorekey"), - testAccAWSS3BucketObjectConfig_withTags(rInt, key, "stuff")), + testAccAWSS3BucketObjectConfig_withTags(rName, key, "stuff")), Check: resource.ComposeTestCheckFunc( testAccCheckAWSS3BucketObjectExists(resourceName, &obj), testAccCheckAWSS3BucketObjectBody(&obj, "stuff"), @@ -1222,12 +1223,32 @@ func testAccCheckAWSS3BucketObjectExists(n string, obj *s3.GetObjectOutput) reso } s3conn := testAccProvider.Meta().(*AWSClient).s3conn - out, err := s3conn.GetObject( - &s3.GetObjectInput{ - Bucket: aws.String(rs.Primary.Attributes["bucket"]), - Key: aws.String(rs.Primary.Attributes["key"]), - IfMatch: aws.String(rs.Primary.Attributes["etag"]), - }) + + input := &s3.GetObjectInput{ + Bucket: aws.String(rs.Primary.Attributes["bucket"]), + Key: aws.String(rs.Primary.Attributes["key"]), + IfMatch: aws.String(rs.Primary.Attributes["etag"]), + } + + var out *s3.GetObjectOutput + + err := resource.Retry(2*time.Minute, func() *resource.RetryError { + var err error + out, err = s3conn.GetObject(input) + if awsErr, ok := err.(awserr.Error); ok { + if awsErr.Code() == "NoSuchKey" { + return resource.RetryableError( + fmt.Errorf("getting object %s, retrying: %w", rs.Primary.Attributes["bucket"], err), + ) + } + } + if err != nil { + return resource.NonRetryableError(err) + } + + return nil + }) + if err != nil { return fmt.Errorf("S3Bucket Object error: %s", err) } @@ -1591,10 +1612,10 @@ resource "aws_s3_bucket_object" "object" { `, randInt, storage_class) } -func testAccAWSS3BucketObjectConfig_withTags(randInt int, key, content string) string { +func testAccAWSS3BucketObjectConfig_withTags(rName, key, content string) string { return fmt.Sprintf(` resource "aws_s3_bucket" "object_bucket" { - bucket = "tf-object-test-bucket-%[1]d" + bucket = %[1]q versioning { enabled = true @@ -1612,13 +1633,13 @@ resource "aws_s3_bucket_object" "object" { Key3 = "CCC" } } -`, randInt, key, content) +`, rName, key, content) } -func testAccAWSS3BucketObjectConfig_withUpdatedTags(randInt int, key, content string) string { +func testAccAWSS3BucketObjectConfig_withUpdatedTags(rName, key, content string) string { return fmt.Sprintf(` resource "aws_s3_bucket" "object_bucket" { - bucket = "tf-object-test-bucket-%[1]d" + bucket = %[1]q versioning { enabled = true @@ -1637,13 +1658,13 @@ resource "aws_s3_bucket_object" "object" { Key5 = "E:/" } } -`, randInt, key, content) +`, rName, key, content) } -func testAccAWSS3BucketObjectConfig_withNoTags(randInt int, key, content string) string { +func testAccAWSS3BucketObjectConfig_withNoTags(rName, key, content string) string { return fmt.Sprintf(` resource "aws_s3_bucket" "object_bucket" { - bucket = "tf-object-test-bucket-%[1]d" + bucket = %[1]q versioning { enabled = true @@ -1655,7 +1676,7 @@ resource "aws_s3_bucket_object" "object" { key = %[2]q content = %[3]q } -`, randInt, key, content) +`, rName, key, content) } func testAccAWSS3BucketObjectConfig_withMetadata(randInt int, metadataKey1, metadataValue1, metadataKey2, metadataValue2 string) string {