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

add ignore_default_tags option to s3 object #33262

Merged
merged 25 commits into from
Oct 31, 2023

Conversation

walteh
Copy link
Contributor

@walteh walteh commented Aug 31, 2023

Description

Introduces an ignore_default_tags option to the aws_s3_object resource. This option allows users to ignore default tags set at the provider level when managing S3 objects.

Organizations that utilize a large number of default tags can encounter limitations when working with S3 objects due to the maximum tag limit (currently 10). This change enables users to more easily work with S3 objects in such environments by providing the option to ignore default tags, thus avoiding the need for aliasing providers to mitigate the tag limitation.

Relations

Closes #19895
Closes #21273

References

I tried my best to not directly manipulate the tags_all object, but could not get around doing it once.

Output from Acceptance Testing

% make testacc TESTS=TestAccS3Object_ignoreDefaultTags PKG=s3
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/s3/... -v -count 1 -parallel 20 -run='TestAccS3Object_ignoreDefaultTags'  -timeout 180m
=== RUN   TestAccS3Object_ignoreDefaultTags_create
=== PAUSE TestAccS3Object_ignoreDefaultTags_create
=== RUN   TestAccS3Object_ignoreDefaultTags_existing
=== PAUSE TestAccS3Object_ignoreDefaultTags_existing
=== CONT  TestAccS3Object_ignoreDefaultTags_create
=== CONT  TestAccS3Object_ignoreDefaultTags_existing
--- PASS: TestAccS3Object_ignoreDefaultTags_existing (143.15s)
--- PASS: TestAccS3Object_ignoreDefaultTags_create (144.11s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/s3 147.881s

@github-actions
Copy link

Community Note

Voting for Prioritization

  • Please vote on this pull request by adding a 👍 reaction to the original post to help the community and maintainers prioritize this pull request.
  • Please see our prioritization guide for information on how we prioritize.
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.

For Submitters

  • Review the contribution guide relating to the type of change you are making to ensure all of the necessary steps have been taken.
  • For new resources and data sources, use skaff to generate scaffolding with comments detailing common expectations.
  • Whether or not the branch has been rebased will not impact prioritization, but doing so is always a welcome surprise.

@github-actions github-actions bot added size/L Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. service/s3 Issues and PRs that pertain to the s3 service. labels Aug 31, 2023
@terraform-aws-provider terraform-aws-provider bot added the needs-triage Waiting for first response or review from a maintainer. label Aug 31, 2023
@walteh walteh marked this pull request as ready for review August 31, 2023 20:58
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Welcome @walteh 👋

It looks like this is your first Pull Request submission to the Terraform AWS Provider! If you haven’t already done so please make sure you have checked out our CONTRIBUTOR guide and FAQ to make sure your contribution is adhering to best practice and has all the necessary elements in place for a successful approval.

Also take a look at our FAQ which details how we prioritize Pull Requests for inclusion.

Thanks again, and welcome to the community! 😃

@justinretzolk justinretzolk added bug Addresses a defect in current functionality. and removed needs-triage Waiting for first response or review from a maintainer. labels Sep 6, 2023
This reverts commit 4ef762a.
% make testacc TESTARGS='-run=TestAccS3Object_tags\|TestAccS3Object_DefaultTags_providerOnly' PKG=s3 ACCTEST_PARALLELISM=2
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/s3/... -v -count 1 -parallel 2  -run=TestAccS3Object_tags\|TestAccS3Object_DefaultTags_providerOnly -timeout 360m
=== RUN   TestAccS3Object_tags
=== PAUSE TestAccS3Object_tags
=== RUN   TestAccS3Object_tagsLeadingSingleSlash
=== PAUSE TestAccS3Object_tagsLeadingSingleSlash
=== RUN   TestAccS3Object_tagsLeadingMultipleSlashes
=== PAUSE TestAccS3Object_tagsLeadingMultipleSlashes
=== RUN   TestAccS3Object_tagsMultipleSlashes
=== PAUSE TestAccS3Object_tagsMultipleSlashes
=== RUN   TestAccS3Object_DefaultTags_providerOnly
=== PAUSE TestAccS3Object_DefaultTags_providerOnly
=== CONT  TestAccS3Object_tags
=== CONT  TestAccS3Object_tagsMultipleSlashes
--- PASS: TestAccS3Object_tagsMultipleSlashes (99.12s)
=== CONT  TestAccS3Object_tagsLeadingMultipleSlashes
--- PASS: TestAccS3Object_tags (101.71s)
=== CONT  TestAccS3Object_tagsLeadingSingleSlash
--- PASS: TestAccS3Object_tagsLeadingMultipleSlashes (94.73s)
=== CONT  TestAccS3Object_DefaultTags_providerOnly
--- PASS: TestAccS3Object_tagsLeadingSingleSlash (98.30s)
--- PASS: TestAccS3Object_DefaultTags_providerOnly (41.48s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/s3	240.436s
% make testacc TESTARGS='-run=TestAccS3Object_DefaultTags_providerAndResource' PKG=s3 ACCTEST_PARALLELISM=2
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/s3/... -v -count 1 -parallel 2  -run=TestAccS3Object_DefaultTags_providerAndResource -timeout 360m
=== RUN   TestAccS3Object_DefaultTags_providerAndResource
=== PAUSE TestAccS3Object_DefaultTags_providerAndResource
=== CONT  TestAccS3Object_DefaultTags_providerAndResource
--- PASS: TestAccS3Object_DefaultTags_providerAndResource (47.65s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/s3	53.235s
% make testacc TESTARGS='-run=TestAccS3Object_DefaultTags_providerAndResourceWithOverride' PKG=s3 ACCTEST_PARALLELISM=2
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/s3/... -v -count 1 -parallel 2  -run=TestAccS3Object_DefaultTags_providerAndResourceWithOverride -timeout 360m
=== RUN   TestAccS3Object_DefaultTags_providerAndResourceWithOverride
=== PAUSE TestAccS3Object_DefaultTags_providerAndResourceWithOverride
=== CONT  TestAccS3Object_DefaultTags_providerAndResourceWithOverride
--- PASS: TestAccS3Object_DefaultTags_providerAndResourceWithOverride (47.28s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/s3	52.702s
@ewbankkit ewbankkit added enhancement Requests to existing resources that expand the functionality or scope. and removed bug Addresses a defect in current functionality. labels Oct 31, 2023
Copy link
Contributor

@ewbankkit ewbankkit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚀.

% make testacc TESTARGS='-run=TestAccS3Object_' PKG=s3 ACCTEST_PARALLELISM=2 
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/s3/... -v -count 1 -parallel 2  -run=TestAccS3Object_ -timeout 360m
=== RUN   TestAccS3Object_basic
=== PAUSE TestAccS3Object_basic
=== RUN   TestAccS3Object_upgradeFromV4
=== PAUSE TestAccS3Object_upgradeFromV4
=== RUN   TestAccS3Object_source
=== PAUSE TestAccS3Object_source
=== RUN   TestAccS3Object_content
=== PAUSE TestAccS3Object_content
=== RUN   TestAccS3Object_etagEncryption
=== PAUSE TestAccS3Object_etagEncryption
=== RUN   TestAccS3Object_contentBase64
=== PAUSE TestAccS3Object_contentBase64
=== RUN   TestAccS3Object_sourceHashTrigger
=== PAUSE TestAccS3Object_sourceHashTrigger
=== RUN   TestAccS3Object_withContentCharacteristics
=== PAUSE TestAccS3Object_withContentCharacteristics
=== RUN   TestAccS3Object_nonVersioned
=== PAUSE TestAccS3Object_nonVersioned
=== RUN   TestAccS3Object_updates
=== PAUSE TestAccS3Object_updates
=== RUN   TestAccS3Object_updateSameFile
=== PAUSE TestAccS3Object_updateSameFile
=== RUN   TestAccS3Object_updatesWithVersioning
=== PAUSE TestAccS3Object_updatesWithVersioning
=== RUN   TestAccS3Object_updatesWithVersioningViaAccessPoint
=== PAUSE TestAccS3Object_updatesWithVersioningViaAccessPoint
=== RUN   TestAccS3Object_kms
=== PAUSE TestAccS3Object_kms
=== RUN   TestAccS3Object_sse
=== PAUSE TestAccS3Object_sse
=== RUN   TestAccS3Object_acl
=== PAUSE TestAccS3Object_acl
=== RUN   TestAccS3Object_metadata
=== PAUSE TestAccS3Object_metadata
=== RUN   TestAccS3Object_storageClass
=== PAUSE TestAccS3Object_storageClass
=== RUN   TestAccS3Object_tags
=== PAUSE TestAccS3Object_tags
=== RUN   TestAccS3Object_tagsLeadingSingleSlash
=== PAUSE TestAccS3Object_tagsLeadingSingleSlash
=== RUN   TestAccS3Object_tagsLeadingMultipleSlashes
=== PAUSE TestAccS3Object_tagsLeadingMultipleSlashes
=== RUN   TestAccS3Object_tagsMultipleSlashes
=== PAUSE TestAccS3Object_tagsMultipleSlashes
=== RUN   TestAccS3Object_DefaultTags_providerOnly
=== PAUSE TestAccS3Object_DefaultTags_providerOnly
=== RUN   TestAccS3Object_DefaultTags_providerAndResource
=== PAUSE TestAccS3Object_DefaultTags_providerAndResource
=== RUN   TestAccS3Object_DefaultTags_providerAndResourceWithOverride
=== PAUSE TestAccS3Object_DefaultTags_providerAndResourceWithOverride
=== RUN   TestAccS3Object_objectLockLegalHoldStartWithNone
=== PAUSE TestAccS3Object_objectLockLegalHoldStartWithNone
=== RUN   TestAccS3Object_objectLockLegalHoldStartWithOn
=== PAUSE TestAccS3Object_objectLockLegalHoldStartWithOn
=== RUN   TestAccS3Object_objectLockRetentionStartWithNone
=== PAUSE TestAccS3Object_objectLockRetentionStartWithNone
=== RUN   TestAccS3Object_objectLockRetentionStartWithSet
=== PAUSE TestAccS3Object_objectLockRetentionStartWithSet
=== RUN   TestAccS3Object_objectBucketKeyEnabled
=== PAUSE TestAccS3Object_objectBucketKeyEnabled
=== RUN   TestAccS3Object_bucketBucketKeyEnabled
=== PAUSE TestAccS3Object_bucketBucketKeyEnabled
=== RUN   TestAccS3Object_defaultBucketSSE
=== PAUSE TestAccS3Object_defaultBucketSSE
=== RUN   TestAccS3Object_ignoreTags
=== PAUSE TestAccS3Object_ignoreTags
=== RUN   TestAccS3Object_checksumAlgorithm
=== PAUSE TestAccS3Object_checksumAlgorithm
=== RUN   TestAccS3Object_keyWithSlashesMigrated
=== PAUSE TestAccS3Object_keyWithSlashesMigrated
=== CONT  TestAccS3Object_basic
=== CONT  TestAccS3Object_tags
--- PASS: TestAccS3Object_basic (46.29s)
=== CONT  TestAccS3Object_objectLockRetentionStartWithNone
--- PASS: TestAccS3Object_tags (103.28s)
=== CONT  TestAccS3Object_keyWithSlashesMigrated
    object_test.go:1625: TestStep 1/2 running init: exit status 1
        
        Error: Failed to query available provider packages
        
        Could not retrieve the list of available versions for provider hashicorp/aws:
        no available releases match the given constraints 5.16.0
        
--- FAIL: TestAccS3Object_keyWithSlashesMigrated (1.48s)
=== CONT  TestAccS3Object_checksumAlgorithm
--- PASS: TestAccS3Object_objectLockRetentionStartWithNone (92.60s)
=== CONT  TestAccS3Object_ignoreTags
--- PASS: TestAccS3Object_checksumAlgorithm (65.20s)
=== CONT  TestAccS3Object_defaultBucketSSE
--- PASS: TestAccS3Object_ignoreTags (59.95s)
=== CONT  TestAccS3Object_bucketBucketKeyEnabled
--- PASS: TestAccS3Object_defaultBucketSSE (33.39s)
=== CONT  TestAccS3Object_objectBucketKeyEnabled
--- PASS: TestAccS3Object_bucketBucketKeyEnabled (26.24s)
=== CONT  TestAccS3Object_objectLockRetentionStartWithSet
--- PASS: TestAccS3Object_objectBucketKeyEnabled (44.19s)
=== CONT  TestAccS3Object_updates
--- PASS: TestAccS3Object_updates (56.55s)
=== CONT  TestAccS3Object_storageClass
--- PASS: TestAccS3Object_objectLockRetentionStartWithSet (103.51s)
=== CONT  TestAccS3Object_metadata
--- PASS: TestAccS3Object_metadata (90.03s)
=== CONT  TestAccS3Object_acl
--- PASS: TestAccS3Object_storageClass (130.32s)
=== CONT  TestAccS3Object_sse
--- PASS: TestAccS3Object_sse (44.98s)
=== CONT  TestAccS3Object_kms
--- PASS: TestAccS3Object_acl (99.37s)
=== CONT  TestAccS3Object_updatesWithVersioningViaAccessPoint
--- PASS: TestAccS3Object_kms (50.24s)
=== CONT  TestAccS3Object_updatesWithVersioning
--- PASS: TestAccS3Object_updatesWithVersioning (55.50s)
=== CONT  TestAccS3Object_updateSameFile
--- PASS: TestAccS3Object_updatesWithVersioningViaAccessPoint (71.90s)
=== CONT  TestAccS3Object_contentBase64
--- PASS: TestAccS3Object_contentBase64 (41.25s)
=== CONT  TestAccS3Object_nonVersioned
    acctest.go:1663: skipping test; environment variable TF_ACC_ASSUME_ROLE_ARN must be set. Usage: Amazon Resource Name (ARN) of existing IAM Role to assume for testing restricted permissions
--- SKIP: TestAccS3Object_nonVersioned (0.00s)
=== CONT  TestAccS3Object_withContentCharacteristics
--- PASS: TestAccS3Object_updateSameFile (53.13s)
=== CONT  TestAccS3Object_sourceHashTrigger
--- PASS: TestAccS3Object_withContentCharacteristics (41.74s)
=== CONT  TestAccS3Object_DefaultTags_providerAndResource
--- PASS: TestAccS3Object_sourceHashTrigger (49.28s)
=== CONT  TestAccS3Object_objectLockLegalHoldStartWithOn
--- PASS: TestAccS3Object_DefaultTags_providerAndResource (49.75s)
=== CONT  TestAccS3Object_objectLockLegalHoldStartWithNone
--- PASS: TestAccS3Object_objectLockLegalHoldStartWithOn (67.87s)
=== CONT  TestAccS3Object_DefaultTags_providerAndResourceWithOverride
--- PASS: TestAccS3Object_DefaultTags_providerAndResourceWithOverride (55.91s)
=== CONT  TestAccS3Object_content
--- PASS: TestAccS3Object_objectLockLegalHoldStartWithNone (97.39s)
=== CONT  TestAccS3Object_etagEncryption
--- PASS: TestAccS3Object_content (46.65s)
=== CONT  TestAccS3Object_tagsMultipleSlashes
--- PASS: TestAccS3Object_etagEncryption (46.39s)
=== CONT  TestAccS3Object_DefaultTags_providerOnly
--- PASS: TestAccS3Object_DefaultTags_providerOnly (46.65s)
=== CONT  TestAccS3Object_source
--- PASS: TestAccS3Object_source (46.89s)
=== CONT  TestAccS3Object_upgradeFromV4
    object_test.go:173: TestStep 1/2 running init: exit status 1
        
        Error: Failed to query available provider packages
        
        Could not retrieve the list of available versions for provider hashicorp/aws:
        no available releases match the given constraints 4.67.0
        
--- PASS: TestAccS3Object_tagsMultipleSlashes (102.55s)
=== CONT  TestAccS3Object_tagsLeadingMultipleSlashes
--- FAIL: TestAccS3Object_upgradeFromV4 (1.63s)
=== CONT  TestAccS3Object_tagsLeadingSingleSlash
--- PASS: TestAccS3Object_tagsLeadingMultipleSlashes (107.88s)
--- PASS: TestAccS3Object_tagsLeadingSingleSlash (110.41s)
FAIL
FAIL	github.com/hashicorp/terraform-provider-aws/internal/service/s3	1077.435s
FAIL
make: *** [testacc] Error 1

Failures are unrelated to this change.

@ewbankkit
Copy link
Contributor

@walteh Thanks for the contribution 🎉 👏.
To be able to extend this functionality to additional resources and for future ability to override additional provider-level configuration settings, the maintainers decided to go with a new configuration block, override_provider with a default_tags nested block (see the modified aws_s3_object documentation).
We reused much of the logic you had implemented.
We intend to make handling of this new block's more centralized, but as we need exactly this default_tags ignore functionality for a new feature to be GA at re:Invent, have implemented in this single resource for now.

@ewbankkit ewbankkit added the tags Pertains to resource tagging. label Oct 31, 2023
@ewbankkit ewbankkit merged commit 95e9dfc into hashicorp:main Oct 31, 2023
63 checks passed
@github-actions github-actions bot added this to the v5.24.0 milestone Oct 31, 2023
Copy link

github-actions bot commented Nov 2, 2023

This functionality has been released in v5.24.0 of the Terraform AWS Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

Copy link

github-actions bot commented Dec 3, 2023

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement Requests to existing resources that expand the functionality or scope. service/s3 Issues and PRs that pertain to the s3 service. size/L Managed by automation to categorize the size of a PR. tags Pertains to resource tagging. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
3 participants