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

feat: Add support for ACL configuration in S3 Destination #17953

Merged
merged 10 commits into from
May 15, 2024

Conversation

caiorcferreira
Copy link
Contributor

Summary

This pull request adds ACL configuration support to the S3 destination plugin. This feature allows to specify the Access Control List (ACL) policy in the S3 destination plugin.

⚠️ This is a contribution to a plugin, and I have read the relevant section of the contribution guidelines 🧑‍🎓 before submitting this PR ⚠️

Problem Addressed

Before this PR, users could not specify an ACL policy when putting an object in an S3 bucket using the S3 destination plugin. This limitation could prevent users from sending data across AWS accounts.

@caiorcferreira caiorcferreira requested a review from a team as a code owner May 13, 2024 20:57
@caiorcferreira caiorcferreira requested review from maaarcelino and removed request for a team May 13, 2024 20:57
@caiorcferreira caiorcferreira changed the title S3 Destination: add support for ACL configuration feat: add support for ACL configuration in S3 Destination May 13, 2024
@caiorcferreira caiorcferreira changed the title feat: add support for ACL configuration in S3 Destination feat: Add support for ACL configuration in S3 Destination May 13, 2024
@erezrokah erezrokah requested review from bbernays and removed request for maaarcelino May 14, 2024 10:39
@bbernays
Copy link
Collaborator

@caiorcferreira - Overall this looks good, would you mind updating it so that the spacing for the files that you updated isn't changed from tabs/spaces? This will help reduce the size of the change

@caiorcferreira
Copy link
Contributor Author

@bbernays just fixed the spacing!

@caiorcferreira
Copy link
Contributor Author

caiorcferreira commented May 15, 2024

Hi @bbernays, could you give me some help? This job failed due to an authentication error. It does not look like related to the PR.

@bbernays
Copy link
Collaborator

Hi @bbernays, could you give me some help? This job failed due to an authentication error. It does not look like related to the PR.

Hi @caiorcferreira - Because the tests need access to our AWS account to test the destination against a real bucket. I am working on copying your branch to my own branch then will run the tests and once those tests pass, we will force merge your pr

@erezrokah erezrokah merged commit 5dbf8da into cloudquery:main May 15, 2024
9 of 11 checks passed
kodiakhq bot pushed a commit that referenced this pull request May 15, 2024
🤖 I have created a release *beep* *boop*
---


## [7.1.0](plugins-destination-s3-v7.0.1...plugins-destination-s3-v7.1.0) (2024-05-15)


### Features

* Add support for ACL configuration in S3 Destination ([#17953](#17953)) ([5dbf8da](5dbf8da))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
@bbernays
Copy link
Collaborator

@caiorcferreira - Thank you for this contribution, we have kicked off the release of v7.1.0 of the S3 destination plugin. It should be available in the CloudQuery Hub in about 20 minutes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants