Skip to content

[AWS S3 Exporter] support canned_acl #37935

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

Closed
KevinThompsonYCRX opened this issue Feb 14, 2025 · 8 comments · Fixed by #37953
Closed

[AWS S3 Exporter] support canned_acl #37935

KevinThompsonYCRX opened this issue Feb 14, 2025 · 8 comments · Fixed by #37953
Labels
enhancement New feature or request exporter/awss3

Comments

@KevinThompsonYCRX
Copy link

Component(s)

No response

Is your feature request related to a problem? Please describe.

The exporter does not allow users to provide an ACL, which many organizations will need.

Describe the solution you'd like

By default this is set to "private", users should be able to pass any of the allowed canned ACLs that Amazon supports

Describe alternatives you've considered

No response

Additional context

The ACL field on the PutObjectInput provides a little more detail, this can be just a straight passthrough from a user input & it would suffice as long as you provide a valid default: https://pkg.go.dev/github.com/aws/aws-sdk-go-v2/service/s3#PutObjectInput

@KevinThompsonYCRX KevinThompsonYCRX added enhancement New feature or request needs triage New item requiring triage labels Feb 14, 2025
Copy link
Contributor

Pinging code owners for exporter/awss3: @atoulme @pdelewski. See Adding Labels via Comments if you do not have permissions to add labels yourself. For example, comment '/label priority:p2 -needs-triaged' to set the priority and remove the needs-triaged label.

@atoulme
Copy link
Contributor

atoulme commented Feb 15, 2025

Alright, would you like to become codeowner to help maintain that feature moving forward?

@kevthompson
Copy link
Contributor

Yeah you can add this account

@atoulme
Copy link
Contributor

atoulme commented Feb 15, 2025

Not so fast ; you need to be an OpenTelemetry member first. You can count on my sponsorship if you go through with it. See here: https://github.com/open-telemetry/community/issues/new?template=membership.md

More info in CONTRIBUTING.md.

@kevthompson
Copy link
Contributor

Looks like I'll need another sponsor as well, I'll look to see if I can contribute somewhere else to find one

@Erog38
Copy link
Contributor

Erog38 commented Apr 11, 2025

This appears to have broken working configurations where S3 buckets do not have an ACL enabled, should this be defaulted to not having the acl decoration enabled unless the option is defined?

@kevthompson
Copy link
Contributor

This appears to have broken working configurations where S3 buckets do not have an ACL enabled, should this be defaulted to not having the acl decoration enabled unless the option is defined?

Based on AWS docs & the description in the Go SDK I though private was the correct default: "By default, all objects are private."

Looking at it again, I see further down it mentions: "Buckets that [disable ACLs] only accept PUT requests that don't specify an ACL or PUT requests that specify bucket owner full control ACLs", so you're correct and this should be left off. I'm out of town this week if you or someone else can take a look at it, and using bucket-owner-full-control should work as an input for now.

@Erog38
Copy link
Contributor

Erog38 commented Apr 14, 2025

No worries dude, I opened #39346 and submitted a PR attached to it #39354 which makes them completely optional.

If you have a sec to stamp your ok with the change, it would likely help get it through.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request exporter/awss3
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants