From bcff8bc4d638cbd352a95895efc3ca43f95948af Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 5 Sep 2023 13:57:31 -0400 Subject: [PATCH 01/25] s3: 'FindObjectByThreePartKey' -> 'FindObjectByThreePartKeyV1'. --- internal/service/s3/bucket_object.go | 34 ++++++++++++++++++++++- internal/service/s3/bucket_object_test.go | 6 ++-- internal/service/s3/object.go | 33 +--------------------- internal/service/s3/object_test.go | 6 ++-- 4 files changed, 40 insertions(+), 39 deletions(-) diff --git a/internal/service/s3/bucket_object.go b/internal/service/s3/bucket_object.go index 0d2e71360438..1b58137f7feb 100644 --- a/internal/service/s3/bucket_object.go +++ b/internal/service/s3/bucket_object.go @@ -14,6 +14,7 @@ import ( "fmt" "io" "log" + "net/http" "os" "strings" "time" @@ -22,8 +23,10 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/s3" "github.com/aws/aws-sdk-go/service/s3/s3manager" + "github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" "github.com/hashicorp/terraform-provider-aws/internal/conns" @@ -209,7 +212,7 @@ func resourceBucketObjectRead(ctx context.Context, d *schema.ResourceData, meta bucket := d.Get("bucket").(string) key := d.Get("key").(string) outputRaw, err := tfresource.RetryWhenNewResourceNotFound(ctx, objectCreationTimeout, func() (interface{}, error) { - return FindObjectByThreePartKey(ctx, conn, bucket, key, "") + return FindObjectByThreePartKeyV1(ctx, conn, bucket, key, "") }, d.IsNewResource()) if !d.IsNewResource() && tfresource.NotFound(err) { @@ -579,3 +582,32 @@ func hasBucketObjectContentChanges(d verify.ResourceDiffer) bool { } return false } + +func FindObjectByThreePartKeyV1(ctx context.Context, conn *s3.S3, bucket, key, etag string) (*s3.HeadObjectOutput, error) { + input := &s3.HeadObjectInput{ + Bucket: aws.String(bucket), + Key: aws.String(key), + } + if etag != "" { + input.IfMatch = aws.String(etag) + } + + output, err := conn.HeadObjectWithContext(ctx, input) + + if tfawserr.ErrStatusCodeEquals(err, http.StatusNotFound) { + return nil, &retry.NotFoundError{ + LastError: err, + LastRequest: input, + } + } + + if err != nil { + return nil, err + } + + if output == nil { + return nil, tfresource.NewEmptyResultError(input) + } + + return output, nil +} diff --git a/internal/service/s3/bucket_object_test.go b/internal/service/s3/bucket_object_test.go index f0abe4867430..7c1b4230bcea 100644 --- a/internal/service/s3/bucket_object_test.go +++ b/internal/service/s3/bucket_object_test.go @@ -1388,7 +1388,7 @@ func testAccCheckBucketObjectDestroy(ctx context.Context) resource.TestCheckFunc continue } - _, err := tfs3.FindObjectByThreePartKey(ctx, conn, rs.Primary.Attributes["bucket"], rs.Primary.Attributes["key"], rs.Primary.Attributes["etag"]) + _, err := tfs3.FindObjectByThreePartKeyV1(ctx, conn, rs.Primary.Attributes["bucket"], rs.Primary.Attributes["key"], rs.Primary.Attributes["etag"]) if tfresource.NotFound(err) { continue @@ -1505,7 +1505,7 @@ func testAccCheckBucketObjectStorageClass(ctx context.Context, n, expectedClass rs := s.RootModule().Resources[n] conn := acctest.Provider.Meta().(*conns.AWSClient).S3Conn(ctx) - out, err := tfs3.FindObjectByThreePartKey(ctx, conn, rs.Primary.Attributes["bucket"], rs.Primary.Attributes["key"], "") + out, err := tfs3.FindObjectByThreePartKeyV1(ctx, conn, rs.Primary.Attributes["bucket"], rs.Primary.Attributes["key"], "") if err != nil { return err @@ -1532,7 +1532,7 @@ func testAccCheckBucketObjectSSE(ctx context.Context, n, expectedSSE string) res rs := s.RootModule().Resources[n] conn := acctest.Provider.Meta().(*conns.AWSClient).S3Conn(ctx) - out, err := tfs3.FindObjectByThreePartKey(ctx, conn, rs.Primary.Attributes["bucket"], rs.Primary.Attributes["key"], "") + out, err := tfs3.FindObjectByThreePartKeyV1(ctx, conn, rs.Primary.Attributes["bucket"], rs.Primary.Attributes["key"], "") if err != nil { return err diff --git a/internal/service/s3/object.go b/internal/service/s3/object.go index e32c18eb2181..4b7b97916392 100644 --- a/internal/service/s3/object.go +++ b/internal/service/s3/object.go @@ -10,7 +10,6 @@ import ( "fmt" "io" "log" - "net/http" "os" "strings" "time" @@ -22,7 +21,6 @@ import ( "github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" "github.com/hashicorp/terraform-provider-aws/internal/conns" @@ -206,7 +204,7 @@ func resourceObjectRead(ctx context.Context, d *schema.ResourceData, meta interf bucket := d.Get("bucket").(string) key := d.Get("key").(string) outputRaw, err := tfresource.RetryWhenNewResourceNotFound(ctx, objectCreationTimeout, func() (interface{}, error) { - return FindObjectByThreePartKey(ctx, conn, bucket, key, "") + return FindObjectByThreePartKeyV1(ctx, conn, bucket, key, "") }, d.IsNewResource()) if !d.IsNewResource() && tfresource.NotFound(err) { @@ -789,32 +787,3 @@ func flattenObjectDate(t *time.Time) string { return t.Format(time.RFC3339) } - -func FindObjectByThreePartKey(ctx context.Context, conn *s3.S3, bucket, key, etag string) (*s3.HeadObjectOutput, error) { - input := &s3.HeadObjectInput{ - Bucket: aws.String(bucket), - Key: aws.String(key), - } - if etag != "" { - input.IfMatch = aws.String(etag) - } - - output, err := conn.HeadObjectWithContext(ctx, input) - - if tfawserr.ErrStatusCodeEquals(err, http.StatusNotFound) { - return nil, &retry.NotFoundError{ - LastError: err, - LastRequest: input, - } - } - - if err != nil { - return nil, err - } - - if output == nil { - return nil, tfresource.NewEmptyResultError(input) - } - - return output, nil -} diff --git a/internal/service/s3/object_test.go b/internal/service/s3/object_test.go index 05e2ecb7514a..81307f4aa82e 100644 --- a/internal/service/s3/object_test.go +++ b/internal/service/s3/object_test.go @@ -1416,7 +1416,7 @@ func testAccCheckObjectDestroy(ctx context.Context) resource.TestCheckFunc { continue } - _, err := tfs3.FindObjectByThreePartKey(ctx, conn, rs.Primary.Attributes["bucket"], rs.Primary.Attributes["key"], rs.Primary.Attributes["etag"]) + _, err := tfs3.FindObjectByThreePartKeyV1(ctx, conn, rs.Primary.Attributes["bucket"], rs.Primary.Attributes["key"], rs.Primary.Attributes["etag"]) if tfresource.NotFound(err) { continue @@ -1533,7 +1533,7 @@ func testAccCheckObjectStorageClass(ctx context.Context, n, expectedClass string rs := s.RootModule().Resources[n] conn := acctest.Provider.Meta().(*conns.AWSClient).S3Conn(ctx) - out, err := tfs3.FindObjectByThreePartKey(ctx, conn, rs.Primary.Attributes["bucket"], rs.Primary.Attributes["key"], "") + out, err := tfs3.FindObjectByThreePartKeyV1(ctx, conn, rs.Primary.Attributes["bucket"], rs.Primary.Attributes["key"], "") if err != nil { return err @@ -1560,7 +1560,7 @@ func testAccCheckObjectSSE(ctx context.Context, n, expectedSSE string) resource. rs := s.RootModule().Resources[n] conn := acctest.Provider.Meta().(*conns.AWSClient).S3Conn(ctx) - out, err := tfs3.FindObjectByThreePartKey(ctx, conn, rs.Primary.Attributes["bucket"], rs.Primary.Attributes["key"], "") + out, err := tfs3.FindObjectByThreePartKeyV1(ctx, conn, rs.Primary.Attributes["bucket"], rs.Primary.Attributes["key"], "") if err != nil { return err From 54c699c2b530005d07c99195d521923b5beacb66 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 5 Sep 2023 14:20:55 -0400 Subject: [PATCH 02/25] s3: 'ObjectListTags' -> 'ObjectListTagsV1'. --- internal/service/s3/bucket_object.go | 2 +- internal/service/s3/bucket_object_data_source.go | 2 +- internal/service/s3/bucket_object_test.go | 2 +- internal/service/s3/object.go | 2 +- internal/service/s3/object_copy_test.go | 2 +- internal/service/s3/object_data_source.go | 2 +- internal/service/s3/object_test.go | 2 +- internal/service/s3/tags.go | 6 +++--- 8 files changed, 10 insertions(+), 10 deletions(-) diff --git a/internal/service/s3/bucket_object.go b/internal/service/s3/bucket_object.go index 1b58137f7feb..4c5f6cc74076 100644 --- a/internal/service/s3/bucket_object.go +++ b/internal/service/s3/bucket_object.go @@ -267,7 +267,7 @@ func resourceBucketObjectRead(ctx context.Context, d *schema.ResourceData, meta // Retry due to S3 eventual consistency tagsRaw, err := tfresource.RetryWhenAWSErrCodeEquals(ctx, 2*time.Minute, func() (interface{}, error) { - return ObjectListTags(ctx, conn, bucket, key) + return ObjectListTagsV1(ctx, conn, bucket, key) }, s3.ErrCodeNoSuchBucket) if err != nil { diff --git a/internal/service/s3/bucket_object_data_source.go b/internal/service/s3/bucket_object_data_source.go index 414cfb07605d..f66bd8f782a6 100644 --- a/internal/service/s3/bucket_object_data_source.go +++ b/internal/service/s3/bucket_object_data_source.go @@ -243,7 +243,7 @@ func dataSourceBucketObjectRead(ctx context.Context, d *schema.ResourceData, met log.Printf("[INFO] Ignoring body of S3 object %s with Content-Type %q", uniqueId, contentType) } - tags, err := ObjectListTags(ctx, conn, bucket, key) + tags, err := ObjectListTagsV1(ctx, conn, bucket, key) if err != nil { return sdkdiag.AppendErrorf(diags, "listing tags for S3 Bucket (%s) Object (%s): %s", bucket, key, err) diff --git a/internal/service/s3/bucket_object_test.go b/internal/service/s3/bucket_object_test.go index 7c1b4230bcea..de8fe87b00f4 100644 --- a/internal/service/s3/bucket_object_test.go +++ b/internal/service/s3/bucket_object_test.go @@ -1582,7 +1582,7 @@ func testAccCheckBucketObjectCheckTags(ctx context.Context, n string, expectedTa rs := s.RootModule().Resources[n] conn := acctest.Provider.Meta().(*conns.AWSClient).S3Conn(ctx) - got, err := tfs3.ObjectListTags(ctx, conn, rs.Primary.Attributes["bucket"], rs.Primary.Attributes["key"]) + got, err := tfs3.ObjectListTagsV1(ctx, conn, rs.Primary.Attributes["bucket"], rs.Primary.Attributes["key"]) if err != nil { return err } diff --git a/internal/service/s3/object.go b/internal/service/s3/object.go index 4b7b97916392..48b911eb8357 100644 --- a/internal/service/s3/object.go +++ b/internal/service/s3/object.go @@ -260,7 +260,7 @@ func resourceObjectRead(ctx context.Context, d *schema.ResourceData, meta interf // Retry due to S3 eventual consistency tagsRaw, err := tfresource.RetryWhenAWSErrCodeEquals(ctx, 2*time.Minute, func() (interface{}, error) { - return ObjectListTags(ctx, conn, bucket, key) + return ObjectListTagsV1(ctx, conn, bucket, key) }, s3.ErrCodeNoSuchBucket) if err != nil { diff --git a/internal/service/s3/object_copy_test.go b/internal/service/s3/object_copy_test.go index 08a00c6b3be5..9718e1ff809f 100644 --- a/internal/service/s3/object_copy_test.go +++ b/internal/service/s3/object_copy_test.go @@ -101,7 +101,7 @@ func testAccCheckObjectCopyDestroy(ctx context.Context) resource.TestCheckFunc { continue } - _, err := tfs3.FindObjectByThreePartKey(ctx, conn, rs.Primary.Attributes["bucket"], rs.Primary.Attributes["key"], rs.Primary.Attributes["etag"]) + _, err := tfs3.FindObjectByThreePartKeyV1(ctx, conn, rs.Primary.Attributes["bucket"], rs.Primary.Attributes["key"], rs.Primary.Attributes["etag"]) if tfresource.NotFound(err) { continue diff --git a/internal/service/s3/object_data_source.go b/internal/service/s3/object_data_source.go index 994745e5b943..75c374f72715 100644 --- a/internal/service/s3/object_data_source.go +++ b/internal/service/s3/object_data_source.go @@ -239,7 +239,7 @@ func dataSourceObjectRead(ctx context.Context, d *schema.ResourceData, meta inte log.Printf("[INFO] Ignoring body of S3 object %s with Content-Type %q", uniqueId, contentType) } - tags, err := ObjectListTags(ctx, conn, bucket, key) + tags, err := ObjectListTagsV1(ctx, conn, bucket, key) if err != nil { return sdkdiag.AppendErrorf(diags, "listing tags for S3 Bucket (%s) Object (%s): %s", bucket, key, err) diff --git a/internal/service/s3/object_test.go b/internal/service/s3/object_test.go index 81307f4aa82e..1cd52f5c8d58 100644 --- a/internal/service/s3/object_test.go +++ b/internal/service/s3/object_test.go @@ -1610,7 +1610,7 @@ func testAccCheckObjectCheckTags(ctx context.Context, n string, expectedTags map rs := s.RootModule().Resources[n] conn := acctest.Provider.Meta().(*conns.AWSClient).S3Conn(ctx) - got, err := tfs3.ObjectListTags(ctx, conn, rs.Primary.Attributes["bucket"], rs.Primary.Attributes["key"]) + got, err := tfs3.ObjectListTagsV1(ctx, conn, rs.Primary.Attributes["bucket"], rs.Primary.Attributes["key"]) if err != nil { return err } diff --git a/internal/service/s3/tags.go b/internal/service/s3/tags.go index af28bb980400..492125944ab8 100644 --- a/internal/service/s3/tags.go +++ b/internal/service/s3/tags.go @@ -87,8 +87,8 @@ func BucketUpdateTags(ctx context.Context, conn s3iface.S3API, identifier string return nil } -// ObjectListTags lists S3 object tags. -func ObjectListTags(ctx context.Context, conn s3iface.S3API, bucket, key string) (tftags.KeyValueTags, error) { +// ObjectListTagsV1 lists S3 object tags. +func ObjectListTagsV1(ctx context.Context, conn s3iface.S3API, bucket, key string) (tftags.KeyValueTags, error) { input := &s3.GetObjectTaggingInput{ Bucket: aws.String(bucket), Key: aws.String(key), @@ -115,7 +115,7 @@ func ObjectUpdateTags(ctx context.Context, conn s3iface.S3API, bucket, key strin newTags := tftags.New(ctx, newTagsMap) // We need to also consider any existing ignored tags. - allTags, err := ObjectListTags(ctx, conn, bucket, key) + allTags, err := ObjectListTagsV1(ctx, conn, bucket, key) if err != nil { return fmt.Errorf("listing resource tags (%s/%s): %w", bucket, key, err) From 74cc696193b394e55c419bf2f1259f7f8e8e8f0b Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 5 Sep 2023 14:33:19 -0400 Subject: [PATCH 03/25] s3: Generate AWS SDK for Go v2 tagging code. --- internal/service/s3/generate.go | 1 + internal/service/s3/tagsv2_gen.go | 60 +++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+) create mode 100644 internal/service/s3/tagsv2_gen.go diff --git a/internal/service/s3/generate.go b/internal/service/s3/generate.go index dbb5e5c3dd54..06a1ce818892 100644 --- a/internal/service/s3/generate.go +++ b/internal/service/s3/generate.go @@ -2,6 +2,7 @@ // SPDX-License-Identifier: MPL-2.0 //go:generate go run ../../generate/tags/main.go -ServiceTagsSlice +//go:generate go run ../../generate/tags/main.go -AWSSDKVersion=2 -ServiceTagsSlice -TagsFunc=tagsV2 -KeyValueTagsFunc=keyValueTagsV2 -GetTagsInFunc=getTagsInV2 -SetTagsOutFunc=setTagsOutV2 -- tagsv2_gen.go //go:generate go run ../../generate/servicepackage/main.go // ONLY generate directives and package declaration! Do not add anything else to this file. diff --git a/internal/service/s3/tagsv2_gen.go b/internal/service/s3/tagsv2_gen.go new file mode 100644 index 000000000000..522c3f210562 --- /dev/null +++ b/internal/service/s3/tagsv2_gen.go @@ -0,0 +1,60 @@ +// Code generated by internal/generate/tags/main.go; DO NOT EDIT. +package s3 + +import ( + "context" + + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/service/s3" + awstypes "github.com/aws/aws-sdk-go-v2/service/s3/types" + tftags "github.com/hashicorp/terraform-provider-aws/internal/tags" + "github.com/hashicorp/terraform-provider-aws/internal/types" +) + +// []*SERVICE.Tag handling + +// tagsV2 returns s3 service tags. +func tagsV2(tags tftags.KeyValueTags) []awstypes.Tag { + result := make([]awstypes.Tag, 0, len(tags)) + + for k, v := range tags.Map() { + tag := awstypes.Tag{ + Key: aws.String(k), + Value: aws.String(v), + } + + result = append(result, tag) + } + + return result +} + +// keyValueTagsV2 creates tftags.KeyValueTags from s3 service tags. +func keyValueTagsV2(ctx context.Context, tags []awstypes.Tag) tftags.KeyValueTags { + m := make(map[string]*string, len(tags)) + + for _, tag := range tags { + m[aws.ToString(tag.Key)] = tag.Value + } + + return tftags.New(ctx, m) +} + +// getTagsInV2 returns s3 service tags from Context. +// nil is returned if there are no input tags. +func getTagsInV2(ctx context.Context) []awstypes.Tag { + if inContext, ok := tftags.FromContext(ctx); ok { + if tags := tagsV2(inContext.TagsIn.UnwrapOrDefault()); len(tags) > 0 { + return tags + } + } + + return nil +} + +// setTagsOutV2 sets s3 service tags in Context. +func setTagsOutV2(ctx context.Context, tags []awstypes.Tag) { + if inContext, ok := tftags.FromContext(ctx); ok { + inContext.TagsOut = types.Some(keyValueTagsV2(ctx, tags)) + } +} From 3bad4a94e1b672a267be09b218c41ab820c925b1 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 5 Sep 2023 15:08:38 -0400 Subject: [PATCH 04/25] r/aws_s3_object_copy: Migrate (except Delete) to AWS SDK for Go v2. --- internal/service/s3/object_copy.go | 228 ++++++++++++++++------------- internal/service/s3/tags.go | 86 +++++++---- 2 files changed, 180 insertions(+), 134 deletions(-) diff --git a/internal/service/s3/object_copy.go b/internal/service/s3/object_copy.go index c59460a1f659..58b7e15e43f4 100644 --- a/internal/service/s3/object_copy.go +++ b/internal/service/s3/object_copy.go @@ -13,13 +13,15 @@ import ( "time" "github.com/YakDriver/regexache" - "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/service/s3" + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/service/s3" + "github.com/aws/aws-sdk-go-v2/service/s3/types" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" "github.com/hashicorp/terraform-provider-aws/internal/conns" "github.com/hashicorp/terraform-provider-aws/internal/create" + "github.com/hashicorp/terraform-provider-aws/internal/enum" "github.com/hashicorp/terraform-provider-aws/internal/errs/sdkdiag" "github.com/hashicorp/terraform-provider-aws/internal/flex" tftags "github.com/hashicorp/terraform-provider-aws/internal/tags" @@ -39,11 +41,11 @@ func ResourceObjectCopy() *schema.Resource { Schema: map[string]*schema.Schema{ "acl": { - Type: schema.TypeString, - Optional: true, - Computed: true, - ValidateFunc: validation.StringInSlice(s3.ObjectCannedACL_Values(), false), - ConflictsWith: []string{"grant"}, + Type: schema.TypeString, + Optional: true, + Computed: true, + ValidateDiagFunc: enum.Validate[types.ObjectCannedACL](), + ConflictsWith: []string{"grant"}, }, "bucket": { Type: schema.TypeString, @@ -158,22 +160,21 @@ func ResourceObjectCopy() *schema.Resource { "permissions": { Type: schema.TypeSet, Required: true, - Set: schema.HashString, Elem: &schema.Schema{ Type: schema.TypeString, - ValidateFunc: validation.StringInSlice([]string{ + ValidateDiagFunc: validation.ToDiagFunc(validation.StringInSlice(enum.Slice( //write permission not valid here - s3.PermissionFullControl, - s3.PermissionRead, - s3.PermissionReadAcp, - s3.PermissionWriteAcp, - }, false), + types.PermissionFullControl, + types.PermissionRead, + types.PermissionReadAcp, + types.PermissionWriteAcp, + ), false)), }, }, "type": { - Type: schema.TypeString, - Required: true, - ValidateFunc: validation.StringInSlice(s3.Type_Values(), false), + Type: schema.TypeString, + Required: true, + ValidateDiagFunc: enum.Validate[types.Type](), }, "uri": { Type: schema.TypeString, @@ -214,21 +215,21 @@ func ResourceObjectCopy() *schema.Resource { Elem: &schema.Schema{Type: schema.TypeString}, }, "metadata_directive": { - Type: schema.TypeString, - Optional: true, - ValidateFunc: validation.StringInSlice(s3.MetadataDirective_Values(), false), + Type: schema.TypeString, + Optional: true, + ValidateDiagFunc: enum.Validate[types.MetadataDirective](), }, "object_lock_legal_hold_status": { - Type: schema.TypeString, - Optional: true, - Computed: true, - ValidateFunc: validation.StringInSlice(s3.ObjectLockLegalHoldStatus_Values(), false), + Type: schema.TypeString, + Optional: true, + Computed: true, + ValidateDiagFunc: enum.Validate[types.ObjectLockLegalHoldStatus](), }, "object_lock_mode": { - Type: schema.TypeString, - Optional: true, - Computed: true, - ValidateFunc: validation.StringInSlice(s3.ObjectLockMode_Values(), false), + Type: schema.TypeString, + Optional: true, + Computed: true, + ValidateDiagFunc: enum.Validate[types.ObjectLockMode](), }, "object_lock_retain_until_date": { Type: schema.TypeString, @@ -241,15 +242,15 @@ func ResourceObjectCopy() *schema.Resource { Computed: true, }, "request_payer": { - Type: schema.TypeString, - Optional: true, - ValidateFunc: validation.StringInSlice(s3.RequestPayer_Values(), false), + Type: schema.TypeString, + Optional: true, + ValidateDiagFunc: enum.Validate[types.RequestPayer](), }, "server_side_encryption": { - Type: schema.TypeString, - Optional: true, - Computed: true, - ValidateFunc: validation.StringInSlice(s3.ServerSideEncryption_Values(), false), + Type: schema.TypeString, + Optional: true, + Computed: true, + ValidateDiagFunc: enum.Validate[types.ServerSideEncryption](), }, "source": { Type: schema.TypeString, @@ -275,15 +276,15 @@ func ResourceObjectCopy() *schema.Resource { Computed: true, }, "storage_class": { - Type: schema.TypeString, - Optional: true, - Computed: true, - ValidateFunc: validation.StringInSlice(s3.ObjectStorageClass_Values(), false), + Type: schema.TypeString, + Optional: true, + Computed: true, + ValidateDiagFunc: enum.Validate[types.ObjectStorageClass](), }, "tagging_directive": { - Type: schema.TypeString, - Optional: true, - ValidateFunc: validation.StringInSlice(s3.TaggingDirective_Values(), false), + Type: schema.TypeString, + Optional: true, + ValidateDiagFunc: enum.Validate[types.TaggingDirective](), }, names.AttrTags: tftags.TagsSchema(), names.AttrTagsAll: tftags.TagsSchemaComputed(), @@ -309,7 +310,7 @@ func resourceObjectCopyCreate(ctx context.Context, d *schema.ResourceData, meta func resourceObjectCopyRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { var diags diag.Diagnostics - conn := meta.(*conns.AWSClient).S3Conn(ctx) + conn := meta.(*conns.AWSClient).S3Client(ctx) bucket := d.Get("bucket").(string) key := d.Get("key").(string) @@ -331,42 +332,31 @@ func resourceObjectCopyRead(ctx context.Context, d *schema.ResourceData, meta in d.Set("content_encoding", output.ContentEncoding) d.Set("content_language", output.ContentLanguage) d.Set("content_type", output.ContentType) - metadata := flex.PointersMapToStringList(output.Metadata) - - // AWS Go SDK capitalizes metadata, this is a workaround. https://github.com/aws/aws-sdk-go/issues/445 - for k, v := range metadata { - delete(metadata, k) - metadata[strings.ToLower(k)] = v - } - - if err := d.Set("metadata", metadata); err != nil { - return sdkdiag.AppendErrorf(diags, "setting metadata: %s", err) - } - d.Set("version_id", output.VersionId) - d.Set("server_side_encryption", output.ServerSideEncryption) - d.Set("website_redirect", output.WebsiteRedirectLocation) + // See https://forums.aws.amazon.com/thread.jspa?threadID=44003 + d.Set("etag", strings.Trim(aws.ToString(output.ETag), `"`)) + d.Set("metadata", output.Metadata) d.Set("object_lock_legal_hold_status", output.ObjectLockLegalHoldStatus) d.Set("object_lock_mode", output.ObjectLockMode) d.Set("object_lock_retain_until_date", flattenObjectDate(output.ObjectLockRetainUntilDate)) + d.Set("version_id", output.VersionId) + d.Set("server_side_encryption", output.ServerSideEncryption) + d.Set("website_redirect", output.WebsiteRedirectLocation) if err := resourceObjectSetKMS(ctx, d, meta, output.SSEKMSKeyId); err != nil { - return sdkdiag.AppendErrorf(diags, "object KMS: %s", err) + return sdkdiag.AppendFromErr(diags, err) } - // See https://forums.aws.amazon.com/thread.jspa?threadID=44003 - d.Set("etag", strings.Trim(aws.StringValue(output.ETag), `"`)) - // The "STANDARD" (which is also the default) storage // class when set would not be included in the results. - d.Set("storage_class", s3.ObjectStorageClassStandard) - if output.StorageClass != nil { // nosemgrep: ci.helper-schema-ResourceData-Set-extraneous-nil-check + d.Set("storage_class", types.ObjectStorageClassStandard) + if output.StorageClass != "" { d.Set("storage_class", output.StorageClass) } // Retry due to S3 eventual consistency - tagsRaw, err := tfresource.RetryWhenAWSErrCodeEquals(ctx, 2*time.Minute, func() (interface{}, error) { + tagsRaw, err := tfresource.RetryWhenIsA[*types.NoSuchBucket](ctx, 2*time.Minute, func() (interface{}, error) { return ObjectListTags(ctx, conn, bucket, key) - }, s3.ErrCodeNoSuchBucket) + }) if err != nil { return sdkdiag.AppendErrorf(diags, "listing tags for S3 Bucket (%s) Object (%s): %s", bucket, key, err) @@ -384,10 +374,9 @@ func resourceObjectCopyRead(ctx context.Context, d *schema.ResourceData, meta in } func resourceObjectCopyUpdate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { - var - // if any of these exist, let the API decide whether to copy - diags diag.Diagnostics + var diags diag.Diagnostics + // if any of these exist, let the API decide whether to copy for _, key := range []string{ "copy_if_match", "copy_if_modified_since", @@ -468,7 +457,7 @@ func resourceObjectCopyDelete(ctx context.Context, d *schema.ResourceData, meta func resourceObjectCopyDoCopy(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { var diags diag.Diagnostics - conn := meta.(*conns.AWSClient).S3Conn(ctx) + conn := meta.(*conns.AWSClient).S3Client(ctx) defaultTagsConfig := meta.(*conns.AWSClient).DefaultTagsConfig tags := defaultTagsConfig.MergeTags(tftags.New(ctx, d.Get("tags").(map[string]interface{}))) @@ -479,11 +468,11 @@ func resourceObjectCopyDoCopy(ctx context.Context, d *schema.ResourceData, meta } if v, ok := d.GetOk("acl"); ok { - input.ACL = aws.String(v.(string)) + input.ACL = types.ObjectCannedACL(v.(string)) } if v, ok := d.GetOk("bucket_key_enabled"); ok { - input.BucketKeyEnabled = aws.Bool(v.(bool)) + input.BucketKeyEnabled = v.(bool) } if v, ok := d.GetOk("cache_control"); ok { @@ -552,7 +541,7 @@ func resourceObjectCopyDoCopy(ctx context.Context, d *schema.ResourceData, meta input.GrantRead = grants.Read input.GrantReadACP = grants.ReadACP input.GrantWriteACP = grants.WriteACP - input.ACL = nil + input.ACL = "" } if v, ok := d.GetOk("kms_encryption_context"); ok { @@ -561,23 +550,23 @@ func resourceObjectCopyDoCopy(ctx context.Context, d *schema.ResourceData, meta if v, ok := d.GetOk("kms_key_id"); ok { input.SSEKMSKeyId = aws.String(v.(string)) - input.ServerSideEncryption = aws.String(s3.ServerSideEncryptionAwsKms) + input.ServerSideEncryption = types.ServerSideEncryptionAwsKms } if v, ok := d.GetOk("metadata"); ok { - input.Metadata = flex.ExpandStringMap(v.(map[string]interface{})) + input.Metadata = flex.ExpandStringValueMap(v.(map[string]interface{})) } if v, ok := d.GetOk("metadata_directive"); ok { - input.MetadataDirective = aws.String(v.(string)) + input.MetadataDirective = types.MetadataDirective(v.(string)) } if v, ok := d.GetOk("object_lock_legal_hold_status"); ok { - input.ObjectLockLegalHoldStatus = aws.String(v.(string)) + input.ObjectLockLegalHoldStatus = types.ObjectLockLegalHoldStatus(v.(string)) } if v, ok := d.GetOk("object_lock_mode"); ok { - input.ObjectLockMode = aws.String(v.(string)) + input.ObjectLockMode = types.ObjectLockMode(v.(string)) } if v, ok := d.GetOk("object_lock_retain_until_date"); ok { @@ -585,11 +574,11 @@ func resourceObjectCopyDoCopy(ctx context.Context, d *schema.ResourceData, meta } if v, ok := d.GetOk("request_payer"); ok { - input.RequestPayer = aws.String(v.(string)) + input.RequestPayer = types.RequestPayer(v.(string)) } if v, ok := d.GetOk("server_side_encryption"); ok { - input.ServerSideEncryption = aws.String(v.(string)) + input.ServerSideEncryption = types.ServerSideEncryption(v.(string)) } if v, ok := d.GetOk("source_customer_algorithm"); ok { @@ -605,11 +594,11 @@ func resourceObjectCopyDoCopy(ctx context.Context, d *schema.ResourceData, meta } if v, ok := d.GetOk("storage_class"); ok { - input.StorageClass = aws.String(v.(string)) + input.StorageClass = types.StorageClass(v.(string)) } if v, ok := d.GetOk("tagging_directive"); ok { - input.TaggingDirective = aws.String(v.(string)) + input.TaggingDirective = types.TaggingDirective(v.(string)) } if len(tags) > 0 { @@ -621,19 +610,22 @@ func resourceObjectCopyDoCopy(ctx context.Context, d *schema.ResourceData, meta input.WebsiteRedirectLocation = aws.String(v.(string)) } - output, err := conn.CopyObjectWithContext(ctx, input) + output, err := conn.CopyObject(ctx, input) + if err != nil { - return sdkdiag.AppendErrorf(diags, "copying S3 object (bucket: %s; key: %s; source: %s): %s", aws.StringValue(input.Bucket), aws.StringValue(input.Key), aws.StringValue(input.CopySource), err) + return sdkdiag.AppendErrorf(diags, "copying S3 object (bucket: %s; key: %s; source: %s): %s", aws.ToString(input.Bucket), aws.ToString(input.Key), aws.ToString(input.CopySource), err) + } + + if d.IsNewResource() { + d.SetId(d.Get("key").(string)) } d.Set("customer_algorithm", output.SSECustomerAlgorithm) d.Set("customer_key_md5", output.SSECustomerKeyMD5) - if output.CopyObjectResult != nil { - d.Set("etag", strings.Trim(aws.StringValue(output.CopyObjectResult.ETag), `"`)) + d.Set("etag", strings.Trim(aws.ToString(output.CopyObjectResult.ETag), `"`)) d.Set("last_modified", flattenObjectDate(output.CopyObjectResult.LastModified)) } - d.Set("expiration", output.Expiration) d.Set("kms_encryption_context", output.SSEKMSEncryptionContext) d.Set("kms_key_id", output.SSEKMSKeyId) @@ -642,11 +634,39 @@ func resourceObjectCopyDoCopy(ctx context.Context, d *schema.ResourceData, meta d.Set("source_version_id", output.CopySourceVersionId) d.Set("version_id", output.VersionId) - d.SetId(d.Get("key").(string)) - return append(diags, resourceObjectRead(ctx, d, meta)...) } +func FindObjectByThreePartKey(ctx context.Context, conn *s3.Client, bucket, key, etag string) (*s3.HeadObjectOutput, error) { + input := &s3.HeadObjectInput{ + Bucket: aws.String(bucket), + Key: aws.String(key), + } + if etag != "" { + input.IfMatch = aws.String(etag) + } + + output, err := conn.HeadObject(ctx, input) + + // TODO https://github.com/aws/smithy-go/blob/main/transport/http/response.go + // if tfawserr.ErrStatusCodeEquals(err, http.StatusNotFound) { + // return nil, &retry.NotFoundError{ + // LastError: err, + // LastRequest: input, + // } + // } + + if err != nil { + return nil, err + } + + if output == nil { + return nil, tfresource.NewEmptyResultError(input) + } + + return output, nil +} + type s3Grants struct { FullControl *string Read *string @@ -659,22 +679,22 @@ func expandObjectCopyGrant(tfMap map[string]interface{}) string { return "" } - apiObject := &s3.Grantee{} + apiObject := &types.Grantee{} if v, ok := tfMap["email"].(string); ok && v != "" { - apiObject.SetEmailAddress(v) + apiObject.EmailAddress = aws.String(v) } if v, ok := tfMap["id"].(string); ok && v != "" { - apiObject.SetID(v) + apiObject.ID = aws.String(v) } if v, ok := tfMap["type"].(string); ok && v != "" { - apiObject.SetType(v) + apiObject.Type = types.Type(v) } if v, ok := tfMap["uri"].(string); ok && v != "" { - apiObject.SetURI(v) + apiObject.URI = aws.String(v) } // Examples: @@ -683,14 +703,14 @@ func expandObjectCopyGrant(tfMap map[string]interface{}) string { //"GrantFullControl": "id=examplee7a2f25102679df27bb0ae12b3f85be6f290b936c4393484", //"GrantWrite": "uri=http://acs.amazonaws.com/groups/s3/LogDelivery" - switch *apiObject.Type { - case s3.TypeAmazonCustomerByEmail: - return fmt.Sprintf("emailaddress=%s", *apiObject.EmailAddress) - case s3.TypeCanonicalUser: - return fmt.Sprintf("id=%s", *apiObject.ID) + switch apiObject.Type { + case types.TypeAmazonCustomerByEmail: + return fmt.Sprintf("emailaddress=%s", aws.ToString(apiObject.EmailAddress)) + case types.TypeCanonicalUser: + return fmt.Sprintf("id=%s", aws.ToString(apiObject.ID)) } - return fmt.Sprintf("uri=%s", *apiObject.URI) + return fmt.Sprintf("uri=%s", aws.ToString(apiObject.URI)) } func expandObjectCopyGrants(tfList []interface{}) *s3Grants { @@ -712,14 +732,14 @@ func expandObjectCopyGrants(tfList []interface{}) *s3Grants { for _, perm := range tfMap["permissions"].(*schema.Set).List() { if v := expandObjectCopyGrant(tfMap); v != "" { - switch perm.(string) { - case s3.PermissionFullControl: + switch types.Permission(perm.(string)) { + case types.PermissionFullControl: grantFullControl = append(grantFullControl, v) - case s3.PermissionRead: + case types.PermissionRead: grantRead = append(grantRead, v) - case s3.PermissionReadAcp: + case types.PermissionReadAcp: grantReadACP = append(grantReadACP, v) - case s3.PermissionWriteAcp: + case types.PermissionWriteAcp: grantWriteACP = append(grantWriteACP, v) } } diff --git a/internal/service/s3/tags.go b/internal/service/s3/tags.go index 492125944ab8..c7aa0a09ccce 100644 --- a/internal/service/s3/tags.go +++ b/internal/service/s3/tags.go @@ -11,10 +11,14 @@ import ( "fmt" "time" - "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/service/s3" - "github.com/aws/aws-sdk-go/service/s3/s3iface" - "github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr" + aws_sdkv2 "github.com/aws/aws-sdk-go-v2/aws" + s3_sdkv2 "github.com/aws/aws-sdk-go-v2/service/s3" + s3types_sdkv2 "github.com/aws/aws-sdk-go-v2/service/s3/types" + aws_sdkv1 "github.com/aws/aws-sdk-go/aws" + s3_sdkv1 "github.com/aws/aws-sdk-go/service/s3" + s3iface_sdkv1 "github.com/aws/aws-sdk-go/service/s3/s3iface" + tfawserr_sdkv1 "github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr" + tfawserr_sdkv2 "github.com/hashicorp/aws-sdk-go-base/v2/tfawserr" tftags "github.com/hashicorp/terraform-provider-aws/internal/tags" "github.com/hashicorp/terraform-provider-aws/internal/tfresource" ) @@ -23,9 +27,9 @@ import ( // BucketListTags lists S3 bucket tags. // The identifier is the bucket name. -func BucketListTags(ctx context.Context, conn s3iface.S3API, identifier string) (tftags.KeyValueTags, error) { - input := &s3.GetBucketTaggingInput{ - Bucket: aws.String(identifier), +func BucketListTags(ctx context.Context, conn s3iface_sdkv1.S3API, identifier string) (tftags.KeyValueTags, error) { + input := &s3_sdkv1.GetBucketTaggingInput{ + Bucket: aws_sdkv1.String(identifier), } output, err := conn.GetBucketTaggingWithContext(ctx, input) @@ -33,7 +37,7 @@ func BucketListTags(ctx context.Context, conn s3iface.S3API, identifier string) // 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 tfawserr.ErrCodeEquals(err, errCodeNoSuchTagSet, errCodeNoSuchTagSetError) { + if tfawserr_sdkv1.ErrCodeEquals(err, errCodeNoSuchTagSet, errCodeNoSuchTagSetError) { return tftags.New(ctx, nil), nil } @@ -46,7 +50,7 @@ func BucketListTags(ctx context.Context, conn s3iface.S3API, identifier string) // BucketUpdateTags updates S3 bucket tags. // The identifier is the bucket name. -func BucketUpdateTags(ctx context.Context, conn s3iface.S3API, identifier string, oldTagsMap, newTagsMap any) error { +func BucketUpdateTags(ctx context.Context, conn s3iface_sdkv1.S3API, identifier string, oldTagsMap, newTagsMap any) error { oldTags := tftags.New(ctx, oldTagsMap) newTags := tftags.New(ctx, newTagsMap) @@ -60,9 +64,9 @@ func BucketUpdateTags(ctx context.Context, conn s3iface.S3API, identifier string ignoredTags := allTags.Ignore(oldTags).Ignore(newTags) if len(newTags)+len(ignoredTags) > 0 { - input := &s3.PutBucketTaggingInput{ - Bucket: aws.String(identifier), - Tagging: &s3.Tagging{ + input := &s3_sdkv1.PutBucketTaggingInput{ + Bucket: aws_sdkv1.String(identifier), + Tagging: &s3_sdkv1.Tagging{ TagSet: Tags(newTags.Merge(ignoredTags)), }, } @@ -73,8 +77,8 @@ func BucketUpdateTags(ctx context.Context, conn s3iface.S3API, identifier string return fmt.Errorf("setting resource tags (%s): %w", identifier, err) } } else if len(oldTags) > 0 && len(ignoredTags) == 0 { - input := &s3.DeleteBucketTaggingInput{ - Bucket: aws.String(identifier), + input := &s3_sdkv1.DeleteBucketTaggingInput{ + Bucket: aws_sdkv1.String(identifier), } _, err := conn.DeleteBucketTaggingWithContext(ctx, input) @@ -87,18 +91,40 @@ func BucketUpdateTags(ctx context.Context, conn s3iface.S3API, identifier string return nil } -// ObjectListTagsV1 lists S3 object tags. -func ObjectListTagsV1(ctx context.Context, conn s3iface.S3API, bucket, key string) (tftags.KeyValueTags, error) { - input := &s3.GetObjectTaggingInput{ - Bucket: aws.String(bucket), - Key: aws.String(key), +// ObjectListTags lists S3 object tags. +func ObjectListTags(ctx context.Context, conn *s3_sdkv2.Client, bucket, key string) (tftags.KeyValueTags, error) { + input := &s3_sdkv2.GetObjectTaggingInput{ + Bucket: aws_sdkv2.String(bucket), + Key: aws_sdkv2.String(key), + } + + outputRaw, err := tfresource.RetryWhenIsA[*s3types_sdkv2.NoSuchKey](ctx, 1*time.Minute, func() (interface{}, error) { + return conn.GetObjectTagging(ctx, input) + }) + + if tfawserr_sdkv2.ErrCodeEquals(err, errCodeNoSuchTagSet, errCodeNoSuchTagSetError) { + return tftags.New(ctx, nil), nil + } + + if err != nil { + return tftags.New(ctx, nil), err + } + + return keyValueTagsV2(ctx, outputRaw.(*s3_sdkv2.GetObjectTaggingOutput).TagSet), nil +} + +// ObjectListTagsV1 lists S3 object tags (AWS SDK for Go v1). +func ObjectListTagsV1(ctx context.Context, conn s3iface_sdkv1.S3API, bucket, key string) (tftags.KeyValueTags, error) { + input := &s3_sdkv1.GetObjectTaggingInput{ + Bucket: aws_sdkv1.String(bucket), + Key: aws_sdkv1.String(key), } outputRaw, err := tfresource.RetryWhenAWSErrCodeEquals(ctx, 1*time.Minute, func() (interface{}, error) { return conn.GetObjectTaggingWithContext(ctx, input) - }, s3.ErrCodeNoSuchKey) + }, s3_sdkv1.ErrCodeNoSuchKey) - if tfawserr.ErrCodeEquals(err, errCodeNoSuchTagSet, errCodeNoSuchTagSetError) { + if tfawserr_sdkv1.ErrCodeEquals(err, errCodeNoSuchTagSet, errCodeNoSuchTagSetError) { return tftags.New(ctx, nil), nil } @@ -106,11 +132,11 @@ func ObjectListTagsV1(ctx context.Context, conn s3iface.S3API, bucket, key strin return tftags.New(ctx, nil), err } - return KeyValueTags(ctx, outputRaw.(*s3.GetObjectTaggingOutput).TagSet), nil + return KeyValueTags(ctx, outputRaw.(*s3_sdkv1.GetObjectTaggingOutput).TagSet), nil } // ObjectUpdateTags updates S3 object tags. -func ObjectUpdateTags(ctx context.Context, conn s3iface.S3API, bucket, key string, oldTagsMap, newTagsMap any) error { +func ObjectUpdateTags(ctx context.Context, conn s3iface_sdkv1.S3API, bucket, key string, oldTagsMap, newTagsMap any) error { oldTags := tftags.New(ctx, oldTagsMap) newTags := tftags.New(ctx, newTagsMap) @@ -124,10 +150,10 @@ func ObjectUpdateTags(ctx context.Context, conn s3iface.S3API, bucket, key strin ignoredTags := allTags.Ignore(oldTags).Ignore(newTags) if len(newTags)+len(ignoredTags) > 0 { - input := &s3.PutObjectTaggingInput{ - Bucket: aws.String(bucket), - Key: aws.String(key), - Tagging: &s3.Tagging{ + input := &s3_sdkv1.PutObjectTaggingInput{ + Bucket: aws_sdkv1.String(bucket), + Key: aws_sdkv1.String(key), + Tagging: &s3_sdkv1.Tagging{ TagSet: Tags(newTags.Merge(ignoredTags)), }, } @@ -138,9 +164,9 @@ func ObjectUpdateTags(ctx context.Context, conn s3iface.S3API, bucket, key strin return fmt.Errorf("setting resource tags (%s/%s): %w", bucket, key, err) } } else if len(oldTags) > 0 && len(ignoredTags) == 0 { - input := &s3.DeleteObjectTaggingInput{ - Bucket: aws.String(bucket), - Key: aws.String(key), + input := &s3_sdkv1.DeleteObjectTaggingInput{ + Bucket: aws_sdkv1.String(bucket), + Key: aws_sdkv1.String(key), } _, err := conn.DeleteObjectTaggingWithContext(ctx, input) From 798ce6be4f9a24f0737756f83d1b3b9a2575bcd5 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 5 Sep 2023 15:10:21 -0400 Subject: [PATCH 05/25] s3: Add '-SkipAWSServiceImp' when generating AWS SDK for Go v2 tagging code. --- internal/service/s3/generate.go | 2 +- internal/service/s3/tagsv2_gen.go | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/service/s3/generate.go b/internal/service/s3/generate.go index 06a1ce818892..bb62eb528838 100644 --- a/internal/service/s3/generate.go +++ b/internal/service/s3/generate.go @@ -2,7 +2,7 @@ // SPDX-License-Identifier: MPL-2.0 //go:generate go run ../../generate/tags/main.go -ServiceTagsSlice -//go:generate go run ../../generate/tags/main.go -AWSSDKVersion=2 -ServiceTagsSlice -TagsFunc=tagsV2 -KeyValueTagsFunc=keyValueTagsV2 -GetTagsInFunc=getTagsInV2 -SetTagsOutFunc=setTagsOutV2 -- tagsv2_gen.go +//go:generate go run ../../generate/tags/main.go -AWSSDKVersion=2 -SkipAWSServiceImp -ServiceTagsSlice -TagsFunc=tagsV2 -KeyValueTagsFunc=keyValueTagsV2 -GetTagsInFunc=getTagsInV2 -SetTagsOutFunc=setTagsOutV2 -- tagsv2_gen.go //go:generate go run ../../generate/servicepackage/main.go // ONLY generate directives and package declaration! Do not add anything else to this file. diff --git a/internal/service/s3/tagsv2_gen.go b/internal/service/s3/tagsv2_gen.go index 522c3f210562..10215b864dd0 100644 --- a/internal/service/s3/tagsv2_gen.go +++ b/internal/service/s3/tagsv2_gen.go @@ -5,7 +5,6 @@ import ( "context" "github.com/aws/aws-sdk-go-v2/aws" - "github.com/aws/aws-sdk-go-v2/service/s3" awstypes "github.com/aws/aws-sdk-go-v2/service/s3/types" tftags "github.com/hashicorp/terraform-provider-aws/internal/tags" "github.com/hashicorp/terraform-provider-aws/internal/types" From d170f3ab41231ed7405e1bdd10d8a73597158df7 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 5 Sep 2023 15:24:35 -0400 Subject: [PATCH 06/25] r/aws_s3_object_copy: Migrate acceptance tests to AWS SDK for Go v2. --- internal/service/s3/object_copy.go | 2 +- internal/service/s3/object_copy_test.go | 29 ++++++++----------------- 2 files changed, 10 insertions(+), 21 deletions(-) diff --git a/internal/service/s3/object_copy.go b/internal/service/s3/object_copy.go index 58b7e15e43f4..5c169fd37eae 100644 --- a/internal/service/s3/object_copy.go +++ b/internal/service/s3/object_copy.go @@ -629,7 +629,7 @@ func resourceObjectCopyDoCopy(ctx context.Context, d *schema.ResourceData, meta d.Set("expiration", output.Expiration) d.Set("kms_encryption_context", output.SSEKMSEncryptionContext) d.Set("kms_key_id", output.SSEKMSKeyId) - d.Set("request_charged", output.RequestCharged) + d.Set("request_charged", output.RequestCharged == types.RequestChargedRequester) d.Set("server_side_encryption", output.ServerSideEncryption) d.Set("source_version_id", output.CopySourceVersionId) d.Set("version_id", output.VersionId) diff --git a/internal/service/s3/object_copy_test.go b/internal/service/s3/object_copy_test.go index 9718e1ff809f..e7d6962cddb5 100644 --- a/internal/service/s3/object_copy_test.go +++ b/internal/service/s3/object_copy_test.go @@ -8,8 +8,6 @@ import ( "fmt" "testing" - "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/service/s3" sdkacctest "github.com/hashicorp/terraform-plugin-testing/helper/acctest" "github.com/hashicorp/terraform-plugin-testing/helper/resource" "github.com/hashicorp/terraform-plugin-testing/terraform" @@ -17,6 +15,7 @@ import ( "github.com/hashicorp/terraform-provider-aws/internal/conns" tfs3 "github.com/hashicorp/terraform-provider-aws/internal/service/s3" "github.com/hashicorp/terraform-provider-aws/internal/tfresource" + "github.com/hashicorp/terraform-provider-aws/names" ) func TestAccS3ObjectCopy_basic(t *testing.T) { @@ -30,7 +29,7 @@ func TestAccS3ObjectCopy_basic(t *testing.T) { resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(ctx, t) }, - ErrorCheck: acctest.ErrorCheck(t, s3.EndpointsID), + ErrorCheck: acctest.ErrorCheck(t, names.S3EndpointID), ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, CheckDestroy: testAccCheckObjectCopyDestroy(ctx), Steps: []resource.TestStep{ @@ -55,7 +54,7 @@ func TestAccS3ObjectCopy_BucketKeyEnabled_bucket(t *testing.T) { resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(ctx, t) }, - ErrorCheck: acctest.ErrorCheck(t, s3.EndpointsID), + ErrorCheck: acctest.ErrorCheck(t, names.S3EndpointID), ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, CheckDestroy: testAccCheckObjectCopyDestroy(ctx), Steps: []resource.TestStep{ @@ -77,7 +76,7 @@ func TestAccS3ObjectCopy_BucketKeyEnabled_object(t *testing.T) { resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(ctx, t) }, - ErrorCheck: acctest.ErrorCheck(t, s3.EndpointsID), + ErrorCheck: acctest.ErrorCheck(t, names.S3EndpointID), ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, CheckDestroy: testAccCheckObjectCopyDestroy(ctx), Steps: []resource.TestStep{ @@ -94,14 +93,14 @@ func TestAccS3ObjectCopy_BucketKeyEnabled_object(t *testing.T) { func testAccCheckObjectCopyDestroy(ctx context.Context) resource.TestCheckFunc { return func(s *terraform.State) error { - conn := acctest.Provider.Meta().(*conns.AWSClient).S3Conn(ctx) + conn := acctest.Provider.Meta().(*conns.AWSClient).S3Client(ctx) for _, rs := range s.RootModule().Resources { if rs.Type != "aws_s3_object_copy" { continue } - _, err := tfs3.FindObjectByThreePartKeyV1(ctx, conn, rs.Primary.Attributes["bucket"], rs.Primary.Attributes["key"], rs.Primary.Attributes["etag"]) + _, err := tfs3.FindObjectByThreePartKey(ctx, conn, rs.Primary.Attributes["bucket"], rs.Primary.Attributes["key"], rs.Primary.Attributes["etag"]) if tfresource.NotFound(err) { continue @@ -125,21 +124,11 @@ func testAccCheckObjectCopyExists(ctx context.Context, n string) resource.TestCh return fmt.Errorf("Not Found: %s", n) } - if rs.Primary.ID == "" { - return fmt.Errorf("No S3 Object ID is set") - } + conn := acctest.Provider.Meta().(*conns.AWSClient).S3Client(ctx) - conn := acctest.Provider.Meta().(*conns.AWSClient).S3Conn(ctx) - _, err := conn.GetObjectWithContext(ctx, &s3.GetObjectInput{ - Bucket: aws.String(rs.Primary.Attributes["bucket"]), - Key: aws.String(rs.Primary.Attributes["key"]), - IfMatch: aws.String(rs.Primary.Attributes["etag"]), - }) - if err != nil { - return fmt.Errorf("S3 Object error: %s", err) - } + _, err := tfs3.FindObjectByThreePartKey(ctx, conn, rs.Primary.Attributes["bucket"], rs.Primary.Attributes["key"], rs.Primary.Attributes["etag"]) - return nil + return err } } From 917bd9414a375a1ce99c5126dd599a3d023ded0b Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 5 Sep 2023 16:02:34 -0400 Subject: [PATCH 07/25] Run 'go get github.com/hashicorp/aws-sdk-go-base/v2@2572ed001c9c61c7f02411dfb0f646000bdb6ab8 && go mod tidy'. --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 786fb465fdf7..286692259e43 100644 --- a/go.mod +++ b/go.mod @@ -71,7 +71,7 @@ require ( github.com/beevik/etree v1.2.0 github.com/google/go-cmp v0.5.9 github.com/hashicorp/aws-cloudformation-resource-schema-sdk-go v0.21.0 - github.com/hashicorp/aws-sdk-go-base/v2 v2.0.0-beta.34 + github.com/hashicorp/aws-sdk-go-base/v2 v2.0.0-beta.34.0.20230905194952-2572ed001c9c github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2 v2.0.0-beta.35 github.com/hashicorp/awspolicyequivalence v1.6.0 github.com/hashicorp/go-cleanhttp v0.5.2 diff --git a/go.sum b/go.sum index 77cd548d5fe6..6d560916551f 100644 --- a/go.sum +++ b/go.sum @@ -239,8 +239,8 @@ github.com/google/uuid v1.3.0 h1:t6JiXgmwXMjEs8VusXIJk2BXHsn+wx8BZdTaoZ5fu7I= github.com/google/uuid v1.3.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/hashicorp/aws-cloudformation-resource-schema-sdk-go v0.21.0 h1:IUypt/TbXiJBkBbE3926CgnjD8IltAitdn7Yive61DY= github.com/hashicorp/aws-cloudformation-resource-schema-sdk-go v0.21.0/go.mod h1:cdTE6F2pCKQobug+RqRaQp7Kz9hIEqiSvpPmb6E5G1w= -github.com/hashicorp/aws-sdk-go-base/v2 v2.0.0-beta.34 h1:WH0OOrhZe6wzOnA+ra0ZV0+5BWSElVriWmudH2S2cFw= -github.com/hashicorp/aws-sdk-go-base/v2 v2.0.0-beta.34/go.mod h1:cR5oVK+h10mSG4T9eHaBAYfacxUlYI5vNfJuIRMGfMA= +github.com/hashicorp/aws-sdk-go-base/v2 v2.0.0-beta.34.0.20230905194952-2572ed001c9c h1:mtg+ASeuoZ/MPpc3FElSJoB0eBKrEsjySjPOjQnJ/eQ= +github.com/hashicorp/aws-sdk-go-base/v2 v2.0.0-beta.34.0.20230905194952-2572ed001c9c/go.mod h1:cR5oVK+h10mSG4T9eHaBAYfacxUlYI5vNfJuIRMGfMA= github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2 v2.0.0-beta.35 h1:FLgIkz1RPYkYG62Q+u7M/JtU2tEKPUDMeDH+WtZ04ic= github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2 v2.0.0-beta.35/go.mod h1:AQknW73NE5hbAZn/ruNomae0OJUNf5xzsAi6yDndWgs= github.com/hashicorp/awspolicyequivalence v1.6.0 h1:7aadmkalbc5ewStC6g3rljx1iNvP4QyAhg2KsHx8bU8= From eb905e6d8f06dfe9fde981c8491aac8a982f6679 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 5 Sep 2023 16:14:26 -0400 Subject: [PATCH 08/25] Use 'tfawserr.ErrHTTPStatusCodeEquals'. --- internal/service/s3/object_copy.go | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/internal/service/s3/object_copy.go b/internal/service/s3/object_copy.go index 5c169fd37eae..751722e21392 100644 --- a/internal/service/s3/object_copy.go +++ b/internal/service/s3/object_copy.go @@ -8,6 +8,7 @@ import ( "context" "fmt" "log" + "net/http" "net/url" "strings" "time" @@ -16,7 +17,9 @@ import ( "github.com/aws/aws-sdk-go-v2/aws" "github.com/aws/aws-sdk-go-v2/service/s3" "github.com/aws/aws-sdk-go-v2/service/s3/types" + "github.com/hashicorp/aws-sdk-go-base/v2/tfawserr" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" "github.com/hashicorp/terraform-provider-aws/internal/conns" @@ -648,13 +651,12 @@ func FindObjectByThreePartKey(ctx context.Context, conn *s3.Client, bucket, key, output, err := conn.HeadObject(ctx, input) - // TODO https://github.com/aws/smithy-go/blob/main/transport/http/response.go - // if tfawserr.ErrStatusCodeEquals(err, http.StatusNotFound) { - // return nil, &retry.NotFoundError{ - // LastError: err, - // LastRequest: input, - // } - // } + if tfawserr.ErrHTTPStatusCodeEquals(err, http.StatusNotFound) { + return nil, &retry.NotFoundError{ + LastError: err, + LastRequest: input, + } + } if err != nil { return nil, err From 0a532c2d55df0446ab5a7fd5e5526ecc386d140b Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 5 Sep 2023 17:10:28 -0400 Subject: [PATCH 09/25] Call 'resourceObjectCopyRead', not 'resourceObjectRead', from 'resourceObjectCopyDoCopy'. --- internal/service/s3/object_copy.go | 31 +++++++++++++----------------- 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/internal/service/s3/object_copy.go b/internal/service/s3/object_copy.go index 751722e21392..a6da456a328d 100644 --- a/internal/service/s3/object_copy.go +++ b/internal/service/s3/object_copy.go @@ -335,26 +335,30 @@ func resourceObjectCopyRead(ctx context.Context, d *schema.ResourceData, meta in d.Set("content_encoding", output.ContentEncoding) d.Set("content_language", output.ContentLanguage) d.Set("content_type", output.ContentType) + d.Set("customer_algorithm", output.SSECustomerAlgorithm) + d.Set("customer_key_md5", output.SSECustomerKeyMD5) // See https://forums.aws.amazon.com/thread.jspa?threadID=44003 d.Set("etag", strings.Trim(aws.ToString(output.ETag), `"`)) + d.Set("expiration", output.Expiration) + d.Set("kms_key_id", output.SSEKMSKeyId) + d.Set("last_modified", flattenObjectDate(output.LastModified)) d.Set("metadata", output.Metadata) d.Set("object_lock_legal_hold_status", output.ObjectLockLegalHoldStatus) d.Set("object_lock_mode", output.ObjectLockMode) d.Set("object_lock_retain_until_date", flattenObjectDate(output.ObjectLockRetainUntilDate)) - d.Set("version_id", output.VersionId) d.Set("server_side_encryption", output.ServerSideEncryption) - d.Set("website_redirect", output.WebsiteRedirectLocation) - - if err := resourceObjectSetKMS(ctx, d, meta, output.SSEKMSKeyId); err != nil { - return sdkdiag.AppendFromErr(diags, err) - } - // The "STANDARD" (which is also the default) storage // class when set would not be included in the results. d.Set("storage_class", types.ObjectStorageClassStandard) if output.StorageClass != "" { d.Set("storage_class", output.StorageClass) } + d.Set("version_id", output.VersionId) + d.Set("website_redirect", output.WebsiteRedirectLocation) + + if err := resourceObjectSetKMS(ctx, d, meta, output.SSEKMSKeyId); err != nil { + return sdkdiag.AppendFromErr(diags, err) + } // Retry due to S3 eventual consistency tagsRaw, err := tfresource.RetryWhenIsA[*types.NoSuchBucket](ctx, 2*time.Minute, func() (interface{}, error) { @@ -623,21 +627,12 @@ func resourceObjectCopyDoCopy(ctx context.Context, d *schema.ResourceData, meta d.SetId(d.Get("key").(string)) } - d.Set("customer_algorithm", output.SSECustomerAlgorithm) - d.Set("customer_key_md5", output.SSECustomerKeyMD5) - if output.CopyObjectResult != nil { - d.Set("etag", strings.Trim(aws.ToString(output.CopyObjectResult.ETag), `"`)) - d.Set("last_modified", flattenObjectDate(output.CopyObjectResult.LastModified)) - } - d.Set("expiration", output.Expiration) + // These attributes aren't returned from HeadObject. d.Set("kms_encryption_context", output.SSEKMSEncryptionContext) - d.Set("kms_key_id", output.SSEKMSKeyId) d.Set("request_charged", output.RequestCharged == types.RequestChargedRequester) - d.Set("server_side_encryption", output.ServerSideEncryption) d.Set("source_version_id", output.CopySourceVersionId) - d.Set("version_id", output.VersionId) - return append(diags, resourceObjectRead(ctx, d, meta)...) + return append(diags, resourceObjectCopyRead(ctx, d, meta)...) } func FindObjectByThreePartKey(ctx context.Context, conn *s3.Client, bucket, key, etag string) (*s3.HeadObjectOutput, error) { From 5108f870567117d5cd51df16583fc481be4c3884 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 5 Sep 2023 17:18:04 -0400 Subject: [PATCH 10/25] Tidy up 'TestAccS3ObjectCopy_basic'. --- internal/service/s3/object_copy_test.go | 82 ++++++++++++++++++++++--- 1 file changed, 75 insertions(+), 7 deletions(-) diff --git a/internal/service/s3/object_copy_test.go b/internal/service/s3/object_copy_test.go index e7d6962cddb5..89d6721327ba 100644 --- a/internal/service/s3/object_copy_test.go +++ b/internal/service/s3/object_copy_test.go @@ -24,8 +24,8 @@ func TestAccS3ObjectCopy_basic(t *testing.T) { rName2 := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_s3_object_copy.test" sourceName := "aws_s3_object.source" - key := "HundBegraven" - sourceKey := "WshngtnNtnls" + sourceKey := "source" + targetKey := "target" resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(ctx, t) }, @@ -34,13 +34,53 @@ func TestAccS3ObjectCopy_basic(t *testing.T) { CheckDestroy: testAccCheckObjectCopyDestroy(ctx), Steps: []resource.TestStep{ { - Config: testAccObjectCopyConfig_basic(rName1, sourceKey, rName2, key), - Check: resource.ComposeTestCheckFunc( + Config: testAccObjectCopyConfig_basic(rName1, sourceKey, rName2, targetKey), + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectCopyExists(ctx, resourceName), + resource.TestCheckNoResourceAttr(resourceName, "acl"), resource.TestCheckResourceAttr(resourceName, "bucket", rName2), - resource.TestCheckResourceAttr(resourceName, "key", key), - resource.TestCheckResourceAttr(resourceName, "source", fmt.Sprintf("%s/%s", rName1, sourceKey)), + resource.TestCheckResourceAttr(resourceName, "bucket_key_enabled", "false"), + resource.TestCheckResourceAttr(resourceName, "cache_control", ""), + resource.TestCheckResourceAttr(resourceName, "content_disposition", ""), + resource.TestCheckResourceAttr(resourceName, "content_encoding", ""), + resource.TestCheckResourceAttr(resourceName, "content_language", ""), + resource.TestCheckResourceAttr(resourceName, "content_type", "binary/octet-stream"), + resource.TestCheckNoResourceAttr(resourceName, "copy_if_match"), + resource.TestCheckNoResourceAttr(resourceName, "copy_if_modified_since"), + resource.TestCheckNoResourceAttr(resourceName, "copy_if_none_match"), + resource.TestCheckNoResourceAttr(resourceName, "copy_if_unmodified_since"), + resource.TestCheckResourceAttr(resourceName, "customer_algorithm", ""), + resource.TestCheckNoResourceAttr(resourceName, "customer_key"), + resource.TestCheckResourceAttr(resourceName, "customer_key_md5", ""), resource.TestCheckResourceAttrPair(resourceName, "etag", sourceName, "etag"), + resource.TestCheckNoResourceAttr(resourceName, "expected_bucket_owner"), + resource.TestCheckNoResourceAttr(resourceName, "expected_source_bucket_owner"), + resource.TestCheckResourceAttr(resourceName, "expiration", ""), + resource.TestCheckNoResourceAttr(resourceName, "expires"), + resource.TestCheckResourceAttr(resourceName, "force_destroy", "false"), + resource.TestCheckResourceAttr(resourceName, "grant.#", "0"), + resource.TestCheckResourceAttr(resourceName, "key", targetKey), + resource.TestCheckResourceAttr(resourceName, "kms_encryption_context", ""), + resource.TestCheckResourceAttr(resourceName, "kms_key_id", ""), + resource.TestCheckResourceAttrSet(resourceName, "last_modified"), + resource.TestCheckResourceAttr(resourceName, "metadata.%", "0"), + resource.TestCheckNoResourceAttr(resourceName, "metadata_directive"), + resource.TestCheckResourceAttr(resourceName, "object_lock_legal_hold_status", ""), + resource.TestCheckResourceAttr(resourceName, "object_lock_mode", ""), + resource.TestCheckResourceAttr(resourceName, "object_lock_retain_until_date", ""), + resource.TestCheckResourceAttr(resourceName, "request_charged", "false"), + resource.TestCheckNoResourceAttr(resourceName, "request_payer"), + resource.TestCheckResourceAttr(resourceName, "server_side_encryption", "AES256"), + resource.TestCheckResourceAttr(resourceName, "source", fmt.Sprintf("%s/%s", rName1, sourceKey)), + resource.TestCheckNoResourceAttr(resourceName, "source_customer_algorithm"), + resource.TestCheckNoResourceAttr(resourceName, "source_customer_key"), + resource.TestCheckNoResourceAttr(resourceName, "source_customer_key_md5"), + resource.TestCheckResourceAttr(resourceName, "source_version_id", ""), + resource.TestCheckResourceAttr(resourceName, "storage_class", "STANDARD"), + resource.TestCheckNoResourceAttr(resourceName, "tagging_directive"), + resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), + resource.TestCheckResourceAttr(resourceName, "version_id", ""), + resource.TestCheckResourceAttr(resourceName, "website_redirect", ""), ), }, }, @@ -132,7 +172,35 @@ func testAccCheckObjectCopyExists(ctx context.Context, n string) resource.TestCh } } -func testAccObjectCopyConfig_basic(rName1, sourceKey, rName2, key string) string { +func testAccObjectCopyConfig_base(sourceBucket, sourceKey, targetBucket string) string { + return fmt.Sprintf(` +resource "aws_s3_bucket" "source" { + bucket = %[1]q +} + +resource "aws_s3_object" "source" { + bucket = aws_s3_bucket.source.bucket + key = %[2]q + content = "0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ" +} + +resource "aws_s3_bucket" "target" { + bucket = %[3]q +} +`, sourceBucket, sourceKey, targetBucket) +} + +func testAccObjectCopyConfig_basic(sourceBucket, sourceKey, targetBucket, targetKey string) string { + return acctest.ConfigCompose(testAccObjectCopyConfig_base(sourceBucket, sourceKey, targetBucket), fmt.Sprintf(` +resource "aws_s3_object_copy" "test" { + bucket = aws_s3_bucket.target.bucket + key = %[1]q + source = "${aws_s3_bucket.source.bucket}/${aws_s3_object.source.key}" +} +`, targetKey)) +} + +func testAccObjectCopyConfig_grant(rName1, sourceKey, rName2, key string) string { return fmt.Sprintf(` resource "aws_s3_bucket" "source" { bucket = %[1]q From 6a7f5563336f49207fd4cfbc94bf602213c05cbb Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 5 Sep 2023 17:19:06 -0400 Subject: [PATCH 11/25] Acceptance test output: % make testacc TESTARGS='-run=TestAccS3ObjectCopy_basic' PKG=s3 ACCTEST_PARALLELISM=2 ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./internal/service/s3/... -v -count 1 -parallel 2 -run=TestAccS3ObjectCopy_basic -timeout 180m === RUN TestAccS3ObjectCopy_basic === PAUSE TestAccS3ObjectCopy_basic === CONT TestAccS3ObjectCopy_basic --- PASS: TestAccS3ObjectCopy_basic (42.01s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/s3 48.447s From ae5b011957790fc72fa77bec32caf3dd846ed988 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 5 Sep 2023 17:22:02 -0400 Subject: [PATCH 12/25] Add 'TestAccS3ObjectCopy_disappears'. --- internal/service/s3/object_copy_test.go | 26 +++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/internal/service/s3/object_copy_test.go b/internal/service/s3/object_copy_test.go index 89d6721327ba..ecf015ab020e 100644 --- a/internal/service/s3/object_copy_test.go +++ b/internal/service/s3/object_copy_test.go @@ -87,6 +87,32 @@ func TestAccS3ObjectCopy_basic(t *testing.T) { }) } +func TestAccS3ObjectCopy_disappears(t *testing.T) { + ctx := acctest.Context(t) + rName1 := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + rName2 := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_s3_object_copy.test" + sourceKey := "source" + targetKey := "target" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, names.S3EndpointID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckObjectCopyDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccObjectCopyConfig_basic(rName1, sourceKey, rName2, targetKey), + Check: resource.ComposeTestCheckFunc( + testAccCheckObjectCopyExists(ctx, resourceName), + acctest.CheckResourceDisappears(ctx, acctest.Provider, tfs3.ResourceObjectCopy(), resourceName), + ), + ExpectNonEmptyPlan: true, + }, + }, + }) +} + func TestAccS3ObjectCopy_BucketKeyEnabled_bucket(t *testing.T) { ctx := acctest.Context(t) rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) From 271f43e96b8bfa083bd03a55865622d9557d6709 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Tue, 5 Sep 2023 17:22:11 -0400 Subject: [PATCH 13/25] Acceptance test output: % make testacc TESTARGS='-run=TestAccS3ObjectCopy_disappears' PKG=s3 ACCTEST_PARALLELISM=2 ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./internal/service/s3/... -v -count 1 -parallel 2 -run=TestAccS3ObjectCopy_disappears -timeout 180m === RUN TestAccS3ObjectCopy_disappears === PAUSE TestAccS3ObjectCopy_disappears === CONT TestAccS3ObjectCopy_disappears --- PASS: TestAccS3ObjectCopy_disappears (43.24s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/s3 48.855s From d8f56b245e8fc38fc715f28e871ff6d6fce1691f Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 6 Sep 2023 10:23:56 -0400 Subject: [PATCH 14/25] Acceptance test output: % make testacc TESTARGS='-run=TestAccS3ObjectCopy_tags' PKG=s3 ACCTEST_PARALLELISM=2 ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./internal/service/s3/... -v -count 1 -parallel 2 -run=TestAccS3ObjectCopy_tags -timeout 180m === RUN TestAccS3ObjectCopy_tags === PAUSE TestAccS3ObjectCopy_tags === CONT TestAccS3ObjectCopy_tags --- PASS: TestAccS3ObjectCopy_tags (68.37s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/s3 73.887s From 22b34edc6dea5b9aca81e6c76f792f3fe13204ec Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 6 Sep 2023 10:39:04 -0400 Subject: [PATCH 15/25] Add 'TestAccS3ObjectCopy_metadata'. --- internal/service/s3/object_copy_test.go | 43 +++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/internal/service/s3/object_copy_test.go b/internal/service/s3/object_copy_test.go index f0abd88608de..2b6372b50ad4 100644 --- a/internal/service/s3/object_copy_test.go +++ b/internal/service/s3/object_copy_test.go @@ -159,6 +159,33 @@ func TestAccS3ObjectCopy_tags(t *testing.T) { }) } +func TestAccS3ObjectCopy_metadata(t *testing.T) { + ctx := acctest.Context(t) + rName1 := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + rName2 := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_s3_object_copy.test" + sourceKey := "source" + targetKey := "target" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, names.S3EndpointID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckObjectCopyDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccObjectCopyConfig_metadata(rName1, sourceKey, rName2, targetKey), + Check: resource.ComposeTestCheckFunc( + testAccCheckObjectCopyExists(ctx, resourceName), + resource.TestCheckResourceAttr(resourceName, "metadata_directive", "REPLACE"), + resource.TestCheckResourceAttr(resourceName, "metadata.%", "1"), + resource.TestCheckResourceAttr(resourceName, "metadata.mk1", "mv1"), + ), + }, + }, + }) +} + func TestAccS3ObjectCopy_BucketKeyEnabled_bucket(t *testing.T) { ctx := acctest.Context(t) rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) @@ -305,6 +332,22 @@ resource "aws_s3_object_copy" "test" { `, targetKey, tagKey1, tagValue1, tagKey2, tagValue2)) } +func testAccObjectCopyConfig_metadata(sourceBucket, sourceKey, targetBucket, targetKey string) string { + return acctest.ConfigCompose(testAccObjectCopyConfig_base(sourceBucket, sourceKey, targetBucket), fmt.Sprintf(` +resource "aws_s3_object_copy" "test" { + bucket = aws_s3_bucket.target.bucket + key = %[1]q + source = "${aws_s3_bucket.source.bucket}/${aws_s3_object.source.key}" + + metadata_directive = "REPLACE" + + metadata = { + "mk1" = "mv1" + } +} +`, targetKey)) +} + func testAccObjectCopyConfig_grant(rName1, sourceKey, rName2, key string) string { return fmt.Sprintf(` resource "aws_s3_bucket" "source" { From e35c8d6c78db2eaff864532517d2dc5662125708 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 6 Sep 2023 10:39:27 -0400 Subject: [PATCH 16/25] Acceptance test output: % make testacc TESTARGS='-run=TestAccS3ObjectCopy_metadata' PKG=s3 ACCTEST_PARALLELISM=2 ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./internal/service/s3/... -v -count 1 -parallel 2 -run=TestAccS3ObjectCopy_metadata -timeout 180m === RUN TestAccS3ObjectCopy_metadata === PAUSE TestAccS3ObjectCopy_metadata === CONT TestAccS3ObjectCopy_metadata --- PASS: TestAccS3ObjectCopy_metadata (74.15s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/s3 95.276s From c08c837da9235f7d58623e6c8e9b0e5628074ec7 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 6 Sep 2023 11:25:56 -0400 Subject: [PATCH 17/25] Add 'TestAccS3ObjectCopy_grant'. --- internal/service/s3/object_copy_test.go | 52 ++++++++++++++++--------- 1 file changed, 34 insertions(+), 18 deletions(-) diff --git a/internal/service/s3/object_copy_test.go b/internal/service/s3/object_copy_test.go index 2b6372b50ad4..5e2aabc0ec68 100644 --- a/internal/service/s3/object_copy_test.go +++ b/internal/service/s3/object_copy_test.go @@ -186,6 +186,36 @@ func TestAccS3ObjectCopy_metadata(t *testing.T) { }) } +func TestAccS3ObjectCopy_grant(t *testing.T) { + ctx := acctest.Context(t) + rName1 := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + rName2 := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_s3_object_copy.test" + sourceKey := "source" + targetKey := "target" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, names.S3EndpointID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckObjectCopyDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccObjectCopyConfig_grant(rName1, sourceKey, rName2, targetKey), + Check: resource.ComposeTestCheckFunc( + testAccCheckObjectCopyExists(ctx, resourceName), + resource.TestCheckResourceAttr(resourceName, "grant.#", "1"), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "grant.*", map[string]string{ + "permissions.#": "1", + "type": "Group", + "uri": "http://acs.amazonaws.com/groups/global/AllUsers", + }), + ), + }, + }, + }) +} + func TestAccS3ObjectCopy_BucketKeyEnabled_bucket(t *testing.T) { ctx := acctest.Context(t) rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) @@ -348,22 +378,8 @@ resource "aws_s3_object_copy" "test" { `, targetKey)) } -func testAccObjectCopyConfig_grant(rName1, sourceKey, rName2, key string) string { - return fmt.Sprintf(` -resource "aws_s3_bucket" "source" { - bucket = %[1]q -} - -resource "aws_s3_object" "source" { - bucket = aws_s3_bucket.source.bucket - key = %[2]q - content = "Ingen ko på isen" -} - -resource "aws_s3_bucket" "target" { - bucket = %[3]q -} - +func testAccObjectCopyConfig_grant(sourceBucket, sourceKey, targetBucket, targetKey string) string { + return acctest.ConfigCompose(testAccObjectCopyConfig_base(sourceBucket, sourceKey, targetBucket), fmt.Sprintf(` resource "aws_s3_bucket_public_access_block" "target" { bucket = aws_s3_bucket.target.id @@ -387,7 +403,7 @@ resource "aws_s3_object_copy" "test" { ] bucket = aws_s3_bucket.target.bucket - key = %[4]q + key = %[1]q source = "${aws_s3_bucket.source.bucket}/${aws_s3_object.source.key}" grant { @@ -396,7 +412,7 @@ resource "aws_s3_object_copy" "test" { permissions = ["READ"] } } -`, rName1, sourceKey, rName2, key) +`, targetKey)) } func testAccObjectCopyConfig_bucketKeyEnabledBucket(rName string) string { From 7595d4e8e51983469edf8be42e5c5961bd7a5494 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 6 Sep 2023 11:26:05 -0400 Subject: [PATCH 18/25] Acceptance test output: % make testacc TESTARGS='-run=TestAccS3ObjectCopy_grant' PKG=s3 ACCTEST_PARALLELISM=2 ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./internal/service/s3/... -v -count 1 -parallel 2 -run=TestAccS3ObjectCopy_grant -timeout 180m === RUN TestAccS3ObjectCopy_grant === PAUSE TestAccS3ObjectCopy_grant === CONT TestAccS3ObjectCopy_grant --- PASS: TestAccS3ObjectCopy_grant (26.52s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/s3 31.858s From cd16437de338e9c2fab354186f2a61b4fb9c0d6f Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 6 Sep 2023 11:42:14 -0400 Subject: [PATCH 19/25] Acceptance test output: % make testacc TESTARGS='-run=TestAccS3ObjectCopy_BucketKeyEnabled_' PKG=s3 ACCTEST_PARALLELISM=2 ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./internal/service/s3/... -v -count 1 -parallel 2 -run=TestAccS3ObjectCopy_BucketKeyEnabled_ -timeout 180m === RUN TestAccS3ObjectCopy_BucketKeyEnabled_bucket === PAUSE TestAccS3ObjectCopy_BucketKeyEnabled_bucket === RUN TestAccS3ObjectCopy_BucketKeyEnabled_object === PAUSE TestAccS3ObjectCopy_BucketKeyEnabled_object === CONT TestAccS3ObjectCopy_BucketKeyEnabled_bucket === CONT TestAccS3ObjectCopy_BucketKeyEnabled_object --- PASS: TestAccS3ObjectCopy_BucketKeyEnabled_object (27.50s) --- PASS: TestAccS3ObjectCopy_BucketKeyEnabled_bucket (27.67s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/s3 33.161s --- internal/service/s3/object_copy_test.go | 71 +++++++++---------------- 1 file changed, 24 insertions(+), 47 deletions(-) diff --git a/internal/service/s3/object_copy_test.go b/internal/service/s3/object_copy_test.go index 5e2aabc0ec68..45e22469be06 100644 --- a/internal/service/s3/object_copy_test.go +++ b/internal/service/s3/object_copy_test.go @@ -218,8 +218,11 @@ func TestAccS3ObjectCopy_grant(t *testing.T) { func TestAccS3ObjectCopy_BucketKeyEnabled_bucket(t *testing.T) { ctx := acctest.Context(t) - rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + rName1 := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + rName2 := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_s3_object_copy.test" + sourceKey := "source" + targetKey := "target" resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(ctx, t) }, @@ -228,7 +231,7 @@ func TestAccS3ObjectCopy_BucketKeyEnabled_bucket(t *testing.T) { CheckDestroy: testAccCheckObjectCopyDestroy(ctx), Steps: []resource.TestStep{ { - Config: testAccObjectCopyConfig_bucketKeyEnabledBucket(rName), + Config: testAccObjectCopyConfig_bucketKeyEnabledBucket(rName1, sourceKey, rName2, targetKey), Check: resource.ComposeTestCheckFunc( testAccCheckObjectCopyExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "bucket_key_enabled", "true"), @@ -240,8 +243,11 @@ func TestAccS3ObjectCopy_BucketKeyEnabled_bucket(t *testing.T) { func TestAccS3ObjectCopy_BucketKeyEnabled_object(t *testing.T) { ctx := acctest.Context(t) - rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + rName1 := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + rName2 := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_s3_object_copy.test" + sourceKey := "source" + targetKey := "target" resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(ctx, t) }, @@ -250,7 +256,7 @@ func TestAccS3ObjectCopy_BucketKeyEnabled_object(t *testing.T) { CheckDestroy: testAccCheckObjectCopyDestroy(ctx), Steps: []resource.TestStep{ { - Config: testAccObjectCopyConfig_bucketKeyEnabled(rName), + Config: testAccObjectCopyConfig_bucketKeyEnabledObject(rName1, sourceKey, rName2, targetKey), Check: resource.ComposeTestCheckFunc( testAccCheckObjectCopyExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "bucket_key_enabled", "true"), @@ -415,28 +421,18 @@ resource "aws_s3_object_copy" "test" { `, targetKey)) } -func testAccObjectCopyConfig_bucketKeyEnabledBucket(rName string) string { - return fmt.Sprintf(` +func testAccObjectCopyConfig_baseBucketKeyEnabled(sourceBucket, sourceKey, targetBucket string) string { + return acctest.ConfigCompose(testAccObjectCopyConfig_base(sourceBucket, sourceKey, targetBucket), fmt.Sprintf(` resource "aws_kms_key" "test" { - description = "Encrypts test objects" + description = %[1]q deletion_window_in_days = 7 } - -resource "aws_s3_bucket" "source" { - bucket = "%[1]s-source" -} - -resource "aws_s3_object" "source" { - bucket = aws_s3_bucket.source.bucket - content = "Ingen ko på isen" - key = "test" -} - -resource "aws_s3_bucket" "target" { - bucket = "%[1]s-target" +`, targetBucket)) } -resource "aws_s3_bucket_server_side_encryption_configuration" "test" { +func testAccObjectCopyConfig_bucketKeyEnabledBucket(sourceBucket, sourceKey, targetBucket, targetKey string) string { + return acctest.ConfigCompose(testAccObjectCopyConfig_baseBucketKeyEnabled(sourceBucket, sourceKey, targetBucket), fmt.Sprintf(` +resource "aws_s3_bucket_server_side_encryption_configuration" "target" { bucket = aws_s3_bucket.target.id rule { @@ -450,42 +446,23 @@ resource "aws_s3_bucket_server_side_encryption_configuration" "test" { resource "aws_s3_object_copy" "test" { # Must have bucket SSE enabled first - depends_on = [aws_s3_bucket_server_side_encryption_configuration.test] + depends_on = [aws_s3_bucket_server_side_encryption_configuration.target] bucket = aws_s3_bucket.target.bucket - key = "test" + key = %[1]q source = "${aws_s3_bucket.source.bucket}/${aws_s3_object.source.key}" } -`, rName) -} - -func testAccObjectCopyConfig_bucketKeyEnabled(rName string) string { - return fmt.Sprintf(` -resource "aws_kms_key" "test" { - description = "Encrypts test objects" - deletion_window_in_days = 7 -} - -resource "aws_s3_bucket" "source" { - bucket = "%[1]s-source" -} - -resource "aws_s3_object" "source" { - bucket = aws_s3_bucket.source.bucket - content = "Ingen ko på isen" - key = "test" -} - -resource "aws_s3_bucket" "target" { - bucket = "%[1]s-target" +`, targetKey)) } +func testAccObjectCopyConfig_bucketKeyEnabledObject(sourceBucket, sourceKey, targetBucket, targetKey string) string { + return acctest.ConfigCompose(testAccObjectCopyConfig_baseBucketKeyEnabled(sourceBucket, sourceKey, targetBucket), fmt.Sprintf(` resource "aws_s3_object_copy" "test" { bucket = aws_s3_bucket.target.bucket bucket_key_enabled = true - key = "test" + key = %[1]q kms_key_id = aws_kms_key.test.arn source = "${aws_s3_bucket.source.bucket}/${aws_s3_object.source.key}" } -`, rName) +`, targetKey)) } From e66f03340e7fa7045c73dd0cacd301abaf3ae276 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 6 Sep 2023 11:47:57 -0400 Subject: [PATCH 20/25] Add 'TestAccS3ObjectCopy_sourceAndTargetWithSlashes'. --- internal/service/s3/object_copy_test.go | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/internal/service/s3/object_copy_test.go b/internal/service/s3/object_copy_test.go index 45e22469be06..0537830d4cf4 100644 --- a/internal/service/s3/object_copy_test.go +++ b/internal/service/s3/object_copy_test.go @@ -266,6 +266,31 @@ func TestAccS3ObjectCopy_BucketKeyEnabled_object(t *testing.T) { }) } +func TestAccS3ObjectCopy_sourceAndTargetWithSlashes(t *testing.T) { + ctx := acctest.Context(t) + rName1 := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + rName2 := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_s3_object_copy.test" + sourceKey := "dir1/dir2/source" + targetKey := "dir3/target" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, names.S3EndpointID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckObjectCopyDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccObjectCopyConfig_basic(rName1, sourceKey, rName2, targetKey), + Check: resource.ComposeTestCheckFunc( + testAccCheckObjectCopyExists(ctx, resourceName), + resource.TestCheckResourceAttr(resourceName, "source", fmt.Sprintf("%s/%s", rName1, sourceKey)), + ), + }, + }, + }) +} + func testAccCheckObjectCopyDestroy(ctx context.Context) resource.TestCheckFunc { return func(s *terraform.State) error { conn := acctest.Provider.Meta().(*conns.AWSClient).S3Client(ctx) From 820f6c5a07934055a3a0d944b51864413cb10443 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 6 Sep 2023 11:48:06 -0400 Subject: [PATCH 21/25] Acceptance test output: % make testacc TESTARGS='-run=TestAccS3ObjectCopy_sourceAndTargetWithSlashes' PKG=s3 ACCTEST_PARALLELISM=2 ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./internal/service/s3/... -v -count 1 -parallel 2 -run=TestAccS3ObjectCopy_sourceAndTargetWithSlashes -timeout 180m === RUN TestAccS3ObjectCopy_sourceAndTargetWithSlashes === PAUSE TestAccS3ObjectCopy_sourceAndTargetWithSlashes === CONT TestAccS3ObjectCopy_sourceAndTargetWithSlashes --- PASS: TestAccS3ObjectCopy_sourceAndTargetWithSlashes (25.79s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/s3 31.058s From ee57bb5d69e67f10fc4867c10703e3022f9142c9 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 6 Sep 2023 12:59:44 -0400 Subject: [PATCH 22/25] External source object in 'TestAccS3ObjectCopy_sourceWithSlashes'. --- internal/service/s3/object_copy_test.go | 56 ++++++++++++++++++------- 1 file changed, 40 insertions(+), 16 deletions(-) diff --git a/internal/service/s3/object_copy_test.go b/internal/service/s3/object_copy_test.go index 0537830d4cf4..1506fc953388 100644 --- a/internal/service/s3/object_copy_test.go +++ b/internal/service/s3/object_copy_test.go @@ -266,13 +266,13 @@ func TestAccS3ObjectCopy_BucketKeyEnabled_object(t *testing.T) { }) } -func TestAccS3ObjectCopy_sourceAndTargetWithSlashes(t *testing.T) { +func TestAccS3ObjectCopy_sourceWithSlashes(t *testing.T) { ctx := acctest.Context(t) rName1 := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) rName2 := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_s3_object_copy.test" sourceKey := "dir1/dir2/source" - targetKey := "dir3/target" + targetKey := "target" resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(ctx, t) }, @@ -281,7 +281,13 @@ func TestAccS3ObjectCopy_sourceAndTargetWithSlashes(t *testing.T) { CheckDestroy: testAccCheckObjectCopyDestroy(ctx), Steps: []resource.TestStep{ { - Config: testAccObjectCopyConfig_basic(rName1, sourceKey, rName2, targetKey), + Config: testAccObjectCopyConfig_baseSourceAndTargetBuckets(rName1, rName2), + Check: resource.ComposeTestCheckFunc( + testAccCheckBucketAddObjects(ctx, "aws_s3_bucket.source", sourceKey), + ), + }, + { + Config: testAccObjectCopyConfig_externalSourceObject(rName1, sourceKey, rName2, targetKey), Check: resource.ComposeTestCheckFunc( testAccCheckObjectCopyExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "source", fmt.Sprintf("%s/%s", rName1, sourceKey)), @@ -332,26 +338,34 @@ func testAccCheckObjectCopyExists(ctx context.Context, n string) resource.TestCh } } -func testAccObjectCopyConfig_base(sourceBucket, sourceKey, targetBucket string) string { +func testAccObjectCopyConfig_baseSourceAndTargetBuckets(sourceBucket, targetBucket string) string { return fmt.Sprintf(` resource "aws_s3_bucket" "source" { bucket = %[1]q + + force_destroy = true +} + +resource "aws_s3_bucket" "target" { + bucket = %[2]q + + force_destroy = true +} +`, sourceBucket, targetBucket) } +func testAccObjectCopyConfig_baseSourceObject(sourceBucket, sourceKey, targetBucket string) string { + return acctest.ConfigCompose(testAccObjectCopyConfig_baseSourceAndTargetBuckets(sourceBucket, targetBucket), fmt.Sprintf(` resource "aws_s3_object" "source" { bucket = aws_s3_bucket.source.bucket - key = %[2]q + key = %[1]q content = "0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ" } - -resource "aws_s3_bucket" "target" { - bucket = %[3]q -} -`, sourceBucket, sourceKey, targetBucket) +`, sourceKey)) } func testAccObjectCopyConfig_basic(sourceBucket, sourceKey, targetBucket, targetKey string) string { - return acctest.ConfigCompose(testAccObjectCopyConfig_base(sourceBucket, sourceKey, targetBucket), fmt.Sprintf(` + return acctest.ConfigCompose(testAccObjectCopyConfig_baseSourceObject(sourceBucket, sourceKey, targetBucket), fmt.Sprintf(` resource "aws_s3_object_copy" "test" { bucket = aws_s3_bucket.target.bucket key = %[1]q @@ -361,7 +375,7 @@ resource "aws_s3_object_copy" "test" { } func testAccObjectCopyConfig_tags1(sourceBucket, sourceKey, targetBucket, targetKey, tagKey1, tagValue1 string) string { - return acctest.ConfigCompose(testAccObjectCopyConfig_base(sourceBucket, sourceKey, targetBucket), fmt.Sprintf(` + return acctest.ConfigCompose(testAccObjectCopyConfig_baseSourceObject(sourceBucket, sourceKey, targetBucket), fmt.Sprintf(` resource "aws_s3_object_copy" "test" { bucket = aws_s3_bucket.target.bucket key = %[1]q @@ -377,7 +391,7 @@ resource "aws_s3_object_copy" "test" { } func testAccObjectCopyConfig_tags2(sourceBucket, sourceKey, targetBucket, targetKey, tagKey1, tagValue1, tagKey2, tagValue2 string) string { - return acctest.ConfigCompose(testAccObjectCopyConfig_base(sourceBucket, sourceKey, targetBucket), fmt.Sprintf(` + return acctest.ConfigCompose(testAccObjectCopyConfig_baseSourceObject(sourceBucket, sourceKey, targetBucket), fmt.Sprintf(` resource "aws_s3_object_copy" "test" { bucket = aws_s3_bucket.target.bucket key = %[1]q @@ -394,7 +408,7 @@ resource "aws_s3_object_copy" "test" { } func testAccObjectCopyConfig_metadata(sourceBucket, sourceKey, targetBucket, targetKey string) string { - return acctest.ConfigCompose(testAccObjectCopyConfig_base(sourceBucket, sourceKey, targetBucket), fmt.Sprintf(` + return acctest.ConfigCompose(testAccObjectCopyConfig_baseSourceObject(sourceBucket, sourceKey, targetBucket), fmt.Sprintf(` resource "aws_s3_object_copy" "test" { bucket = aws_s3_bucket.target.bucket key = %[1]q @@ -410,7 +424,7 @@ resource "aws_s3_object_copy" "test" { } func testAccObjectCopyConfig_grant(sourceBucket, sourceKey, targetBucket, targetKey string) string { - return acctest.ConfigCompose(testAccObjectCopyConfig_base(sourceBucket, sourceKey, targetBucket), fmt.Sprintf(` + return acctest.ConfigCompose(testAccObjectCopyConfig_baseSourceObject(sourceBucket, sourceKey, targetBucket), fmt.Sprintf(` resource "aws_s3_bucket_public_access_block" "target" { bucket = aws_s3_bucket.target.id @@ -447,7 +461,7 @@ resource "aws_s3_object_copy" "test" { } func testAccObjectCopyConfig_baseBucketKeyEnabled(sourceBucket, sourceKey, targetBucket string) string { - return acctest.ConfigCompose(testAccObjectCopyConfig_base(sourceBucket, sourceKey, targetBucket), fmt.Sprintf(` + return acctest.ConfigCompose(testAccObjectCopyConfig_baseSourceObject(sourceBucket, sourceKey, targetBucket), fmt.Sprintf(` resource "aws_kms_key" "test" { description = %[1]q deletion_window_in_days = 7 @@ -491,3 +505,13 @@ resource "aws_s3_object_copy" "test" { } `, targetKey)) } + +func testAccObjectCopyConfig_externalSourceObject(sourceBucket, sourceKey, targetBucket, targetKey string) string { + return acctest.ConfigCompose(testAccObjectCopyConfig_baseSourceAndTargetBuckets(sourceBucket, targetBucket), fmt.Sprintf(` +resource "aws_s3_object_copy" "test" { + bucket = aws_s3_bucket.target.bucket + key = %[2]q + source = "${aws_s3_bucket.source.bucket}/%[1]s" +} +`, sourceKey, targetKey)) +} From a0feb96bac719e35dd0af77d6b35f5967a55266d Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 6 Sep 2023 13:00:01 -0400 Subject: [PATCH 23/25] Acceptance test output: % make testacc TESTARGS='-run=TestAccS3ObjectCopy_sourceWithSlashes' PKG=s3 ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./internal/service/s3/... -v -count 1 -parallel 20 -run=TestAccS3ObjectCopy_sourceWithSlashes -timeout 180m === RUN TestAccS3ObjectCopy_sourceWithSlashes === PAUSE TestAccS3ObjectCopy_sourceWithSlashes === CONT TestAccS3ObjectCopy_sourceWithSlashes --- PASS: TestAccS3ObjectCopy_sourceWithSlashes (44.62s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/s3 50.263s From b16ed8f24328ae0ffc77add061e5716fa40d0434 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 6 Sep 2023 13:46:00 -0400 Subject: [PATCH 24/25] r/aws_s3_object_copy: Amazon S3 Object Tags are strongly consistent -- remove retry. --- internal/service/s3/object_copy.go | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/internal/service/s3/object_copy.go b/internal/service/s3/object_copy.go index 2afedc15b784..9628dd10186d 100644 --- a/internal/service/s3/object_copy.go +++ b/internal/service/s3/object_copy.go @@ -11,7 +11,6 @@ import ( "net/http" "net/url" "strings" - "time" "github.com/YakDriver/regexache" "github.com/aws/aws-sdk-go-v2/aws" @@ -360,21 +359,12 @@ func resourceObjectCopyRead(ctx context.Context, d *schema.ResourceData, meta in return sdkdiag.AppendFromErr(diags, err) } - // Retry due to S3 eventual consistency - tagsRaw, err := tfresource.RetryWhenIsA[*types.NoSuchBucket](ctx, 2*time.Minute, func() (interface{}, error) { - return ObjectListTags(ctx, conn, bucket, key) - }) + tags, err := ObjectListTags(ctx, conn, bucket, key) if err != nil { return sdkdiag.AppendErrorf(diags, "listing tags for S3 Bucket (%s) Object (%s): %s", bucket, key, err) } - tags, ok := tagsRaw.(tftags.KeyValueTags) - - if !ok { - return sdkdiag.AppendErrorf(diags, "listing tags for S3 Bucket (%s) Object (%s): unable to convert tags", bucket, key) - } - setTagsOut(ctx, Tags(tags)) return diags From 99defa8fdab51ee06692d03f03267c18b05b9959 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 6 Sep 2023 13:49:51 -0400 Subject: [PATCH 25/25] s3: 'propagationTimeout' -> 's3BucketPropagationTimeout'. --- internal/service/s3/bucket_inventory.go | 4 ++-- internal/service/s3/bucket_logging.go | 2 +- internal/service/s3/bucket_metric.go | 2 +- internal/service/s3/bucket_notification.go | 2 +- internal/service/s3/bucket_public_access_block.go | 2 +- internal/service/s3/bucket_replication_configuration.go | 6 +++--- .../s3/bucket_server_side_encryption_configuration.go | 6 +++--- internal/service/s3/wait.go | 7 +++++-- 8 files changed, 17 insertions(+), 14 deletions(-) diff --git a/internal/service/s3/bucket_inventory.go b/internal/service/s3/bucket_inventory.go index 2d2cb1b4fc3f..f65589d9b6b1 100644 --- a/internal/service/s3/bucket_inventory.go +++ b/internal/service/s3/bucket_inventory.go @@ -232,7 +232,7 @@ func resourceBucketInventoryPut(ctx context.Context, d *schema.ResourceData, met } log.Printf("[DEBUG] Putting S3 bucket inventory configuration: %s", input) - err := retry.RetryContext(ctx, propagationTimeout, func() *retry.RetryError { + err := retry.RetryContext(ctx, s3BucketPropagationTimeout, func() *retry.RetryError { _, err := conn.PutBucketInventoryConfigurationWithContext(ctx, input) if tfawserr.ErrCodeEquals(err, s3.ErrCodeNoSuchBucket) { @@ -310,7 +310,7 @@ func resourceBucketInventoryRead(ctx context.Context, d *schema.ResourceData, me log.Printf("[DEBUG] Reading S3 bucket inventory configuration: %s", input) var output *s3.GetBucketInventoryConfigurationOutput - err = retry.RetryContext(ctx, propagationTimeout, func() *retry.RetryError { + err = retry.RetryContext(ctx, s3BucketPropagationTimeout, func() *retry.RetryError { var err error output, err = conn.GetBucketInventoryConfigurationWithContext(ctx, input) diff --git a/internal/service/s3/bucket_logging.go b/internal/service/s3/bucket_logging.go index 9192b9731d40..8e6532cd4735 100644 --- a/internal/service/s3/bucket_logging.go +++ b/internal/service/s3/bucket_logging.go @@ -135,7 +135,7 @@ func resourceBucketLoggingCreate(ctx context.Context, d *schema.ResourceData, me d.SetId(CreateResourceID(bucket, expectedBucketOwner)) - _, err = tfresource.RetryWhenNotFound(ctx, propagationTimeout, func() (interface{}, error) { + _, err = tfresource.RetryWhenNotFound(ctx, s3BucketPropagationTimeout, func() (interface{}, error) { return FindBucketLogging(ctx, conn, bucket, expectedBucketOwner) }) diff --git a/internal/service/s3/bucket_metric.go b/internal/service/s3/bucket_metric.go index b2fe2f25b11e..3728aa6672c8 100644 --- a/internal/service/s3/bucket_metric.go +++ b/internal/service/s3/bucket_metric.go @@ -93,7 +93,7 @@ func resourceBucketMetricPut(ctx context.Context, d *schema.ResourceData, meta i } log.Printf("[DEBUG] Putting S3 Bucket Metrics Configuration: %s", input) - err := retry.RetryContext(ctx, propagationTimeout, func() *retry.RetryError { + err := retry.RetryContext(ctx, s3BucketPropagationTimeout, func() *retry.RetryError { _, err := conn.PutBucketMetricsConfigurationWithContext(ctx, input) if tfawserr.ErrCodeEquals(err, s3.ErrCodeNoSuchBucket) { diff --git a/internal/service/s3/bucket_notification.go b/internal/service/s3/bucket_notification.go index 2020a1d77d7a..10c0ad3fcd45 100644 --- a/internal/service/s3/bucket_notification.go +++ b/internal/service/s3/bucket_notification.go @@ -335,7 +335,7 @@ func resourceBucketNotificationPut(ctx context.Context, d *schema.ResourceData, } log.Printf("[DEBUG] S3 bucket: %s, Putting notification: %v", bucket, i) - err := retry.RetryContext(ctx, propagationTimeout, func() *retry.RetryError { + err := retry.RetryContext(ctx, s3BucketPropagationTimeout, func() *retry.RetryError { _, err := conn.PutBucketNotificationConfigurationWithContext(ctx, i) if tfawserr.ErrCodeEquals(err, s3.ErrCodeNoSuchBucket) { diff --git a/internal/service/s3/bucket_public_access_block.go b/internal/service/s3/bucket_public_access_block.go index 1e9a0f746c02..c2eb902b709c 100644 --- a/internal/service/s3/bucket_public_access_block.go +++ b/internal/service/s3/bucket_public_access_block.go @@ -114,7 +114,7 @@ func resourceBucketPublicAccessBlockRead(ctx context.Context, d *schema.Resource // Retry for eventual consistency on creation var output *s3.GetPublicAccessBlockOutput - err := retry.RetryContext(ctx, propagationTimeout, func() *retry.RetryError { + err := retry.RetryContext(ctx, s3BucketPropagationTimeout, func() *retry.RetryError { var err error output, err = conn.GetPublicAccessBlockWithContext(ctx, input) diff --git a/internal/service/s3/bucket_replication_configuration.go b/internal/service/s3/bucket_replication_configuration.go index ff8e6ec6a5ac..9a47ddd6a2be 100644 --- a/internal/service/s3/bucket_replication_configuration.go +++ b/internal/service/s3/bucket_replication_configuration.go @@ -328,7 +328,7 @@ func resourceBucketReplicationConfigurationCreate(ctx context.Context, d *schema input.Token = aws.String(v.(string)) } - err := retry.RetryContext(ctx, propagationTimeout, func() *retry.RetryError { + err := retry.RetryContext(ctx, s3BucketPropagationTimeout, func() *retry.RetryError { _, err := conn.PutBucketReplicationWithContext(ctx, input) if tfawserr.ErrCodeEquals(err, s3.ErrCodeNoSuchBucket) || tfawserr.ErrMessageContains(err, "InvalidRequest", "Versioning must be 'Enabled' on the bucket") { return retry.RetryableError(err) @@ -349,7 +349,7 @@ func resourceBucketReplicationConfigurationCreate(ctx context.Context, d *schema d.SetId(bucket) - _, err = tfresource.RetryWhenNotFound(ctx, propagationTimeout, func() (interface{}, error) { + _, err = tfresource.RetryWhenNotFound(ctx, s3BucketPropagationTimeout, func() (interface{}, error) { return FindBucketReplicationConfigurationByID(ctx, conn, d.Id()) }) @@ -405,7 +405,7 @@ func resourceBucketReplicationConfigurationUpdate(ctx context.Context, d *schema input.Token = aws.String(v.(string)) } - err := retry.RetryContext(ctx, propagationTimeout, func() *retry.RetryError { + err := retry.RetryContext(ctx, s3BucketPropagationTimeout, func() *retry.RetryError { _, err := conn.PutBucketReplicationWithContext(ctx, input) if tfawserr.ErrCodeEquals(err, s3.ErrCodeNoSuchBucket) || tfawserr.ErrMessageContains(err, "InvalidRequest", "Versioning must be 'Enabled' on the bucket") { return retry.RetryableError(err) diff --git a/internal/service/s3/bucket_server_side_encryption_configuration.go b/internal/service/s3/bucket_server_side_encryption_configuration.go index c33030988cb2..4d9fa1020902 100644 --- a/internal/service/s3/bucket_server_side_encryption_configuration.go +++ b/internal/service/s3/bucket_server_side_encryption_configuration.go @@ -92,7 +92,7 @@ func resourceBucketServerSideEncryptionConfigurationCreate(ctx context.Context, input.ExpectedBucketOwner = aws.String(expectedBucketOwner) } - _, err := tfresource.RetryWhenAWSErrCodeEquals(ctx, propagationTimeout, + _, err := tfresource.RetryWhenAWSErrCodeEquals(ctx, s3BucketPropagationTimeout, func() (interface{}, error) { return conn.PutBucketEncryptionWithContext(ctx, input) }, @@ -106,7 +106,7 @@ func resourceBucketServerSideEncryptionConfigurationCreate(ctx context.Context, d.SetId(CreateResourceID(bucket, expectedBucketOwner)) - _, err = tfresource.RetryWhenNotFound(ctx, propagationTimeout, func() (interface{}, error) { + _, err = tfresource.RetryWhenNotFound(ctx, s3BucketPropagationTimeout, func() (interface{}, error) { return FindBucketServerSideEncryptionConfiguration(ctx, conn, bucket, expectedBucketOwner) }) @@ -164,7 +164,7 @@ func resourceBucketServerSideEncryptionConfigurationUpdate(ctx context.Context, input.ExpectedBucketOwner = aws.String(expectedBucketOwner) } - _, err = tfresource.RetryWhenAWSErrCodeEquals(ctx, propagationTimeout, + _, err = tfresource.RetryWhenAWSErrCodeEquals(ctx, s3BucketPropagationTimeout, func() (interface{}, error) { return conn.PutBucketEncryptionWithContext(ctx, input) }, diff --git a/internal/service/s3/wait.go b/internal/service/s3/wait.go index d3cb732456d7..bdc075f93f33 100644 --- a/internal/service/s3/wait.go +++ b/internal/service/s3/wait.go @@ -17,7 +17,10 @@ const ( lifecycleConfigurationExtraRetryDelay = 5 * time.Second lifecycleConfigurationRulesPropagationTimeout = 3 * time.Minute lifecycleConfigurationRulesSteadyTimeout = 2 * time.Minute - propagationTimeout = 1 * time.Minute + + // General timeout for S3 bucket changes to propagate. + // See https://docs.aws.amazon.com/AmazonS3/latest/userguide/Welcome.html#ConsistencyModel. + s3BucketPropagationTimeout = 1 * time.Minute // nosemgrep:ci.s3-in-const-name, ci.s3-in-var-name // LifecycleConfigurationRulesStatusReady occurs when all configured rules reach their desired state (Enabled or Disabled) LifecycleConfigurationRulesStatusReady = "READY" @@ -26,7 +29,7 @@ const ( ) func retryWhenBucketNotFound(ctx context.Context, f func() (interface{}, error)) (interface{}, error) { - return tfresource.RetryWhenAWSErrCodeEquals(ctx, propagationTimeout, f, s3.ErrCodeNoSuchBucket) + return tfresource.RetryWhenAWSErrCodeEquals(ctx, s3BucketPropagationTimeout, f, s3.ErrCodeNoSuchBucket) } func waitForLifecycleConfigurationRulesStatus(ctx context.Context, conn *s3.S3, bucket, expectedBucketOwner string, rules []*s3.LifecycleRule) error {