diff --git a/aws/internal/keyvaluetags/s3_tags.go b/aws/internal/keyvaluetags/s3_tags.go index 90182d759f06..a858b48e51c3 100644 --- a/aws/internal/keyvaluetags/s3_tags.go +++ b/aws/internal/keyvaluetags/s3_tags.go @@ -4,11 +4,15 @@ 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" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource" ) // Custom S3 tag service update functions using the same format as generated code. @@ -86,7 +90,27 @@ 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 tfresource.TimedOut(err) { + output, err = conn.GetObjectTagging(input) + } if tfawserr.ErrCodeEquals(err, tfs3.ErrCodeNoSuchTagSet) { return New(nil), nil diff --git a/aws/resource_aws_s3_bucket_object.go b/aws/resource_aws_s3_bucket_object.go index 76c63be4ed6b..fae139d95029 100644 --- a/aws/resource_aws_s3_bucket_object.go +++ b/aws/resource_aws_s3_bucket_object.go @@ -8,6 +8,7 @@ import ( "io" "log" "os" + "regexp" "strings" "time" @@ -462,8 +463,10 @@ func resourceAwsS3BucketObjectDelete(d *schema.ResourceData, meta interface{}) e bucket := d.Get("bucket").(string) key := d.Get("key").(string) - // We are effectively ignoring any leading '/' in the key name as aws.Config.DisableRestProtocolURICleaning is false - key = strings.TrimPrefix(key, "/") + // We are effectively ignoring all leading '/'s in the key name and + // treating multiple '/'s as a single '/' as aws.Config.DisableRestProtocolURICleaning is false + key = strings.TrimLeft(key, "/") + key = regexp.MustCompile(`/+`).ReplaceAllString(key, "/") var err error if _, ok := d.GetOk("version_id"); ok { diff --git a/aws/resource_aws_s3_bucket_object_test.go b/aws/resource_aws_s3_bucket_object_test.go index 79c7d31c28e7..8201af3f7694 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), @@ -702,10 +703,10 @@ func TestAccAWSS3BucketObject_tags(t *testing.T) { }) } -func TestAccAWSS3BucketObject_tagsLeadingSlash(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_tagsLeadingSlash(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_tagsLeadingSlash(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_tagsLeadingSlash(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,135 @@ func TestAccAWSS3BucketObject_tagsLeadingSlash(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), + testAccCheckAWSS3BucketObjectBody(&obj4, "changed stuff"), + resource.TestCheckResourceAttr(resourceName, "tags.%", "3"), + resource.TestCheckResourceAttr(resourceName, "tags.Key1", "AAA"), + resource.TestCheckResourceAttr(resourceName, "tags.Key2", "BBB"), + resource.TestCheckResourceAttr(resourceName, "tags.Key3", "CCC"), + ), + }, + }, + }) +} + +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" + key := "/////test-key" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSS3BucketObjectDestroy, + Steps: []resource.TestStep{ + { + PreConfig: func() {}, + Config: testAccAWSS3BucketObjectConfig_withTags(rName, key, "stuff"), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSS3BucketObjectExists(resourceName, &obj1), + testAccCheckAWSS3BucketObjectBody(&obj1, "stuff"), + resource.TestCheckResourceAttr(resourceName, "tags.%", "3"), + resource.TestCheckResourceAttr(resourceName, "tags.Key1", "AAA"), + resource.TestCheckResourceAttr(resourceName, "tags.Key2", "BBB"), + resource.TestCheckResourceAttr(resourceName, "tags.Key3", "CCC"), + ), + }, + { + PreConfig: func() {}, + Config: testAccAWSS3BucketObjectConfig_withUpdatedTags(rName, key, "stuff"), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSS3BucketObjectExists(resourceName, &obj2), + testAccCheckAWSS3BucketObjectVersionIdEquals(&obj2, &obj1), + testAccCheckAWSS3BucketObjectBody(&obj2, "stuff"), + resource.TestCheckResourceAttr(resourceName, "tags.%", "4"), + resource.TestCheckResourceAttr(resourceName, "tags.Key2", "BBB"), + resource.TestCheckResourceAttr(resourceName, "tags.Key3", "X X"), + resource.TestCheckResourceAttr(resourceName, "tags.Key4", "DDD"), + resource.TestCheckResourceAttr(resourceName, "tags.Key5", "E:/"), + ), + }, + { + PreConfig: func() {}, + Config: testAccAWSS3BucketObjectConfig_withNoTags(rName, key, "stuff"), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSS3BucketObjectExists(resourceName, &obj3), + testAccCheckAWSS3BucketObjectVersionIdEquals(&obj3, &obj2), + testAccCheckAWSS3BucketObjectBody(&obj3, "stuff"), + resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), + ), + }, + { + PreConfig: func() {}, + Config: testAccAWSS3BucketObjectConfig_withTags(rName, key, "changed stuff"), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSS3BucketObjectExists(resourceName, &obj4), + testAccCheckAWSS3BucketObjectVersionIdDiffers(&obj4, &obj3), + testAccCheckAWSS3BucketObjectBody(&obj4, "changed stuff"), + resource.TestCheckResourceAttr(resourceName, "tags.%", "3"), + resource.TestCheckResourceAttr(resourceName, "tags.Key1", "AAA"), + resource.TestCheckResourceAttr(resourceName, "tags.Key2", "BBB"), + resource.TestCheckResourceAttr(resourceName, "tags.Key3", "CCC"), + ), + }, + }, + }) +} + +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" + key := "first//second///third//" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSS3BucketObjectDestroy, + Steps: []resource.TestStep{ + { + PreConfig: func() {}, + Config: testAccAWSS3BucketObjectConfig_withTags(rName, key, "stuff"), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSS3BucketObjectExists(resourceName, &obj1), + testAccCheckAWSS3BucketObjectBody(&obj1, "stuff"), + resource.TestCheckResourceAttr(resourceName, "tags.%", "3"), + resource.TestCheckResourceAttr(resourceName, "tags.Key1", "AAA"), + resource.TestCheckResourceAttr(resourceName, "tags.Key2", "BBB"), + resource.TestCheckResourceAttr(resourceName, "tags.Key3", "CCC"), + ), + }, + { + PreConfig: func() {}, + Config: testAccAWSS3BucketObjectConfig_withUpdatedTags(rName, key, "stuff"), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSS3BucketObjectExists(resourceName, &obj2), + testAccCheckAWSS3BucketObjectVersionIdEquals(&obj2, &obj1), + testAccCheckAWSS3BucketObjectBody(&obj2, "stuff"), + resource.TestCheckResourceAttr(resourceName, "tags.%", "4"), + resource.TestCheckResourceAttr(resourceName, "tags.Key2", "BBB"), + resource.TestCheckResourceAttr(resourceName, "tags.Key3", "X X"), + resource.TestCheckResourceAttr(resourceName, "tags.Key4", "DDD"), + resource.TestCheckResourceAttr(resourceName, "tags.Key5", "E:/"), + ), + }, + { + PreConfig: func() {}, + Config: testAccAWSS3BucketObjectConfig_withNoTags(rName, key, "stuff"), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSS3BucketObjectExists(resourceName, &obj3), + testAccCheckAWSS3BucketObjectVersionIdEquals(&obj3, &obj2), + testAccCheckAWSS3BucketObjectBody(&obj3, "stuff"), + resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), + ), + }, + { + PreConfig: func() {}, + Config: testAccAWSS3BucketObjectConfig_withTags(rName, key, "changed stuff"), Check: resource.ComposeTestCheckFunc( testAccCheckAWSS3BucketObjectExists(resourceName, &obj4), testAccCheckAWSS3BucketObjectVersionIdDiffers(&obj4, &obj3), @@ -979,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{ @@ -992,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"), @@ -1007,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"), @@ -1094,12 +1223,35 @@ 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 isResourceTimeoutError(err) { + out, err = s3conn.GetObject(input) + } + if err != nil { return fmt.Errorf("S3Bucket Object error: %s", err) } @@ -1463,10 +1615,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 @@ -1484,13 +1636,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 @@ -1509,13 +1661,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 @@ -1527,7 +1679,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 {