From 98226c1e40db4a3770a4687a0632a54d6e053ad6 Mon Sep 17 00:00:00 2001 From: Ismael Martinez Ramos Date: Fri, 5 Apr 2019 14:41:52 +0100 Subject: [PATCH 01/25] feat(cdk): adding tags parameter option to cdk deploy command to allow to tag full stacks and their associated resources. fixes #932 --- packages/aws-cdk/bin/cdk.ts | 5 +- packages/aws-cdk/lib/api/deploy-stack.ts | 4 +- packages/aws-cdk/lib/api/deployment-target.ts | 2 + packages/aws-cdk/lib/cdk-toolkit.ts | 6 ++ packages/aws-cdk/lib/settings.ts | 55 ++++++++++++++++--- 5 files changed, 61 insertions(+), 11 deletions(-) diff --git a/packages/aws-cdk/bin/cdk.ts b/packages/aws-cdk/bin/cdk.ts index b629bb0ce9a27..f1d68bc4f3897 100644 --- a/packages/aws-cdk/bin/cdk.ts +++ b/packages/aws-cdk/bin/cdk.ts @@ -63,6 +63,7 @@ async function parseCommandLineArguments() { .option('exclusively', { type: 'boolean', alias: 'e', desc: 'only deploy requested stacks, don\'t include dependencies' }) .option('require-approval', { type: 'string', choices: [RequireApproval.Never, RequireApproval.AnyChange, RequireApproval.Broadening], desc: 'what security-sensitive changes need manual approval' })) .option('ci', { type: 'boolean', desc: 'Force CI detection. Use --no-ci to disable CI autodetection.', default: process.env.CI !== undefined }) + .option('tags', { type: 'array', alias: 't', desc: 'tags to add to the stack', nargs: 1, requiresArg: 'KEY=VALUE'}) .command('destroy [STACKS..]', 'Destroy the stack(s) named STACKS', yargs => yargs .option('exclusively', { type: 'boolean', alias: 'x', desc: 'only deploy requested stacks, don\'t include dependees' }) .option('force', { type: 'boolean', alias: 'f', desc: 'Do not ask for confirmation before destroying the stacks' })) @@ -105,7 +106,6 @@ async function initCommandLine() { proxyAddress: argv.proxy, ec2creds: argv.ec2creds, }); - const configuration = new Configuration(argv); await configuration.load(); @@ -198,7 +198,8 @@ async function initCommandLine() { roleArn: args.roleArn, requireApproval: configuration.settings.get(['requireApproval']), ci: args.ci, - reuseAssets: args['build-exclude'] + reuseAssets: args['build-exclude'], + tags: configuration.settings.get(['tags']) }); case 'destroy': diff --git a/packages/aws-cdk/lib/api/deploy-stack.ts b/packages/aws-cdk/lib/api/deploy-stack.ts index d7c8f96da323a..cb247c3b1c379 100644 --- a/packages/aws-cdk/lib/api/deploy-stack.ts +++ b/packages/aws-cdk/lib/api/deploy-stack.ts @@ -32,6 +32,7 @@ export interface DeployStackOptions { quiet?: boolean; ci?: boolean; reuseAssets?: string[]; + tags?: [ {Key: string, Value: string}] } const LARGE_TEMPLATE_SIZE_KB = 50; @@ -73,7 +74,8 @@ export async function deployStack(options: DeployStackOptions): Promise { From 26c8921c885dddc05adf4178cf1e060f65c3c575 Mon Sep 17 00:00:00 2001 From: Ismael Martinez Ramos Date: Fri, 5 Apr 2019 15:38:43 +0100 Subject: [PATCH 02/25] feat(cdk) removing duplicated code and adding missing semicolons --- packages/aws-cdk/lib/api/deployment-target.ts | 2 +- packages/aws-cdk/lib/cdk-toolkit.ts | 2 +- packages/aws-cdk/lib/settings.ts | 14 -------------- 3 files changed, 2 insertions(+), 16 deletions(-) diff --git a/packages/aws-cdk/lib/api/deployment-target.ts b/packages/aws-cdk/lib/api/deployment-target.ts index bf405a70e8981..59857213d934b 100644 --- a/packages/aws-cdk/lib/api/deployment-target.ts +++ b/packages/aws-cdk/lib/api/deployment-target.ts @@ -28,7 +28,7 @@ export interface DeployStackOptions { ci?: boolean; toolkitStackName?: string; reuseAssets?: string[]; - tags?: [ {Key: string, Value: string}] + tags?: [ {Key: string, Value: string}]; } export interface ProvisionerProps { diff --git a/packages/aws-cdk/lib/cdk-toolkit.ts b/packages/aws-cdk/lib/cdk-toolkit.ts index dbaaa7953e46c..b5c70f2fddb5a 100644 --- a/packages/aws-cdk/lib/cdk-toolkit.ts +++ b/packages/aws-cdk/lib/cdk-toolkit.ts @@ -235,5 +235,5 @@ export interface DeployOptions { /** * Tags to pass to CloudFormation for deployment */ - tags?: [ {Key: string, Value: string}] + tags?: [ {Key: string, Value: string}]; } \ No newline at end of file diff --git a/packages/aws-cdk/lib/settings.ts b/packages/aws-cdk/lib/settings.ts index 2402d3b93a8cb..6c66ee47d5330 100644 --- a/packages/aws-cdk/lib/settings.ts +++ b/packages/aws-cdk/lib/settings.ts @@ -187,20 +187,6 @@ export class Settings { const context = this.parseStringContextListToObject(argv); const tags = this.parseStringTagsListToObject(argv); - // Turn list of KEY=VALUE strings into an object - for (const assignment of ((argv as any).tags || [])) { - const tag = {Key: "", Value: ""}; - const parts = assignment.split('=', 2); - if (parts.length === 2) { - debug('CLI argument tags: %s=%s', parts[0], parts[1]); - tag.Key = parts[0]; - tag.Value = parts[1]; - tags.push(tag); - } else { - warning('Tags argument is not an assignment (key=value): %s', assignment); - } - } - return new Settings({ app: argv.app, browser: argv.browser, From 0ee589ae57d101da9ec98422bdf78585b285f20a Mon Sep 17 00:00:00 2001 From: Ismael Martinez Ramos Date: Mon, 8 Apr 2019 11:44:39 +0100 Subject: [PATCH 03/25] feat(cdk): #2185 Reading the stack metadata to find tags if no tags option is provided --- packages/aws-cdk/lib/api/cxapp/stacks.ts | 28 +++++++++++++++++++ packages/aws-cdk/lib/api/deploy-stack.ts | 3 +- packages/aws-cdk/lib/api/deployment-target.ts | 3 +- packages/aws-cdk/lib/cdk-toolkit.ts | 10 +++++-- packages/aws-cdk/lib/settings.ts | 8 ++---- 5 files changed, 42 insertions(+), 10 deletions(-) diff --git a/packages/aws-cdk/lib/api/cxapp/stacks.ts b/packages/aws-cdk/lib/api/cxapp/stacks.ts index 85cea47b6a9ab..ef75ab0c9bcbb 100644 --- a/packages/aws-cdk/lib/api/cxapp/stacks.ts +++ b/packages/aws-cdk/lib/api/cxapp/stacks.ts @@ -61,6 +61,7 @@ export interface AppStacksProps { * In a class because it shares some global state */ export class AppStacks { + /** * Since app execution basically always synthesizes all the stacks, * we can invoke it once and cache the response for subsequent calls. @@ -225,6 +226,23 @@ export class AppStacks { } } + public getTagsFromStackMetadata(stack: SelectedStack): Tags { + const tags = []; + for (const id of Object.keys(stack.metadata)) { + const metadata = stack.metadata[id]; + for (const entry of metadata) { + switch (entry.type) { + case "Tags": + for (const tag of entry.data) { + print(tag.key, tag.value); + tags.push(new Tag(tag.key, tag.value)); + } + } + } + } + return tags; + } + /** * Extracts 'aws:cdk:warning|info|error' metadata entries from the stack synthesis */ @@ -373,3 +391,13 @@ export interface SelectedStack extends cxapi.SynthesizedStack { */ originalName: string; } + +export type Tags = Tag[]; +export class Tag { + public readonly Key: string; + public Value: string; + constructor(key: string, value: string) { + this.Key = key; + this.Value = value; + } +} \ No newline at end of file diff --git a/packages/aws-cdk/lib/api/deploy-stack.ts b/packages/aws-cdk/lib/api/deploy-stack.ts index cb247c3b1c379..6e85d8ab79eee 100644 --- a/packages/aws-cdk/lib/api/deploy-stack.ts +++ b/packages/aws-cdk/lib/api/deploy-stack.ts @@ -2,6 +2,7 @@ import cxapi = require('@aws-cdk/cx-api'); import aws = require('aws-sdk'); import colors = require('colors/safe'); import uuid = require('uuid'); +import { Tags } from "../api/cxapp/stacks"; import { prepareAssets } from '../assets'; import { debug, error, print } from '../logging'; import { toYAML } from '../serialize'; @@ -32,7 +33,7 @@ export interface DeployStackOptions { quiet?: boolean; ci?: boolean; reuseAssets?: string[]; - tags?: [ {Key: string, Value: string}] + tags?: Tags; } const LARGE_TEMPLATE_SIZE_KB = 50; diff --git a/packages/aws-cdk/lib/api/deployment-target.ts b/packages/aws-cdk/lib/api/deployment-target.ts index 59857213d934b..548ccb4c3d747 100644 --- a/packages/aws-cdk/lib/api/deployment-target.ts +++ b/packages/aws-cdk/lib/api/deployment-target.ts @@ -1,4 +1,5 @@ import cxapi = require('@aws-cdk/cx-api'); +import { Tags } from "../api/cxapp/stacks"; import { debug } from '../logging'; import { deserializeStructure } from '../serialize'; import { Mode } from './aws-auth/credentials'; @@ -28,7 +29,7 @@ export interface DeployStackOptions { ci?: boolean; toolkitStackName?: string; reuseAssets?: string[]; - tags?: [ {Key: string, Value: string}]; + tags?: Tags; } export interface ProvisionerProps { diff --git a/packages/aws-cdk/lib/cdk-toolkit.ts b/packages/aws-cdk/lib/cdk-toolkit.ts index b5c70f2fddb5a..c4caabda5c71e 100644 --- a/packages/aws-cdk/lib/cdk-toolkit.ts +++ b/packages/aws-cdk/lib/cdk-toolkit.ts @@ -1,7 +1,7 @@ import colors = require('colors/safe'); import fs = require('fs-extra'); import { format, promisify } from 'util'; -import { AppStacks, ExtendedStackSelection } from "./api/cxapp/stacks"; +import { AppStacks, ExtendedStackSelection, Tags } from "./api/cxapp/stacks"; import { IDeploymentTarget } from './api/deployment-target'; import { printSecurityDiff, printStackDiff, RequireApproval } from './diff'; import { data, error, highlight, print, success } from './logging'; @@ -109,6 +109,10 @@ export class CdkToolkit { print('%s: deploying...', colors.bold(stack.name)); } + if (!options.tags || options.tags.length === 0) { + options.tags = this.appStacks.getTagsFromStackMetadata(stack); + } + try { const result = await this.provisioner.deployStack({ stack, @@ -235,5 +239,5 @@ export interface DeployOptions { /** * Tags to pass to CloudFormation for deployment */ - tags?: [ {Key: string, Value: string}]; -} \ No newline at end of file + tags?: Tags; +} diff --git a/packages/aws-cdk/lib/settings.ts b/packages/aws-cdk/lib/settings.ts index 6c66ee47d5330..2466899f5dcd6 100644 --- a/packages/aws-cdk/lib/settings.ts +++ b/packages/aws-cdk/lib/settings.ts @@ -2,6 +2,7 @@ import fs = require('fs-extra'); import os = require('os'); import fs_path = require('path'); import yargs = require('yargs'); +import { Tag, Tags } from './api/cxapp/stacks'; import { debug, warning } from './logging'; import util = require('./util'); @@ -229,16 +230,13 @@ export class Settings { } private static parseStringTagsListToObject(argv: yargs.Arguments) { - const tags: any = []; + const tags: Tags = []; for (const assignment of ((argv as any).tags || [])) { - const tag = {Key: "", Value: ""}; const parts = assignment.split('=', 2); if (parts.length === 2) { debug('CLI argument tags: %s=%s', parts[0], parts[1]); - tag.Key = parts[0]; - tag.Value = parts[1]; - tags.push(tag); + tags.push(new Tag(parts[0], parts[1])); } else { warning('Tags argument is not an assignment (key=value): %s', assignment); } From 6444ba4c30f76b7e16a22a9eb98f1a896cbc9f54 Mon Sep 17 00:00:00 2001 From: Ismael Martinez Ramos Date: Mon, 8 Apr 2019 11:55:13 +0100 Subject: [PATCH 04/25] Removing print from getTagsFromStackMetadata --- packages/aws-cdk/lib/api/cxapp/stacks.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/aws-cdk/lib/api/cxapp/stacks.ts b/packages/aws-cdk/lib/api/cxapp/stacks.ts index ef75ab0c9bcbb..80dc3e520b549 100644 --- a/packages/aws-cdk/lib/api/cxapp/stacks.ts +++ b/packages/aws-cdk/lib/api/cxapp/stacks.ts @@ -234,7 +234,6 @@ export class AppStacks { switch (entry.type) { case "Tags": for (const tag of entry.data) { - print(tag.key, tag.value); tags.push(new Tag(tag.key, tag.value)); } } @@ -400,4 +399,4 @@ export class Tag { this.Key = key; this.Value = value; } -} \ No newline at end of file +} From 14c08df043fc73bba1903bed60f5c3ccb820ef79 Mon Sep 17 00:00:00 2001 From: Ismael Martinez Ramos Date: Thu, 11 Apr 2019 14:58:45 +0100 Subject: [PATCH 05/25] feat(cdk): #2185 Changes from PR comments --- packages/@aws-cdk/cx-api/lib/cxapi.ts | 5 +++++ packages/aws-cdk/lib/api/cxapp/stacks.ts | 20 ++++++++------------ 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/packages/@aws-cdk/cx-api/lib/cxapi.ts b/packages/@aws-cdk/cx-api/lib/cxapi.ts index ae76ea0ea1eac..4a87e622aec9f 100644 --- a/packages/@aws-cdk/cx-api/lib/cxapi.ts +++ b/packages/@aws-cdk/cx-api/lib/cxapi.ts @@ -177,3 +177,8 @@ export const OUTFILE_NAME = 'cdk.out'; * Disable the collection and reporting of version information. */ export const DISABLE_VERSION_REPORTING = 'aws:cdk:disable-version-reporting'; + +/** + * Tag metadata key. + */ +export const TAG_METADATA_KEY = 'aws:cdk:path'; diff --git a/packages/aws-cdk/lib/api/cxapp/stacks.ts b/packages/aws-cdk/lib/api/cxapp/stacks.ts index 80dc3e520b549..35eb5df0c1719 100644 --- a/packages/aws-cdk/lib/api/cxapp/stacks.ts +++ b/packages/aws-cdk/lib/api/cxapp/stacks.ts @@ -231,11 +231,10 @@ export class AppStacks { for (const id of Object.keys(stack.metadata)) { const metadata = stack.metadata[id]; for (const entry of metadata) { - switch (entry.type) { - case "Tags": - for (const tag of entry.data) { - tags.push(new Tag(tag.key, tag.value)); - } + if (entry.type === cxapi.TAG_METADATA_KEY) { + for (const tag of entry.data) { + tags.push(tag); + } } } } @@ -392,11 +391,8 @@ export interface SelectedStack extends cxapi.SynthesizedStack { } export type Tags = Tag[]; -export class Tag { - public readonly Key: string; - public Value: string; - constructor(key: string, value: string) { - this.Key = key; - this.Value = value; - } + +interface Tag { + readonly key: string; + readonly value: string; } From 71883f2451e5f9470b45f769d08e46755128982c Mon Sep 17 00:00:00 2001 From: Ismael Martinez Ramos Date: Thu, 11 Apr 2019 15:51:05 +0100 Subject: [PATCH 06/25] Changes from PR comments. Fixing build --- packages/aws-cdk/lib/api/cxapp/stacks.ts | 10 ++++------ packages/aws-cdk/lib/api/deploy-stack.ts | 4 ++-- packages/aws-cdk/lib/api/deployment-target.ts | 4 ++-- packages/aws-cdk/lib/cdk-toolkit.ts | 4 ++-- packages/aws-cdk/lib/settings.ts | 9 ++++++--- 5 files changed, 16 insertions(+), 15 deletions(-) diff --git a/packages/aws-cdk/lib/api/cxapp/stacks.ts b/packages/aws-cdk/lib/api/cxapp/stacks.ts index 35eb5df0c1719..772b2eb9ced0d 100644 --- a/packages/aws-cdk/lib/api/cxapp/stacks.ts +++ b/packages/aws-cdk/lib/api/cxapp/stacks.ts @@ -226,7 +226,7 @@ export class AppStacks { } } - public getTagsFromStackMetadata(stack: SelectedStack): Tags { + public getTagsFromStackMetadata(stack: SelectedStack): Tag[] { const tags = []; for (const id of Object.keys(stack.metadata)) { const metadata = stack.metadata[id]; @@ -390,9 +390,7 @@ export interface SelectedStack extends cxapi.SynthesizedStack { originalName: string; } -export type Tags = Tag[]; - -interface Tag { - readonly key: string; - readonly value: string; +export interface Tag { + readonly Key: string; + readonly Value: string; } diff --git a/packages/aws-cdk/lib/api/deploy-stack.ts b/packages/aws-cdk/lib/api/deploy-stack.ts index 6e85d8ab79eee..90d079127dcb0 100644 --- a/packages/aws-cdk/lib/api/deploy-stack.ts +++ b/packages/aws-cdk/lib/api/deploy-stack.ts @@ -2,7 +2,7 @@ import cxapi = require('@aws-cdk/cx-api'); import aws = require('aws-sdk'); import colors = require('colors/safe'); import uuid = require('uuid'); -import { Tags } from "../api/cxapp/stacks"; +import { Tag } from "../api/cxapp/stacks"; import { prepareAssets } from '../assets'; import { debug, error, print } from '../logging'; import { toYAML } from '../serialize'; @@ -33,7 +33,7 @@ export interface DeployStackOptions { quiet?: boolean; ci?: boolean; reuseAssets?: string[]; - tags?: Tags; + tags?: Tag[]; } const LARGE_TEMPLATE_SIZE_KB = 50; diff --git a/packages/aws-cdk/lib/api/deployment-target.ts b/packages/aws-cdk/lib/api/deployment-target.ts index 548ccb4c3d747..8def2eca23920 100644 --- a/packages/aws-cdk/lib/api/deployment-target.ts +++ b/packages/aws-cdk/lib/api/deployment-target.ts @@ -1,5 +1,5 @@ import cxapi = require('@aws-cdk/cx-api'); -import { Tags } from "../api/cxapp/stacks"; +import { Tag } from "../api/cxapp/stacks"; import { debug } from '../logging'; import { deserializeStructure } from '../serialize'; import { Mode } from './aws-auth/credentials'; @@ -29,7 +29,7 @@ export interface DeployStackOptions { ci?: boolean; toolkitStackName?: string; reuseAssets?: string[]; - tags?: Tags; + tags?: Tag[]; } export interface ProvisionerProps { diff --git a/packages/aws-cdk/lib/cdk-toolkit.ts b/packages/aws-cdk/lib/cdk-toolkit.ts index c4caabda5c71e..a97112e18542d 100644 --- a/packages/aws-cdk/lib/cdk-toolkit.ts +++ b/packages/aws-cdk/lib/cdk-toolkit.ts @@ -1,7 +1,7 @@ import colors = require('colors/safe'); import fs = require('fs-extra'); import { format, promisify } from 'util'; -import { AppStacks, ExtendedStackSelection, Tags } from "./api/cxapp/stacks"; +import { AppStacks, ExtendedStackSelection, Tag } from "./api/cxapp/stacks"; import { IDeploymentTarget } from './api/deployment-target'; import { printSecurityDiff, printStackDiff, RequireApproval } from './diff'; import { data, error, highlight, print, success } from './logging'; @@ -239,5 +239,5 @@ export interface DeployOptions { /** * Tags to pass to CloudFormation for deployment */ - tags?: Tags; + tags?: Tag[]; } diff --git a/packages/aws-cdk/lib/settings.ts b/packages/aws-cdk/lib/settings.ts index 2466899f5dcd6..9e0a0f220813d 100644 --- a/packages/aws-cdk/lib/settings.ts +++ b/packages/aws-cdk/lib/settings.ts @@ -2,7 +2,7 @@ import fs = require('fs-extra'); import os = require('os'); import fs_path = require('path'); import yargs = require('yargs'); -import { Tag, Tags } from './api/cxapp/stacks'; +import { Tag } from './api/cxapp/stacks'; import { debug, warning } from './logging'; import util = require('./util'); @@ -230,13 +230,16 @@ export class Settings { } private static parseStringTagsListToObject(argv: yargs.Arguments) { - const tags: Tags = []; + const tags: Tag[] = []; for (const assignment of ((argv as any).tags || [])) { const parts = assignment.split('=', 2); if (parts.length === 2) { debug('CLI argument tags: %s=%s', parts[0], parts[1]); - tags.push(new Tag(parts[0], parts[1])); + tags.push({ + Key: parts[0], + Value: parts[1] + }); } else { warning('Tags argument is not an assignment (key=value): %s', assignment); } From 9defac82780435ebb185c81e5e51b9a87079f19f Mon Sep 17 00:00:00 2001 From: Ismael Martinez Ramos Date: Thu, 11 Apr 2019 20:23:31 +0100 Subject: [PATCH 07/25] Correcting copy/paste error and changing the name to TAGS_METADATA_KEY as it is more accurate --- packages/aws-cdk/lib/api/cxapp/stacks.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/aws-cdk/lib/api/cxapp/stacks.ts b/packages/aws-cdk/lib/api/cxapp/stacks.ts index 772b2eb9ced0d..26214020c466e 100644 --- a/packages/aws-cdk/lib/api/cxapp/stacks.ts +++ b/packages/aws-cdk/lib/api/cxapp/stacks.ts @@ -231,7 +231,7 @@ export class AppStacks { for (const id of Object.keys(stack.metadata)) { const metadata = stack.metadata[id]; for (const entry of metadata) { - if (entry.type === cxapi.TAG_METADATA_KEY) { + if (entry.type === cxapi.TAGS_METADATA_KEY) { for (const tag of entry.data) { tags.push(tag); } From e046be628353cee0e1d99a35f3cf2bad6ebaf7b5 Mon Sep 17 00:00:00 2001 From: Ismael Martinez Ramos Date: Thu, 11 Apr 2019 20:24:18 +0100 Subject: [PATCH 08/25] Correcting copy/paste error and changing the name to TAGS_METADATA_KEY as it is more accurate --- packages/@aws-cdk/cx-api/lib/cxapi.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@aws-cdk/cx-api/lib/cxapi.ts b/packages/@aws-cdk/cx-api/lib/cxapi.ts index 4a87e622aec9f..5589833fdbe3a 100644 --- a/packages/@aws-cdk/cx-api/lib/cxapi.ts +++ b/packages/@aws-cdk/cx-api/lib/cxapi.ts @@ -181,4 +181,4 @@ export const DISABLE_VERSION_REPORTING = 'aws:cdk:disable-version-reporting'; /** * Tag metadata key. */ -export const TAG_METADATA_KEY = 'aws:cdk:path'; +export const TAGS_METADATA_KEY = 'aws:cdk:tags'; From caaeff512defaac6c0969eef9a3f7d9a4af636f6 Mon Sep 17 00:00:00 2001 From: Ismael Martinez Ramos Date: Mon, 22 Apr 2019 15:04:03 +0100 Subject: [PATCH 09/25] WIP with tag aspects --- packages/@aws-cdk/cdk/lib/stack.ts | 15 +++++++++++++++ packages/@aws-cdk/cdk/lib/tag-aspect.ts | 18 ++++++++++++++---- 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/packages/@aws-cdk/cdk/lib/stack.ts b/packages/@aws-cdk/cdk/lib/stack.ts index ed1023ffaee36..f119370377dd4 100644 --- a/packages/@aws-cdk/cdk/lib/stack.ts +++ b/packages/@aws-cdk/cdk/lib/stack.ts @@ -5,6 +5,7 @@ import { Construct, IConstruct, PATH_SEP } from './construct'; import { Environment } from './environment'; import { HashedAddressingScheme, IAddressingScheme, LogicalIDs } from './logical-id'; import { ISynthesisSession } from './synthesis'; +import { TagManager } from './tag-manager'; import { makeUniqueId } from './uniqueid'; export interface StackProps { @@ -42,6 +43,13 @@ export interface StackProps { readonly autoDeploy?: boolean; } +export interface ITaggable { + /** + * TagManager to set, remove and format tags + */ + readonly tags: TagManager; +} + const STACK_SYMBOL = Symbol('@aws-cdk/cdk.CfnReference'); /** @@ -70,6 +78,13 @@ export class Stack extends Construct { return x[STACK_SYMBOL] === true; } + /** + * Check whether the given construct is Taggable + */ + public static isTaggable(construct: any): construct is ITaggable { + return (construct as any).tags !== undefined; + } + private static readonly VALID_STACK_NAME_REGEX = /^[A-Za-z][A-Za-z0-9-]*$/; /** diff --git a/packages/@aws-cdk/cdk/lib/tag-aspect.ts b/packages/@aws-cdk/cdk/lib/tag-aspect.ts index e6f373fffea92..959f8d50fe604 100644 --- a/packages/@aws-cdk/cdk/lib/tag-aspect.ts +++ b/packages/@aws-cdk/cdk/lib/tag-aspect.ts @@ -1,5 +1,6 @@ import { IAspect } from './aspect'; import { CfnResource, ITaggable } from './cfn-resource'; +import { Stack } from './stack'; import { IConstruct } from './construct'; /** @@ -71,12 +72,21 @@ export abstract class TagBase implements IAspect { } public visit(construct: IConstruct): void { - if (!CfnResource.isCfnResource(construct)) { + const isCfnResource = CfnResource.isCfnResource(construct); + const isStack = Stack.isStack(construct); + if (!isCfnResource && !isStack) { return; } - const resource = construct as CfnResource; - if (CfnResource.isTaggable(resource)) { - this.applyTag(resource); + if (isCfnResource) { + const resource = construct as CfnResource; + if (CfnResource.isTaggable(resource)) { + this.applyTag(resource); + } + } else { + const resource = construct as Stack; + if (Stack.isTaggable(resource)) { + this.applyTag(resource); + } } } From ca252b04b7dfd2fe85abca9d0530403d87cfabbf Mon Sep 17 00:00:00 2001 From: Ismael Martinez Ramos Date: Wed, 24 Apr 2019 15:13:28 +0100 Subject: [PATCH 10/25] WIP --- packages/@aws-cdk/cdk/lib/cfn-resource.ts | 8 +------ packages/@aws-cdk/cdk/lib/stack.ts | 26 ++++++++++++++++------- packages/@aws-cdk/cdk/lib/tag-aspect.ts | 5 +++-- packages/@aws-cdk/cdk/lib/tag-manager.ts | 7 ++++++ packages/aws-cdk/bin/cdk.ts | 2 +- packages/aws-cdk/lib/api/cxapp/stacks.ts | 2 ++ packages/aws-cdk/lib/cdk-toolkit.ts | 7 +++--- 7 files changed, 35 insertions(+), 22 deletions(-) diff --git a/packages/@aws-cdk/cdk/lib/cfn-resource.ts b/packages/@aws-cdk/cdk/lib/cfn-resource.ts index bcf6ba2f380e9..9f4157531c905 100644 --- a/packages/@aws-cdk/cdk/lib/cfn-resource.ts +++ b/packages/@aws-cdk/cdk/lib/cfn-resource.ts @@ -2,7 +2,7 @@ import cxapi = require('@aws-cdk/cx-api'); import { CfnCondition } from './cfn-condition'; import { Construct, IConstruct } from './construct'; import { CreationPolicy, DeletionPolicy, UpdatePolicy } from './resource-policy'; -import { TagManager } from './tag-manager'; +import { ITaggable } from './tag-manager'; import { capitalizePropertyNames, ignoreEmpty, PostResolveToken } from './util'; // import required to be here, otherwise causes a cycle when running the generated JavaScript // tslint:disable-next-line:ordered-imports @@ -21,12 +21,6 @@ export interface CfnResourceProps { readonly properties?: any; } -export interface ITaggable { - /** - * TagManager to set, remove and format tags - */ - readonly tags: TagManager; -} /** * Represents a CloudFormation resource. */ diff --git a/packages/@aws-cdk/cdk/lib/stack.ts b/packages/@aws-cdk/cdk/lib/stack.ts index dc5855432d9ff..21f278f82c529 100644 --- a/packages/@aws-cdk/cdk/lib/stack.ts +++ b/packages/@aws-cdk/cdk/lib/stack.ts @@ -5,7 +5,7 @@ import { Construct, IConstruct, PATH_SEP } from './construct'; import { Environment } from './environment'; import { HashedAddressingScheme, IAddressingScheme, LogicalIDs } from './logical-id'; import { ISynthesisSession } from './synthesis'; -import { TagManager } from './tag-manager'; +import { ITaggable } from './tag-manager'; import { makeUniqueId } from './uniqueid'; export interface StackProps { @@ -43,13 +43,6 @@ export interface StackProps { readonly autoDeploy?: boolean; } -export interface ITaggable { - /** - * TagManager to set, remove and format tags - */ - readonly tags: TagManager; -} - const STACK_SYMBOL = Symbol.for('@aws-cdk/cdk.Stack'); /** @@ -562,7 +555,10 @@ export class Stack extends Construct { visit(this); + this.node.addMetadata(cxapi.TAGS_METADATA_KEY, [new Tag("Team", "Isma"), new Tag("Group", "fdsajkl3")]); + const app = this.parentApp(); + if (app && app.node.metadata.length > 0) { output[PATH_SEP] = app.node.metadata; } @@ -575,6 +571,18 @@ export class Stack extends Construct { output[PATH_SEP + node.node.path] = node.node.metadata.map(md => node.node.resolve(md) as cxapi.MetadataEntry); } + const resource = node as Stack; + if (Stack.isTaggable(resource)) { + node.node.addMetadata(cxapi.TAGS_METADATA_KEY, [new Tag("Team", "Isma"), new Tag("Group", "fdsajkl3")]); + print(resource); + print(resource.tags); + print(resource.tags.renderTags()); + // const stackTags = resource.tags.renderTags() + // stackTags.map(tag => node.node.) + // node.node.metadata.map(md => node.node.resolve(md) as cxapi.MetadataEntry); + // this.applyStackTag(resource); + } + for (const child of node.node.children) { visit(child); } @@ -672,11 +680,13 @@ function cfnElements(node: IConstruct, into: CfnElement[] = []): CfnElement[] { } // These imports have to be at the end to prevent circular imports +import { print } from 'util'; import { ArnComponents, arnFromComponents, parseArn } from './arn'; import { CfnElement } from './cfn-element'; import { CfnReference } from './cfn-reference'; import { CfnResource } from './cfn-resource'; import { Aws, ScopedAws } from './pseudo'; +import { Tag } from './tag-aspect'; /** * Find all resources in a set of constructs diff --git a/packages/@aws-cdk/cdk/lib/tag-aspect.ts b/packages/@aws-cdk/cdk/lib/tag-aspect.ts index 959f8d50fe604..9781c279b5d14 100644 --- a/packages/@aws-cdk/cdk/lib/tag-aspect.ts +++ b/packages/@aws-cdk/cdk/lib/tag-aspect.ts @@ -1,7 +1,8 @@ import { IAspect } from './aspect'; -import { CfnResource, ITaggable } from './cfn-resource'; -import { Stack } from './stack'; +import { CfnResource} from './cfn-resource'; import { IConstruct } from './construct'; +import { Stack } from './stack'; +import { ITaggable } from './tag-manager'; /** * Properties for a tag diff --git a/packages/@aws-cdk/cdk/lib/tag-manager.ts b/packages/@aws-cdk/cdk/lib/tag-manager.ts index 9a7b24da4ddfc..20437d2242426 100644 --- a/packages/@aws-cdk/cdk/lib/tag-manager.ts +++ b/packages/@aws-cdk/cdk/lib/tag-manager.ts @@ -158,6 +158,13 @@ const TAG_FORMATTERS: {[key: string]: ITagFormatter} = { [TagType.NotTaggable]: new NoFormat(), }; +export interface ITaggable { + /** + * TagManager to set, remove and format tags + */ + readonly tags: TagManager; +} + /** * TagManager facilitates a common implementation of tagging for Constructs. */ diff --git a/packages/aws-cdk/bin/cdk.ts b/packages/aws-cdk/bin/cdk.ts index f7cbccde75ab2..7afe49be71add 100644 --- a/packages/aws-cdk/bin/cdk.ts +++ b/packages/aws-cdk/bin/cdk.ts @@ -62,7 +62,7 @@ async function parseCommandLineArguments() { .option('exclusively', { type: 'boolean', alias: 'e', desc: 'only deploy requested stacks, don\'t include dependencies' }) .option('require-approval', { type: 'string', choices: [RequireApproval.Never, RequireApproval.AnyChange, RequireApproval.Broadening], desc: 'what security-sensitive changes need manual approval' })) .option('ci', { type: 'boolean', desc: 'Force CI detection. Use --no-ci to disable CI autodetection.', default: process.env.CI !== undefined }) - .option('tags', { type: 'array', alias: 't', desc: 'tags to add to the stack', nargs: 1, requiresArg: 'KEY=VALUE'}) + .option('tags', { type: 'array', alias: 't', desc: 'tags to add to the stack (KEY=VALUE)', nargs: 1, requiresArg: true }) .command('destroy [STACKS..]', 'Destroy the stack(s) named STACKS', yargs => yargs .option('exclusively', { type: 'boolean', alias: 'x', desc: 'only deploy requested stacks, don\'t include dependees' }) .option('force', { type: 'boolean', alias: 'f', desc: 'Do not ask for confirmation before destroying the stacks' })) diff --git a/packages/aws-cdk/lib/api/cxapp/stacks.ts b/packages/aws-cdk/lib/api/cxapp/stacks.ts index 973a729b08ada..016fbe86cc396 100644 --- a/packages/aws-cdk/lib/api/cxapp/stacks.ts +++ b/packages/aws-cdk/lib/api/cxapp/stacks.ts @@ -232,9 +232,11 @@ export class AppStacks { const tags = []; for (const id of Object.keys(stack.metadata)) { const metadata = stack.metadata[id]; + print('%s: metadata...', metadata); for (const entry of metadata) { if (entry.type === cxapi.TAGS_METADATA_KEY) { for (const tag of entry.data) { + print('%s: metadata tags...', tag); tags.push(tag); } } diff --git a/packages/aws-cdk/lib/cdk-toolkit.ts b/packages/aws-cdk/lib/cdk-toolkit.ts index 22265d0e6731d..2c35f8c35467f 100644 --- a/packages/aws-cdk/lib/cdk-toolkit.ts +++ b/packages/aws-cdk/lib/cdk-toolkit.ts @@ -1,6 +1,6 @@ import colors = require('colors/safe'); import fs = require('fs-extra'); -import { format, promisify } from 'util'; +import { format } from 'util'; import { AppStacks, ExtendedStackSelection, Tag } from "./api/cxapp/stacks"; import { IDeploymentTarget } from './api/deployment-target'; import { printSecurityDiff, printStackDiff, RequireApproval } from './diff'; @@ -108,8 +108,10 @@ export class CdkToolkit { print('%s: deploying...', colors.bold(stack.name)); } + print('%s: with tags...', options.tags); if (!options.tags || options.tags.length === 0) { options.tags = this.appStacks.getTagsFromStackMetadata(stack); + print('%s: with tags...', options.tags); } try { @@ -234,12 +236,9 @@ export interface DeployOptions { * Reuse the assets with the given asset IDs */ reuseAssets?: string[]; -<<<<<<< HEAD /** * Tags to pass to CloudFormation for deployment */ tags?: Tag[]; -======= ->>>>>>> master } From 5e43a649596f9496cdc85c4bb0ad1a6e1519cf9b Mon Sep 17 00:00:00 2001 From: Ismael Martinez Ramos Date: Fri, 26 Apr 2019 16:57:21 +0100 Subject: [PATCH 11/25] Adding the tags via properties and the stack.node.apply. WIP --- packages/aws-cdk/lib/api/cxapp/stacks.ts | 17 ++++++++++------- packages/aws-cdk/lib/cdk-toolkit.ts | 2 -- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/packages/aws-cdk/lib/api/cxapp/stacks.ts b/packages/aws-cdk/lib/api/cxapp/stacks.ts index 016fbe86cc396..382f0f40a5e7a 100644 --- a/packages/aws-cdk/lib/api/cxapp/stacks.ts +++ b/packages/aws-cdk/lib/api/cxapp/stacks.ts @@ -229,20 +229,23 @@ export class AppStacks { } public getTagsFromStackMetadata(stack: SelectedStack): Tag[] { - const tags = []; + const metadataTags: Tag[] = []; for (const id of Object.keys(stack.metadata)) { const metadata = stack.metadata[id]; - print('%s: metadata...', metadata); for (const entry of metadata) { if (entry.type === cxapi.TAGS_METADATA_KEY) { - for (const tag of entry.data) { - print('%s: metadata tags...', tag); - tags.push(tag); + if (entry.data) { + entry.data.forEach((tags: { key: string; value: string }) => { + metadataTags.push({ + Key: tags.key, + Value: tags.value + }); + }); } } } } - return tags; + return metadataTags; } /** @@ -397,4 +400,4 @@ export interface SelectedStack extends cxapi.SynthesizedStack { export interface Tag { readonly Key: string; readonly Value: string; -} +} \ No newline at end of file diff --git a/packages/aws-cdk/lib/cdk-toolkit.ts b/packages/aws-cdk/lib/cdk-toolkit.ts index 2c35f8c35467f..e6780f14492bb 100644 --- a/packages/aws-cdk/lib/cdk-toolkit.ts +++ b/packages/aws-cdk/lib/cdk-toolkit.ts @@ -108,10 +108,8 @@ export class CdkToolkit { print('%s: deploying...', colors.bold(stack.name)); } - print('%s: with tags...', options.tags); if (!options.tags || options.tags.length === 0) { options.tags = this.appStacks.getTagsFromStackMetadata(stack); - print('%s: with tags...', options.tags); } try { From 53f0158ba8139773b4ff2c87a68c7a241fd880ec Mon Sep 17 00:00:00 2001 From: Ismael Martinez Ramos Date: Fri, 26 Apr 2019 16:57:59 +0100 Subject: [PATCH 12/25] Adding the tags via properties and the stack.node.apply. WIP --- packages/@aws-cdk/cdk/lib/stack.ts | 42 ++++++++++--------------- packages/@aws-cdk/cdk/lib/tag-aspect.ts | 16 +++------- 2 files changed, 21 insertions(+), 37 deletions(-) diff --git a/packages/@aws-cdk/cdk/lib/stack.ts b/packages/@aws-cdk/cdk/lib/stack.ts index 21f278f82c529..05d7d5090c666 100644 --- a/packages/@aws-cdk/cdk/lib/stack.ts +++ b/packages/@aws-cdk/cdk/lib/stack.ts @@ -5,9 +5,7 @@ import { Construct, IConstruct, PATH_SEP } from './construct'; import { Environment } from './environment'; import { HashedAddressingScheme, IAddressingScheme, LogicalIDs } from './logical-id'; import { ISynthesisSession } from './synthesis'; -import { ITaggable } from './tag-manager'; import { makeUniqueId } from './uniqueid'; - export interface StackProps { /** * The AWS environment (account/region) where this stack will be deployed. @@ -41,6 +39,8 @@ export interface StackProps { * @default true */ readonly autoDeploy?: boolean; + + readonly tags?: { [key: string]: string }; } const STACK_SYMBOL = Symbol.for('@aws-cdk/cdk.Stack'); @@ -49,6 +49,7 @@ const STACK_SYMBOL = Symbol.for('@aws-cdk/cdk.Stack'); * A root construct which represents a single CloudFormation stack. */ export class Stack extends Construct { + /** * Adds a metadata annotation "aws:cdk:physical-name" to the construct if physicalName * is non-null. This can be used later by tools and aspects to determine if resources @@ -71,15 +72,10 @@ export class Stack extends Construct { return obj[STACK_SYMBOL] === true; } - /** - * Check whether the given construct is Taggable - */ - public static isTaggable(construct: any): construct is ITaggable { - return (construct as any).tags !== undefined; - } - private static readonly VALID_STACK_NAME_REGEX = /^[A-Za-z][A-Za-z0-9-]*$/; + public readonly tags: TagManager = new TagManager(TagType.Standard, "AWS:Cloudformation::Stack"); + /** * Lists all missing contextual information. * This is returned when the stack is synthesized under the 'missing' attribute @@ -161,6 +157,13 @@ export class Stack extends Construct { this.logicalIds = new LogicalIDs(props && props.namingScheme ? props.namingScheme : new HashedAddressingScheme()); this.name = props.stackName !== undefined ? props.stackName : this.calculateStackName(); this.autoDeploy = props && props.autoDeploy === false ? false : true; + if (props.tags) { + for (const key in props.tags) { + if (props.tags.hasOwnProperty(key)) { + this.tags.setTag(key, props.tags[key]); + } + } + } } /** @@ -198,6 +201,7 @@ export class Stack extends Construct { public _toCloudFormation() { // before we begin synthesis, we shall lock this stack, so children cannot be added this.node.lock(); + this.node.addMetadata(cxapi.TAGS_METADATA_KEY, this.tags.renderTags()); try { const template: any = { @@ -555,8 +559,6 @@ export class Stack extends Construct { visit(this); - this.node.addMetadata(cxapi.TAGS_METADATA_KEY, [new Tag("Team", "Isma"), new Tag("Group", "fdsajkl3")]); - const app = this.parentApp(); if (app && app.node.metadata.length > 0) { @@ -566,23 +568,12 @@ export class Stack extends Construct { return output; function visit(node: IConstruct) { + if (node.node.metadata.length > 0) { // Make the path absolute output[PATH_SEP + node.node.path] = node.node.metadata.map(md => node.node.resolve(md) as cxapi.MetadataEntry); } - const resource = node as Stack; - if (Stack.isTaggable(resource)) { - node.node.addMetadata(cxapi.TAGS_METADATA_KEY, [new Tag("Team", "Isma"), new Tag("Group", "fdsajkl3")]); - print(resource); - print(resource.tags); - print(resource.tags.renderTags()); - // const stackTags = resource.tags.renderTags() - // stackTags.map(tag => node.node.) - // node.node.metadata.map(md => node.node.resolve(md) as cxapi.MetadataEntry); - // this.applyStackTag(resource); - } - for (const child of node.node.children) { visit(child); } @@ -680,13 +671,12 @@ function cfnElements(node: IConstruct, into: CfnElement[] = []): CfnElement[] { } // These imports have to be at the end to prevent circular imports -import { print } from 'util'; import { ArnComponents, arnFromComponents, parseArn } from './arn'; import { CfnElement } from './cfn-element'; import { CfnReference } from './cfn-reference'; -import { CfnResource } from './cfn-resource'; +import { CfnResource, TagType } from './cfn-resource'; import { Aws, ScopedAws } from './pseudo'; -import { Tag } from './tag-aspect'; +import { TagManager } from './tag-manager'; /** * Find all resources in a set of constructs diff --git a/packages/@aws-cdk/cdk/lib/tag-aspect.ts b/packages/@aws-cdk/cdk/lib/tag-aspect.ts index 9781c279b5d14..5d6aa0192575b 100644 --- a/packages/@aws-cdk/cdk/lib/tag-aspect.ts +++ b/packages/@aws-cdk/cdk/lib/tag-aspect.ts @@ -1,3 +1,4 @@ +// import cxapi = require('@aws-cdk/cx-api'); import { IAspect } from './aspect'; import { CfnResource} from './cfn-resource'; import { IConstruct } from './construct'; @@ -78,16 +79,9 @@ export abstract class TagBase implements IAspect { if (!isCfnResource && !isStack) { return; } - if (isCfnResource) { - const resource = construct as CfnResource; - if (CfnResource.isTaggable(resource)) { + const resource = isCfnResource ? construct as CfnResource : construct as Stack; + if (CfnResource.isTaggable(resource)) { this.applyTag(resource); - } - } else { - const resource = construct as Stack; - if (Stack.isTaggable(resource)) { - this.applyTag(resource); - } } } @@ -115,7 +109,7 @@ export class Tag extends TagBase { } protected applyTag(resource: ITaggable) { - if (resource.tags.applyTagAspectHere(this.props.includeResourceTypes, this.props.excludeResourceTypes)) { + if (resource.tags.applyTagAspectHere(this.props.includeResourceTypes, this.props.excludeResourceTypes) || Stack.isStack(resource)) { resource.tags.setTag( this.key, this.value, @@ -138,7 +132,7 @@ export class RemoveTag extends TagBase { } protected applyTag(resource: ITaggable): void { - if (resource.tags.applyTagAspectHere(this.props.includeResourceTypes, this.props.excludeResourceTypes)) { + if (resource.tags.applyTagAspectHere(this.props.includeResourceTypes, this.props.excludeResourceTypes) || Stack.isStack(resource)) { resource.tags.removeTag(this.key, this.props.priority !== undefined ? this.props.priority : this.defaultPriority); } } From 8eed2c836520bfe53e21f6bb5cff7e2f991f6847 Mon Sep 17 00:00:00 2001 From: Ismael Martinez Ramos Date: Thu, 2 May 2019 14:04:39 +0100 Subject: [PATCH 13/25] changes from comments --- packages/@aws-cdk/cdk/lib/cfn-resource.ts | 1 + packages/@aws-cdk/cdk/lib/stack.ts | 11 +++----- packages/@aws-cdk/cdk/lib/tag-manager.ts | 32 +++++++++++++++++++++++ packages/aws-cdk/lib/api/cxapp/stacks.ts | 11 ++------ 4 files changed, 38 insertions(+), 17 deletions(-) diff --git a/packages/@aws-cdk/cdk/lib/cfn-resource.ts b/packages/@aws-cdk/cdk/lib/cfn-resource.ts index 9f4157531c905..f97184d62877f 100644 --- a/packages/@aws-cdk/cdk/lib/cfn-resource.ts +++ b/packages/@aws-cdk/cdk/lib/cfn-resource.ts @@ -258,6 +258,7 @@ export enum TagType { Standard = 'StandardTag', AutoScalingGroup = 'AutoScalingGroupTag', Map = 'StringToStringMap', + KeyValue = 'KeyValue', NotTaggable = 'NotTaggable', } diff --git a/packages/@aws-cdk/cdk/lib/stack.ts b/packages/@aws-cdk/cdk/lib/stack.ts index 05d7d5090c666..5acb768e3531c 100644 --- a/packages/@aws-cdk/cdk/lib/stack.ts +++ b/packages/@aws-cdk/cdk/lib/stack.ts @@ -74,7 +74,7 @@ export class Stack extends Construct { private static readonly VALID_STACK_NAME_REGEX = /^[A-Za-z][A-Za-z0-9-]*$/; - public readonly tags: TagManager = new TagManager(TagType.Standard, "AWS:Cloudformation::Stack"); + public readonly tags: TagManager; /** * Lists all missing contextual information. @@ -157,13 +157,8 @@ export class Stack extends Construct { this.logicalIds = new LogicalIDs(props && props.namingScheme ? props.namingScheme : new HashedAddressingScheme()); this.name = props.stackName !== undefined ? props.stackName : this.calculateStackName(); this.autoDeploy = props && props.autoDeploy === false ? false : true; - if (props.tags) { - for (const key in props.tags) { - if (props.tags.hasOwnProperty(key)) { - this.tags.setTag(key, props.tags[key]); - } - } - } + const tags = props === undefined ? undefined : props.tags; + this.tags = new TagManager(TagType.KeyValue, "AWS:Cloudformation::Stack", tags); } /** diff --git a/packages/@aws-cdk/cdk/lib/tag-manager.ts b/packages/@aws-cdk/cdk/lib/tag-manager.ts index 20437d2242426..0129bbdba1378 100644 --- a/packages/@aws-cdk/cdk/lib/tag-manager.ts +++ b/packages/@aws-cdk/cdk/lib/tag-manager.ts @@ -18,6 +18,10 @@ interface CfnAsgTag { propagateAtLaunch: boolean; } +interface StackTag { + Key: string; + Value: string; +} /** * Interface for converter between CloudFormation and internal tag representations */ @@ -142,6 +146,33 @@ class MapFormatter implements ITagFormatter { } } +class KeyValueFormatter implements ITagFormatter { + public parseTags(keyValueTags: any, priority: number): Tag[] { + const tags: Tag[] = []; + for (const key in keyValueTags.tags) { + if (keyValueTags.tags.hasOwnProperty(key)) { + const value = keyValueTags.tags[key]; + tags.push({ + key, + value, + priority + }); + } + } + return tags; + } + public formatTags(unformattedTags: Tag[]): any { + const tags: StackTag[] = []; + unformattedTags.forEach(tag => { + tags.push({ + Key: tag.key, + Value: tag.value + }); + }); + return tags; + } +} + class NoFormat implements ITagFormatter { public parseTags(_cfnPropertyTags: any): Tag[] { return []; @@ -155,6 +186,7 @@ const TAG_FORMATTERS: {[key: string]: ITagFormatter} = { [TagType.AutoScalingGroup]: new AsgFormatter(), [TagType.Standard]: new StandardFormatter(), [TagType.Map]: new MapFormatter(), + [TagType.KeyValue]: new KeyValueFormatter(), [TagType.NotTaggable]: new NoFormat(), }; diff --git a/packages/aws-cdk/lib/api/cxapp/stacks.ts b/packages/aws-cdk/lib/api/cxapp/stacks.ts index 382f0f40a5e7a..271e9e4c181d0 100644 --- a/packages/aws-cdk/lib/api/cxapp/stacks.ts +++ b/packages/aws-cdk/lib/api/cxapp/stacks.ts @@ -229,19 +229,12 @@ export class AppStacks { } public getTagsFromStackMetadata(stack: SelectedStack): Tag[] { - const metadataTags: Tag[] = []; + let metadataTags: Tag[] = []; for (const id of Object.keys(stack.metadata)) { const metadata = stack.metadata[id]; for (const entry of metadata) { if (entry.type === cxapi.TAGS_METADATA_KEY) { - if (entry.data) { - entry.data.forEach((tags: { key: string; value: string }) => { - metadataTags.push({ - Key: tags.key, - Value: tags.value - }); - }); - } + metadataTags = entry.data; } } } From 90fc71f43c6cd840c62726043a242039f0fa39fb Mon Sep 17 00:00:00 2001 From: Ismael Martinez Ramos Date: Fri, 3 May 2019 14:12:45 +0100 Subject: [PATCH 14/25] Only adding the metadata when there are tags --- packages/@aws-cdk/cdk/lib/stack.ts | 4 +++- packages/@aws-cdk/cdk/lib/tag-manager.ts | 4 ++++ packages/aws-cdk/lib/api/cxapp/stacks.ts | 7 +++---- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/packages/@aws-cdk/cdk/lib/stack.ts b/packages/@aws-cdk/cdk/lib/stack.ts index 5acb768e3531c..fa0a69c34c62f 100644 --- a/packages/@aws-cdk/cdk/lib/stack.ts +++ b/packages/@aws-cdk/cdk/lib/stack.ts @@ -196,7 +196,9 @@ export class Stack extends Construct { public _toCloudFormation() { // before we begin synthesis, we shall lock this stack, so children cannot be added this.node.lock(); - this.node.addMetadata(cxapi.TAGS_METADATA_KEY, this.tags.renderTags()); + if (this.tags.hasTags()) { + this.node.addMetadata(cxapi.TAGS_METADATA_KEY, this.tags.renderTags()); + } try { const template: any = { diff --git a/packages/@aws-cdk/cdk/lib/tag-manager.ts b/packages/@aws-cdk/cdk/lib/tag-manager.ts index 0129bbdba1378..815170d9f47b0 100644 --- a/packages/@aws-cdk/cdk/lib/tag-manager.ts +++ b/packages/@aws-cdk/cdk/lib/tag-manager.ts @@ -256,6 +256,10 @@ export class TagManager { return true; } + public hasTags(): boolean { + return this.tags.size > 0; + } + private _setTag(...tags: Tag[]) { for (const tag of tags) { if (tag.priority >= (this.priorities.get(tag.key) || 0)) { diff --git a/packages/aws-cdk/lib/api/cxapp/stacks.ts b/packages/aws-cdk/lib/api/cxapp/stacks.ts index 271e9e4c181d0..da91f95785e9f 100644 --- a/packages/aws-cdk/lib/api/cxapp/stacks.ts +++ b/packages/aws-cdk/lib/api/cxapp/stacks.ts @@ -228,17 +228,16 @@ export class AppStacks { } } - public getTagsFromStackMetadata(stack: SelectedStack): Tag[] { - let metadataTags: Tag[] = []; + public getTagsFromStackMetadata(stack: SelectedStack): Tag[] | undefined { for (const id of Object.keys(stack.metadata)) { const metadata = stack.metadata[id]; for (const entry of metadata) { if (entry.type === cxapi.TAGS_METADATA_KEY) { - metadataTags = entry.data; + return entry.data; } } } - return metadataTags; + return; } /** From d0cc809c75481c5937b9d2c8927aa74fe52c928d Mon Sep 17 00:00:00 2001 From: Ismael Martinez Ramos Date: Fri, 3 May 2019 16:44:42 +0100 Subject: [PATCH 15/25] Adding some tests for the KeyValue and hasTags in tag-manager --- .../@aws-cdk/cdk/test/test.tag-manager.ts | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/packages/@aws-cdk/cdk/test/test.tag-manager.ts b/packages/@aws-cdk/cdk/test/test.tag-manager.ts index dd9b1cb86e51e..a921ca38f91fb 100644 --- a/packages/@aws-cdk/cdk/test/test.tag-manager.ts +++ b/packages/@aws-cdk/cdk/test/test.tag-manager.ts @@ -44,14 +44,21 @@ export = { test.deepEqual(mgr.renderTags(), undefined ); test.done(); }, + '#hasTags() returns false'(test: Test) { + const mgr = new TagManager(TagType.Standard, 'AWS::Resource::Type'); + test.equal(mgr.hasTags(), false ); + test.done(); + } }, - '#renderTags() handles standard, map, and ASG tag formats'(test: Test) { + '#renderTags() handles standard, map, keyValue, and ASG tag formats'(test: Test) { const tagged: TagManager[] = []; const standard = new TagManager(TagType.Standard, 'AWS::Resource::Type'); const asg = new TagManager(TagType.AutoScalingGroup, 'AWS::Resource::Type'); + const keyValue = new TagManager(TagType.KeyValue, 'AWS::Resource::Type'); const mapper = new TagManager(TagType.Map, 'AWS::Resource::Type'); tagged.push(standard); tagged.push(asg); + tagged.push(keyValue); tagged.push(mapper); for (const res of tagged) { res.setTag('foo', 'bar'); @@ -65,12 +72,23 @@ export = { {key: 'foo', value: 'bar', propagateAtLaunch: true}, {key: 'asg', value: 'only', propagateAtLaunch: false}, ]); + test.deepEqual(keyValue.renderTags(), [ + { Key: 'foo', Value : 'bar' }, + { Key: 'asg', Value : 'only' } + ]); test.deepEqual(mapper.renderTags(), { foo: 'bar', asg: 'only', }); test.done(); }, + 'when there are tags it hasTags returns true'(test: Test) { + const mgr = new TagManager(TagType.Standard, 'AWS::Resource::Type'); + mgr.setTag('key', 'myVal', 2); + mgr.setTag('key', 'newVal', 1); + test.equal(mgr.hasTags(), true); + test.done(); + }, 'tags with higher or equal priority always take precedence'(test: Test) { const mgr = new TagManager(TagType.Standard, 'AWS::Resource::Type'); mgr.setTag('key', 'myVal', 2); From bc1bb26eb9b17d9d797861efa2178e66cbd42a32 Mon Sep 17 00:00:00 2001 From: Ismael Martinez Ramos Date: Mon, 6 May 2019 12:24:11 +0100 Subject: [PATCH 16/25] Moving the isTaggable to the Contructor and tidying up after code review comments --- packages/@aws-cdk/cdk/lib/cfn-resource.ts | 8 -------- packages/@aws-cdk/cdk/lib/construct.ts | 10 +++++++++- packages/@aws-cdk/cdk/lib/tag-aspect.ts | 9 +++++---- packages/aws-cdk/lib/settings.ts | 4 ++-- 4 files changed, 16 insertions(+), 15 deletions(-) diff --git a/packages/@aws-cdk/cdk/lib/cfn-resource.ts b/packages/@aws-cdk/cdk/lib/cfn-resource.ts index f97184d62877f..75977fef00ebe 100644 --- a/packages/@aws-cdk/cdk/lib/cfn-resource.ts +++ b/packages/@aws-cdk/cdk/lib/cfn-resource.ts @@ -2,7 +2,6 @@ import cxapi = require('@aws-cdk/cx-api'); import { CfnCondition } from './cfn-condition'; import { Construct, IConstruct } from './construct'; import { CreationPolicy, DeletionPolicy, UpdatePolicy } from './resource-policy'; -import { ITaggable } from './tag-manager'; import { capitalizePropertyNames, ignoreEmpty, PostResolveToken } from './util'; // import required to be here, otherwise causes a cycle when running the generated JavaScript // tslint:disable-next-line:ordered-imports @@ -48,13 +47,6 @@ export class CfnResource extends CfnRefElement { return (construct as any).resourceType !== undefined; } - /** - * Check whether the given construct is Taggable - */ - public static isTaggable(construct: any): construct is ITaggable { - return (construct as any).tags !== undefined; - } - /** * Options for this resource, such as condition, update policy etc. */ diff --git a/packages/@aws-cdk/cdk/lib/construct.ts b/packages/@aws-cdk/cdk/lib/construct.ts index 4c98cabc21c44..e5d05e0fcfe0e 100644 --- a/packages/@aws-cdk/cdk/lib/construct.ts +++ b/packages/@aws-cdk/cdk/lib/construct.ts @@ -573,6 +573,13 @@ export class Construct implements IConstruct { return (x as any).prepare !== undefined && (x as any).validate !== undefined; } + /** + * Check whether the given construct is Taggable + */ + public static isTaggable(construct: any): construct is ITaggable { + return (construct as any).tags !== undefined; + } + /** * Construct node. */ @@ -726,4 +733,5 @@ export interface OutgoingReference { } // Import this _after_ everything else to help node work the classes out in the correct order... -import { Reference } from './reference'; \ No newline at end of file +import { Reference } from './reference'; +import { ITaggable } from './tag-manager'; diff --git a/packages/@aws-cdk/cdk/lib/tag-aspect.ts b/packages/@aws-cdk/cdk/lib/tag-aspect.ts index 5d6aa0192575b..eb71b4ff75e69 100644 --- a/packages/@aws-cdk/cdk/lib/tag-aspect.ts +++ b/packages/@aws-cdk/cdk/lib/tag-aspect.ts @@ -1,7 +1,7 @@ // import cxapi = require('@aws-cdk/cx-api'); import { IAspect } from './aspect'; import { CfnResource} from './cfn-resource'; -import { IConstruct } from './construct'; +import { Construct, IConstruct } from './construct'; import { Stack } from './stack'; import { ITaggable } from './tag-manager'; @@ -80,12 +80,13 @@ export abstract class TagBase implements IAspect { return; } const resource = isCfnResource ? construct as CfnResource : construct as Stack; - if (CfnResource.isTaggable(resource)) { + if (Construct.isTaggable(resource)) { this.applyTag(resource); } } protected abstract applyTag(resource: ITaggable): void; + } /** @@ -109,7 +110,7 @@ export class Tag extends TagBase { } protected applyTag(resource: ITaggable) { - if (resource.tags.applyTagAspectHere(this.props.includeResourceTypes, this.props.excludeResourceTypes) || Stack.isStack(resource)) { + if (resource.tags.applyTagAspectHere(this.props.includeResourceTypes, this.props.excludeResourceTypes)) { resource.tags.setTag( this.key, this.value, @@ -132,7 +133,7 @@ export class RemoveTag extends TagBase { } protected applyTag(resource: ITaggable): void { - if (resource.tags.applyTagAspectHere(this.props.includeResourceTypes, this.props.excludeResourceTypes) || Stack.isStack(resource)) { + if (resource.tags.applyTagAspectHere(this.props.includeResourceTypes, this.props.excludeResourceTypes)) { resource.tags.removeTag(this.key, this.props.priority !== undefined ? this.props.priority : this.defaultPriority); } } diff --git a/packages/aws-cdk/lib/settings.ts b/packages/aws-cdk/lib/settings.ts index 98385e8fa3662..47370d43238e4 100644 --- a/packages/aws-cdk/lib/settings.ts +++ b/packages/aws-cdk/lib/settings.ts @@ -212,7 +212,7 @@ export class Settings { return ret; } - private static parseStringContextListToObject(argv: yargs.Arguments) { + private static parseStringContextListToObject(argv: yargs.Arguments): any { const context: any = {}; for (const assignment of ((argv as any).context || [])) { @@ -230,7 +230,7 @@ export class Settings { return context; } - private static parseStringTagsListToObject(argv: yargs.Arguments) { + private static parseStringTagsListToObject(argv: yargs.Arguments): Tag[] { const tags: Tag[] = []; for (const assignment of ((argv as any).tags || [])) { From bbeebd227ad2a81016d5cb8c897efad2ad8dceb3 Mon Sep 17 00:00:00 2001 From: Ismael Martinez Ramos Date: Fri, 31 May 2019 10:44:21 +0100 Subject: [PATCH 17/25] Changes to address most of the code review comments --- packages/@aws-cdk/cdk/lib/construct.ts | 2 ++ packages/@aws-cdk/cdk/lib/stack.ts | 26 ++++++++++++------- packages/@aws-cdk/cdk/lib/tag-aspect.ts | 12 ++------- packages/@aws-cdk/cdk/lib/tag-manager.ts | 6 +++++ .../@aws-cdk/cdk/test/test.tag-manager.ts | 4 +-- packages/@aws-cdk/cx-api/lib/cxapi.ts | 2 +- packages/aws-cdk/lib/api/cxapp/stacks.ts | 6 ++--- packages/aws-cdk/lib/api/deployment-target.ts | 2 +- 8 files changed, 34 insertions(+), 26 deletions(-) diff --git a/packages/@aws-cdk/cdk/lib/construct.ts b/packages/@aws-cdk/cdk/lib/construct.ts index e2614ebf7daa5..6a987fe653297 100644 --- a/packages/@aws-cdk/cdk/lib/construct.ts +++ b/packages/@aws-cdk/cdk/lib/construct.ts @@ -713,3 +713,5 @@ export interface OutgoingReference { // Import this _after_ everything else to help node work the classes out in the correct order... import { ITaggable } from './tag-manager'; + +import { Reference } from './reference'; \ No newline at end of file diff --git a/packages/@aws-cdk/cdk/lib/stack.ts b/packages/@aws-cdk/cdk/lib/stack.ts index 454ae0b69bb4a..e33d175e3005a 100644 --- a/packages/@aws-cdk/cdk/lib/stack.ts +++ b/packages/@aws-cdk/cdk/lib/stack.ts @@ -41,6 +41,11 @@ export interface StackProps { */ readonly autoDeploy?: boolean; + /** + * Stack tags that will be applied to all the taggable resources and the stack itself. + * + * @default {} + */ readonly tags?: { [key: string]: string }; } @@ -49,7 +54,7 @@ const STACK_SYMBOL = Symbol.for('@aws-cdk/cdk.Stack'); /** * A root construct which represents a single CloudFormation stack. */ -export class Stack extends Construct { +export class Stack extends Construct implements ITaggable { /** * Adds a metadata annotation "aws:cdk:physical-name" to the construct if physicalName @@ -75,8 +80,6 @@ export class Stack extends Construct { private static readonly VALID_STACK_NAME_REGEX = /^[A-Za-z][A-Za-z0-9-]*$/; - public readonly tags: TagManager; - /** * Lists all missing contextual information. * This is returned when the stack is synthesized under the 'missing' attribute @@ -118,6 +121,11 @@ export class Stack extends Construct { */ public readonly autoDeploy: boolean; + /** + * Stack tags + */ + public readonly tags: TagManager; + /** * Other stacks this stack depends on */ @@ -154,8 +162,7 @@ export class Stack extends Construct { this.logicalIds = new LogicalIDs(props && props.namingScheme ? props.namingScheme : new HashedAddressingScheme()); this.name = props.stackName !== undefined ? props.stackName : this.calculateStackName(); this.autoDeploy = props && props.autoDeploy === false ? false : true; - const tags = props === undefined ? undefined : props.tags; - this.tags = new TagManager(TagType.KeyValue, "AWS:Cloudformation::Stack", tags); + this.tags = new TagManager(TagType.KeyValue, "aws:cdk:stack", props.tags); if (!Stack.VALID_STACK_NAME_REGEX.test(this.name)) { throw new Error(`Stack name must match the regular expression: ${Stack.VALID_STACK_NAME_REGEX.toString()}, got '${name}'`); @@ -197,9 +204,6 @@ export class Stack extends Construct { public _toCloudFormation() { // before we begin synthesis, we shall lock this stack, so children cannot be added this.node.lock(); - if (this.tags.hasTags()) { - this.node.addMetadata(cxapi.TAGS_METADATA_KEY, this.tags.renderTags()); - } try { const template: any = { @@ -499,6 +503,10 @@ export class Stack extends Construct { } } } + + if (this.tags.hasTags()) { + this.node.addMetadata(cxapi.TAGS_METADATA_KEY, this.tags.renderTags()); + } } protected synthesize(builder: cxapi.CloudAssemblyBuilder): void { @@ -677,7 +685,7 @@ import { CfnElement } from './cfn-element'; import { CfnReference } from './cfn-reference'; import { CfnResource, TagType } from './cfn-resource'; import { Aws, ScopedAws } from './pseudo'; -import { TagManager } from './tag-manager'; +import { ITaggable, TagManager } from './tag-manager'; /** * Find all resources in a set of constructs diff --git a/packages/@aws-cdk/cdk/lib/tag-aspect.ts b/packages/@aws-cdk/cdk/lib/tag-aspect.ts index eb71b4ff75e69..07ab84fd252e2 100644 --- a/packages/@aws-cdk/cdk/lib/tag-aspect.ts +++ b/packages/@aws-cdk/cdk/lib/tag-aspect.ts @@ -1,8 +1,6 @@ // import cxapi = require('@aws-cdk/cx-api'); import { IAspect } from './aspect'; -import { CfnResource} from './cfn-resource'; import { Construct, IConstruct } from './construct'; -import { Stack } from './stack'; import { ITaggable } from './tag-manager'; /** @@ -74,14 +72,8 @@ export abstract class TagBase implements IAspect { } public visit(construct: IConstruct): void { - const isCfnResource = CfnResource.isCfnResource(construct); - const isStack = Stack.isStack(construct); - if (!isCfnResource && !isStack) { - return; - } - const resource = isCfnResource ? construct as CfnResource : construct as Stack; - if (Construct.isTaggable(resource)) { - this.applyTag(resource); + if (Construct.isTaggable(construct)) { + this.applyTag(construct); } } diff --git a/packages/@aws-cdk/cdk/lib/tag-manager.ts b/packages/@aws-cdk/cdk/lib/tag-manager.ts index 815170d9f47b0..75efb429cbc83 100644 --- a/packages/@aws-cdk/cdk/lib/tag-manager.ts +++ b/packages/@aws-cdk/cdk/lib/tag-manager.ts @@ -146,6 +146,9 @@ class MapFormatter implements ITagFormatter { } } +/** + * StackTags are of the format { Key: key, Value: value } + */ class KeyValueFormatter implements ITagFormatter { public parseTags(keyValueTags: any, priority: number): Tag[] { const tags: Tag[] = []; @@ -190,6 +193,9 @@ const TAG_FORMATTERS: {[key: string]: ITagFormatter} = { [TagType.NotTaggable]: new NoFormat(), }; +/** + * Interface to implement tags. + */ export interface ITaggable { /** * TagManager to set, remove and format tags diff --git a/packages/@aws-cdk/cdk/test/test.tag-manager.ts b/packages/@aws-cdk/cdk/test/test.tag-manager.ts index a921ca38f91fb..05d4ee7c132f5 100644 --- a/packages/@aws-cdk/cdk/test/test.tag-manager.ts +++ b/packages/@aws-cdk/cdk/test/test.tag-manager.ts @@ -41,12 +41,12 @@ export = { 'when there are no tags': { '#renderTags() returns undefined'(test: Test) { const mgr = new TagManager(TagType.Standard, 'AWS::Resource::Type'); - test.deepEqual(mgr.renderTags(), undefined ); + test.deepEqual(mgr.renderTags(), undefined); test.done(); }, '#hasTags() returns false'(test: Test) { const mgr = new TagManager(TagType.Standard, 'AWS::Resource::Type'); - test.equal(mgr.hasTags(), false ); + test.equal(mgr.hasTags(), false); test.done(); } }, diff --git a/packages/@aws-cdk/cx-api/lib/cxapi.ts b/packages/@aws-cdk/cx-api/lib/cxapi.ts index 1e69943039839..989d8961ae1a1 100644 --- a/packages/@aws-cdk/cx-api/lib/cxapi.ts +++ b/packages/@aws-cdk/cx-api/lib/cxapi.ts @@ -33,7 +33,7 @@ export const DISABLE_ASSET_STAGING_CONTEXT = 'aws:cdk:disable-asset-staging'; /** * Tag metadata key. */ -export const TAGS_METADATA_KEY = 'aws:cdk:tags'; +export const TAGS_METADATA_KEY = 'aws:cdk:stack-tags'; /** * If this context key is set, the CDK will stage assets under the specified diff --git a/packages/aws-cdk/lib/api/cxapp/stacks.ts b/packages/aws-cdk/lib/api/cxapp/stacks.ts index 3eafc4574a971..0f21dced61db2 100644 --- a/packages/aws-cdk/lib/api/cxapp/stacks.ts +++ b/packages/aws-cdk/lib/api/cxapp/stacks.ts @@ -234,7 +234,7 @@ export class AppStacks { } } - public getTagsFromStackMetadata(stack: SelectedStack): Tag[] | undefined { + public getTagsFromStackMetadata(stack: cxapi.CloudFormationStackArtifact): Tag[] { for (const id of Object.keys(stack.metadata)) { const metadata = stack.metadata[id]; for (const entry of metadata) { @@ -243,7 +243,7 @@ export class AppStacks { } } } - return; + return []; } /** @@ -382,7 +382,7 @@ function includeUpstreamStacks( } } -export interface SelectedStack extends cxapi.SynthesizedStack { +export interface SelectedStack extends cxapi.CloudFormationStackArtifact { /** * The original name of the stack before renaming */ diff --git a/packages/aws-cdk/lib/api/deployment-target.ts b/packages/aws-cdk/lib/api/deployment-target.ts index 64b7bc5cd788e..ebccbe8174d19 100644 --- a/packages/aws-cdk/lib/api/deployment-target.ts +++ b/packages/aws-cdk/lib/api/deployment-target.ts @@ -1,5 +1,5 @@ -import { Tag } from "../api/cxapp/stacks"; import { CloudFormationStackArtifact } from '@aws-cdk/cx-api'; +import { Tag } from "../api/cxapp/stacks"; import { debug } from '../logging'; import { deserializeStructure } from '../serialize'; import { Mode } from './aws-auth/credentials'; From 196b2ca8a6e1b56193a7b40155997eb9825f569d Mon Sep 17 00:00:00 2001 From: Ismael Martinez Ramos Date: Fri, 31 May 2019 13:27:26 +0100 Subject: [PATCH 18/25] Reverting the move of the check for if the resource/stack is taggable to the contructor as otherwise we start to get quite a lot of code duplication --- packages/@aws-cdk/cdk/lib/cfn-resource.ts | 2 +- packages/@aws-cdk/cdk/lib/stack.ts | 12 ++++++------ packages/@aws-cdk/cdk/lib/tag-aspect.ts | 1 - packages/@aws-cdk/cdk/lib/tag-manager.ts | 3 +++ packages/aws-cdk/lib/api/cxapp/stacks.ts | 7 ------- 5 files changed, 10 insertions(+), 15 deletions(-) diff --git a/packages/@aws-cdk/cdk/lib/cfn-resource.ts b/packages/@aws-cdk/cdk/lib/cfn-resource.ts index d11fe309c228d..6b59ca5d30f77 100644 --- a/packages/@aws-cdk/cdk/lib/cfn-resource.ts +++ b/packages/@aws-cdk/cdk/lib/cfn-resource.ts @@ -198,7 +198,7 @@ export class CfnResource extends CfnRefElement { public _toCloudFormation(): object { try { // merge property overrides onto properties and then render (and validate). - const tags = CfnResource.isTaggable(this) ? this.tags.renderTags() : undefined; + const tags = Construct.isTaggable(this) ? this.tags.renderTags() : undefined; const properties = deepMerge( this.properties || {}, { tags }, diff --git a/packages/@aws-cdk/cdk/lib/stack.ts b/packages/@aws-cdk/cdk/lib/stack.ts index e33d175e3005a..5451f43cc4388 100644 --- a/packages/@aws-cdk/cdk/lib/stack.ts +++ b/packages/@aws-cdk/cdk/lib/stack.ts @@ -80,6 +80,11 @@ export class Stack extends Construct implements ITaggable { private static readonly VALID_STACK_NAME_REGEX = /^[A-Za-z][A-Za-z0-9-]*$/; + /** + * Tags to be applied to the stack. + */ + public readonly tags: TagManager; + /** * Lists all missing contextual information. * This is returned when the stack is synthesized under the 'missing' attribute @@ -121,11 +126,6 @@ export class Stack extends Construct implements ITaggable { */ public readonly autoDeploy: boolean; - /** - * Stack tags - */ - public readonly tags: TagManager; - /** * Other stacks this stack depends on */ @@ -685,7 +685,7 @@ import { CfnElement } from './cfn-element'; import { CfnReference } from './cfn-reference'; import { CfnResource, TagType } from './cfn-resource'; import { Aws, ScopedAws } from './pseudo'; -import { ITaggable, TagManager } from './tag-manager'; +import { TagManager, ITaggable } from './tag-manager'; /** * Find all resources in a set of constructs diff --git a/packages/@aws-cdk/cdk/lib/tag-aspect.ts b/packages/@aws-cdk/cdk/lib/tag-aspect.ts index 07ab84fd252e2..051dcfaacbe75 100644 --- a/packages/@aws-cdk/cdk/lib/tag-aspect.ts +++ b/packages/@aws-cdk/cdk/lib/tag-aspect.ts @@ -78,7 +78,6 @@ export abstract class TagBase implements IAspect { } protected abstract applyTag(resource: ITaggable): void; - } /** diff --git a/packages/@aws-cdk/cdk/lib/tag-manager.ts b/packages/@aws-cdk/cdk/lib/tag-manager.ts index 75efb429cbc83..b0e0b2ce58f70 100644 --- a/packages/@aws-cdk/cdk/lib/tag-manager.ts +++ b/packages/@aws-cdk/cdk/lib/tag-manager.ts @@ -262,6 +262,9 @@ export class TagManager { return true; } + /** + * Returns true if there are any tags defined + */ public hasTags(): boolean { return this.tags.size > 0; } diff --git a/packages/aws-cdk/lib/api/cxapp/stacks.ts b/packages/aws-cdk/lib/api/cxapp/stacks.ts index 0f21dced61db2..c27fd77e55022 100644 --- a/packages/aws-cdk/lib/api/cxapp/stacks.ts +++ b/packages/aws-cdk/lib/api/cxapp/stacks.ts @@ -382,13 +382,6 @@ function includeUpstreamStacks( } } -export interface SelectedStack extends cxapi.CloudFormationStackArtifact { - /** - * The original name of the stack before renaming - */ - originalName: string; -} - export interface Tag { readonly Key: string; readonly Value: string; From 08eb2b3cd26f1fd2e08c785687a2a90961ba5524 Mon Sep 17 00:00:00 2001 From: Ismael Martinez Ramos Date: Fri, 31 May 2019 13:43:18 +0100 Subject: [PATCH 19/25] Fixing the build (alphabeticall order of imports) --- packages/@aws-cdk/cdk/lib/stack.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@aws-cdk/cdk/lib/stack.ts b/packages/@aws-cdk/cdk/lib/stack.ts index 5451f43cc4388..c94a515dff5af 100644 --- a/packages/@aws-cdk/cdk/lib/stack.ts +++ b/packages/@aws-cdk/cdk/lib/stack.ts @@ -685,7 +685,7 @@ import { CfnElement } from './cfn-element'; import { CfnReference } from './cfn-reference'; import { CfnResource, TagType } from './cfn-resource'; import { Aws, ScopedAws } from './pseudo'; -import { TagManager, ITaggable } from './tag-manager'; +import { ITaggable, TagManager } from './tag-manager'; /** * Find all resources in a set of constructs From f0a219ee12f31e63dc4d72f061357f1b443a9893 Mon Sep 17 00:00:00 2001 From: Ismael Martinez Ramos Date: Mon, 3 Jun 2019 08:57:20 +0100 Subject: [PATCH 20/25] Moving isTaggable from the constructor to the TagManager --- packages/@aws-cdk/cdk/lib/cfn-resource.ts | 3 ++- packages/@aws-cdk/cdk/lib/construct.ts | 9 --------- packages/@aws-cdk/cdk/lib/tag-aspect.ts | 4 ++-- packages/@aws-cdk/cdk/lib/tag-manager.ts | 8 ++++++++ 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/packages/@aws-cdk/cdk/lib/cfn-resource.ts b/packages/@aws-cdk/cdk/lib/cfn-resource.ts index 6b59ca5d30f77..337a6034e3430 100644 --- a/packages/@aws-cdk/cdk/lib/cfn-resource.ts +++ b/packages/@aws-cdk/cdk/lib/cfn-resource.ts @@ -7,6 +7,7 @@ import { capitalizePropertyNames, ignoreEmpty, PostResolveToken } from './util'; // tslint:disable-next-line:ordered-imports import { CfnRefElement } from './cfn-element'; import { CfnReference } from './cfn-reference'; +import { TagManager } from './tag-manager'; export interface CfnResourceProps { /** @@ -198,7 +199,7 @@ export class CfnResource extends CfnRefElement { public _toCloudFormation(): object { try { // merge property overrides onto properties and then render (and validate). - const tags = Construct.isTaggable(this) ? this.tags.renderTags() : undefined; + const tags = TagManager.isTaggable(this) ? this.tags.renderTags() : undefined; const properties = deepMerge( this.properties || {}, { tags }, diff --git a/packages/@aws-cdk/cdk/lib/construct.ts b/packages/@aws-cdk/cdk/lib/construct.ts index 6a987fe653297..eb6ff7ca3ef06 100644 --- a/packages/@aws-cdk/cdk/lib/construct.ts +++ b/packages/@aws-cdk/cdk/lib/construct.ts @@ -589,13 +589,6 @@ export class Construct implements IConstruct { return (x as any).prepare !== undefined && (x as any).validate !== undefined; } - /** - * Check whether the given construct is Taggable - */ - public static isTaggable(construct: any): construct is ITaggable { - return (construct as any).tags !== undefined; - } - /** * Construct node. */ @@ -712,6 +705,4 @@ export interface OutgoingReference { // Import this _after_ everything else to help node work the classes out in the correct order... -import { ITaggable } from './tag-manager'; - import { Reference } from './reference'; \ No newline at end of file diff --git a/packages/@aws-cdk/cdk/lib/tag-aspect.ts b/packages/@aws-cdk/cdk/lib/tag-aspect.ts index 051dcfaacbe75..56c770993344c 100644 --- a/packages/@aws-cdk/cdk/lib/tag-aspect.ts +++ b/packages/@aws-cdk/cdk/lib/tag-aspect.ts @@ -1,7 +1,7 @@ // import cxapi = require('@aws-cdk/cx-api'); import { IAspect } from './aspect'; import { Construct, IConstruct } from './construct'; -import { ITaggable } from './tag-manager'; +import { ITaggable, TagManager } from './tag-manager'; /** * Properties for a tag @@ -72,7 +72,7 @@ export abstract class TagBase implements IAspect { } public visit(construct: IConstruct): void { - if (Construct.isTaggable(construct)) { + if (TagManager.isTaggable(construct)) { this.applyTag(construct); } } diff --git a/packages/@aws-cdk/cdk/lib/tag-manager.ts b/packages/@aws-cdk/cdk/lib/tag-manager.ts index b0e0b2ce58f70..81a2c234fcdd2 100644 --- a/packages/@aws-cdk/cdk/lib/tag-manager.ts +++ b/packages/@aws-cdk/cdk/lib/tag-manager.ts @@ -207,6 +207,14 @@ export interface ITaggable { * TagManager facilitates a common implementation of tagging for Constructs. */ export class TagManager { + + /** + * Check whether the given construct is Taggable + */ + public static isTaggable(construct: any): construct is ITaggable { + return (construct as any).tags !== undefined; + } + private readonly tags = new Map(); private readonly priorities = new Map(); private readonly tagFormatter: ITagFormatter; From 0e3c1014543420e8e46fcfa08e10d512dc4a1461 Mon Sep 17 00:00:00 2001 From: Ismael Martinez Ramos Date: Mon, 3 Jun 2019 09:16:53 +0100 Subject: [PATCH 21/25] renaming TAGS_METADATA_KEY to STACK_TAGS_METADATA_KEY and moving it to metadata.ts --- packages/@aws-cdk/cdk/lib/stack.ts | 2 +- packages/@aws-cdk/cx-api/lib/cxapi.ts | 5 ----- packages/@aws-cdk/cx-api/lib/metadata.ts | 5 +++++ 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/@aws-cdk/cdk/lib/stack.ts b/packages/@aws-cdk/cdk/lib/stack.ts index c94a515dff5af..0a3d5c8d200c6 100644 --- a/packages/@aws-cdk/cdk/lib/stack.ts +++ b/packages/@aws-cdk/cdk/lib/stack.ts @@ -505,7 +505,7 @@ export class Stack extends Construct implements ITaggable { } if (this.tags.hasTags()) { - this.node.addMetadata(cxapi.TAGS_METADATA_KEY, this.tags.renderTags()); + this.node.addMetadata(metadata.TAGS_METADATA_KEY, this.tags.renderTags()); } } diff --git a/packages/@aws-cdk/cx-api/lib/cxapi.ts b/packages/@aws-cdk/cx-api/lib/cxapi.ts index 989d8961ae1a1..e918c087d9d17 100644 --- a/packages/@aws-cdk/cx-api/lib/cxapi.ts +++ b/packages/@aws-cdk/cx-api/lib/cxapi.ts @@ -30,11 +30,6 @@ export const DISABLE_VERSION_REPORTING = 'aws:cdk:disable-version-reporting'; */ export const DISABLE_ASSET_STAGING_CONTEXT = 'aws:cdk:disable-asset-staging'; -/** - * Tag metadata key. - */ -export const TAGS_METADATA_KEY = 'aws:cdk:stack-tags'; - /** * If this context key is set, the CDK will stage assets under the specified * directory. Otherwise, assets will not be staged. diff --git a/packages/@aws-cdk/cx-api/lib/metadata.ts b/packages/@aws-cdk/cx-api/lib/metadata.ts index 25e0f730105a4..49957c3d1403e 100644 --- a/packages/@aws-cdk/cx-api/lib/metadata.ts +++ b/packages/@aws-cdk/cx-api/lib/metadata.ts @@ -19,6 +19,11 @@ export const ERROR_METADATA_KEY = 'aws:cdk:error'; */ export const PATH_METADATA_KEY = 'aws:cdk:path'; +/** + * Tag metadata key. + */ +export const STACK_TAGS_METADATA_KEY = 'aws:cdk:stack-tags'; + export enum SynthesisMessageLevel { INFO = 'info', WARNING = 'warning', From 4d97f7cf6ae9e339894a1483d46d3dbc55303039 Mon Sep 17 00:00:00 2001 From: Ismael Martinez Ramos Date: Mon, 3 Jun 2019 09:28:39 +0100 Subject: [PATCH 22/25] Missing fixes from code review changes --- packages/@aws-cdk/cdk/lib/stack.ts | 2 +- packages/@aws-cdk/cdk/lib/tag-aspect.ts | 2 +- packages/aws-cdk/lib/api/cxapp/stacks.ts | 9 ++++++++- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/packages/@aws-cdk/cdk/lib/stack.ts b/packages/@aws-cdk/cdk/lib/stack.ts index 0a3d5c8d200c6..186e58e39d64d 100644 --- a/packages/@aws-cdk/cdk/lib/stack.ts +++ b/packages/@aws-cdk/cdk/lib/stack.ts @@ -505,7 +505,7 @@ export class Stack extends Construct implements ITaggable { } if (this.tags.hasTags()) { - this.node.addMetadata(metadata.TAGS_METADATA_KEY, this.tags.renderTags()); + this.node.addMetadata(metadata.STACK_TAGS_METADATA_KEY, this.tags.renderTags()); } } diff --git a/packages/@aws-cdk/cdk/lib/tag-aspect.ts b/packages/@aws-cdk/cdk/lib/tag-aspect.ts index 56c770993344c..d2f50ca7cd1f5 100644 --- a/packages/@aws-cdk/cdk/lib/tag-aspect.ts +++ b/packages/@aws-cdk/cdk/lib/tag-aspect.ts @@ -1,6 +1,6 @@ // import cxapi = require('@aws-cdk/cx-api'); import { IAspect } from './aspect'; -import { Construct, IConstruct } from './construct'; +import { IConstruct } from './construct'; import { ITaggable, TagManager } from './tag-manager'; /** diff --git a/packages/aws-cdk/lib/api/cxapp/stacks.ts b/packages/aws-cdk/lib/api/cxapp/stacks.ts index c27fd77e55022..4fc7dc3d68a65 100644 --- a/packages/aws-cdk/lib/api/cxapp/stacks.ts +++ b/packages/aws-cdk/lib/api/cxapp/stacks.ts @@ -238,7 +238,7 @@ export class AppStacks { for (const id of Object.keys(stack.metadata)) { const metadata = stack.metadata[id]; for (const entry of metadata) { - if (entry.type === cxapi.TAGS_METADATA_KEY) { + if (entry.type === cxapi.STACK_TAGS_METADATA_KEY) { return entry.data; } } @@ -382,6 +382,13 @@ function includeUpstreamStacks( } } +export interface SelectedStack extends cxapi.CloudFormationStackArtifact { + /** + * The original name of the stack before renaming + */ + originalName: string; +} + export interface Tag { readonly Key: string; readonly Value: string; From 26d1870b93a2022c6c9e011afc4a005ac1413361 Mon Sep 17 00:00:00 2001 From: Ismael Martinez Ramos Date: Mon, 3 Jun 2019 09:43:54 +0100 Subject: [PATCH 23/25] Fixing build --- packages/@aws-cdk/cdk/lib/stack.ts | 2 +- packages/@aws-cdk/cdk/lib/tag-manager.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/@aws-cdk/cdk/lib/stack.ts b/packages/@aws-cdk/cdk/lib/stack.ts index 186e58e39d64d..41d3626c57fe0 100644 --- a/packages/@aws-cdk/cdk/lib/stack.ts +++ b/packages/@aws-cdk/cdk/lib/stack.ts @@ -505,7 +505,7 @@ export class Stack extends Construct implements ITaggable { } if (this.tags.hasTags()) { - this.node.addMetadata(metadata.STACK_TAGS_METADATA_KEY, this.tags.renderTags()); + this.node.addMetadata(cxapi.STACK_TAGS_METADATA_KEY, this.tags.renderTags()); } } diff --git a/packages/@aws-cdk/cdk/lib/tag-manager.ts b/packages/@aws-cdk/cdk/lib/tag-manager.ts index 81a2c234fcdd2..4ceccb800bba3 100644 --- a/packages/@aws-cdk/cdk/lib/tag-manager.ts +++ b/packages/@aws-cdk/cdk/lib/tag-manager.ts @@ -214,7 +214,7 @@ export class TagManager { public static isTaggable(construct: any): construct is ITaggable { return (construct as any).tags !== undefined; } - + private readonly tags = new Map(); private readonly priorities = new Map(); private readonly tagFormatter: ITagFormatter; From d04c9c199e6eccb41f2cc925b4140cfc63ad0517 Mon Sep 17 00:00:00 2001 From: Ismael Martinez Ramos Date: Mon, 3 Jun 2019 11:59:38 +0100 Subject: [PATCH 24/25] Fixing an issue with the tags not been populated if pass via the constructor arguments as we now don't pass the full StackProps object --- packages/@aws-cdk/cdk/lib/tag-manager.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/@aws-cdk/cdk/lib/tag-manager.ts b/packages/@aws-cdk/cdk/lib/tag-manager.ts index 4ceccb800bba3..ed6a2e6504136 100644 --- a/packages/@aws-cdk/cdk/lib/tag-manager.ts +++ b/packages/@aws-cdk/cdk/lib/tag-manager.ts @@ -152,9 +152,9 @@ class MapFormatter implements ITagFormatter { class KeyValueFormatter implements ITagFormatter { public parseTags(keyValueTags: any, priority: number): Tag[] { const tags: Tag[] = []; - for (const key in keyValueTags.tags) { - if (keyValueTags.tags.hasOwnProperty(key)) { - const value = keyValueTags.tags[key]; + for (const key in keyValueTags) { + if (keyValueTags.hasOwnProperty(key)) { + const value = keyValueTags[key]; tags.push({ key, value, From bf518e3bc76319db06fdf07145df724ef988bd9a Mon Sep 17 00:00:00 2001 From: Ismael Martinez Ramos Date: Mon, 3 Jun 2019 12:19:34 +0100 Subject: [PATCH 25/25] Adding docs --- packages/aws-cdk/lib/api/cxapp/stacks.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/aws-cdk/lib/api/cxapp/stacks.ts b/packages/aws-cdk/lib/api/cxapp/stacks.ts index 4fc7dc3d68a65..f0691bc85edc9 100644 --- a/packages/aws-cdk/lib/api/cxapp/stacks.ts +++ b/packages/aws-cdk/lib/api/cxapp/stacks.ts @@ -234,6 +234,9 @@ export class AppStacks { } } + /** + * Returns and array with the tags available in the stack metadata. + */ public getTagsFromStackMetadata(stack: cxapi.CloudFormationStackArtifact): Tag[] { for (const id of Object.keys(stack.metadata)) { const metadata = stack.metadata[id];