Skip to content

S3Control: implement Tagging support for S3 Bucket #13435

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.

Sign up for GitHub

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

Jump to bottom
Merged
bentsku merged 1 commit into from
Nov 28, 2025
Merged

S3Control: implement Tagging support for S3 Bucket #13435

bentsku merged 1 commit into from
Nov 28, 2025

Conversation

@bentsku
Copy link
Contributor

@bentsku bentsku commented Nov 28, 2025

Motivation

Following the #13426 report and fixing the first reported issue, when trying the sample, I got a report from Terraform than the tagging operations for S3 were not implemented, but after having a better look I finally realized it was S3 Control:

│ Error: listing tags for S3 (Simple Storage) Bucket (my-test-bucket): operation error S3 Control: ListTagsForResource, https response error StatusCode: 501, RequestID: 8f07ba3c-e0ef-4497-9c57-d4f4d6af4238, HostID: , api error InternalFailure: No moto route for service s3control on path /v20180820/tags/arn%3Aaws%3As3%3A%3A%3Amy-test-bucket found.
│ 
│   with aws_s3_bucket.test,
│   on main.tf line 37, in resource "aws_s3_bucket" "test":
│   37: resource "aws_s3_bucket" "test" {
│

I first got this issue:

│ Error: listing tags for S3 (Simple Storage) Bucket (my-test-bucket): operation error S3 Control: ListTagsForResource, failed to resolve service endpoint, endpoint rule error, AccountId must only contain a-z, A-Z, 0-9 and `-`.
│ 
│   with aws_s3_bucket.test,
│   on main.tf line 37, in resource "aws_s3_bucket" "test":
│   37: resource "aws_s3_bucket" "test" {
│

Because the skip_requesting_account_id = true was set, but worked after removing it, so something to keep note of.

This PR implements implements the 3 tagging operations ListTagsForResource, TagResource and UntagResource for S3 Buckets via S3 Control.
This unblocks the 6.23 AWS Terraform provider and it now works if you check out this PR.

A follow up from this pr will be to implement PutBucketAbac and GetBucketAbac.

Another thing to note is that S3 Control is heavily tied and coupled to S3, and seems to be able to mutate S3 internal state in AWS without going through public APIs, which justify the uses of the s3_stores directly here.

Changes

  • implement the 3 tagging operations in the S3 Control provider
  • add the tests related to the feature

Tests

To fully validate that the issue is fixed for the Terraform provider, you can try the sample given in the linked issue, just remove the skip_requesting_account_id = true line.

@bentsku bentsku added this to the 4.12 milestone Nov 28, 2025
@bentsku bentsku requested review from aidehn, k-a-il and silv-io November 28, 2025 12:26
@bentsku bentsku self-assigned this Nov 28, 2025
@bentsku bentsku added aws:s3 Amazon Simple Storage Service semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases aws:s3control AWS S3 Control docs: skip Pull request does not require documentation changes notes: needed Pull request should be mentioned in the release notes labels Nov 28, 2025
@github-actions
Copy link

github-actions bot commented Nov 28, 2025

LocalStack Community integration with Pro

 2 files  ±    0  2 suites  ±0   36s ⏱️ - 2h 2m 7s
15 tests  - 4 960  5 ✅  - 4 592  10 💤  - 368  0 ❌ ±0 
17 runs   - 4 960  5 ✅  - 4 592  12 💤  - 368  0 ❌ ±0 

Results for commit 7374569. ± Comparison against base commit 9e9f65f.

This pull request removes 4963 and adds 3 tests. Note that renamed tests count towards both. 
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_lambda_dynamodb
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_opensearch_crud
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_search_books
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_setup
tests.aws.scenario.kinesis_firehose.test_kinesis_firehose.TestKinesisFirehoseScenario ‑ test_kinesis_firehose_s3
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_destination_sns
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_infra
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_prefill_dynamodb_table
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input0-SUCCEEDED]
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input1-SUCCEEDED]
…

tests.aws.services.s3control.test_s3control.TestS3ControlTagging ‑ test_tag_lifecycle
tests.aws.services.s3control.test_s3control.TestS3ControlTagging ‑ test_tag_operation_no_bucket
tests.aws.services.s3control.test_s3control.TestS3ControlTagging ‑ test_tag_resource_validation

@github-actions
Copy link

github-actions bot commented Nov 28, 2025

Test Results - Preflight, Unit

22 669 tests  ±0   20 901 ✅ ±0   6m 21s ⏱️ -2s
     1 suites ±0    1 768 💤 ±0 
     1 files   ±0        0 ❌ ±0 

Results for commit 7374569. ± Comparison against base commit 9e9f65f.

@github-actions
Copy link

github-actions bot commented Nov 28, 2025

Test Results (amd64) - Acceptance

7 tests  ±0   5 ✅ ±0   3m 30s ⏱️ +6s
1 suites ±0   2 💤 ±0 
1 files   ±0   0 ❌ ±0 

Results for commit 7374569. ± Comparison against base commit 9e9f65f.

@github-actions
Copy link

github-actions bot commented Nov 28, 2025

Test Results (amd64) - Integration, Bootstrap

 5 files  ±    0   5 suites  ±0   9m 1s ⏱️ - 2h 34m 1s
39 tests  - 5 310  29 ✅  - 4 782  10 💤  - 528  0 ❌ ±0 
45 runs   - 5 310  29 ✅  - 4 782  16 💤  - 528  0 ❌ ±0 

Results for commit 7374569. ± Comparison against base commit 9e9f65f.

This pull request removes 5313 and adds 3 tests. Note that renamed tests count towards both. 
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_lambda_dynamodb
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_opensearch_crud
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_search_books
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_setup
tests.aws.scenario.kinesis_firehose.test_kinesis_firehose.TestKinesisFirehoseScenario ‑ test_kinesis_firehose_s3
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_destination_sns
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_infra
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_prefill_dynamodb_table
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input0-SUCCEEDED]
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input1-SUCCEEDED]
…

tests.aws.services.s3control.test_s3control.TestS3ControlTagging ‑ test_tag_lifecycle
tests.aws.services.s3control.test_s3control.TestS3ControlTagging ‑ test_tag_operation_no_bucket
tests.aws.services.s3control.test_s3control.TestS3ControlTagging ‑ test_tag_resource_validation

@bentsku bentsku marked this pull request as ready for review November 28, 2025 13:02
k-a-il
k-a-il approved these changes Nov 28, 2025
View reviewed changes
Copy link
Contributor

@k-a-il k-a-il left a comment

Choose a reason for hiding this comment

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

Great to see this parity gap to be fixed. I tested S3Control locally, and it works as expected 🚀

@bentsku bentsku merged commit dcf088a into main Nov 28, 2025
63 checks passed
@bentsku bentsku deleted the s3-control-tagging-ops branch November 28, 2025 14:49
@bentsku bentsku mentioned this pull request Nov 28, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@k-a-il k-a-il k-a-il approved these changes

@silv-io silv-io Awaiting requested review from silv-io

@aidehn aidehn Awaiting requested review from aidehn

Assignees

@bentsku bentsku

Labels

aws:s3control AWS S3 Control aws:s3 Amazon Simple Storage Service docs: skip Pull request does not require documentation changes notes: needed Pull request should be mentioned in the release notes semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases

Projects

None yet

Milestone

4.12

Development

Successfully merging this pull request may close these issues.

3 participants

@bentsku @k-a-il