Skip to content

Commit 55a3c4c

Browse files
authoredMar 12, 2025··
fix(core): message including tokens from annotations cannot output correctly (#33706)
### Issue # (if applicable) Closes #33707 ### Reason for this change If a stack with name 'some-stack' includes an info annotation ```ts Annotations.of(this).addInfo(`stackId: ${this.stackId}`); ``` then the following output results: ``` [Info at /some-stack] [object Object] ``` That's because data comes from Annotations and the data can be of object type containing 'Fn::Join' or 'Ref' when tokens are included in Annotations. The issue mentioned a proposal to output the data in the form of tokens like `[Info at /CdkSampleStack] ${Token[AWS::StackId.1116]}`. ### Description of changes **Approach 1** for now. (I am still wondering if approach 3 would be better...) See below: ### Approach 1 The PR makes messages with tokens by annotations unresolved. #### NOTE This change would also output a token format in `manifest.json`. **If users run integ tests with annotations including tokens, the manifest.json would change for every run.** (like `${Token[AWS::StackId.1119]}` -> `${Token[AWS::StackId.123]}` -> `${Token[AWS::StackId.521]}` -> ...) ```json { // ... "CdkSampleStack": { // ... "metadata": { "/CdkSampleStack": [ { "type": "aws:cdk:info", "data": "stackId: ${Token[AWS::StackId.1119]}", ``` ### Approach 2 Change the type for the `msg.entry.data` (`MetadataEntryData` for `MetadataEntry`) to a string type with `JSON.stringify` if the type is an objective type in cdk-cli. https://github.com/aws/aws-cdk-cli/blob/cdk%40v2.1003.0/packages/%40aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts#L771 Then I had submitted the [PR](aws/aws-cdk-cli#101) in aws-cdk-cli. But talked with Rico that the change should be made inside cdk-lib and leave the token unrendered. aws/aws-cdk-cli#101 (comment) ### Approach 3 Change the data type to a string type after resolve if the data is by annotations with tokens. This approach doesn't make differences in manifest.json for every run and the original format (with 'Ref' or 'Fn::Join') is kept. However, the issue for this PR and comments in the PR submitted (aws-cdk-cli) has proposed the approach with unresolved tokens, I decided the approach 1 for now. 63fd78b ```ts if (node.node.metadata.length > 0) { // Make the path absolute output[Node.PATH_SEP + node.node.path] = node.node.metadata.map(md => { const resolved = stack.resolve(md) as cxschema.MetadataEntry; const isAnnotation = [ cxschema.ArtifactMetadataEntryType.ERROR, cxschema.ArtifactMetadataEntryType.WARN, cxschema.ArtifactMetadataEntryType.INFO, ].includes(md.type as cxschema.ArtifactMetadataEntryType); // Transform the data to a string for the case where Annotations include a token. // Otherwise, the message is resolved and output as `[object Object]` after synth // because the message will be object type using 'Ref' or 'Fn::Join'. const mdWithStringData: cxschema.MetadataEntry = { ...resolved, data: (isAnnotation && typeof resolved.data === 'object') ? JSON.stringify(resolved.data) : resolved.data, }; return mdWithStringData; }); } ``` This approach outputs the message as the following style: ``` {"Fn::Join":["",["Cannot add a resource policy to your dead letter queue associated with rule ",{"Ref":"Rule4C995B7F"}," because the queue is in a different account. You must add the resource policy manually to the dead letter queue in account 444455556666. [ack: @aws-cdk/aws-events-targets:manuallyAddDLQResourcePolicy]"]]} ``` ### Additional Information see: #33707 (comment) aws/aws-cdk-cli#101 (comment) ### Describe any new or updated permissions being added ### Description of how you validated changes Unit tests. ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 3f1fecf commit 55a3c4c

File tree

6 files changed

+49
-38
lines changed

6 files changed

+49
-38
lines changed
 

‎packages/aws-cdk-lib/aws-events-targets/test/lambda/lambda.test.ts

+3-7
Original file line numberDiff line numberDiff line change
@@ -320,13 +320,9 @@ test('must display a warning when using a Dead Letter Queue from another account
320320

321321
Template.fromStack(stack1).resourceCountIs('AWS::SQS::QueuePolicy', 0);
322322

323-
Annotations.fromStack(stack1).hasWarning('/Stack1/Rule', Match.objectLike({
324-
'Fn::Join': Match.arrayWith([
325-
Match.arrayWith([
326-
'Cannot add a resource policy to your dead letter queue associated with rule ',
327-
]),
328-
]),
329-
}));
323+
Annotations.fromStack(stack1).hasWarning('/Stack1/Rule', Match.stringLikeRegexp(
324+
'Cannot add a resource policy to your dead letter queue associated with rule \\${Token\\[TOKEN\\.[0-9]+\\]} because the queue is in a different account\\. You must add the resource policy manually to the dead letter queue in account 444455556666\\. \\[ack: @aws-cdk/aws-events-targets:manuallyAddDLQResourcePolicy\\]',
325+
));
330326
});
331327

332328
test('specifying retry policy', () => {

‎packages/aws-cdk-lib/aws-lambda/test/function.test.ts

+1-16
Original file line numberDiff line numberDiff line change
@@ -254,22 +254,7 @@ describe('function', () => {
254254

255255
expect(getWarnings(app.synth())).toEqual([
256256
{
257-
message: {
258-
'Fn::Join': [
259-
'',
260-
[
261-
'addPermission() has no effect on a Lambda Function with region=us-west-2, account=123456789012, in a Stack with region=',
262-
{
263-
Ref: 'AWS::Region',
264-
},
265-
', account=',
266-
{
267-
Ref: 'AWS::AccountId',
268-
},
269-
'. Suppress this warning if this is is intentional, or pass sameEnvironment=true to fromFunctionAttributes() if you would like to add the permissions. [ack: UnclearLambdaEnvironment]',
270-
],
271-
],
272-
},
257+
message: expect.stringMatching(/^addPermission\(\) has no effect on a Lambda Function with region=us-west-2, account=123456789012, in a Stack with region=\${Token\[AWS\.Region\.\d+]}, account=\${Token\[AWS\.AccountId\.\d+]}. Suppress this warning if this is is intentional, or pass sameEnvironment=true to fromFunctionAttributes\(\) if you would like to add the permissions\. \[ack: UnclearLambdaEnvironment]$/),
273258
path: '/Default/Imported',
274259
},
275260
]);

‎packages/aws-cdk-lib/aws-s3-notifications/test/queue.test.ts

+3-11
Original file line numberDiff line numberDiff line change
@@ -109,15 +109,7 @@ test('if the queue is encrypted with a imported kms key, printout warning', () =
109109

110110
bucket.addObjectCreatedNotification(new notif.SqsDestination(queue));
111111

112-
Annotations.fromStack(stack).hasWarning('/Default/ImportedKey', `Can not change key policy of imported kms key. Ensure that your key policy contains the following permissions: \n${JSON.stringify({
113-
Action: [
114-
'kms:GenerateDataKey*',
115-
'kms:Decrypt',
116-
],
117-
Effect: 'Allow',
118-
Principal: {
119-
Service: 's3.amazonaws.com',
120-
},
121-
Resource: '*',
122-
}, null, 2)} [ack: @aws-cdk/aws-s3-notifications:sqsKMSPermissionsNotAdded]`);
112+
Annotations.fromStack(stack).hasWarning('/Default/ImportedKey', Match.stringLikeRegexp(
113+
'Can not change key policy of imported kms key\\. Ensure that your key policy contains the following permissions: \\n\\{\\n "Action": \\[\\n "kms:GenerateDataKey\\*",\\n "kms:Decrypt"\\n \\],\\n "Effect": "Allow",\\n "Principal": \\{\\n "Service": "\\${Token\\[s3\\.amazonaws\\.com\\.[0-9]+\\]}"\\n \\},\\n "Resource": "\\*"\\n\\} \\[ack: @aws-cdk/aws-s3-notifications:sqsKMSPermissionsNotAdded\\]',
114+
));
123115
});

‎packages/aws-cdk-lib/aws-s3/test/notification.test.ts

+7-3
Original file line numberDiff line numberDiff line change
@@ -162,9 +162,13 @@ describe('notification', () => {
162162
});
163163

164164
// THEN - Following is warning thrown as a part of fix in : https://github.com/aws/aws-cdk/pull/31212
165-
const warningMessage = { 'Fn::Join': ['', ["Can't combine imported IManagedPolicy: arn:", { Ref: 'AWS::Partition' }, ':iam::aws:policy/service-role/AWSLambdaBasicExecutionRole to imported role IRole: DevsNotAllowedToTouch. Use ManagedPolicy directly. [ack: @aws-cdk/aws-iam:IRoleCantBeUsedWithIManagedPolicy]']] };
166-
const warningFromStack = Annotations.fromStack(stack).findWarning('*', {});
167-
expect(warningFromStack[0].entry.data).toEqual(warningMessage);
165+
const warningMessage = /Can't combine imported IManagedPolicy: arn:\${Token\[AWS\.Partition\.\d+\]}:iam::aws:policy\/service-role\/AWSLambdaBasicExecutionRole to imported role IRole: DevsNotAllowedToTouch\. Use ManagedPolicy directly\. \[ack: @aws-cdk\/aws-iam:IRoleCantBeUsedWithIManagedPolicy\]/;
166+
const warningFromStack = Annotations.fromStack(stack).findWarning('*',
167+
Match.stringLikeRegexp(
168+
'@aws-cdk/aws-iam:IRoleCantBeUsedWithIManagedPolicy',
169+
),
170+
);
171+
expect(warningFromStack[0].entry.data).toEqual(expect.stringMatching(warningMessage));
168172
});
169173

170174
test('If `Role` is provided, PutBucketNotification, GetBucketNotification will be added along with `service-role/AWSLambdaBasicExecutionRole`', () => {

‎packages/aws-cdk-lib/core/lib/stack-synthesizers/_shared.ts

+17-1
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,23 @@ function collectStackMetadata(stack: Stack) {
100100

101101
if (node.node.metadata.length > 0) {
102102
// Make the path absolute
103-
output[Node.PATH_SEP + node.node.path] = node.node.metadata.map(md => stack.resolve(md) as cxschema.MetadataEntry);
103+
output[Node.PATH_SEP + node.node.path] = node.node.metadata.map(md => {
104+
// If Annotations include a token, the message is resolved and output as `[object Object]` after synth
105+
// because the message will be object type using 'Ref' or 'Fn::Join'.
106+
// It would be easier for users to understand if the message from Annotations were output in token form,
107+
// rather than in `[object Object]` or the object type.
108+
// Therefore, we don't resolve the message if it's from Annotations.
109+
if ([
110+
cxschema.ArtifactMetadataEntryType.ERROR,
111+
cxschema.ArtifactMetadataEntryType.WARN,
112+
cxschema.ArtifactMetadataEntryType.INFO,
113+
].includes(md.type as cxschema.ArtifactMetadataEntryType)) {
114+
return md;
115+
}
116+
117+
const resolved = stack.resolve(md);
118+
return resolved as cxschema.MetadataEntry;
119+
});
104120
}
105121

106122
for (const child of node.node.children) {

‎packages/aws-cdk-lib/core/test/annotations.test.ts

+18
Original file line numberDiff line numberDiff line change
@@ -129,4 +129,22 @@ describe('annotations', () => {
129129
// THEN
130130
expect(getWarnings(app.synth())).toEqual([]);
131131
});
132+
133+
test('don\'t resolve the message if tokens are included', () => {
134+
// GIVEN
135+
const app = new App();
136+
const stack = new Stack(app, 'S1');
137+
const c1 = new Construct(stack, 'C1');
138+
139+
// WHEN
140+
Annotations.of(c1).addWarningV2('MESSAGE', `stackId: ${stack.stackId}`);
141+
142+
// THEN
143+
expect(getWarnings(app.synth())).toEqual([
144+
{
145+
path: '/S1/C1',
146+
message: expect.stringMatching(/stackId: \${Token\[AWS::StackId\.\d+\]} \[ack: MESSAGE\]/),
147+
},
148+
]);
149+
});
132150
});

0 commit comments

Comments
 (0)
Please sign in to comment.