-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
chore(cloudfront-origins): grant key permissions to distribution #31188
Conversation
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.
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.
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
scope, | ||
resolver: new DefaultTokenResolver(new StringConcat()), | ||
})); | ||
} |
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.
Do you have links to examples of this trick when the code is doing JSON.stringify(Tokenization.resolve ...
? Trying to understand what it does. Does it resolve to the logical ID?
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.
example of usage in iam module:
aws-cdk/packages/aws-cdk-lib/aws-iam/lib/private/util.ts
Lines 24 to 31 in af50620
export function generatePolicyName(scope: IConstruct, logicalId: string): string { | |
// as logicalId is itself a Token, resolve it first | |
const resolvedLogicalId = Tokenization.resolve(logicalId, { | |
scope, | |
resolver: new DefaultTokenResolver(new StringConcat()), | |
}); | |
return lastNCharacters(resolvedLogicalId, MAX_POLICY_NAME_LEN); | |
} |
Output from when I tested it locally is like this:
[Info at /MigrationOacStack/MyDistribution/Origin2] Granting OAC permissions to access KMS key for S3 bucket origin {"Ref":"MyBucketF68F3FF0"} may cause a circular dependency error when this stack deploys.
The key policy references the distribution's id, the distribution references the bucket, and the bucket references the key.
See the 'Using OAC for a SSE-KMS encrypted S3 origin' section in the module README for more details.
So yes it resolves to the logical ID. I thought it would be helpful to include the specific resource involved in the info message, but there might be a cleaner way to do this
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.
I see. Thank you. Looks good to me.
Comments on closed issues and PRs are hard for our team to see. |
Changes:
distributionId
optional