-
Notifications
You must be signed in to change notification settings - Fork 4.3k
feat(s3): attribute-based access control #36229
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
base: main
Are you sure you want to change the base?
Conversation
… buckets - Introduced `abacStatus` property to the S3 bucket construct to enable or disable ABAC. - Updated integration test to include `abacStatus` for both KMS-encrypted and S3-managed buckets. - Modified the snapshot to reflect changes in the bucket configuration. - Enhanced documentation to explain how to enable and disable ABAC for S3 buckets.
| }); | ||
| }); | ||
|
|
||
| test.each([true, false])('bucket with ABAC status %s', (abacStatus) => { |
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.
Since the handling changes based on whether the value is undefined, it's a good idea to also check for undefined with Match.absent.
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.
I've added it!
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.
Could you combine them into one? Because they are tests for a single concern, it will be easier to grasp the test cases than separating them into individual tests.
test.each([
[true, 'Enabled'],
[false, 'Disabled'],
[undefined, Match.absent()],
])('bucket with ABAC status %s', (abacStatus, expected) => {
const stack = new cdk.Stack();
new s3.Bucket(stack, 'MyBucket', {
abacStatus,
});
Template.fromStack(stack).hasResourceProperties('AWS::S3::Bucket', {
AbacStatus: expected,
});
});|
@go-to-k Thank you for your review! I've addressed your comment. |
Co-authored-by: Kenta Goto <24818752+go-to-k@users.noreply.github.com>
|
@go-to-k I've updated! |
|
Did you forget to push your commit? :) |
|
@go-to-k I'm sorry for forgetting to push it. |
go-to-k
left a comment
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 changes.
Issue # (if applicable)
None
Reason for this change
AWS S3 general purpose bucket now supports for Attribute-based access control (ABAC).
https://docs.aws.amazon.com/ja_jp/AmazonS3/latest/userguide/buckets-tagging.html
Description of changes
abacStatustoBucketPropsDescribe any new or updated permissions being added
None
Description of how you validated changes
add both unit and integ tests
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license