diff --git a/packages/aws-cdk-lib/core/lib/cfn-resource.ts b/packages/aws-cdk-lib/core/lib/cfn-resource.ts index 490dd5b646f81..b8b1733f016fc 100644 --- a/packages/aws-cdk-lib/core/lib/cfn-resource.ts +++ b/packages/aws-cdk-lib/core/lib/cfn-resource.ts @@ -482,9 +482,12 @@ export class CfnResource extends CfnRefElement { protected get cfnProperties(): { [key: string]: any } { const props = this._cfnProperties || {}; - if (TagManager.isTaggable(this)) { + const tagMgr = TagManager.of(this); + if (tagMgr) { const tagsProp: { [key: string]: any } = {}; - tagsProp[this.tags.tagPropertyName] = this.tags.renderTags(); + // If this object has a TagManager, then render it out into the correct field. We assume there + // is no shadow tags object, so we don't pass anything to renderTags(). + tagsProp[tagMgr.tagPropertyName] = tagMgr.renderTags(); return deepMerge(props, tagsProp); } return props; diff --git a/packages/aws-cdk-lib/core/lib/tag-aspect.ts b/packages/aws-cdk-lib/core/lib/tag-aspect.ts index 65f79be611ba0..c93d382a6fa80 100644 --- a/packages/aws-cdk-lib/core/lib/tag-aspect.ts +++ b/packages/aws-cdk-lib/core/lib/tag-aspect.ts @@ -1,7 +1,7 @@ import { Construct, IConstruct } from 'constructs'; import { Annotations } from './annotations'; import { IAspect, Aspects } from './aspect'; -import { ITaggable, TagManager } from './tag-manager'; +import { ITaggable, ITaggableV2, TagManager } from './tag-manager'; /** * Properties for a tag @@ -72,12 +72,15 @@ abstract class TagBase implements IAspect { } public visit(construct: IConstruct): void { - if (TagManager.isTaggable(construct)) { + if (TagManager.isTaggableV2(construct)) { + this.applyTagV2(construct); + } else if (TagManager.isTaggable(construct)) { this.applyTag(construct); } } protected abstract applyTag(resource: ITaggable): void; + protected abstract applyTagV2(resource: ITaggableV2): void; } /** @@ -121,8 +124,16 @@ export class Tag extends TagBase { } protected applyTag(resource: ITaggable) { - if (resource.tags.applyTagAspectHere(this.props.includeResourceTypes, this.props.excludeResourceTypes)) { - resource.tags.setTag( + this.applyManager(resource.tags); + } + + protected applyTagV2(resource: ITaggableV2) { + this.applyManager(resource.cdkTagManager); + } + + private applyManager(mgr: TagManager) { + if (mgr.applyTagAspectHere(this.props.includeResourceTypes, this.props.excludeResourceTypes)) { + mgr.setTag( this.key, this.value, this.props.priority ?? this.defaultPriority, @@ -173,8 +184,16 @@ export class RemoveTag extends TagBase { } protected applyTag(resource: ITaggable): void { - if (resource.tags.applyTagAspectHere(this.props.includeResourceTypes, this.props.excludeResourceTypes)) { - resource.tags.removeTag(this.key, this.props.priority ?? this.defaultPriority); + this.applyManager(resource.tags); + } + + protected applyTagV2(resource: ITaggableV2): void { + this.applyManager(resource.cdkTagManager); + } + + private applyManager(mgr: TagManager) { + if (mgr.applyTagAspectHere(this.props.includeResourceTypes, this.props.excludeResourceTypes)) { + mgr.removeTag(this.key, this.props.priority ?? this.defaultPriority); } } } diff --git a/packages/aws-cdk-lib/core/lib/tag-manager.ts b/packages/aws-cdk-lib/core/lib/tag-manager.ts index ed7cd8ce1cae7..c2ef6ec60c3e7 100644 --- a/packages/aws-cdk-lib/core/lib/tag-manager.ts +++ b/packages/aws-cdk-lib/core/lib/tag-manager.ts @@ -238,6 +238,23 @@ export interface ITaggable { readonly tags: TagManager; } +/** + * Modernized version of ITaggable + * + * `ITaggable` has a problem: for a number of L1 resources, we failed to generate + * `tags: TagManager`, and generated `tags: CfnSomeResource.TagProperty[]` instead. + * + * To mark these resources as taggable, we need to put the `TagManager` in a new property + * whose name is unlikely to conflict with any existing properties. Hence, a new interface + * for that purpose. All future resources will implement `ITaggableV2`. + */ +export interface ITaggableV2 { + /** + * TagManager to set, remove and format tags + */ + readonly cdkTagManager: TagManager; +} + /** * Options to configure TagManager behavior */ @@ -283,7 +300,22 @@ export class TagManager { * Check whether the given construct is Taggable */ public static isTaggable(construct: any): construct is ITaggable { - return (construct as any).tags !== undefined; + const tags = (construct as any).tags; + return tags && typeof tags === 'object' && tags.constructor.name === 'TagManager'; + } + + /** + * Check whether the given construct is ITaggableV2 + */ + public static isTaggableV2(construct: any): construct is ITaggableV2 { + return (construct as any).cdkTagManager !== undefined; + } + + /** + * Return the TagManager associated with the given construct, if any + */ + public static of(construct: any): TagManager | undefined { + return TagManager.isTaggableV2(construct) ? construct.cdkTagManager : TagManager.isTaggable(construct) ? construct.tags : undefined; } /** @@ -303,21 +335,19 @@ export class TagManager { public readonly renderedTags: IResolvable; private readonly tags = new Map(); - private readonly dynamicTags: any; + private dynamicTags?: any; private readonly priorities = new Map(); private readonly tagFormatter: ITagFormatter; private readonly resourceTypeName: string; - private readonly initialTagPriority = 50; + private readonly externalTagPriority = 50; + private readonly didHaveInitialTags: boolean; - constructor(tagType: TagType, resourceTypeName: string, tagStructure?: any, options: TagManagerOptions = { }) { + constructor(tagType: TagType, resourceTypeName: string, initialTags?: any, options: TagManagerOptions = { }) { this.resourceTypeName = resourceTypeName; this.tagFormatter = TAG_FORMATTERS()[tagType]; - if (tagStructure !== undefined) { - const parseTagsResult = this.tagFormatter.parseTags(tagStructure, this.initialTagPriority); - this.dynamicTags = parseTagsResult.dynamicTags; - this._setTag(...parseTagsResult.tags); - } this.tagPropertyName = options.tagPropertyName || 'tags'; + this.parseExternalTags(initialTags); + this.didHaveInitialTags = initialTags !== undefined; this.renderedTags = Lazy.any({ produce: () => this.renderTags() }); } @@ -353,8 +383,13 @@ export class TagManager { * which will return a `Lazy` value that will resolve to the correct * tags at synthesis time. */ - public renderTags(): any { + public renderTags(combineWithTags?: any): any { + if (combineWithTags !== undefined && this.didHaveInitialTags) { + throw new Error('Specify external tags either during the creation of TagManager, or as a parameter to renderTags(), but not both'); + } + this.parseExternalTags(combineWithTags); 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; @@ -412,4 +447,17 @@ export class TagManager { return Array.from(this.tags.values()) .sort((a, b) => a.key.localeCompare(b.key)); } + + /** + * Parse external tags. + * + * Set the parseable ones into this tag manager. Save the rest (tokens, lazies) in `this.dynamicTags`. + */ + private parseExternalTags(initialTags: any) { + if (initialTags !== undefined) { + const parseTagsResult = this.tagFormatter.parseTags(initialTags, this.externalTagPriority); + this.dynamicTags = parseTagsResult.dynamicTags; + this._setTag(...parseTagsResult.tags); + } + } } diff --git a/packages/aws-cdk-lib/core/test/tag-aspect.test.ts b/packages/aws-cdk-lib/core/test/tag-aspect.test.ts index 8695f4e1393bb..a39f4063d0c19 100644 --- a/packages/aws-cdk-lib/core/test/tag-aspect.test.ts +++ b/packages/aws-cdk-lib/core/test/tag-aspect.test.ts @@ -1,8 +1,9 @@ import { Construct } from 'constructs'; -import { CfnResource, CfnResourceProps, RemoveTag, Stack, Tag, TagManager, TagType, Aspects, Tags } from '../lib'; +import { toCloudFormation } from './util'; +import { CfnResource, CfnResourceProps, RemoveTag, Stack, Tag, TagManager, TagType, Aspects, Tags, ITaggable, ITaggableV2 } from '../lib'; import { synthesize } from '../lib/private/synthesis'; -class TaggableResource extends CfnResource { +class TaggableResource extends CfnResource implements ITaggable { public readonly tags: TagManager; constructor(scope: Construct, id: string, props: CfnResourceProps) { super(scope, id, props); @@ -14,7 +15,19 @@ class TaggableResource extends CfnResource { } } -class AsgTaggableResource extends CfnResource { +class TaggableResource2 extends CfnResource implements ITaggableV2 { + public readonly cdkTagManager: TagManager; + constructor(scope: Construct, id: string, props: CfnResourceProps) { + super(scope, id, props); + const tags = props.properties?.tags; + this.cdkTagManager = new TagManager(TagType.STANDARD, 'AWS::Fake::Resource', tags); + } + public testProperties() { + return this.cfnProperties; + } +} + +class AsgTaggableResource extends CfnResource implements ITaggable { public readonly tags: TagManager; constructor(scope: Construct, id: string, props: CfnResourceProps) { super(scope, id, props); @@ -26,7 +39,7 @@ class AsgTaggableResource extends CfnResource { } } -class MapTaggableResource extends CfnResource { +class MapTaggableResource extends CfnResource implements ITaggable { public readonly tags: TagManager; constructor(scope: Construct, id: string, props: CfnResourceProps) { super(scope, id, props); @@ -39,12 +52,14 @@ class MapTaggableResource extends CfnResource { } describe('tag aspect', () => { - test('Tag visit all children of the applied node', () => { + test.each([ + ['TaggableResource', TaggableResource], ['TaggableResource2', TaggableResource2], + ])('Tag visit all children of the applied node, using class %s', (_, taggableClass) => { const root = new Stack(); - const res = new TaggableResource(root, 'FakeResource', { + const res = new taggableClass(root, 'FakeResource', { type: 'AWS::Fake::Thing', }); - const res2 = new TaggableResource(res, 'FakeResource', { + const res2 = new taggableClass(res, 'FakeResource', { type: 'AWS::Fake::Thing', }); const asg = new AsgTaggableResource(res, 'AsgFakeResource', { @@ -58,10 +73,18 @@ describe('tag aspect', () => { synthesize(root); - expect(res.tags.renderTags()).toEqual([{ key: 'foo', value: 'bar' }]); - expect(res2.tags.renderTags()).toEqual([{ key: 'foo', value: 'bar' }]); + expect(TagManager.of(res)?.renderTags()).toEqual([{ key: 'foo', value: 'bar' }]); + expect(TagManager.of(res2)?.renderTags()).toEqual([{ key: 'foo', value: 'bar' }]); expect(map.tags.renderTags()).toEqual({ foo: 'bar' }); expect(asg.tags.renderTags()).toEqual([{ key: 'foo', value: 'bar', propagateAtLaunch: true }]); + + const template = toCloudFormation(root); + expect(template.Resources.FakeResource).toEqual({ + Type: 'AWS::Fake::Thing', + Properties: { + tags: [{ key: 'foo', value: 'bar' }], + }, + }); }); test('The last aspect applied takes precedence', () => { diff --git a/packages/aws-cdk-lib/core/test/tag-manager.test.ts b/packages/aws-cdk-lib/core/test/tag-manager.test.ts index 5911447e1ea6a..86641a7a8af20 100644 --- a/packages/aws-cdk-lib/core/test/tag-manager.test.ts +++ b/packages/aws-cdk-lib/core/test/tag-manager.test.ts @@ -9,6 +9,34 @@ describe('tag manager', () => { expect(mgr.tagPropertyName).toEqual(tagPropName); }); + test.each(['early' as const, 'late' as const])('supplying tags %s works for MAP tags', (when) => { + const externalTags = { someTag: 'someValue' }; + const mgr = new TagManager(TagType.MAP, 'Foo', when === 'early' ? externalTags : undefined); + mgr.setTag('givenTag', 'givenValue'); + + expect(mgr.renderTags(when === 'late' ? externalTags : undefined)).toEqual({ + givenTag: 'givenValue', + someTag: 'someValue', + }); + }); + + test.each(['early' as const, 'late' as const])('supplying tags %s works for STANDARD tags', (when) => { + const externalTags = [{ key: 'someTag', value: 'someValue' }]; + const mgr = new TagManager(TagType.STANDARD, 'Foo', when === 'early' ? externalTags : undefined); + mgr.setTag('givenTag', 'givenValue'); + + expect(mgr.renderTags(when === 'late' ? externalTags : undefined)).toEqual([ + { + key: 'givenTag', + value: 'givenValue', + }, + { + key: 'someTag', + value: 'someValue', + }, + ]); + }); + test('#setTag() supports setting a tag regardless of Type', () => { const notTaggable = new TagManager(TagType.NOT_TAGGABLE, 'AWS::Resource::Type'); notTaggable.setTag('key', 'value'); @@ -129,6 +157,61 @@ describe('tag manager', () => { ]); }); + test('can add direct tags: STANDARD', () => { + // GIVEN + const mgr = new TagManager(TagType.STANDARD, 'AWS::Resource::Type'); + + // WHEN + mgr.setTag('key', 'value'); + const rendered = mgr.renderTags([ + { key: 'key2', value: 'value2' }, + ]); + + // THEN + expect(rendered).toEqual([ + { key: 'key', value: 'value' }, + { key: 'key2', value: 'value2' }, + ]); + }); + + test('can add direct tags: MAP', () => { + // GIVEN + const mgr = new TagManager(TagType.MAP, 'AWS::Resource::Type'); + + // WHEN + mgr.setTag('key', 'value'); + const rendered = mgr.renderTags({ + key2: 'value2', + }); + + // THEN + expect(rendered).toEqual({ + key: 'value', + key2: 'value2', + }); + }); + + test('may not specify external tags both at TagManager creation AND into renderTags', () => { + // GIVEN + const mgr = new TagManager(TagType.MAP, 'AWS::Resource::Type', { initial: 'tag' }); + + // WHEN + expect(() => mgr.renderTags({ + external: 'tag', + })).toThrow(/not both/); + }); + + test('it is safe to call renderTags multiple times with external tags', () => { + // GIVEN + const mgr = new TagManager(TagType.STANDARD, 'AWS::Resource::Type'); + mgr.setTag('tagOne', 'one'); + mgr.setTag('tagTwo', 'two'); + + // WHEN + const renders = [1, 2].map(() => mgr.renderTags([{ key: 'external', value: 'tag' }])); + expect(renders[0]).toEqual(renders[1]); + }); + test('excludeResourceTypes only tags resources that do not match', () => { const mgr = new TagManager(TagType.STANDARD, 'AWS::Fake::Resource'); diff --git a/tools/@aws-cdk/cfn2ts/lib/codegen.ts b/tools/@aws-cdk/cfn2ts/lib/codegen.ts index 1e9a0637d3e86..9457d7bca6e92 100644 --- a/tools/@aws-cdk/cfn2ts/lib/codegen.ts +++ b/tools/@aws-cdk/cfn2ts/lib/codegen.ts @@ -17,6 +17,8 @@ enum TreeAttributes { CFN_PROPS = 'aws:cdk:cloudformation:props' } +export const LEGACY_TAGGING: Record = {}; + interface Dictionary { [key: string]: T; } export interface CodeGeneratorOptions { @@ -351,6 +353,7 @@ export default class CodeGenerator { this.code.line(); for (const prop of Object.values(propMap)) { if (schema.isTagPropertyName(upcaseFirst(prop)) && schema.isTaggableResource(spec)) { + LEGACY_TAGGING[cfnName] = upcaseFirst(prop); this.code.line(`this.tags = new ${TAG_MANAGER}(${tagType(spec)}, ${cfnResourceTypeName}, props.${prop}, { tagPropertyName: '${prop}' });`); } else { this.code.line(`this.${prop} = props.${prop};`); diff --git a/tools/@aws-cdk/cfn2ts/lib/index.ts b/tools/@aws-cdk/cfn2ts/lib/index.ts index 0ebfe78828ed7..dc94e54f93dc8 100644 --- a/tools/@aws-cdk/cfn2ts/lib/index.ts +++ b/tools/@aws-cdk/cfn2ts/lib/index.ts @@ -4,7 +4,7 @@ import * as pkglint from '@aws-cdk/pkglint'; import * as fs from 'fs-extra'; import { AugmentationGenerator, AugmentationsGeneratorOptions } from './augmentation-generator'; import { CannedMetricsGenerator } from './canned-metrics-generator'; -import CodeGenerator, { CodeGeneratorOptions } from './codegen'; +import CodeGenerator, { CodeGeneratorOptions, LEGACY_TAGGING } from './codegen'; import { packageName } from './genspec'; interface GenerateOutput { @@ -141,6 +141,8 @@ export async function generateAll( moduleMap[name].files = outputFiles; })); + await fs.writeJson('tagging.json', LEGACY_TAGGING, { spaces: 2 }); + return moduleMap; }