From 5c9b06796119cf975c89626e06f4cd9f9575d171 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Sun, 20 Jan 2019 17:49:21 +0100 Subject: [PATCH 01/11] feat(core): allow multiple Apps Change the meaning of `App` to represent the application the user is defining (as opposed to the internals of the CDK program). The names of Apps (if given) will be incorporated into the Stack name, so that it becomes possible to instantiate your Stacks multiple times just like other constructs (without having to make the `id` parameter unique). A new stack property `stackName` exists to force the stack to a different name. If a single `App` is instantiated without parameters, the observable behavior of your CDK app won't change. If you insantiate more than one `App`, all of them must receive ids. To represent the CDK internals, a new class called `Program` exists, which the user doesn't need to interact with. `app.run()` is no longer necessary, and is deprecated. It still exists to avoid breaking existing programs, but will be removed in a future release. --- packages/@aws-cdk/cdk/lib/app.ts | 228 ++++++++---------- .../@aws-cdk/cdk/lib/cloudformation/stack.ts | 94 ++++---- packages/@aws-cdk/cdk/lib/core/construct.ts | 2 +- packages/@aws-cdk/cdk/lib/index.ts | 1 + packages/@aws-cdk/cdk/lib/program.ts | 179 ++++++++++++++ packages/@aws-cdk/cdk/lib/util/collections.ts | 11 + packages/@aws-cdk/cdk/lib/util/json.ts | 26 ++ .../cdk/test/cloudformation/test.stack.ts | 35 +++ packages/@aws-cdk/cdk/test/test.app.ts | 42 ++-- packages/@aws-cdk/cdk/test/test.context.ts | 12 +- tools/cdk-build-tools/lib/os.ts | 6 +- 11 files changed, 434 insertions(+), 202 deletions(-) create mode 100644 packages/@aws-cdk/cdk/lib/program.ts create mode 100644 packages/@aws-cdk/cdk/lib/util/collections.ts create mode 100644 packages/@aws-cdk/cdk/lib/util/json.ts diff --git a/packages/@aws-cdk/cdk/lib/app.ts b/packages/@aws-cdk/cdk/lib/app.ts index dc38817aafba8..ef3d5fe3852fe 100644 --- a/packages/@aws-cdk/cdk/lib/app.ts +++ b/packages/@aws-cdk/cdk/lib/app.ts @@ -1,20 +1,83 @@ import cxapi = require('@aws-cdk/cx-api'); -import fs = require('fs'); -import path = require('path'); import { Stack } from './cloudformation/stack'; -import { IConstruct, MetadataEntry, PATH_SEP, Root } from './core/construct'; +import { Construct } from './core/construct'; +import { IConstruct, MetadataEntry, PATH_SEP } from './core/construct'; +import { Environment } from './environment'; +import { Program } from './program'; /** - * Represents a CDK program. + * Properties for an App */ -export class App extends Root { +export interface AppProps { + /** + * The AWS environment (account/region) where stacks in this app will be deployed. + * + * If not supplied, the `default-account` and `default-region` context parameters will be + * used. If they are undefined, it will not be possible to deploy the stack. + * + * @default Automatically determined + */ + env?: Environment; + + /** + * The CDK Program instance in which this app will be defined. + * + * You don't have to pass this, it exists for testing purposes. Only + * supply this parameter if you know what you are doing. + * + * @default Automatically created + */ + program?: Program; +} + +/** + * Used as construct name if no ID is supplied for the App + */ +const DEFAULT_APP_NAME = 'App'; + +/** + * An instance of your App + */ +export class App extends Construct { + /** + * True if the given construct is a default-named App + */ + public static isDefaultApp(construct: IConstruct) { + return App.isApp(construct) && construct.defaultAppName; + } + + /** + * True if the given construct is an App object + */ + public static isApp(construct: IConstruct): construct is App { + return (construct as any).defaultAppName !== undefined; + } + + /** + * The environment in which this stack is deployed. + */ + public readonly env: Environment; + + /** + * Whether the App id was supplied + */ + private defaultAppName: boolean; + /** * Initializes a CDK application. + * * @param request Optional toolkit request (e.g. for tests) */ - constructor() { - super(); - this.loadContext(); + constructor(id?: string, props: AppProps = {}) { + // For tests, we use a fresh Program every time + const program = props.program || (process.env.CDK_TEST_MODE === '1' + ? new Program() + : Program.defaultInstance()); + + super(program, id || DEFAULT_APP_NAME); + + this.env = props.env || {}; + this.defaultAppName = id === undefined; } private get stacks() { @@ -30,25 +93,10 @@ export class App extends Root { } /** - * Runs the program. Output is written to output directory as specified in the request. + * @deprecated */ - public run(): void { - const outdir = process.env[cxapi.OUTDIR_ENV]; - if (!outdir) { - process.stderr.write(`ERROR: The environment variable "${cxapi.OUTDIR_ENV}" is not defined\n`); - process.stderr.write('AWS CDK Toolkit (>= 0.11.0) is required in order to interact with this program.\n'); - process.exit(1); - return; - } - - const result: cxapi.SynthesizeResponse = { - version: cxapi.PROTO_RESPONSE_VERSION, - stacks: this.synthesizeStacks(Object.keys(this.stacks)), - runtime: this.collectRuntimeInformation() - }; - - const outfile = path.join(outdir, cxapi.OUTFILE_NAME); - fs.writeFileSync(outfile, JSON.stringify(result, undefined, 2)); + public run() { + this.node.addWarning(`It's not necessary to call app.run() anymore`); } /** @@ -58,15 +106,6 @@ export class App extends Root { public synthesizeStack(stackName: string): cxapi.SynthesizedStack { const stack = this.getStack(stackName); - this.node.prepareTree(); - - // first, validate this stack and stop if there are errors. - const errors = stack.node.validateTree(); - if (errors.length > 0) { - const errorList = errors.map(e => `[${e.source.node.path}] ${e.message}`).join('\n '); - throw new Error(`Stack validation failed with the following errors:\n ${errorList}`); - } - const account = stack.env.account || 'unknown-account'; const region = stack.env.region || 'unknown-region'; @@ -78,12 +117,12 @@ export class App extends Root { const missing = Object.keys(stack.missingContext).length ? stack.missingContext : undefined; return { - name: stack.node.id, + name: stack.name, environment, missing, template: stack.toCloudFormation(), metadata: this.collectMetadata(stack), - dependsOn: noEmptyArray(stack.dependencies().map(s => s.node.id)), + dependsOn: noEmptyArray(stack.dependencies().map(s => s.name)), }; } @@ -106,16 +145,16 @@ export class App extends Root { visit(stack); - // add app-level metadata under "." + // add app-level metadata as well if (this.node.metadata.length > 0) { - output[PATH_SEP] = this.node.metadata; + output[PATH_SEP + this.node.path] = this.node.metadata; } return output; function visit(node: IConstruct) { if (node.node.metadata.length > 0) { - // Make the path absolute + // Make the path absolute. output[PATH_SEP + node.node.path] = node.node.metadata.map(md => node.node.resolve(md) as MetadataEntry); } @@ -125,27 +164,20 @@ export class App extends Root { } } - private collectRuntimeInformation(): cxapi.AppRuntime { - const libraries: { [name: string]: string } = {}; - - for (const fileName of Object.keys(require.cache)) { - const pkg = findNpmPackage(fileName); - if (pkg && !pkg.private) { - libraries[pkg.name] = pkg.version; - } + protected validate(): string[] { + if (numberOfAppsInProgram(this) > 1 && this.node.id === DEFAULT_APP_NAME) { + return ['When constructing more than one App, all of them must have ids']; } + return []; + } - // include only libraries that are in the @aws-cdk npm scope - for (const name of Object.keys(libraries)) { - if (!name.startsWith('@aws-cdk/')) { - delete libraries[name]; - } - } - - // add jsii runtime version - libraries['jsii-runtime'] = getJsiiAgentVersion(); - - return { libraries }; + /** + * Synthesize the App + */ + protected synthesize() { + return { + stacks: this.synthesizeStacks(Object.keys(this.stacks)), + }; } private getStack(stackname: string) { @@ -159,77 +191,25 @@ export class App extends Root { } return stack; } +} - private loadContext() { - const contextJson = process.env[cxapi.CONTEXT_ENV]; - const context = !contextJson ? { } : JSON.parse(contextJson); - for (const key of Object.keys(context)) { - this.node.setContext(key, context[key]); - } - } +function noEmptyArray(xs: T[]): T[] | undefined { + return xs.length > 0 ? xs : undefined; } /** - * Determines which NPM module a given loaded javascript file is from. - * - * The only infromation that is available locally is a list of Javascript files, - * and every source file is associated with a search path to resolve the further - * ``require`` calls made from there, which includes its own directory on disk, - * and parent directories - for example: - * - * [ '...repo/packages/aws-cdk-resources/lib/cfn/node_modules', - * '...repo/packages/aws-cdk-resources/lib/node_modules', - * '...repo/packages/aws-cdk-resources/node_modules', - * '...repo/packages/node_modules', - * // etc... - * ] - * - * We are looking for ``package.json`` that is anywhere in the tree, except it's - * in the parent directory, not in the ``node_modules`` directory. For this - * reason, we strip the ``/node_modules`` suffix off each path and use regular - * module resolution to obtain a reference to ``package.json``. - * - * @param fileName a javascript file name. - * @returns the NPM module infos (aka ``package.json`` contents), or - * ``undefined`` if the lookup was unsuccessful. + * Cound the Apps in the construct tree */ -function findNpmPackage(fileName: string): { name: string, version: string, private?: boolean } | undefined { - const mod = require.cache[fileName]; - const paths = mod.paths.map(stripNodeModules); - - try { - const packagePath = require.resolve('package.json', { paths }); - return require(packagePath); - } catch (e) { - return undefined; - } - - /** - * @param s a path. - * @returns ``s`` with any terminating ``/node_modules`` - * (or ``\\node_modules``) stripped off.) - */ - function stripNodeModules(s: string): string { - if (s.endsWith('/node_modules') || s.endsWith('\\node_modules')) { - // /node_modules is 13 characters - return s.substr(0, s.length - 13); - } - return s; - } +function numberOfAppsInProgram(app: IConstruct): number { + return findRoot(app).node.findAll().filter(App.isApp).length; } -function getJsiiAgentVersion() { - let jsiiAgent = process.env.JSII_AGENT; - - // if JSII_AGENT is not specified, we will assume this is a node.js runtime - // and plug in our node.js version - if (!jsiiAgent) { - jsiiAgent = `node.js/${process.version}`; +/** + * Return the root of the construct tree + */ +function findRoot(current: IConstruct): IConstruct { + while (current.node.scope !== undefined) { + current = current.node.scope; } - - return jsiiAgent; -} - -function noEmptyArray(xs: T[]): T[] | undefined { - return xs.length > 0 ? xs : undefined; + return current; } \ No newline at end of file diff --git a/packages/@aws-cdk/cdk/lib/cloudformation/stack.ts b/packages/@aws-cdk/cdk/lib/cloudformation/stack.ts index b93d32b7e52a9..87c970ecbf44f 100644 --- a/packages/@aws-cdk/cdk/lib/cloudformation/stack.ts +++ b/packages/@aws-cdk/cdk/lib/cloudformation/stack.ts @@ -2,6 +2,8 @@ import cxapi = require('@aws-cdk/cx-api'); import { App } from '../app'; import { Construct, IConstruct } from '../core/construct'; import { Environment } from '../environment'; +import { firstDefined } from '../util/collections'; +import { merge } from '../util/json'; import { CfnReference } from './cfn-tokens'; import { HashedAddressingScheme, IAddressingScheme, LogicalIDs } from './logical-id'; import { Resource } from './resource'; @@ -15,6 +17,13 @@ export interface StackProps { */ env?: Environment; + /** + * Name to deploy the stack with + * + * @default Derived from construct IDs + */ + stackName?: string; + /** * Strategy for logical ID generation * @@ -75,11 +84,6 @@ export class Stack extends Construct { */ public readonly missingContext: { [key: string]: cxapi.MissingContext } = { }; - /** - * The environment in which this stack is deployed. - */ - public readonly env: Environment; - /** * Logical ID generation strategy */ @@ -105,6 +109,13 @@ export class Stack extends Construct { */ private readonly stackDependencies = new Set(); + /** + * Environment as configured via props + * + * (Both on Stack and inherited from App) + */ + private readonly configuredEnv: Environment; + /** * Creates a new stack. * @@ -112,7 +123,7 @@ export class Stack extends Construct { * @param name The name of the CloudFormation stack. Defaults to "Stack". * @param props Stack properties. */ - public constructor(scope?: App, name?: string, private readonly props?: StackProps) { + public constructor(scope?: App, name?: string, props: StackProps = {}) { // For unit test convenience parents are optional, so bypass the type check when calling the parent. super(scope!, name!); @@ -120,10 +131,13 @@ export class Stack extends Construct { throw new Error(`Stack name must match the regular expression: ${Stack.VALID_STACK_NAME_REGEX.toString()}, got '${name}'`); } - this.env = this.parseEnvironment(props); + this.configuredEnv = { + account: firstDefined(props.env && props.env.account, scope && scope.env.account), + region: firstDefined(props.env && props.env.region, scope && scope.env.region), + }; this.logicalIds = new LogicalIDs(props && props.namingScheme ? props.namingScheme : new HashedAddressingScheme()); - this.name = this.node.id; + this.name = props.stackName !== undefined ? props.stackName : this.calculateStackName(); } /** @@ -179,6 +193,16 @@ export class Stack extends Construct { } } + /** + * The environment in which this stack is deployed. + */ + public get env(): Environment { + return { + account: firstDefined(this.configuredEnv.account, this.node.getContext(cxapi.DEFAULT_ACCOUNT_CONTEXT_KEY)), + region: firstDefined(this.configuredEnv.region, this.node.getContext(cxapi.DEFAULT_REGION_CONTEXT_KEY)), + }; + } + /** * @param why more information about why region is required. * @returns The region in which this stack is deployed. Throws if region is not defined. @@ -264,8 +288,8 @@ export class Stack extends Construct { * to the correct account at deployment time. */ public get accountId(): string { - if (this.props && this.props.env && this.props.env.account) { - return this.props.env.account; + if (this.configuredEnv.account !== undefined) { + return this.configuredEnv.account; } return new Aws(this).accountId; } @@ -278,8 +302,8 @@ export class Stack extends Construct { * to the correct region at deployment time. */ public get region(): string { - if (this.props && this.props.env && this.props.env.region) { - return this.props.env.region; + if (this.configuredEnv.region !== undefined) { + return this.configuredEnv.region; } return new Aws(this).region; } @@ -410,26 +434,6 @@ export class Stack extends Construct { } } - /** - * Applied defaults to environment attributes. - */ - private parseEnvironment(props?: StackProps) { - // start with `env`. - const env: Environment = (props && props.env) || { }; - - // if account is not specified, attempt to read from context. - if (!env.account) { - env.account = this.node.getContext(cxapi.DEFAULT_ACCOUNT_CONTEXT_KEY); - } - - // if region is not specified, attempt to read from context. - if (!env.region) { - env.region = this.node.getContext(cxapi.DEFAULT_REGION_CONTEXT_KEY); - } - - return env; - } - /** * Check whether this stack has a (transitive) dependency on another stack */ @@ -440,25 +444,15 @@ export class Stack extends Construct { } return false; } -} -function merge(template: any, part: any) { - for (const section of Object.keys(part)) { - const src = part[section]; - - // create top-level section if it doesn't exist - let dest = template[section]; - if (!dest) { - template[section] = dest = src; - } else { - // add all entities from source section to destination section - for (const id of Object.keys(src)) { - if (id in dest) { - throw new Error(`section '${section}' already contains '${id}'`); - } - dest[id] = src[id]; - } - } + /** + * Calculcate the stack name based on the construct path + */ + private calculateStackName() { + return this.node.rootPath() + .filter(c => !App.isDefaultApp(c)) + .map(c => c.node.id) + .join('-'); } } diff --git a/packages/@aws-cdk/cdk/lib/core/construct.ts b/packages/@aws-cdk/cdk/lib/core/construct.ts index e6d2c76e67ba9..3c14428254389 100644 --- a/packages/@aws-cdk/cdk/lib/core/construct.ts +++ b/packages/@aws-cdk/cdk/lib/core/construct.ts @@ -450,7 +450,7 @@ export class ConstructNode { /** * Return the path of components up to but excluding the root */ - private rootPath(): IConstruct[] { + public rootPath(): IConstruct[] { const ancestors = this.ancestors(); ancestors.shift(); return ancestors; diff --git a/packages/@aws-cdk/cdk/lib/index.ts b/packages/@aws-cdk/cdk/lib/index.ts index 72e4e00592b2d..547348b3806b9 100644 --- a/packages/@aws-cdk/cdk/lib/index.ts +++ b/packages/@aws-cdk/cdk/lib/index.ts @@ -24,6 +24,7 @@ export * from './cloudformation/arn'; export * from './cloudformation/secret'; export * from './app'; +export * from './program'; export * from './context'; export * from './environment'; diff --git a/packages/@aws-cdk/cdk/lib/program.ts b/packages/@aws-cdk/cdk/lib/program.ts new file mode 100644 index 0000000000000..7afabb7c6aaef --- /dev/null +++ b/packages/@aws-cdk/cdk/lib/program.ts @@ -0,0 +1,179 @@ +import cxapi = require('@aws-cdk/cx-api'); +import fs = require('fs'); +import path = require('path'); +import { IConstruct, Root } from './core/construct'; +import { merge } from './util/json'; + +/** + * Represents a CDK program. + */ +export class Program extends Root { + /** + * Return the default singleton Program instance + */ + public static defaultInstance(): Program { + if (Program.DefaultInstance === undefined) { + Program.DefaultInstance = new Program(); + Program.DefaultInstance.initializeDefaultProgram(); + } + return Program.DefaultInstance; + } + + private static DefaultInstance?: Program; + + constructor() { + super(); + + this.loadContext(); + } + + /** + * Runs the program. Output is written to output directory as specified in the request. + */ + public run(): void { + const outdir = process.env[cxapi.OUTDIR_ENV]; + if (!outdir) { + process.stderr.write(`ERROR: The environment variable "${cxapi.OUTDIR_ENV}" is not defined\n`); + process.stderr.write('AWS CDK Toolkit (>= 0.11.0) is required in order to interact with this program.\n'); + process.exit(1); + return; + } + + const result = this.synthesizeAll(); + + const outfile = path.join(outdir, cxapi.OUTFILE_NAME); + fs.writeFileSync(outfile, JSON.stringify(result, undefined, 2)); + } + + /** + * Calculate the CDK app response + */ + public synthesizeAll(): cxapi.SynthesizeResponse { + this.node.prepareTree(); + + // first, validate this stack and stop if there are errors. + const errors = this.node.validateTree(); + if (errors.length > 0) { + const errorList = errors.map(e => `[${e.source.node.path}] ${e.message}`).join('\n '); + throw new Error(`Stack validation failed with the following errors:\n ${errorList}`); + } + + const result: any = { + version: cxapi.PROTO_RESPONSE_VERSION, + runtime: this.collectRuntimeInformation() + }; + + for (const synthesizable of this.node.findAll().filter(isSynthesizable)) { + merge(result, synthesizable.synthesize(), true); + } + + return result; + } + + /** + * Initialize the default Program instance + */ + private initializeDefaultProgram() { + process.once('beforeExit', () => this.run()); + } + + private collectRuntimeInformation(): cxapi.AppRuntime { + const libraries: { [name: string]: string } = {}; + + for (const fileName of Object.keys(require.cache)) { + const pkg = findNpmPackage(fileName); + if (pkg && !pkg.private) { + libraries[pkg.name] = pkg.version; + } + } + + // include only libraries that are in the @aws-cdk npm scope + for (const name of Object.keys(libraries)) { + if (!name.startsWith('@aws-cdk/')) { + delete libraries[name]; + } + } + + // add jsii runtime version + libraries['jsii-runtime'] = getJsiiAgentVersion(); + + return { libraries }; + } + + private loadContext() { + const contextJson = process.env[cxapi.CONTEXT_ENV]; + const context = !contextJson ? { } : JSON.parse(contextJson); + for (const key of Object.keys(context)) { + this.node.setContext(key, context[key]); + } + } +} + +function getJsiiAgentVersion() { + let jsiiAgent = process.env.JSII_AGENT; + + // if JSII_AGENT is not specified, we will assume this is a node.js runtime + // and plug in our node.js version + if (!jsiiAgent) { + jsiiAgent = `node.js/${process.version}`; + } + + return jsiiAgent; +} + +/** + * Determines which NPM module a given loaded javascript file is from. + * + * The only infromation that is available locally is a list of Javascript files, + * and every source file is associated with a search path to resolve the further + * ``require`` calls made from there, which includes its own directory on disk, + * and parent directories - for example: + * + * [ '...repo/packages/aws-cdk-resources/lib/cfn/node_modules', + * '...repo/packages/aws-cdk-resources/lib/node_modules', + * '...repo/packages/aws-cdk-resources/node_modules', + * '...repo/packages/node_modules', + * // etc... + * ] + * + * We are looking for ``package.json`` that is anywhere in the tree, except it's + * in the parent directory, not in the ``node_modules`` directory. For this + * reason, we strip the ``/node_modules`` suffix off each path and use regular + * module resolution to obtain a reference to ``package.json``. + * + * @param fileName a javascript file name. + * @returns the NPM module infos (aka ``package.json`` contents), or + * ``undefined`` if the lookup was unsuccessful. + */ +function findNpmPackage(fileName: string): { name: string, version: string, private?: boolean } | undefined { + const mod = require.cache[fileName]; + const paths = mod.paths.map(stripNodeModules); + + try { + const packagePath = require.resolve('package.json', { paths }); + return require(packagePath); + } catch (e) { + return undefined; + } + + /** + * @param s a path. + * @returns ``s`` with any terminating ``/node_modules`` + * (or ``\\node_modules``) stripped off.) + */ + function stripNodeModules(s: string): string { + if (s.endsWith('/node_modules') || s.endsWith('\\node_modules')) { + // /node_modules is 13 characters + return s.substr(0, s.length - 13); + } + return s; + } +} + +interface ISynthesizable extends IConstruct { + synthesize(): any; +} + +function isSynthesizable(x: IConstruct): x is ISynthesizable { + return (x as any).synthesize !== undefined; +} \ No newline at end of file diff --git a/packages/@aws-cdk/cdk/lib/util/collections.ts b/packages/@aws-cdk/cdk/lib/util/collections.ts new file mode 100644 index 0000000000000..78811498b5319 --- /dev/null +++ b/packages/@aws-cdk/cdk/lib/util/collections.ts @@ -0,0 +1,11 @@ +/** + * Return the fist value that is not undefined + */ +export function firstDefined(...xs: Array): T | undefined { + for (const x of xs) { + if (x !== undefined) { + return x; + } + } + return undefined; +} \ No newline at end of file diff --git a/packages/@aws-cdk/cdk/lib/util/json.ts b/packages/@aws-cdk/cdk/lib/util/json.ts new file mode 100644 index 0000000000000..65f3a3f907223 --- /dev/null +++ b/packages/@aws-cdk/cdk/lib/util/json.ts @@ -0,0 +1,26 @@ +export function merge(template: any, part: any, mergeLists = false) { + for (const section of Object.keys(part)) { + const src = part[section]; + + // create top-level section if it doesn't exist + let dest = template[section]; + if (!dest) { + template[section] = dest = src; + } else { + // add all entities from source section to destination section + for (const id of Object.keys(src)) { + if (Array.isArray(dest[id]) && mergeLists) { + if (!Array.isArray(src[id])) { + throw new Error(`Destination '${id}' in '${section}' is a list, but not in source: '${src}'`); + } + dest[id].push(...src[id]); + } else { + if (id in dest) { + throw new Error(`section '${section}' already contains '${id}'`); + } + dest[id] = src[id]; + } + } + } + } +} \ No newline at end of file diff --git a/packages/@aws-cdk/cdk/test/cloudformation/test.stack.ts b/packages/@aws-cdk/cdk/test/cloudformation/test.stack.ts index 880068e92d589..05609c4fbeeec 100644 --- a/packages/@aws-cdk/cdk/test/cloudformation/test.stack.ts +++ b/packages/@aws-cdk/cdk/test/cloudformation/test.stack.ts @@ -345,6 +345,41 @@ export = { test.done(); }, + + 'Stack inherits environment from App'(test: Test) { + // WHEN + const app = new App(undefined, { env: { account: '12345' }}); + const stack = new Stack(app, 'Stack', { env: { region: 'us-north-west' }}); + + // THEN + test.deepEqual(stack.env, { + account: '12345', + region: 'us-north-west' + }); + + test.done(); + }, + + 'Stack name can be overridden via properties'(test: Test) { + // WHEN + const stack = new Stack(undefined, 'Stack', { stackName: 'otherName' }); + + // THEN + test.deepEqual(stack.name, 'otherName'); + + test.done(); + }, + + 'Stack name is inherited from App name if available'(test: Test) { + // WHEN + const app = new App('Prod'); + const stack = new Stack(app, 'Stack'); + + // THEN + test.deepEqual(stack.name, 'Prod-Stack'); + + test.done(); + }, }; class StackWithPostProcessor extends Stack { diff --git a/packages/@aws-cdk/cdk/test/test.app.ts b/packages/@aws-cdk/cdk/test/test.app.ts index b0e5edff50db5..3e4774a335539 100644 --- a/packages/@aws-cdk/cdk/test/test.app.ts +++ b/packages/@aws-cdk/cdk/test/test.app.ts @@ -3,7 +3,7 @@ import fs = require('fs'); import { Test } from 'nodeunit'; import os = require('os'); import path = require('path'); -import { Construct, Resource, Stack, StackProps } from '../lib'; +import { Construct, Program, Resource, Stack, StackProps } from '../lib'; import { App } from '../lib/app'; function withApp(context: { [key: string]: any } | undefined, block: (app: App) => void) { @@ -16,11 +16,12 @@ function withApp(context: { [key: string]: any } | undefined, block: (app: App) delete process.env[cxapi.CONTEXT_ENV]; } - const app = new App(); + const program = new Program(); + const app = new App(undefined, { program }); block(app); - app.run(); + program.run(); const outfile = path.join(outdir, cxapi.OUTFILE_NAME); const response = JSON.parse(fs.readFileSync(outfile).toString()); @@ -119,25 +120,25 @@ export = { const output = stack.metadata; stripStackTraces(output); - test.ok(output['/'], 'app-level metadata is included under "."'); - test.equal(output['/'][0].type, 'applevel'); - test.equal(output['/'][0].data, 123); + test.ok(output['/App'], 'app-level metadata is included under "."'); + test.equal(output['/App'][0].type, 'applevel'); + test.equal(output['/App'][0].data, 123); - test.ok(output['/stack1'], 'the construct "stack1" has metadata'); - test.equal(output['/stack1'][0].type, 'meta'); - test.equal(output['/stack1'][0].data, 111); - test.ok(output['/stack1'][0].trace.length > 0, 'trace contains multiple entries'); + test.ok(output['/App/stack1'], 'the construct "stack1" has metadata'); + test.equal(output['/App/stack1'][0].type, 'meta'); + test.equal(output['/App/stack1'][0].data, 111); + test.ok(output['/App/stack1'][0].trace.length > 0, 'trace contains multiple entries'); - test.ok(output['/stack1/s1c2']); - test.equal(output['/stack1/s1c2'].length, 2, 'two entries'); - test.equal(output['/stack1/s1c2'][0].data, 'warning1'); + test.ok(output['/App/stack1/s1c2']); + test.equal(output['/App/stack1/s1c2'].length, 2, 'two entries'); + test.equal(output['/App/stack1/s1c2'][0].data, 'warning1'); const stack2 = synthStack('stack2', true); const output2 = stack2.metadata; - test.ok(output2['/stack2/s1c2']); - test.equal(output2['/stack2/s1c2'][0].type, 'meta'); - test.deepEqual(output2['/stack2/s1c2'][0].data, { key: 'value' }); + test.ok(output2['/App/stack2/s1c2']); + test.equal(output2['/App/stack2/s1c2'][0].type, 'meta'); + test.deepEqual(output2['/App/stack2/s1c2'][0].data, { key: 'value' }); test.done(); }, @@ -192,8 +193,7 @@ export = { test.done(); }, - 'app.synthesizeStack(stack) performs validation first (app.validateAll()) and if there are errors, it returns the errors'(test: Test) { - + 'Program.synthesizeAll(stack) performs validation first and returns the errors'(test: Test) { class Child extends Construct { protected validate() { return [ `Error from ${this.node.id}` ]; @@ -201,17 +201,17 @@ export = { } class Parent extends Stack { - } - const app = new App(); + const program = new Program(); + const app = new App(undefined, { program }); const parent = new Parent(app, 'Parent'); new Child(parent, 'C1'); new Child(parent, 'C2'); test.throws(() => { - app.synthesizeStacks(['Parent']); + program.synthesizeAll(); }, /Stack validation failed with the following errors/); test.done(); diff --git a/packages/@aws-cdk/cdk/test/test.context.ts b/packages/@aws-cdk/cdk/test/test.context.ts index 0752b7fa1c2f1..d268d52d57d34 100644 --- a/packages/@aws-cdk/cdk/test/test.context.ts +++ b/packages/@aws-cdk/cdk/test/test.context.ts @@ -1,7 +1,7 @@ import cxapi = require('@aws-cdk/cx-api'); import { Test } from 'nodeunit'; import { App, AvailabilityZoneProvider, Construct, ContextProvider, - MetadataEntry, SSMParameterProvider, Stack } from '../lib'; + MetadataEntry, Program, SSMParameterProvider, Stack } from '../lib'; export = { 'AvailabilityZoneProvider returns a list with dummy values if the context is not available'(test: Test) { @@ -93,7 +93,8 @@ export = { }, 'Return default values if "env" is undefined to facilitate unit tests, but also expect metadata to include "error" messages'(test: Test) { - const app = new App(); + const program = new Program(); + const app = new App(undefined, { program }); const stack = new Stack(app, 'test-stack'); const child = new Construct(stack, 'ChildConstruct'); @@ -101,10 +102,11 @@ export = { test.deepEqual(new AvailabilityZoneProvider(stack).availabilityZones, [ 'dummy1a', 'dummy1b', 'dummy1c' ]); test.deepEqual(new SSMParameterProvider(child, {parameterName: 'foo'}).parameterValue(), 'dummy'); - const output = app.synthesizeStack(stack.node.id); + const output = program.synthesizeAll().stacks[0]; - const azError: MetadataEntry | undefined = output.metadata['/test-stack'].find(x => x.type === cxapi.ERROR_METADATA_KEY); - const ssmError: MetadataEntry | undefined = output.metadata['/test-stack/ChildConstruct'].find(x => x.type === cxapi.ERROR_METADATA_KEY); + const azError: MetadataEntry | undefined = output.metadata['/App/test-stack'].find(x => x.type === cxapi.ERROR_METADATA_KEY); + // tslint:disable-next-line:max-line-length + const ssmError: MetadataEntry | undefined = output.metadata['/App/test-stack/ChildConstruct'].find(x => x.type === cxapi.ERROR_METADATA_KEY); test.ok(azError && (azError.data as string).includes('Cannot determine scope for context provider availability-zones')); test.ok(ssmError && (ssmError.data as string).includes('Cannot determine scope for context provider ssm')); diff --git a/tools/cdk-build-tools/lib/os.ts b/tools/cdk-build-tools/lib/os.ts index 76a88e4a200f7..7d8b8c58d6cbd 100644 --- a/tools/cdk-build-tools/lib/os.ts +++ b/tools/cdk-build-tools/lib/os.ts @@ -16,7 +16,11 @@ export async function shell(command: string[], timers?: Timers): Promise const child = child_process.spawn(command[0], command.slice(1), { // Need this for Windows where we want .cmd and .bat to be found as well. shell: true, - stdio: [ 'ignore', 'pipe', 'inherit' ] + stdio: [ 'ignore', 'pipe', 'inherit' ], + env: { + CDK_TEST_MODE: '1', + ...process.env, + }, }); return new Promise((resolve, reject) => { From 022ed012ed1824a9bebef0d36772539d81fffc83 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Mon, 21 Jan 2019 17:02:13 -0800 Subject: [PATCH 02/11] Restored contract of synthesizeStacks() --- packages/@aws-cdk/cdk/lib/app.ts | 29 +++++++++++++++++++- packages/@aws-cdk/cdk/lib/program.ts | 7 ++--- packages/@aws-cdk/cdk/lib/util/validation.ts | 12 ++++++++ packages/@aws-cdk/cdk/test/test.app.ts | 4 +-- 4 files changed, 44 insertions(+), 8 deletions(-) create mode 100644 packages/@aws-cdk/cdk/lib/util/validation.ts diff --git a/packages/@aws-cdk/cdk/lib/app.ts b/packages/@aws-cdk/cdk/lib/app.ts index ef3d5fe3852fe..ee651a80abbb0 100644 --- a/packages/@aws-cdk/cdk/lib/app.ts +++ b/packages/@aws-cdk/cdk/lib/app.ts @@ -4,6 +4,7 @@ import { Construct } from './core/construct'; import { IConstruct, MetadataEntry, PATH_SEP } from './core/construct'; import { Environment } from './environment'; import { Program } from './program'; +import { validateAndThrow } from './util/validation'; /** * Properties for an App @@ -63,6 +64,16 @@ export class App extends Construct { */ private defaultAppName: boolean; + /** + * Whether validate() was already run or not + */ + private validated: boolean; + + /** + * Whether prepare() was already run or not + */ + private prepared: boolean; + /** * Initializes a CDK application. * @@ -78,6 +89,8 @@ export class App extends Construct { this.env = props.env || {}; this.defaultAppName = id === undefined; + this.validated = false; + this.prepared = false; } private get stacks() { @@ -104,6 +117,15 @@ export class App extends Construct { * @param stackName The name of the stack to synthesize */ public synthesizeStack(stackName: string): cxapi.SynthesizedStack { + // Maintain the existing contract that `synthesizeStack` can be called + // by consumers, and that it will prepare and validate the construct tree. + if (!this.prepared) { + this.node.prepareTree(); + } + if (!this.validated) { + validateAndThrow(this); + } + const stack = this.getStack(stackName); const account = stack.env.account || 'unknown-account'; @@ -164,7 +186,12 @@ export class App extends Construct { } } + protected prepare() { + this.prepared = true; + } + protected validate(): string[] { + this.validated = true; if (numberOfAppsInProgram(this) > 1 && this.node.id === DEFAULT_APP_NAME) { return ['When constructing more than one App, all of them must have ids']; } @@ -174,7 +201,7 @@ export class App extends Construct { /** * Synthesize the App */ - protected synthesize() { + protected synthesize(): any { return { stacks: this.synthesizeStacks(Object.keys(this.stacks)), }; diff --git a/packages/@aws-cdk/cdk/lib/program.ts b/packages/@aws-cdk/cdk/lib/program.ts index 7afabb7c6aaef..2cc6549a41309 100644 --- a/packages/@aws-cdk/cdk/lib/program.ts +++ b/packages/@aws-cdk/cdk/lib/program.ts @@ -3,6 +3,7 @@ import fs = require('fs'); import path = require('path'); import { IConstruct, Root } from './core/construct'; import { merge } from './util/json'; +import { validateAndThrow } from './util/validation'; /** * Represents a CDK program. @@ -52,11 +53,7 @@ export class Program extends Root { this.node.prepareTree(); // first, validate this stack and stop if there are errors. - const errors = this.node.validateTree(); - if (errors.length > 0) { - const errorList = errors.map(e => `[${e.source.node.path}] ${e.message}`).join('\n '); - throw new Error(`Stack validation failed with the following errors:\n ${errorList}`); - } + validateAndThrow(this); const result: any = { version: cxapi.PROTO_RESPONSE_VERSION, diff --git a/packages/@aws-cdk/cdk/lib/util/validation.ts b/packages/@aws-cdk/cdk/lib/util/validation.ts new file mode 100644 index 0000000000000..db4f670d055d8 --- /dev/null +++ b/packages/@aws-cdk/cdk/lib/util/validation.ts @@ -0,0 +1,12 @@ +import { IConstruct } from "../core/construct"; + +/** + * Validate the given construct and throw if there are any errors + */ +export function validateAndThrow(construct: IConstruct) { + const errors = construct.node.validateTree(); + if (errors.length > 0) { + const errorList = errors.map(e => `[${e.source.node.path}] ${e.message}`).join('\n '); + throw new Error(`Stack validation failed with the following errors:\n ${errorList}`); + } +} \ No newline at end of file diff --git a/packages/@aws-cdk/cdk/test/test.app.ts b/packages/@aws-cdk/cdk/test/test.app.ts index 3e4774a335539..17e0bfbd6f2a2 100644 --- a/packages/@aws-cdk/cdk/test/test.app.ts +++ b/packages/@aws-cdk/cdk/test/test.app.ts @@ -193,7 +193,7 @@ export = { test.done(); }, - 'Program.synthesizeAll(stack) performs validation first and returns the errors'(test: Test) { + 'app.synthesizeStack(stack) performs validation first and returns any errors'(test: Test) { class Child extends Construct { protected validate() { return [ `Error from ${this.node.id}` ]; @@ -211,7 +211,7 @@ export = { new Child(parent, 'C2'); test.throws(() => { - program.synthesizeAll(); + app.synthesizeStacks(['Parent']); }, /Stack validation failed with the following errors/); test.done(); From 22f818afb393ac7ba81a397f87193c20ce078364 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Mon, 21 Jan 2019 17:19:53 -0800 Subject: [PATCH 03/11] Make linter more explicit about actionable errors, ignore API violations --- packages/@aws-cdk/cdk/package.json | 6 ++++-- tools/awslint/bin/awslint.ts | 7 ++++--- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/packages/@aws-cdk/cdk/package.json b/packages/@aws-cdk/cdk/package.json index 6376c03863e3e..3c843773b854a 100644 --- a/packages/@aws-cdk/cdk/package.json +++ b/packages/@aws-cdk/cdk/package.json @@ -29,8 +29,10 @@ }, "awslint": { "exclude": [ - "construct-ctor:@aws-cdk/cdk.App.", + "construct-ctor:@aws-cdk/cdk.App..params[0]", + "construct-ctor:@aws-cdk/cdk.App..params[1]", "construct-ctor:@aws-cdk/cdk.Root.", + "construct-ctor:@aws-cdk/cdk.Program.", "construct-ctor:@aws-cdk/cdk.Stack..params*" ] }, @@ -84,4 +86,4 @@ "engines": { "node": ">= 8.10.0" } -} \ No newline at end of file +} diff --git a/tools/awslint/bin/awslint.ts b/tools/awslint/bin/awslint.ts index 1fbb08f94df2f..e8a0fc844752b 100644 --- a/tools/awslint/bin/awslint.ts +++ b/tools/awslint/bin/awslint.ts @@ -19,7 +19,7 @@ async function main() { .option('exclude', { alias: 'x', type: 'array', desc: 'do not evaludate these rules (takes priority over --include)', default: [ ] }) .option('save', { type: 'boolean', desc: 'updates package.json with "exclude" statements for all failing rules', default: false }) .option('verbose', { alias: 'v', type: 'boolean', desc: 'verbose output (prints all assertions)', default: false }) - .option('quiet', { alias: 'q', type: 'boolean', desc: 'quiet mode - shows only errors', default: false }) + .option('quiet', { alias: 'q', type: 'boolean', desc: 'quiet mode - shows only errors', default: true }) .option('force', { type: 'boolean', desc: 'succeed silently if this is not a jsii module', default: true }) .option('config', { type: 'boolean', desc: 'reads options from the "awslint" section in package.json', default: true }) .option('debug', { type: 'boolean', desc: 'debug output', default: false }) @@ -136,10 +136,12 @@ async function main() { color = colors.blue; } break; + default: } if (color) { - console.error(color(`${colors.bold(diag.scope)} -- ${DiagnosticLevel[diag.level].toLowerCase()}: ${diag.message} [${diag.rule.code}]`)); + const indent = DiagnosticLevel[diag.level].length; + console.error(color(`${DiagnosticLevel[diag.level].toLowerCase()}: ${diag.message}\n${' '.repeat(indent)} [${diag.rule.code}:${colors.bold(diag.scope)}]`)); } } } @@ -196,7 +198,6 @@ async function main() { // compile! return true; } - } main().catch(e => { From f765650ab77974734e8f9907520500e4436fa6bd Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Mon, 21 Jan 2019 18:34:37 -0800 Subject: [PATCH 04/11] Fix stack inspector --- packages/@aws-cdk/assert/lib/inspector.ts | 24 ++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/packages/@aws-cdk/assert/lib/inspector.ts b/packages/@aws-cdk/assert/lib/inspector.ts index 0b7a13490acab..90c468d0173d3 100644 --- a/packages/@aws-cdk/assert/lib/inspector.ts +++ b/packages/@aws-cdk/assert/lib/inspector.ts @@ -51,11 +51,33 @@ export class StackPathInspector extends Inspector { } public get value(): { [key: string]: any } | undefined { - const md = this.stack.metadata[`/${this.stack.name}${this.path}`]; + const md = this.getPathMetadata(); if (md === undefined) { return undefined; } const resourceMd = md.find(entry => entry.type === 'aws:cdk:logicalId'); if (resourceMd === undefined) { return undefined; } const logicalId = resourceMd.data; return this.stack.template.Resources[logicalId]; } + + /** + * Return the metadata entry for the given stack and path + * + * Complicated slightly by the fact that the stack may or may not + * have an "App" parent, whose id will appear in the metadata key + * but there's no way to get the app's id otherwise. + */ + private getPathMetadata() { + for (const [mdKey, mdValue] of Object.entries(this.stack.metadata)) { + // Looks like either: + // "/App/Stack/Resource/Path" + // "/Stack/Resource/Path" + const parts = mdKey.split('/'); + + if ((parts[1] === this.stack.name && ('/' + parts.slice(2).join('/')) === this.path) + || (parts[2] === this.stack.name && ('/' + parts.slice(3).join('/')) === this.path)) { + return mdValue; + } + } + return undefined; + } } From 6cc27c5b727aa65434c35d3c4f108be3c363f6ca Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Tue, 22 Jan 2019 15:24:24 -0800 Subject: [PATCH 05/11] Update applet expectations --- packages/@aws-cdk/applet-js/bin/cdk-applet-js.ts | 5 +++-- packages/@aws-cdk/applet-js/test/expected1.json | 4 ++-- packages/@aws-cdk/applet-js/test/expected2.json | 6 +++--- .../@aws-cdk/applet-js/test/test-multistack-expected.json | 4 ++-- .../@aws-cdk/applet-js/test/test-nonstack-expected.json | 2 +- 5 files changed, 11 insertions(+), 10 deletions(-) diff --git a/packages/@aws-cdk/applet-js/bin/cdk-applet-js.ts b/packages/@aws-cdk/applet-js/bin/cdk-applet-js.ts index f75a94a1338f3..82b2bf2e2f727 100644 --- a/packages/@aws-cdk/applet-js/bin/cdk-applet-js.ts +++ b/packages/@aws-cdk/applet-js/bin/cdk-applet-js.ts @@ -37,12 +37,13 @@ async function main() { const searchDir = path.dirname(appletFile); const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'cdkapplet')); try { - const app = new cdk.App(); + const program = new cdk.Program(); + const app = new cdk.App(undefined, { program }); for (const [name, definition] of Object.entries(appletMap)) { await constructStack(app, searchDir, tempDir, name, definition); } - app.run(); + program.run(); } finally { await fs.remove(tempDir); } diff --git a/packages/@aws-cdk/applet-js/test/expected1.json b/packages/@aws-cdk/applet-js/test/expected1.json index 5f49964fdcf1e..2e6daa1337940 100644 --- a/packages/@aws-cdk/applet-js/test/expected1.json +++ b/packages/@aws-cdk/applet-js/test/expected1.json @@ -13,7 +13,7 @@ } }, "metadata": { - "/TestApplet/P1": [ + "/App/TestApplet/P1": [ { "type": "aws:cdk:logicalId", "data": "P1", @@ -22,7 +22,7 @@ ] } ], - "/TestApplet/P2": [ + "/App/TestApplet/P2": [ { "type": "aws:cdk:logicalId", "data": "P2", diff --git a/packages/@aws-cdk/applet-js/test/expected2.json b/packages/@aws-cdk/applet-js/test/expected2.json index 3ef09cc69797f..dba2db5146bf5 100644 --- a/packages/@aws-cdk/applet-js/test/expected2.json +++ b/packages/@aws-cdk/applet-js/test/expected2.json @@ -17,7 +17,7 @@ } }, "metadata": { - "/TestApplet/P1": [ + "/App/TestApplet/P1": [ { "type": "aws:cdk:logicalId", "data": "P1", @@ -26,7 +26,7 @@ ] } ], - "/TestApplet/P2": [ + "/App/TestApplet/P2": [ { "type": "aws:cdk:logicalId", "data": "P2", @@ -35,7 +35,7 @@ ] } ], - "/TestApplet/P3": [ + "/App/TestApplet/P3": [ { "type": "aws:cdk:logicalId", "data": "P3", diff --git a/packages/@aws-cdk/applet-js/test/test-multistack-expected.json b/packages/@aws-cdk/applet-js/test/test-multistack-expected.json index d3cfb592f5396..7aa5b19724bac 100644 --- a/packages/@aws-cdk/applet-js/test/test-multistack-expected.json +++ b/packages/@aws-cdk/applet-js/test/test-multistack-expected.json @@ -13,7 +13,7 @@ } }, "metadata": { - "/Stack2/P1": [ + "/App/Stack2/P1": [ { "type": "aws:cdk:logicalId", "data": "P1", @@ -22,7 +22,7 @@ ] } ], - "/Stack2/P2": [ + "/App/Stack2/P2": [ { "type": "aws:cdk:logicalId", "data": "P2", diff --git a/packages/@aws-cdk/applet-js/test/test-nonstack-expected.json b/packages/@aws-cdk/applet-js/test/test-nonstack-expected.json index 3abdc38dc8187..3c44264a0e8f5 100644 --- a/packages/@aws-cdk/applet-js/test/test-nonstack-expected.json +++ b/packages/@aws-cdk/applet-js/test/test-nonstack-expected.json @@ -9,7 +9,7 @@ } }, "metadata": { - "/NoStackApplet/Default/P1": [ + "/App/NoStackApplet/Default/P1": [ { "type": "aws:cdk:logicalId", "data": "P1", From d06e02b2d79efc30e12dfe09954270d28f62631c Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Tue, 22 Jan 2019 15:52:35 -0800 Subject: [PATCH 06/11] Only set test mode for unit tests --- tools/cdk-build-tools/bin/cdk-build.ts | 4 ++-- tools/cdk-build-tools/bin/cdk-package.ts | 4 ++-- tools/cdk-build-tools/bin/cdk-test.ts | 9 ++++++--- tools/cdk-build-tools/lib/compile.ts | 8 ++++---- tools/cdk-build-tools/lib/os.ts | 14 ++++++++------ 5 files changed, 22 insertions(+), 17 deletions(-) diff --git a/tools/cdk-build-tools/bin/cdk-build.ts b/tools/cdk-build-tools/bin/cdk-build.ts index 7bc758deebd05..5abbd81da252e 100644 --- a/tools/cdk-build-tools/bin/cdk-build.ts +++ b/tools/cdk-build-tools/bin/cdk-build.ts @@ -28,12 +28,12 @@ async function main() { const options = cdkBuildOptions(); if (options.pre) { - await shell(options.pre, timers); + await shell(options.pre, { timers }); } // See if we need to call cfn2ts if (options.cloudformation) { - await shell(['cfn2ts', `--scope=${options.cloudformation}`], timers); + await shell(['cfn2ts', `--scope=${options.cloudformation}`], { timers }); } await compileCurrentPackage(timers, { jsii: args.jsii, tsc: args.tsc }); diff --git a/tools/cdk-build-tools/bin/cdk-package.ts b/tools/cdk-build-tools/bin/cdk-package.ts index dee69e2f35ff0..07a346cfcc8d1 100644 --- a/tools/cdk-build-tools/bin/cdk-package.ts +++ b/tools/cdk-build-tools/bin/cdk-package.ts @@ -36,10 +36,10 @@ async function main() { } if (pkg.jsii) { - await shell([ args.jsiiPacmak, args.verbose ? '-vvv' : '-v', '-o', outdir ], timers); + await shell([ args.jsiiPacmak, args.verbose ? '-vvv' : '-v', '-o', outdir ], { timers }); } else { // just "npm pack" and deploy to "outdir" - const tarball = (await shell([ 'npm', 'pack' ], timers)).trim(); + const tarball = (await shell([ 'npm', 'pack' ], { timers })).trim(); const target = path.join(outdir, 'js'); await fs.remove(target); await fs.mkdirp(target); diff --git a/tools/cdk-build-tools/bin/cdk-test.ts b/tools/cdk-build-tools/bin/cdk-test.ts index abde8013b2073..5e1b28d15df9d 100644 --- a/tools/cdk-build-tools/bin/cdk-test.ts +++ b/tools/cdk-build-tools/bin/cdk-test.ts @@ -43,7 +43,7 @@ async function main() { const options = cdkBuildOptions(); if (options.test) { - await shell(options.test, timers); + await shell(options.test, { timers }); } const testFiles = await unitTestFiles(); @@ -70,12 +70,15 @@ async function main() { testCommand.push(args.nodeunit); testCommand.push(...testFiles.map(f => f.path)); - await shell(testCommand, timers); + await shell(testCommand, { + timers, + env: { CDK_TEST_MODE: '1' } + }); } // Run integration test if the package has integ test files if (await hasIntegTests()) { - await shell(['cdk-integ-assert'], timers); + await shell(['cdk-integ-assert'], { timers }); } } diff --git a/tools/cdk-build-tools/lib/compile.ts b/tools/cdk-build-tools/lib/compile.ts index 7785aa6920f4f..0acfa383152df 100644 --- a/tools/cdk-build-tools/lib/compile.ts +++ b/tools/cdk-build-tools/lib/compile.ts @@ -7,7 +7,7 @@ import { Timers } from "./timer"; * Run the compiler on the current package */ export async function compileCurrentPackage(timers: Timers, compilers: CompilerOverrides = {}): Promise { - await shell(packageCompiler(compilers), timers); + await shell(packageCompiler(compilers), { timers }); // Find files in bin/ that look like they should be executable, and make them so. const scripts = currentPackageJson().bin || {}; @@ -16,7 +16,7 @@ export async function compileCurrentPackage(timers: Timers, compilers: CompilerO } // Always call linters - await shell(['tslint', '--project', '.'], timers); - await shell(['pkglint'], timers); - await shell([ path.join(__dirname, '..', 'bin', 'cdk-awslint') ], timers); + await shell(['tslint', '--project', '.'], { timers }); + await shell(['pkglint'], { timers }); + await shell([ path.join(__dirname, '..', 'bin', 'cdk-awslint') ], { timers }); } diff --git a/tools/cdk-build-tools/lib/os.ts b/tools/cdk-build-tools/lib/os.ts index 7d8b8c58d6cbd..5d024cb8264e5 100644 --- a/tools/cdk-build-tools/lib/os.ts +++ b/tools/cdk-build-tools/lib/os.ts @@ -3,13 +3,18 @@ import fs = require('fs'); import util = require('util'); import { Timers } from "./timer"; +export interface ShellOptions { + timers?: Timers; + env?: {[key: string]: string}; +} + /** * A shell command that does what you want * * Is platform-aware, handles errors nicely. */ -export async function shell(command: string[], timers?: Timers): Promise { - const timer = (timers || new Timers()).start(command[0]); +export async function shell(command: string[], options: ShellOptions = {}): Promise { + const timer = (options.timers || new Timers()).start(command[0]); await makeShellScriptExecutable(command[0]); @@ -17,10 +22,7 @@ export async function shell(command: string[], timers?: Timers): Promise // Need this for Windows where we want .cmd and .bat to be found as well. shell: true, stdio: [ 'ignore', 'pipe', 'inherit' ], - env: { - CDK_TEST_MODE: '1', - ...process.env, - }, + env: { ...process.env, ...(options.env || {}) } }); return new Promise((resolve, reject) => { From 0778bd7825ae3addd4e2fe1191578cff9db247c9 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Tue, 22 Jan 2019 15:54:14 -0800 Subject: [PATCH 07/11] Mention we should do this better once using Jest --- packages/@aws-cdk/cdk/lib/app.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/@aws-cdk/cdk/lib/app.ts b/packages/@aws-cdk/cdk/lib/app.ts index ee651a80abbb0..a29ec0f5e952a 100644 --- a/packages/@aws-cdk/cdk/lib/app.ts +++ b/packages/@aws-cdk/cdk/lib/app.ts @@ -80,7 +80,9 @@ export class App extends Construct { * @param request Optional toolkit request (e.g. for tests) */ constructor(id?: string, props: AppProps = {}) { - // For tests, we use a fresh Program every time + // For tests, we use a fresh Program every time. Right now, we configure this as + // an environment variable set by 'cdk-test'. If we were using Jest, + // we could use Jest initialization to configure testing mode. const program = props.program || (process.env.CDK_TEST_MODE === '1' ? new Program() : Program.defaultInstance()); From 6c7888bd7f9d12ac40a6ff1d38114740cfa7e585 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Wed, 23 Jan 2019 09:45:36 -0800 Subject: [PATCH 08/11] Make VPC tag names consistent --- packages/@aws-cdk/aws-ec2/lib/vpc.ts | 22 ++++++++++++++++++++-- packages/@aws-cdk/aws-ec2/test/test.vpc.ts | 8 ++++---- 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/packages/@aws-cdk/aws-ec2/lib/vpc.ts b/packages/@aws-cdk/aws-ec2/lib/vpc.ts index 93af5eb8ed121..b8e140dda30ff 100644 --- a/packages/@aws-cdk/aws-ec2/lib/vpc.ts +++ b/packages/@aws-cdk/aws-ec2/lib/vpc.ts @@ -1,4 +1,5 @@ import cdk = require('@aws-cdk/cdk'); +import { Stack } from '@aws-cdk/cdk'; import { CfnEIP, CfnInternetGateway, CfnNatGateway, CfnRoute } from './ec2.generated'; import { CfnRouteTable, CfnSubnet, CfnSubnetRouteTableAssociation, CfnVPC, CfnVPCGatewayAttachment } from './ec2.generated'; import { NetworkBuilder } from './network-util'; @@ -299,7 +300,7 @@ export class VpcNetwork extends VpcNetworkBase implements cdk.ITaggable { } this.tags = new cdk.TagManager(this, { initialTags: props.tags}); - this.tags.setTag(NAME_TAG, this.node.path, { overwrite: false }); + this.tags.setTag(NAME_TAG, legacyPath(this), { overwrite: false }); const cidrBlock = ifUndefined(props.cidr, VpcNetwork.DEFAULT_CIDR_RANGE); this.networkBuilder = new NetworkBuilder(cidrBlock); @@ -556,7 +557,7 @@ export class VpcSubnet extends cdk.Construct implements IVpcSubnet, cdk.ITaggabl constructor(scope: cdk.Construct, id: string, props: VpcSubnetProps) { super(scope, id); this.tags = new cdk.TagManager(this, {initialTags: props.tags}); - this.tags.setTag(NAME_TAG, this.node.path, {overwrite: false}); + this.tags.setTag(NAME_TAG, legacyPath(this), {overwrite: false}); this.availabilityZone = props.availabilityZone; const subnet = new CfnSubnet(this, 'Subnet', { @@ -714,3 +715,20 @@ class ImportedVpcSubnet extends cdk.Construct implements IVpcSubnet { return this.props; } } + +/** + * Return the legacy path of the construct, used for tagging. + * + * Tag names look like: + * + * MyStack/Vpc/Subnet, for production stacks + * + * We do a slightly complicated calculation here because we want to + * keep the tag names that were created using previous versions of + * the CDK stable. + */ +function legacyPath(construct: cdk.IConstruct): string { + const stack = Stack.find(construct); + const path = construct.node.ancestors(stack).map(a => a.node.id); + return `${stack.name}/${path.join('/')}`; +} \ No newline at end of file diff --git a/packages/@aws-cdk/aws-ec2/test/test.vpc.ts b/packages/@aws-cdk/aws-ec2/test/test.vpc.ts index ec8aa87d1403e..86a872cc29048 100644 --- a/packages/@aws-cdk/aws-ec2/test/test.vpc.ts +++ b/packages/@aws-cdk/aws-ec2/test/test.vpc.ts @@ -29,7 +29,7 @@ export = { const stack = getTestStack(); new VpcNetwork(stack, 'TheVPC'); expect(stack).to(haveResource('AWS::EC2::VPC', - hasTags( [ {Key: 'Name', Value: 'TheVPC'} ]))); + hasTags( [ {Key: 'Name', Value: 'TestStack/TheVPC'} ]))); test.done(); }, @@ -312,7 +312,7 @@ export = { Tags: [ { Key: 'Name', - Value: `VPC/egressSubnet${i}`, + Value: `TestStack/VPC/egressSubnet${i}`, } ] })); @@ -381,12 +381,12 @@ export = { const stack = getTestStack(); const vpc = new VpcNetwork(stack, 'TheVPC'); for (const subnet of vpc.publicSubnets) { - const tag = {Key: 'Name', Value: subnet.node.path}; + const tag = {Key: 'Name', Value: 'TestStack/TheVPC/' + subnet.node.id}; expect(stack).to(haveResource('AWS::EC2::NatGateway', hasTags([tag]))); expect(stack).to(haveResource('AWS::EC2::RouteTable', hasTags([tag]))); } for (const subnet of vpc.privateSubnets) { - const tag = {Key: 'Name', Value: subnet.node.path}; + const tag = {Key: 'Name', Value: 'TestStack/TheVPC/' + subnet.node.id}; expect(stack).to(haveResource('AWS::EC2::RouteTable', hasTags([tag]))); } test.done(); From ac93c0634a43cb97ba7afe7dd145b0933598bff4 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Wed, 23 Jan 2019 17:41:58 -0800 Subject: [PATCH 09/11] Fix metadata changes --- packages/@aws-cdk/cdk/lib/app.ts | 22 ++++++------------- packages/@aws-cdk/cdk/lib/core/construct.ts | 14 ++++++++++-- packages/@aws-cdk/cdk/lib/program.ts | 2 ++ .../cdk/test/cloudformation/test.resource.ts | 4 ++-- .../cdk/test/cloudformation/test.stack.ts | 11 +++++----- 5 files changed, 28 insertions(+), 25 deletions(-) diff --git a/packages/@aws-cdk/cdk/lib/app.ts b/packages/@aws-cdk/cdk/lib/app.ts index a29ec0f5e952a..d743dbf1d3f58 100644 --- a/packages/@aws-cdk/cdk/lib/app.ts +++ b/packages/@aws-cdk/cdk/lib/app.ts @@ -167,25 +167,17 @@ export class App extends Construct { public collectMetadata(stack: Stack) { const output: { [id: string]: MetadataEntry[] } = { }; - visit(stack); + // Add in App metadata into every stack + output[PATH_SEP + this.node.path] = this.node.metadata; - // add app-level metadata as well - if (this.node.metadata.length > 0) { - output[PATH_SEP + this.node.path] = this.node.metadata; - } - - return output; - - function visit(node: IConstruct) { - if (node.node.metadata.length > 0) { + for (const construct of stack.node.findAll()) { + if (construct.node.metadata.length > 0) { // Make the path absolute. - output[PATH_SEP + node.node.path] = node.node.metadata.map(md => node.node.resolve(md) as MetadataEntry); - } - - for (const child of node.node.children) { - visit(child); + output[PATH_SEP + construct.node.path] = construct.node.metadata.map(md => construct.node.resolve(md) as MetadataEntry); } } + + return output; } protected prepare() { diff --git a/packages/@aws-cdk/cdk/lib/core/construct.ts b/packages/@aws-cdk/cdk/lib/core/construct.ts index 3c14428254389..07d60d6dd3182 100644 --- a/packages/@aws-cdk/cdk/lib/core/construct.ts +++ b/packages/@aws-cdk/cdk/lib/core/construct.ts @@ -448,11 +448,13 @@ export class ConstructNode { } /** - * Return the path of components up to but excluding the root + * Return the path of components up to but excluding the root (if the root is Program) */ public rootPath(): IConstruct[] { const ancestors = this.ancestors(); - ancestors.shift(); + if (Root.isRoot(ancestors[0])) { + ancestors.shift(); + } return ancestors; } @@ -536,6 +538,14 @@ export class Construct implements IConstruct { * No scope and no name. */ export class Root extends Construct { + /** + * Return whether the given instance is a Root object + */ + public static isRoot(x: IConstruct): x is Root { + return (x as any)._isRoot === true; + } + + protected _isRoot = true; constructor() { // Bypass type checks super(undefined as any, ''); diff --git a/packages/@aws-cdk/cdk/lib/program.ts b/packages/@aws-cdk/cdk/lib/program.ts index 2cc6549a41309..544f4eb119304 100644 --- a/packages/@aws-cdk/cdk/lib/program.ts +++ b/packages/@aws-cdk/cdk/lib/program.ts @@ -22,6 +22,8 @@ export class Program extends Root { private static DefaultInstance?: Program; + protected _isProgram = true; + constructor() { super(); diff --git a/packages/@aws-cdk/cdk/test/cloudformation/test.resource.ts b/packages/@aws-cdk/cdk/test/cloudformation/test.resource.ts index 3b0ee595aa35a..fc9fb583ae813 100644 --- a/packages/@aws-cdk/cdk/test/cloudformation/test.resource.ts +++ b/packages/@aws-cdk/cdk/test/cloudformation/test.resource.ts @@ -587,7 +587,7 @@ export = { }, '"aws:cdk:path" metadata is added if "aws:cdk:path-metadata" context is set to true'(test: Test) { - const stack = new Stack(); + const stack = new Stack(undefined, 'Stack'); stack.node.setContext(cxapi.PATH_METADATA_ENABLE_CONTEXT, true); const parent = new Construct(stack, 'Parent'); @@ -599,7 +599,7 @@ export = { test.deepEqual(stack.toCloudFormation(), { Resources: { ParentMyResource4B1FDBCC: { Type: 'MyResourceType', - Metadata: { [cxapi.PATH_METADATA_KEY]: 'Parent/MyResource' } } } }); + Metadata: { [cxapi.PATH_METADATA_KEY]: 'Stack/Parent/MyResource' } } } }); test.done(); } diff --git a/packages/@aws-cdk/cdk/test/cloudformation/test.stack.ts b/packages/@aws-cdk/cdk/test/cloudformation/test.stack.ts index 05609c4fbeeec..c771ffc70edbe 100644 --- a/packages/@aws-cdk/cdk/test/cloudformation/test.stack.ts +++ b/packages/@aws-cdk/cdk/test/cloudformation/test.stack.ts @@ -126,7 +126,7 @@ export = { }, 'Stack.findResource will fail if the element is not a resource'(test: Test) { - const stack = new Stack(); + const stack = new Stack(undefined, 'Stack'); const p = new Parameter(stack, 'MyParam', { type: 'String' }); @@ -135,16 +135,15 @@ export = { }, 'Stack.getByPath can be used to find any CloudFormation element (Parameter, Output, etc)'(test: Test) { - - const stack = new Stack(); + const stack = new Stack(undefined, 'Stack'); const p = new Parameter(stack, 'MyParam', { type: 'String' }); const o = new Output(stack, 'MyOutput'); const c = new Condition(stack, 'MyCondition'); - test.equal(stack.node.findChild(p.node.path), p); - test.equal(stack.node.findChild(o.node.path), o); - test.equal(stack.node.findChild(c.node.path), c); + test.equal(stack.node.findChild(p.stackPath), p); + test.equal(stack.node.findChild(o.stackPath), o); + test.equal(stack.node.findChild(c.stackPath), c); test.done(); }, From f4201f2075d937a9943bb8d78b4e368460961159 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Thu, 24 Jan 2019 15:52:16 -0800 Subject: [PATCH 10/11] Use other environment variable, fix some tests --- packages/@aws-cdk/aws-ec2/lib/security-group.ts | 3 ++- packages/@aws-cdk/aws-ec2/lib/util.ts | 2 +- packages/@aws-cdk/aws-ec2/lib/vpc.ts | 15 +++++++-------- .../@aws-cdk/aws-ec2/test/test.connections.ts | 6 +++--- packages/@aws-cdk/cdk/lib/app.ts | 13 ++++--------- packages/@aws-cdk/cdk/lib/program.ts | 10 ++++++++++ packages/@aws-cdk/cdk/test/test.app.ts | 3 +++ tools/cdk-build-tools/bin/cdk-test.ts | 5 +---- 8 files changed, 31 insertions(+), 26 deletions(-) diff --git a/packages/@aws-cdk/aws-ec2/lib/security-group.ts b/packages/@aws-cdk/aws-ec2/lib/security-group.ts index 0a22448141753..a459fae6526e0 100644 --- a/packages/@aws-cdk/aws-ec2/lib/security-group.ts +++ b/packages/@aws-cdk/aws-ec2/lib/security-group.ts @@ -2,6 +2,7 @@ import { Construct, IConstruct, ITaggable, Output, TagManager, Tags, Token } fro import { Connections, IConnectable } from './connections'; import { CfnSecurityGroup, CfnSecurityGroupEgress, CfnSecurityGroupIngress } from './ec2.generated'; import { IPortRange, ISecurityGroupRule } from './security-group-rule'; +import { defaultName } from './vpc'; import { IVpcNetwork } from './vpc-ref'; export interface ISecurityGroup extends IConstruct, ISecurityGroupRule, IConnectable { @@ -172,7 +173,7 @@ export class SecurityGroup extends SecurityGroupBase implements ITaggable { super(scope, id); this.tags = new TagManager(this, { initialTags: props.tags}); - const groupDescription = props.description || this.node.path; + const groupDescription = props.description || defaultName(this); this.allowAllOutbound = props.allowAllOutbound !== false; diff --git a/packages/@aws-cdk/aws-ec2/lib/util.ts b/packages/@aws-cdk/aws-ec2/lib/util.ts index 77a11e16bb305..04f0904c994fb 100644 --- a/packages/@aws-cdk/aws-ec2/lib/util.ts +++ b/packages/@aws-cdk/aws-ec2/lib/util.ts @@ -161,4 +161,4 @@ export function range(n: number): number[] { ret.push(i); } return ret; -} +} \ No newline at end of file diff --git a/packages/@aws-cdk/aws-ec2/lib/vpc.ts b/packages/@aws-cdk/aws-ec2/lib/vpc.ts index b8e140dda30ff..b0f7296b0553c 100644 --- a/packages/@aws-cdk/aws-ec2/lib/vpc.ts +++ b/packages/@aws-cdk/aws-ec2/lib/vpc.ts @@ -1,5 +1,4 @@ import cdk = require('@aws-cdk/cdk'); -import { Stack } from '@aws-cdk/cdk'; import { CfnEIP, CfnInternetGateway, CfnNatGateway, CfnRoute } from './ec2.generated'; import { CfnRouteTable, CfnSubnet, CfnSubnetRouteTableAssociation, CfnVPC, CfnVPCGatewayAttachment } from './ec2.generated'; import { NetworkBuilder } from './network-util'; @@ -300,7 +299,7 @@ export class VpcNetwork extends VpcNetworkBase implements cdk.ITaggable { } this.tags = new cdk.TagManager(this, { initialTags: props.tags}); - this.tags.setTag(NAME_TAG, legacyPath(this), { overwrite: false }); + this.tags.setTag(NAME_TAG, defaultName(this), { overwrite: false }); const cidrBlock = ifUndefined(props.cidr, VpcNetwork.DEFAULT_CIDR_RANGE); this.networkBuilder = new NetworkBuilder(cidrBlock); @@ -557,7 +556,7 @@ export class VpcSubnet extends cdk.Construct implements IVpcSubnet, cdk.ITaggabl constructor(scope: cdk.Construct, id: string, props: VpcSubnetProps) { super(scope, id); this.tags = new cdk.TagManager(this, {initialTags: props.tags}); - this.tags.setTag(NAME_TAG, legacyPath(this), {overwrite: false}); + this.tags.setTag(NAME_TAG, defaultName(this), {overwrite: false}); this.availabilityZone = props.availabilityZone; const subnet = new CfnSubnet(this, 'Subnet', { @@ -723,12 +722,12 @@ class ImportedVpcSubnet extends cdk.Construct implements IVpcSubnet { * * MyStack/Vpc/Subnet, for production stacks * - * We do a slightly complicated calculation here because we want to - * keep the tag names that were created using previous versions of - * the CDK stable. + * We do a slightly complicated calculation here (instead of just taking + * construct.node.path) because we want to keep the tag names that were created + * using previous versions of the CDK stable. */ -function legacyPath(construct: cdk.IConstruct): string { - const stack = Stack.find(construct); +export function defaultName(construct: cdk.IConstruct): string { + const stack = cdk.Stack.find(construct); const path = construct.node.ancestors(stack).map(a => a.node.id); return `${stack.name}/${path.join('/')}`; } \ No newline at end of file diff --git a/packages/@aws-cdk/aws-ec2/test/test.connections.ts b/packages/@aws-cdk/aws-ec2/test/test.connections.ts index 9cc0482c1f9f2..1b27078a946f6 100644 --- a/packages/@aws-cdk/aws-ec2/test/test.connections.ts +++ b/packages/@aws-cdk/aws-ec2/test/test.connections.ts @@ -67,7 +67,7 @@ export = { 'security groups added to connections after rule still gets rule'(test: Test) { // GIVEN - const stack = new Stack(); + const stack = new Stack(undefined, 'TestStack'); const vpc = new VpcNetwork(stack, 'VPC'); const sg1 = new SecurityGroup(stack, 'SecurityGroup1', { vpc, allowAllOutbound: false }); const sg2 = new SecurityGroup(stack, 'SecurityGroup2', { vpc, allowAllOutbound: false }); @@ -79,7 +79,7 @@ export = { // THEN expect(stack).to(haveResource('AWS::EC2::SecurityGroup', { - GroupDescription: "SecurityGroup1", + GroupDescription: "TestStack/SecurityGroup1", SecurityGroupIngress: [ { Description: "from 0.0.0.0/0:88", @@ -92,7 +92,7 @@ export = { })); expect(stack).to(haveResource('AWS::EC2::SecurityGroup', { - GroupDescription: "SecurityGroup2", + GroupDescription: "TestStack/SecurityGroup2", SecurityGroupIngress: [ { Description: "from 0.0.0.0/0:88", diff --git a/packages/@aws-cdk/cdk/lib/app.ts b/packages/@aws-cdk/cdk/lib/app.ts index d743dbf1d3f58..509b270709a81 100644 --- a/packages/@aws-cdk/cdk/lib/app.ts +++ b/packages/@aws-cdk/cdk/lib/app.ts @@ -80,13 +80,7 @@ export class App extends Construct { * @param request Optional toolkit request (e.g. for tests) */ constructor(id?: string, props: AppProps = {}) { - // For tests, we use a fresh Program every time. Right now, we configure this as - // an environment variable set by 'cdk-test'. If we were using Jest, - // we could use Jest initialization to configure testing mode. - const program = props.program || (process.env.CDK_TEST_MODE === '1' - ? new Program() - : Program.defaultInstance()); - + const program = props.program || Program.defaultInstance(); super(program, id || DEFAULT_APP_NAME); this.env = props.env || {}; @@ -168,9 +162,10 @@ export class App extends Construct { const output: { [id: string]: MetadataEntry[] } = { }; // Add in App metadata into every stack - output[PATH_SEP + this.node.path] = this.node.metadata; + emit(this); + stack.node.findAll().forEach(emit); - for (const construct of stack.node.findAll()) { + function emit(construct: IConstruct) { if (construct.node.metadata.length > 0) { // Make the path absolute. output[PATH_SEP + construct.node.path] = construct.node.metadata.map(md => construct.node.resolve(md) as MetadataEntry); diff --git a/packages/@aws-cdk/cdk/lib/program.ts b/packages/@aws-cdk/cdk/lib/program.ts index 544f4eb119304..6fddc51b1e877 100644 --- a/packages/@aws-cdk/cdk/lib/program.ts +++ b/packages/@aws-cdk/cdk/lib/program.ts @@ -11,8 +11,18 @@ import { validateAndThrow } from './util/validation'; export class Program extends Root { /** * Return the default singleton Program instance + * + * Return the same instance every time when running via the toolkit + * (i.e., when doing an actual app synthesis). If not, we're most + * likely running as a unit test, in which case we'll make a fresh + * Program every time. */ public static defaultInstance(): Program { + if (process.env[cxapi.OUTDIR_ENV] === undefined) { + // Not running via toolkit + return new Program(); + } + if (Program.DefaultInstance === undefined) { Program.DefaultInstance = new Program(); Program.DefaultInstance.initializeDefaultProgram(); diff --git a/packages/@aws-cdk/cdk/test/test.app.ts b/packages/@aws-cdk/cdk/test/test.app.ts index 17e0bfbd6f2a2..856373755e456 100644 --- a/packages/@aws-cdk/cdk/test/test.app.ts +++ b/packages/@aws-cdk/cdk/test/test.app.ts @@ -27,6 +27,9 @@ function withApp(context: { [key: string]: any } | undefined, block: (app: App) const response = JSON.parse(fs.readFileSync(outfile).toString()); fs.unlinkSync(outfile); fs.rmdirSync(outdir); + + // Be sure to not leave this environment variable set + delete process.env[cxapi.OUTDIR_ENV]; return response; } diff --git a/tools/cdk-build-tools/bin/cdk-test.ts b/tools/cdk-build-tools/bin/cdk-test.ts index 5e1b28d15df9d..2507298c6e6da 100644 --- a/tools/cdk-build-tools/bin/cdk-test.ts +++ b/tools/cdk-build-tools/bin/cdk-test.ts @@ -70,10 +70,7 @@ async function main() { testCommand.push(args.nodeunit); testCommand.push(...testFiles.map(f => f.path)); - await shell(testCommand, { - timers, - env: { CDK_TEST_MODE: '1' } - }); + await shell(testCommand, { timers }); } // Run integration test if the package has integ test files From 9943293fa91c19b08e16483e115c60a8ac2f8941 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Mon, 4 Feb 2019 14:51:48 +0100 Subject: [PATCH 11/11] Use a symbol for isApp --- packages/@aws-cdk/cdk/lib/app.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/@aws-cdk/cdk/lib/app.ts b/packages/@aws-cdk/cdk/lib/app.ts index 509b270709a81..70371356cab54 100644 --- a/packages/@aws-cdk/cdk/lib/app.ts +++ b/packages/@aws-cdk/cdk/lib/app.ts @@ -6,6 +6,8 @@ import { Environment } from './environment'; import { Program } from './program'; import { validateAndThrow } from './util/validation'; +const isAppSymbol = Symbol.for('aws-cdk:isApp'); + /** * Properties for an App */ @@ -51,7 +53,7 @@ export class App extends Construct { * True if the given construct is an App object */ public static isApp(construct: IConstruct): construct is App { - return (construct as any).defaultAppName !== undefined; + return (construct as any)[isAppSymbol] === true; } /** @@ -87,6 +89,8 @@ export class App extends Construct { this.defaultAppName = id === undefined; this.validated = false; this.prepared = false; + + Object.defineProperty(this, isAppSymbol, { value: true }); } private get stacks() {