fix(cdk): merge cloudFormation tags with aspect tags (#1762)

This modifies the behavior of TagManager to enable the merging of tags
provided during Cfn* properties with tag aspects. If a collision occurs
the aspects tag precedence.

Fixes #1725

Author: moofish32

packages/@aws-cdk/cdk/README.md

@@ -117,11 +117,19 @@ has a few features that are covered later to explain how this works.
 
 ### API
 
-In order to enable additional controls a Tags can specifically include or
+In order to enable additional controls a Tag can specifically include or
 exclude a CloudFormation Resource Type, propagate tags for an autoscaling group,
 and use priority to override the default precedence. See the `TagProps` 
 interface for more details. 
 
+Tags can be configured by using the properties for the AWS CloudFormation layer
+resources or by using the tag aspects described here. The aspects will always
+take precedence over the AWS CloudFormation layer in the event of a name
+collision. The tags will be merged otherwise. For the aspect based tags, the
+tags applied closest to the resource will take precedence, given an equal
+priority. A higher priority tag will always take precedence over a lower
+priority tag.
+
 #### applyToLaunchedInstances
 
 This property is a boolean that defaults to `true`. When `true` and the aspect

packages/@aws-cdk/cdk/lib/aspects/tag-aspect.ts

@@ -1,8 +1,58 @@
 import { ITaggable, Resource } from '../cloudformation/resource';
 import { IConstruct } from '../core/construct';
-import { TagProps } from '../core/tag-manager';
 import { IAspect } from './aspect';
 
+/**
+ * Properties for a tag
+ */
+export interface TagProps {
+  /**
+   * Whether the tag should be applied to instances in an AutoScalingGroup
+   *
+   * @default true
+   */
+  applyToLaunchedInstances?: boolean;
+
+  /**
+   * An array of Resource Types that will not receive this tag
+   *
+   * An empty array will allow this tag to be applied to all resources. A
+   * non-empty array will apply this tag only if the Resource type is not in
+   * this array.
+   * @default []
+   */
+  excludeResourceTypes?: string[];
+
+  /**
+   * An array of Resource Types that will receive this tag
+   *
+   * An empty array will match any Resource. A non-empty array will apply this
+   * tag only to Resource types that are included in this array.
+   * @default []
+   */
+  includeResourceTypes?: string[];
+
+  /**
+   * Priority of the tag operation
+   *
+   * Higher or equal priority tags will take precedence.
+   *
+   * Setting priority will enable the user to control tags when they need to not
+   * follow the default precedence pattern of last applied and closest to the
+   * construct in the tree.
+   *
+   * @default
+   *
+   * Default priorities:
+   *
+   * - 100 for {@link SetTag}
+   * - 200 for {@link RemoveTag}
+   * - 50 for tags added directly to CloudFormation resources
+   *
+   */
+  priority?: number;
+}
+
 /**
  * The common functionality for Tag and Remove Tag Aspects
  */
@@ -43,18 +93,25 @@ export class Tag extends TagBase {
    */
   public readonly value: string;
 
+  private readonly defaultPriority = 100;
+
   constructor(key: string, value: string, props: TagProps = {}) {
     super(key, props);
-    this.props.applyToLaunchedInstances = props.applyToLaunchedInstances !== false;
-    this.props.priority = props.priority === undefined ? 0 : props.priority;
     if (value === undefined) {
       throw new Error('Tag must have a value');
     }
     this.value = value;
   }
 
   protected applyTag(resource: ITaggable) {
-    resource.tags.setTag(this.key, this.value!, this.props);
+    if (resource.tags.applyTagAspectHere(this.props.includeResourceTypes, this.props.excludeResourceTypes)) {
+      resource.tags.setTag(
+        this.key,
+        this.value,
+        this.props.priority !== undefined ? this.props.priority : this.defaultPriority,
+        this.props.applyToLaunchedInstances !== false
+      );
+    }
   }
 }
 
@@ -63,13 +120,15 @@ export class Tag extends TagBase {
  */
 export class RemoveTag extends TagBase {
 
+  private readonly defaultPriority = 200;
+
   constructor(key: string, props: TagProps = {}) {
     super(key, props);
-    this.props.priority = props.priority === undefined ? 1 : props.priority;
   }
 
   protected applyTag(resource: ITaggable): void {
-    resource.tags.removeTag(this.key, this.props);
-    return;
+    if (resource.tags.applyTagAspectHere(this.props.includeResourceTypes, this.props.excludeResourceTypes)) {
+      resource.tags.removeTag(this.key, this.props.priority !== undefined ? this.props.priority : this.defaultPriority);
+    }
   }
 }

packages/@aws-cdk/cdk/lib/cloudformation/resource.ts

@@ -206,12 +206,13 @@ export class Resource extends Referenceable {
    */
   public toCloudFormation(): object {
     try {
-      if (Resource.isTaggable(this)) {
-        const tags = this.tags.renderTags();
-        this.properties.tags = tags === undefined ? this.properties.tags : tags;
-      }
       // merge property overrides onto properties and then render (and validate).
-      const properties = this.renderProperties(deepMerge(this.properties || { }, this.untypedPropertyOverrides));
+      const tags = Resource.isTaggable(this) ? this.tags.renderTags() : undefined;
+      const properties = this.renderProperties(deepMerge(
+        this.properties || {},
+        { tags },
+        this.untypedPropertyOverrides
+      ));
 
       return {
         Resources: {
@@ -254,7 +255,6 @@ export class Resource extends Referenceable {
   protected renderProperties(properties: any): { [key: string]: any } {
     return properties;
   }
-
 }
 
 export enum TagType {
@@ -312,33 +312,35 @@ export interface ResourceOptions {
  * Merges `source` into `target`, overriding any existing values.
  * `null`s will cause a value to be deleted.
  */
-export function deepMerge(target: any, source: any) {
-  if (typeof(source) !== 'object' || typeof(target) !== 'object') {
-    throw new Error(`Invalid usage. Both source (${JSON.stringify(source)}) and target (${JSON.stringify(target)}) must be objects`);
-  }
+export function deepMerge(target: any, ...sources: any[]) {
+  for (const source of sources) {
+    if (typeof(source) !== 'object' || typeof(target) !== 'object') {
+      throw new Error(`Invalid usage. Both source (${JSON.stringify(source)}) and target (${JSON.stringify(target)}) must be objects`);
+    }
 
-  for (const key of Object.keys(source)) {
-    const value = source[key];
-    if (typeof(value) === 'object' && value != null && !Array.isArray(value)) {
-      // if the value at the target is not an object, override it with an
-      // object so we can continue the recursion
-      if (typeof(target[key]) !== 'object') {
-        target[key] = { };
-      }
+    for (const key of Object.keys(source)) {
+      const value = source[key];
+      if (typeof(value) === 'object' && value != null && !Array.isArray(value)) {
+        // if the value at the target is not an object, override it with an
+        // object so we can continue the recursion
+        if (typeof(target[key]) !== 'object') {
+          target[key] = { };
+        }
 
-      deepMerge(target[key], value);
+        deepMerge(target[key], value);
 
-      // if the result of the merge is an empty object, it's because the
-      // eventual value we assigned is `undefined`, and there are no
-      // sibling concrete values alongside, so we can delete this tree.
-      const output = target[key];
-      if (typeof(output) === 'object' && Object.keys(output).length === 0) {
+        // if the result of the merge is an empty object, it's because the
+        // eventual value we assigned is `undefined`, and there are no
+        // sibling concrete values alongside, so we can delete this tree.
+        const output = target[key];
+        if (typeof(output) === 'object' && Object.keys(output).length === 0) {
+          delete target[key];
+        }
+      } else if (value === undefined) {
         delete target[key];
+      } else {
+        target[key] = value;
       }
-    } else if (value === undefined) {
-      delete target[key];
-    } else {
-      target[key] = value;
     }
   }