-
Notifications
You must be signed in to change notification settings - Fork 4k
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
fix(codepipeline): default cross-region S3 buckets allow public access #17722
Conversation
…fault. The cross region S3 buckets that are created should have block public access by default. fixes aws#16411
Same approach as for the artifact bucket: https://github.com/aws/aws-cdk/blob/master/packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts#L382 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution @david-richer-adsk!
packages/@aws-cdk/aws-codepipeline/lib/private/cross-region-support-stack.ts
Outdated
Show resolved
Hide resolved
Pull request has been modified.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We also need a unit test for this one 🙂.
Does this look like the right place to add a check? https://github.com/aws/aws-cdk/blob/master/packages/%40aws-cdk/aws-codepipeline/test/cross-env.test.ts#L132 |
Yes, that's probably a good spot. You can use the |
Pull request has been modified.
Should be good to go, let me know if anything is missing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good @david-richer-adsk! One question.
Co-authored-by: Adam Ruka <[email protected]>
Pull request has been modified.
@skinny85 ready to be merged? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution @david-richer-adsk!
Yep! In the future, make sure to re-request my review after you're done pushing your changes (there's a button in the top-right corner of the PR page, next to my avatar), this way I won't miss it 🙂. Thanks, |
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
Will do! |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
The cross region S3 buckets that are created should have block public access by default.
Fixes #16411
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license