From 55110761da2a182bdb7828ce669c74092c855b0e Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Fri, 12 Oct 2018 11:31:34 +0300 Subject: [PATCH] fix(toolkit): multi-stack apps cannot be synthesized or deployed (#911) Due to a recent cx protocol change (#868), some toolkit commands stopped respecting the "selected" stacks (the ones specified in the command line). "cdk synth" would always return the first stack, and "cdk deploy" would always deploy all stacks. Since we have test coverage gaps in the toolkit (#294), we did not discover this before we released. This change includes an initial set of integration tests for the toolkit. At the moment they should be manually executed when toolkit changes are made, but we will execute them in a pipeline. Fixes #910 --- packages/aws-cdk/bin/cdk.ts | 52 ++++++------- packages/aws-cdk/integ-tests/README.md | 4 + packages/aws-cdk/integ-tests/app/.gitignore | 1 + packages/aws-cdk/integ-tests/app/app.js | 24 ++++++ packages/aws-cdk/integ-tests/app/cdk.json | 4 + packages/aws-cdk/integ-tests/common.bash | 73 +++++++++++++++++++ .../integ-tests/test-cdk-deploy-all.sh | 21 ++++++ .../aws-cdk/integ-tests/test-cdk-deploy.sh | 26 +++++++ packages/aws-cdk/integ-tests/test-cdk-diff.sh | 16 ++++ packages/aws-cdk/integ-tests/test-cdk-ls.sh | 14 ++++ .../aws-cdk/integ-tests/test-cdk-synth.sh | 25 +++++++ packages/aws-cdk/integ-tests/test.sh | 21 ++++++ 12 files changed, 253 insertions(+), 28 deletions(-) create mode 100644 packages/aws-cdk/integ-tests/README.md create mode 100644 packages/aws-cdk/integ-tests/app/.gitignore create mode 100644 packages/aws-cdk/integ-tests/app/app.js create mode 100644 packages/aws-cdk/integ-tests/app/cdk.json create mode 100644 packages/aws-cdk/integ-tests/common.bash create mode 100755 packages/aws-cdk/integ-tests/test-cdk-deploy-all.sh create mode 100755 packages/aws-cdk/integ-tests/test-cdk-deploy.sh create mode 100755 packages/aws-cdk/integ-tests/test-cdk-diff.sh create mode 100755 packages/aws-cdk/integ-tests/test-cdk-ls.sh create mode 100755 packages/aws-cdk/integ-tests/test-cdk-synth.sh create mode 100755 packages/aws-cdk/integ-tests/test.sh diff --git a/packages/aws-cdk/bin/cdk.ts b/packages/aws-cdk/bin/cdk.ts index b853e17cc0961..e990910be1e38 100644 --- a/packages/aws-cdk/bin/cdk.ts +++ b/packages/aws-cdk/bin/cdk.ts @@ -270,8 +270,8 @@ async function initCommandLine() { if (environmentGlobs.length === 0) { environmentGlobs = [ '**' ]; // default to ALL } - const stackInfos = await selectStacks(); - const availableEnvironments = distinct(stackInfos.map(stack => stack.environment) + const stacks = await selectStacks(); + const availableEnvironments = distinct(stacks.map(stack => stack.environment) .filter(env => env !== undefined) as cxapi.Environment[]); const environments = availableEnvironments.filter(env => environmentGlobs.find(glob => minimatch(env!.name, glob))); if (environments.length === 0) { @@ -324,31 +324,28 @@ async function initCommandLine() { doInteractive: boolean, outputDir: string|undefined, json: boolean): Promise { - const stackIds = await selectStacks(...stackNames); - renames.validateSelectedStacks(stackIds); + const stacks = await selectStacks(...stackNames); + renames.validateSelectedStacks(stacks); if (doInteractive) { - if (stackIds.length !== 1) { - throw new Error(`When using interactive synthesis, must select exactly one stack. Got: ${listStackNames(stackIds)}`); + if (stacks.length !== 1) { + throw new Error(`When using interactive synthesis, must select exactly one stack. Got: ${listStackNames(stacks)}`); } - return await interactive(stackIds[0], argv.verbose, (stack) => synthesizeStack(stack)); + return await interactive(stacks[0], argv.verbose, (stack) => synthesizeStack(stack)); } - if (stackIds.length > 1 && outputDir == null) { + if (stacks.length > 1 && outputDir == null) { // tslint:disable-next-line:max-line-length - throw new Error(`Multiple stacks selected (${listStackNames(stackIds)}), but output is directed to stdout. Either select one stack, or use --output to send templates to a directory.`); + throw new Error(`Multiple stacks selected (${listStackNames(stacks)}), but output is directed to stdout. Either select one stack, or use --output to send templates to a directory.`); } - const response = await synthesizeStacks(); - const synthesizedStacks = response.stacks; - if (outputDir == null) { - return synthesizedStacks[0].template; // Will be printed in main() + return stacks[0].template; // Will be printed in main() } fs.mkdirpSync(outputDir); - for (const stack of synthesizedStacks) { + for (const stack of stacks) { const finalName = renames.finalName(stack.name); const fileName = `${outputDir}/${finalName}.template.${json ? 'json' : 'yaml'}`; highlight(fileName); @@ -579,13 +576,11 @@ async function initCommandLine() { } async function cliDeploy(stackNames: string[], toolkitStackName: string) { - const stackIds = await selectStacks(...stackNames); - renames.validateSelectedStacks(stackIds); + const stacks = await selectStacks(...stackNames); + renames.validateSelectedStacks(stacks); - const response = await synthesizeStacks(); - - for (const stack of response.stacks) { - if (stackIds.length !== 1) { highlight(stack.name); } + for (const stack of stacks) { + if (stacks.length !== 1) { highlight(stack.name); } if (!stack.environment) { // tslint:disable-next-line:max-line-length throw new Error(`Stack ${stack.name} does not define an environment, and AWS credentials could not be obtained from standard locations or no region was configured.`); @@ -603,6 +598,7 @@ async function initCommandLine() { const result = await deployStack(stack, aws, toolkitInfo, deployName); const message = result.noOp ? ` ✅ Stack was already up-to-date, it has ARN ${colors.blue(result.stackArn)}` : ` ✅ Deployment of stack %s completed successfully, it has ARN ${colors.blue(result.stackArn)}`; + data(result.stackArn); success(message, colors.blue(stack.name)); for (const name of Object.keys(result.outputs)) { const value = result.outputs[name]; @@ -616,18 +612,18 @@ async function initCommandLine() { } async function cliDestroy(stackNames: string[], force: boolean) { - const stackIds = await selectStacks(...stackNames); - renames.validateSelectedStacks(stackIds); + const stacks = await selectStacks(...stackNames); + renames.validateSelectedStacks(stacks); if (!force) { // tslint:disable-next-line:max-line-length - const confirmed = await util.promisify(promptly.confirm)(`Are you sure you want to delete: ${colors.blue(stackIds.map(s => s.name).join(', '))} (y/n)?`); + const confirmed = await util.promisify(promptly.confirm)(`Are you sure you want to delete: ${colors.blue(stacks.map(s => s.name).join(', '))} (y/n)?`); if (!confirmed) { return; } } - for (const stack of stackIds) { + for (const stack of stacks) { const deployName = renames.finalName(stack.name); success(' ⏳ Starting destruction of stack %s...', colors.blue(deployName)); @@ -687,14 +683,14 @@ async function initCommandLine() { * Match a single stack from the list of available stacks */ async function findStack(name: string): Promise { - const stackIds = await selectStacks(name); + const stacks = await selectStacks(name); // Could have been a glob so check that we evaluated to exactly one - if (stackIds.length > 1) { - throw new Error(`This command requires exactly one stack and we matched more than one: ${stackIds.map(x => x.name)}`); + if (stacks.length > 1) { + throw new Error(`This command requires exactly one stack and we matched more than one: ${stacks.map(x => x.name)}`); } - return stackIds[0].name; + return stacks[0].name; } function logDefaults() { diff --git a/packages/aws-cdk/integ-tests/README.md b/packages/aws-cdk/integ-tests/README.md new file mode 100644 index 0000000000000..425d53b1bfe84 --- /dev/null +++ b/packages/aws-cdk/integ-tests/README.md @@ -0,0 +1,4 @@ +# CDK toolkit integreation tests + +To run, just execute `./test.sh`. The test uses the default AWS credentials. + diff --git a/packages/aws-cdk/integ-tests/app/.gitignore b/packages/aws-cdk/integ-tests/app/.gitignore new file mode 100644 index 0000000000000..d4aa116a26c73 --- /dev/null +++ b/packages/aws-cdk/integ-tests/app/.gitignore @@ -0,0 +1 @@ +!*.js diff --git a/packages/aws-cdk/integ-tests/app/app.js b/packages/aws-cdk/integ-tests/app/app.js new file mode 100644 index 0000000000000..5ec721079c981 --- /dev/null +++ b/packages/aws-cdk/integ-tests/app/app.js @@ -0,0 +1,24 @@ +const cdk = require('@aws-cdk/cdk'); +const sns = require('@aws-cdk/aws-sns'); + +class MyStack extends cdk.Stack { + constructor(parent, id) { + super(parent, id); + new sns.Topic(this, 'topic'); + } +} + +class YourStack extends cdk.Stack { + constructor(parent, id) { + super(parent, id); + new sns.Topic(this, 'topic1'); + new sns.Topic(this, 'topic2'); + } +} + +const app = new cdk.App(); + +new MyStack(app, 'cdk-toolkit-integration-test-1'); +new YourStack(app, 'cdk-toolkit-integration-test-2'); + +app.run(); \ No newline at end of file diff --git a/packages/aws-cdk/integ-tests/app/cdk.json b/packages/aws-cdk/integ-tests/app/cdk.json new file mode 100644 index 0000000000000..f0075b1c9e33b --- /dev/null +++ b/packages/aws-cdk/integ-tests/app/cdk.json @@ -0,0 +1,4 @@ +{ + "app": "node app.js", + "versionReporting": false +} diff --git a/packages/aws-cdk/integ-tests/common.bash b/packages/aws-cdk/integ-tests/common.bash new file mode 100644 index 0000000000000..035c22c33a68b --- /dev/null +++ b/packages/aws-cdk/integ-tests/common.bash @@ -0,0 +1,73 @@ +function cleanup_stack() { + local stack_arn=$1 + echo "| ensuring ${stack_arn} is cleaned up" + if aws cloudformation describe-stacks --stack-name ${stack_arn} 2> /dev/null; then + aws cloudformation delete-stack --stack-name ${stack_arn} + fi +} + +function cleanup() { + cleanup_stack cdk-toolkit-integration-test-1 + cleanup_stack cdk-toolkit-integration-test-2 +} + +function setup() { + cleanup + cd app + + npm i --no-save @aws-cdk/cdk @aws-cdk/aws-sns +} + +function fail() { + echo "❌ $@" + exit 1 +} + +function assert_diff() { + local test=$1 + local actual=$2 + local expected=$3 + + diff ${actual} ${expected} || { + echo + echo "+-----------" + echo "| expected:" + cat ${expected} + echo "|--" + echo + echo "+-----------" + echo "| actual:" + cat ${actual} + echo "|--" + echo + fail "assertion failed. ${test}" + } +} + +function assert() { + local command="$1" + + local expected=$(mktemp) + local actual=$(mktemp) + + echo "| running ${command}" + + $command > ${actual} || { + fail "command ${command} non-zero exit code" + } + + cat > ${expected} + + assert_diff "command: ${command}" "${actual}" "${expected}" +} + +function assert_lines() { + local data="$1" + local expected="$2" + echo "| assert that last command returned ${expected} line(s)" + + local lines="$(echo "${data}" | wc -l)" + if [ "${lines}" -ne "${expected}" ]; then + fail "response has ${lines} lines and we expected ${expected} lines to be returned" + fi +} diff --git a/packages/aws-cdk/integ-tests/test-cdk-deploy-all.sh b/packages/aws-cdk/integ-tests/test-cdk-deploy-all.sh new file mode 100755 index 0000000000000..2621f03357e55 --- /dev/null +++ b/packages/aws-cdk/integ-tests/test-cdk-deploy-all.sh @@ -0,0 +1,21 @@ +#!/bin/bash +set -euo pipefail +scriptdir=$(cd $(dirname $0) && pwd) +source ${scriptdir}/common.bash +# ---------------------------------------------------------- + +setup + +stack_arns=$(cdk deploy) +echo "Stack deployed successfully" + +# verify that we only deployed a single stack (there's a single ARN in the output) +lines="$(echo "${stack_arns}" | wc -l)" +if [ "${lines}" -ne 2 ]; then + fail "cdk deploy returned ${lines} arns and we expected 2" +fi + +cdk destroy -f cdk-toolkit-integration-test-1 +cdk destroy -f cdk-toolkit-integration-test-2 + +echo "✅ success" diff --git a/packages/aws-cdk/integ-tests/test-cdk-deploy.sh b/packages/aws-cdk/integ-tests/test-cdk-deploy.sh new file mode 100755 index 0000000000000..a44253ec9c375 --- /dev/null +++ b/packages/aws-cdk/integ-tests/test-cdk-deploy.sh @@ -0,0 +1,26 @@ +#!/bin/bash +set -euo pipefail +scriptdir=$(cd $(dirname $0) && pwd) +source ${scriptdir}/common.bash +# ---------------------------------------------------------- + +setup + +stack_arn=$(cdk deploy cdk-toolkit-integration-test-2) +echo "Stack deployed successfully" + +# verify that we only deployed a single stack (there's a single ARN in the output) +assert_lines "${stack_arn}" 1 + +# verify the number of resources in the stack +response_json=$(mktemp).json +aws cloudformation describe-stack-resources --stack-name ${stack_arn} > ${response_json} +resource_count=$(node -e "console.log(require('${response_json}').StackResources.length)") +if [ "${resource_count}" -ne 2 ]; then + fail "stack has ${resource_count} resources, and we expected two" +fi + +# destroy +cdk destroy -f cdk-toolkit-integration-test-2 + +echo "✅ success" diff --git a/packages/aws-cdk/integ-tests/test-cdk-diff.sh b/packages/aws-cdk/integ-tests/test-cdk-diff.sh new file mode 100755 index 0000000000000..5b117c1645a9f --- /dev/null +++ b/packages/aws-cdk/integ-tests/test-cdk-diff.sh @@ -0,0 +1,16 @@ +#!/bin/bash +set -euo pipefail +scriptdir=$(cd $(dirname $0) && pwd) +source ${scriptdir}/common.bash +# ---------------------------------------------------------- + +setup + +function cdk_diff() { + cdk diff $1 2>&1 || true +} + +assert_lines "$(cdk_diff cdk-toolkit-integration-test-1)" 1 +assert_lines "$(cdk_diff cdk-toolkit-integration-test-2)" 2 + +echo "✅ success" diff --git a/packages/aws-cdk/integ-tests/test-cdk-ls.sh b/packages/aws-cdk/integ-tests/test-cdk-ls.sh new file mode 100755 index 0000000000000..6ec85bb01d4cb --- /dev/null +++ b/packages/aws-cdk/integ-tests/test-cdk-ls.sh @@ -0,0 +1,14 @@ +#!/bin/bash +set -euo pipefail +scriptdir=$(cd $(dirname $0) && pwd) +source ${scriptdir}/common.bash +# ---------------------------------------------------------- + +setup + +assert "cdk ls" <