aws-cdk icon indicating copy to clipboard operation
aws-cdk copied to clipboard

fix(core): fix policy synthesizer logic for precreated roles

Open GavinZZ opened this issue 1 year ago • 1 comments

Issue # (if applicable)

Closes https://github.com/aws/aws-cdk/issues/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 customizeRoles is used, the iam-policy-report.txt report will contain a list of 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:

"Resource": {
      "Fn::GetAtt": [
           "SomeResource",
           "Arn"
       ]
}

The policy report will instead get:

"(Path/To/SomeResource.Arn)"

Current issues

There are two main issues here:

  1. Policy synthesizer (which is used for customizeRoles to generate report) is created with App scope. This caused the failure in the original issue Resolution 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.
  2. The policy synthesizer was using 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).Arn instead of "(Path/To/SomeResource.Arn)".
  3. Pseudo parameters like AWS::NoValue would be rendered as Tokens in the policy report which is not idea. Update it to make it output NOVALUE.

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

GavinZZ avatar Oct 09 '24 23:10 GavinZZ

Discussed with Kendra offline and she's fine to merge this. Removing do-not-merge.

GavinZZ avatar Oct 22 '24 17:10 GavinZZ

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).

mergify[bot] avatar Oct 22 '24 17:10 mergify[bot]

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
  • Commit ID: c77e869cb6814727e0bea0bbba8ca5ae0db530eb
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation avatar Oct 22 '24 18:10 aws-cdk-automation

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).

mergify[bot] avatar Oct 22 '24 18:10 mergify[bot]

Comments on closed issues and PRs are hard for our team to see. If you need help, please open a new issue that references this one.

github-actions[bot] avatar Oct 22 '24 18:10 github-actions[bot]