Skip to content

Commit

Permalink
Revert "fix(cli): always exit with 0 on cdk diff (#4650)" (#4708)
Browse files Browse the repository at this point in the history
This change includes a breaking change in our stable APIs.
We will have to discuss this furthre in order to make a decision
to actually break.

This reverts commit 4f765b2.
  • Loading branch information
Elad Ben-Israel authored Oct 28, 2019
1 parent d7654e7 commit 7d8a68a
Show file tree
Hide file tree
Showing 5 changed files with 13 additions and 49 deletions.
4 changes: 1 addition & 3 deletions packages/aws-cdk/bin/cdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ async function parseCommandLineArguments() {
.option('context-lines', { type: 'number', desc: 'Number of context lines to include in arbitrary JSON diff rendering', default: 3, requiresArg: true })
.option('template', { type: 'string', desc: 'The path to the CloudFormation template to compare with', requiresArg: true })
.option('strict', { type: 'boolean', desc: 'Do not filter out AWS::CDK::Metadata resources', default: false }))
.option('fail', { type: 'boolean', desc: 'Fail with exit code 1 in case of diff', default: false })
.command('metadata [STACK]', 'Returns all metadata associated with this stack')
.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 })
Expand Down Expand Up @@ -184,8 +183,7 @@ async function initCommandLine() {
exclusively: args.exclusively,
templatePath: args.template,
strict: args.strict,
contextLines: args.contextLines,
fail: args.fail
contextLines: args.contextLines
});

case 'bootstrap':
Expand Down
17 changes: 6 additions & 11 deletions packages/aws-cdk/lib/cdk-toolkit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export class CdkToolkit {
const contextLines = options.contextLines || 3;
const stream = options.stream || process.stderr;

let diffs = 0;
let ret = 0;
if (options.templatePath !== undefined) {
// Compare single stack against fixed template
if (stacks.length !== 1) {
Expand All @@ -62,17 +62,19 @@ export class CdkToolkit {
throw new Error(`There is no file at ${options.templatePath}`);
}
const template = deserializeStructure(await fs.readFile(options.templatePath, { encoding: 'UTF-8' }));
diffs = printStackDiff(template, stacks[0], strict, contextLines, stream);
ret = printStackDiff(template, stacks[0], strict, contextLines, options.stream);
} else {
// Compare N stacks against deployed templates
for (const stack of stacks) {
stream.write(format('Stack %s\n', colors.bold(stack.name)));
const currentTemplate = await this.provisioner.readCurrentTemplate(stack);
diffs = printStackDiff(currentTemplate, stack, strict, contextLines, stream);
if (printStackDiff(currentTemplate, stack, !!options.strict, options.contextLines || 3, stream) !== 0) {
ret = 1;
}
}
}

return diffs && options.fail ? 1 : 0;
return ret;
}

public async deploy(options: DeployOptions) {
Expand Down Expand Up @@ -245,13 +247,6 @@ export interface DiffOptions {
* @default stderr
*/
stream?: NodeJS.WritableStream;

/**
* Whether to fail with exit code 1 in case of diff
*
* @default false
*/
fail?: boolean;
}

export interface DeployOptions {
Expand Down
13 changes: 5 additions & 8 deletions packages/aws-cdk/test/integ/cli/test-cdk-diff.sh
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,11 @@ source ${scriptdir}/common.bash

setup

cdk diff ${STACK_NAME_PREFIX}-test-1 2>&1 | grep "AWS::SNS::Topic"
cdk diff ${STACK_NAME_PREFIX}-test-2 2>&1 | grep "AWS::SNS::Topic"
function cdk_diff() {
cdk diff $1 2>&1 || true
}

fail=0
cdk diff --fail ${STACK_NAME_PREFIX}-test-1 2>&1 || fail=1

if [ $fail -ne 1 ]; then
fail 'cdk diff with --fail does not fail'
fi
cdk_diff ${STACK_NAME_PREFIX}-test-1 | grep "AWS::SNS::Topic"
cdk_diff ${STACK_NAME_PREFIX}-test-2 | grep "AWS::SNS::Topic"

echo "✅ success"
2 changes: 1 addition & 1 deletion packages/aws-cdk/test/integ/cli/test-cdk-iam-diff.sh
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ source ${scriptdir}/common.bash
setup

function nonfailing_diff() {
cdk diff $1 2>&1 | strip_color_codes
( cdk diff $1 2>&1 || true ) | strip_color_codes
}

assert "nonfailing_diff ${STACK_NAME_PREFIX}-iam-test" <<HERE
Expand Down
26 changes: 0 additions & 26 deletions packages/aws-cdk/test/test.diff.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,32 +53,6 @@ export = {
test.ok(plainTextOutput.indexOf('Stack A') > -1, `Did not contain "Stack A": ${plainTextOutput}`);
test.ok(plainTextOutput.indexOf('Stack B') > -1, `Did not contain "Stack B": ${plainTextOutput}`);

test.equals(0, exitCode);

test.done();
},

async 'exits with 1 with diffs and fail set to true'(test: Test) {
// GIVEN
const provisioner: IDeploymentTarget = {
async readCurrentTemplate(_stack: cxapi.CloudFormationStackArtifact): Promise<Template> {
return {};
},
async deployStack(_options: DeployStackOptions): Promise<DeployStackResult> {
return { noOp: true, outputs: {}, stackArn: ''};
}
};
const toolkit = new CdkToolkit({ appStacks, provisioner });
const buffer = new StringWritable();

// WHEN
const exitCode = await toolkit.diff({
stackNames: ['A'],
stream: buffer,
fail: true
});

// THEN
test.equals(1, exitCode);

test.done();
Expand Down

0 comments on commit 7d8a68a

Please sign in to comment.