Skip to content

Commit

Permalink
fix(cli): diff against multiple stacks do not always fail if any have…
Browse files Browse the repository at this point in the history
… a diff (aws#7690)

### Commit Message
Currently, the diff command only remembers if there are any diffs
with the last stack that was diffed. So, if all of your stacks have
a diff except for the last one, the diff command would exit
successfully even if told to fail on diff.

This fix corrects the behavior so that if any of the stacks have
a diff, then the exit code would be non-zero if configured to do so.

fixes aws#7492
### End Commit Message

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
polothy authored and Curtis Eppel committed Aug 11, 2020
1 parent 3791995 commit 03be0f3
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 2 deletions.
2 changes: 1 addition & 1 deletion packages/aws-cdk/lib/cdk-toolkit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ export class CdkToolkit {
for (const stack of stacks.stackArtifacts) {
stream.write(format('Stack %s\n', colors.bold(stack.displayName)));
const currentTemplate = await this.props.cloudFormation.readCurrentTemplate(stack);
diffs = printStackDiff(currentTemplate, stack, strict, contextLines, stream);
diffs += printStackDiff(currentTemplate, stack, strict, contextLines, stream);
}
}

Expand Down
27 changes: 26 additions & 1 deletion packages/aws-cdk/test/diff.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { Writable } from 'stream';
import { NodeStringDecoder, StringDecoder } from 'string_decoder';
import * as cxschema from '@aws-cdk/cloud-assembly-schema';
import { CloudFormationStackArtifact } from '@aws-cdk/cx-api';
import { CloudFormationDeployments } from '../lib/api/cloudformation-deployments';
import { CdkToolkit } from '../lib/cdk-toolkit';
import { classMockOf, MockCloudExecutable } from './util';
Expand Down Expand Up @@ -31,6 +32,10 @@ beforeEach(() => {
},
],
},
},
{
stackName: 'D',
template: { resource: 'D' },
}],
});

Expand All @@ -44,7 +49,12 @@ beforeEach(() => {
});

// Default implementations
cloudFormation.readCurrentTemplate.mockResolvedValue({});
cloudFormation.readCurrentTemplate.mockImplementation((stackArtifact: CloudFormationStackArtifact) => {
if (stackArtifact.stackName === 'D') {
return Promise.resolve({ resource: 'D' });
}
return Promise.resolve({});
});
cloudFormation.deployStack.mockImplementation((options) => Promise.resolve({
noOp: true,
outputs: {},
Expand Down Expand Up @@ -86,6 +96,21 @@ test('exits with 1 with diffs and fail set to true', async () => {
expect(exitCode).toBe(1);
});

test('exits with 1 with diff in first stack, but not in second stack and fail set to true', async () => {
// GIVEN
const buffer = new StringWritable();

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

// THEN
expect(exitCode).toBe(1);
});

test('throws an error during diffs on stack with error metadata', async () => {
const buffer = new StringWritable();

Expand Down
26 changes: 26 additions & 0 deletions packages/aws-cdk/test/integ/cli/cli.integtest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,32 @@ integTest('cdk diff', async () => {
.rejects.toThrow('exited with error');
});

integTest('cdk diff --fail on multiple stacks exits with error if any of the stacks contains a diff', async () => {
// GIVEN
const diff1 = await cdk(['diff', fullStackName('test-1')]);
expect(diff1).toContain('AWS::SNS::Topic');

await cdkDeploy('test-2');
const diff2 = await cdk(['diff', fullStackName('test-2')]);
expect(diff2).toContain('There were no differences');

// WHEN / THEN
await expect(cdk(['diff', '--fail', fullStackName('test-1'), fullStackName('test-2')])).rejects.toThrow('exited with error');
});

integTest('cdk diff --fail with multiple stack exits with if any of the stacks contains a diff', async () => {
// GIVEN
await cdkDeploy('test-1');
const diff1 = await cdk(['diff', fullStackName('test-1')]);
expect(diff1).toContain('There were no differences');

const diff2 = await cdk(['diff', fullStackName('test-2')]);
expect(diff2).toContain('AWS::SNS::Topic');

// WHEN / THEN
await expect(cdk(['diff', '--fail', fullStackName('test-1'), fullStackName('test-2')])).rejects.toThrow('exited with error');
});

integTest('deploy stack with docker asset', async () => {
await cdkDeploy('docker');
});
Expand Down

0 comments on commit 03be0f3

Please sign in to comment.