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 cross-account for CopyObject #9806

Merged
merged 2 commits into from Dec 6, 2023
Merged

add cross-account for CopyObject #9806

merged 2 commits into from Dec 6, 2023

Conversation

bentsku
Copy link
Contributor

@bentsku bentsku commented Dec 5, 2023

Motivation

As reported here: https://discuss.localstack.cloud/t/nosuchbucket-error-when-copying-file/640
The user had an issue where the bucket was created by another account, and he would then try to call CopyObject on it.
I don't really have the setup for testing multi-account against AWS, but it feels like as S3 buckets are global, this should be possible and only a matter of IAM permission / ACL.

https://docs.aws.amazon.com/AmazonS3/latest/API/API_CopyObject.html

Changes

  • make CopyObject use cross-account access while getting the S3 buckets
  • add a multi account test for it

@bentsku bentsku added aws:s3 Amazon Simple Storage Service area: multi-account Multi-tenancy in LocalStack semver: patch Non-breaking changes which can be included in patch releases labels Dec 5, 2023
@bentsku bentsku self-assigned this Dec 5, 2023
Copy link

github-actions bot commented Dec 5, 2023

S3 Image Test Results (AMD64 / ARM64)

    2 files  ±0      2 suites  ±0   3m 11s ⏱️ +4s
377 tests +1  327 ✔️ +1    50 💤 ±0  0 ±0 
754 runs  +2  654 ✔️ +2  100 💤 ±0  0 ±0 

Results for commit 227069c. ± Comparison against base commit ce33f7a.

@coveralls
Copy link

Coverage Status

coverage: 84.224% (+0.02%) from 84.209%
when pulling 227069c on fix-s3-copy-obj
into ce33f7a on master.

Copy link

github-actions bot commented Dec 5, 2023

LocalStack Community integration with Pro

       2 files         2 suites   1h 10m 45s ⏱️
2 384 tests 2 140 ✔️ 244 💤 0
2 385 runs  2 140 ✔️ 245 💤 0

Results for commit 227069c.

Copy link
Contributor

@macnev2013 macnev2013 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. LGTM 🚀

@bentsku bentsku merged commit 0595d87 into master Dec 6, 2023
32 checks passed
@bentsku bentsku deleted the fix-s3-copy-obj branch December 6, 2023 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: multi-account Multi-tenancy in LocalStack aws:s3 Amazon Simple Storage Service semver: patch Non-breaking changes which can be included in patch releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants