fix(dynamodb): throw error when grantee is an unsupported ServicePrincipal#37335
fix(dynamodb): throw error when grantee is an unsupported ServicePrincipal#37335mergify[bot] merged 7 commits intomainfrom
Conversation
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
|
|
||||||||||||||||||
|
|
||||||||||||||||||
| Annotations.of(this).addWarningV2( | ||
| '@aws-cdk/aws-dynamodb:servicePrincipalGrantDropped', | ||
| 'DynamoDB grant* methods do not support ServicePrincipal grantees. ' + | ||
| 'Use table.addToResourcePolicy() for an explicit service-specific table policy ' + | ||
| 'with required service principal, actions, and conditions', | ||
| ); | ||
| return Grant.drop(grantee, 'DynamoDB grant* does not support ServicePrincipal grantees'); |
There was a problem hiding this comment.
Kinda sounds like that SPs are never allowed here, but elsewhere you have stated at a small limited list of SPs is actually allowed. I don't think we should drop all SPs, but have a allow list.
There was a problem hiding this comment.
Yes, there are few SPs documented for DynamoDB resource policies, but they are integration-specific and require their own actions/conditions (e.g. replication.dynamodb.amazonaws.com). The generic grant* methods add standard data actions (dynamodb:GetItem, dynamodb:PutItem, etc.), which are not the right action set for those service-principal resource policies.
So even if we had an allowlist of permitted SPs, the actions being granted by grant* methods could produce incorrect resource policy which might not fail deployment, but it could still be functionally incorrect.
Because of that, I think the right path for SPs is the explicit addToResourcePolicy API, where the caller can provide the correct integration-specific principal, actions, and conditions.
Verify that DynamoDB tables can grant access to the three allowlisted service principals (redshift, replication.dynamodb, glue) and that the resulting CloudFormation template contains the expected resource policy. 🤖 Assisted by the code-assist SOP
|
Snapshot deployment passed: https://github.com/aws/aws-cdk/actions/runs/23655284133/job/68910779532?pr=37335 Removing label. |
|
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). |
Merge Queue Status
This pull request spent 52 minutes 18 seconds in the queue, including 52 minutes 10 seconds running CI. Required conditions to merge
|
|
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)
Closes #37273.
Reason for this change
After v2.222.0, calling
table.grantReadWriteData(new ServicePrincipal(...))started generating DynamoDB resource-based policies containing service principals. This is a regression caused by the combination of #35554 (which fixedaddToResourcePolicyto actually take effect) and #35817 (which enabled the grant framework to automatically discover DynamoDB resource policies). Previously,addToResourcePolicywas a no-op, so the grant silently did nothing.For unsupported service principals (e.g.
myservice.amazonaws.com), this produces an invalid resource policy that fails at CloudFormation deploy time with: "Invalid policy document: Policy contains invalid service principal".However, DynamoDB does support specific service principals in resource policies:
redshift.amazonaws.com— Redshift zero-ETL integrationreplication.dynamodb.amazonaws.com— Multi-account global tablesglue.amazonaws.com— SageMaker Lakehouse zero-ETL via AWS GlueDescription of changes
Throw
ValidationErrorfor unsupported service principals — Grant methods (grant,grantReadData,grantWriteData,grantReadWriteData,grantFullAccess) on bothTableandTableV2now detectServicePrincipalgrantees and throw aValidationErrorat synth time, directing users totable.addToResourcePolicy()instead. This fails fast rather than producing an invalid template that fails at deploy time.Allowlist known-valid service principals — Added
KNOWN_DYNAMODB_SERVICE_PRINCIPALSallowlist inprivate/principal-utils.tscontaining the three documented service principals. The newisUnsupportedServicePrincipal()function walks the principal wrapper chain (handlingPrincipalWithConditions,SessionTagsPrincipal, etc.) to extract the service name and check it against the allowlist. Allowlisted principals pass throughgrant*methods normally.Integration test — Added
integ.dynamodb.grant-service-principalverifying the three allowlisted principals produce correct resource policies in the synthesized CloudFormation template.Describe any new or updated permissions being added
No new or updated IAM permissions. This change prevents invalid IAM resource policy statements from being generated for unsupported service principals, while allowing valid ones through.
Description of how you validated changes
dynamodb.test.ts(186 passing),table-v2.test.ts(135 passing)bedrock.amazonaws.com,lambda.amazonaws.com) throwValidationErrorredshift,replication.dynamodb,glue) succeed.withConditions()) handled correctly for both casesinteg.dynamodb.grant-service-principaldeployed successfully with snapshot containing resource policy statements for all three allowlisted principalsChecklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license