Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow empty values for tags for S3 resources #35710

Merged
merged 25 commits into from Mar 4, 2024
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
2c9340a
Removes unneeded `nosemgrep`
gdavison Feb 7, 2024
26322c7
Adds acceptance tests for empty tag values for `aws_vpc` and `aws_lb`
gdavison Feb 8, 2024
6705558
Implements transparent tagging for `aws_s3_bucket`
gdavison Feb 8, 2024
85a6129
Simplifies tag on create
gdavison Feb 8, 2024
56ad462
Linting fix
gdavison Feb 8, 2024
dd170d7
Adds attribute `arn` to S3 Object and variants
gdavison Feb 9, 2024
fc99997
Implements transparent tagging for `aws_s3_object`
gdavison Feb 9, 2024
9827a8f
Removes explicit tag read from `aws_s3_bucket`
gdavison Feb 9, 2024
c315542
Implements transparent tagging for `aws_s3_object_copy`
gdavison Feb 9, 2024
4fc6e9e
Implements transparent tagging for `aws_s3_bucket_object`
gdavison Feb 10, 2024
ced5403
Cleanup
gdavison Feb 10, 2024
4961d64
Linting fixes
gdavison Feb 12, 2024
b32b186
Adds CHANGELOG entry
gdavison Feb 13, 2024
a067366
Fixes error reading object in directory bucket
gdavison Feb 13, 2024
5ca4425
Cleanup
gdavison Feb 13, 2024
2e646e4
Merge branch 'main' into b-s3-empty-tag
ewbankkit Feb 22, 2024
8022adf
Add 'TestAccS3Object_tagsViaAccessPoint'.
ewbankkit Feb 22, 2024
360c0f2
'TestAccS3Object_tagsViaAccessPoint' -> 'TestAccS3Object_tagsViaAcces…
ewbankkit Feb 22, 2024
1f4e15f
Add 'TestAccS3Object_tagsViaAccessPointAlias'.
ewbankkit Feb 22, 2024
6aa9a41
Add 'TestAccS3Object_tagsViaMultiRegionAccessPoint'.
ewbankkit Feb 22, 2024
bb18355
r/aws_s3_object: Tidy up some acceptance test configurations.
ewbankkit Feb 22, 2024
f839e33
Add 'TestAccS3Object_tagsViaObjectLambdaAccessPointARN'.
ewbankkit Feb 22, 2024
b93e82b
Updates `TestAccS3Object_directoryBucket` and `TestAccS3ObjectCopy_di…
gdavison Feb 22, 2024
a218262
Updates handling for Objects accessed via access points
gdavison Feb 23, 2024
232cd5c
Linting fix
gdavison Feb 23, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
101 changes: 101 additions & 0 deletions internal/service/ec2/vpc_test.go
Expand Up @@ -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,
Expand Down
104 changes: 104 additions & 0 deletions internal/service/elbv2/load_balancer_test.go
Expand Up @@ -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
Expand Down
46 changes: 6 additions & 40 deletions internal/service/s3/bucket.go
Expand Up @@ -45,11 +45,11 @@ 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")
// @Tags
// @Tags(identifierAttribute="bucket", resourceType="Bucket")
func resourceBucket() *schema.Resource {
return &schema.Resource{
CreateWithoutTimeout: resourceBucketCreate,
Expand Down Expand Up @@ -774,6 +774,10 @@ func resourceBucketCreate(ctx context.Context, d *schema.ResourceData, meta inte
return sdkdiag.AppendErrorf(diags, "waiting for S3 Bucket (%s) create: %s", d.Id(), err)
}

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)...)
}

Expand Down Expand Up @@ -1138,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
}

Expand Down Expand Up @@ -1557,23 +1540,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)...)
}

Expand Down
30 changes: 11 additions & 19 deletions internal/service/s3/bucket_object.go
Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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)
Expand All @@ -246,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
}

Expand Down Expand Up @@ -325,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)...)
}

Expand Down Expand Up @@ -380,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

Expand Down Expand Up @@ -482,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())
Expand Down
7 changes: 7 additions & 0 deletions internal/service/s3/bucket_object_data_source.go
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down