-
Notifications
You must be signed in to change notification settings - Fork 4.3k
feat(s3tables-alpha): add TablePolicy support to L2 construct library #35223
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
Conversation
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.
(This review is outdated)
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
}); | ||
}); | ||
|
||
it('creates IAM policies for a role', () => { |
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.
For all the tests which add read/write permissions to the table, can we have a test which check that the role/user policy is affected inline in the cfn templates ? The addition of read/write permissions should affect the role/user policies by adding the S3 tables based policies
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.
These tests already check for the identity policy on the principal resource, and resource policy is only added as a fallback option. This behavior is documented more here: https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_iam.Grant.html#static-addwbrtowbrprincipalwbrorwbrresourceoptions
packages/@aws-cdk/aws-s3tables-alpha/test/integration/integ.table-with-grants.ts
Outdated
Show resolved
Hide resolved
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
Comments on closed issues and PRs are hard for our team to see. |
Issue # (if applicable)
Related to #33054
Reason for this change
This PR includes backward-compatible changes being made to add L2 support for the CfnTable and CfnTablePolicy constructs with a consistent user interface, recommended defaults, and in-built validations for managing Table level IAM resource policies.
Description of changes
New L2 Construct: TablePolicy: defines an underlying CfnTablePolicy resource
New methods added to Table construct:
addToResourcePolicy
: Attaches a policy statement to the Table's IAM policygrantRead
: Grants read access to the table for the given principalgrantWrite
: Grants write access to the table for the given principalgrantReadWrite
: Grants read and write access to the table for the given principalDescribe any new or updated permissions being added
s3tables:UpdateTableMetadataLocation
s3tables:RenameTable
s3tables:PutTableData
s3tables:UpdateTableMetadataLocation
s3tables:CreateTable
Description of how you validated changes
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license