Skip to content

Commit

Permalink
resource/s3_bucket_object: Add retry
Browse files Browse the repository at this point in the history
  • Loading branch information
@YakDriver
YakDriver committed Feb 12, 2021
1 parent 693d594 commit 39a98e0
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 39 deletions.
22 changes: 21 additions & 1 deletion aws/internal/keyvaluetags/s3_tags.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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
Expand Down
97 changes: 59 additions & 38 deletions aws/resource_aws_s3_bucket_object_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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{
Expand All @@ -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"),
Expand All @@ -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),
Expand All @@ -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),
Expand All @@ -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),
Expand All @@ -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{
Expand All @@ -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"),
Expand All @@ -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),
Expand All @@ -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),
Expand All @@ -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),
Expand All @@ -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{
Expand All @@ -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"),
Expand All @@ -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),
Expand All @@ -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),
Expand All @@ -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),
Expand All @@ -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{
Expand All @@ -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"),
Expand All @@ -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),
Expand All @@ -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),
Expand All @@ -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),
Expand Down Expand Up @@ -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{
Expand All @@ -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"),
Expand All @@ -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"),
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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 {
Expand Down

0 comments on commit 39a98e0

Please sign in to comment.