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

226: Add Expected Bucket Owner #238

Merged
merged 5 commits into from
Jul 30, 2024

Conversation

houserx-ioannis
Copy link
Contributor

@houserx-ioannis houserx-ioannis commented Jul 10, 2024

what

This PR addresses #226 about not being able to specify expected bucket owner in various S3 resources.

why

From AWS docs:

Because Amazon S3 identifies buckets based on their names, an application that uses an incorrect bucket name in a request could inadvertently perform operations against a different bucket than expected. To help avoid unintentional bucket interactions in situations like this, you can use bucket owner condition. Bucket owner condition enables you to verify that the target bucket is owned by the expected AWS account, providing an additional layer of assurance that your S3 operations are having the effects you intend.

references

@houserx-ioannis houserx-ioannis requested review from a team as code owners July 10, 2024 06:53
@mergify mergify bot added the triage Needs triage label Jul 10, 2024
Copy link
Member

@Gowiem Gowiem left a comment

Choose a reason for hiding this comment

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

Generally looking good! Unfortunately, we need to run some automation to pass our tests. Mind making the suggested change and doing the following locally, adding + committing the result, and pushing to your branch?

make init
make readme

Thanks!

variables.tf Outdated Show resolved Hide resolved
main.tf Show resolved Hide resolved
@houserx-jmcc
Copy link
Contributor

houserx-jmcc commented Jul 24, 2024

Generally looking good! Unfortunately, we need to run some automation to pass our tests. Mind making the suggested change and doing the following locally, adding + committing the result, and pushing to your branch?

make init
make readme

Thanks!

@Gowiem I pushed a commit with these updates, thank you!

@Gowiem
Copy link
Member

Gowiem commented Jul 30, 2024

/terratest

Copy link
Member

@Gowiem Gowiem left a comment

Choose a reason for hiding this comment

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

:shipit:

@Gowiem Gowiem added minor New features that do not break anything feature New functionality labels Jul 30, 2024
@mergify mergify bot removed the triage Needs triage label Jul 30, 2024
@Gowiem Gowiem added the needs-cloudposse Needs Cloud Posse assistance label Jul 30, 2024
Copy link

mergify bot commented Jul 30, 2024

Important

Cloud Posse Engineering Team Review Required

This pull request modifies files that require Cloud Posse's review. Please be patient, and a core maintainer will review your changes.

To expedite this process, reach out to us on Slack in the #pr-reviews channel.

@Gowiem Gowiem enabled auto-merge (squash) July 30, 2024 19:25
@Gowiem Gowiem merged commit 42320bf into cloudposse:main Jul 30, 2024
35 of 53 checks passed
@Gowiem Gowiem removed the needs-cloudposse Needs Cloud Posse assistance label Jul 30, 2024
Copy link

These changes were released in v4.4.0.

@houserx-jmcc houserx-jmcc deleted the issue_226_add_expected_bucket_owner branch July 30, 2024 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New functionality minor New features that do not break anything
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enhancement: include expected_bucket_owner
3 participants