Skip to content

Commit 18c19fd

Browse files
authored
fix: deploy-time stack tags cause synthesis to fail (#32041)
In #31457, we introduced a change that made synthesis fail if one of the stack tags was a deploy-time value. Since stack tags are assigned outside a CloudFormation context, deploy-time values cannot be evaluated, so the stack ends up with a tag like `{ Key: "my-tag", Value: "${Token[1234]}" }`, which is probably not what is intended. Worse, those tags are automatically propagated to all resources in the stack by CloudFormation, and some may validate the tag value and find that `$` or any of the other characters are not valid tag values. The intent was that customers would be alerted to these kinds of mistakes and apply their tags to resources, or skip stacks when applying tags to large scopes: ```ts Tags.of(this).add('my-tag', Fn.importValue('SomeExport'), { excludeResourceTypes: ['aws:cdk:stack'], }); ``` The previous change was a bit drastic in its attempts. In this one we ignore the unresolved tags and add a warning instead. That way, synthesis still succeeds. Closes #32040. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent dd61d12 commit 18c19fd

File tree

2 files changed

+23
-9
lines changed

2 files changed

+23
-9
lines changed

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

+14-7
Original file line numberDiff line numberDiff line change
@@ -26,15 +26,22 @@ export function addStackArtifactToAssembly(
2626
// nested stack tags are applied at the AWS::CloudFormation::Stack resource
2727
// level and are not needed in the cloud assembly.
2828
if (Object.entries(stackTags).length > 0) {
29-
for (const [k, v] of Object.entries(stackTags)) {
30-
if (Token.isUnresolved(k) || Token.isUnresolved(v)) {
31-
throw new Error(`Stack tags may not contain deploy-time values (tag: ${k}=${v}). Apply tags containing deploy-time values to resources only, avoid tagging stacks.`);
32-
}
29+
const resolvedTags = Object.entries(stackTags).filter(([k, v]) => !(Token.isUnresolved(k) || Token.isUnresolved(v)));
30+
const unresolvedTags = Object.entries(stackTags).filter(([k, v]) => Token.isUnresolved(k) || Token.isUnresolved(v));
31+
32+
if (unresolvedTags.length > 0) {
33+
const rendered = unresolvedTags.map(([k, v]) => `${Token.isUnresolved(k) ? '<TOKEN>': k}=${Token.isUnresolved(v) ? '<TOKEN>' : v}`).join(', ');
34+
stack.node.addMetadata(
35+
cxschema.ArtifactMetadataEntryType.WARN,
36+
`Ignoring stack tags that contain deploy-time values (found: ${rendered}). Apply tags containing deploy-time values to resources only, avoid tagging stacks (for example using { excludeResourceTypes: ['aws:cdk:stack'] }).`,
37+
);
3338
}
3439

35-
stack.node.addMetadata(
36-
cxschema.ArtifactMetadataEntryType.STACK_TAGS,
37-
Object.entries(stackTags).map(([key, value]) => ({ Key: key, Value: value })));
40+
if (resolvedTags.length > 0) {
41+
stack.node.addMetadata(
42+
cxschema.ArtifactMetadataEntryType.STACK_TAGS,
43+
resolvedTags.map(([key, value]) => ({ Key: key, Value: value })));
44+
}
3845
}
3946

4047
const deps = [

packages/aws-cdk-lib/core/test/stack.test.ts

+9-2
Original file line numberDiff line numberDiff line change
@@ -2075,7 +2075,7 @@ describe('stack', () => {
20752075
expect(asm.getStackArtifact(stack2.artifactId).tags).toEqual(expected);
20762076
});
20772077

2078-
test('stack tags may not contain tokens', () => {
2078+
test('warning when stack tags contain tokens', () => {
20792079
// GIVEN
20802080
const app = new App({
20812081
stackTraces: false,
@@ -2087,7 +2087,14 @@ describe('stack', () => {
20872087
},
20882088
});
20892089

2090-
expect(() => app.synth()).toThrow(/Stack tags may not contain deploy-time values/);
2090+
const asm = app.synth();
2091+
const stackArtifact = asm.stacks[0];
2092+
expect(stackArtifact.manifest.metadata?.['/stack1']).toEqual([
2093+
{
2094+
type: 'aws:cdk:warning',
2095+
data: expect.stringContaining('Ignoring stack tags that contain deploy-time values'),
2096+
},
2097+
]);
20912098
});
20922099

20932100
test('stack notification arns are reflected in the stack artifact properties', () => {

0 commit comments

Comments
 (0)