fix(dynamodb): stream resource policy for Table(v1) #31515
Conversation
github-actions
bot
added
effort/medium
There was a problem hiding this comment.
The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.
A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.
LeeroyHannigan
changed the title
|
Clarification Request: README for TableV1 Updated packages/aws-cdk-lib/aws-dynamodb/TABLE_V1_API.md |
aws-cdk-automation
added
the
pr/reviewer-clarification-requested
LeeroyHannigan
changed the title
aws-cdk-automation
dismissed
their stale review
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
aws-cdk-automation
added
the
pr/needs-community-review
| If you have a global table replica, note that it does not support the addition of a resource-based policy. |
There was a problem hiding this comment.
Any idea if this statement apply to the steam resource-based policy as well? If yes, we should update this.
There was a problem hiding this comment.
Stream RBP will be fine. TableV1 uses a custom resource to handle replicas for global tables. It can include a RBP if we update the Lambda code. But we're trying to convince users to adopt TableV2
|
|
||
| /** | ||
| * Resource policy to assign to a DynamoDB stream. | ||
| * @see https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-dynamodb-globaltable-replicaspecification.html#cfn-dynamodb-globaltable-replicaspecification-replicastreamspecification |
There was a problem hiding this comment.
| * @see https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-dynamodb-globaltable-replicaspecification.html#cfn-dynamodb-globaltable-replicaspecification-replicastreamspecification | |
| * @see https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-dynamodb-table-streamspecification.html#cfn-dynamodb-table-streamspecification-resourcepolicy |
since this Table(v1) uses the AWS::DynamoDB::Table resource type.
There was a problem hiding this comment.
Thank you. Copy and paste let me down 😂
| * Resource policy to assign to table. | ||
| * @attribute | ||
| */ | ||
| public abstract streamResourcePolicy?: iam.PolicyDocument; |
There was a problem hiding this comment.
Related to this field, we should probably update the grantStream and the grantStreamRead methods. [Edit:] By update I mean, those methods should add permissions to this streamResourcePolicy I believe.
And make this field protected to limit the exposure as there is no other constructs need to consume this at them the moment.
Let me know what you think.
|
Added some comments and questions. Thank you for creating the PR! |
aws-cdk-automation
removed
the
pr/needs-community-review
|
This PR has been in the MERGE CONFLICTS state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week. |
|
This PR has been deemed to be abandoned, and will be automatically closed. Please create a new PR for these changes if you think this decision has been made in error. |
aws-cdk-automation
added
the
closed-for-staleness
|
Comments on closed issues and PRs are hard for our team to see. |
Issue # (if applicable)
Closes #31229.
Reason for this change
Users were unable to add a resource policy to a DynamoDB stream
Description of changes
Implemented resource policies for Table (V1) streams
Description of how you validated changes
Integ testing added
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license