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

[exporter/awss3exporter] Add canned_acl as an input #37953

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

kevthompson
Copy link

@kevthompson kevthompson commented Feb 15, 2025

Description

Adds an additional input canned_acl to use when uploading S3 objects. Followed the same patterns as the storage_class option.

Link to tracking issue

Fixes #37935

Testing

Updated config in all test files and added testdata/config-s3_canned-acl.yaml to validate input.

Tested against personal S3 bucket, with various canned ACLs & with ACLs disabled at the bucket level

Documentation

Updated README to include new input with default value.

@kevthompson kevthompson requested review from atoulme and a team as code owners February 15, 2025 19:41
Copy link

linux-foundation-easycla bot commented Feb 15, 2025

CLA Signed

The committers listed above are authorized under a signed CLA.

@atoulme
Copy link
Contributor

atoulme commented Feb 15, 2025

Changes look good to me, but canned seems like it would be too buttery: https://gaushalausa.org/product/a2-ghee-2lbs/

@kevthompson
Copy link
Author

Need to build locally & test against an actual AWS bucket still, may also need to allow an empty string for the ACL.

@kevthompson
Copy link
Author

kevthompson commented Feb 24, 2025

Need to build locally & test against an actual AWS bucket still, may also need to allow an empty string for the ACL.

Validated against my own s3 bucket & behavior all seems correct. If ACLs are disabled at the bucket level, uploads work unless you specify an ACL.

@atoulme
Copy link
Contributor

atoulme commented Feb 28, 2025

I am kicking the build, please hold off merging main, I have to kick the build off every time you do :)

@atoulme atoulme added the ready to merge Code review completed; ready to merge by maintainers label Feb 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exporter/awss3 ready to merge Code review completed; ready to merge by maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[AWS S3 Exporter] support canned_acl
2 participants