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

Adds ability to block public access ACLs and Policies on the created S3 bucket #23

Closed
wants to merge 2 commits into from

Conversation

Gowiem
Copy link
Member

@Gowiem Gowiem commented May 8, 2020

Info

Adds usage of the aws_s3_bucket_public_access_block resource to disallow public access policies and ACLs on the created bucket. Adds flag variable enable_block_public_access to control usage of that resource.

@adamcrews
Copy link
Contributor

I'd like to request that the individual blocking options are exposed. I did that with this PR on the log-storage module: cloudposse/terraform-aws-s3-log-storage#25

In our environment we occasionally use the flexibility of the various block options. I also believe it would be good for these modules to be consistent in the way they are configured since they are very similar modules.

@Gowiem
Copy link
Member Author

Gowiem commented Jun 5, 2020

@adamcrews Seems legit. I didn't have need for specifying the underlying options, but I can get behind others needing to do so. I'll try to update my branch next time I get the chance.

@adamcrews
Copy link
Contributor

adamcrews commented Jun 5, 2020

@Gowiem I'm happy to whip up a new PR if you don't mind me stepping on your toes.

@Gowiem
Copy link
Member Author

Gowiem commented Jun 5, 2020

@adamcrews Go for it. 👍

@osterman
Copy link
Member

@Gowiem we're finally catching up on PRs. Moved tests to GitHub actions.

Can fix the merge conflict?

@adamcrews
Copy link
Contributor

This was replaced by #27, closing this PR. Thanks @Gowiem for the contribution.

@adamcrews adamcrews closed this Jun 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants