Skip to content

fix CopyObject not copying tag of current object #11403

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
merged 1 commit into from
Aug 22, 2024
Merged

fix CopyObject not copying tag of current object #11403

merged 1 commit into from
Aug 22, 2024

Conversation

bentsku
Copy link
Contributor

@bentsku bentsku commented Aug 22, 2024 •
edited
Loading

Motivation

As reported in #11389, we did not copy the source object tags when the TaggingDirective was set to COPY or nothing, which default to COPY.

This was due to a small oversight, we were using the parsed version id to create the tag key, but if you don't specify a version id, you get the current object (top of the stack, the one that was last added). Instead of using the parsed version id, we now use the source object version id directly.

Changes

  • use the source object version id to create the object unique key for the tags
  • add a new test regarding versioned bucket and tagging directive to confirm the change: it first copy an object without specifying the version id (to get the current one), then does the same against a specific version id which isn't current

fixes #11389

@bentsku bentsku added aws:s3 Amazon Simple Storage Service semver: patch Non-breaking changes which can be included in patch releases labels Aug 22, 2024
@bentsku bentsku requested a review from cloutierMat August 22, 2024 02:03
@bentsku bentsku self-assigned this Aug 22, 2024
@github-actions GitHub Actions
Copy link

S3 Image Test Results (AMD64 / ARM64)

  2 files    2 suites   3m 48s ⏱️
415 tests 363 ✅  52 💤 0 ❌
830 runs  726 ✅ 104 💤 0 ❌

Results for commit 277421d.

@github-actions GitHub Actions
Copy link

LocalStack Community integration with Pro

    2 files  ±    0      2 suites  ±0   48m 51s ⏱️ - 47m 14s
1 427 tests  - 1 898  1 267 ✅  - 1 664  160 💤  - 234  0 ❌ ±0 
1 429 runs   - 1 898  1 267 ✅  - 1 664  162 💤  - 234  0 ❌ ±0 

Results for commit 277421d. ± Comparison against base commit fd3a5a9.

This pull request removes 1901 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.s3.test_s3.TestS3 ‑ test_s3_copy_tagging_directive_versioned[COPY]
tests.aws.services.s3.test_s3.TestS3 ‑ test_s3_copy_tagging_directive_versioned[None]
tests.aws.services.s3.test_s3.TestS3 ‑ test_s3_copy_tagging_directive_versioned[REPLACE]

@bentsku bentsku added this to the 3.7 milestone Aug 22, 2024
cloutierMat
cloutierMat approved these changes Aug 22, 2024
View reviewed changes
Copy link
Contributor

@cloutierMat cloutierMat left a comment

Choose a reason for hiding this comment

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

LGTM! 🎉

@bentsku bentsku merged commit 2ca5d19 into master Aug 22, 2024
50 checks passed
@bentsku bentsku deleted the fix-s3-copy-tag branch August 22, 2024 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cloutierMat cloutierMat cloutierMat approved these changes

Assignees

@bentsku bentsku

Labels
aws:s3 Amazon Simple Storage Service semver: patch Non-breaking changes which can be included in patch releases
Projects
None yet
Milestone
3.7
Development

Successfully merging this pull request may close these issues.

bug: S3 object tags are not copied if versioning is enabled on a S3 bucket
2 participants
@bentsku @cloutierMat