From 2a3752c9239f8f89aeb6dd3c929b4061a6fd2ac2 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Thu, 10 Jan 2019 13:52:42 +0100 Subject: [PATCH 1/2] fix(aws-cdk): destroy stacks in reverse order Fixes #1508. --- packages/aws-cdk/bin/cdk.ts | 4 ++++ packages/aws-cdk/integ-tests/app/app.js | 4 ++++ packages/aws-cdk/integ-tests/test-cdk-order.sh | 10 +++++----- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/packages/aws-cdk/bin/cdk.ts b/packages/aws-cdk/bin/cdk.ts index ef82eedbfe31e..a552e8a847f7e 100644 --- a/packages/aws-cdk/bin/cdk.ts +++ b/packages/aws-cdk/bin/cdk.ts @@ -366,6 +366,10 @@ async function initCommandLine() { async function cliDestroy(stackNames: string[], force: boolean, roleArn: string | undefined) { const stacks = await appStacks.selectStacks(...stackNames); + + // The stacks will have been ordered for deployment, so reverse them for deletion. + stacks.reverse(); + renames.validateSelectedStacks(stacks); if (!force) { diff --git a/packages/aws-cdk/integ-tests/app/app.js b/packages/aws-cdk/integ-tests/app/app.js index c07d5a43551c1..28c231e442326 100644 --- a/packages/aws-cdk/integ-tests/app/app.js +++ b/packages/aws-cdk/integ-tests/app/app.js @@ -33,6 +33,8 @@ class IamStack extends cdk.Stack { class ProvidingStack extends cdk.Stack { constructor(parent, id) { super(parent, id); + + new sns.Topic(this, 'BogusTopic'); // Some filler } } @@ -40,6 +42,8 @@ class ConsumingStack extends cdk.Stack { constructor(parent, id, providingStack) { super(parent, id); + + new sns.Topic(this, 'BogusTopic'); // Some filler new cdk.Output(this, 'IConsumedSomething', { value: providingStack.stackName }); } } diff --git a/packages/aws-cdk/integ-tests/test-cdk-order.sh b/packages/aws-cdk/integ-tests/test-cdk-order.sh index 9aa46451fbcfa..410f1adc90478 100755 --- a/packages/aws-cdk/integ-tests/test-cdk-order.sh +++ b/packages/aws-cdk/integ-tests/test-cdk-order.sh @@ -6,10 +6,10 @@ source ${scriptdir}/common.bash setup -# ls order == synthesis order == provider before consumer -assert "cdk list | grep -- -order-" < Date: Thu, 10 Jan 2019 14:46:37 +0100 Subject: [PATCH 2/2] feat(aws-cdk): automatically include dependency stacks Automatically include stacks that have a dependency relationship with the requested stacks, unless `--exclusively` (`-e`) is passed on the command line. Fixes #1505. --- packages/aws-cdk/bin/cdk.ts | 28 +++-- .../aws-cdk/integ-tests/test-cdk-order.sh | 8 +- .../aws-cdk/lib/api/cxapp/environments.ts | 4 +- packages/aws-cdk/lib/api/cxapp/stacks.ts | 101 +++++++++++++++++- 4 files changed, 120 insertions(+), 21 deletions(-) diff --git a/packages/aws-cdk/bin/cdk.ts b/packages/aws-cdk/bin/cdk.ts index a552e8a847f7e..e869181ad8f51 100644 --- a/packages/aws-cdk/bin/cdk.ts +++ b/packages/aws-cdk/bin/cdk.ts @@ -9,7 +9,7 @@ import yargs = require('yargs'); import { bootstrapEnvironment, deployStack, destroyStack, loadToolkitInfo, Mode, SDK } from '../lib'; import { environmentsFromDescriptors, globEnvironmentsFromStacks } from '../lib/api/cxapp/environments'; -import { AppStacks, listStackNames } from '../lib/api/cxapp/stacks'; +import { AppStacks, ExtendedStackSelection, listStackNames } from '../lib/api/cxapp/stacks'; import { leftPad } from '../lib/api/util/string-manipulation'; import { printSecurityDiff, printStackDiff, RequireApproval } from '../lib/diff'; import { availableInitLanguages, cliInit, printAvailableTemplates } from '../lib/init'; @@ -53,13 +53,16 @@ async function parseCommandLineArguments() { .command([ 'list', 'ls' ], 'Lists all stacks in the app', yargs => yargs .option('long', { type: 'boolean', default: false, alias: 'l', desc: 'display environment information for each stack' })) .command([ 'synthesize [STACKS..]', 'synth [STACKS..]' ], 'Synthesizes and prints the CloudFormation template for this stack', yargs => yargs + .option('exclusively', { type: 'boolean', alias: 'e', desc: 'only deploy requested stacks, don\'t include dependencies' }) .option('interactive', { type: 'boolean', alias: 'i', desc: 'interactively watch and show template updates' }) .option('output', { type: 'string', alias: 'o', desc: 'write CloudFormation template for requested stacks to the given directory' }) .option('numbered', { type: 'boolean', alias: 'n', desc: 'Prefix filenames with numbers to indicate deployment ordering' })) .command('bootstrap [ENVIRONMENTS..]', 'Deploys the CDK toolkit stack into an AWS environment') .command('deploy [STACKS..]', 'Deploys the stack(s) named STACKS into your AWS account', yargs => yargs + .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' })) .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' })) .command('diff [STACK]', 'Compares the specified stack with the deployed stack or a local template file, and returns with status 1 if any difference is found', yargs => yargs .option('context-lines', { type: 'number', desc: 'number of context lines to include in arbitrary JSON diff rendering', default: 3 }) @@ -167,14 +170,14 @@ async function initCommandLine() { return await cliBootstrap(args.ENVIRONMENTS, toolkitStackName, args.roleArn); case 'deploy': - return await cliDeploy(args.STACKS, toolkitStackName, args.roleArn, configuration.combined.get(['requireApproval'])); + return await cliDeploy(args.STACKS, args.exclusively, toolkitStackName, args.roleArn, configuration.combined.get(['requireApproval'])); case 'destroy': - return await cliDestroy(args.STACKS, args.force, args.roleArn); + return await cliDestroy(args.STACKS, args.exclusively, args.force, args.roleArn); case 'synthesize': case 'synth': - return await cliSynthesize(args.STACKS, args.interactive, args.output, args.json, args.numbered); + return await cliSynthesize(args.STACKS, args.exclusively, args.interactive, args.output, args.json, args.numbered); case 'metadata': return await cliMetadata(await findStack(args.STACK)); @@ -239,11 +242,12 @@ async function initCommandLine() { * should be supplied, where the templates will be written. */ async function cliSynthesize(stackNames: string[], + exclusively: boolean, doInteractive: boolean, outputDir: string|undefined, json: boolean, numbered: boolean): Promise { - const stacks = await appStacks.selectStacks(...stackNames); + const stacks = await appStacks.selectStacks(stackNames, exclusively ? ExtendedStackSelection.None : ExtendedStackSelection.Upstream); renames.validateSelectedStacks(stacks); if (doInteractive) { @@ -300,10 +304,14 @@ async function initCommandLine() { return 0; // exit-code } - async function cliDeploy(stackNames: string[], toolkitStackName: string, roleArn: string | undefined, requireApproval: RequireApproval) { + async function cliDeploy(stackNames: string[], + exclusively: boolean, + toolkitStackName: string, + roleArn: string | undefined, + requireApproval: RequireApproval) { if (requireApproval === undefined) { requireApproval = RequireApproval.Broadening; } - const stacks = await appStacks.selectStacks(...stackNames); + const stacks = await appStacks.selectStacks(stackNames, exclusively ? ExtendedStackSelection.None : ExtendedStackSelection.Upstream); renames.validateSelectedStacks(stacks); for (const stack of stacks) { @@ -364,8 +372,8 @@ async function initCommandLine() { } } - async function cliDestroy(stackNames: string[], force: boolean, roleArn: string | undefined) { - const stacks = await appStacks.selectStacks(...stackNames); + async function cliDestroy(stackNames: string[], exclusively: boolean, force: boolean, roleArn: string | undefined) { + const stacks = await appStacks.selectStacks(stackNames, exclusively ? ExtendedStackSelection.None : ExtendedStackSelection.Downstream); // The stacks will have been ordered for deployment, so reverse them for deletion. stacks.reverse(); @@ -438,7 +446,7 @@ async function initCommandLine() { * Match a single stack from the list of available stacks */ async function findStack(name: string): Promise { - const stacks = await appStacks.selectStacks(name); + const stacks = await appStacks.selectStacks([name], ExtendedStackSelection.None); // Could have been a glob so check that we evaluated to exactly one if (stacks.length > 1) { diff --git a/packages/aws-cdk/integ-tests/test-cdk-order.sh b/packages/aws-cdk/integ-tests/test-cdk-order.sh index 410f1adc90478..3825579eac07e 100755 --- a/packages/aws-cdk/integ-tests/test-cdk-order.sh +++ b/packages/aws-cdk/integ-tests/test-cdk-order.sh @@ -6,10 +6,10 @@ source ${scriptdir}/common.bash setup -# Deploy all stacks with an ordering component between them -cdk deploy cdk-toolkit-integration-order-\* +# Deploy the consuming stack which will include the producing stack +cdk deploy cdk-toolkit-integration-order-consuming -# Destroy them again -cdk destroy -f cdk-toolkit-integration-order-\* +# Destroy the providing stack which will include the consuming stack +cdk destroy -f cdk-toolkit-integration-order-providing echo "✅ success" diff --git a/packages/aws-cdk/lib/api/cxapp/environments.ts b/packages/aws-cdk/lib/api/cxapp/environments.ts index a744622f29cde..ff63349b85e15 100644 --- a/packages/aws-cdk/lib/api/cxapp/environments.ts +++ b/packages/aws-cdk/lib/api/cxapp/environments.ts @@ -1,12 +1,12 @@ import cxapi = require('@aws-cdk/cx-api'); import minimatch = require('minimatch'); -import { AppStacks } from './stacks'; +import { AppStacks, ExtendedStackSelection } from './stacks'; export async function globEnvironmentsFromStacks(appStacks: AppStacks, environmentGlobs: string[]): Promise { if (environmentGlobs.length === 0) { environmentGlobs = [ '**' ]; // default to ALL } - const stacks = await appStacks.selectStacks(); + const stacks = await appStacks.selectStacks([], ExtendedStackSelection.None); const availableEnvironments = distinct(stacks.map(stack => stack.environment) .filter(env => env !== undefined) as cxapi.Environment[]); diff --git a/packages/aws-cdk/lib/api/cxapp/stacks.ts b/packages/aws-cdk/lib/api/cxapp/stacks.ts index 0e1a316b0264d..b6def12806b04 100644 --- a/packages/aws-cdk/lib/api/cxapp/stacks.ts +++ b/packages/aws-cdk/lib/api/cxapp/stacks.ts @@ -1,4 +1,5 @@ import cxapi = require('@aws-cdk/cx-api'); +import colors = require('colors/safe'); import minimatch = require('minimatch'); import yargs = require('yargs'); import contextproviders = require('../../context-providers'); @@ -30,7 +31,7 @@ export class AppStacks { * It's an error if there are no stacks to select, or if one of the requested parameters * refers to a nonexistant stack. */ - public async selectStacks(...selectors: string[]): Promise { + public async selectStacks(selectors: string[], extendedSelection: ExtendedStackSelection): Promise { selectors = selectors.filter(s => s != null); // filter null/undefined const stacks: cxapi.SynthesizedStack[] = await this.listStacks(); @@ -43,14 +44,19 @@ export class AppStacks { return stacks; } + const allStacks = new Map(); + for (const stack of stacks) { + allStacks.set(stack.name, stack); + } + // For every selector argument, pick stacks from the list. - const matched = new Set(); + const selectedStacks = new Map(); for (const pattern of selectors) { let found = false; for (const stack of stacks) { - if (minimatch(stack.name, pattern)) { - matched.add(stack.name); + if (minimatch(stack.name, pattern) && !selectedStacks.has(stack.name)) { + selectedStacks.set(stack.name, stack); found = true; } } @@ -60,7 +66,17 @@ export class AppStacks { } } - return stacks.filter(s => matched.has(s.name)); + switch (extendedSelection) { + case ExtendedStackSelection.Downstream: + includeDownstreamStacks(selectedStacks, allStacks); + break; + case ExtendedStackSelection.Upstream: + includeUpstreamStacks(selectedStacks, allStacks); + break; + } + + // Filter original array because it is in the right order + return stacks.filter(s => selectedStacks.has(s.name)); } /** @@ -205,4 +221,79 @@ export class AppStacks { */ export function listStackNames(stacks: cxapi.SynthesizedStack[]): string { return stacks.map(s => s.name).join(', '); +} + +/** + * When selecting stacks, what other stacks to include because of dependencies + */ +export enum ExtendedStackSelection { + /** + * Don't select any extra stacks + */ + None, + + /** + * Include stacks that this stack depends on + */ + Upstream, + + /** + * Include stacks that depend on this stack + */ + Downstream +} + +/** + * Include stacks that depend on the stacks already in the set + * + * Modifies `selectedStacks` in-place. + */ +function includeDownstreamStacks(selectedStacks: Map, allStacks: Map) { + const added = new Array(); + + let madeProgress = true; + while (madeProgress) { + madeProgress = false; + + for (const [name, stack] of allStacks) { + // Select this stack if it's not selected yet AND it depends on a stack that's in the selected set + if (!selectedStacks.has(name) && (stack.dependsOn || []).some(dependencyName => selectedStacks.has(dependencyName))) { + selectedStacks.set(name, stack); + added.push(name); + madeProgress = true; + } + } + } + + if (added.length > 0) { + print('Including depending stacks: %s', colors.bold(added.join(', '))); + } +} + +/** + * Include stacks that that stacks in the set depend on + * + * Modifies `selectedStacks` in-place. + */ +function includeUpstreamStacks(selectedStacks: Map, allStacks: Map) { + const added = new Array(); + let madeProgress = true; + while (madeProgress) { + madeProgress = false; + + for (const stack of selectedStacks.values()) { + // Select an additional stack if it's not selected yet and a dependency of a selected stack (and exists, obviously) + for (const dependencyName of (stack.dependsOn || [])) { + if (!selectedStacks.has(dependencyName) && allStacks.has(dependencyName)) { + added.push(dependencyName); + selectedStacks.set(dependencyName, allStacks.get(dependencyName)!); + madeProgress = true; + } + } + } + } + + if (added.length > 0) { + print('Including dependency stacks: %s', colors.bold(added.join(', '))); + } } \ No newline at end of file