fix(core): fix policy synthesizer logic for precreated roles#31710
fix(core): fix policy synthesizer logic for precreated roles#31710mergify[bot] merged 8 commits intomainfrom
Conversation
60ae7c1 to
1ce33ef
Compare
Leo10Gama
left a comment
There was a problem hiding this comment.
Overall changes look good! Just a few minor comments from me.
packages/@aws-cdk-testing/framework-integ/test/aws-iam/test/integ.customize-role.ts
Show resolved
Hide resolved
...eg/test/aws-dynamodb/test/integ.table-with-customized-role.js.snapshot/iam-policy-report.txt
Outdated
Show resolved
Hide resolved
Leo10Gama
left a comment
There was a problem hiding this comment.
Changes LGTM! Will leave the do-not-merge label on for a second review from core to confirm changes are fine
|
Discussed with Kendra offline and she's fine to merge this. Removing |
|
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). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
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 #31653
Reason for this change
With Role.customizeRoles enabled, dynamodb.Table.addGlobalSecondaryIndex causes an error. This is a critical blocker for customers who require the use of customizeRoles.
Description of changes
Intended behaviour
When
customizeRolesis used, theiam-policy-report.txtreport will contain a listof IAM roles and associated permissions that would have been created. This report is
generated so that it attempts to resolve any references and replace with a more user
friendly value.
The following are some examples of the value that will appear in the report:
The policy report will instead get:
"(Path/To/SomeResource.Arn)"Current issues
There are two main issues here:
Appscope. This caused the failure in the original issueResolution error: PolicySynthesizer at 'PolicySynthesizer' should be created in the scope of a Stack, but no Stack found.because token resolution requires a Stack scope not an App scope.DefaultTokenResolver. The default token resolution class does not generate the same format of output values for the policy report. i.e. A concatenated token value, i.e.${Token[Token.X]}/index/*would be converted to(PhysicalId).Arninstead of"(Path/To/SomeResource.Arn)".AWS::NoValuewould be rendered asTokensin the policy report which is not idea. Update it to make it outputNOVALUE.This PR addresses the above two issues.
Description of how you validated changes
New and 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