-
Notifications
You must be signed in to change notification settings - Fork 4.3k
fix(dynamodb): resolve circular dependency with AccountRootPrincipal grants #35983
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(dynamodb): resolve circular dependency with AccountRootPrincipal grants #35983
Conversation
…tables - Enhance DynamoDB Table construct to support `addToResourcePolicy` method - Update integration tests to demonstrate resource policy configuration - Modify table implementation to allow flexible resource policy management - Improve documentation and test coverage for resource policy feature - Ensure compatibility with existing DynamoDB table configurations
Abogical
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! I've asked for another reviewer to this.
|
Thank you for contributing! Your pull request will be automatically updated and merged (do not update manually, and be sure to allow changes to be pushed to your fork). |
|
This pull request has been removed from the queue for the following reason: The pull request can't be updated
You should update or rebase your pull request manually. If you do, this pull request will automatically be requeued once the queue conditions match again. |
|
Thank you for contributing! Your pull request will be automatically updated and merged (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 automatically updated and merged (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)
Closes #35967.
Reason for this change
In CDK 2.222.0, PR #35554 fixed
addToResourcePolicy()to actually work (it was previously a no-op). This exposed a circular dependency issue when usinggrantReadData()or other grant methods withAccountRootPrincipal.When
AccountRootPrincipalis used with grant methods, the IAM Grant system adds the policy to the table's resource policy (since it's in the same account). The resource policy statement included the table's ARN (!GetAtt Table.Arn), creating a circular dependency: Table → ResourcePolicy → Table.Arn → Table.This is a regression that breaks existing user code that worked in 2.221.1.
Description of changes
Applied the established KMS grant pattern to DynamoDB by adding
resourceSelfArns: ['*']parameter toGrant.addToPrincipalOrResource()calls in thecombinedGrantmethod.How it works:
resourceArnscontains actual table ARNs → used for principal policies (IAM user/role policies)resourceSelfArns: ['*']→ used for resource policies (table's resource policy)!GetAtt Table.ArnWhy wildcard is safe:
Files modified:
packages/aws-cdk-lib/aws-dynamodb/lib/table.ts- AddedresourceSelfArns: ['*']tocombinedGrantmethodpackages/aws-cdk-lib/aws-dynamodb/lib/table-v2-base.ts- Applied identical change for Table V2packages/aws-cdk-lib/aws-dynamodb/README.md- Added documentation about grant methods and resource policy interactionBefore (causes circular dependency):
After (no circular dependency):
CloudFormation template change:
{ "Resources": { "Table": { "Type": "AWS::DynamoDB::Table", "Properties": { "ResourcePolicy": { "PolicyDocument": { "Statement": [{ "Action": ["dynamodb:BatchGetItem", "dynamodb:GetItem", "dynamodb:Query", "dynamodb:Scan"], "Effect": "Allow", "Principal": { "AWS": "arn:aws:iam::ACCOUNT:root" }, "Resource": "*" }] } } } } } }Describe any new or updated permissions being added
N/A - This fix does not add new permissions. It resolves how existing grant methods generate resource policies to avoid circular dependencies.
Description of how you validated changes
Unit tests: Added 2 new tests validating
AccountRootPrincipalwith grant methodspackages/aws-cdk-lib/aws-dynamodb/test/dynamodb.test.ts: Test for Table V1packages/aws-cdk-lib/aws-dynamodb/test/table-v2.test.ts: Test for Table V2*) to avoid circular dependencyIntegration tests: Enhanced existing integration test with grant scenario
packages/@aws-cdk-testing/framework-integ/test/aws-dynamodb/test/integ.dynamodb.add-to-resource-policy.tsgrantWriteData(new AccountRootPrincipal())works without circular dependencyRegression testing: All 346 existing tests pass
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license