fix(tags): excludeResourceTypes does not work for tags applied at stack level (under feature flag) (#31443)

Stacks are considered taggable, and so `Tags.of(this).add('key', 'value')` used to add tags to Stacks in scope. Usually this happens if `this` is an instance of `Stack`, which it commonly is in user code.

Since `Tags.of(...)` walks the construct tree, it will add tags to the stack *and* to all the resources in the stack. Then, come deploy time, CloudFormation will also try and apply all the stack tags to the resources again. This is both unnecessary, as well as leads to loss of control: `excludeResourceTypes` appears to not work, since it will lead to resources not being tagged in the template (good) but then the resources will still be tagged by CloudFormation because the stack itself is tagged (bad).

Also, if the tags applied this way contain intrinsics, they will contain nonsense because they are applied in a context where CloudFormation expressions don't work.

## In this change

There is way to prevent Stacks from being tagged, by including `aws:cdk:stack` in the list of `excludeResourceTypes` (this is a fake resource type that Stack tags respect).

Under a feature flag, `@aws-cdk/core:explicitStackTags`, this is now the default behavior. That resource type will be excluded by default, unless it is listed in the `includeResourceTypes` list. However, doing `includeResourceTypes` is still not desirable: stack tags should be applied directly on the `Stack` object if desired.

This requires a user to make a conscious decision between resource-level and stack-level tagging: either apply tags to the stack, which will apply it to all resources but remove the ability to do `excludeResourceTypes`; or apply tags to (groups of) resources inside the template.

Another benefit is that for tags applied at the stack level, this will resolve the following issue: #15947, as resources "becoming" taggable all of a sudden will not affect the template anymore.

Closes #28017. Closes #33945. Closes #30055.

----

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

Author: rix0rrr

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

@@ -121,7 +121,15 @@ export interface StackProps {
   readonly stackName?: string;
 
   /**
-   * Stack tags that will be applied to all the taggable resources and the stack itself.
+   * Tags that will be applied to the Stack
+   *
+   * These tags are applied to the CloudFormation Stack itself. They will not
+   * appear in the CloudFormation template.
+   *
+   * However, at deployment time, CloudFormation will apply these tags to all
+   * resources in the stack that support tagging. You will not be able to exempt
+   * resources from tagging (using the `excludeResourceTypes` property of
+   * `Tags.of(...).add()`) for tags applied in this way.
    *
    * @default {}
    */
@@ -1609,6 +1617,24 @@ export class Stack extends Construct implements ITaggable {
       pattern,
     ));
   }
+
+  /**
+   * Configure a stack tag
+   *
+   * At deploy time, CloudFormation will automatically apply all stack tags to all resources in the stack.
+   */
+  public addStackTag(tagName: string, tagValue: string) {
+    this.tags.setTag(tagName, tagValue);
+  }
+
+  /**
+   * Remove a stack tag
+   *
+   * At deploy time, CloudFormation will automatically apply all stack tags to all resources in the stack.
+   */
+  public removeStackTag(tagName: string) {
+    this.tags.removeTag(tagName, 0);
+  }
 }
 
 function merge(template: any, fragment: any): void {

packages/aws-cdk-lib/core/lib/tag-aspect.ts

@@ -2,6 +2,8 @@ import { Construct, IConstruct } from 'constructs';
 import { Annotations } from './annotations';
 import { IAspect, Aspects, AspectOptions } from './aspect';
 import { UnscopedValidationError } from './errors';
+import { FeatureFlags } from './feature-flags';
+import * as cxapi from '../../cx-api';
 import { mutatingAspectPrio32333 } from './private/aspect-prio';
 import { ITaggable, ITaggableV2, TagManager } from './tag-manager';
 
@@ -22,6 +24,7 @@ export interface TagProps {
    * 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 []
    */
   readonly excludeResourceTypes?: string[];
@@ -155,12 +158,41 @@ export class Tags {
     return new Tags(scope);
   }
 
-  private constructor(private readonly scope: IConstruct) { }
+  private readonly explicitStackTags: boolean;
+
+  private constructor(private readonly scope: IConstruct) {
+    this.explicitStackTags = FeatureFlags.of(scope).isEnabled(cxapi.EXPLICIT_STACK_TAGS) ?? false;
+  }
 
   /**
-   * add tags to the node of a construct and all its the taggable children
+   * Add tags to the node of a construct and all its the taggable children
+   *
+   * ## Tagging and CloudFormation Stacks
+   *
+   * If the feature flag `@aws-cdk/core:explicitStackTags` is set to `true`
+   * (recommended modern behavior), Stacks will not automatically be tagged.
+   * Stack tags should be configured on Stacks directly (preferred), or
+   * you must explicitly include the resource type `aws:cdk:stack` in the
+   * `includeResourceTypes` array.
+   *
+   * If the feature flag is set to `false` (legacy behavior) then both Stacks
+   * and resources in the indicated scope will both be tagged by default, which
+   * leads to tags being applied twice (once in the template, and then once
+   * again automatically by CloudFormation as part of the stack deployment).
+   * That behavior leads to loss of control as `excludeResourceTypes` will
+   * prevent tags from appearing in the template, but they will still be
+   * applied to the Stack and hence CloudFormation will still apply them
+   * to the resource.
    */
   public add(key: string, value: string, props: TagProps = {}) {
+    // Implicitly add `aws:cdk:stack` to the `excludeResourceTypes` array in modern behavior
+    if (this.explicitStackTags && !props.includeResourceTypes?.includes('aws:cdk:stack')) {
+      props = {
+        ...props,
+        excludeResourceTypes: [...props.excludeResourceTypes ?? [], 'aws:cdk:stack'],
+      };
+    }
+
     const tag = new Tag(key, value, props);
     const options: AspectOptions = { priority: mutatingAspectPrio32333(this.scope) };
     Aspects.of(this.scope).add(tag, options);

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

@@ -14,6 +14,11 @@ import {
   PERMISSIONS_BOUNDARY_CONTEXT_KEY,
   Aspects,
   Stage,
+  TagManager,
+  Resource,
+  TagType,
+  ITaggable,
+  ITaggableV2,
   Token,
 } from '../lib';
 import { Intrinsic } from '../lib/private/intrinsic';
@@ -2162,6 +2167,89 @@ describe('stack', () => {
     expect(asm.getStackArtifact(stack2.artifactId).tags).toEqual(expected);
   });
 
+  describe.each([false, true])(`with ${cxapi.EXPLICIT_STACK_TAGS} set to %p`, (explicitStackTags) => {
+    let app: App;
+
+    beforeEach(() => {
+      app = new App({
+        stackTraces: false,
+        context: {
+          [cxapi.NEW_STYLE_STACK_SYNTHESIS_CONTEXT]: false,
+          [cxapi.EXPLICIT_STACK_TAGS]: explicitStackTags,
+        },
+      });
+    });
+
+    test('stack tags are both in stack metadata and stack artifact properties', () => {
+      // GIVEN
+
+      const stack = new Stack(app, 'stack1', {
+        tags: {
+          foo: 'bar',
+        },
+      });
+
+      // THEN
+      const asm = app.synth();
+
+      const stackArtifact = asm.getStackArtifact(stack.artifactId);
+      expect(stackArtifact.manifest.metadata).toEqual({
+        '/stack1': [
+          {
+            type: 'aws:cdk:stack-tags',
+            data: [{ key: 'foo', value: 'bar' }],
+          },
+        ],
+      });
+      expect(stackArtifact.tags).toEqual({ foo: 'bar' });
+    });
+
+    test('stack tags do not appear in template', () => {
+      const stack = new Stack(app, 'stack1', {
+        tags: {
+          foo: 'bar',
+        },
+      });
+      new TaggableResource(stack, 'res');
+
+      // THEN
+      const asm = app.synth();
+      const stackArtifact = asm.getStackArtifact(stack.artifactId);
+      expect(stackArtifact.template.Resources.res).toEqual({
+        Type: 'AWS::Taggable::Resource',
+        Properties: {
+          R: 1,
+        },
+      });
+    });
+
+    test('tags added using Tags.of() are or are not applied to Stacks', () => {
+      const stack = new Stack(app, 'stack1');
+      Tags.of(stack).add('resourceTag', 'resourceValue');
+
+      // THEN
+      const asm = app.synth();
+      const stackArtifact = asm.getStackArtifact(stack.artifactId);
+      if (explicitStackTags) {
+        expect(stackArtifact.tags).toEqual({});
+      } else {
+        expect(stackArtifact.tags).toEqual({ resourceTag: 'resourceValue' });
+      }
+    });
+
+    test('tags added using Tags.of() are applied to Stacks if explicitly included', () => {
+      const stack = new Stack(app, 'stack1');
+      Tags.of(stack).add('resourceTag', 'resourceValue', {
+        includeResourceTypes: ['aws:cdk:stack'],
+      });
+
+      // THEN
+      const asm = app.synth();
+      const stackArtifact = asm.getStackArtifact(stack.artifactId);
+      expect(stackArtifact.tags).toEqual({ resourceTag: 'resourceValue' });
+    });
+  });
+
   test('warning when stack tags contain tokens', () => {
     // GIVEN
     const app = new App({
@@ -2573,3 +2661,19 @@ class StackWithPostProcessor extends Stack {
     return template;
   }
 }
+
+class TaggableResource extends CfnResource implements ITaggableV2 {
+  public readonly cdkTagManager = new TagManager(TagType.KEY_VALUE, 'TaggableResource', {}, {
+    tagPropertyName: 'Tags',
+  });
+
+  constructor(scope: Construct, id: string) {
+    super(scope, id, {
+      type: 'AWS::Taggable::Resource',
+      properties: {
+        R: 1,
+        Tags: Lazy.any({ produce: () => this.cdkTagManager.renderTags() }),
+      },
+    });
+  }
+}

packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md

@@ -104,6 +104,7 @@ Flags come in three types:
 | [@aws-cdk/aws-s3:publicAccessBlockedByDefault](#aws-cdkaws-s3publicaccessblockedbydefault) | When enabled, setting any combination of options for BlockPublicAccess will automatically set true for any options not defined. | 2.196.0 | fix |
 | [@aws-cdk/aws-lambda:useCdkManagedLogGroup](#aws-cdkaws-lambdausecdkmanagedloggroup) | When enabled, CDK creates and manages loggroup for the lambda function | 2.200.0 | new default |
 | [@aws-cdk/aws-kms:applyImportedAliasPermissionsToPrincipal](#aws-cdkaws-kmsapplyimportedaliaspermissionstoprincipal) | Enable grant methods on Aliases imported by name to use kms:ResourceAliases condition | 2.202.0 | fix |
+| [@aws-cdk/core:explicitStackTags](#aws-cdkcoreexplicitstacktags) | When enabled, stack tags need to be assigned explicitly on a Stack. | V2NEXT | new default |
 
 <!-- END table -->
 
@@ -167,6 +168,7 @@ The following json shows the current recommended set of flags, as `cdk init` wou
     "@aws-cdk/aws-ecs:removeDefaultDeploymentAlarm": true,
     "@aws-cdk/custom-resources:logApiResponseDataPropertyTrueDefault": false,
     "@aws-cdk/aws-s3:keepNotificationInImportedBucket": false,
+    "@aws-cdk/core:explicitStackTags": true,
     "@aws-cdk/aws-ecs:enableImdsBlockingDeprecatedFeature": false,
     "@aws-cdk/aws-ecs:disableEcsImdsBlocking": true,
     "@aws-cdk/aws-ecs:reduceEc2FargateCloudWatchPermissions": true,
@@ -2196,4 +2198,31 @@ When disabled, grant calls on imported aliases will be dropped (no-op) to mainta
 **Compatibility with old behavior:** Remove calls to the grant* methods on the aliases referenced by name
 
 
+### @aws-cdk/core:explicitStackTags
+
+*When enabled, stack tags need to be assigned explicitly on a Stack.*
+
+Flag type: New default behavior
+
+Without this feature flag enabled, if tags are added to a Stack using
+`Tags.of(scope).add(...)`, they will be added to both the stack and all resources
+in the stack template.
+
+That leads to the tags being applied twice: once in the template, and once
+again automatically by CloudFormation, which will apply all stack tags to
+all resources in the stack. This leads to loss of control, as the
+`excludeResourceTypes` option of the Tags API will not have any effect.
+
+With this flag enabled, tags added to a stack using `Tags.of(...)` are ignored,
+and Stack tags must be configured explicitly on the Stack object.
+
+
+| Since | Unset behaves like | Recommended value |
+| ----- | ----- | ----- |
+| (not in v1) |  |  |
+| V2NEXT | `false` | `true` |
+
+**Compatibility with old behavior:** Configure stack-level tags using `new Stack(..., { tags: { ... } })`.
+
+
 <!-- END details -->

packages/aws-cdk-lib/cx-api/lib/features.ts

@@ -112,6 +112,7 @@ export const ECS_REMOVE_DEFAULT_DEPLOYMENT_ALARM = '@aws-cdk/aws-ecs:removeDefau
 export const LOG_API_RESPONSE_DATA_PROPERTY_TRUE_DEFAULT = '@aws-cdk/custom-resources:logApiResponseDataPropertyTrueDefault';
 export const S3_KEEP_NOTIFICATION_IN_IMPORTED_BUCKET = '@aws-cdk/aws-s3:keepNotificationInImportedBucket';
 export const USE_NEW_S3URI_PARAMETERS_FOR_BEDROCK_INVOKE_MODEL_TASK = '@aws-cdk/aws-stepfunctions-tasks:useNewS3UriParametersForBedrockInvokeModelTask';
+export const EXPLICIT_STACK_TAGS = '@aws-cdk/core:explicitStackTags';
 export const REDUCE_EC2_FARGATE_CLOUDWATCH_PERMISSIONS = '@aws-cdk/aws-ecs:reduceEc2FargateCloudWatchPermissions';
 export const DYNAMODB_TABLEV2_RESOURCE_POLICY_PER_REPLICA = '@aws-cdk/aws-dynamodb:resourcePolicyPerReplica';
 export const EC2_SUM_TIMEOUT_ENABLED = '@aws-cdk/aws-ec2:ec2SumTImeoutEnabled';
@@ -1177,6 +1178,26 @@ export const FLAGS: Record<string, FlagInfo> = {
   },
 
   //////////////////////////////////////////////////////////////////////
+  [EXPLICIT_STACK_TAGS]: {
+    type: FlagType.ApiDefault,
+    summary: 'When enabled, stack tags need to be assigned explicitly on a Stack.',
+    detailsMd: `
+    Without this feature flag enabled, if tags are added to a Stack using
+    \`Tags.of(scope).add(...)\`, they will be added to both the stack and all resources
+    in the stack template.
+
+    That leads to the tags being applied twice: once in the template, and once
+    again automatically by CloudFormation, which will apply all stack tags to
+    all resources in the stack. This leads to loss of control, as the
+    \`excludeResourceTypes\` option of the Tags API will not have any effect.
+
+    With this flag enabled, tags added to a stack using \`Tags.of(...)\` are ignored,
+    and Stack tags must be configured explicitly on the Stack object.
+    `,
+    introducedIn: { v2: 'V2NEXT' },
+    recommendedValue: true,
+    compatibilityWithOldBehaviorMd: 'Configure stack-level tags using `new Stack(..., { tags: { ... } })`.',
+  },
   [Enable_IMDS_Blocking_Deprecated_Feature]: {
     type: FlagType.Temporary,
     summary: 'When set to true along with canContainersAccessInstanceRole=false in ECS cluster, new updated ' +

packages/aws-cdk-lib/recommended-feature-flags.json

@@ -51,6 +51,7 @@
   "@aws-cdk/aws-ecs:removeDefaultDeploymentAlarm": true,
   "@aws-cdk/custom-resources:logApiResponseDataPropertyTrueDefault": false,
   "@aws-cdk/aws-s3:keepNotificationInImportedBucket": false,
+  "@aws-cdk/core:explicitStackTags": true,
   "@aws-cdk/aws-ecs:enableImdsBlockingDeprecatedFeature": false,
   "@aws-cdk/aws-ecs:disableEcsImdsBlocking": true,
   "@aws-cdk/aws-ecs:reduceEc2FargateCloudWatchPermissions": true,