Skip to content

Commit

Permalink
fix(CLI): diff --template crashes (#29896)
Browse files Browse the repository at this point in the history
### Issue # (if applicable)

Closes #29890.

### Reason for this change

`cdk diff` crashes with `--template`.

### Description of changes

The addition of changeset logic had a leftover refactor that should not have been leftover (trying to pass a template directly instead of a stack artifact). Removes changeset creation code from fixed template mode, which should never create a changeset, and adds a unit test for fixed template diffs so we don't break this in the future.

### Description of how you validated changes

unit tests + manual testing. CLI integ tests will be added in a follow-up PR. 

### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
comcalvi authored Apr 24, 2024
1 parent 7360a88 commit 466f170
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 33 deletions.
35 changes: 2 additions & 33 deletions packages/aws-cdk/lib/cdk-toolkit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,41 +136,10 @@ export class CdkToolkit {
throw new Error(`There is no file at ${options.templatePath}`);
}

let changeSet = undefined;

if (options.changeSet) {
let stackExists = false;
try {
stackExists = await this.props.deployments.stackExists({
stack: stacks.firstStack,
deployName: stacks.firstStack.stackName,
tryLookupRole: true,
});
} catch (e: any) {
debug(e.message);
stream.write('Checking if the stack exists before creating the changeset has failed, will base the diff on template differences (run again with -v to see the reason)\n');
stackExists = false;
}

if (stackExists) {
changeSet = await createDiffChangeSet({
stack: stacks.firstStack,
uuid: uuid.v4(),
deployments: this.props.deployments,
willExecute: false,
sdkProvider: this.props.sdkProvider,
parameters: Object.assign({}, parameterMap['*'], parameterMap[stacks.firstStack.stackName]),
stream,
});
} else {
debug(`the stack '${stacks.firstStack.stackName}' has not been deployed to CloudFormation or describeStacks call failed, skipping changeset creation.`);
}
}

const template = deserializeStructure(await fs.readFile(options.templatePath, { encoding: 'UTF-8' }));
diffs = options.securityOnly
? numberFromBool(printSecurityDiff(template, stacks.firstStack, RequireApproval.Broadening, changeSet))
: printStackDiff(template, stacks.firstStack.template, strict, contextLines, quiet, changeSet, false, stream);
? numberFromBool(printSecurityDiff(template, stacks.firstStack, RequireApproval.Broadening, undefined))
: printStackDiff(template, stacks.firstStack, strict, contextLines, quiet, undefined, false, stream);
} else {
// Compare N stacks against deployed templates
for (const stack of stacks.stackArtifacts) {
Expand Down
69 changes: 69 additions & 0 deletions packages/aws-cdk/test/diff.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,75 @@ let cloudExecutable: MockCloudExecutable;
let cloudFormation: jest.Mocked<Deployments>;
let toolkit: CdkToolkit;

describe('fixed template', () => {
const templatePath = 'oldTemplate.json';
beforeEach(() => {
const oldTemplate = {
Resources: {
SomeResource: {
Type: 'AWS::SomeService::SomeResource',
Properties: {
Something: 'old-value',
},
},
},
};

cloudExecutable = new MockCloudExecutable({
stacks: [{
stackName: 'A',
template: {
Resources: {
SomeResource: {
Type: 'AWS::SomeService::SomeResource',
Properties: {
Something: 'new-value',
},
},
},
},
}],
});

toolkit = new CdkToolkit({
cloudExecutable,
deployments: cloudFormation,
configuration: cloudExecutable.configuration,
sdkProvider: cloudExecutable.sdkProvider,
});

fs.writeFileSync(templatePath, JSON.stringify(oldTemplate));
});

afterEach(() => fs.rmSync(templatePath));

test('fixed template with valid templates', async () => {
// GIVEN
const buffer = new StringWritable();

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

// THEN
const plainTextOutput = buffer.data.replace(/\x1B\[[0-?]*[ -/]*[@-~]/g, '');
expect(exitCode).toBe(0);
expect(plainTextOutput.replace(/\x1B\[[0-?]*[ -/]*[@-~]/g, '')).toContain(`Resources
[~] AWS::SomeService::SomeResource SomeResource
└─ [~] Something
├─ [-] old-value
└─ [+] new-value
✨ Number of stacks with differences: 1
`);
});
});

describe('imports', () => {
beforeEach(() => {
const outputToJson = {
Expand Down

0 comments on commit 466f170

Please sign in to comment.