From 872db4f9d81d631cc24164e4e81a0db37c6f3c8a Mon Sep 17 00:00:00 2001
From: Rico Huijbers <rix0rrr@gmail.com>
Date: Wed, 6 Nov 2024 20:38:59 +0100
Subject: [PATCH] fix: deploy-time stack tags cause synthesis to fail

In https://github.com/aws/aws-cdk/pull/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.
---
 .../core/lib/stack-synthesizers/_shared.ts    | 21 ++++++++++++-------
 packages/aws-cdk-lib/core/test/stack.test.ts  | 11 ++++++++--
 2 files changed, 23 insertions(+), 9 deletions(-)

diff --git a/packages/aws-cdk-lib/core/lib/stack-synthesizers/_shared.ts b/packages/aws-cdk-lib/core/lib/stack-synthesizers/_shared.ts
index 46cb62b27c218..686968298475c 100644
--- a/packages/aws-cdk-lib/core/lib/stack-synthesizers/_shared.ts
+++ b/packages/aws-cdk-lib/core/lib/stack-synthesizers/_shared.ts
@@ -26,15 +26,22 @@ export function addStackArtifactToAssembly(
   // nested stack tags are applied at the AWS::CloudFormation::Stack resource
   // level and are not needed in the cloud assembly.
   if (Object.entries(stackTags).length > 0) {
-    for (const [k, v] of Object.entries(stackTags)) {
-      if (Token.isUnresolved(k) || Token.isUnresolved(v)) {
-        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.`);
-      }
+    const resolvedTags = Object.entries(stackTags).filter(([k, v]) => !(Token.isUnresolved(k) || Token.isUnresolved(v)));
+    const unresolvedTags = Object.entries(stackTags).filter(([k, v]) => Token.isUnresolved(k) || Token.isUnresolved(v));
+
+    if (unresolvedTags.length > 0) {
+      const rendered = unresolvedTags.map(([k, v]) => `${Token.isUnresolved(k) ? '<TOKEN>': k}=${Token.isUnresolved(v) ? '<TOKEN>' : v}`).join(', ');
+      stack.node.addMetadata(
+        cxschema.ArtifactMetadataEntryType.WARN,
+        `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'] }).`,
+      );
     }
 
-    stack.node.addMetadata(
-      cxschema.ArtifactMetadataEntryType.STACK_TAGS,
-      Object.entries(stackTags).map(([key, value]) => ({ Key: key, Value: value })));
+    if (resolvedTags.length > 0) {
+      stack.node.addMetadata(
+        cxschema.ArtifactMetadataEntryType.STACK_TAGS,
+        resolvedTags.map(([key, value]) => ({ Key: key, Value: value })));
+    }
   }
 
   const deps = [
diff --git a/packages/aws-cdk-lib/core/test/stack.test.ts b/packages/aws-cdk-lib/core/test/stack.test.ts
index 0f67d1ad6ac7b..399e355063dfb 100644
--- a/packages/aws-cdk-lib/core/test/stack.test.ts
+++ b/packages/aws-cdk-lib/core/test/stack.test.ts
@@ -2075,7 +2075,7 @@ describe('stack', () => {
     expect(asm.getStackArtifact(stack2.artifactId).tags).toEqual(expected);
   });
 
-  test('stack tags may not contain tokens', () => {
+  test('warning when stack tags contain tokens', () => {
     // GIVEN
     const app = new App({
       stackTraces: false,
@@ -2087,7 +2087,14 @@ describe('stack', () => {
       },
     });
 
-    expect(() => app.synth()).toThrow(/Stack tags may not contain deploy-time values/);
+    const asm = app.synth();
+    const stackArtifact = asm.stacks[0];
+    expect(stackArtifact.manifest.metadata?.['/stack1']).toEqual([
+      {
+        type: 'aws:cdk:warning',
+        data: expect.stringContaining('Ignoring stack tags that contain deploy-time values'),
+      },
+    ]);
   });
 
   test('stack notification arns are reflected in the stack artifact properties', () => {