From 79d4a51f8fac734e80605229128af74066185674 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Thu, 18 Apr 2024 10:20:08 -0700 Subject: [PATCH 1/5] diff no longer crashes --- .../cloudformation-diff/lib/diff-template.ts | 4 +++ .../test/diff-template.test.ts | 13 +++++++ packages/aws-cdk/lib/cdk-toolkit.ts | 35 ++----------------- 3 files changed, 19 insertions(+), 33 deletions(-) diff --git a/packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts b/packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts index 118df03c6ebc5..226d9581e60ce 100644 --- a/packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts +++ b/packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts @@ -51,6 +51,10 @@ export function fullDiff( isImport?: boolean, ): types.TemplateDiff { + if (!currentTemplate || !newTemplate) { + throw new Error('trying to diff a template that is not defined'); + } + normalize(currentTemplate); normalize(newTemplate); const theDiff = diffTemplate(currentTemplate, newTemplate); diff --git a/packages/@aws-cdk/cloudformation-diff/test/diff-template.test.ts b/packages/@aws-cdk/cloudformation-diff/test/diff-template.test.ts index 318f27b6bbf30..12290ef1e31c9 100644 --- a/packages/@aws-cdk/cloudformation-diff/test/diff-template.test.ts +++ b/packages/@aws-cdk/cloudformation-diff/test/diff-template.test.ts @@ -624,6 +624,19 @@ test('metadata changes are rendered in the diff', () => { expect(differences.resources.differenceCount).toBe(1); }); +test('diff complains if it receives an undefined template', async () => { + // GIVEN + const currentTemplate: any = undefined; + + // WHEN + const newTemplate = { + Resources: {}, + }; + + // THEN + expect(() => fullDiff(currentTemplate, newTemplate)).toThrow(/trying to diff a template that is not defined/); +}); + describe('changeset', () => { test('changeset overrides spec replacements', () => { // GIVEN 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) { From c51f20e6d6346cc5e8b0a1da9d3d0ad31f9350b2 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Thu, 18 Apr 2024 14:39:08 -0700 Subject: [PATCH 2/5] working test --- packages/aws-cdk/test/diff.test.ts | 67 ++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/packages/aws-cdk/test/diff.test.ts b/packages/aws-cdk/test/diff.test.ts index ea5c67cf0b2d7..ef246eb1f6db7 100644 --- a/packages/aws-cdk/test/diff.test.ts +++ b/packages/aws-cdk/test/diff.test.ts @@ -14,6 +14,73 @@ 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)); + }); + + 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 = { From 0631c887fcc960c7e66da6c0cacc896e769fa796 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Thu, 18 Apr 2024 14:40:04 -0700 Subject: [PATCH 3/5] test file cleanup --- packages/aws-cdk/test/diff.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/aws-cdk/test/diff.test.ts b/packages/aws-cdk/test/diff.test.ts index ef246eb1f6db7..0155f74dd192d 100644 --- a/packages/aws-cdk/test/diff.test.ts +++ b/packages/aws-cdk/test/diff.test.ts @@ -54,6 +54,8 @@ describe('fixed template', () => { fs.writeFileSync(templatePath, JSON.stringify(oldTemplate)); }); + afterEach(() => fs.rmSync(templatePath)); + test('fixed template with valid templates', async () => { // GIVEN const buffer = new StringWritable(); From 9d806f13ae639a43cf99e38655b61b22a821dc21 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Fri, 19 Apr 2024 08:38:56 -0700 Subject: [PATCH 4/5] remove error --- .../cloudformation-diff/lib/diff-template.ts | 5 ----- .../cloudformation-diff/test/diff-template.test.ts | 13 ------------- 2 files changed, 18 deletions(-) diff --git a/packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts b/packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts index 226d9581e60ce..e3bac39373c8a 100644 --- a/packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts +++ b/packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts @@ -50,11 +50,6 @@ export function fullDiff( changeSet?: DescribeChangeSetOutput, isImport?: boolean, ): types.TemplateDiff { - - if (!currentTemplate || !newTemplate) { - throw new Error('trying to diff a template that is not defined'); - } - normalize(currentTemplate); normalize(newTemplate); const theDiff = diffTemplate(currentTemplate, newTemplate); diff --git a/packages/@aws-cdk/cloudformation-diff/test/diff-template.test.ts b/packages/@aws-cdk/cloudformation-diff/test/diff-template.test.ts index 12290ef1e31c9..318f27b6bbf30 100644 --- a/packages/@aws-cdk/cloudformation-diff/test/diff-template.test.ts +++ b/packages/@aws-cdk/cloudformation-diff/test/diff-template.test.ts @@ -624,19 +624,6 @@ test('metadata changes are rendered in the diff', () => { expect(differences.resources.differenceCount).toBe(1); }); -test('diff complains if it receives an undefined template', async () => { - // GIVEN - const currentTemplate: any = undefined; - - // WHEN - const newTemplate = { - Resources: {}, - }; - - // THEN - expect(() => fullDiff(currentTemplate, newTemplate)).toThrow(/trying to diff a template that is not defined/); -}); - describe('changeset', () => { test('changeset overrides spec replacements', () => { // GIVEN From 235f3ae8ff22b8aeeaa3112365cad5dbecc914a1 Mon Sep 17 00:00:00 2001 From: Calvin Combs Date: Fri, 19 Apr 2024 08:40:07 -0700 Subject: [PATCH 5/5] whitespace --- packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts b/packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts index e3bac39373c8a..118df03c6ebc5 100644 --- a/packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts +++ b/packages/@aws-cdk/cloudformation-diff/lib/diff-template.ts @@ -50,6 +50,7 @@ export function fullDiff( changeSet?: DescribeChangeSetOutput, isImport?: boolean, ): types.TemplateDiff { + normalize(currentTemplate); normalize(newTemplate); const theDiff = diffTemplate(currentTemplate, newTemplate);