Skip to content

Commit

Permalink
fix(cli): cdk bootstrap is broken due to --no-execute (#5092)
Browse files Browse the repository at this point in the history
The change that introduced the `--no-execute` feature (#4852) did not take into account that `cdk bootstrap` uses the same code path for deployment, and therefore `execute` was undefined, and resolved to false (despite the documented default).
This change moves `execute` switch from the global scope into `deploy` and to `bootstrap` and passes it into the `cliBootstrap` command, as well as respects the default in case the option passed to the deployment utility is `undefined`.
It also emits a message to STDOUT in case `--no-execute` is provided to let user know that deployment hasn't really finished. Otherwise, it appears as if deployment is successful.

The `deploy-no-execute` integration test was also "failing successfully" due to invalid usage of bash conditions, so this is also fixed here.

Added integration test to verify that --no-execute is respected by `cdk bootstrap`.
  • Loading branch information
Elad Ben-Israel authored and mergify[bot] committed Nov 19, 2019
1 parent 2d92ab3 commit 7acc588
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 12 deletions.
19 changes: 13 additions & 6 deletions packages/aws-cdk/bin/cdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,21 +45,26 @@ async function parseCommandLineArguments() {
.option('output', { type: 'string', alias: 'o', desc: 'Emits the synthesized cloud assembly into a directory (default: cdk.out)', requiresArg: true })
.option('no-color', { type: 'boolean', desc: 'Removes colors and other style from console output', default: false })
.command([ 'list [STACKS..]', 'ls [STACKS..]' ], 'Lists all stacks in the app', yargs => yargs
.option('long', { type: 'boolean', default: false, alias: 'l', desc: 'Display environment information for each stack' }))
.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('exclusively', { type: 'boolean', alias: 'e', desc: 'Only deploy requested stacks, don\'t include dependencies' })
)
.command('bootstrap [ENVIRONMENTS..]', 'Deploys the CDK toolkit stack into an AWS environment', yargs => yargs
.option('bootstrap-bucket-name', { type: 'string', alias: ['b', 'toolkit-bucket-name'], desc: 'The name of the CDK toolkit bucket', default: undefined })
.option('bootstrap-kms-key-id', { type: 'string', desc: 'AWS KMS master key ID used for the SSE-KMS encryption', default: undefined })
.option('tags', { type: 'array', alias: 't', desc: 'Tags to add for the stack (KEY=VALUE)', nargs: 1, requiresArg: true, default: [] }))
.option('tags', { type: 'array', alias: 't', desc: 'Tags to add for the stack (KEY=VALUE)', nargs: 1, requiresArg: true, default: [] })
.option('execute', {type: 'boolean', desc: 'Whether to execute ChangeSet (--no-execute will NOT execute the ChangeSet)', default: true})
)
.command('deploy [STACKS..]', 'Deploys the stack(s) named STACKS into your AWS account', yargs => yargs
.option('build-exclude', { type: 'array', alias: 'E', nargs: 1, desc: 'Do not rebuild asset with the given ID. Can be specified multiple times.', default: [] })
.option('exclusively', { type: 'boolean', alias: 'e', desc: 'Only deploy requested stacks, don\'t include dependencies' })
.option('require-approval', { type: 'string', choices: [RequireApproval.Never, RequireApproval.AnyChange, RequireApproval.Broadening], desc: 'What security-sensitive changes need manual approval' })
.option('ci', { type: 'boolean', desc: 'Force CI detection. Use --no-ci to disable CI autodetection.', default: process.env.CI !== undefined })
.option('notification-arns', {type: 'array', desc: 'ARNs of SNS topics that CloudFormation will notify with stack related events', nargs: 1, requiresArg: true})
.option('tags', { type: 'array', alias: 't', desc: 'Tags to add to the stack (KEY=VALUE)', nargs: 1, requiresArg: true }))
.option('tags', { type: 'array', alias: 't', desc: 'Tags to add to the stack (KEY=VALUE)', nargs: 1, requiresArg: true })
.option('execute', {type: 'boolean', desc: 'Whether to execute ChangeSet (--no-execute will NOT execute the ChangeSet)', default: true})
)
.command('destroy [STACKS..]', 'Destroy the stack(s) named STACKS', yargs => yargs
.option('exclusively', { type: 'boolean', alias: 'e', 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' }))
Expand All @@ -72,7 +77,8 @@ async function parseCommandLineArguments() {
.command('init [TEMPLATE]', 'Create a new, empty CDK project from a template. Invoked without TEMPLATE, the app template will be used.', yargs => yargs
.option('language', { type: 'string', alias: 'l', desc: 'The language to be used for the new project (default can be configured in ~/.cdk.json)', choices: initTemplateLanuages })
.option('list', { type: 'boolean', desc: 'List the available templates' })
.option('generate-only', { type: 'boolean', default: false, desc: 'If true, only generates project files, without executing additional operations such as setting up a git repo, installing dependencies or compiling the project'}))
.option('generate-only', { type: 'boolean', default: false, desc: 'If true, only generates project files, without executing additional operations such as setting up a git repo, installing dependencies or compiling the project'})
)
.commandDir('../lib/commands', { exclude: /^_.*/ })
.version(version.DISPLAY_VERSION)
.demandCommand(1, '') // just print help
Expand Down Expand Up @@ -193,7 +199,8 @@ async function initCommandLine() {
return await cliBootstrap(args.ENVIRONMENTS, toolkitStackName, args.roleArn, {
bucketName: configuration.settings.get(['toolkitBucket', 'bucketName']),
kmsKeyId: configuration.settings.get(['toolkitBucket', 'kmsKeyId']),
tags: configuration.settings.get(['tags'])
tags: configuration.settings.get(['tags']),
execute: args.execute
});

case 'deploy':
Expand Down
14 changes: 12 additions & 2 deletions packages/aws-cdk/lib/api/bootstrap-environment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,12 @@ export interface BootstrapEnvironmentProps {
*
* @default - None.
*/
tags?: Tag[];
readonly tags?: Tag[];
/**
* Whether to execute the changeset or only create it and leave it in review.
* @default true
*/
readonly execute?: boolean;
}

/** @experimental */
Expand Down Expand Up @@ -90,5 +95,10 @@ export async function bootstrapEnvironment(environment: cxapi.Environment, aws:
});

const assembly = builder.buildAssembly();
return await deployStack({ stack: assembly.getStackByName(toolkitStackName), sdk: aws, roleArn, tags: props.tags });
return await deployStack({
stack: assembly.getStackByName(toolkitStackName),
sdk: aws, roleArn,
tags: props.tags,
execute: props.execute
});
}
11 changes: 8 additions & 3 deletions packages/aws-cdk/lib/api/deploy-stack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ export interface DeployStackOptions {
ci?: boolean;
reuseAssets?: string[];
tags?: Tag[];

/**
* Whether to execute the changeset or leave it in review.
* @default true
*/
execute?: boolean;
}

Expand Down Expand Up @@ -93,7 +98,8 @@ export async function deployStack(options: DeployStackOptions): Promise<DeploySt
return { noOp: true, outputs: await getStackOutputs(cfn, deployName), stackArn: changeSet.StackId! };
}

if (options.execute) {
const execute = options.execute === undefined ? true : options.execute;
if (execute) {
debug('Initiating execution of changeset %s on stack %s', changeSetName, deployName);
await cfn.executeChangeSet({StackName: deployName, ChangeSetName: changeSetName}).promise();
// tslint:disable-next-line:max-line-length
Expand All @@ -105,8 +111,7 @@ export async function deployStack(options: DeployStackOptions): Promise<DeploySt
}
debug('Stack %s has completed updating', deployName);
} else {
debug('Entering no-execute workflow for ChangeSet %s on stack %s', changeSetName, deployName);
cfn.executeChangeSet();
print(`Changeset %s created and waiting in review for manual execution (--no-execute)`, changeSetName);
}
return { noOp: false, outputs: await getStackOutputs(cfn, deployName), stackArn: changeSet.StackId! };
}
Expand Down
25 changes: 25 additions & 0 deletions packages/aws-cdk/test/integ/cli/test-cdk-bootstrap-no-execute.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
#!/bin/bash
set -euo pipefail
scriptdir=$(cd $(dirname $0) && pwd)
source ${scriptdir}/common.bash
# ----------------------------------------------------------

setup

bootstrap_stack_name="toolkit-stack-1-${RANDOM}"

# deploy with --no-execute (leaves stack in review)
cdk bootstrap --toolkit-stack-name ${bootstrap_stack_name} --no-execute

response_json=$(mktemp).json
aws cloudformation describe-stacks --stack-name ${bootstrap_stack_name} > ${response_json}

stack_status=$(node -e "console.log(require('${response_json}').Stacks[0].StackStatus)")
if [ ! "${stack_status}" == "REVIEW_IN_PROGRESS" ]; then
fail "Expected stack to be in status REVIEW_IN_PROGRESS but got ${stack_status}"
fi

# destroy
aws cloudformation delete-stack --stack-name ${bootstrap_stack_name}

echo "✅ success"
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ response_json=$(mktemp).json
aws cloudformation describe-stacks --stack-name ${stack_arn} > ${response_json}

stack_status=$(node -e "console.log(require('${response_json}').Stacks[0].StackStatus)")
if [ "${stack_status}" -ne "REVIEW_IN_PROGRESS" ]; then
if [ ! "${stack_status}" == "REVIEW_IN_PROGRESS" ]; then
fail "Expected stack to be in status REVIEW_IN_PROGRESS but got ${stack_status}"
fi

Expand Down

0 comments on commit 7acc588

Please sign in to comment.