fix(cfn-include): allow CFN Functions in Tags (#19923)

Our `TagManger` does not allow expressing Tags that included top-level CloudFormation functions,
like `Fn::If`. While that's not a big issue when writing CDK code directly,
it makes it impossible to include some CloudFormation templates that use that pattern.

Introduce the concept of "dynamic tags" to the `TagManager` that allows preserving these,
and return them alongside the "regular" Tags when rendering.

Fixes #16889

----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/master/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

Author: skinny85

packages/@aws-cdk/cloudformation-include/test/test-templates/if-in-tags.json

@@ -0,0 +1,29 @@
+{
+  "Conditions": {
+    "ValcacheServerEnabled": true
+  },
+  "Resources": {
+    "TxAutoScalingGroup": {
+      "Type": "AWS::AutoScaling::AutoScalingGroup",
+      "Properties": {
+        "MinSize": "1",
+        "MaxSize": "3",
+        "Tags": [
+          {
+            "Fn::If": [
+              "ValcacheServerEnabled",
+              {
+                "Key": "datomic:cache-group",
+                "Value": "SystemName",
+                "PropagateAtLaunch": true
+              },
+              {
+                "Ref": "AWS::NoValue"
+              }
+            ]
+          }
+        ]
+      }
+    }
+  }
+}

packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts

@@ -228,6 +228,14 @@ describe('CDK Include', () => {
     );
   });
 
+  test('can ingest a template using Fn::If in Tags, and output it unchanged', () => {
+    includeTestTemplate(stack, 'if-in-tags.json');
+
+    Template.fromStack(stack).templateMatches(
+      loadTestFileToJsObject('if-in-tags.json'),
+    );
+  });
+
   test('can ingest a UserData script, and output it unchanged', () => {
     includeTestTemplate(stack, 'user-data.json');
 

packages/@aws-cdk/core/lib/tag-manager.ts

@@ -24,6 +24,24 @@ interface StackTag {
   Key: string;
   Value: string;
 }
+
+/**
+ * The results of parsing Tags.
+ */
+interface ParseTagsResult {
+  /**
+   * The "simple" (meaning, not including complex CloudFormation functions)
+   * tags that were found.
+   */
+  readonly tags: Tag[];
+
+  /**
+   * The collection of "dynamic" (meaning, including complex CloudFormation functions)
+   * tags that were found.
+   */
+  readonly dynamicTags: any;
+}
+
 /**
  * Interface for converter between CloudFormation and internal tag representations
  */
@@ -38,31 +56,33 @@ interface ITagFormatter {
    *
    * Use the given priority.
    */
-  parseTags(cfnPropertyTags: any, priority: number): Tag[];
+  parseTags(cfnPropertyTags: any, priority: number): ParseTagsResult;
 }
 
 /**
  * Standard tags are a list of { key, value } objects
  */
 class StandardFormatter implements ITagFormatter {
-  public parseTags(cfnPropertyTags: any, priority: number): Tag[] {
+  public parseTags(cfnPropertyTags: any, priority: number): ParseTagsResult {
     if (!Array.isArray(cfnPropertyTags)) {
       throw new Error(`Invalid tag input expected array of {key, value} have ${JSON.stringify(cfnPropertyTags)}`);
     }
 
     const tags: Tag[] = [];
+    const dynamicTags: any = [];
     for (const tag of cfnPropertyTags) {
       if (tag.key === undefined || tag.value === undefined) {
-        throw new Error(`Invalid tag input expected {key, value} have ${JSON.stringify(tag)}`);
+        dynamicTags.push(tag);
+      } else {
+        // using interp to ensure Token is now string
+        tags.push({
+          key: `${tag.key}`,
+          value: `${tag.value}`,
+          priority,
+        });
       }
-      // using interp to ensure Token is now string
-      tags.push({
-        key: `${tag.key}`,
-        value: `${tag.value}`,
-        priority,
-      });
     }
-    return tags;
+    return { tags, dynamicTags };
   }
 
   public formatTags(tags: Tag[]): any {
@@ -73,36 +93,38 @@ class StandardFormatter implements ITagFormatter {
         value: tag.value,
       });
     }
-    return cfnTags.length === 0 ? undefined : cfnTags;
+    return cfnTags;
   }
 }
 
 /**
  * ASG tags are a list of { key, value, propagateAtLaunch } objects
  */
 class AsgFormatter implements ITagFormatter {
-  public parseTags(cfnPropertyTags: any, priority: number): Tag[] {
-    const tags: Tag[] = [];
+  public parseTags(cfnPropertyTags: any, priority: number): ParseTagsResult {
     if (!Array.isArray(cfnPropertyTags)) {
       throw new Error(`Invalid tag input expected array of {key, value, propagateAtLaunch} have ${JSON.stringify(cfnPropertyTags)}`);
     }
 
+    const tags: Tag[] = [];
+    const dynamicTags: any = [];
     for (const tag of cfnPropertyTags) {
       if (tag.key === undefined ||
-        tag.value === undefined ||
-        tag.propagateAtLaunch === undefined) {
-        throw new Error(`Invalid tag input expected {key, value, propagateAtLaunch} have ${JSON.stringify(tag)}`);
+          tag.value === undefined ||
+          tag.propagateAtLaunch === undefined) {
+        dynamicTags.push(tag);
+      } else {
+        // using interp to ensure Token is now string
+        tags.push({
+          key: `${tag.key}`,
+          value: `${tag.value}`,
+          priority,
+          applyToLaunchedInstances: !!tag.propagateAtLaunch,
+        });
       }
-      // using interp to ensure Token is now string
-      tags.push({
-        key: `${tag.key}`,
-        value: `${tag.value}`,
-        priority,
-        applyToLaunchedInstances: !!tag.propagateAtLaunch,
-      });
     }
 
-    return tags;
+    return { tags, dynamicTags };
   }
 
   public formatTags(tags: Tag[]): any {
@@ -114,20 +136,20 @@ class AsgFormatter implements ITagFormatter {
         propagateAtLaunch: tag.applyToLaunchedInstances !== false,
       });
     }
-    return cfnTags.length === 0 ? undefined : cfnTags;
+    return cfnTags;
   }
 }
 
 /**
  * Some CloudFormation constructs use a { key: value } map for tags
  */
 class MapFormatter implements ITagFormatter {
-  public parseTags(cfnPropertyTags: any, priority: number): Tag[] {
-    const tags: Tag[] = [];
+  public parseTags(cfnPropertyTags: any, priority: number): ParseTagsResult {
     if (Array.isArray(cfnPropertyTags) || typeof(cfnPropertyTags) !== 'object') {
       throw new Error(`Invalid tag input expected map of {key: value} have ${JSON.stringify(cfnPropertyTags)}`);
     }
 
+    const tags: Tag[] = [];
     for (const [key, value] of Object.entries(cfnPropertyTags)) {
       tags.push({
         key,
@@ -136,23 +158,23 @@ class MapFormatter implements ITagFormatter {
       });
     }
 
-    return tags;
+    return { tags, dynamicTags: undefined };
   }
 
   public formatTags(tags: Tag[]): any {
-    const cfnTags: {[key: string]: string} = {};
+    const cfnTags: { [key: string]: string } = {};
     for (const tag of tags) {
       cfnTags[`${tag.key}`] = `${tag.value}`;
     }
-    return Object.keys(cfnTags).length === 0 ? undefined : cfnTags;
+    return cfnTags;
   }
 }
 
 /**
  * StackTags are of the format { Key: key, Value: value }
  */
 class KeyValueFormatter implements ITagFormatter {
-  public parseTags(keyValueTags: any, priority: number): Tag[] {
+  public parseTags(keyValueTags: any, priority: number): ParseTagsResult {
     const tags: Tag[] = [];
     for (const key in keyValueTags) {
       if (keyValueTags.hasOwnProperty(key)) {
@@ -164,8 +186,9 @@ class KeyValueFormatter implements ITagFormatter {
         });
       }
     }
-    return tags;
+    return { tags, dynamicTags: undefined };
   }
+
   public formatTags(unformattedTags: Tag[]): any {
     const tags: StackTag[] = [];
     unformattedTags.forEach(tag => {
@@ -174,20 +197,20 @@ class KeyValueFormatter implements ITagFormatter {
         Value: tag.value,
       });
     });
-    return tags.length > 0 ? tags : undefined;
+    return tags;
   }
 }
 
 class NoFormat implements ITagFormatter {
-  public parseTags(_cfnPropertyTags: any): Tag[] {
-    return [];
+  public parseTags(_cfnPropertyTags: any): ParseTagsResult {
+    return { tags: [], dynamicTags: undefined };
   }
+
   public formatTags(_tags: Tag[]): any {
     return undefined;
   }
 }
 
-
 let _tagFormattersCache: {[key: string]: ITagFormatter} | undefined;
 
 /**
@@ -203,7 +226,7 @@ function TAG_FORMATTERS(): {[key: string]: ITagFormatter} {
     [TagType.KEY_VALUE]: new KeyValueFormatter(),
     [TagType.NOT_TAGGABLE]: new NoFormat(),
   });
-};
+}
 
 /**
  * Interface to implement tags.
@@ -258,7 +281,6 @@ export interface TagManagerOptions {
  *
  */
 export class TagManager {
-
   /**
    * Check whether the given construct is Taggable
    */
@@ -283,6 +305,7 @@ export class TagManager {
   public readonly renderedTags: IResolvable;
 
   private readonly tags = new Map<string, Tag>();
+  private readonly dynamicTags: any;
   private readonly priorities = new Map<string, number>();
   private readonly tagFormatter: ITagFormatter;
   private readonly resourceTypeName: string;
@@ -292,7 +315,9 @@ export class TagManager {
     this.resourceTypeName = resourceTypeName;
     this.tagFormatter = TAG_FORMATTERS()[tagType];
     if (tagStructure !== undefined) {
-      this._setTag(...this.tagFormatter.parseTags(tagStructure, this.initialTagPriority));
+      const parseTagsResult = this.tagFormatter.parseTags(tagStructure, this.initialTagPriority);
+      this.dynamicTags = parseTagsResult.dynamicTags;
+      this._setTag(...parseTagsResult.tags);
     }
     this.tagPropertyName = options.tagPropertyName || 'tags';
 
@@ -331,7 +356,14 @@ export class TagManager {
    * tags at synthesis time.
    */
   public renderTags(): any {
-    return this.tagFormatter.formatTags(this.sortedTags);
+    const formattedTags = this.tagFormatter.formatTags(this.sortedTags);
+    if (Array.isArray(formattedTags) || Array.isArray(this.dynamicTags)) {
+      const ret = [...formattedTags ?? [], ...this.dynamicTags ?? []];
+      return ret.length > 0 ? ret : undefined;
+    } else {
+      const ret = { ...formattedTags ?? {}, ...this.dynamicTags ?? {} };
+      return Object.keys(ret).length > 0 ? ret : undefined;
+    }
   }
 
   /**
@@ -378,7 +410,8 @@ export class TagManager {
     }
   }
 
-  private get sortedTags() {
-    return Array.from(this.tags.values()).sort((a, b) => a.key.localeCompare(b.key));
+  private get sortedTags(): Tag[] {
+    return Array.from(this.tags.values())
+      .sort((a, b) => a.key.localeCompare(b.key));
   }
 }