From 2c9340a3f09cea5a1854ad2820c6beaf4f91f0a0 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Wed, 7 Feb 2024 14:44:19 -0800 Subject: [PATCH 01/24] Removes unneeded `nosemgrep` --- internal/service/s3/bucket.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/service/s3/bucket.go b/internal/service/s3/bucket.go index e81aba21ef92..6c6ee8e834f3 100644 --- a/internal/service/s3/bucket.go +++ b/internal/service/s3/bucket.go @@ -45,7 +45,7 @@ import ( const ( // General timeout for S3 bucket changes to propagate. // See https://docs.aws.amazon.com/AmazonS3/latest/userguide/Welcome.html#ConsistencyModel. - bucketPropagationTimeout = 2 * time.Minute // nosemgrep:ci.s3-in-const-name, ci.s3-in-var-name + bucketPropagationTimeout = 2 * time.Minute ) // @SDKResource("aws_s3_bucket", name="Bucket") From 26322c72bedd9b5c3ffac8dff0a2df02a412a516 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Wed, 7 Feb 2024 17:29:32 -0800 Subject: [PATCH 02/24] Adds acceptance tests for empty tag values for `aws_vpc` and `aws_lb` --- internal/service/ec2/vpc_test.go | 101 ++++++++++++++++++ internal/service/elbv2/load_balancer_test.go | 104 +++++++++++++++++++ 2 files changed, 205 insertions(+) diff --git a/internal/service/ec2/vpc_test.go b/internal/service/ec2/vpc_test.go index c7c26b5dd2a6..4a240455e2f0 100644 --- a/internal/service/ec2/vpc_test.go +++ b/internal/service/ec2/vpc_test.go @@ -574,6 +574,107 @@ func TestAccVPC_DefaultTagsProviderAndResource_moveDuplicateTags(t *testing.T) { }) } +func TestAccVPC_Tags_EmptyTag_OnCreate(t *testing.T) { + ctx := acctest.Context(t) + var vpc awstypes.Vpc + resourceName := "aws_vpc.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, names.EC2), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckVPCDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccVPCConfig_tags1("key1", ""), + Check: resource.ComposeAggregateTestCheckFunc( + acctest.CheckVPCExistsV2(ctx, resourceName, &vpc), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(resourceName, "tags.key1", ""), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + +func TestAccVPC_Tags_EmptyTag_OnUpdate_Add(t *testing.T) { + ctx := acctest.Context(t) + var vpc awstypes.Vpc + resourceName := "aws_vpc.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, names.EC2), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckVPCDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccVPCConfig_tags1("key1", "value1"), + Check: resource.ComposeAggregateTestCheckFunc( + acctest.CheckVPCExistsV2(ctx, resourceName, &vpc), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1"), + ), + }, + { + Config: testAccVPCConfig_tags2("key1", "value1", "key2", ""), + Check: resource.ComposeAggregateTestCheckFunc( + acctest.CheckVPCExistsV2(ctx, resourceName, &vpc), + resource.TestCheckResourceAttr(resourceName, "tags.%", "2"), + resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1"), + resource.TestCheckResourceAttr(resourceName, "tags.key2", ""), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + +func TestAccVPC_Tags_EmptyTag_OnUpdate_Replace(t *testing.T) { + ctx := acctest.Context(t) + var vpc awstypes.Vpc + resourceName := "aws_vpc.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, names.EC2), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckVPCDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccVPCConfig_tags1("key1", "value1"), + Check: resource.ComposeAggregateTestCheckFunc( + acctest.CheckVPCExistsV2(ctx, resourceName, &vpc), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1"), + ), + }, + { + Config: testAccVPCConfig_tags1("key1", ""), + Check: resource.ComposeAggregateTestCheckFunc( + acctest.CheckVPCExistsV2(ctx, resourceName, &vpc), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(resourceName, "tags.key1", ""), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + // TestAccVPC_DynamicResourceTagsMergedWithLocals_ignoreChanges ensures computed "tags_all" // attributes are correctly determined when the provider-level default_tags block // is left unused and resource tags (merged with local.tags) are only known at apply time, diff --git a/internal/service/elbv2/load_balancer_test.go b/internal/service/elbv2/load_balancer_test.go index f139a5764c9c..f177dafb2d73 100644 --- a/internal/service/elbv2/load_balancer_test.go +++ b/internal/service/elbv2/load_balancer_test.go @@ -372,6 +372,110 @@ func TestAccELBV2LoadBalancer_tags(t *testing.T) { }) } +func TestAccELBV2LoadBalancer_Tags_EmptyTag_OnCreate(t *testing.T) { + ctx := acctest.Context(t) + var conf elbv2.LoadBalancer + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_lb.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, names.S3EndpointID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckLoadBalancerDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccLoadBalancerConfig_tags1(rName, "key1", ""), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckLoadBalancerExists(ctx, resourceName, &conf), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(resourceName, "tags.key1", ""), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + +func TestAccELBV2LoadBalancer_Tags_EmptyTag_OnUpdate_Add(t *testing.T) { + ctx := acctest.Context(t) + var conf elbv2.LoadBalancer + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_lb.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, names.S3EndpointID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckLoadBalancerDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccLoadBalancerConfig_tags1(rName, "key1", "value1"), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckLoadBalancerExists(ctx, resourceName, &conf), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1"), + ), + }, + { + Config: testAccLoadBalancerConfig_tags2(rName, "key1", "value1", "key2", ""), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckLoadBalancerExists(ctx, resourceName, &conf), + resource.TestCheckResourceAttr(resourceName, "tags.%", "2"), + resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1"), + resource.TestCheckResourceAttr(resourceName, "tags.key2", ""), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + +func TestAccELBV2LoadBalancer_Tags_EmptyTag_OnUpdate_Replace(t *testing.T) { + ctx := acctest.Context(t) + var conf elbv2.LoadBalancer + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_lb.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, names.S3EndpointID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckLoadBalancerDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccLoadBalancerConfig_tags1(rName, "key1", "value1"), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckLoadBalancerExists(ctx, resourceName, &conf), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1"), + ), + }, + { + Config: testAccLoadBalancerConfig_tags1(rName, "key1", ""), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckLoadBalancerExists(ctx, resourceName, &conf), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(resourceName, "tags.key1", ""), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + func TestAccELBV2LoadBalancer_ipv6SubnetMapping(t *testing.T) { ctx := acctest.Context(t) var conf elbv2.LoadBalancer From 6705558395f722e6179b3e2944ec0f8267fa60db Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Wed, 7 Feb 2024 17:34:05 -0800 Subject: [PATCH 03/24] Implements transparent tagging for `aws_s3_bucket` --- internal/service/s3/bucket.go | 24 +--- internal/service/s3/bucket_test.go | 129 +++++++++++++++++++++ internal/service/s3/service_package_gen.go | 5 +- internal/service/s3/tags.go | 61 +++++++++- 4 files changed, 197 insertions(+), 22 deletions(-) diff --git a/internal/service/s3/bucket.go b/internal/service/s3/bucket.go index 6c6ee8e834f3..9cded02e983d 100644 --- a/internal/service/s3/bucket.go +++ b/internal/service/s3/bucket.go @@ -49,7 +49,7 @@ const ( ) // @SDKResource("aws_s3_bucket", name="Bucket") -// @Tags +// @Tags(identifierAttribute="bucket", resourceType="Bucket") func resourceBucket() *schema.Resource { return &schema.Resource{ CreateWithoutTimeout: resourceBucketCreate, @@ -774,6 +774,11 @@ func resourceBucketCreate(ctx context.Context, d *schema.ResourceData, meta inte return sdkdiag.AppendErrorf(diags, "waiting for S3 Bucket (%s) create: %s", d.Id(), err) } + tagsIn := getTagsIn(ctx) + if len(tagsIn) != 0 { + bucketCreateTags(ctx, conn, d.Id(), tagsIn) + } + return append(diags, resourceBucketUpdate(ctx, d, meta)...) } @@ -1557,23 +1562,6 @@ func resourceBucketUpdate(ctx context.Context, d *schema.ResourceData, meta inte } } - // - // Bucket Tags. - // - if d.HasChange("tags_all") { - o, n := d.GetChange("tags_all") - - // Retry due to S3 eventual consistency. - _, err := tfresource.RetryWhenAWSErrCodeEquals(ctx, d.Timeout(schema.TimeoutUpdate), func() (interface{}, error) { - terr := bucketUpdateTags(ctx, conn, d.Id(), o, n) - return nil, terr - }, errCodeNoSuchBucket) - - if err != nil { - return sdkdiag.AppendErrorf(diags, "updating S3 Bucket (%s) tags: %s", d.Id(), err) - } - } - return append(diags, resourceBucketRead(ctx, d, meta)...) } diff --git a/internal/service/s3/bucket_test.go b/internal/service/s3/bucket_test.go index e6cf0886e069..7797f76bbde2 100644 --- a/internal/service/s3/bucket_test.go +++ b/internal/service/s3/bucket_test.go @@ -572,6 +572,110 @@ func TestAccS3Bucket_Tags_withNoSystemTags(t *testing.T) { }) } +func TestAccS3Bucket_Tags_EmptyTag_OnCreate(t *testing.T) { + ctx := acctest.Context(t) + bucketName := sdkacctest.RandomWithPrefix("tf-test-bucket") + resourceName := "aws_s3_bucket.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, names.S3EndpointID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckBucketDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccBucketConfig_tags1(bucketName, "key1", ""), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckBucketExists(ctx, resourceName), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(resourceName, "tags.key1", ""), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"force_destroy"}, + }, + }, + }) +} + +func TestAccS3Bucket_Tags_EmptyTag_OnUpdate_Add(t *testing.T) { + ctx := acctest.Context(t) + bucketName := sdkacctest.RandomWithPrefix("tf-test-bucket") + resourceName := "aws_s3_bucket.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, names.S3EndpointID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckBucketDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccBucketConfig_tags1(bucketName, "key1", "value1"), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckBucketExists(ctx, resourceName), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1"), + ), + }, + { + Config: testAccBucketConfig_tags2(bucketName, "key1", "value1", "key2", ""), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckBucketExists(ctx, resourceName), + resource.TestCheckResourceAttr(resourceName, "tags.%", "2"), + resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1"), + resource.TestCheckResourceAttr(resourceName, "tags.key2", ""), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"force_destroy"}, + }, + }, + }) +} + +func TestAccS3Bucket_Tags_EmptyTag_OnUpdate_Replace(t *testing.T) { + ctx := acctest.Context(t) + bucketName := sdkacctest.RandomWithPrefix("tf-test-bucket") + resourceName := "aws_s3_bucket.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, names.S3EndpointID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckBucketDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccBucketConfig_tags1(bucketName, "key1", "value1"), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckBucketExists(ctx, resourceName), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1"), + ), + }, + { + Config: testAccBucketConfig_tags1(bucketName, "key1", ""), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckBucketExists(ctx, resourceName), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(resourceName, "tags.key1", ""), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"force_destroy"}, + }, + }, + }) +} + func TestAccS3Bucket_Tags_withSystemTags(t *testing.T) { ctx := acctest.Context(t) resourceName := "aws_s3_bucket.test" @@ -4254,6 +4358,31 @@ resource "aws_s3_bucket" "bucket6" { `, randInt) } +func testAccBucketConfig_tags1(bucketName, tagKey1, tagValue1 string) string { + return fmt.Sprintf(` +resource "aws_s3_bucket" "test" { + bucket = %[1]q + + tags = { + %[2]q = %[3]q + } +} +`, bucketName, tagKey1, tagValue1) +} + +func testAccBucketConfig_tags2(bucketName, tagKey1, tagValue1, tagKey2, tagValue2 string) string { + return fmt.Sprintf(` +resource "aws_s3_bucket" "test" { + bucket = %[1]q + + tags = { + %[2]q = %[3]q + %[4]q = %[5]q + } +} +`, bucketName, tagKey1, tagValue1, tagKey2, tagValue2) +} + func testAccBucketConfig_objectLockEnabledNoDefaultRetention(bucketName string) string { return fmt.Sprintf(` resource "aws_s3_bucket" "test" { diff --git a/internal/service/s3/service_package_gen.go b/internal/service/s3/service_package_gen.go index 0f91cd5e3645..935a4d2c157b 100644 --- a/internal/service/s3/service_package_gen.go +++ b/internal/service/s3/service_package_gen.go @@ -76,7 +76,10 @@ func (p *servicePackage) SDKResources(ctx context.Context) []*types.ServicePacka Factory: resourceBucket, TypeName: "aws_s3_bucket", Name: "Bucket", - Tags: &types.ServicePackageResourceTags{}, + Tags: &types.ServicePackageResourceTags{ + IdentifierAttribute: "bucket", + ResourceType: "Bucket", + }, }, { Factory: resourceBucketAccelerateConfiguration, diff --git a/internal/service/s3/tags.go b/internal/service/s3/tags.go index 259973901670..73c22b05b7ea 100644 --- a/internal/service/s3/tags.go +++ b/internal/service/s3/tags.go @@ -12,13 +12,24 @@ 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" + awstypes "github.com/aws/aws-sdk-go-v2/service/s3/types" "github.com/hashicorp/aws-sdk-go-base/v2/tfawserr" + "github.com/hashicorp/terraform-plugin-log/tflog" + "github.com/hashicorp/terraform-provider-aws/internal/conns" tftags "github.com/hashicorp/terraform-provider-aws/internal/tags" + "github.com/hashicorp/terraform-provider-aws/internal/types/option" ) // Custom S3 tag service update functions using the same format as generated code. +func bucketCreateTags(ctx context.Context, conn *s3.Client, identifier string, tags []awstypes.Tag) error { + if len(tags) == 0 { + return nil + } + + return bucketUpdateTags(ctx, conn, identifier, nil, keyValueTags(ctx, tags)) +} + // bucketListTags lists S3 bucket tags. // The identifier is the bucket name. func bucketListTags(ctx context.Context, conn *s3.Client, identifier string, optFns ...func(*s3.Options)) (tftags.KeyValueTags, error) { @@ -60,7 +71,7 @@ func bucketUpdateTags(ctx context.Context, conn *s3.Client, identifier string, o if len(newTags)+len(ignoredTags) > 0 { input := &s3.PutBucketTaggingInput{ Bucket: aws.String(identifier), - Tagging: &types.Tagging{ + Tagging: &awstypes.Tagging{ TagSet: Tags(newTags.Merge(ignoredTags)), }, } @@ -123,7 +134,7 @@ func objectUpdateTags(ctx context.Context, conn *s3.Client, bucket, key string, input := &s3.PutObjectTaggingInput{ Bucket: aws.String(bucket), Key: aws.String(key), - Tagging: &types.Tagging{ + Tagging: &awstypes.Tagging{ TagSet: Tags(newTags.Merge(ignoredTags)), }, } @@ -148,3 +159,47 @@ func objectUpdateTags(ctx context.Context, conn *s3.Client, bucket, key string, return nil } + +// ListTags lists s3 service tags and set them in Context. +// It is called from outside this package. +func (p *servicePackage) ListTags(ctx context.Context, meta any, identifier, resourceType string) error { + var ( + tags tftags.KeyValueTags + err error + ) + switch resourceType { + case "Bucket": + tags, err = bucketListTags(ctx, meta.(*conns.AWSClient).S3Client(ctx), identifier) + + default: + tflog.Warn(ctx, "ListTags not implemented for resource type", map[string]any{ + "service_package": p.ServicePackageName(), + "resource_name": resourceType, + }) + return nil + } + + if err != nil { + return err + } + + if inContext, ok := tftags.FromContext(ctx); ok { + inContext.TagsOut = option.Some(tags) + } + + return nil +} + +// UpdateTags updates s3 service tags. +// It is called from outside this package. +func (p *servicePackage) UpdateTags(ctx context.Context, meta any, identifier, resourceType string, oldTags, newTags any) error { + switch resourceType { + case "Bucket": + return bucketUpdateTags(ctx, meta.(*conns.AWSClient).S3Client(ctx), identifier, oldTags, newTags) + } + tflog.Warn(ctx, "UpdateTags not implemented for resource type", map[string]any{ + "service_package": p.ServicePackageName(), + "resource_name": resourceType, + }) + return nil +} From 85a6129e731c98448a60673f2ec18deb6c81c782 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Wed, 7 Feb 2024 17:49:16 -0800 Subject: [PATCH 04/24] Simplifies tag on create --- internal/service/s3/bucket.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/internal/service/s3/bucket.go b/internal/service/s3/bucket.go index 9cded02e983d..0b0544d06a88 100644 --- a/internal/service/s3/bucket.go +++ b/internal/service/s3/bucket.go @@ -774,10 +774,7 @@ func resourceBucketCreate(ctx context.Context, d *schema.ResourceData, meta inte return sdkdiag.AppendErrorf(diags, "waiting for S3 Bucket (%s) create: %s", d.Id(), err) } - tagsIn := getTagsIn(ctx) - if len(tagsIn) != 0 { - bucketCreateTags(ctx, conn, d.Id(), tagsIn) - } + bucketCreateTags(ctx, conn, d.Id(), getTagsIn(ctx)) return append(diags, resourceBucketUpdate(ctx, d, meta)...) } From 56ad462f5b4e06c808f28fc4b65255496ebf373a Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Thu, 8 Feb 2024 11:19:33 -0800 Subject: [PATCH 05/24] Linting fix --- internal/service/s3/bucket.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/service/s3/bucket.go b/internal/service/s3/bucket.go index 0b0544d06a88..acb38a4e7def 100644 --- a/internal/service/s3/bucket.go +++ b/internal/service/s3/bucket.go @@ -774,7 +774,9 @@ func resourceBucketCreate(ctx context.Context, d *schema.ResourceData, meta inte return sdkdiag.AppendErrorf(diags, "waiting for S3 Bucket (%s) create: %s", d.Id(), err) } - bucketCreateTags(ctx, conn, d.Id(), getTagsIn(ctx)) + if err := bucketCreateTags(ctx, conn, d.Id(), getTagsIn(ctx)); err != nil { + return sdkdiag.AppendErrorf(diags, "setting S3 Bucket (%s) tags: %s", d.Id(), err) + } return append(diags, resourceBucketUpdate(ctx, d, meta)...) } From dd170d78bcc76b73ec381a75e06c74e05a773d38 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Fri, 9 Feb 2024 11:31:11 -0800 Subject: [PATCH 06/24] Adds attribute `arn` to S3 Object and variants --- internal/service/s3/bucket_object.go | 7 + internal/service/s3/bucket_object_test.go | 166 +++++---- internal/service/s3/bucket_test.go | 160 ++++----- internal/service/s3/object.go | 15 + internal/service/s3/object_copy.go | 7 + internal/service/s3/object_copy_test.go | 45 +-- internal/service/s3/object_data_source.go | 7 + .../service/s3/object_data_source_test.go | 29 +- internal/service/s3/object_test.go | 322 +++++++++++++----- website/docs/d/s3_object.html.markdown | 1 + website/docs/r/s3_bucket_object.html.markdown | 2 +- website/docs/r/s3_object.html.markdown | 2 +- website/docs/r/s3_object_copy.html.markdown | 2 +- 13 files changed, 500 insertions(+), 265 deletions(-) diff --git a/internal/service/s3/bucket_object.go b/internal/service/s3/bucket_object.go index d562dbed713a..190d5c767c34 100644 --- a/internal/service/s3/bucket_object.go +++ b/internal/service/s3/bucket_object.go @@ -62,6 +62,10 @@ func resourceBucketObject() *schema.Resource { Optional: true, ValidateDiagFunc: enum.Validate[types.ObjectCannedACL](), }, + "arn": { + Type: schema.TypeString, + Computed: true, + }, "bucket": { Deprecated: "Use the aws_s3_object resource instead", Type: schema.TypeString, @@ -220,6 +224,9 @@ func resourceBucketObjectRead(ctx context.Context, d *schema.ResourceData, meta return sdkdiag.AppendErrorf(diags, "reading S3 Object (%s): %s", d.Id(), err) } + arn := newObjectARN(meta.(*conns.AWSClient).Partition, bucket, key) + d.Set("arn", arn.String()) + d.Set("bucket_key_enabled", output.BucketKeyEnabled) d.Set("cache_control", output.CacheControl) d.Set("content_disposition", output.ContentDisposition) diff --git a/internal/service/s3/bucket_object_test.go b/internal/service/s3/bucket_object_test.go index 8690e29b2dba..495633b740dc 100644 --- a/internal/service/s3/bucket_object_test.go +++ b/internal/service/s3/bucket_object_test.go @@ -42,19 +42,19 @@ func TestAccS3BucketObject_noNameNoKey(t *testing.T) { Steps: []resource.TestStep{ { PreConfig: func() {}, - Config: testAccBucketObjectConfig_basic("", "a key"), + Config: testAccBucketObjectConfig_invalid("", "a key"), ExpectError: bucketError, }, { PreConfig: func() {}, - Config: testAccBucketObjectConfig_basic("a name", ""), + Config: testAccBucketObjectConfig_invalid("a name", ""), ExpectError: keyError, }, }, }) } -func TestAccS3BucketObject_empty(t *testing.T) { +func TestAccS3BucketObject_basic(t *testing.T) { ctx := acctest.Context(t) var obj s3.GetObjectOutput resourceName := "aws_s3_bucket_object.object" @@ -68,10 +68,36 @@ func TestAccS3BucketObject_empty(t *testing.T) { Steps: []resource.TestStep{ { PreConfig: func() {}, - Config: testAccBucketObjectConfig_empty(rName), - Check: resource.ComposeTestCheckFunc( + Config: testAccBucketObjectConfig_basic(rName), + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &obj), testAccCheckObjectBody(&obj, ""), + resource.TestCheckResourceAttr(resourceName, "acl", string(types.BucketCannedACLPrivate)), + resource.TestCheckResourceAttr(resourceName, "arn", fmt.Sprintf("arn:aws:s3:::%s/test-key", rName)), + resource.TestCheckResourceAttr(resourceName, "bucket", rName), + resource.TestCheckResourceAttr(resourceName, "bucket_key_enabled", "false"), + resource.TestCheckResourceAttr(resourceName, "cache_control", ""), + resource.TestCheckNoResourceAttr(resourceName, "content"), + resource.TestCheckNoResourceAttr(resourceName, "content_base64"), + resource.TestCheckResourceAttr(resourceName, "content_disposition", ""), + resource.TestCheckResourceAttr(resourceName, "content_encoding", ""), + resource.TestCheckResourceAttr(resourceName, "content_language", ""), + resource.TestCheckResourceAttr(resourceName, "content_type", "application/octet-stream"), + resource.TestCheckResourceAttrSet(resourceName, "etag"), + resource.TestCheckResourceAttr(resourceName, "force_destroy", "false"), + resource.TestCheckResourceAttr(resourceName, "key", "test-key"), + resource.TestCheckNoResourceAttr(resourceName, "kms_key_id"), + resource.TestCheckResourceAttr(resourceName, "metadata.%", "0"), + 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, "server_side_encryption", "AES256"), + resource.TestCheckNoResourceAttr(resourceName, "source"), + resource.TestCheckNoResourceAttr(resourceName, "source_hash"), + resource.TestCheckResourceAttr(resourceName, "storage_class", "STANDARD"), + resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), + resource.TestCheckResourceAttr(resourceName, "version_id", ""), + resource.TestCheckResourceAttr(resourceName, "website_redirect", ""), ), }, { @@ -102,7 +128,7 @@ func TestAccS3BucketObject_source(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketObjectConfig_source(rName, source), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &obj), testAccCheckObjectBody(&obj, "{anything will do }"), ), @@ -133,7 +159,7 @@ func TestAccS3BucketObject_content(t *testing.T) { { PreConfig: func() {}, Config: testAccBucketObjectConfig_content(rName, "some_bucket_content"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &obj), testAccCheckObjectBody(&obj, "some_bucket_content"), ), @@ -166,7 +192,7 @@ func TestAccS3BucketObject_etagEncryption(t *testing.T) { { PreConfig: func() {}, Config: testAccBucketObjectConfig_etagEncryption(rName, source), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &obj), testAccCheckObjectBody(&obj, "{anything will do }"), resource.TestCheckResourceAttr(resourceName, "etag", "7b006ff4d70f68cc65061acf2f802e6f"), @@ -198,7 +224,7 @@ func TestAccS3BucketObject_contentBase64(t *testing.T) { { PreConfig: func() {}, Config: testAccBucketObjectConfig_contentBase64(rName, base64.StdEncoding.EncodeToString([]byte("some_bucket_content"))), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &obj), testAccCheckObjectBody(&obj, "some_bucket_content"), ), @@ -236,7 +262,7 @@ func TestAccS3BucketObject_sourceHashTrigger(t *testing.T) { { PreConfig: func() {}, Config: testAccBucketObjectConfig_sourceHashTrigger(rName, filename), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &obj), testAccCheckObjectBody(&obj, "Ebben!"), resource.TestCheckResourceAttr(resourceName, "source_hash", "7c7e02a79f28968882bb1426c8f8bfc6"), @@ -247,7 +273,7 @@ func TestAccS3BucketObject_sourceHashTrigger(t *testing.T) { { PreConfig: func() {}, Config: testAccBucketObjectConfig_sourceHashTrigger(rName, filename), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &updated_obj), testAccCheckObjectBody(&updated_obj, "Ne andrò lontana"), resource.TestCheckResourceAttr(resourceName, "source_hash", "cffc5e20de2d21764145b1124c9b337b"), @@ -281,7 +307,7 @@ func TestAccS3BucketObject_withContentCharacteristics(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketObjectConfig_contentCharacteristics(rName, source), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &obj), testAccCheckObjectBody(&obj, "{anything will do }"), resource.TestCheckResourceAttr(resourceName, "content_type", "binary/octet-stream"), @@ -308,7 +334,7 @@ func TestAccS3BucketObject_nonVersioned(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketObjectConfig_nonVersioned(rName, sourceInitial), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &originalObj), testAccCheckObjectBody(&originalObj, "initial object state"), resource.TestCheckResourceAttr(resourceName, "version_id", ""), @@ -344,7 +370,7 @@ func TestAccS3BucketObject_updates(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketObjectConfig_updateable(rName, false, sourceInitial), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &originalObj), testAccCheckObjectBody(&originalObj, "initial object state"), resource.TestCheckResourceAttr(resourceName, "etag", "647d1d58e1011c743ec67d5e8af87b53"), @@ -355,7 +381,7 @@ func TestAccS3BucketObject_updates(t *testing.T) { }, { Config: testAccBucketObjectConfig_updateable(rName, false, sourceModified), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &modifiedObj), testAccCheckObjectBody(&modifiedObj, "modified object"), resource.TestCheckResourceAttr(resourceName, "etag", "1c7fd13df1515c2a13ad9eb068931f09"), @@ -403,7 +429,7 @@ func TestAccS3BucketObject_updateSameFile(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketObjectConfig_updateable(rName, false, filename), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &originalObj), testAccCheckObjectBody(&originalObj, startingData), resource.TestCheckResourceAttr(resourceName, "etag", "aa48b42f36a2652cbee40c30a5df7d25"), @@ -413,7 +439,7 @@ func TestAccS3BucketObject_updateSameFile(t *testing.T) { }, { Config: testAccBucketObjectConfig_updateable(rName, false, filename), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &modifiedObj), testAccCheckObjectBody(&modifiedObj, changingData), resource.TestCheckResourceAttr(resourceName, "etag", "fafc05f8c4da0266a99154681ab86e8c"), @@ -442,7 +468,7 @@ func TestAccS3BucketObject_updatesWithVersioning(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketObjectConfig_updateable(rName, true, sourceInitial), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &originalObj), testAccCheckObjectBody(&originalObj, "initial versioned object state"), resource.TestCheckResourceAttr(resourceName, "etag", "cee4407fa91906284e2a5e5e03e86b1b"), @@ -450,7 +476,7 @@ func TestAccS3BucketObject_updatesWithVersioning(t *testing.T) { }, { Config: testAccBucketObjectConfig_updateable(rName, true, sourceModified), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &modifiedObj), testAccCheckObjectBody(&modifiedObj, "modified versioned object"), resource.TestCheckResourceAttr(resourceName, "etag", "00b8c73b1b50e7cc932362c7225b8e29"), @@ -488,7 +514,7 @@ func TestAccS3BucketObject_updatesWithVersioningViaAccessPoint(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketObjectConfig_updateableViaAccessPoint(rName, string(types.BucketVersioningStatusEnabled), sourceInitial), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &originalObj), testAccCheckObjectBody(&originalObj, "initial versioned object state"), resource.TestCheckResourceAttrPair(resourceName, "bucket", accessPointResourceName, "arn"), @@ -497,7 +523,7 @@ func TestAccS3BucketObject_updatesWithVersioningViaAccessPoint(t *testing.T) { }, { Config: testAccBucketObjectConfig_updateableViaAccessPoint(rName, string(types.BucketVersioningStatusEnabled), sourceModified), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &modifiedObj), testAccCheckObjectBody(&modifiedObj, "modified versioned object"), resource.TestCheckResourceAttr(resourceName, "etag", "00b8c73b1b50e7cc932362c7225b8e29"), @@ -526,7 +552,7 @@ func TestAccS3BucketObject_kms(t *testing.T) { { PreConfig: func() {}, Config: testAccBucketObjectConfig_kmsID(rName, source), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &obj), testAccCheckObjectSSE(ctx, resourceName, "aws:kms"), testAccCheckObjectBody(&obj, "{anything will do }"), @@ -561,7 +587,7 @@ func TestAccS3BucketObject_sse(t *testing.T) { { PreConfig: func() {}, Config: testAccBucketObjectConfig_sse(rName, source), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &obj), testAccCheckObjectSSE(ctx, resourceName, "AES256"), testAccCheckObjectBody(&obj, "{anything will do }"), @@ -592,7 +618,7 @@ func TestAccS3BucketObject_acl(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketObjectConfig_acl(rName, "some_bucket_content", string(types.BucketCannedACLPrivate), true), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &obj1), testAccCheckObjectBody(&obj1, "some_bucket_content"), resource.TestCheckResourceAttr(resourceName, "acl", string(types.BucketCannedACLPrivate)), @@ -601,7 +627,7 @@ func TestAccS3BucketObject_acl(t *testing.T) { }, { Config: testAccBucketObjectConfig_acl(rName, "some_bucket_content", string(types.BucketCannedACLPublicRead), false), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &obj2), testAccCheckObjectVersionIDEquals(&obj2, &obj1), testAccCheckObjectBody(&obj2, "some_bucket_content"), @@ -611,7 +637,7 @@ func TestAccS3BucketObject_acl(t *testing.T) { }, { Config: testAccBucketObjectConfig_acl(rName, "changed_some_bucket_content", string(types.BucketCannedACLPrivate), true), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &obj3), testAccCheckObjectVersionIDDiffers(&obj3, &obj2), testAccCheckObjectBody(&obj3, "changed_some_bucket_content"), @@ -644,7 +670,7 @@ func TestAccS3BucketObject_metadata(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketObjectConfig_metadata(rName, "key1", "value1", "key2", "value2"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &obj), resource.TestCheckResourceAttr(resourceName, "metadata.%", "2"), resource.TestCheckResourceAttr(resourceName, "metadata.key1", "value1"), @@ -653,7 +679,7 @@ func TestAccS3BucketObject_metadata(t *testing.T) { }, { Config: testAccBucketObjectConfig_metadata(rName, "key1", "value1updated", "key3", "value3"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &obj), resource.TestCheckResourceAttr(resourceName, "metadata.%", "2"), resource.TestCheckResourceAttr(resourceName, "metadata.key1", "value1updated"), @@ -661,8 +687,8 @@ func TestAccS3BucketObject_metadata(t *testing.T) { ), }, { - Config: testAccBucketObjectConfig_empty(rName), - Check: resource.ComposeTestCheckFunc( + Config: testAccBucketObjectConfig_basic(rName), + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &obj), resource.TestCheckResourceAttr(resourceName, "metadata.%", "0"), ), @@ -693,7 +719,7 @@ func TestAccS3BucketObject_storageClass(t *testing.T) { { PreConfig: func() {}, Config: testAccBucketObjectConfig_content(rName, "some_bucket_content"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &obj), resource.TestCheckResourceAttr(resourceName, "storage_class", "STANDARD"), testAccCheckObjectStorageClass(ctx, resourceName, "STANDARD"), @@ -701,7 +727,7 @@ func TestAccS3BucketObject_storageClass(t *testing.T) { }, { Config: testAccBucketObjectConfig_storageClass(rName, "REDUCED_REDUNDANCY"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &obj), resource.TestCheckResourceAttr(resourceName, "storage_class", "REDUCED_REDUNDANCY"), testAccCheckObjectStorageClass(ctx, resourceName, "REDUCED_REDUNDANCY"), @@ -709,7 +735,7 @@ func TestAccS3BucketObject_storageClass(t *testing.T) { }, { Config: testAccBucketObjectConfig_storageClass(rName, "GLACIER"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( // Can't GetObject on an object in Glacier without restoring it. resource.TestCheckResourceAttr(resourceName, "storage_class", "GLACIER"), testAccCheckObjectStorageClass(ctx, resourceName, "GLACIER"), @@ -717,7 +743,7 @@ func TestAccS3BucketObject_storageClass(t *testing.T) { }, { Config: testAccBucketObjectConfig_storageClass(rName, "INTELLIGENT_TIERING"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &obj), resource.TestCheckResourceAttr(resourceName, "storage_class", "INTELLIGENT_TIERING"), testAccCheckObjectStorageClass(ctx, resourceName, "INTELLIGENT_TIERING"), @@ -725,7 +751,7 @@ func TestAccS3BucketObject_storageClass(t *testing.T) { }, { Config: testAccBucketObjectConfig_storageClass(rName, "DEEP_ARCHIVE"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( // Can't GetObject on an object in DEEP_ARCHIVE without restoring it. resource.TestCheckResourceAttr(resourceName, "storage_class", "DEEP_ARCHIVE"), testAccCheckObjectStorageClass(ctx, resourceName, "DEEP_ARCHIVE"), @@ -758,7 +784,7 @@ func TestAccS3BucketObject_tags(t *testing.T) { { PreConfig: func() {}, Config: testAccBucketObjectConfig_tags(rName, key, "stuff"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &obj1), testAccCheckObjectBody(&obj1, "stuff"), resource.TestCheckResourceAttr(resourceName, "tags.%", "3"), @@ -770,7 +796,7 @@ func TestAccS3BucketObject_tags(t *testing.T) { { PreConfig: func() {}, Config: testAccBucketObjectConfig_updatedTags(rName, key, "stuff"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &obj2), testAccCheckObjectVersionIDEquals(&obj2, &obj1), testAccCheckObjectBody(&obj2, "stuff"), @@ -784,7 +810,7 @@ func TestAccS3BucketObject_tags(t *testing.T) { { PreConfig: func() {}, Config: testAccBucketObjectConfig_noTags(rName, key, "stuff"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &obj3), testAccCheckObjectVersionIDEquals(&obj3, &obj2), testAccCheckObjectBody(&obj3, "stuff"), @@ -794,7 +820,7 @@ func TestAccS3BucketObject_tags(t *testing.T) { { PreConfig: func() {}, Config: testAccBucketObjectConfig_tags(rName, key, "changed stuff"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &obj4), testAccCheckObjectVersionIDDiffers(&obj4, &obj3), testAccCheckObjectBody(&obj4, "changed stuff"), @@ -831,7 +857,7 @@ func TestAccS3BucketObject_tagsLeadingSingleSlash(t *testing.T) { { PreConfig: func() {}, Config: testAccBucketObjectConfig_tags(rName, key, "stuff"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &obj1), testAccCheckObjectBody(&obj1, "stuff"), resource.TestCheckResourceAttr(resourceName, "tags.%", "3"), @@ -843,7 +869,7 @@ func TestAccS3BucketObject_tagsLeadingSingleSlash(t *testing.T) { { PreConfig: func() {}, Config: testAccBucketObjectConfig_updatedTags(rName, key, "stuff"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &obj2), testAccCheckObjectVersionIDEquals(&obj2, &obj1), testAccCheckObjectBody(&obj2, "stuff"), @@ -857,7 +883,7 @@ func TestAccS3BucketObject_tagsLeadingSingleSlash(t *testing.T) { { PreConfig: func() {}, Config: testAccBucketObjectConfig_noTags(rName, key, "stuff"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &obj3), testAccCheckObjectVersionIDEquals(&obj3, &obj2), testAccCheckObjectBody(&obj3, "stuff"), @@ -867,7 +893,7 @@ func TestAccS3BucketObject_tagsLeadingSingleSlash(t *testing.T) { { PreConfig: func() {}, Config: testAccBucketObjectConfig_tags(rName, key, "changed stuff"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &obj4), testAccCheckObjectVersionIDDiffers(&obj4, &obj3), testAccCheckObjectBody(&obj4, "changed stuff"), @@ -904,7 +930,7 @@ func TestAccS3BucketObject_tagsLeadingMultipleSlashes(t *testing.T) { { PreConfig: func() {}, Config: testAccBucketObjectConfig_tags(rName, key, "stuff"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &obj1), testAccCheckObjectBody(&obj1, "stuff"), resource.TestCheckResourceAttr(resourceName, "tags.%", "3"), @@ -916,7 +942,7 @@ func TestAccS3BucketObject_tagsLeadingMultipleSlashes(t *testing.T) { { PreConfig: func() {}, Config: testAccBucketObjectConfig_updatedTags(rName, key, "stuff"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &obj2), testAccCheckObjectVersionIDEquals(&obj2, &obj1), testAccCheckObjectBody(&obj2, "stuff"), @@ -930,7 +956,7 @@ func TestAccS3BucketObject_tagsLeadingMultipleSlashes(t *testing.T) { { PreConfig: func() {}, Config: testAccBucketObjectConfig_noTags(rName, key, "stuff"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &obj3), testAccCheckObjectVersionIDEquals(&obj3, &obj2), testAccCheckObjectBody(&obj3, "stuff"), @@ -940,7 +966,7 @@ func TestAccS3BucketObject_tagsLeadingMultipleSlashes(t *testing.T) { { PreConfig: func() {}, Config: testAccBucketObjectConfig_tags(rName, key, "changed stuff"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &obj4), testAccCheckObjectVersionIDDiffers(&obj4, &obj3), testAccCheckObjectBody(&obj4, "changed stuff"), @@ -970,7 +996,7 @@ func TestAccS3BucketObject_tagsMultipleSlashes(t *testing.T) { { PreConfig: func() {}, Config: testAccBucketObjectConfig_tags(rName, key, "stuff"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &obj1), testAccCheckObjectBody(&obj1, "stuff"), resource.TestCheckResourceAttr(resourceName, "tags.%", "3"), @@ -982,7 +1008,7 @@ func TestAccS3BucketObject_tagsMultipleSlashes(t *testing.T) { { PreConfig: func() {}, Config: testAccBucketObjectConfig_updatedTags(rName, key, "stuff"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &obj2), testAccCheckObjectVersionIDEquals(&obj2, &obj1), testAccCheckObjectBody(&obj2, "stuff"), @@ -996,7 +1022,7 @@ func TestAccS3BucketObject_tagsMultipleSlashes(t *testing.T) { { PreConfig: func() {}, Config: testAccBucketObjectConfig_noTags(rName, key, "stuff"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &obj3), testAccCheckObjectVersionIDEquals(&obj3, &obj2), testAccCheckObjectBody(&obj3, "stuff"), @@ -1006,7 +1032,7 @@ func TestAccS3BucketObject_tagsMultipleSlashes(t *testing.T) { { PreConfig: func() {}, Config: testAccBucketObjectConfig_tags(rName, key, "changed stuff"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &obj4), testAccCheckObjectVersionIDDiffers(&obj4, &obj3), testAccCheckObjectBody(&obj4, "changed stuff"), @@ -1034,7 +1060,7 @@ func TestAccS3BucketObject_objectLockLegalHoldStartWithNone(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketObjectConfig_noLockLegalHold(rName, "stuff"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &obj1), testAccCheckObjectBody(&obj1, "stuff"), resource.TestCheckResourceAttr(resourceName, "object_lock_legal_hold_status", ""), @@ -1044,7 +1070,7 @@ func TestAccS3BucketObject_objectLockLegalHoldStartWithNone(t *testing.T) { }, { Config: testAccBucketObjectConfig_lockLegalHold(rName, "stuff", "ON"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &obj2), testAccCheckObjectVersionIDEquals(&obj2, &obj1), testAccCheckObjectBody(&obj2, "stuff"), @@ -1056,7 +1082,7 @@ func TestAccS3BucketObject_objectLockLegalHoldStartWithNone(t *testing.T) { // Remove legal hold but create a new object version to test force_destroy { Config: testAccBucketObjectConfig_lockLegalHold(rName, "changed stuff", "OFF"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &obj3), testAccCheckObjectVersionIDDiffers(&obj3, &obj2), testAccCheckObjectBody(&obj3, "changed stuff"), @@ -1083,7 +1109,7 @@ func TestAccS3BucketObject_objectLockLegalHoldStartWithOn(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketObjectConfig_lockLegalHold(rName, "stuff", "ON"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &obj1), testAccCheckObjectBody(&obj1, "stuff"), resource.TestCheckResourceAttr(resourceName, "object_lock_legal_hold_status", "ON"), @@ -1093,7 +1119,7 @@ func TestAccS3BucketObject_objectLockLegalHoldStartWithOn(t *testing.T) { }, { Config: testAccBucketObjectConfig_lockLegalHold(rName, "stuff", "OFF"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &obj2), testAccCheckObjectVersionIDEquals(&obj2, &obj1), testAccCheckObjectBody(&obj2, "stuff"), @@ -1121,7 +1147,7 @@ func TestAccS3BucketObject_objectLockRetentionStartWithNone(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketObjectConfig_noLockRetention(rName, "stuff"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &obj1), testAccCheckObjectBody(&obj1, "stuff"), resource.TestCheckResourceAttr(resourceName, "object_lock_legal_hold_status", ""), @@ -1131,7 +1157,7 @@ func TestAccS3BucketObject_objectLockRetentionStartWithNone(t *testing.T) { }, { Config: testAccBucketObjectConfig_lockRetention(rName, "stuff", retainUntilDate), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &obj2), testAccCheckObjectVersionIDEquals(&obj2, &obj1), testAccCheckObjectBody(&obj2, "stuff"), @@ -1143,7 +1169,7 @@ func TestAccS3BucketObject_objectLockRetentionStartWithNone(t *testing.T) { // Remove retention period but create a new object version to test force_destroy { Config: testAccBucketObjectConfig_noLockRetention(rName, "changed stuff"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &obj3), testAccCheckObjectVersionIDDiffers(&obj3, &obj2), testAccCheckObjectBody(&obj3, "changed stuff"), @@ -1173,7 +1199,7 @@ func TestAccS3BucketObject_objectLockRetentionStartWithSet(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketObjectConfig_lockRetention(rName, "stuff", retainUntilDate1), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &obj1), testAccCheckObjectBody(&obj1, "stuff"), resource.TestCheckResourceAttr(resourceName, "object_lock_legal_hold_status", ""), @@ -1183,7 +1209,7 @@ func TestAccS3BucketObject_objectLockRetentionStartWithSet(t *testing.T) { }, { Config: testAccBucketObjectConfig_lockRetention(rName, "stuff", retainUntilDate2), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &obj2), testAccCheckObjectVersionIDEquals(&obj2, &obj1), testAccCheckObjectBody(&obj2, "stuff"), @@ -1194,7 +1220,7 @@ func TestAccS3BucketObject_objectLockRetentionStartWithSet(t *testing.T) { }, { Config: testAccBucketObjectConfig_lockRetention(rName, "stuff", retainUntilDate3), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &obj3), testAccCheckObjectVersionIDEquals(&obj3, &obj2), testAccCheckObjectBody(&obj3, "stuff"), @@ -1205,7 +1231,7 @@ func TestAccS3BucketObject_objectLockRetentionStartWithSet(t *testing.T) { }, { Config: testAccBucketObjectConfig_noLockRetention(rName, "stuff"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &obj4), testAccCheckObjectVersionIDEquals(&obj4, &obj3), testAccCheckObjectBody(&obj4, "stuff"), @@ -1232,7 +1258,7 @@ func TestAccS3BucketObject_objectBucketKeyEnabled(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketObjectConfig_objectKeyEnabled(rName, "stuff"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &obj), testAccCheckObjectBody(&obj, "stuff"), resource.TestCheckResourceAttr(resourceName, "bucket_key_enabled", "true"), @@ -1256,7 +1282,7 @@ func TestAccS3BucketObject_bucketBucketKeyEnabled(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketObjectConfig_bucketKeyEnabled(rName, "stuff"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &obj), testAccCheckObjectBody(&obj, "stuff"), resource.TestCheckResourceAttr(resourceName, "bucket_key_enabled", "true"), @@ -1280,7 +1306,7 @@ func TestAccS3BucketObject_defaultBucketSSE(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketObjectConfig_defaultSSE(rName, "stuff"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &obj1), testAccCheckObjectBody(&obj1, "stuff"), ), @@ -1307,7 +1333,7 @@ func TestAccS3BucketObject_ignoreTags(t *testing.T) { Config: acctest.ConfigCompose( acctest.ConfigIgnoreTagsKeyPrefixes1("ignorekey"), testAccBucketObjectConfig_noTags(rName, key, "stuff")), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &obj), testAccCheckObjectBody(&obj, "stuff"), testAccCheckObjectUpdateTags(ctx, resourceName, nil, map[string]string{"ignorekey1": "ignorevalue1"}), @@ -1322,7 +1348,7 @@ func TestAccS3BucketObject_ignoreTags(t *testing.T) { Config: acctest.ConfigCompose( acctest.ConfigIgnoreTagsKeyPrefixes1("ignorekey"), testAccBucketObjectConfig_tags(rName, key, "stuff")), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketObjectExists(ctx, resourceName, &obj), testAccCheckObjectBody(&obj, "stuff"), resource.TestCheckResourceAttr(resourceName, "tags.%", "3"), @@ -1394,7 +1420,7 @@ func testAccCheckBucketObjectExists(ctx context.Context, n string, v *s3.GetObje } } -func testAccBucketObjectConfig_basic(bucket, key string) string { +func testAccBucketObjectConfig_invalid(bucket, key string) string { return fmt.Sprintf(` resource "aws_s3_bucket_object" "object" { bucket = %[1]q @@ -1403,7 +1429,7 @@ resource "aws_s3_bucket_object" "object" { `, bucket, key) } -func testAccBucketObjectConfig_empty(rName string) string { +func testAccBucketObjectConfig_basic(rName string) string { return fmt.Sprintf(` resource "aws_s3_bucket" "test" { bucket = %[1]q diff --git a/internal/service/s3/bucket_test.go b/internal/service/s3/bucket_test.go index 7797f76bbde2..0e1c9bbd6eb5 100644 --- a/internal/service/s3/bucket_test.go +++ b/internal/service/s3/bucket_test.go @@ -127,7 +127,7 @@ func TestAccS3Bucket_Basic_emptyString(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketConfig_emptyString, - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), acctest.CheckResourceAttrNameGenerated(resourceName, "bucket"), resource.TestCheckResourceAttr(resourceName, "bucket_prefix", id.UniqueIdPrefix), @@ -155,7 +155,7 @@ func TestAccS3Bucket_Basic_nameGenerated(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketConfig_nameGenerated, - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), acctest.CheckResourceAttrNameGenerated(resourceName, "bucket"), resource.TestCheckResourceAttr(resourceName, "bucket_prefix", id.UniqueIdPrefix), @@ -183,7 +183,7 @@ func TestAccS3Bucket_Basic_namePrefix(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketConfig_namePrefix("tf-test-"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), acctest.CheckResourceAttrNameFromPrefix(resourceName, "bucket", "tf-test-"), resource.TestCheckResourceAttr(resourceName, "bucket_prefix", "tf-test-"), @@ -212,7 +212,7 @@ func TestAccS3Bucket_Basic_forceDestroy(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketConfig_forceDestroy(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), testAccCheckBucketAddObjects(ctx, resourceName, "data.txt", "prefix/more_data.txt"), ), @@ -234,7 +234,7 @@ func TestAccS3Bucket_Basic_forceDestroyWithObjectVersions(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketConfig_forceDestroyObjectVersions(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), testAccCheckBucketAddObjects(ctx, resourceName, "data.txt", "prefix/more_data.txt"), testAccCheckBucketDeleteObjects(ctx, resourceName, "data.txt"), // Creates a delete marker. @@ -263,7 +263,7 @@ func TestAccS3Bucket_Basic_forceDestroyWithEmptyPrefixes(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketConfig_forceDestroy(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), testAccCheckBucketAddObjects(ctx, resourceName, "data.txt", "/extraleadingslash.txt"), ), @@ -285,7 +285,7 @@ func TestAccS3Bucket_Basic_forceDestroyWithObjectLockEnabled(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketConfig_forceDestroyObjectLockEnabled(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), testAccCheckBucketAddObjectsWithLegalHold(ctx, resourceName, "data.txt", "prefix/more_data.txt"), ), @@ -310,7 +310,7 @@ func TestAccS3Bucket_Basic_acceleration(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketConfig_acceleration(bucketName, string(types.BucketAccelerateStatusEnabled)), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "acceleration_status", string(types.BucketAccelerateStatusEnabled)), ), @@ -323,7 +323,7 @@ func TestAccS3Bucket_Basic_acceleration(t *testing.T) { }, { Config: testAccBucketConfig_acceleration(bucketName, string(types.BucketAccelerateStatusSuspended)), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "acceleration_status", string(types.BucketAccelerateStatusSuspended)), ), @@ -345,7 +345,7 @@ func TestAccS3Bucket_Basic_keyEnabled(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketConfig_defaultEncryptionKeyEnabledKMSMasterKey(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "server_side_encryption_configuration.#", "1"), resource.TestCheckResourceAttr(resourceName, "server_side_encryption_configuration.0.rule.#", "1"), @@ -378,7 +378,7 @@ func TestAccS3Bucket_Basic_requestPayer(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketConfig_requestPayer(bucketName, string(types.PayerBucketOwner)), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "request_payer", string(types.PayerBucketOwner)), ), @@ -391,7 +391,7 @@ func TestAccS3Bucket_Basic_requestPayer(t *testing.T) { }, { Config: testAccBucketConfig_requestPayer(bucketName, string(types.PayerRequester)), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "request_payer", string(types.PayerRequester)), ), @@ -416,7 +416,7 @@ func TestAccS3Bucket_disappears(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketConfig_basic(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), acctest.CheckResourceDisappears(ctx, acctest.Provider, tfs3.ResourceBucket(), resourceName), ), @@ -525,7 +525,7 @@ func TestAccS3Bucket_Tags_withNoSystemTags(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketConfig_tags(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "tags.%", "3"), resource.TestCheckResourceAttr(resourceName, "tags.Key1", "AAA"), @@ -541,7 +541,7 @@ func TestAccS3Bucket_Tags_withNoSystemTags(t *testing.T) { }, { Config: testAccBucketConfig_updatedTags(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "tags.%", "4"), resource.TestCheckResourceAttr(resourceName, "tags.Key2", "BBB"), @@ -552,7 +552,7 @@ func TestAccS3Bucket_Tags_withNoSystemTags(t *testing.T) { }, { Config: testAccBucketConfig_noTags(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), ), @@ -560,7 +560,7 @@ func TestAccS3Bucket_Tags_withNoSystemTags(t *testing.T) { // Verify update from 0 tags. { Config: testAccBucketConfig_tags(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "tags.%", "3"), resource.TestCheckResourceAttr(resourceName, "tags.Key1", "AAA"), @@ -714,7 +714,7 @@ func TestAccS3Bucket_Tags_withSystemTags(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketConfig_noTags(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), acctest.CheckResourceDisappears(ctx, acctest.Provider, tfs3.ResourceBucket(), resourceName), @@ -729,7 +729,7 @@ func TestAccS3Bucket_Tags_withSystemTags(t *testing.T) { }, { Config: testAccBucketConfig_tags(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "tags.%", "3"), resource.TestCheckResourceAttr(resourceName, "tags.Key1", "AAA"), @@ -740,7 +740,7 @@ func TestAccS3Bucket_Tags_withSystemTags(t *testing.T) { }, { Config: testAccBucketConfig_updatedTags(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "tags.%", "4"), resource.TestCheckResourceAttr(resourceName, "tags.Key2", "BBB"), @@ -752,7 +752,7 @@ func TestAccS3Bucket_Tags_withSystemTags(t *testing.T) { }, { Config: testAccBucketConfig_noTags(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), testAccCheckBucketTagKeys(ctx, resourceName, "aws:cloudformation:stack-name", "aws:cloudformation:stack-id", "aws:cloudformation:logical-id"), @@ -777,7 +777,7 @@ func TestAccS3Bucket_Tags_ignoreTags(t *testing.T) { Config: acctest.ConfigCompose( acctest.ConfigIgnoreTagsKeyPrefixes1("ignorekey"), testAccBucketConfig_noTags(bucketName)), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), testAccCheckBucketUpdateTags(ctx, resourceName, nil, map[string]string{"ignorekey1": "ignorevalue1"}), resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), @@ -790,7 +790,7 @@ func TestAccS3Bucket_Tags_ignoreTags(t *testing.T) { Config: acctest.ConfigCompose( acctest.ConfigIgnoreTagsKeyPrefixes1("ignorekey"), testAccBucketConfig_tags(bucketName)), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "tags.%", "3"), resource.TestCheckResourceAttr(resourceName, "tags.Key1", "AAA"), @@ -821,7 +821,7 @@ func TestAccS3Bucket_Manage_lifecycleBasic(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketConfig_lifecycle(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "lifecycle_rule.#", "6"), resource.TestCheckResourceAttr(resourceName, "lifecycle_rule.0.id", "id1"), @@ -891,7 +891,7 @@ func TestAccS3Bucket_Manage_lifecycleBasic(t *testing.T) { }, { Config: testAccBucketConfig_basic(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), ), }, @@ -912,7 +912,7 @@ func TestAccS3Bucket_Manage_lifecycleExpireMarkerOnly(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketConfig_lifecycleExpireMarker(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "lifecycle_rule.0.id", "id1"), resource.TestCheckResourceAttr(resourceName, "lifecycle_rule.0.prefix", "path1/"), @@ -929,7 +929,7 @@ func TestAccS3Bucket_Manage_lifecycleExpireMarkerOnly(t *testing.T) { }, { Config: testAccBucketConfig_basic(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), ), }, @@ -951,7 +951,7 @@ func TestAccS3Bucket_Manage_lifecycleRuleExpirationEmptyBlock(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketConfig_lifecycleRuleExpirationEmptyBlock(rName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), ), }, @@ -973,7 +973,7 @@ func TestAccS3Bucket_Manage_lifecycleRuleAbortIncompleteMultipartUploadDaysNoExp Steps: []resource.TestStep{ { Config: testAccBucketConfig_lifecycleRuleAbortIncompleteMultipartUploadDays(rName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), ), }, @@ -1000,14 +1000,14 @@ func TestAccS3Bucket_Manage_lifecycleRemove(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketConfig_lifecycle(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "lifecycle_rule.#", "6"), ), }, { Config: testAccBucketConfig_basic(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), // As Lifecycle Rule is a Computed field, removing them from terraform will not // trigger an update to remove them from the S3 bucket. @@ -1031,7 +1031,7 @@ func TestAccS3Bucket_Manage_objectLock(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketConfig_objectLockEnabledNoDefaultRetention(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "object_lock_enabled", "true"), resource.TestCheckResourceAttr(resourceName, "object_lock_configuration.#", "1"), @@ -1047,7 +1047,7 @@ func TestAccS3Bucket_Manage_objectLock(t *testing.T) { }, { Config: testAccBucketConfig_objectLockEnabledDefaultRetention(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "object_lock_configuration.#", "1"), resource.TestCheckResourceAttr(resourceName, "object_lock_configuration.0.object_lock_enabled", "Enabled"), @@ -1073,7 +1073,7 @@ func TestAccS3Bucket_Manage_objectLock_deprecatedEnabled(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketConfig_objectLockEnabledNoDefaultRetentionDeprecatedEnabled(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "object_lock_enabled", "true"), resource.TestCheckResourceAttr(resourceName, "object_lock_configuration.#", "1"), @@ -1104,7 +1104,7 @@ func TestAccS3Bucket_Manage_objectLock_migrate(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketConfig_objectLockEnabledNoDefaultRetentionDeprecatedEnabled(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "object_lock_enabled", "true"), resource.TestCheckResourceAttr(resourceName, "object_lock_configuration.#", "1"), @@ -1132,7 +1132,7 @@ func TestAccS3Bucket_Manage_objectLockWithVersioning(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketConfig_objectLockEnabledVersioning(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "object_lock_enabled", "true"), resource.TestCheckResourceAttr(resourceName, "object_lock_configuration.#", "1"), @@ -1162,7 +1162,7 @@ func TestAccS3Bucket_Manage_objectLockWithVersioning_deprecatedEnabled(t *testin Steps: []resource.TestStep{ { Config: testAccBucketConfig_objectLockEnabledVersioningDeprecatedEnabled(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "object_lock_enabled", "true"), resource.TestCheckResourceAttr(resourceName, "object_lock_configuration.#", "1"), @@ -1192,7 +1192,7 @@ func TestAccS3Bucket_Manage_versioning(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketConfig_versioning(bucketName, true), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "versioning.#", "1"), resource.TestCheckResourceAttr(resourceName, "versioning.0.enabled", "true"), @@ -1207,7 +1207,7 @@ func TestAccS3Bucket_Manage_versioning(t *testing.T) { }, { Config: testAccBucketConfig_versioning(bucketName, false), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "versioning.#", "1"), resource.TestCheckResourceAttr(resourceName, "versioning.0.enabled", "false"), @@ -1237,7 +1237,7 @@ func TestAccS3Bucket_Manage_versioningDisabled(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketConfig_versioning(bucketName, false), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "versioning.#", "1"), resource.TestCheckResourceAttr(resourceName, "versioning.0.enabled", "false"), @@ -1267,7 +1267,7 @@ func TestAccS3Bucket_Manage_MFADeleteDisabled(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketConfig_versioningMFADelete(bucketName, false), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "versioning.#", "1"), resource.TestCheckResourceAttr(resourceName, "versioning.0.enabled", "false"), @@ -1297,7 +1297,7 @@ func TestAccS3Bucket_Manage_versioningAndMFADeleteDisabled(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketConfig_versioningDisabledAndMFADelete(bucketName, false), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "versioning.#", "1"), resource.TestCheckResourceAttr(resourceName, "versioning.0.enabled", "false"), @@ -1336,7 +1336,7 @@ func TestAccS3Bucket_Replication_basic(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketConfig_replication(bucketName, string(types.StorageClassStandard)), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExistsWithProvider(ctx, resourceName, acctest.RegionProviderFunc(region, &providers)), resource.TestCheckResourceAttr(resourceName, "replication_configuration.#", "1"), resource.TestCheckResourceAttrPair(resourceName, "replication_configuration.0.role", iamRoleResourceName, "arn"), @@ -1346,7 +1346,7 @@ func TestAccS3Bucket_Replication_basic(t *testing.T) { }, { Config: testAccBucketConfig_replication(bucketName, string(types.StorageClassGlacier)), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExistsWithProvider(ctx, resourceName, acctest.RegionProviderFunc(region, &providers)), resource.TestCheckResourceAttr(resourceName, "replication_configuration.#", "1"), resource.TestCheckResourceAttrPair(resourceName, "replication_configuration.0.role", iamRoleResourceName, "arn"), @@ -1356,7 +1356,7 @@ func TestAccS3Bucket_Replication_basic(t *testing.T) { }, { Config: testAccBucketConfig_replicationSSEKMSEncryptedObjects(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExistsWithProvider(ctx, resourceName, acctest.RegionProviderFunc(region, &providers)), resource.TestCheckResourceAttr(resourceName, "replication_configuration.#", "1"), resource.TestCheckResourceAttrPair(resourceName, "replication_configuration.0.role", iamRoleResourceName, "arn"), @@ -1388,7 +1388,7 @@ func TestAccS3Bucket_Replication_multipleDestinationsEmptyFilter(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketConfig_replicationMultipleDestinationsEmptyFilter(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExistsWithProvider(ctx, resourceName, acctest.RegionProviderFunc(region, &providers)), testAccCheckBucketExistsWithProvider(ctx, "aws_s3_bucket.destination", acctest.RegionProviderFunc(alternateRegion, &providers)), testAccCheckBucketExistsWithProvider(ctx, "aws_s3_bucket.destination2", acctest.RegionProviderFunc(alternateRegion, &providers)), @@ -1459,7 +1459,7 @@ func TestAccS3Bucket_Replication_multipleDestinationsNonEmptyFilter(t *testing.T Steps: []resource.TestStep{ { Config: testAccBucketConfig_replicationMultipleDestinationsNonEmptyFilter(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExistsWithProvider(ctx, resourceName, acctest.RegionProviderFunc(region, &providers)), testAccCheckBucketExistsWithProvider(ctx, "aws_s3_bucket.destination", acctest.RegionProviderFunc(alternateRegion, &providers)), testAccCheckBucketExistsWithProvider(ctx, "aws_s3_bucket.destination2", acctest.RegionProviderFunc(alternateRegion, &providers)), @@ -1535,7 +1535,7 @@ func TestAccS3Bucket_Replication_twoDestination(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketConfig_replicationMultipleDestinationsTwoDestination(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExistsWithProvider(ctx, resourceName, acctest.RegionProviderFunc(region, &providers)), testAccCheckBucketExistsWithProvider(ctx, "aws_s3_bucket.destination", acctest.RegionProviderFunc(alternateRegion, &providers)), testAccCheckBucketExistsWithProvider(ctx, "aws_s3_bucket.destination2", acctest.RegionProviderFunc(alternateRegion, &providers)), @@ -1597,7 +1597,7 @@ func TestAccS3Bucket_Replication_ruleDestinationAccessControlTranslation(t *test Steps: []resource.TestStep{ { Config: testAccBucketConfig_replicationAccessControlTranslation(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExistsWithProvider(ctx, resourceName, acctest.RegionProviderFunc(region, &providers)), resource.TestCheckResourceAttr(resourceName, "replication_configuration.#", "1"), resource.TestCheckResourceAttrPair(resourceName, "replication_configuration.0.role", iamRoleResourceName, "arn"), @@ -1618,7 +1618,7 @@ func TestAccS3Bucket_Replication_ruleDestinationAccessControlTranslation(t *test }, { Config: testAccBucketConfig_replicationSSEKMSEncryptedObjectsAndAccessControlTranslation(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExistsWithProvider(ctx, resourceName, acctest.RegionProviderFunc(region, &providers)), resource.TestCheckResourceAttr(resourceName, "replication_configuration.#", "1"), resource.TestCheckResourceAttrPair(resourceName, "replication_configuration.0.role", iamRoleResourceName, "arn"), @@ -1651,7 +1651,7 @@ func TestAccS3Bucket_Replication_ruleDestinationAddAccessControlTranslation(t *t Steps: []resource.TestStep{ { Config: testAccBucketConfig_replicationRulesDestination(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExistsWithProvider(ctx, resourceName, acctest.RegionProviderFunc(region, &providers)), resource.TestCheckResourceAttr(resourceName, "replication_configuration.#", "1"), resource.TestCheckResourceAttrPair(resourceName, "replication_configuration.0.role", iamRoleResourceName, "arn"), @@ -1672,7 +1672,7 @@ func TestAccS3Bucket_Replication_ruleDestinationAddAccessControlTranslation(t *t }, { Config: testAccBucketConfig_replicationAccessControlTranslation(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExistsWithProvider(ctx, resourceName, acctest.RegionProviderFunc(region, &providers)), resource.TestCheckResourceAttr(resourceName, "replication_configuration.#", "1"), resource.TestCheckResourceAttrPair(resourceName, "replication_configuration.0.role", iamRoleResourceName, "arn"), @@ -1705,7 +1705,7 @@ func TestAccS3Bucket_Replication_withoutStorageClass(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketConfig_replicationNoStorageClass(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExistsWithProvider(ctx, resourceName, acctest.RegionProviderFunc(region, &providers)), testAccCheckBucketExistsWithProvider(ctx, "aws_s3_bucket.destination", acctest.RegionProviderFunc(alternateRegion, &providers)), ), @@ -1771,7 +1771,7 @@ func TestAccS3Bucket_Replication_withoutPrefix(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketConfig_replicationNoPrefix(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExistsWithProvider(ctx, resourceName, acctest.RegionProviderFunc(region, &providers)), testAccCheckBucketExistsWithProvider(ctx, "aws_s3_bucket.destination", acctest.RegionProviderFunc(alternateRegion, &providers)), ), @@ -1813,7 +1813,7 @@ func TestAccS3Bucket_Replication_schemaV2(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketConfig_replicationV2DeleteMarkerReplicationDisabled(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExistsWithProvider(ctx, resourceName, acctest.RegionProviderFunc(region, &providers)), resource.TestCheckResourceAttr(resourceName, "replication_configuration.#", "1"), resource.TestCheckResourceAttrPair(resourceName, "replication_configuration.0.role", iamRoleResourceName, "arn"), @@ -1823,7 +1823,7 @@ func TestAccS3Bucket_Replication_schemaV2(t *testing.T) { }, { Config: testAccBucketConfig_replicationV2NoTags(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExistsWithProvider(ctx, resourceName, acctest.RegionProviderFunc(region, &providers)), resource.TestCheckResourceAttr(resourceName, "replication_configuration.#", "1"), resource.TestCheckResourceAttrPair(resourceName, "replication_configuration.0.role", iamRoleResourceName, "arn"), @@ -1844,7 +1844,7 @@ func TestAccS3Bucket_Replication_schemaV2(t *testing.T) { }, { Config: testAccBucketConfig_replicationV2OnlyOneTag(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExistsWithProvider(ctx, resourceName, acctest.RegionProviderFunc(region, &providers)), resource.TestCheckResourceAttr(resourceName, "replication_configuration.#", "1"), resource.TestCheckResourceAttrPair(resourceName, "replication_configuration.0.role", iamRoleResourceName, "arn"), @@ -1854,7 +1854,7 @@ func TestAccS3Bucket_Replication_schemaV2(t *testing.T) { }, { Config: testAccBucketConfig_replicationV2PrefixAndTags(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExistsWithProvider(ctx, resourceName, acctest.RegionProviderFunc(region, &providers)), resource.TestCheckResourceAttr(resourceName, "replication_configuration.#", "1"), resource.TestCheckResourceAttrPair(resourceName, "replication_configuration.0.role", iamRoleResourceName, "arn"), @@ -1864,7 +1864,7 @@ func TestAccS3Bucket_Replication_schemaV2(t *testing.T) { }, { Config: testAccBucketConfig_replicationV2MultipleTags(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExistsWithProvider(ctx, resourceName, acctest.RegionProviderFunc(region, &providers)), resource.TestCheckResourceAttr(resourceName, "replication_configuration.#", "1"), resource.TestCheckResourceAttrPair(resourceName, "replication_configuration.0.role", iamRoleResourceName, "arn"), @@ -1890,7 +1890,7 @@ func TestAccS3Bucket_Replication_schemaV2SameRegion(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketConfig_replicationV2SameRegionNoTags(rName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "replication_configuration.#", "1"), acctest.CheckResourceAttrGlobalARN(resourceName, "replication_configuration.0.role", "iam", fmt.Sprintf("role/%s", rName)), @@ -1935,7 +1935,7 @@ func TestAccS3Bucket_Replication_RTC_valid(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketConfig_replicationV2RTC(bucketName, 15), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExistsWithProvider(ctx, resourceName, acctest.RegionProviderFunc(region, &providers)), resource.TestCheckResourceAttr(resourceName, "replication_configuration.#", "1"), resource.TestCheckResourceAttrPair(resourceName, "replication_configuration.0.role", iamRoleResourceName, "arn"), @@ -1945,7 +1945,7 @@ func TestAccS3Bucket_Replication_RTC_valid(t *testing.T) { }, { Config: testAccBucketConfig_replicationV2RTCNoMinutes(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExistsWithProvider(ctx, resourceName, acctest.RegionProviderFunc(region, &providers)), resource.TestCheckResourceAttr(resourceName, "replication_configuration.#", "1"), resource.TestCheckResourceAttrPair(resourceName, "replication_configuration.0.role", iamRoleResourceName, "arn"), @@ -1955,7 +1955,7 @@ func TestAccS3Bucket_Replication_RTC_valid(t *testing.T) { }, { Config: testAccBucketConfig_replicationV2RTCNoStatus(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExistsWithProvider(ctx, resourceName, acctest.RegionProviderFunc(region, &providers)), resource.TestCheckResourceAttr(resourceName, "replication_configuration.#", "1"), resource.TestCheckResourceAttrPair(resourceName, "replication_configuration.0.role", iamRoleResourceName, "arn"), @@ -1965,7 +1965,7 @@ func TestAccS3Bucket_Replication_RTC_valid(t *testing.T) { }, { Config: testAccBucketConfig_replicationV2RTCNotConfigured(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExistsWithProvider(ctx, resourceName, acctest.RegionProviderFunc(region, &providers)), resource.TestCheckResourceAttr(resourceName, "replication_configuration.#", "1"), resource.TestCheckResourceAttrPair(resourceName, "replication_configuration.0.role", iamRoleResourceName, "arn"), @@ -2020,7 +2020,7 @@ func TestAccS3Bucket_Security_corsUpdate(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketConfig_cors(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "cors_rule.#", "1"), resource.TestCheckResourceAttr(resourceName, "cors_rule.0.allowed_headers.#", "1"), @@ -2050,7 +2050,7 @@ func TestAccS3Bucket_Security_corsUpdate(t *testing.T) { }, { Config: testAccBucketConfig_cors(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "cors_rule.#", "1"), resource.TestCheckResourceAttr(resourceName, "cors_rule.0.allowed_headers.#", "1"), @@ -2113,7 +2113,7 @@ func TestAccS3Bucket_Security_corsDelete(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketConfig_cors(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), deleteBucketCors(resourceName), ), @@ -2136,7 +2136,7 @@ func TestAccS3Bucket_Security_corsEmptyOrigin(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketConfig_corsEmptyOrigin(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "cors_rule.#", "1"), resource.TestCheckResourceAttr(resourceName, "cors_rule.0.allowed_headers.#", "1"), @@ -2175,7 +2175,7 @@ func TestAccS3Bucket_Security_corsSingleMethodAndEmptyOrigin(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketConfig_corsSingleMethodAndEmptyOrigin(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), ), }, @@ -2202,7 +2202,7 @@ func TestAccS3Bucket_Security_logging(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketConfig_logging(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "logging.#", "1"), resource.TestCheckResourceAttrPair(resourceName, "logging.0.target_bucket", "aws_s3_bucket.log_bucket", "id"), @@ -2232,7 +2232,7 @@ func TestAccS3Bucket_Security_enableDefaultEncryptionWhenTypical(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketConfig_defaultEncryptionKMSMasterKey(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "server_side_encryption_configuration.#", "1"), resource.TestCheckResourceAttr(resourceName, "server_side_encryption_configuration.0.rule.#", "1"), @@ -2264,7 +2264,7 @@ func TestAccS3Bucket_Security_enableDefaultEncryptionWhenAES256IsUsed(t *testing Steps: []resource.TestStep{ { Config: testAccBucketConfig_defaultEncryptionDefaultKey(bucketName, string(types.ServerSideEncryptionAes256)), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "server_side_encryption_configuration.#", "1"), resource.TestCheckResourceAttr(resourceName, "server_side_encryption_configuration.0.rule.#", "1"), @@ -2333,7 +2333,7 @@ func TestAccS3Bucket_Web_simple(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketConfig_website(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "website.#", "1"), resource.TestCheckResourceAttr(resourceName, "website.0.index_document", "index.html"), @@ -2348,7 +2348,7 @@ func TestAccS3Bucket_Web_simple(t *testing.T) { }, { Config: testAccBucketConfig_websiteAndError(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "website.#", "1"), resource.TestCheckResourceAttr(resourceName, "website.0.index_document", "index.html"), @@ -2360,7 +2360,7 @@ func TestAccS3Bucket_Web_simple(t *testing.T) { // As Website is a Computed field, removing them from terraform will not // trigger an update to remove them from the S3 bucket. Config: testAccBucketConfig_basic(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "website.#", "1"), resource.TestCheckResourceAttr(resourceName, "website.0.index_document", "index.html"), @@ -2386,7 +2386,7 @@ func TestAccS3Bucket_Web_redirect(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketConfig_websiteAndRedirect(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "website.#", "1"), resource.TestCheckResourceAttr(resourceName, "website.0.redirect_all_requests_to", "hashicorp.com?my=query"), @@ -2401,7 +2401,7 @@ func TestAccS3Bucket_Web_redirect(t *testing.T) { }, { Config: testAccBucketConfig_websiteAndHTTPSRedirect(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "website.#", "1"), resource.TestCheckResourceAttr(resourceName, "website.0.redirect_all_requests_to", "https://hashicorp.com?my=query"), @@ -2412,7 +2412,7 @@ func TestAccS3Bucket_Web_redirect(t *testing.T) { // As Website is a Computed field, removing them from terraform will not // trigger an update to remove them from the S3 bucket. Config: testAccBucketConfig_basic(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "website.#", "1"), resource.TestCheckResourceAttr(resourceName, "website.0.redirect_all_requests_to", "https://hashicorp.com?my=query"), @@ -2437,7 +2437,7 @@ func TestAccS3Bucket_Web_routingRules(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccBucketConfig_websiteAndRoutingRules(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "website.#", "1"), resource.TestCheckResourceAttr(resourceName, "website.0.error_document", "error.html"), @@ -2456,7 +2456,7 @@ func TestAccS3Bucket_Web_routingRules(t *testing.T) { // As Website is a Computed field, removing them from terraform will not // trigger an update to remove them from the S3 bucket. Config: testAccBucketConfig_basic(bucketName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "website.#", "1"), resource.TestCheckResourceAttr(resourceName, "website.0.error_document", "error.html"), diff --git a/internal/service/s3/object.go b/internal/service/s3/object.go index 274fa050136c..4fd8315fa159 100644 --- a/internal/service/s3/object.go +++ b/internal/service/s3/object.go @@ -69,6 +69,10 @@ func resourceObject() *schema.Resource { Computed: true, ValidateDiagFunc: enum.Validate[types.ObjectCannedACL](), }, + "arn": { + Type: schema.TypeString, + Computed: true, + }, "bucket": { Type: schema.TypeString, Required: true, @@ -276,6 +280,9 @@ func resourceObjectRead(ctx context.Context, d *schema.ResourceData, meta interf return sdkdiag.AppendErrorf(diags, "reading S3 Object (%s): %s", d.Id(), err) } + arn := newObjectARN(meta.(*conns.AWSClient).Partition, bucket, key) + d.Set("arn", arn.String()) + d.Set("bucket_key_enabled", output.BucketKeyEnabled) d.Set("cache_control", output.CacheControl) d.Set("checksum_crc32", output.ChecksumCRC32) @@ -787,3 +794,11 @@ func expandDefaultTags(ctx context.Context, tfMap map[string]interface{}) *tftag return data } + +func newObjectARN(partition string, bucket, key string) arn.ARN { + return arn.ARN{ + Partition: partition, + Service: "s3", + Resource: fmt.Sprintf("%s/%s", bucket, key), + } +} diff --git a/internal/service/s3/object_copy.go b/internal/service/s3/object_copy.go index 174aa30e6828..1813ac03e3e1 100644 --- a/internal/service/s3/object_copy.go +++ b/internal/service/s3/object_copy.go @@ -48,6 +48,10 @@ func resourceObjectCopy() *schema.Resource { ValidateDiagFunc: enum.Validate[types.ObjectCannedACL](), ConflictsWith: []string{"grant"}, }, + "arn": { + Type: schema.TypeString, + Computed: true, + }, "bucket": { Type: schema.TypeString, Required: true, @@ -356,6 +360,9 @@ func resourceObjectCopyRead(ctx context.Context, d *schema.ResourceData, meta in return sdkdiag.AppendErrorf(diags, "reading S3 Object (%s): %s", d.Id(), err) } + arn := newObjectARN(meta.(*conns.AWSClient).Partition, bucket, key) + d.Set("arn", arn.String()) + d.Set("bucket_key_enabled", output.BucketKeyEnabled) d.Set("cache_control", output.CacheControl) d.Set("checksum_crc32", output.ChecksumCRC32) diff --git a/internal/service/s3/object_copy_test.go b/internal/service/s3/object_copy_test.go index 4c466dfe7884..5605e2b6b9d6 100644 --- a/internal/service/s3/object_copy_test.go +++ b/internal/service/s3/object_copy_test.go @@ -22,8 +22,8 @@ import ( func TestAccS3ObjectCopy_basic(t *testing.T) { ctx := acctest.Context(t) - rName1 := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) - rName2 := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + rNameSource := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + rNameTarget := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_s3_object_copy.test" sourceName := "aws_s3_object.source" sourceKey := "source" @@ -36,11 +36,12 @@ func TestAccS3ObjectCopy_basic(t *testing.T) { CheckDestroy: testAccCheckObjectCopyDestroy(ctx), Steps: []resource.TestStep{ { - Config: testAccObjectCopyConfig_basic(rName1, sourceKey, rName2, targetKey), + Config: testAccObjectCopyConfig_basic(rNameSource, sourceKey, rNameTarget, targetKey), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectCopyExists(ctx, resourceName), resource.TestCheckNoResourceAttr(resourceName, "acl"), - resource.TestCheckResourceAttr(resourceName, "bucket", rName2), + resource.TestCheckResourceAttr(resourceName, "arn", fmt.Sprintf("arn:aws:s3:::%s/%s", rNameTarget, targetKey)), + resource.TestCheckResourceAttr(resourceName, "bucket", rNameTarget), resource.TestCheckResourceAttr(resourceName, "bucket_key_enabled", "false"), resource.TestCheckResourceAttr(resourceName, "cache_control", ""), resource.TestCheckNoResourceAttr(resourceName, "checksum_algorithm"), @@ -78,7 +79,7 @@ func TestAccS3ObjectCopy_basic(t *testing.T) { 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.TestCheckResourceAttr(resourceName, "source", fmt.Sprintf("%s/%s", rNameSource, sourceKey)), resource.TestCheckNoResourceAttr(resourceName, "source_customer_algorithm"), resource.TestCheckNoResourceAttr(resourceName, "source_customer_key"), resource.TestCheckNoResourceAttr(resourceName, "source_customer_key_md5"), @@ -110,7 +111,7 @@ func TestAccS3ObjectCopy_disappears(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccObjectCopyConfig_basic(rName1, sourceKey, rName2, targetKey), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectCopyExists(ctx, resourceName), acctest.CheckResourceDisappears(ctx, acctest.Provider, tfs3.ResourceObjectCopy(), resourceName), ), @@ -136,7 +137,7 @@ func TestAccS3ObjectCopy_tags(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccObjectCopyConfig_tags1(rName1, sourceKey, rName2, targetKey, "key1", "value1"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectCopyExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "tagging_directive", "REPLACE"), resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), @@ -145,7 +146,7 @@ func TestAccS3ObjectCopy_tags(t *testing.T) { }, { Config: testAccObjectCopyConfig_tags2(rName1, sourceKey, rName2, targetKey, "key1", "value1updated", "key2", "value2"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectCopyExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "tagging_directive", "REPLACE"), resource.TestCheckResourceAttr(resourceName, "tags.%", "2"), @@ -155,7 +156,7 @@ func TestAccS3ObjectCopy_tags(t *testing.T) { }, { Config: testAccObjectCopyConfig_tags1(rName1, sourceKey, rName2, targetKey, "key2", "value2"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectCopyExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "tagging_directive", "REPLACE"), resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), @@ -182,7 +183,7 @@ func TestAccS3ObjectCopy_metadata(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccObjectCopyConfig_metadata(rName1, sourceKey, rName2, targetKey), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectCopyExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "metadata_directive", "REPLACE"), resource.TestCheckResourceAttr(resourceName, "metadata.%", "1"), @@ -209,7 +210,7 @@ func TestAccS3ObjectCopy_grant(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccObjectCopyConfig_grant(rName1, sourceKey, rName2, targetKey), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectCopyExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "grant.#", "1"), resource.TestCheckTypeSetElemNestedAttrs(resourceName, "grant.*", map[string]string{ @@ -239,7 +240,7 @@ func TestAccS3ObjectCopy_BucketKeyEnabled_bucket(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccObjectCopyConfig_bucketKeyEnabledBucket(rName1, sourceKey, rName2, targetKey), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectCopyExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "bucket_key_enabled", "true"), ), @@ -264,7 +265,7 @@ func TestAccS3ObjectCopy_BucketKeyEnabled_object(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccObjectCopyConfig_bucketKeyEnabledObject(rName1, sourceKey, rName2, targetKey), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectCopyExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "bucket_key_enabled", "true"), ), @@ -289,13 +290,13 @@ func TestAccS3ObjectCopy_sourceWithSlashes(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccObjectCopyConfig_baseSourceAndTargetBuckets(rName1, rName2), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketAddObjects(ctx, "aws_s3_bucket.source", sourceKey), ), }, { Config: testAccObjectCopyConfig_externalSourceObject(rName1, sourceKey, rName2, targetKey), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectCopyExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "source", fmt.Sprintf("%s/%s", rName1, sourceKey)), ), @@ -320,7 +321,7 @@ func TestAccS3ObjectCopy_checksumAlgorithm(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccObjectCopyConfig_checksumAlgorithm(rName1, sourceKey, rName2, targetKey, "CRC32C"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectCopyExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "checksum_algorithm", "CRC32C"), resource.TestCheckResourceAttr(resourceName, "checksum_crc32", ""), @@ -331,7 +332,7 @@ func TestAccS3ObjectCopy_checksumAlgorithm(t *testing.T) { }, { Config: testAccObjectCopyConfig_checksumAlgorithm(rName1, sourceKey, rName2, targetKey, "SHA1"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectCopyExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "checksum_algorithm", "SHA1"), resource.TestCheckResourceAttr(resourceName, "checksum_crc32", ""), @@ -360,14 +361,14 @@ func TestAccS3ObjectCopy_objectLockLegalHold(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccObjectCopyConfig_lockLegalHold(rName1, sourceKey, rName2, targetKey, "ON"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectCopyExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "object_lock_legal_hold_status", "ON"), ), }, { Config: testAccObjectCopyConfig_lockLegalHold(rName1, sourceKey, rName2, targetKey, "OFF"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectCopyExists(ctx, resourceName), resource.TestCheckResourceAttr(resourceName, "object_lock_legal_hold_status", "OFF"), ), @@ -392,7 +393,7 @@ func TestAccS3ObjectCopy_targetWithMultipleSlashes(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccObjectCopyConfig_basic(rName1, sourceKey, rName2, targetKey), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( resource.TestCheckResourceAttr(resourceName, "key", targetKey), resource.TestCheckResourceAttr(resourceName, "source", fmt.Sprintf("%s/%s", rName1, sourceKey)), ), @@ -423,7 +424,7 @@ func TestAccS3ObjectCopy_targetWithMultipleSlashesMigrated(t *testing.T) { }, }, Config: testAccObjectCopyConfig_basic(rName1, sourceKey, rName2, targetKey), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( resource.TestCheckResourceAttr(resourceName, "key", targetKey), resource.TestCheckResourceAttr(resourceName, "source", fmt.Sprintf("%s/%s", rName1, sourceKey)), ), @@ -571,7 +572,7 @@ func TestAccS3ObjectCopy_basicViaAccessPoint(t *testing.T) { resource.TestCheckNoResourceAttr(resourceName, "source_customer_algorithm"), resource.TestCheckNoResourceAttr(resourceName, "source_customer_key"), resource.TestCheckNoResourceAttr(resourceName, "source_customer_key_md5"), - resource.TestCheckResourceAttrSet(resourceName, "source_version_id"), + resource.TestCheckResourceAttr(resourceName, "source_version_id", ""), resource.TestCheckResourceAttr(resourceName, "storage_class", "STANDARD"), resource.TestCheckNoResourceAttr(resourceName, "tagging_directive"), resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), diff --git a/internal/service/s3/object_data_source.go b/internal/service/s3/object_data_source.go index 5f96710af9fb..e768e4f8f61d 100644 --- a/internal/service/s3/object_data_source.go +++ b/internal/service/s3/object_data_source.go @@ -32,6 +32,10 @@ func dataSourceObject() *schema.Resource { ReadWithoutTimeout: dataSourceObjectRead, Schema: map[string]*schema.Schema{ + "arn": { + Type: schema.TypeString, + Computed: true, + }, "body": { Type: schema.TypeString, Computed: true, @@ -201,6 +205,9 @@ func dataSourceObjectRead(ctx context.Context, d *schema.ResourceData, meta inte } d.SetId(id) + arn := newObjectARN(meta.(*conns.AWSClient).Partition, bucket, key) + d.Set("arn", arn.String()) + d.Set("bucket_key_enabled", output.BucketKeyEnabled) d.Set("cache_control", output.CacheControl) d.Set("checksum_crc32", output.ChecksumCRC32) diff --git a/internal/service/s3/object_data_source_test.go b/internal/service/s3/object_data_source_test.go index a7f62eefc8b5..8b87e7825973 100644 --- a/internal/service/s3/object_data_source_test.go +++ b/internal/service/s3/object_data_source_test.go @@ -32,6 +32,7 @@ func TestAccS3ObjectDataSource_basic(t *testing.T) { { Config: testAccObjectDataSourceConfig_basic(rName), Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttrPair(dataSourceName, "arn", resourceName, "arn"), resource.TestCheckNoResourceAttr(dataSourceName, "body"), resource.TestCheckNoResourceAttr(dataSourceName, "checksum_mode"), resource.TestCheckResourceAttr(resourceName, "checksum_crc32", ""), @@ -67,7 +68,7 @@ func TestAccS3ObjectDataSource_basicViaAccessPoint(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccObjectDataSourceConfig_basicViaAccessPoint(rName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( resource.TestCheckResourceAttrPair(dataSourceName, "bucket", accessPointResourceName, "arn"), resource.TestCheckResourceAttrPair(dataSourceName, "key", resourceName, "key"), ), @@ -90,7 +91,7 @@ func TestAccS3ObjectDataSource_readableBody(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccObjectDataSourceConfig_readableBody(rName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( resource.TestCheckResourceAttr(dataSourceName, "body", "yes"), resource.TestCheckResourceAttr(dataSourceName, "content_length", "3"), resource.TestCheckResourceAttrPair(dataSourceName, "content_type", resourceName, "content_type"), @@ -119,7 +120,7 @@ func TestAccS3ObjectDataSource_kmsEncrypted(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccObjectDataSourceConfig_kmsEncrypted(rName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( resource.TestCheckResourceAttr(dataSourceName, "body", "Keep Calm and Carry On"), resource.TestCheckResourceAttr(dataSourceName, "content_length", "22"), resource.TestCheckResourceAttrPair(dataSourceName, "content_type", resourceName, "content_type"), @@ -150,7 +151,7 @@ func TestAccS3ObjectDataSource_bucketKeyEnabled(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccObjectDataSourceConfig_bucketKeyEnabled(rName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( resource.TestCheckResourceAttr(dataSourceName, "body", "Keep Calm and Carry On"), resource.TestCheckResourceAttrPair(dataSourceName, "bucket_key_enabled", resourceName, "bucket_key_enabled"), resource.TestCheckResourceAttr(dataSourceName, "content_length", "22"), @@ -228,7 +229,7 @@ func TestAccS3ObjectDataSource_objectLockLegalHoldOff(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccObjectDataSourceConfig_lockLegalHoldOff(rName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( resource.TestCheckNoResourceAttr(dataSourceName, "body"), resource.TestCheckResourceAttr(dataSourceName, "content_length", "11"), resource.TestCheckResourceAttrPair(dataSourceName, "content_type", resourceName, "content_type"), @@ -258,7 +259,7 @@ func TestAccS3ObjectDataSource_objectLockLegalHoldOn(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccObjectDataSourceConfig_lockLegalHoldOn(rName, retainUntilDate), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( resource.TestCheckNoResourceAttr(dataSourceName, "body"), resource.TestCheckResourceAttr(dataSourceName, "content_length", "11"), resource.TestCheckResourceAttrPair(dataSourceName, "content_type", resourceName, "content_type"), @@ -294,7 +295,7 @@ func TestAccS3ObjectDataSource_leadingSlash(t *testing.T) { }, { // nosemgrep:ci.test-config-funcs-correct-form Config: conf, - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( resource.TestCheckResourceAttr(dataSourceName1, "body", "yes"), resource.TestCheckResourceAttr(dataSourceName1, "content_length", "3"), resource.TestCheckResourceAttrPair(dataSourceName1, "content_type", resourceName, "content_type"), @@ -340,7 +341,7 @@ func TestAccS3ObjectDataSource_multipleSlashes(t *testing.T) { }, { // nosemgrep:ci.test-config-funcs-correct-form Config: conf, - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( resource.TestCheckResourceAttr(dataSourceName1, "body", "yes"), resource.TestCheckResourceAttr(dataSourceName1, "content_length", "3"), resource.TestCheckResourceAttrPair(dataSourceName1, "content_type", resourceName1, "content_type"), @@ -396,7 +397,7 @@ func TestAccS3ObjectDataSource_leadingDotSlash(t *testing.T) { }, { // nosemgrep:ci.test-config-funcs-correct-form Config: conf, - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( resource.TestCheckResourceAttr(dataSourceName1, "body", "yes"), resource.TestCheckResourceAttr(dataSourceName1, "content_length", "3"), resource.TestCheckResourceAttrPair(dataSourceName1, "content_type", resourceName, "content_type"), @@ -435,7 +436,7 @@ func TestAccS3ObjectDataSource_leadingMultipleSlashes(t *testing.T) { }, { // nosemgrep:ci.test-config-funcs-correct-form Config: conf, - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( resource.TestCheckResourceAttr(dataSourceName1, "body", "yes"), resource.TestCheckResourceAttr(dataSourceName1, "content_length", "3"), resource.TestCheckResourceAttrPair(dataSourceName1, "content_type", resourceName, "content_type"), @@ -473,7 +474,7 @@ func TestAccS3ObjectDataSource_checksumMode(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccObjectDataSourceConfig_checksumMode(rName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( resource.TestCheckResourceAttr(dataSourceName, "checksum_mode", "ENABLED"), resource.TestCheckResourceAttrPair(dataSourceName, "checksum_crc32", resourceName, "checksum_crc32"), resource.TestCheckResourceAttrPair(dataSourceName, "checksum_crc32c", resourceName, "checksum_crc32c"), @@ -499,7 +500,7 @@ func TestAccS3ObjectDataSource_metadata(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccObjectDataSourceConfig_metadata(rName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( resource.TestCheckResourceAttr(dataSourceName, "metadata.%", "2"), resource.TestCheckResourceAttr(dataSourceName, "metadata.key1", "value1"), resource.TestCheckResourceAttr(dataSourceName, "metadata.key2", "Value2"), @@ -524,7 +525,7 @@ func TestAccS3ObjectDataSource_metadataUppercaseKey(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccObjectDataSourceConfig_metadataBucketOnly(rName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckBucketAddObjectWithMetadata(ctx, bucketResourceName, key, map[string]string{ "key1": "value1", "Key2": "Value2", @@ -533,7 +534,7 @@ func TestAccS3ObjectDataSource_metadataUppercaseKey(t *testing.T) { }, { Config: testAccObjectDataSourceConfig_metadataBucketAndDS(rName, key), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( resource.TestCheckResourceAttr(dataSourceName, "metadata.%", "2"), resource.TestCheckResourceAttr(dataSourceName, "metadata.key1", "value1"), // https://pkg.go.dev/github.com/aws/aws-sdk-go-v2/service/s3#HeadObjectOutput diff --git a/internal/service/s3/object_test.go b/internal/service/s3/object_test.go index 92e0c6e231e7..1eb347aa7ece 100644 --- a/internal/service/s3/object_test.go +++ b/internal/service/s3/object_test.go @@ -125,6 +125,7 @@ func TestAccS3Object_basic(t *testing.T) { testAccCheckObjectExists(ctx, resourceName, &obj), testAccCheckObjectBody(&obj, ""), resource.TestCheckNoResourceAttr(resourceName, "acl"), + resource.TestCheckResourceAttr(resourceName, "arn", fmt.Sprintf("arn:aws:s3:::%s/test-key", rName)), resource.TestCheckResourceAttr(resourceName, "bucket", rName), resource.TestCheckResourceAttr(resourceName, "bucket_key_enabled", "false"), resource.TestCheckResourceAttr(resourceName, "cache_control", ""), @@ -181,7 +182,7 @@ func TestAccS3Object_disappears(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccObjectConfig_basic(rName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj), acctest.CheckResourceDisappears(ctx, acctest.Provider, tfs3.ResourceObject(), resourceName), ), @@ -205,7 +206,7 @@ func TestAccS3Object_Disappears_bucket(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccObjectConfig_basic(rName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj), acctest.CheckResourceDisappears(ctx, acctest.Provider, tfs3.ResourceBucket(), "aws_s3_bucket.test"), ), @@ -234,7 +235,7 @@ func TestAccS3Object_upgradeFromV4(t *testing.T) { }, }, Config: testAccObjectConfig_basic(rName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj), ), }, @@ -264,7 +265,7 @@ func TestAccS3Object_source(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccObjectConfig_source(rName, source), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj), testAccCheckObjectBody(&obj, "{anything will do }"), ), @@ -294,7 +295,7 @@ func TestAccS3Object_content(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccObjectConfig_content(rName, "some_bucket_content"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj), testAccCheckObjectBody(&obj, "some_bucket_content"), ), @@ -326,7 +327,7 @@ func TestAccS3Object_etagEncryption(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccObjectConfig_etagEncryption(rName, source), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj), testAccCheckObjectBody(&obj, "{anything will do }"), resource.TestCheckResourceAttr(resourceName, "etag", "7b006ff4d70f68cc65061acf2f802e6f"), @@ -357,7 +358,7 @@ func TestAccS3Object_contentBase64(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccObjectConfig_contentBase64(rName, base64.StdEncoding.EncodeToString([]byte("some_bucket_content"))), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj), testAccCheckObjectBody(&obj, "some_bucket_content"), ), @@ -394,7 +395,7 @@ func TestAccS3Object_sourceHashTrigger(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccObjectConfig_sourceHashTrigger(rName, filename), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj), testAccCheckObjectBody(&obj, "Ebben!"), resource.TestCheckResourceAttr(resourceName, "source_hash", "7c7e02a79f28968882bb1426c8f8bfc6"), @@ -404,7 +405,7 @@ func TestAccS3Object_sourceHashTrigger(t *testing.T) { }, { Config: testAccObjectConfig_sourceHashTrigger(rName, filename), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &updated_obj), testAccCheckObjectBody(&updated_obj, "Ne andrò lontana"), resource.TestCheckResourceAttr(resourceName, "source_hash", "cffc5e20de2d21764145b1124c9b337b"), @@ -438,7 +439,7 @@ func TestAccS3Object_withContentCharacteristics(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccObjectConfig_contentCharacteristics(rName, source), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj), testAccCheckObjectBody(&obj, "{anything will do }"), resource.TestCheckResourceAttr(resourceName, "content_type", "binary/octet-stream"), @@ -465,7 +466,7 @@ func TestAccS3Object_nonVersioned(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccObjectConfig_nonVersioned(rName, sourceInitial), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &originalObj), testAccCheckObjectBody(&originalObj, "initial object state"), resource.TestCheckResourceAttr(resourceName, "version_id", ""), @@ -501,7 +502,7 @@ func TestAccS3Object_updates(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccObjectConfig_updateable(rName, false, sourceInitial), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &originalObj), testAccCheckObjectBody(&originalObj, "initial object state"), resource.TestCheckResourceAttr(resourceName, "etag", "647d1d58e1011c743ec67d5e8af87b53"), @@ -512,7 +513,7 @@ func TestAccS3Object_updates(t *testing.T) { }, { Config: testAccObjectConfig_updateable(rName, false, sourceModified), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &modifiedObj), testAccCheckObjectBody(&modifiedObj, "modified object"), resource.TestCheckResourceAttr(resourceName, "etag", "1c7fd13df1515c2a13ad9eb068931f09"), @@ -560,7 +561,7 @@ func TestAccS3Object_updateSameFile(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccObjectConfig_updateable(rName, false, filename), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &originalObj), testAccCheckObjectBody(&originalObj, startingData), resource.TestCheckResourceAttr(resourceName, "etag", "aa48b42f36a2652cbee40c30a5df7d25"), @@ -570,7 +571,7 @@ func TestAccS3Object_updateSameFile(t *testing.T) { }, { Config: testAccObjectConfig_updateable(rName, false, filename), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &modifiedObj), testAccCheckObjectBody(&modifiedObj, changingData), resource.TestCheckResourceAttr(resourceName, "etag", "fafc05f8c4da0266a99154681ab86e8c"), @@ -599,7 +600,7 @@ func TestAccS3Object_updatesWithVersioning(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccObjectConfig_updateable(rName, true, sourceInitial), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &originalObj), testAccCheckObjectBody(&originalObj, "initial versioned object state"), resource.TestCheckResourceAttr(resourceName, "etag", "cee4407fa91906284e2a5e5e03e86b1b"), @@ -607,7 +608,7 @@ func TestAccS3Object_updatesWithVersioning(t *testing.T) { }, { Config: testAccObjectConfig_updateable(rName, true, sourceModified), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &modifiedObj), testAccCheckObjectBody(&modifiedObj, "modified versioned object"), resource.TestCheckResourceAttr(resourceName, "etag", "00b8c73b1b50e7cc932362c7225b8e29"), @@ -645,7 +646,7 @@ func TestAccS3Object_updatesWithVersioningViaAccessPoint(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccObjectConfig_updateableViaAccessPoint(rName, true, sourceInitial), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &originalObj), testAccCheckObjectBody(&originalObj, "initial versioned object state"), resource.TestCheckResourceAttrPair(resourceName, "bucket", accessPointResourceName, "arn"), @@ -654,7 +655,7 @@ func TestAccS3Object_updatesWithVersioningViaAccessPoint(t *testing.T) { }, { Config: testAccObjectConfig_updateableViaAccessPoint(rName, true, sourceModified), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &modifiedObj), testAccCheckObjectBody(&modifiedObj, "modified versioned object"), resource.TestCheckResourceAttr(resourceName, "etag", "00b8c73b1b50e7cc932362c7225b8e29"), @@ -682,7 +683,7 @@ func TestAccS3Object_kms(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccObjectConfig_kmsID(rName, source), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj), testAccCheckObjectSSE(ctx, resourceName, "aws:kms"), testAccCheckObjectBody(&obj, "{anything will do }"), @@ -716,7 +717,7 @@ func TestAccS3Object_sse(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccObjectConfig_sse(rName, source), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj), testAccCheckObjectSSE(ctx, resourceName, "AES256"), testAccCheckObjectBody(&obj, "{anything will do }"), @@ -747,7 +748,7 @@ func TestAccS3Object_acl(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccObjectConfig_acl(rName, "some_bucket_content", string(types.BucketCannedACLPrivate), true), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj1), testAccCheckObjectBody(&obj1, "some_bucket_content"), resource.TestCheckResourceAttr(resourceName, "acl", string(types.BucketCannedACLPrivate)), @@ -756,7 +757,7 @@ func TestAccS3Object_acl(t *testing.T) { }, { Config: testAccObjectConfig_acl(rName, "some_bucket_content", string(types.BucketCannedACLPublicRead), false), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj2), testAccCheckObjectVersionIDEquals(&obj2, &obj1), testAccCheckObjectBody(&obj2, "some_bucket_content"), @@ -766,7 +767,7 @@ func TestAccS3Object_acl(t *testing.T) { }, { Config: testAccObjectConfig_acl(rName, "changed_some_bucket_content", string(types.BucketCannedACLPrivate), true), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj3), testAccCheckObjectVersionIDDiffers(&obj3, &obj2), testAccCheckObjectBody(&obj3, "changed_some_bucket_content"), @@ -799,7 +800,7 @@ func TestAccS3Object_metadata(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccObjectConfig_metadata(rName, "key1", "value1", "key2", "value2"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj), resource.TestCheckResourceAttr(resourceName, "metadata.%", "2"), resource.TestCheckResourceAttr(resourceName, "metadata.key1", "value1"), @@ -808,7 +809,7 @@ func TestAccS3Object_metadata(t *testing.T) { }, { Config: testAccObjectConfig_metadata(rName, "key1", "value1updated", "key3", "value3"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj), resource.TestCheckResourceAttr(resourceName, "metadata.%", "2"), resource.TestCheckResourceAttr(resourceName, "metadata.key1", "value1updated"), @@ -817,7 +818,7 @@ func TestAccS3Object_metadata(t *testing.T) { }, { Config: testAccObjectConfig_basic(rName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj), resource.TestCheckResourceAttr(resourceName, "metadata.%", "0"), ), @@ -847,7 +848,7 @@ func TestAccS3Object_storageClass(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccObjectConfig_content(rName, "some_bucket_content"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj), resource.TestCheckResourceAttr(resourceName, "storage_class", "STANDARD"), testAccCheckObjectStorageClass(ctx, resourceName, "STANDARD"), @@ -855,7 +856,7 @@ func TestAccS3Object_storageClass(t *testing.T) { }, { Config: testAccObjectConfig_storageClass(rName, "REDUCED_REDUNDANCY"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj), resource.TestCheckResourceAttr(resourceName, "storage_class", "REDUCED_REDUNDANCY"), testAccCheckObjectStorageClass(ctx, resourceName, "REDUCED_REDUNDANCY"), @@ -863,7 +864,7 @@ func TestAccS3Object_storageClass(t *testing.T) { }, { Config: testAccObjectConfig_storageClass(rName, "GLACIER"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( // Can't GetObject on an object in Glacier without restoring it. resource.TestCheckResourceAttr(resourceName, "storage_class", "GLACIER"), testAccCheckObjectStorageClass(ctx, resourceName, "GLACIER"), @@ -871,7 +872,7 @@ func TestAccS3Object_storageClass(t *testing.T) { }, { Config: testAccObjectConfig_storageClass(rName, "INTELLIGENT_TIERING"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj), resource.TestCheckResourceAttr(resourceName, "storage_class", "INTELLIGENT_TIERING"), testAccCheckObjectStorageClass(ctx, resourceName, "INTELLIGENT_TIERING"), @@ -879,7 +880,7 @@ func TestAccS3Object_storageClass(t *testing.T) { }, { Config: testAccObjectConfig_storageClass(rName, "DEEP_ARCHIVE"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( // Can't GetObject on an object in DEEP_ARCHIVE without restoring it. resource.TestCheckResourceAttr(resourceName, "storage_class", "DEEP_ARCHIVE"), testAccCheckObjectStorageClass(ctx, resourceName, "DEEP_ARCHIVE"), @@ -911,7 +912,7 @@ func TestAccS3Object_tags(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccObjectConfig_tags(rName, key, "stuff"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj1), testAccCheckObjectBody(&obj1, "stuff"), resource.TestCheckResourceAttr(resourceName, "tags.%", "3"), @@ -922,7 +923,7 @@ func TestAccS3Object_tags(t *testing.T) { }, { Config: testAccObjectConfig_updatedTags(rName, key, "stuff"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj2), testAccCheckObjectVersionIDEquals(&obj2, &obj1), testAccCheckObjectBody(&obj2, "stuff"), @@ -935,7 +936,7 @@ func TestAccS3Object_tags(t *testing.T) { }, { Config: testAccObjectConfig_noTags(rName, key, "stuff"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj3), testAccCheckObjectVersionIDEquals(&obj3, &obj2), testAccCheckObjectBody(&obj3, "stuff"), @@ -944,7 +945,7 @@ func TestAccS3Object_tags(t *testing.T) { }, { Config: testAccObjectConfig_tags(rName, key, "changed stuff"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj4), testAccCheckObjectVersionIDDiffers(&obj4, &obj3), testAccCheckObjectBody(&obj4, "changed stuff"), @@ -980,7 +981,7 @@ func TestAccS3Object_tagsLeadingSingleSlash(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccObjectConfig_tags(rName, key, "stuff"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj1), testAccCheckObjectBody(&obj1, "stuff"), resource.TestCheckResourceAttr(resourceName, "tags.%", "3"), @@ -991,7 +992,7 @@ func TestAccS3Object_tagsLeadingSingleSlash(t *testing.T) { }, { Config: testAccObjectConfig_updatedTags(rName, key, "stuff"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj2), testAccCheckObjectVersionIDEquals(&obj2, &obj1), testAccCheckObjectBody(&obj2, "stuff"), @@ -1004,7 +1005,7 @@ func TestAccS3Object_tagsLeadingSingleSlash(t *testing.T) { }, { Config: testAccObjectConfig_noTags(rName, key, "stuff"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj3), testAccCheckObjectVersionIDEquals(&obj3, &obj2), testAccCheckObjectBody(&obj3, "stuff"), @@ -1013,7 +1014,7 @@ func TestAccS3Object_tagsLeadingSingleSlash(t *testing.T) { }, { Config: testAccObjectConfig_tags(rName, key, "changed stuff"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj4), testAccCheckObjectVersionIDDiffers(&obj4, &obj3), testAccCheckObjectBody(&obj4, "changed stuff"), @@ -1049,7 +1050,7 @@ func TestAccS3Object_tagsLeadingMultipleSlashes(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccObjectConfig_tags(rName, key, "stuff"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj1), testAccCheckObjectBody(&obj1, "stuff"), resource.TestCheckResourceAttr(resourceName, "tags.%", "3"), @@ -1060,7 +1061,7 @@ func TestAccS3Object_tagsLeadingMultipleSlashes(t *testing.T) { }, { Config: testAccObjectConfig_updatedTags(rName, key, "stuff"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj2), testAccCheckObjectVersionIDEquals(&obj2, &obj1), testAccCheckObjectBody(&obj2, "stuff"), @@ -1073,7 +1074,7 @@ func TestAccS3Object_tagsLeadingMultipleSlashes(t *testing.T) { }, { Config: testAccObjectConfig_noTags(rName, key, "stuff"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj3), testAccCheckObjectVersionIDEquals(&obj3, &obj2), testAccCheckObjectBody(&obj3, "stuff"), @@ -1082,7 +1083,7 @@ func TestAccS3Object_tagsLeadingMultipleSlashes(t *testing.T) { }, { Config: testAccObjectConfig_tags(rName, key, "changed stuff"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj4), testAccCheckObjectVersionIDDiffers(&obj4, &obj3), testAccCheckObjectBody(&obj4, "changed stuff"), @@ -1111,7 +1112,7 @@ func TestAccS3Object_tagsMultipleSlashes(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccObjectConfig_tags(rName, key, "stuff"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj1), testAccCheckObjectBody(&obj1, "stuff"), resource.TestCheckResourceAttr(resourceName, "tags.%", "3"), @@ -1122,7 +1123,7 @@ func TestAccS3Object_tagsMultipleSlashes(t *testing.T) { }, { Config: testAccObjectConfig_updatedTags(rName, key, "stuff"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj2), testAccCheckObjectVersionIDEquals(&obj2, &obj1), testAccCheckObjectBody(&obj2, "stuff"), @@ -1135,7 +1136,7 @@ func TestAccS3Object_tagsMultipleSlashes(t *testing.T) { }, { Config: testAccObjectConfig_noTags(rName, key, "stuff"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj3), testAccCheckObjectVersionIDEquals(&obj3, &obj2), testAccCheckObjectBody(&obj3, "stuff"), @@ -1144,7 +1145,7 @@ func TestAccS3Object_tagsMultipleSlashes(t *testing.T) { }, { Config: testAccObjectConfig_tags(rName, key, "changed stuff"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj4), testAccCheckObjectVersionIDDiffers(&obj4, &obj3), testAccCheckObjectBody(&obj4, "changed stuff"), @@ -1158,6 +1159,119 @@ func TestAccS3Object_tagsMultipleSlashes(t *testing.T) { }) } +func TestAccS3Object_Tags_EmptyTag_OnCreate(t *testing.T) { + ctx := acctest.Context(t) + var obj s3.GetObjectOutput + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + key := "test-key" + resourceName := "aws_s3_object.object" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, names.S3EndpointID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckBucketDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccObjectConfig_tags1(rName, key, "key1", ""), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckObjectExists(ctx, resourceName, &obj), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(resourceName, "tags.key1", ""), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"force_destroy"}, + ImportStateId: fmt.Sprintf("s3://%s/%s", rName, key), + }, + }, + }) +} + +func TestAccS3Object_Tags_EmptyTag_OnUpdate_Add(t *testing.T) { + ctx := acctest.Context(t) + var obj s3.GetObjectOutput + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + key := "test-key" + resourceName := "aws_s3_object.object" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, names.S3EndpointID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckBucketDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccObjectConfig_tags1(rName, key, "key1", "value1"), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckObjectExists(ctx, resourceName, &obj), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1"), + ), + }, + { + Config: testAccObjectConfig_tags2(rName, key, "key1", "value1", "key2", ""), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckObjectExists(ctx, resourceName, &obj), + resource.TestCheckResourceAttr(resourceName, "tags.%", "2"), + resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1"), + resource.TestCheckResourceAttr(resourceName, "tags.key2", ""), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"force_destroy"}, + ImportStateId: fmt.Sprintf("s3://%s/%s", rName, key), + }, + }, + }) +} + +func TestAccS3Object_Tags_EmptyTag_OnUpdate_Replace(t *testing.T) { + ctx := acctest.Context(t) + var obj s3.GetObjectOutput + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + key := "test-key" + resourceName := "aws_s3_object.object" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, names.S3EndpointID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckBucketDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccObjectConfig_tags1(rName, key, "key1", "value1"), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckObjectExists(ctx, resourceName, &obj), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1"), + ), + }, + { + Config: testAccObjectConfig_tags1(rName, key, "key1", ""), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckObjectExists(ctx, resourceName, &obj), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(resourceName, "tags.key1", ""), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"force_destroy"}, + ImportStateId: fmt.Sprintf("s3://%s/%s", rName, key), + }, + }, + }) +} + func TestAccS3Object_DefaultTags_providerOnly(t *testing.T) { ctx := acctest.Context(t) var obj s3.GetObjectOutput @@ -1175,7 +1289,7 @@ func TestAccS3Object_DefaultTags_providerOnly(t *testing.T) { acctest.ConfigDefaultTags_Tags1("providerkey1", "providervalue1"), testAccObjectConfig_basic(rName), ), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj), resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), resource.TestCheckResourceAttr(resourceName, "tags_all.%", "1"), @@ -1211,7 +1325,7 @@ func TestAccS3Object_DefaultTags_providerAndResource(t *testing.T) { acctest.ConfigDefaultTags_Tags1("providerkey1", "providervalue1"), testAccObjectConfig_tags(rName, key, "stuff"), ), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj), resource.TestCheckResourceAttr(resourceName, "tags.%", "3"), resource.TestCheckResourceAttr(resourceName, "tags.Key1", "A@AA"), @@ -1229,7 +1343,7 @@ func TestAccS3Object_DefaultTags_providerAndResource(t *testing.T) { acctest.ConfigDefaultTags_Tags1("providerkey1", "providervalue1"), testAccObjectConfig_updatedTags(rName, key, "stuff"), ), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj), resource.TestCheckResourceAttr(resourceName, "tags.%", "4"), resource.TestCheckResourceAttr(resourceName, "tags.Key2", "B@BB"), @@ -1266,7 +1380,7 @@ func TestAccS3Object_DefaultTags_providerAndResourceWithOverride(t *testing.T) { acctest.ConfigDefaultTags_Tags1("providerkey1", "providervalue1"), testAccObjectConfig_tagsWithOverride(rName, key, "stuff"), ), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj), resource.TestCheckResourceAttr(resourceName, "tags.%", "3"), resource.TestCheckResourceAttr(resourceName, "tags.Key1", "A@AA"), @@ -1283,7 +1397,7 @@ func TestAccS3Object_DefaultTags_providerAndResourceWithOverride(t *testing.T) { acctest.ConfigDefaultTags_Tags1("providerkey1", "providervalue1"), testAccObjectConfig_updatedTagsWithOverride(rName, key, "stuff"), ), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj), resource.TestCheckResourceAttr(resourceName, "tags.%", "4"), resource.TestCheckResourceAttr(resourceName, "tags.Key2", "B@BB"), @@ -1315,7 +1429,7 @@ func TestAccS3Object_objectLockLegalHoldStartWithNone(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccObjectConfig_noLockLegalHold(rName, "stuff"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj1), testAccCheckObjectBody(&obj1, "stuff"), resource.TestCheckResourceAttr(resourceName, "object_lock_legal_hold_status", ""), @@ -1325,7 +1439,7 @@ func TestAccS3Object_objectLockLegalHoldStartWithNone(t *testing.T) { }, { Config: testAccObjectConfig_lockLegalHold(rName, "stuff", "ON"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj2), testAccCheckObjectVersionIDEquals(&obj2, &obj1), testAccCheckObjectBody(&obj2, "stuff"), @@ -1337,7 +1451,7 @@ func TestAccS3Object_objectLockLegalHoldStartWithNone(t *testing.T) { // Remove legal hold but create a new object version to test force_destroy { Config: testAccObjectConfig_lockLegalHold(rName, "changed stuff", "OFF"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj3), testAccCheckObjectVersionIDDiffers(&obj3, &obj2), testAccCheckObjectBody(&obj3, "changed stuff"), @@ -1364,7 +1478,7 @@ func TestAccS3Object_objectLockLegalHoldStartWithOn(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccObjectConfig_lockLegalHold(rName, "stuff", "ON"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj1), testAccCheckObjectBody(&obj1, "stuff"), resource.TestCheckResourceAttr(resourceName, "object_lock_legal_hold_status", "ON"), @@ -1374,7 +1488,7 @@ func TestAccS3Object_objectLockLegalHoldStartWithOn(t *testing.T) { }, { Config: testAccObjectConfig_lockLegalHold(rName, "stuff", "OFF"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj2), testAccCheckObjectVersionIDEquals(&obj2, &obj1), testAccCheckObjectBody(&obj2, "stuff"), @@ -1402,7 +1516,7 @@ func TestAccS3Object_objectLockRetentionStartWithNone(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccObjectConfig_noLockRetention(rName, "stuff"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj1), testAccCheckObjectBody(&obj1, "stuff"), resource.TestCheckResourceAttr(resourceName, "object_lock_legal_hold_status", ""), @@ -1412,7 +1526,7 @@ func TestAccS3Object_objectLockRetentionStartWithNone(t *testing.T) { }, { Config: testAccObjectConfig_lockRetention(rName, "stuff", retainUntilDate), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj2), testAccCheckObjectVersionIDEquals(&obj2, &obj1), testAccCheckObjectBody(&obj2, "stuff"), @@ -1424,7 +1538,7 @@ func TestAccS3Object_objectLockRetentionStartWithNone(t *testing.T) { // Remove retention period but create a new object version to test force_destroy { Config: testAccObjectConfig_noLockRetention(rName, "changed stuff"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj3), testAccCheckObjectVersionIDDiffers(&obj3, &obj2), testAccCheckObjectBody(&obj3, "changed stuff"), @@ -1454,7 +1568,7 @@ func TestAccS3Object_objectLockRetentionStartWithSet(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccObjectConfig_lockRetention(rName, "stuff", retainUntilDate1), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj1), testAccCheckObjectBody(&obj1, "stuff"), resource.TestCheckResourceAttr(resourceName, "object_lock_legal_hold_status", ""), @@ -1464,7 +1578,7 @@ func TestAccS3Object_objectLockRetentionStartWithSet(t *testing.T) { }, { Config: testAccObjectConfig_lockRetention(rName, "stuff", retainUntilDate2), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj2), testAccCheckObjectVersionIDEquals(&obj2, &obj1), testAccCheckObjectBody(&obj2, "stuff"), @@ -1475,7 +1589,7 @@ func TestAccS3Object_objectLockRetentionStartWithSet(t *testing.T) { }, { Config: testAccObjectConfig_lockRetention(rName, "stuff", retainUntilDate3), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj3), testAccCheckObjectVersionIDEquals(&obj3, &obj2), testAccCheckObjectBody(&obj3, "stuff"), @@ -1486,7 +1600,7 @@ func TestAccS3Object_objectLockRetentionStartWithSet(t *testing.T) { }, { Config: testAccObjectConfig_noLockRetention(rName, "stuff"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj4), testAccCheckObjectVersionIDEquals(&obj4, &obj3), testAccCheckObjectBody(&obj4, "stuff"), @@ -1513,7 +1627,7 @@ func TestAccS3Object_objectBucketKeyEnabled(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccObjectConfig_bucketKeyEnabled(rName, "stuff"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj), testAccCheckObjectBody(&obj, "stuff"), resource.TestCheckResourceAttr(resourceName, "bucket_key_enabled", "true"), @@ -1537,7 +1651,7 @@ func TestAccS3Object_bucketBucketKeyEnabled(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccObjectConfig_bucketBucketKeyEnabled(rName, "stuff"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj), testAccCheckObjectBody(&obj, "stuff"), resource.TestCheckResourceAttr(resourceName, "bucket_key_enabled", "true"), @@ -1561,7 +1675,7 @@ func TestAccS3Object_defaultBucketSSE(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccObjectConfig_defaultBucketSSE(rName, "stuff"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj1), testAccCheckObjectBody(&obj1, "stuff"), ), @@ -1587,7 +1701,7 @@ func TestAccS3Object_ignoreTags(t *testing.T) { Config: acctest.ConfigCompose( acctest.ConfigIgnoreTagsKeyPrefixes1("ignorekey"), testAccObjectConfig_noTags(rName, key, "stuff")), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj), testAccCheckObjectBody(&obj, "stuff"), testAccCheckObjectUpdateTags(ctx, resourceName, nil, map[string]string{"ignorekey1": "ignorevalue1"}), @@ -1601,7 +1715,7 @@ func TestAccS3Object_ignoreTags(t *testing.T) { Config: acctest.ConfigCompose( acctest.ConfigIgnoreTagsKeyPrefixes1("ignorekey"), testAccObjectConfig_tags(rName, key, "stuff")), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj), testAccCheckObjectBody(&obj, "stuff"), resource.TestCheckResourceAttr(resourceName, "tags.%", "3"), @@ -1634,7 +1748,7 @@ func TestAccS3Object_checksumAlgorithm(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccObjectConfig_checksumAlgorithm(rName, "CRC32"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj), testAccCheckObjectBody(&obj, "ABCDEFGHIJKLMNOPQRSTUVWXYZ"), resource.TestCheckResourceAttr(resourceName, "checksum_algorithm", "CRC32"), @@ -1653,7 +1767,7 @@ func TestAccS3Object_checksumAlgorithm(t *testing.T) { }, { Config: testAccObjectConfig_checksumAlgorithm(rName, "SHA256"), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj), testAccCheckObjectBody(&obj, "ABCDEFGHIJKLMNOPQRSTUVWXYZ"), resource.TestCheckResourceAttr(resourceName, "checksum_algorithm", "SHA256"), @@ -1687,7 +1801,7 @@ func TestAccS3Object_keyWithSlashesMigrated(t *testing.T) { }, }, Config: testAccObjectConfig_keyWithSlashes(rName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj), resource.TestCheckResourceAttr(resourceName, "bucket", rName), resource.TestCheckResourceAttr(resourceName, "key", "/a/b//c///d/////e/"), @@ -1787,7 +1901,7 @@ func TestAccS3Object_DirectoryBucket_disappears(t *testing.T) { // nosemgrep:ci. Steps: []resource.TestStep{ { Config: testAccObjectConfig_directoryBucket(rName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj), acctest.CheckResourceDisappears(ctx, acctest.Provider, tfs3.ResourceObject(), resourceName), ), @@ -1814,7 +1928,7 @@ func TestAccS3Object_DirectoryBucket_DefaultTags_providerOnly(t *testing.T) { acctest.ConfigDefaultTags_Tags1("providerkey1", "providervalue1"), testAccObjectConfig_directoryBucket(rName), ), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj), ), }, @@ -1837,7 +1951,7 @@ func TestAccS3Object_prefix(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccObjectConfig_prefix(rName), - Check: resource.ComposeTestCheckFunc( + Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj), resource.TestCheckResourceAttr(resourceName, "content_type", "application/x-directory"), resource.TestCheckResourceAttr(resourceName, "key", "pfx/"), @@ -2566,6 +2680,62 @@ resource "aws_s3_object" "object" { `, rName, key, content) } +func testAccObjectConfig_tags1(rName, key, tagKey1, tagValue1 string) string { + return fmt.Sprintf(` +resource "aws_s3_bucket" "test" { + bucket = %[1]q +} + +resource "aws_s3_bucket_versioning" "test" { + bucket = aws_s3_bucket.test.id + versioning_configuration { + status = "Enabled" + } +} + +resource "aws_s3_object" "object" { + # Must have bucket versioning enabled first + bucket = aws_s3_bucket_versioning.test.bucket +// key = %[2]q +// content = %[3]q + key = %[2]q +// content = "test-content" + + tags = { + %[3]q = %[4]q + } +} +`, rName, key, tagKey1, tagValue1) +} + +func testAccObjectConfig_tags2(rName, key, tagKey1, tagValue1, tagKey2, tagValue2 string) string { + return fmt.Sprintf(` +resource "aws_s3_bucket" "test" { + bucket = %[1]q +} + +resource "aws_s3_bucket_versioning" "test" { + bucket = aws_s3_bucket.test.id + versioning_configuration { + status = "Enabled" + } +} + +resource "aws_s3_object" "object" { + # Must have bucket versioning enabled first + bucket = aws_s3_bucket_versioning.test.bucket +// key = %[2]q +// content = %[3]q +key = %[2]q + + tags = { + %[3]q = %[4]q + %[5]q = %[6]q + } +} +`, rName, key, tagKey1, tagValue1, tagKey2, tagValue2) +} + func testAccObjectConfig_metadata(rName string, metadataKey1, metadataValue1, metadataKey2, metadataValue2 string) string { return fmt.Sprintf(` resource "aws_s3_bucket" "test" { diff --git a/website/docs/d/s3_object.html.markdown b/website/docs/d/s3_object.html.markdown index 65a8d0df4814..e84ddd6842db 100644 --- a/website/docs/d/s3_object.html.markdown +++ b/website/docs/d/s3_object.html.markdown @@ -66,6 +66,7 @@ This data source supports the following arguments: This data source exports the following attributes in addition to the arguments above: +* `arn` - ARN of the object. * `body` - Object data (see **limitations above** to understand cases in which this field is actually available) * `bucket_key_enabled` - (Optional) Whether or not to use [Amazon S3 Bucket Keys](https://docs.aws.amazon.com/AmazonS3/latest/dev/bucket-key.html) for SSE-KMS. * `cache_control` - Caching behavior along the request/reply chain. diff --git a/website/docs/r/s3_bucket_object.html.markdown b/website/docs/r/s3_bucket_object.html.markdown index dd18f3e4469f..bf4f8ccc8350 100644 --- a/website/docs/r/s3_bucket_object.html.markdown +++ b/website/docs/r/s3_bucket_object.html.markdown @@ -173,8 +173,8 @@ If no content is provided through `source`, `content` or `content_base64`, then This resource exports the following attributes in addition to the arguments above: +* `arn` - ARN of the object. * `etag` - ETag generated for the object (an MD5 sum of the object content). For plaintext objects or objects encrypted with an AWS-managed key, the hash is an MD5 digest of the object data. For objects encrypted with a KMS key or objects created by either the Multipart Upload or Part Copy operation, the hash is not an MD5 digest, regardless of the method of encryption. More information on possible values can be found on [Common Response Headers](https://docs.aws.amazon.com/AmazonS3/latest/API/RESTCommonResponseHeaders.html). -* `id` - `key` of the resource supplied above * `tags_all` - Map of tags assigned to the resource, including those inherited from the provider [`default_tags` configuration block](https://registry.terraform.io/providers/hashicorp/aws/latest/docs#default_tags-configuration-block). * `version_id` - Unique version ID value for the object, if bucket versioning is enabled. diff --git a/website/docs/r/s3_object.html.markdown b/website/docs/r/s3_object.html.markdown index 152fd236427a..95a525dde04b 100644 --- a/website/docs/r/s3_object.html.markdown +++ b/website/docs/r/s3_object.html.markdown @@ -208,12 +208,12 @@ The `override_provider` block supports the following: This resource exports the following attributes in addition to the arguments above: +* `arn` - ARN of the object. * `checksum_crc32` - The base64-encoded, 32-bit CRC32 checksum of the object. * `checksum_crc32c` - The base64-encoded, 32-bit CRC32C checksum of the object. * `checksum_sha1` - The base64-encoded, 160-bit SHA-1 digest of the object. * `checksum_sha256` - The base64-encoded, 256-bit SHA-256 digest of the object. * `etag` - ETag generated for the object (an MD5 sum of the object content). For plaintext objects or objects encrypted with an AWS-managed key, the hash is an MD5 digest of the object data. For objects encrypted with a KMS key or objects created by either the Multipart Upload or Part Copy operation, the hash is not an MD5 digest, regardless of the method of encryption. More information on possible values can be found on [Common Response Headers](https://docs.aws.amazon.com/AmazonS3/latest/API/RESTCommonResponseHeaders.html). -* `id` - `key` of the resource supplied above * `tags_all` - Map of tags assigned to the resource, including those inherited from the provider [`default_tags` configuration block](https://registry.terraform.io/providers/hashicorp/aws/latest/docs#default_tags-configuration-block). * `version_id` - Unique version ID value for the object, if bucket versioning is enabled. diff --git a/website/docs/r/s3_object_copy.html.markdown b/website/docs/r/s3_object_copy.html.markdown index bb89f7975850..15bc54db0708 100644 --- a/website/docs/r/s3_object_copy.html.markdown +++ b/website/docs/r/s3_object_copy.html.markdown @@ -93,13 +93,13 @@ This configuration block has the following optional arguments (one of the three This resource exports the following attributes in addition to the arguments above: +* `arn` - ARN of the object. * `checksum_crc32` - The base64-encoded, 32-bit CRC32 checksum of the object. * `checksum_crc32c` - The base64-encoded, 32-bit CRC32C checksum of the object. * `checksum_sha1` - The base64-encoded, 160-bit SHA-1 digest of the object. * `checksum_sha256` - The base64-encoded, 256-bit SHA-256 digest of the object. * `etag` - ETag generated for the object (an MD5 sum of the object content). For plaintext objects or objects encrypted with an AWS-managed key, the hash is an MD5 digest of the object data. For objects encrypted with a KMS key or objects created by either the Multipart Upload or Part Copy operation, the hash is not an MD5 digest, regardless of the method of encryption. More information on possible values can be found on [Common Response Headers](https://docs.aws.amazon.com/AmazonS3/latest/API/RESTCommonResponseHeaders.html). * `expiration` - If the object expiration is configured, this attribute will be set. -* `id` - The `key` of the resource supplied above. * `last_modified` - Returns the date that the object was last modified, in [RFC3339 format](https://tools.ietf.org/html/rfc3339#section-5.8). * `request_charged` - If present, indicates that the requester was successfully charged for the request. * `source_version_id` - Version of the copied object in the source bucket. From fc999974146c9f2b810b75ff588ad5f9b7a1c5b7 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Fri, 9 Feb 2024 14:17:00 -0800 Subject: [PATCH 07/24] Implements transparent tagging for `aws_s3_object` --- internal/service/s3/object.go | 58 +++++++++++++--------- internal/service/s3/object_test.go | 6 +-- internal/service/s3/service_package_gen.go | 5 +- internal/service/s3/tags.go | 32 ++++++++---- 4 files changed, 65 insertions(+), 36 deletions(-) diff --git a/internal/service/s3/object.go b/internal/service/s3/object.go index 4fd8315fa159..b3cdb004c507 100644 --- a/internal/service/s3/object.go +++ b/internal/service/s3/object.go @@ -40,7 +40,7 @@ import ( ) // @SDKResource("aws_s3_object", name="Object") -// @Tags +// @Tags(identifierAttribute="arn", resourceType="Object") func resourceObject() *schema.Resource { return &schema.Resource{ CreateWithoutTimeout: resourceObjectCreate, @@ -313,12 +313,6 @@ func resourceObjectRead(ctx context.Context, d *schema.ResourceData, meta interf return sdkdiag.AppendFromErr(diags, err) } - if tags, err := objectListTags(ctx, conn, bucket, key, optFns...); err == nil { - setTagsOut(ctx, Tags(tags)) - } else if !tfawserr.ErrHTTPStatusCodeEquals(err, http.StatusNotImplemented) { // Directory buckets return HTTP status code 501, NotImplemented. - return sdkdiag.AppendErrorf(diags, "listing tags for S3 Bucket (%s) Object (%s): %s", bucket, key, err) - } - return diags } @@ -398,14 +392,6 @@ func resourceObjectUpdate(ctx context.Context, d *schema.ResourceData, meta inte } } - if d.HasChange("tags_all") { - o, n := d.GetChange("tags_all") - - if err := objectUpdateTags(ctx, conn, bucket, key, o, n, optFns...); err != nil { - return sdkdiag.AppendErrorf(diags, "updating tags: %s", err) - } - } - return append(diags, resourceObjectRead(ctx, d, meta)...) } @@ -461,7 +447,6 @@ func resourceObjectUpload(ctx context.Context, d *schema.ResourceData, meta inte var diags diag.Diagnostics conn := meta.(*conns.AWSClient).S3Client(ctx) var optFns []func(*s3.Options) - defaultTagsConfig := meta.(*conns.AWSClient).DefaultTagsConfig bucket := d.Get("bucket").(string) if isDirectoryBucket(bucket) { @@ -472,13 +457,6 @@ func resourceObjectUpload(ctx context.Context, d *schema.ResourceData, meta inte optFns = append(optFns, func(o *s3.Options) { o.UseARNRegion = true }) } - tags := tftags.New(ctx, d.Get("tags").(map[string]interface{})) - if ignoreProviderDefaultTags(ctx, d) { - tags = tags.RemoveDefaultConfig(defaultTagsConfig) - } else { - tags = defaultTagsConfig.MergeTags(tftags.New(ctx, tags)) - } - var body io.ReadSeeker if v, ok := d.GetOk("source"); ok { @@ -582,6 +560,14 @@ func resourceObjectUpload(ctx context.Context, d *schema.ResourceData, meta inte input.StorageClass = types.StorageClass(v.(string)) } + defaultTagsConfig := meta.(*conns.AWSClient).DefaultTagsConfig + tags := tftags.New(ctx, getContextTags(ctx)) + if ignoreProviderDefaultTags(ctx, d) { + tags = tags.RemoveDefaultConfig(defaultTagsConfig) + } else { + tags = defaultTagsConfig.MergeTags(tftags.New(ctx, tags)) + } + if len(tags) > 0 { // The tag-set must be encoded as URL Query parameters. input.Tagging = aws.String(tags.IgnoreAWS().URLEncode()) @@ -802,3 +788,29 @@ func newObjectARN(partition string, bucket, key string) arn.ARN { Resource: fmt.Sprintf("%s/%s", bucket, key), } } + +type objectARN struct { + arn.ARN + Bucket string + Key string +} + +func parseObjectARN(s string) (objectARN, error) { + arn, err := arn.Parse(s) + if err != nil { + return objectARN{}, err + } + + result := objectARN{ + ARN: arn, + } + + parts := strings.SplitN(arn.Resource, "/", 2) + if len(parts) != 2 { + return objectARN{}, fmt.Errorf("S3 Object ARN: unexpected resource section: %s", arn.Resource) + } + result.Bucket = parts[0] + result.Key = parts[1] + + return result, nil +} diff --git a/internal/service/s3/object_test.go b/internal/service/s3/object_test.go index 1eb347aa7ece..c8669aeeb4c1 100644 --- a/internal/service/s3/object_test.go +++ b/internal/service/s3/object_test.go @@ -1159,7 +1159,7 @@ func TestAccS3Object_tagsMultipleSlashes(t *testing.T) { }) } -func TestAccS3Object_Tags_EmptyTag_OnCreate(t *testing.T) { +func TestAccS3Object_tags_EmptyTag_OnCreate(t *testing.T) { ctx := acctest.Context(t) var obj s3.GetObjectOutput rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) @@ -1191,7 +1191,7 @@ func TestAccS3Object_Tags_EmptyTag_OnCreate(t *testing.T) { }) } -func TestAccS3Object_Tags_EmptyTag_OnUpdate_Add(t *testing.T) { +func TestAccS3Object_tags_EmptyTag_OnUpdate_Add(t *testing.T) { ctx := acctest.Context(t) var obj s3.GetObjectOutput rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) @@ -1232,7 +1232,7 @@ func TestAccS3Object_Tags_EmptyTag_OnUpdate_Add(t *testing.T) { }) } -func TestAccS3Object_Tags_EmptyTag_OnUpdate_Replace(t *testing.T) { +func TestAccS3Object_tags_EmptyTag_OnUpdate_Replace(t *testing.T) { ctx := acctest.Context(t) var obj s3.GetObjectOutput rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) diff --git a/internal/service/s3/service_package_gen.go b/internal/service/s3/service_package_gen.go index 935a4d2c157b..35bca9c8cda8 100644 --- a/internal/service/s3/service_package_gen.go +++ b/internal/service/s3/service_package_gen.go @@ -186,7 +186,10 @@ func (p *servicePackage) SDKResources(ctx context.Context) []*types.ServicePacka Factory: resourceObject, TypeName: "aws_s3_object", Name: "Object", - Tags: &types.ServicePackageResourceTags{}, + Tags: &types.ServicePackageResourceTags{ + IdentifierAttribute: "arn", + ResourceType: "Object", + }, }, { Factory: resourceObjectCopy, diff --git a/internal/service/s3/tags.go b/internal/service/s3/tags.go index 73c22b05b7ea..d38ae9a9c7b8 100644 --- a/internal/service/s3/tags.go +++ b/internal/service/s3/tags.go @@ -14,7 +14,6 @@ import ( "github.com/aws/aws-sdk-go-v2/service/s3" awstypes "github.com/aws/aws-sdk-go-v2/service/s3/types" "github.com/hashicorp/aws-sdk-go-base/v2/tfawserr" - "github.com/hashicorp/terraform-plugin-log/tflog" "github.com/hashicorp/terraform-provider-aws/internal/conns" tftags "github.com/hashicorp/terraform-provider-aws/internal/tags" "github.com/hashicorp/terraform-provider-aws/internal/types/option" @@ -171,11 +170,14 @@ func (p *servicePackage) ListTags(ctx context.Context, meta any, identifier, res case "Bucket": tags, err = bucketListTags(ctx, meta.(*conns.AWSClient).S3Client(ctx), identifier) + case "Object": + objectARN, err := parseObjectARN(identifier) + if err != nil { + return err + } + tags, err = objectListTags(ctx, meta.(*conns.AWSClient).S3Client(ctx), objectARN.Bucket, objectARN.Key) + default: - tflog.Warn(ctx, "ListTags not implemented for resource type", map[string]any{ - "service_package": p.ServicePackageName(), - "resource_name": resourceType, - }) return nil } @@ -196,10 +198,22 @@ func (p *servicePackage) UpdateTags(ctx context.Context, meta any, identifier, r switch resourceType { case "Bucket": return bucketUpdateTags(ctx, meta.(*conns.AWSClient).S3Client(ctx), identifier, oldTags, newTags) + + case "Object": + objectARN, err := parseObjectARN(identifier) + if err != nil { + return err + } + return objectUpdateTags(ctx, meta.(*conns.AWSClient).S3Client(ctx), objectARN.Bucket, objectARN.Key, oldTags, newTags) + + default: + return nil + } +} + +func getContextTags(ctx context.Context) tftags.KeyValueTags { + if inContext, ok := tftags.FromContext(ctx); ok { + return inContext.TagsIn.UnwrapOrDefault() } - tflog.Warn(ctx, "UpdateTags not implemented for resource type", map[string]any{ - "service_package": p.ServicePackageName(), - "resource_name": resourceType, - }) return nil } From 9827a8f1dee20b0d38a7f3f0a859c10cc2b341c8 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Fri, 9 Feb 2024 15:24:23 -0800 Subject: [PATCH 08/24] Removes explicit tag read from `aws_s3_bucket` --- internal/service/s3/bucket.go | 21 --------------------- internal/service/s3/errors.go | 1 - internal/service/s3/tags.go | 8 ++------ 3 files changed, 2 insertions(+), 28 deletions(-) diff --git a/internal/service/s3/bucket.go b/internal/service/s3/bucket.go index acb38a4e7def..dfd7f520f10b 100644 --- a/internal/service/s3/bucket.go +++ b/internal/service/s3/bucket.go @@ -1142,27 +1142,6 @@ func resourceBucketRead(ctx context.Context, d *schema.ResourceData, meta interf d.Set("website_endpoint", endpoint) } - // - // Bucket Tags. - // - tags, err := retryWhenNoSuchBucketError(ctx, d.Timeout(schema.TimeoutRead), func() (tftags.KeyValueTags, error) { - return bucketListTags(ctx, conn, d.Id()) - }) - - if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, errCodeNoSuchBucket) { - log.Printf("[WARN] S3 Bucket (%s) not found, removing from state", d.Id()) - d.SetId("") - return diags - } - - switch { - case err == nil: - setTagsOut(ctx, Tags(tags)) - case tfawserr.ErrCodeEquals(err, errCodeMethodNotAllowed, errCodeNotImplemented, errCodeXNotImplemented): - default: - return sdkdiag.AppendErrorf(diags, "listing tags for S3 Bucket (%s): %s", d.Id(), err) - } - return diags } diff --git a/internal/service/s3/errors.go b/internal/service/s3/errors.go index 97863903404c..6e331c25e288 100644 --- a/internal/service/s3/errors.go +++ b/internal/service/s3/errors.go @@ -29,7 +29,6 @@ const ( errCodeNoSuchKey = "NoSuchKey" errCodeNoSuchPublicAccessBlockConfiguration = "NoSuchPublicAccessBlockConfiguration" errCodeNoSuchTagSet = "NoSuchTagSet" - errCodeNoSuchTagSetError = "NoSuchTagSetError" errCodeNoSuchWebsiteConfiguration = "NoSuchWebsiteConfiguration" errCodeNotImplemented = "NotImplemented" // errCodeObjectLockConfigurationNotFound should be used with tfawserr.ErrCodeContains, not tfawserr.ErrCodeEquals. diff --git a/internal/service/s3/tags.go b/internal/service/s3/tags.go index d38ae9a9c7b8..bf5e3cfdb212 100644 --- a/internal/service/s3/tags.go +++ b/internal/service/s3/tags.go @@ -38,13 +38,9 @@ func bucketListTags(ctx context.Context, conn *s3.Client, identifier string, opt output, err := conn.GetBucketTagging(ctx, input, optFns...) - // 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.ErrCodeEquals(err, errCodeNoSuchTagSet, errCodeMethodNotAllowed, errCodeNotImplemented, errCodeXNotImplemented) { return tftags.New(ctx, nil), nil } - if err != nil { return tftags.New(ctx, nil), err } @@ -104,7 +100,7 @@ func objectListTags(ctx context.Context, conn *s3.Client, bucket, key string, op output, err := conn.GetObjectTagging(ctx, input, optFns...) - if tfawserr.ErrCodeEquals(err, errCodeNoSuchTagSet, errCodeNoSuchTagSetError) { + if tfawserr.ErrCodeEquals(err, errCodeNoSuchTagSet) { return tftags.New(ctx, nil), nil } From c31554215d1e096877b828a67a5640c0a79e4892 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Fri, 9 Feb 2024 15:39:04 -0800 Subject: [PATCH 09/24] Implements transparent tagging for `aws_s3_object_copy` --- internal/service/s3/object_copy.go | 17 +--- internal/service/s3/object_copy_test.go | 95 ++++++++++++++++++++++ internal/service/s3/service_package_gen.go | 5 +- 3 files changed, 103 insertions(+), 14 deletions(-) diff --git a/internal/service/s3/object_copy.go b/internal/service/s3/object_copy.go index 1813ac03e3e1..059368b84d17 100644 --- a/internal/service/s3/object_copy.go +++ b/internal/service/s3/object_copy.go @@ -8,7 +8,6 @@ import ( "context" "fmt" "log" - "net/http" "net/url" "strings" @@ -16,7 +15,6 @@ import ( "github.com/aws/aws-sdk-go-v2/aws/arn" "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/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" @@ -32,7 +30,7 @@ import ( ) // @SDKResource("aws_s3_object_copy", name="Object Copy") -// @Tags +// @Tags(identifierAttribute="arn", resourceType="ObjectCopy") func resourceObjectCopy() *schema.Resource { return &schema.Resource{ CreateWithoutTimeout: resourceObjectCopyCreate, @@ -398,12 +396,6 @@ func resourceObjectCopyRead(ctx context.Context, d *schema.ResourceData, meta in return sdkdiag.AppendFromErr(diags, err) } - if tags, err := objectListTags(ctx, conn, bucket, key, optFns...); err == nil { - setTagsOut(ctx, Tags(tags)) - } else if !tfawserr.ErrHTTPStatusCodeEquals(err, http.StatusNotImplemented) { // Directory buckets return HTTP status code 501, NotImplemented. - return sdkdiag.AppendErrorf(diags, "listing tags for S3 Bucket (%s) Object (%s): %s", bucket, key, err) - } - return diags } @@ -455,8 +447,6 @@ func resourceObjectCopyUpdate(ctx context.Context, d *schema.ResourceData, meta "source_customer_key_md5", "storage_class", "tagging_directive", - "tags", - "tags_all", "website_redirect", } if d.HasChanges(args...) { @@ -498,7 +488,6 @@ func resourceObjectCopyDoCopy(ctx context.Context, d *schema.ResourceData, meta var diags diag.Diagnostics conn := meta.(*conns.AWSClient).S3Client(ctx) var optFns []func(*s3.Options) - defaultTagsConfig := meta.(*conns.AWSClient).DefaultTagsConfig bucket := d.Get("bucket").(string) if isDirectoryBucket(bucket) { @@ -508,7 +497,6 @@ func resourceObjectCopyDoCopy(ctx context.Context, d *schema.ResourceData, meta if arn.IsARN(bucket) && conn.Options().Region == names.GlobalRegionID { optFns = append(optFns, func(o *s3.Options) { o.UseARNRegion = true }) } - tags := defaultTagsConfig.MergeTags(tftags.New(ctx, d.Get("tags").(map[string]interface{}))) input := &s3.CopyObjectInput{ Bucket: aws.String(bucket), @@ -654,6 +642,9 @@ func resourceObjectCopyDoCopy(ctx context.Context, d *schema.ResourceData, meta input.TaggingDirective = types.TaggingDirective(v.(string)) } + defaultTagsConfig := meta.(*conns.AWSClient).DefaultTagsConfig + tags := tftags.New(ctx, getContextTags(ctx)) + tags = defaultTagsConfig.MergeTags(tags) if len(tags) > 0 { // The tag-set must be encoded as URL Query parameters. input.Tagging = aws.String(tags.IgnoreAWS().URLEncode()) diff --git a/internal/service/s3/object_copy_test.go b/internal/service/s3/object_copy_test.go index 5605e2b6b9d6..540923a31517 100644 --- a/internal/service/s3/object_copy_test.go +++ b/internal/service/s3/object_copy_test.go @@ -167,6 +167,101 @@ func TestAccS3ObjectCopy_tags(t *testing.T) { }) } +func TestAccS3ObjectCopy_tags_EmptyTag_OnCreate(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_tags1(rName1, sourceKey, rName2, targetKey, "key1", ""), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckObjectCopyExists(ctx, resourceName), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(resourceName, "tags.key1", ""), + ), + }, + }, + }) +} + +func TestAccS3ObjectCopy_tags_EmptyTag_OnUpdate_Add(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_tags1(rName1, sourceKey, rName2, targetKey, "key1", "value1"), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckObjectCopyExists(ctx, resourceName), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1"), + ), + }, + { + Config: testAccObjectCopyConfig_tags2(rName1, sourceKey, rName2, targetKey, "key1", "value1", "key2", ""), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckObjectCopyExists(ctx, resourceName), + resource.TestCheckResourceAttr(resourceName, "tags.%", "2"), + resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1"), + resource.TestCheckResourceAttr(resourceName, "tags.key2", ""), + ), + }, + }, + }) +} + +func TestAccS3ObjectCopy_tags_EmptyTag_OnUpdate_Replace(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_tags1(rName1, sourceKey, rName2, targetKey, "key1", "value1"), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckObjectCopyExists(ctx, resourceName), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1"), + ), + }, + { + Config: testAccObjectCopyConfig_tags1(rName1, sourceKey, rName2, targetKey, "key1", ""), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckObjectCopyExists(ctx, resourceName), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(resourceName, "tags.key1", ""), + ), + }, + }, + }) +} + func TestAccS3ObjectCopy_metadata(t *testing.T) { ctx := acctest.Context(t) rName1 := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) diff --git a/internal/service/s3/service_package_gen.go b/internal/service/s3/service_package_gen.go index 35bca9c8cda8..7ed039d0ef54 100644 --- a/internal/service/s3/service_package_gen.go +++ b/internal/service/s3/service_package_gen.go @@ -195,7 +195,10 @@ func (p *servicePackage) SDKResources(ctx context.Context) []*types.ServicePacka Factory: resourceObjectCopy, TypeName: "aws_s3_object_copy", Name: "Object Copy", - Tags: &types.ServicePackageResourceTags{}, + Tags: &types.ServicePackageResourceTags{ + IdentifierAttribute: "arn", + ResourceType: "ObjectCopy", + }, }, } } From 4fc6e9e9d3708f79a32a5651bc9fe65321aeddac Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Fri, 9 Feb 2024 16:13:26 -0800 Subject: [PATCH 10/24] Implements transparent tagging for `aws_s3_bucket_object` --- internal/service/s3/bucket_object.go | 23 +-- .../service/s3/bucket_object_data_source.go | 7 + .../s3/bucket_object_data_source_test.go | 1 + internal/service/s3/bucket_object_test.go | 164 ++++++++++++++++++ internal/service/s3/service_package_gen.go | 5 +- internal/service/s3/tags.go | 4 +- 6 files changed, 182 insertions(+), 22 deletions(-) diff --git a/internal/service/s3/bucket_object.go b/internal/service/s3/bucket_object.go index 190d5c767c34..0d7651472fb3 100644 --- a/internal/service/s3/bucket_object.go +++ b/internal/service/s3/bucket_object.go @@ -38,7 +38,7 @@ import ( ) // @SDKResource("aws_s3_bucket_object", name="Bucket Object") -// @Tags +// @Tags(identifierAttribute="arn", resourceType="BucketObject") func resourceBucketObject() *schema.Resource { return &schema.Resource{ CreateWithoutTimeout: resourceBucketObjectCreate, @@ -253,14 +253,6 @@ func resourceBucketObjectRead(ctx context.Context, d *schema.ResourceData, meta return sdkdiag.AppendFromErr(diags, err) } - 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) - } - - setTagsOut(ctx, Tags(tags)) - return diags } @@ -332,14 +324,6 @@ func resourceBucketObjectUpdate(ctx context.Context, d *schema.ResourceData, met } } - if d.HasChange("tags_all") { - o, n := d.GetChange("tags_all") - - if err := objectUpdateTags(ctx, conn, bucket, key, o, n); err != nil { - return sdkdiag.AppendErrorf(diags, "updating tags: %s", err) - } - } - return append(diags, resourceBucketObjectRead(ctx, d, meta)...) } @@ -387,8 +371,6 @@ func resourceBucketObjectUpload(ctx context.Context, d *schema.ResourceData, met var diags diag.Diagnostics conn := meta.(*conns.AWSClient).S3Client(ctx) uploader := manager.NewUploader(conn) - defaultTagsConfig := meta.(*conns.AWSClient).DefaultTagsConfig - tags := defaultTagsConfig.MergeTags(tftags.New(ctx, d.Get("tags").(map[string]interface{}))) var body io.ReadSeeker @@ -489,6 +471,9 @@ func resourceBucketObjectUpload(ctx context.Context, d *schema.ResourceData, met input.StorageClass = types.StorageClass(v.(string)) } + defaultTagsConfig := meta.(*conns.AWSClient).DefaultTagsConfig + tags := tftags.New(ctx, getContextTags(ctx)) + tags = defaultTagsConfig.MergeTags(tags) if len(tags) > 0 { // The tag-set must be encoded as URL Query parameters. input.Tagging = aws.String(tags.IgnoreAWS().URLEncode()) diff --git a/internal/service/s3/bucket_object_data_source.go b/internal/service/s3/bucket_object_data_source.go index fee9f726c2a6..a6b7bb5e0fdf 100644 --- a/internal/service/s3/bucket_object_data_source.go +++ b/internal/service/s3/bucket_object_data_source.go @@ -29,6 +29,10 @@ func dataSourceBucketObject() *schema.Resource { ReadWithoutTimeout: dataSourceBucketObjectRead, Schema: map[string]*schema.Schema{ + "arn": { + Type: schema.TypeString, + Computed: true, + }, "body": { Type: schema.TypeString, Computed: true, @@ -169,6 +173,9 @@ func dataSourceBucketObjectRead(ctx context.Context, d *schema.ResourceData, met } d.SetId(id) + arn := newObjectARN(meta.(*conns.AWSClient).Partition, bucket, key) + d.Set("arn", arn.String()) + d.Set("bucket_key_enabled", out.BucketKeyEnabled) d.Set("cache_control", out.CacheControl) d.Set("content_disposition", out.ContentDisposition) diff --git a/internal/service/s3/bucket_object_data_source_test.go b/internal/service/s3/bucket_object_data_source_test.go index a2a9d22d5920..e410874e373c 100644 --- a/internal/service/s3/bucket_object_data_source_test.go +++ b/internal/service/s3/bucket_object_data_source_test.go @@ -35,6 +35,7 @@ func TestAccS3BucketObjectDataSource_basic(t *testing.T) { { Config: testAccBucketObjectDataSourceConfig_basic(rInt), Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttrPair(dataSourceName, "arn", resourceName, "arn"), resource.TestCheckResourceAttr(dataSourceName, "content_length", "11"), resource.TestCheckResourceAttrPair(dataSourceName, "content_type", resourceName, "content_type"), resource.TestCheckResourceAttrPair(dataSourceName, "etag", resourceName, "etag"), diff --git a/internal/service/s3/bucket_object_test.go b/internal/service/s3/bucket_object_test.go index 495633b740dc..204ee762b6f8 100644 --- a/internal/service/s3/bucket_object_test.go +++ b/internal/service/s3/bucket_object_test.go @@ -1046,6 +1046,119 @@ func TestAccS3BucketObject_tagsMultipleSlashes(t *testing.T) { }) } +func TestAccS3BucketObject_tags_EmptyTag_OnCreate(t *testing.T) { + ctx := acctest.Context(t) + var obj s3.GetObjectOutput + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + key := "test-key" + resourceName := "aws_s3_bucket_object.object" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, names.S3EndpointID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckBucketObjectDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccBucketObjectConfig_tags1(rName, key, "key1", ""), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckBucketObjectExists(ctx, resourceName, &obj), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(resourceName, "tags.key1", ""), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"acl", "force_destroy"}, + ImportStateId: fmt.Sprintf("s3://%s/%s", rName, key), + }, + }, + }) +} + +func TestAccS3BucketObject_tags_EmptyTag_OnUpdate_Add(t *testing.T) { + ctx := acctest.Context(t) + var obj s3.GetObjectOutput + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + key := "test-key" + resourceName := "aws_s3_bucket_object.object" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, names.S3EndpointID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckBucketObjectDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccBucketObjectConfig_tags1(rName, key, "key1", "value1"), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckBucketObjectExists(ctx, resourceName, &obj), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1"), + ), + }, + { + Config: testAccBucketObjectConfig_tags2(rName, key, "key1", "value1", "key2", ""), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckBucketObjectExists(ctx, resourceName, &obj), + resource.TestCheckResourceAttr(resourceName, "tags.%", "2"), + resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1"), + resource.TestCheckResourceAttr(resourceName, "tags.key2", ""), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"acl", "force_destroy"}, + ImportStateId: fmt.Sprintf("s3://%s/%s", rName, key), + }, + }, + }) +} + +func TestAccS3BucketObject_tags_EmptyTag_OnUpdate_Replace(t *testing.T) { + ctx := acctest.Context(t) + var obj s3.GetObjectOutput + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + key := "test-key" + resourceName := "aws_s3_bucket_object.object" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, names.S3EndpointID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckBucketObjectDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccBucketObjectConfig_tags1(rName, key, "key1", "value1"), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckBucketObjectExists(ctx, resourceName, &obj), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1"), + ), + }, + { + Config: testAccBucketObjectConfig_tags1(rName, key, "key1", ""), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckBucketObjectExists(ctx, resourceName, &obj), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(resourceName, "tags.key1", ""), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"acl", "force_destroy"}, + ImportStateId: fmt.Sprintf("s3://%s/%s", rName, key), + }, + }, + }) +} + func TestAccS3BucketObject_objectLockLegalHoldStartWithNone(t *testing.T) { ctx := acctest.Context(t) var obj1, obj2, obj3 s3.GetObjectOutput @@ -1764,6 +1877,57 @@ resource "aws_s3_bucket_object" "object" { `, rName, key, content) } +func testAccBucketObjectConfig_tags1(rName, key, tagKey1, tagValue1 string) string { + return fmt.Sprintf(` +resource "aws_s3_bucket" "test" { + bucket = %[1]q +} + +resource "aws_s3_bucket_versioning" "test" { + bucket = aws_s3_bucket.test.id + versioning_configuration { + status = "Enabled" + } +} + +resource "aws_s3_bucket_object" "object" { + # Must have bucket versioning enabled first + bucket = aws_s3_bucket_versioning.test.bucket + key = %[2]q + + tags = { + %[3]q = %[4]q + } +} +`, rName, key, tagKey1, tagValue1) +} + +func testAccBucketObjectConfig_tags2(rName, key, tagKey1, tagValue1, tagKey2, tagValue2 string) string { + return fmt.Sprintf(` +resource "aws_s3_bucket" "test" { + bucket = %[1]q +} + +resource "aws_s3_bucket_versioning" "test" { + bucket = aws_s3_bucket.test.id + versioning_configuration { + status = "Enabled" + } +} + +resource "aws_s3_bucket_object" "object" { + # Must have bucket versioning enabled first + bucket = aws_s3_bucket_versioning.test.bucket + key = %[2]q + + tags = { + %[3]q = %[4]q + %[5]q = %[6]q + } +} +`, rName, key, tagKey1, tagValue1, tagKey2, tagValue2) +} + func testAccBucketObjectConfig_metadata(rName string, metadataKey1, metadataValue1, metadataKey2, metadataValue2 string) string { return fmt.Sprintf(` resource "aws_s3_bucket" "test" { diff --git a/internal/service/s3/service_package_gen.go b/internal/service/s3/service_package_gen.go index 7ed039d0ef54..7cf7cd3990ee 100644 --- a/internal/service/s3/service_package_gen.go +++ b/internal/service/s3/service_package_gen.go @@ -135,7 +135,10 @@ func (p *servicePackage) SDKResources(ctx context.Context) []*types.ServicePacka Factory: resourceBucketObject, TypeName: "aws_s3_bucket_object", Name: "Bucket Object", - Tags: &types.ServicePackageResourceTags{}, + Tags: &types.ServicePackageResourceTags{ + IdentifierAttribute: "arn", + ResourceType: "BucketObject", + }, }, { Factory: resourceBucketObjectLockConfiguration, diff --git a/internal/service/s3/tags.go b/internal/service/s3/tags.go index bf5e3cfdb212..5689721f4aa3 100644 --- a/internal/service/s3/tags.go +++ b/internal/service/s3/tags.go @@ -166,7 +166,7 @@ func (p *servicePackage) ListTags(ctx context.Context, meta any, identifier, res case "Bucket": tags, err = bucketListTags(ctx, meta.(*conns.AWSClient).S3Client(ctx), identifier) - case "Object": + case "Object", "ObjectCopy", "BucketObject": objectARN, err := parseObjectARN(identifier) if err != nil { return err @@ -195,7 +195,7 @@ func (p *servicePackage) UpdateTags(ctx context.Context, meta any, identifier, r case "Bucket": return bucketUpdateTags(ctx, meta.(*conns.AWSClient).S3Client(ctx), identifier, oldTags, newTags) - case "Object": + case "Object", "ObjectCopy", "BucketObject": objectARN, err := parseObjectARN(identifier) if err != nil { return err From ced5403eb285a9dc47c5b26cdf34da1ff896f64d Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Fri, 9 Feb 2024 16:28:56 -0800 Subject: [PATCH 11/24] Cleanup --- internal/service/s3/object_test.go | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/internal/service/s3/object_test.go b/internal/service/s3/object_test.go index c8669aeeb4c1..7bc4668dda8c 100644 --- a/internal/service/s3/object_test.go +++ b/internal/service/s3/object_test.go @@ -2695,11 +2695,8 @@ resource "aws_s3_bucket_versioning" "test" { resource "aws_s3_object" "object" { # Must have bucket versioning enabled first - bucket = aws_s3_bucket_versioning.test.bucket -// key = %[2]q -// content = %[3]q + bucket = aws_s3_bucket_versioning.test.bucket key = %[2]q -// content = "test-content" tags = { %[3]q = %[4]q @@ -2723,14 +2720,12 @@ resource "aws_s3_bucket_versioning" "test" { resource "aws_s3_object" "object" { # Must have bucket versioning enabled first - bucket = aws_s3_bucket_versioning.test.bucket -// key = %[2]q -// content = %[3]q -key = %[2]q + bucket = aws_s3_bucket_versioning.test.bucket + key = %[2]q tags = { %[3]q = %[4]q - %[5]q = %[6]q + %[5]q = %[6]q } } `, rName, key, tagKey1, tagValue1, tagKey2, tagValue2) From 4961d64a251113939ee1f62b47233f9951d5cf17 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Mon, 12 Feb 2024 12:19:41 -0800 Subject: [PATCH 12/24] Linting fixes --- internal/service/s3/bucket_object_test.go | 2 +- internal/service/s3/object_copy_test.go | 2 +- internal/service/s3/object_test.go | 2 +- internal/service/s3/tags.go | 3 ++- 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/internal/service/s3/bucket_object_test.go b/internal/service/s3/bucket_object_test.go index 204ee762b6f8..c8780999b512 100644 --- a/internal/service/s3/bucket_object_test.go +++ b/internal/service/s3/bucket_object_test.go @@ -73,7 +73,7 @@ func TestAccS3BucketObject_basic(t *testing.T) { testAccCheckBucketObjectExists(ctx, resourceName, &obj), testAccCheckObjectBody(&obj, ""), resource.TestCheckResourceAttr(resourceName, "acl", string(types.BucketCannedACLPrivate)), - resource.TestCheckResourceAttr(resourceName, "arn", fmt.Sprintf("arn:aws:s3:::%s/test-key", rName)), + acctest.CheckResourceAttrGlobalARNNoAccount(resourceName, "arn", "s3", fmt.Sprintf("%s/test-key", rName)), resource.TestCheckResourceAttr(resourceName, "bucket", rName), resource.TestCheckResourceAttr(resourceName, "bucket_key_enabled", "false"), resource.TestCheckResourceAttr(resourceName, "cache_control", ""), diff --git a/internal/service/s3/object_copy_test.go b/internal/service/s3/object_copy_test.go index 540923a31517..5eb9dcc6a998 100644 --- a/internal/service/s3/object_copy_test.go +++ b/internal/service/s3/object_copy_test.go @@ -40,7 +40,7 @@ func TestAccS3ObjectCopy_basic(t *testing.T) { Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectCopyExists(ctx, resourceName), resource.TestCheckNoResourceAttr(resourceName, "acl"), - resource.TestCheckResourceAttr(resourceName, "arn", fmt.Sprintf("arn:aws:s3:::%s/%s", rNameTarget, targetKey)), + acctest.CheckResourceAttrGlobalARNNoAccount(resourceName, "arn", "s3", fmt.Sprintf("%s/%s", rNameTarget, targetKey)), resource.TestCheckResourceAttr(resourceName, "bucket", rNameTarget), resource.TestCheckResourceAttr(resourceName, "bucket_key_enabled", "false"), resource.TestCheckResourceAttr(resourceName, "cache_control", ""), diff --git a/internal/service/s3/object_test.go b/internal/service/s3/object_test.go index 7bc4668dda8c..6de542a8a391 100644 --- a/internal/service/s3/object_test.go +++ b/internal/service/s3/object_test.go @@ -125,7 +125,7 @@ func TestAccS3Object_basic(t *testing.T) { testAccCheckObjectExists(ctx, resourceName, &obj), testAccCheckObjectBody(&obj, ""), resource.TestCheckNoResourceAttr(resourceName, "acl"), - resource.TestCheckResourceAttr(resourceName, "arn", fmt.Sprintf("arn:aws:s3:::%s/test-key", rName)), + acctest.CheckResourceAttrGlobalARNNoAccount(resourceName, "arn", "s3", fmt.Sprintf("%s/test-key", rName)), resource.TestCheckResourceAttr(resourceName, "bucket", rName), resource.TestCheckResourceAttr(resourceName, "bucket_key_enabled", "false"), resource.TestCheckResourceAttr(resourceName, "cache_control", ""), diff --git a/internal/service/s3/tags.go b/internal/service/s3/tags.go index 5689721f4aa3..253fd773f831 100644 --- a/internal/service/s3/tags.go +++ b/internal/service/s3/tags.go @@ -167,7 +167,8 @@ func (p *servicePackage) ListTags(ctx context.Context, meta any, identifier, res tags, err = bucketListTags(ctx, meta.(*conns.AWSClient).S3Client(ctx), identifier) case "Object", "ObjectCopy", "BucketObject": - objectARN, err := parseObjectARN(identifier) + var objectARN objectARN + objectARN, err = parseObjectARN(identifier) if err != nil { return err } From b32b18617528c7b6e0505abb2052187d184508ed Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Tue, 13 Feb 2024 14:14:52 -0800 Subject: [PATCH 13/24] Adds CHANGELOG entry --- .changelog/35710.txt | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 .changelog/35710.txt diff --git a/.changelog/35710.txt b/.changelog/35710.txt new file mode 100644 index 000000000000..b34005774a2e --- /dev/null +++ b/.changelog/35710.txt @@ -0,0 +1,27 @@ +```release-note:bug +resource/aws_s3_bucket: Tags with empty values no longer remove all tags. +``` + +```release-note:bug +resource/aws_s3_bucket_object: Tags with empty values no longer remove all tags. +``` + +```release-note:bug +resource/aws_s3_object: Tags with empty values no longer remove all tags. +``` + +```release-note:bug +resource/aws_s3_object_copy: Tags with empty values no longer remove all tags. +``` + +```release-note:enhancement +resource/aws_s3_bucket_object: Adds attribute `arn`. +``` + +```release-note:enhancement +resource/aws_s3_object: Adds attribute `arn`. +``` + +```release-note:enhancement +resource/aws_s3_object_copy: Adds attribute `arn`. +``` From a06736622e0151ad2fded156f57c9f8ae55c44a7 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Tue, 13 Feb 2024 15:39:20 -0800 Subject: [PATCH 14/24] Fixes error reading object in directory bucket --- internal/service/s3/tags.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/internal/service/s3/tags.go b/internal/service/s3/tags.go index 253fd773f831..09c0b7d72111 100644 --- a/internal/service/s3/tags.go +++ b/internal/service/s3/tags.go @@ -9,6 +9,7 @@ package s3 import ( "context" "fmt" + "net/http" "github.com/aws/aws-sdk-go-v2/aws" "github.com/aws/aws-sdk-go-v2/service/s3" @@ -104,6 +105,10 @@ func objectListTags(ctx context.Context, conn *s3.Client, bucket, key string, op return tftags.New(ctx, nil), nil } + if tfawserr.ErrHTTPStatusCodeEquals(err, http.StatusNotImplemented) { // Directory buckets return HTTP status code 501, NotImplemented. + return tftags.New(ctx, nil), nil + } + if err != nil { return tftags.New(ctx, nil), err } From 5ca4425e7360a9538d4a5d8864cf7f90e26b7dc8 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Tue, 13 Feb 2024 15:39:56 -0800 Subject: [PATCH 15/24] Cleanup --- internal/service/s3/bucket_test.go | 2 +- internal/service/s3/object_test.go | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/service/s3/bucket_test.go b/internal/service/s3/bucket_test.go index 0e1c9bbd6eb5..0afeef282f5d 100644 --- a/internal/service/s3/bucket_test.go +++ b/internal/service/s3/bucket_test.go @@ -699,7 +699,7 @@ func TestAccS3Bucket_Tags_withSystemTags(t *testing.T) { ClientRequestToken: aws.String(requestToken), } - log.Printf("[DEBUG] Deleting CloudFormation stack: %s", req) + log.Printf("[DEBUG] Deleting CloudFormation stack: %s", stackID) if _, err := conn.DeleteStackWithContext(ctx, req); err != nil { return fmt.Errorf("error deleting CloudFormation stack: %w", err) } diff --git a/internal/service/s3/object_test.go b/internal/service/s3/object_test.go index 6de542a8a391..65a511384562 100644 --- a/internal/service/s3/object_test.go +++ b/internal/service/s3/object_test.go @@ -1930,6 +1930,8 @@ func TestAccS3Object_DirectoryBucket_DefaultTags_providerOnly(t *testing.T) { ), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj), + resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), + resource.TestCheckResourceAttr(resourceName, "tags_all.%", "0"), ), }, }, From 8022adffb09d891f45353da89953867e59f2a031 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Thu, 22 Feb 2024 10:23:31 -0500 Subject: [PATCH 16/24] Add 'TestAccS3Object_tagsViaAccessPoint'. --- internal/service/s3/object_test.go | 108 +++++++++++++++++++++++++++++ 1 file changed, 108 insertions(+) diff --git a/internal/service/s3/object_test.go b/internal/service/s3/object_test.go index 3b4d9a4a4f9e..c95b7e9f250a 100644 --- a/internal/service/s3/object_test.go +++ b/internal/service/s3/object_test.go @@ -1415,6 +1415,47 @@ func TestAccS3Object_DefaultTags_providerAndResourceWithOverride(t *testing.T) { }) } +func TestAccS3Object_tagsViaAccessPoint(t *testing.T) { + ctx := acctest.Context(t) + var obj1, obj2 s3.GetObjectOutput + resourceName := "aws_s3_object.object" + key := "test-key" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, names.S3ServiceID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckObjectDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccObjectConfig_tagsViaAccessPoint(rName, key, "stuff"), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckObjectExists(ctx, resourceName, &obj1), + testAccCheckObjectBody(&obj1, "stuff"), + resource.TestCheckResourceAttr(resourceName, "tags.%", "3"), + resource.TestCheckResourceAttr(resourceName, "tags.Key1", "A@AA"), + resource.TestCheckResourceAttr(resourceName, "tags.Key2", "BBB"), + resource.TestCheckResourceAttr(resourceName, "tags.Key3", "CCC"), + ), + }, + { + Config: testAccObjectConfig_updatedTagsViaAccessPoint(rName, key, "stuff"), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckObjectExists(ctx, resourceName, &obj2), + testAccCheckObjectVersionIDEquals(&obj2, &obj1), + testAccCheckObjectBody(&obj2, "stuff"), + resource.TestCheckResourceAttr(resourceName, "tags.%", "4"), + resource.TestCheckResourceAttr(resourceName, "tags.Key2", "B@BB"), + resource.TestCheckResourceAttr(resourceName, "tags.Key3", "X X"), + resource.TestCheckResourceAttr(resourceName, "tags.Key4", "DDD"), + resource.TestCheckResourceAttr(resourceName, "tags.Key5", "E:/"), + ), + }, + }, + }) +} + func TestAccS3Object_objectLockLegalHoldStartWithNone(t *testing.T) { ctx := acctest.Context(t) var obj1, obj2, obj3 s3.GetObjectOutput @@ -2733,6 +2774,73 @@ resource "aws_s3_object" "object" { `, rName, key, tagKey1, tagValue1, tagKey2, tagValue2) } +func testAccObjectConfig_tagsViaAccessPoint(rName, key, content string) string { + return fmt.Sprintf(` +resource "aws_s3_bucket" "test" { + bucket = %[1]q +} + +resource "aws_s3_bucket_versioning" "test" { + bucket = aws_s3_bucket.test.id + versioning_configuration { + status = "Enabled" + } +} + +resource "aws_s3_access_point" "test" { + # Must have bucket versioning enabled first + bucket = aws_s3_bucket_versioning.test.bucket + name = %[1]q +} + +resource "aws_s3_object" "object" { + bucket = aws_s3_access_point.test.arn + key = %[2]q + content = %[3]q + + tags = { + Key1 = "A@AA" + Key2 = "BBB" + Key3 = "CCC" + } +} +`, rName, key, content) +} + +func testAccObjectConfig_updatedTagsViaAccessPoint(rName, key, content string) string { + return fmt.Sprintf(` +resource "aws_s3_bucket" "test" { + bucket = %[1]q +} + +resource "aws_s3_bucket_versioning" "test" { + bucket = aws_s3_bucket.test.id + versioning_configuration { + status = "Enabled" + } +} + +resource "aws_s3_access_point" "test" { + # Must have bucket versioning enabled first + bucket = aws_s3_bucket_versioning.test.bucket + name = %[1]q +} + +resource "aws_s3_object" "object" { + bucket = aws_s3_access_point.test.arn + key = %[2]q + content = %[3]q + + tags = { + Key2 = "B@BB" + Key3 = "X X" + Key4 = "DDD" + Key5 = "E:/" + } +} +`, rName, key, content) +} + func testAccObjectConfig_metadata(rName string, metadataKey1, metadataValue1, metadataKey2, metadataValue2 string) string { return fmt.Sprintf(` resource "aws_s3_bucket" "test" { From 360c0f2ce057abb426d07adff5caaf169bc8c702 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Thu, 22 Feb 2024 10:30:15 -0500 Subject: [PATCH 17/24] 'TestAccS3Object_tagsViaAccessPoint' -> 'TestAccS3Object_tagsViaAccessPointARN'. --- internal/service/s3/object_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/service/s3/object_test.go b/internal/service/s3/object_test.go index c95b7e9f250a..b2acaec2f15c 100644 --- a/internal/service/s3/object_test.go +++ b/internal/service/s3/object_test.go @@ -1415,7 +1415,7 @@ func TestAccS3Object_DefaultTags_providerAndResourceWithOverride(t *testing.T) { }) } -func TestAccS3Object_tagsViaAccessPoint(t *testing.T) { +func TestAccS3Object_tagsViaAccessPointARN(t *testing.T) { ctx := acctest.Context(t) var obj1, obj2 s3.GetObjectOutput resourceName := "aws_s3_object.object" @@ -1429,7 +1429,7 @@ func TestAccS3Object_tagsViaAccessPoint(t *testing.T) { CheckDestroy: testAccCheckObjectDestroy(ctx), Steps: []resource.TestStep{ { - Config: testAccObjectConfig_tagsViaAccessPoint(rName, key, "stuff"), + Config: testAccObjectConfig_tagsViaAccessPointARN(rName, key, "stuff"), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj1), testAccCheckObjectBody(&obj1, "stuff"), @@ -1440,7 +1440,7 @@ func TestAccS3Object_tagsViaAccessPoint(t *testing.T) { ), }, { - Config: testAccObjectConfig_updatedTagsViaAccessPoint(rName, key, "stuff"), + Config: testAccObjectConfig_updatedTagsViaAccessPointARN(rName, key, "stuff"), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj2), testAccCheckObjectVersionIDEquals(&obj2, &obj1), @@ -2774,7 +2774,7 @@ resource "aws_s3_object" "object" { `, rName, key, tagKey1, tagValue1, tagKey2, tagValue2) } -func testAccObjectConfig_tagsViaAccessPoint(rName, key, content string) string { +func testAccObjectConfig_tagsViaAccessPointARN(rName, key, content string) string { return fmt.Sprintf(` resource "aws_s3_bucket" "test" { bucket = %[1]q @@ -2807,7 +2807,7 @@ resource "aws_s3_object" "object" { `, rName, key, content) } -func testAccObjectConfig_updatedTagsViaAccessPoint(rName, key, content string) string { +func testAccObjectConfig_updatedTagsViaAccessPointARN(rName, key, content string) string { return fmt.Sprintf(` resource "aws_s3_bucket" "test" { bucket = %[1]q From 1f4e15f81ac1c4325c980144f6dbd3d03dc271d1 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Thu, 22 Feb 2024 10:32:15 -0500 Subject: [PATCH 18/24] Add 'TestAccS3Object_tagsViaAccessPointAlias'. --- internal/service/s3/object_test.go | 108 +++++++++++++++++++++++++++++ 1 file changed, 108 insertions(+) diff --git a/internal/service/s3/object_test.go b/internal/service/s3/object_test.go index b2acaec2f15c..27b2cdd00010 100644 --- a/internal/service/s3/object_test.go +++ b/internal/service/s3/object_test.go @@ -1456,6 +1456,47 @@ func TestAccS3Object_tagsViaAccessPointARN(t *testing.T) { }) } +func TestAccS3Object_tagsViaAccessPointAlias(t *testing.T) { + ctx := acctest.Context(t) + var obj1, obj2 s3.GetObjectOutput + resourceName := "aws_s3_object.object" + key := "test-key" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, names.S3ServiceID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckObjectDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccObjectConfig_tagsViaAccessPointAlias(rName, key, "stuff"), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckObjectExists(ctx, resourceName, &obj1), + testAccCheckObjectBody(&obj1, "stuff"), + resource.TestCheckResourceAttr(resourceName, "tags.%", "3"), + resource.TestCheckResourceAttr(resourceName, "tags.Key1", "A@AA"), + resource.TestCheckResourceAttr(resourceName, "tags.Key2", "BBB"), + resource.TestCheckResourceAttr(resourceName, "tags.Key3", "CCC"), + ), + }, + { + Config: testAccObjectConfig_updatedTagsViaAccessPointAlias(rName, key, "stuff"), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckObjectExists(ctx, resourceName, &obj2), + testAccCheckObjectVersionIDEquals(&obj2, &obj1), + testAccCheckObjectBody(&obj2, "stuff"), + resource.TestCheckResourceAttr(resourceName, "tags.%", "4"), + resource.TestCheckResourceAttr(resourceName, "tags.Key2", "B@BB"), + resource.TestCheckResourceAttr(resourceName, "tags.Key3", "X X"), + resource.TestCheckResourceAttr(resourceName, "tags.Key4", "DDD"), + resource.TestCheckResourceAttr(resourceName, "tags.Key5", "E:/"), + ), + }, + }, + }) +} + func TestAccS3Object_objectLockLegalHoldStartWithNone(t *testing.T) { ctx := acctest.Context(t) var obj1, obj2, obj3 s3.GetObjectOutput @@ -2841,6 +2882,73 @@ resource "aws_s3_object" "object" { `, rName, key, content) } +func testAccObjectConfig_tagsViaAccessPointAlias(rName, key, content string) string { + return fmt.Sprintf(` +resource "aws_s3_bucket" "test" { + bucket = %[1]q +} + +resource "aws_s3_bucket_versioning" "test" { + bucket = aws_s3_bucket.test.id + versioning_configuration { + status = "Enabled" + } +} + +resource "aws_s3_access_point" "test" { + # Must have bucket versioning enabled first + bucket = aws_s3_bucket_versioning.test.bucket + name = %[1]q +} + +resource "aws_s3_object" "object" { + bucket = aws_s3_access_point.test.alias + key = %[2]q + content = %[3]q + + tags = { + Key1 = "A@AA" + Key2 = "BBB" + Key3 = "CCC" + } +} +`, rName, key, content) +} + +func testAccObjectConfig_updatedTagsViaAccessPointAlias(rName, key, content string) string { + return fmt.Sprintf(` +resource "aws_s3_bucket" "test" { + bucket = %[1]q +} + +resource "aws_s3_bucket_versioning" "test" { + bucket = aws_s3_bucket.test.id + versioning_configuration { + status = "Enabled" + } +} + +resource "aws_s3_access_point" "test" { + # Must have bucket versioning enabled first + bucket = aws_s3_bucket_versioning.test.bucket + name = %[1]q +} + +resource "aws_s3_object" "object" { + bucket = aws_s3_access_point.test.alias + key = %[2]q + content = %[3]q + + tags = { + Key2 = "B@BB" + Key3 = "X X" + Key4 = "DDD" + Key5 = "E:/" + } +} +`, rName, key, content) +} + func testAccObjectConfig_metadata(rName string, metadataKey1, metadataValue1, metadataKey2, metadataValue2 string) string { return fmt.Sprintf(` resource "aws_s3_bucket" "test" { From 6aa9a410293ed49b45d2f2fe5e84fae561652369 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Thu, 22 Feb 2024 11:02:17 -0500 Subject: [PATCH 19/24] Add 'TestAccS3Object_tagsViaMultiRegionAccessPoint'. --- internal/service/s3/object_test.go | 116 +++++++++++++++++++++++++++++ 1 file changed, 116 insertions(+) diff --git a/internal/service/s3/object_test.go b/internal/service/s3/object_test.go index 27b2cdd00010..a30a43a392d8 100644 --- a/internal/service/s3/object_test.go +++ b/internal/service/s3/object_test.go @@ -1497,6 +1497,47 @@ func TestAccS3Object_tagsViaAccessPointAlias(t *testing.T) { }) } +func TestAccS3Object_tagsViaMultiRegionAccessPoint(t *testing.T) { + ctx := acctest.Context(t) + var obj1, obj2 s3.GetObjectOutput + resourceName := "aws_s3_object.object" + key := "test-key" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, names.S3ServiceID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckObjectDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccObjectConfig_tagsViaMultiRegionAccessPoint(rName, key, "stuff"), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckObjectExists(ctx, resourceName, &obj1), + testAccCheckObjectBody(&obj1, "stuff"), + resource.TestCheckResourceAttr(resourceName, "tags.%", "3"), + resource.TestCheckResourceAttr(resourceName, "tags.Key1", "A@AA"), + resource.TestCheckResourceAttr(resourceName, "tags.Key2", "BBB"), + resource.TestCheckResourceAttr(resourceName, "tags.Key3", "CCC"), + ), + }, + { + Config: testAccObjectConfig_updatedTagsViaMultiRegionAccessPoint(rName, key, "stuff"), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckObjectExists(ctx, resourceName, &obj2), + testAccCheckObjectVersionIDEquals(&obj2, &obj1), + testAccCheckObjectBody(&obj2, "stuff"), + resource.TestCheckResourceAttr(resourceName, "tags.%", "4"), + resource.TestCheckResourceAttr(resourceName, "tags.Key2", "B@BB"), + resource.TestCheckResourceAttr(resourceName, "tags.Key3", "X X"), + resource.TestCheckResourceAttr(resourceName, "tags.Key4", "DDD"), + resource.TestCheckResourceAttr(resourceName, "tags.Key5", "E:/"), + ), + }, + }, + }) +} + func TestAccS3Object_objectLockLegalHoldStartWithNone(t *testing.T) { ctx := acctest.Context(t) var obj1, obj2, obj3 s3.GetObjectOutput @@ -2949,6 +2990,81 @@ resource "aws_s3_object" "object" { `, rName, key, content) } +func testAccObjectConfig_tagsViaMultiRegionAccessPoint(rName, key, content string) string { + return fmt.Sprintf(` +resource "aws_s3_bucket" "test" { + bucket = %[1]q +} + +resource "aws_s3_bucket_versioning" "test" { + bucket = aws_s3_bucket.test.id + versioning_configuration { + status = "Enabled" + } +} + +resource "aws_s3control_multi_region_access_point" "test" { + details { + name = %[1]q + + region { + bucket = aws_s3_bucket_versioning.test.bucket + } + } +} + +resource "aws_s3_object" "object" { + bucket = aws_s3control_multi_region_access_point.test.arn + key = %[2]q + content = %[3]q + + tags = { + Key1 = "A@AA" + Key2 = "BBB" + Key3 = "CCC" + } +} +`, rName, key, content) +} + +func testAccObjectConfig_updatedTagsViaMultiRegionAccessPoint(rName, key, content string) string { + return fmt.Sprintf(` +resource "aws_s3_bucket" "test" { + bucket = %[1]q +} + +resource "aws_s3_bucket_versioning" "test" { + bucket = aws_s3_bucket.test.id + versioning_configuration { + status = "Enabled" + } +} + +resource "aws_s3control_multi_region_access_point" "test" { + details { + name = %[1]q + + region { + bucket = aws_s3_bucket_versioning.test.bucket + } + } +} + +resource "aws_s3_object" "object" { + bucket = aws_s3control_multi_region_access_point.test.arn + key = %[2]q + content = %[3]q + + tags = { + Key2 = "B@BB" + Key3 = "X X" + Key4 = "DDD" + Key5 = "E:/" + } +} +`, rName, key, content) +} + func testAccObjectConfig_metadata(rName string, metadataKey1, metadataValue1, metadataKey2, metadataValue2 string) string { return fmt.Sprintf(` resource "aws_s3_bucket" "test" { From bb18355b18283ba721c6588cee0a7d3ed8b4d13e Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Thu, 22 Feb 2024 11:24:23 -0500 Subject: [PATCH 20/24] r/aws_s3_object: Tidy up some acceptance test configurations. --- internal/service/s3/object_test.go | 235 ++++++++++------------------- 1 file changed, 77 insertions(+), 158 deletions(-) diff --git a/internal/service/s3/object_test.go b/internal/service/s3/object_test.go index a30a43a392d8..862d94f24340 100644 --- a/internal/service/s3/object_test.go +++ b/internal/service/s3/object_test.go @@ -645,7 +645,7 @@ func TestAccS3Object_updatesWithVersioningViaAccessPoint(t *testing.T) { CheckDestroy: testAccCheckObjectDestroy(ctx), Steps: []resource.TestStep{ { - Config: testAccObjectConfig_updateableViaAccessPoint(rName, true, sourceInitial), + Config: testAccObjectConfig_updateableViaAccessPoint(rName, sourceInitial), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &originalObj), testAccCheckObjectBody(&originalObj, "initial versioned object state"), @@ -654,7 +654,7 @@ func TestAccS3Object_updatesWithVersioningViaAccessPoint(t *testing.T) { ), }, { - Config: testAccObjectConfig_updateableViaAccessPoint(rName, true, sourceModified), + Config: testAccObjectConfig_updateableViaAccessPoint(rName, sourceModified), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &modifiedObj), testAccCheckObjectBody(&modifiedObj, "modified versioned object"), @@ -2410,6 +2410,52 @@ func testAccCheckObjectCheckTags(ctx context.Context, n string, expectedTags map } } +func testAccObjectConfig_baseAccessPoint(rName string) string { + return fmt.Sprintf(` +resource "aws_s3_bucket" "test" { + bucket = %[1]q +} + +resource "aws_s3_bucket_versioning" "test" { + bucket = aws_s3_bucket.test.id + versioning_configuration { + status = "Enabled" + } +} + +resource "aws_s3_access_point" "test" { + # Must have bucket versioning enabled first + bucket = aws_s3_bucket_versioning.test.bucket + name = %[1]q +} +`, rName) +} + +func testAccObjectConfig_baseMultiRegionAccessPoint(rName string) string { + return fmt.Sprintf(` +resource "aws_s3_bucket" "test" { + bucket = %[1]q +} + +resource "aws_s3_bucket_versioning" "test" { + bucket = aws_s3_bucket.test.id + versioning_configuration { + status = "Enabled" + } +} + +resource "aws_s3control_multi_region_access_point" "test" { + details { + name = %[1]q + + region { + bucket = aws_s3_bucket_versioning.test.bucket + } + } +} +`, rName) +} + func testAccObjectConfig_basic(rName string) string { return fmt.Sprintf(` resource "aws_s3_bucket" "test" { @@ -2538,32 +2584,15 @@ resource "aws_s3_object" "object" { `, rName, bucketVersioning, source) } -func testAccObjectConfig_updateableViaAccessPoint(rName string, bucketVersioning bool, source string) string { - return fmt.Sprintf(` -resource "aws_s3_bucket" "test" { - bucket = %[1]q -} - -resource "aws_s3_bucket_versioning" "test" { - bucket = aws_s3_bucket.test.id - versioning_configuration { - status = "Enabled" - } -} - -resource "aws_s3_access_point" "test" { - # Must have bucket versioning enabled first - bucket = aws_s3_bucket_versioning.test.bucket - name = %[1]q -} - +func testAccObjectConfig_updateableViaAccessPoint(rName string, source string) string { + return acctest.ConfigCompose(testAccObjectConfig_baseAccessPoint(rName), fmt.Sprintf(` resource "aws_s3_object" "test" { bucket = aws_s3_access_point.test.arn key = "updateable-key" - source = %[3]q - etag = filemd5(%[3]q) + source = %[1]q + etag = filemd5(%[1]q) } -`, rName, bucketVersioning, source) +`, source)) } func testAccObjectConfig_kmsID(rName string, source string) string { @@ -2857,28 +2886,11 @@ resource "aws_s3_object" "object" { } func testAccObjectConfig_tagsViaAccessPointARN(rName, key, content string) string { - return fmt.Sprintf(` -resource "aws_s3_bucket" "test" { - bucket = %[1]q -} - -resource "aws_s3_bucket_versioning" "test" { - bucket = aws_s3_bucket.test.id - versioning_configuration { - status = "Enabled" - } -} - -resource "aws_s3_access_point" "test" { - # Must have bucket versioning enabled first - bucket = aws_s3_bucket_versioning.test.bucket - name = %[1]q -} - + return acctest.ConfigCompose(testAccObjectConfig_baseAccessPoint(rName), fmt.Sprintf(` resource "aws_s3_object" "object" { bucket = aws_s3_access_point.test.arn - key = %[2]q - content = %[3]q + key = %[1]q + content = %[2]q tags = { Key1 = "A@AA" @@ -2886,32 +2898,15 @@ resource "aws_s3_object" "object" { Key3 = "CCC" } } -`, rName, key, content) +`, key, content)) } func testAccObjectConfig_updatedTagsViaAccessPointARN(rName, key, content string) string { - return fmt.Sprintf(` -resource "aws_s3_bucket" "test" { - bucket = %[1]q -} - -resource "aws_s3_bucket_versioning" "test" { - bucket = aws_s3_bucket.test.id - versioning_configuration { - status = "Enabled" - } -} - -resource "aws_s3_access_point" "test" { - # Must have bucket versioning enabled first - bucket = aws_s3_bucket_versioning.test.bucket - name = %[1]q -} - + return acctest.ConfigCompose(testAccObjectConfig_baseAccessPoint(rName), fmt.Sprintf(` resource "aws_s3_object" "object" { bucket = aws_s3_access_point.test.arn - key = %[2]q - content = %[3]q + key = %[1]q + content = %[2]q tags = { Key2 = "B@BB" @@ -2920,32 +2915,15 @@ resource "aws_s3_object" "object" { Key5 = "E:/" } } -`, rName, key, content) +`, key, content)) } func testAccObjectConfig_tagsViaAccessPointAlias(rName, key, content string) string { - return fmt.Sprintf(` -resource "aws_s3_bucket" "test" { - bucket = %[1]q -} - -resource "aws_s3_bucket_versioning" "test" { - bucket = aws_s3_bucket.test.id - versioning_configuration { - status = "Enabled" - } -} - -resource "aws_s3_access_point" "test" { - # Must have bucket versioning enabled first - bucket = aws_s3_bucket_versioning.test.bucket - name = %[1]q -} - + return acctest.ConfigCompose(testAccObjectConfig_baseAccessPoint(rName), fmt.Sprintf(` resource "aws_s3_object" "object" { bucket = aws_s3_access_point.test.alias - key = %[2]q - content = %[3]q + key = %[1]q + content = %[2]q tags = { Key1 = "A@AA" @@ -2953,32 +2931,15 @@ resource "aws_s3_object" "object" { Key3 = "CCC" } } -`, rName, key, content) +`, key, content)) } func testAccObjectConfig_updatedTagsViaAccessPointAlias(rName, key, content string) string { - return fmt.Sprintf(` -resource "aws_s3_bucket" "test" { - bucket = %[1]q -} - -resource "aws_s3_bucket_versioning" "test" { - bucket = aws_s3_bucket.test.id - versioning_configuration { - status = "Enabled" - } -} - -resource "aws_s3_access_point" "test" { - # Must have bucket versioning enabled first - bucket = aws_s3_bucket_versioning.test.bucket - name = %[1]q -} - + return acctest.ConfigCompose(testAccObjectConfig_baseAccessPoint(rName), fmt.Sprintf(` resource "aws_s3_object" "object" { bucket = aws_s3_access_point.test.alias - key = %[2]q - content = %[3]q + key = %[1]q + content = %[2]q tags = { Key2 = "B@BB" @@ -2987,36 +2948,15 @@ resource "aws_s3_object" "object" { Key5 = "E:/" } } -`, rName, key, content) +`, key, content)) } func testAccObjectConfig_tagsViaMultiRegionAccessPoint(rName, key, content string) string { - return fmt.Sprintf(` -resource "aws_s3_bucket" "test" { - bucket = %[1]q -} - -resource "aws_s3_bucket_versioning" "test" { - bucket = aws_s3_bucket.test.id - versioning_configuration { - status = "Enabled" - } -} - -resource "aws_s3control_multi_region_access_point" "test" { - details { - name = %[1]q - - region { - bucket = aws_s3_bucket_versioning.test.bucket - } - } -} - + return acctest.ConfigCompose(testAccObjectConfig_baseMultiRegionAccessPoint(rName), fmt.Sprintf(` resource "aws_s3_object" "object" { bucket = aws_s3control_multi_region_access_point.test.arn - key = %[2]q - content = %[3]q + key = %[1]q + content = %[2]q tags = { Key1 = "A@AA" @@ -3024,36 +2964,15 @@ resource "aws_s3_object" "object" { Key3 = "CCC" } } -`, rName, key, content) +`, key, content)) } func testAccObjectConfig_updatedTagsViaMultiRegionAccessPoint(rName, key, content string) string { - return fmt.Sprintf(` -resource "aws_s3_bucket" "test" { - bucket = %[1]q -} - -resource "aws_s3_bucket_versioning" "test" { - bucket = aws_s3_bucket.test.id - versioning_configuration { - status = "Enabled" - } -} - -resource "aws_s3control_multi_region_access_point" "test" { - details { - name = %[1]q - - region { - bucket = aws_s3_bucket_versioning.test.bucket - } - } -} - + return acctest.ConfigCompose(testAccObjectConfig_baseMultiRegionAccessPoint(rName), fmt.Sprintf(` resource "aws_s3_object" "object" { bucket = aws_s3control_multi_region_access_point.test.arn - key = %[2]q - content = %[3]q + key = %[1]q + content = %[2]q tags = { Key2 = "B@BB" @@ -3062,7 +2981,7 @@ resource "aws_s3_object" "object" { Key5 = "E:/" } } -`, rName, key, content) +`, key, content)) } func testAccObjectConfig_metadata(rName string, metadataKey1, metadataValue1, metadataKey2, metadataValue2 string) string { From f839e338c654bf4833c2b67b5fd1afdb5cbe285f Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Thu, 22 Feb 2024 11:32:15 -0500 Subject: [PATCH 21/24] Add 'TestAccS3Object_tagsViaObjectLambdaAccessPointARN'. --- internal/service/s3/object_test.go | 104 +++++++++++++++++++++++++++++ 1 file changed, 104 insertions(+) diff --git a/internal/service/s3/object_test.go b/internal/service/s3/object_test.go index 862d94f24340..dbb668f17761 100644 --- a/internal/service/s3/object_test.go +++ b/internal/service/s3/object_test.go @@ -1538,6 +1538,47 @@ func TestAccS3Object_tagsViaMultiRegionAccessPoint(t *testing.T) { }) } +func TestAccS3Object_tagsViaObjectLambdaAccessPointARN(t *testing.T) { + ctx := acctest.Context(t) + var obj1, obj2 s3.GetObjectOutput + resourceName := "aws_s3_object.object" + key := "test-key" + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, names.S3ServiceID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckObjectDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccObjectConfig_tagsViaObjectLambdaAccessPointARN(rName, key, "stuff"), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckObjectExists(ctx, resourceName, &obj1), + testAccCheckObjectBody(&obj1, "stuff"), + resource.TestCheckResourceAttr(resourceName, "tags.%", "3"), + resource.TestCheckResourceAttr(resourceName, "tags.Key1", "A@AA"), + resource.TestCheckResourceAttr(resourceName, "tags.Key2", "BBB"), + resource.TestCheckResourceAttr(resourceName, "tags.Key3", "CCC"), + ), + }, + { + Config: testAccObjectConfig_updatedTagsViaObjectLambdaAccessPointARN(rName, key, "stuff"), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckObjectExists(ctx, resourceName, &obj2), + testAccCheckObjectVersionIDEquals(&obj2, &obj1), + testAccCheckObjectBody(&obj2, "stuff"), + resource.TestCheckResourceAttr(resourceName, "tags.%", "4"), + resource.TestCheckResourceAttr(resourceName, "tags.Key2", "B@BB"), + resource.TestCheckResourceAttr(resourceName, "tags.Key3", "X X"), + resource.TestCheckResourceAttr(resourceName, "tags.Key4", "DDD"), + resource.TestCheckResourceAttr(resourceName, "tags.Key5", "E:/"), + ), + }, + }, + }) +} + func TestAccS3Object_objectLockLegalHoldStartWithNone(t *testing.T) { ctx := acctest.Context(t) var obj1, obj2, obj3 s3.GetObjectOutput @@ -2456,6 +2497,36 @@ resource "aws_s3control_multi_region_access_point" "test" { `, rName) } +func testAccObjectConfig_baseObjectLambdaAccessPoint(rName string) string { + return acctest.ConfigCompose(testAccObjectConfig_baseAccessPoint(rName), acctest.ConfigLambdaBase(rName, rName, rName), fmt.Sprintf(` +resource "aws_lambda_function" "test" { + filename = "test-fixtures/lambdatest.zip" + function_name = %[1]q + role = aws_iam_role.iam_for_lambda.arn + handler = "index.handler" + runtime = "nodejs20.x" +} + +resource "aws_s3control_object_lambda_access_point" "test" { + name = %[1]q + + configuration { + supporting_access_point = aws_s3_access_point.test.arn + + transformation_configuration { + actions = ["GetObject"] + + content_transformation { + aws_lambda { + function_arn = aws_lambda_function.test.arn + } + } + } + } +} +`, rName)) +} + func testAccObjectConfig_basic(rName string) string { return fmt.Sprintf(` resource "aws_s3_bucket" "test" { @@ -2984,6 +3055,39 @@ resource "aws_s3_object" "object" { `, key, content)) } +func testAccObjectConfig_tagsViaObjectLambdaAccessPointARN(rName, key, content string) string { + return acctest.ConfigCompose(testAccObjectConfig_baseObjectLambdaAccessPoint(rName), fmt.Sprintf(` +resource "aws_s3_object" "object" { + bucket = aws_s3control_object_lambda_access_point.test.arn + key = %[1]q + content = %[2]q + + tags = { + Key1 = "A@AA" + Key2 = "BBB" + Key3 = "CCC" + } +} +`, key, content)) +} + +func testAccObjectConfig_updatedTagsViaObjectLambdaAccessPointARN(rName, key, content string) string { + return acctest.ConfigCompose(testAccObjectConfig_baseObjectLambdaAccessPoint(rName), fmt.Sprintf(` +resource "aws_s3_object" "object" { + bucket = aws_s3control_object_lambda_access_point.test.arn + key = %[1]q + content = %[2]q + + tags = { + Key2 = "B@BB" + Key3 = "X X" + Key4 = "DDD" + Key5 = "E:/" + } +} +`, key, content)) +} + func testAccObjectConfig_metadata(rName string, metadataKey1, metadataValue1, metadataKey2, metadataValue2 string) string { return fmt.Sprintf(` resource "aws_s3_bucket" "test" { From b93e82bbc8340fccc9f345ba3cb22e1dff779c31 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Thu, 22 Feb 2024 11:27:31 -0800 Subject: [PATCH 22/24] Updates `TestAccS3Object_directoryBucket` and `TestAccS3ObjectCopy_directoryBucket` to test object ARN and bucket value --- internal/service/s3/object_copy_test.go | 4 +++- internal/service/s3/object_test.go | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/internal/service/s3/object_copy_test.go b/internal/service/s3/object_copy_test.go index 0aae3af19f8e..8ca61977a16b 100644 --- a/internal/service/s3/object_copy_test.go +++ b/internal/service/s3/object_copy_test.go @@ -8,6 +8,7 @@ import ( "fmt" "testing" + "github.com/YakDriver/regexache" "github.com/aws/aws-sdk-go-v2/aws/arn" "github.com/aws/aws-sdk-go-v2/service/s3" sdkacctest "github.com/hashicorp/terraform-plugin-testing/helper/acctest" @@ -552,7 +553,8 @@ func TestAccS3ObjectCopy_directoryBucket(t *testing.T) { Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectCopyExists(ctx, resourceName), resource.TestCheckNoResourceAttr(resourceName, "acl"), - resource.TestCheckResourceAttrSet(resourceName, "bucket"), + acctest.MatchResourceAttrGlobalARNNoAccount(resourceName, "arn", "s3", regexache.MustCompile(fmt.Sprintf(`%s--[-a-z0-9]+--x-s3/%s$`, rName2, targetKey))), + resource.TestMatchResourceAttr(resourceName, "bucket", regexache.MustCompile(fmt.Sprintf(`^%s--[-a-z0-9]+--x-s3$`, rName2))), resource.TestCheckResourceAttr(resourceName, "bucket_key_enabled", "false"), resource.TestCheckResourceAttr(resourceName, "cache_control", ""), resource.TestCheckNoResourceAttr(resourceName, "checksum_algorithm"), diff --git a/internal/service/s3/object_test.go b/internal/service/s3/object_test.go index dbb668f17761..42c5e1a09533 100644 --- a/internal/service/s3/object_test.go +++ b/internal/service/s3/object_test.go @@ -1998,7 +1998,8 @@ func TestAccS3Object_directoryBucket(t *testing.T) { testAccCheckObjectExists(ctx, resourceName, &obj), testAccCheckObjectBody(&obj, ""), resource.TestCheckNoResourceAttr(resourceName, "acl"), - resource.TestCheckResourceAttrSet(resourceName, "bucket"), + acctest.MatchResourceAttrGlobalARNNoAccount(resourceName, "arn", "s3", regexache.MustCompile(fmt.Sprintf(`%s--[-a-z0-9]+--x-s3/%s$`, rName, "test-key"))), + resource.TestMatchResourceAttr(resourceName, "bucket", regexache.MustCompile(fmt.Sprintf(`^%s--[-a-z0-9]+--x-s3$`, rName))), resource.TestCheckResourceAttr(resourceName, "bucket_key_enabled", "false"), resource.TestCheckResourceAttr(resourceName, "cache_control", ""), resource.TestCheckNoResourceAttr(resourceName, "checksum_algorithm"), From a21826202a1703eb092540d691b24a7b40d41dc5 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Fri, 23 Feb 2024 12:30:32 -0800 Subject: [PATCH 23/24] Updates handling for Objects accessed via access points --- internal/service/s3/bucket_object.go | 5 +- .../service/s3/bucket_object_data_source.go | 5 +- internal/service/s3/object.go | 39 +-- internal/service/s3/object_arn.go | 64 ++++ internal/service/s3/object_arn_test.go | 294 ++++++++++++++++++ internal/service/s3/object_copy.go | 5 +- internal/service/s3/object_data_source.go | 5 +- internal/service/s3/object_test.go | 15 +- 8 files changed, 385 insertions(+), 47 deletions(-) create mode 100644 internal/service/s3/object_arn.go create mode 100644 internal/service/s3/object_arn_test.go diff --git a/internal/service/s3/bucket_object.go b/internal/service/s3/bucket_object.go index 0d7651472fb3..fe0d296ce847 100644 --- a/internal/service/s3/bucket_object.go +++ b/internal/service/s3/bucket_object.go @@ -224,7 +224,10 @@ func resourceBucketObjectRead(ctx context.Context, d *schema.ResourceData, meta return sdkdiag.AppendErrorf(diags, "reading S3 Object (%s): %s", d.Id(), err) } - arn := newObjectARN(meta.(*conns.AWSClient).Partition, bucket, key) + arn, err := newObjectARN(meta.(*conns.AWSClient).Partition, bucket, key) + if err != nil { + return sdkdiag.AppendErrorf(diags, "reading S3 Object (%s): %s", d.Id(), err) + } d.Set("arn", arn.String()) d.Set("bucket_key_enabled", output.BucketKeyEnabled) diff --git a/internal/service/s3/bucket_object_data_source.go b/internal/service/s3/bucket_object_data_source.go index a6b7bb5e0fdf..bbf11662a427 100644 --- a/internal/service/s3/bucket_object_data_source.go +++ b/internal/service/s3/bucket_object_data_source.go @@ -173,7 +173,10 @@ func dataSourceBucketObjectRead(ctx context.Context, d *schema.ResourceData, met } d.SetId(id) - arn := newObjectARN(meta.(*conns.AWSClient).Partition, bucket, key) + arn, err := newObjectARN(meta.(*conns.AWSClient).Partition, bucket, key) + if err != nil { + return sdkdiag.AppendErrorf(diags, "reading S3 Bucket (%s) Object (%s): %s", bucket, key, err) + } d.Set("arn", arn.String()) d.Set("bucket_key_enabled", out.BucketKeyEnabled) diff --git a/internal/service/s3/object.go b/internal/service/s3/object.go index b3cdb004c507..606a9a43dcc6 100644 --- a/internal/service/s3/object.go +++ b/internal/service/s3/object.go @@ -280,7 +280,10 @@ func resourceObjectRead(ctx context.Context, d *schema.ResourceData, meta interf return sdkdiag.AppendErrorf(diags, "reading S3 Object (%s): %s", d.Id(), err) } - arn := newObjectARN(meta.(*conns.AWSClient).Partition, bucket, key) + arn, err := newObjectARN(meta.(*conns.AWSClient).Partition, bucket, key) + if err != nil { + return sdkdiag.AppendErrorf(diags, "reading S3 Object (%s): %s", d.Id(), err) + } d.Set("arn", arn.String()) d.Set("bucket_key_enabled", output.BucketKeyEnabled) @@ -780,37 +783,3 @@ func expandDefaultTags(ctx context.Context, tfMap map[string]interface{}) *tftag return data } - -func newObjectARN(partition string, bucket, key string) arn.ARN { - return arn.ARN{ - Partition: partition, - Service: "s3", - Resource: fmt.Sprintf("%s/%s", bucket, key), - } -} - -type objectARN struct { - arn.ARN - Bucket string - Key string -} - -func parseObjectARN(s string) (objectARN, error) { - arn, err := arn.Parse(s) - if err != nil { - return objectARN{}, err - } - - result := objectARN{ - ARN: arn, - } - - parts := strings.SplitN(arn.Resource, "/", 2) - if len(parts) != 2 { - return objectARN{}, fmt.Errorf("S3 Object ARN: unexpected resource section: %s", arn.Resource) - } - result.Bucket = parts[0] - result.Key = parts[1] - - return result, nil -} diff --git a/internal/service/s3/object_arn.go b/internal/service/s3/object_arn.go new file mode 100644 index 000000000000..99f409a659a8 --- /dev/null +++ b/internal/service/s3/object_arn.go @@ -0,0 +1,64 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package s3 + +import ( + "fmt" + "strings" + + "github.com/YakDriver/regexache" + "github.com/aws/aws-sdk-go-v2/aws/arn" +) + +func newObjectARN(partition string, bucket, key string) (arn.ARN, error) { + if arn.IsARN(bucket) { + bucketARN, err := arn.Parse(bucket) + if err != nil { + return arn.ARN{}, fmt.Errorf("S3 Object ARN: unexpected bucket ARN: %s", bucket) + } + bucketARN.Resource = fmt.Sprintf("%s/%s", bucketARN.Resource, key) + return bucketARN, nil + } + return arn.ARN{ + Partition: partition, + Service: "s3", + Resource: fmt.Sprintf("%s/%s", bucket, key), + }, nil +} + +type objectARN struct { + arn.ARN + Bucket string + Key string +} + +func parseObjectARN(s string) (objectARN, error) { + arn, err := arn.Parse(s) + if err != nil { + return objectARN{}, err + } + + result := objectARN{ + ARN: arn, + } + + if strings.HasPrefix(arn.Resource, "accesspoint/") { + re := regexache.MustCompile(`^(arn:[^:]+:[^:]+:[^:]*:[^:]*:accesspoint/[^/]+)/(.+)$`) + m := re.FindStringSubmatch(s) + if len(m) == 3 { + result.Bucket = m[1] + result.Key = m[2] + return result, nil + } + } + + parts := strings.SplitN(arn.Resource, "/", 2) + if len(parts) != 2 { + return objectARN{}, fmt.Errorf("S3 Object ARN: unexpected resource section: %s", arn.Resource) + } + result.Bucket = parts[0] + result.Key = parts[1] + + return result, nil +} diff --git a/internal/service/s3/object_arn_test.go b/internal/service/s3/object_arn_test.go new file mode 100644 index 000000000000..cbd5cf2faa55 --- /dev/null +++ b/internal/service/s3/object_arn_test.go @@ -0,0 +1,294 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package s3 + +import ( + "testing" + + "github.com/aws/aws-sdk-go-v2/aws/arn" +) + +func TestNewObjectARN_GeneralPurposeBucket(t *testing.T) { + t.Parallel() + + expectedARN := arn.ARN{ + Partition: "test-partition", + Service: "s3", + Resource: "test-bucket/test-key", + } + + arn, err := newObjectARN("test-partition", "test-bucket", "test-key") + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + + equalARN(t, arn, expectedARN) +} + +func TestNewObjectARN_GeneralPurposeBucket_AccessPointInBucketName(t *testing.T) { + t.Parallel() + + expectedARN := arn.ARN{ + Partition: "test-partition", + Service: "s3", + Resource: "test-accesspoint-bucket/test-key", + } + + arn, err := newObjectARN("test-partition", "test-accesspoint-bucket", "test-key") + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + + equalARN(t, arn, expectedARN) +} + +func TestNewObjectARN_AccessPoint(t *testing.T) { + t.Parallel() + + expectedARN := arn.ARN{ + Partition: "test-partition", + Service: "s3", + Region: "us-west-2", + AccountID: "123456789012", + Resource: "accesspoint/test-accesspoint/test-key", + } + + apARN := arn.ARN{ + Partition: "test-partition", + Service: "s3", + Region: "us-west-2", + AccountID: "123456789012", + Resource: "accesspoint/test-accesspoint", + } + + arn, err := newObjectARN("ignored", apARN.String(), "test-key") + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + + equalARN(t, arn, expectedARN) +} + +func TestNewObjectARN_MultiRegionAccessPoint(t *testing.T) { + t.Parallel() + + expectedARN := arn.ARN{ + Partition: "test-partition", + Service: "s3", + AccountID: "123456789012", + Resource: "accesspoint/test-multi-region-accesspoint/test-key", + } + + mrapARN := arn.ARN{ + Partition: "test-partition", + Service: "s3", + AccountID: "123456789012", + Resource: "accesspoint/test-multi-region-accesspoint", + } + + arn, err := newObjectARN("ignored", mrapARN.String(), "test-key") + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + + equalARN(t, arn, expectedARN) +} + +func TestNewObjectARN_ObjectLambdaAccessPoint(t *testing.T) { + t.Parallel() + + expectedARN := arn.ARN{ + Partition: "test-partition", + Service: "s3-object-lambda", + AccountID: "123456789012", + Resource: "accesspoint/test-object-lambda-accesspoint/test-key", + } + + olapARN := arn.ARN{ + Partition: "test-partition", + Service: "s3-object-lambda", + AccountID: "123456789012", + Resource: "accesspoint/test-object-lambda-accesspoint", + } + + arn, err := newObjectARN("ignored", olapARN.String(), "test-key") + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + + equalARN(t, arn, expectedARN) +} + +func TestParseObjectARN_GeneralPurposeBucket(t *testing.T) { + t.Parallel() + + expectedObjectARN := objectARN{ + ARN: arn.ARN{ + Partition: "test-partition", + Service: "s3", + Resource: "test-bucket/test-key", + }, + Bucket: "test-bucket", + Key: "test-key", + } + + oARN, _ := newObjectARN("test-partition", "test-bucket", "test-key") + + parsed, err := parseObjectARN(oARN.String()) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + + equalObjectARN(t, parsed, expectedObjectARN) +} + +func TestParseObjectARN_GeneralPurposeBucket_AccessPointBucketName(t *testing.T) { + t.Parallel() + + expectedObjectARN := objectARN{ + ARN: arn.ARN{ + Partition: "test-partition", + Service: "s3", + Resource: "accesspoint/test-key", + }, + Bucket: "accesspoint", + Key: "test-key", + } + + oARN, _ := newObjectARN("test-partition", "accesspoint", "test-key") + + parsed, err := parseObjectARN(oARN.String()) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + + equalObjectARN(t, parsed, expectedObjectARN) +} + +func TestParseObjectARN_AccessPoint(t *testing.T) { + t.Parallel() + + expectedObjectARN := objectARN{ + ARN: arn.ARN{ + Partition: "test-partition", + Service: "s3", + Region: "us-west-2", + AccountID: "123456789012", + Resource: "accesspoint/test-accesspoint/test-key", + }, + Bucket: "arn:test-partition:s3:us-west-2:123456789012:accesspoint/test-accesspoint", + Key: "test-key", + } + + apARN := arn.ARN{ + Partition: "test-partition", + Service: "s3", + Region: "us-west-2", + AccountID: "123456789012", + Resource: "accesspoint/test-accesspoint", + } + + oARN, _ := newObjectARN("ignored", apARN.String(), "test-key") + + parsed, err := parseObjectARN(oARN.String()) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + + equalObjectARN(t, parsed, expectedObjectARN) +} + +func TestParseObjectARN_MultiRegionAccessPoint(t *testing.T) { + t.Parallel() + + expectedObjectARN := objectARN{ + ARN: arn.ARN{ + Partition: "test-partition", + Service: "s3", + AccountID: "123456789012", + Resource: "accesspoint/test-multi-region-accesspoint/test-key", + }, + Bucket: "arn:test-partition:s3::123456789012:accesspoint/test-multi-region-accesspoint", + Key: "test-key", + } + + mrapARN := arn.ARN{ + Partition: "test-partition", + Service: "s3", + AccountID: "123456789012", + Resource: "accesspoint/test-multi-region-accesspoint", + } + + oARN, _ := newObjectARN("ignored", mrapARN.String(), "test-key") + + parsed, err := parseObjectARN(oARN.String()) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + + equalObjectARN(t, parsed, expectedObjectARN) +} + +func TestParseObjectARN_ObjectLambdaAccessPoint(t *testing.T) { + t.Parallel() + + expectedObjectARN := objectARN{ + ARN: arn.ARN{ + Partition: "test-partition", + Service: "s3-object-lambda", + AccountID: "123456789012", + Resource: "accesspoint/test-object-lambda-accesspoint/test-key", + }, + Bucket: "arn:test-partition:s3-object-lambda::123456789012:accesspoint/test-object-lambda-accesspoint", + Key: "test-key", + } + + olapARN := arn.ARN{ + Partition: "test-partition", + Service: "s3-object-lambda", + AccountID: "123456789012", + Resource: "accesspoint/test-object-lambda-accesspoint", + } + + oARN, _ := newObjectARN("ignored", olapARN.String(), "test-key") + + parsed, err := parseObjectARN(oARN.String()) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + + equalObjectARN(t, parsed, expectedObjectARN) +} + +func equalARN(t *testing.T, a, e arn.ARN) { + t.Helper() + + if a.Partition != e.Partition { + t.Errorf("partition: expected %q, got %q", e.Partition, a.Partition) + } + if a.Service != e.Service { + t.Errorf("service: expected %q, got %q", e.Service, a.Service) + } + if a.Region != e.Region { + t.Errorf("region: expected %q, got %q", e.Region, a.Region) + } + if a.AccountID != e.AccountID { + t.Errorf("account ID: expected %q, got %q", e.AccountID, a.AccountID) + } + if a.Resource != e.Resource { + t.Errorf("resource: expected %q, got %q", e.Resource, a.Resource) + } +} + +func equalObjectARN(t *testing.T, a, e objectARN) { + t.Helper() + + equalARN(t, a.ARN, e.ARN) + if a.Bucket != e.Bucket { + t.Errorf("bucket: expected %q, got %q", e.Bucket, a.Bucket) + } + if a.Key != e.Key { + t.Errorf("key: expected %q, got %q", e.Key, a.Key) + } +} diff --git a/internal/service/s3/object_copy.go b/internal/service/s3/object_copy.go index 059368b84d17..54e9a65a3901 100644 --- a/internal/service/s3/object_copy.go +++ b/internal/service/s3/object_copy.go @@ -358,7 +358,10 @@ func resourceObjectCopyRead(ctx context.Context, d *schema.ResourceData, meta in return sdkdiag.AppendErrorf(diags, "reading S3 Object (%s): %s", d.Id(), err) } - arn := newObjectARN(meta.(*conns.AWSClient).Partition, bucket, key) + arn, err := newObjectARN(meta.(*conns.AWSClient).Partition, bucket, key) + if err != nil { + return sdkdiag.AppendErrorf(diags, "reading S3 Object (%s): %s", d.Id(), err) + } d.Set("arn", arn.String()) d.Set("bucket_key_enabled", output.BucketKeyEnabled) diff --git a/internal/service/s3/object_data_source.go b/internal/service/s3/object_data_source.go index e768e4f8f61d..4c86ca9e8fb1 100644 --- a/internal/service/s3/object_data_source.go +++ b/internal/service/s3/object_data_source.go @@ -205,7 +205,10 @@ func dataSourceObjectRead(ctx context.Context, d *schema.ResourceData, meta inte } d.SetId(id) - arn := newObjectARN(meta.(*conns.AWSClient).Partition, bucket, key) + arn, err := newObjectARN(meta.(*conns.AWSClient).Partition, bucket, key) + if err != nil { + return sdkdiag.AppendErrorf(diags, "reading S3 Bucket (%s) Object (%s): %s", bucket, key, err) + } d.Set("arn", arn.String()) d.Set("bucket_key_enabled", output.BucketKeyEnabled) diff --git a/internal/service/s3/object_test.go b/internal/service/s3/object_test.go index 42c5e1a09533..989d33c1045b 100644 --- a/internal/service/s3/object_test.go +++ b/internal/service/s3/object_test.go @@ -649,6 +649,7 @@ func TestAccS3Object_updatesWithVersioningViaAccessPoint(t *testing.T) { Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &originalObj), testAccCheckObjectBody(&originalObj, "initial versioned object state"), + acctest.CheckResourceAttrRegionalARN(resourceName, "arn", "s3", fmt.Sprintf("accesspoint/%s/updateable-key", rName)), resource.TestCheckResourceAttrPair(resourceName, "bucket", accessPointResourceName, "arn"), resource.TestCheckResourceAttr(resourceName, "etag", "cee4407fa91906284e2a5e5e03e86b1b"), ), @@ -1432,7 +1433,6 @@ func TestAccS3Object_tagsViaAccessPointARN(t *testing.T) { Config: testAccObjectConfig_tagsViaAccessPointARN(rName, key, "stuff"), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj1), - testAccCheckObjectBody(&obj1, "stuff"), resource.TestCheckResourceAttr(resourceName, "tags.%", "3"), resource.TestCheckResourceAttr(resourceName, "tags.Key1", "A@AA"), resource.TestCheckResourceAttr(resourceName, "tags.Key2", "BBB"), @@ -1444,7 +1444,6 @@ func TestAccS3Object_tagsViaAccessPointARN(t *testing.T) { Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj2), testAccCheckObjectVersionIDEquals(&obj2, &obj1), - testAccCheckObjectBody(&obj2, "stuff"), resource.TestCheckResourceAttr(resourceName, "tags.%", "4"), resource.TestCheckResourceAttr(resourceName, "tags.Key2", "B@BB"), resource.TestCheckResourceAttr(resourceName, "tags.Key3", "X X"), @@ -1459,21 +1458,21 @@ func TestAccS3Object_tagsViaAccessPointARN(t *testing.T) { func TestAccS3Object_tagsViaAccessPointAlias(t *testing.T) { ctx := acctest.Context(t) var obj1, obj2 s3.GetObjectOutput - resourceName := "aws_s3_object.object" - key := "test-key" + const resourceName = "aws_s3_object.object" + const key = "test-key" rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { acctest.PreCheck(ctx, t) }, ErrorCheck: acctest.ErrorCheck(t, names.S3ServiceID), ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, - CheckDestroy: testAccCheckObjectDestroy(ctx), + CheckDestroy: acctest.CheckDestroyNoop, // Cannot access the object via the access point alias after the access point is destroyed Steps: []resource.TestStep{ { Config: testAccObjectConfig_tagsViaAccessPointAlias(rName, key, "stuff"), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj1), - testAccCheckObjectBody(&obj1, "stuff"), + acctest.MatchResourceAttrGlobalARNNoAccount(resourceName, "arn", "s3", regexache.MustCompile(fmt.Sprintf(`%s-\w+-s3alias/%s`, rName[:20], key))), resource.TestCheckResourceAttr(resourceName, "tags.%", "3"), resource.TestCheckResourceAttr(resourceName, "tags.Key1", "A@AA"), resource.TestCheckResourceAttr(resourceName, "tags.Key2", "BBB"), @@ -1485,7 +1484,6 @@ func TestAccS3Object_tagsViaAccessPointAlias(t *testing.T) { Check: resource.ComposeAggregateTestCheckFunc( testAccCheckObjectExists(ctx, resourceName, &obj2), testAccCheckObjectVersionIDEquals(&obj2, &obj1), - testAccCheckObjectBody(&obj2, "stuff"), resource.TestCheckResourceAttr(resourceName, "tags.%", "4"), resource.TestCheckResourceAttr(resourceName, "tags.Key2", "B@BB"), resource.TestCheckResourceAttr(resourceName, "tags.Key3", "X X"), @@ -1508,7 +1506,7 @@ func TestAccS3Object_tagsViaMultiRegionAccessPoint(t *testing.T) { PreCheck: func() { acctest.PreCheck(ctx, t) }, ErrorCheck: acctest.ErrorCheck(t, names.S3ServiceID), ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, - CheckDestroy: testAccCheckObjectDestroy(ctx), + CheckDestroy: acctest.CheckDestroyNoop, // Cannot access the object via the access point alias after the access point is destroyed Steps: []resource.TestStep{ { Config: testAccObjectConfig_tagsViaMultiRegionAccessPoint(rName, key, "stuff"), @@ -1539,6 +1537,7 @@ func TestAccS3Object_tagsViaMultiRegionAccessPoint(t *testing.T) { } func TestAccS3Object_tagsViaObjectLambdaAccessPointARN(t *testing.T) { + t.Skip("Accessing Objects via Lambda Access Points is not yet supported") ctx := acctest.Context(t) var obj1, obj2 s3.GetObjectOutput resourceName := "aws_s3_object.object" From 232cd5cc992fa3a71dd74d594b42cf267890b038 Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Fri, 23 Feb 2024 15:11:18 -0800 Subject: [PATCH 24/24] Linting fix --- internal/service/s3/object_arn_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/service/s3/object_arn_test.go b/internal/service/s3/object_arn_test.go index cbd5cf2faa55..d139947a294d 100644 --- a/internal/service/s3/object_arn_test.go +++ b/internal/service/s3/object_arn_test.go @@ -49,7 +49,7 @@ func TestNewObjectARN_AccessPoint(t *testing.T) { expectedARN := arn.ARN{ Partition: "test-partition", Service: "s3", - Region: "us-west-2", + Region: "us-west-2", //lintignore:AWSAT003 AccountID: "123456789012", Resource: "accesspoint/test-accesspoint/test-key", } @@ -57,7 +57,7 @@ func TestNewObjectARN_AccessPoint(t *testing.T) { apARN := arn.ARN{ Partition: "test-partition", Service: "s3", - Region: "us-west-2", + Region: "us-west-2", //lintignore:AWSAT003 AccountID: "123456789012", Resource: "accesspoint/test-accesspoint", } @@ -173,18 +173,18 @@ func TestParseObjectARN_AccessPoint(t *testing.T) { ARN: arn.ARN{ Partition: "test-partition", Service: "s3", - Region: "us-west-2", + Region: "us-west-2", //lintignore:AWSAT003 AccountID: "123456789012", Resource: "accesspoint/test-accesspoint/test-key", }, - Bucket: "arn:test-partition:s3:us-west-2:123456789012:accesspoint/test-accesspoint", + Bucket: "arn:test-partition:s3:us-west-2:123456789012:accesspoint/test-accesspoint", //lintignore:AWSAT003 Key: "test-key", } apARN := arn.ARN{ Partition: "test-partition", Service: "s3", - Region: "us-west-2", + Region: "us-west-2", //lintignore:AWSAT003 AccountID: "123456789012", Resource: "accesspoint/test-accesspoint", }