From 466f170af409d0c9c44f0f03a6eb5a72553db29b Mon Sep 17 00:00:00 2001 From: Calvin Combs <66279577+comcalvi@users.noreply.github.com> Date: Tue, 23 Apr 2024 18:45:49 -0700 Subject: [PATCH] fix(CLI): `diff --template` crashes (#29896) ### 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* --- packages/aws-cdk/lib/cdk-toolkit.ts | 35 +-------------- packages/aws-cdk/test/diff.test.ts | 69 +++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 33 deletions(-) diff --git a/packages/aws-cdk/lib/cdk-toolkit.ts b/packages/aws-cdk/lib/cdk-toolkit.ts index e1c4571182999..854b7ec6419c2 100644 --- a/packages/aws-cdk/lib/cdk-toolkit.ts +++ b/packages/aws-cdk/lib/cdk-toolkit.ts @@ -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) { diff --git a/packages/aws-cdk/test/diff.test.ts b/packages/aws-cdk/test/diff.test.ts index ea5c67cf0b2d7..0155f74dd192d 100644 --- a/packages/aws-cdk/test/diff.test.ts +++ b/packages/aws-cdk/test/diff.test.ts @@ -14,6 +14,75 @@ let cloudExecutable: MockCloudExecutable; let cloudFormation: jest.Mocked; 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 = {