fix(CLI): diff reports wrong count of changed stacks (#26796)

The current logic counts the number of changes in any parent stacks. It doesn't count nested stacks correctly, and doesn't count parent stacks correctly.

With this change:
<img width="1047" alt="Screenshot 2023-08-18 at 9 43 49 AM" src="https:/aws/aws-cdk/assets/66279577/b417baa7-58d9-454a-a735-4bd406f1c126">

Without this change: 
<img width="1047" alt="Screenshot 2023-08-18 at 9 51 55 AM" src="https:/aws/aws-cdk/assets/66279577/85e87e72-25ec-47af-96af-f47f7c43a4f2">

Closes #26818.

----

*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/api/deployments.ts

@@ -6,7 +6,7 @@ import { ISDK } from './aws-auth/sdk';
 import { CredentialsOptions, SdkForEnvironment, SdkProvider } from './aws-auth/sdk-provider';
 import { deployStack, DeployStackResult, destroyStack, makeBodyParameterAndUpload, DeploymentMethod } from './deploy-stack';
 import { HotswapMode } from './hotswap/common';
-import { loadCurrentTemplateWithNestedStacks, loadCurrentTemplate } from './nested-stack-helpers';
+import { loadCurrentTemplateWithNestedStacks, loadCurrentTemplate, flattenNestedStackNames, TemplateWithNestedStackCount } from './nested-stack-helpers';
 import { ToolkitInfo } from './toolkit-info';
 import { CloudFormationStack, Template, ResourcesToImport, ResourceIdentifierSummaries } from './util/cloudformation';
 import { StackActivityProgress } from './util/cloudformation/stack-activity-monitor';
@@ -300,9 +300,13 @@ export class Deployments {
  public async readCurrentTemplateWithNestedStacks(
  rootStackArtifact: cxapi.CloudFormationStackArtifact,
  retrieveProcessedTemplate: boolean = false,
- ): Promise<Template> {
+ ): Promise<TemplateWithNestedStackCount> {
  const sdk = (await this.prepareSdkWithLookupOrDeployRole(rootStackArtifact)).stackSdk;
- return (await loadCurrentTemplateWithNestedStacks(rootStackArtifact, sdk, retrieveProcessedTemplate)).deployedTemplate;
+ const templateWithNestedStacks = await loadCurrentTemplateWithNestedStacks(rootStackArtifact, sdk, retrieveProcessedTemplate);
+ return {
+ deployedTemplate: templateWithNestedStacks.deployedTemplate,
+ nestedStackCount: flattenNestedStackNames(templateWithNestedStacks.nestedStackNames).length,
+ };
  }
 
  public async readCurrentTemplate(stackArtifact: cxapi.CloudFormationStackArtifact): Promise<Template> {

packages/aws-cdk/lib/api/nested-stack-helpers.ts

@@ -15,6 +15,11 @@ export interface NestedStackNames {
  readonly nestedChildStackNames: { [logicalId: string]: NestedStackNames };
 }
 
+export interface TemplateWithNestedStackCount {
+ readonly deployedTemplate: Template;
+ readonly nestedStackCount: number;
+}
+
 /**
  * Reads the currently deployed template from CloudFormation and adds a
  * property, `NestedTemplate`, to any nested stacks that appear in either
@@ -40,6 +45,19 @@ export async function loadCurrentTemplateWithNestedStacks(
  };
 }
 
+export function flattenNestedStackNames(nestedStackNames: { [nestedStackLogicalId: string]: NestedStackNames }): string[] {
+ const nameList = [];
+ for (const key of Object.keys(nestedStackNames)) {
+ nameList.push(key);
+
+ if (Object.keys(nestedStackNames[key].nestedChildStackNames).length !== 0) {
+ flattenNestedStacksHelper(nestedStackNames[key].nestedChildStackNames, nameList);
+ }
+ }
+
+ return nameList;
+}
+
 /**
  * Returns the currently deployed template from CloudFormation that corresponds to `stackArtifact`.
  */
@@ -135,6 +153,16 @@ function isCdkManagedNestedStack(stackResource: any): stackResource is NestedSta
  return stackResource.Type === 'AWS::CloudFormation::Stack' && stackResource.Metadata && stackResource.Metadata['aws:asset:path'];
 }
 
+function flattenNestedStacksHelper(nestedStackNames: { [logicalId: string]: NestedStackNames }, nameList: string[]) {
+ for (const key of Object.keys(nestedStackNames)) {
+ nameList.push(key);
+
+ if (Object.keys(nestedStackNames[key].nestedChildStackNames).length !== 0) {
+ flattenNestedStacksHelper(nestedStackNames[key].nestedChildStackNames, nameList);
+ }
+ }
+}
+
 interface StackTemplates {
  readonly generatedTemplate: any;
  readonly deployedTemplate: any;

packages/aws-cdk/lib/cdk-toolkit.ts

@@ -140,10 +140,18 @@ export class CdkToolkit {
  stream.write(format('Stack %s\n', chalk.bold(stack.displayName)));
  }
 
- const currentTemplate = await this.props.deployments.readCurrentTemplateWithNestedStacks(stack, options.compareAgainstProcessedTemplate);
- diffs += options.securityOnly
- ? numberFromBool(printSecurityDiff(currentTemplate, stack, RequireApproval.Broadening))
- : printStackDiff(currentTemplate, stack, strict, contextLines, quiet, stream);
+ const templateWithNames = await this.props.deployments.readCurrentTemplateWithNestedStacks(
+ stack, options.compareAgainstProcessedTemplate,
+ );
+ const currentTemplate = templateWithNames.deployedTemplate;
+ const nestedStackCount = templateWithNames.nestedStackCount;
+
+ const stackCount =
+ options.securityOnly
+ ? (numberFromBool(printSecurityDiff(currentTemplate, stack, RequireApproval.Broadening)) > 0 ? 1 : 0)
+ : (printStackDiff(currentTemplate, stack, strict, contextLines, quiet, stream) > 0 ? 1 : 0);
+
+ diffs += stackCount + nestedStackCount;
  }
  }
 

packages/aws-cdk/test/api/cloudformation-deployments.test.ts

@@ -246,9 +246,11 @@ test('readCurrentTemplateWithNestedStacks() can handle non-Resources in the temp
  );
 
  // WHEN
- const deployedTemplate = await deployments.readCurrentTemplateWithNestedStacks(rootStack);
+ const nestedStackCount = (await deployments.readCurrentTemplateWithNestedStacks(rootStack)).nestedStackCount;
+ const deployedTemplate = (await deployments.readCurrentTemplateWithNestedStacks(rootStack)).deployedTemplate;
 
  // THEN
+ expect(nestedStackCount).toEqual(1);
  expect(deployedTemplate).toEqual({
  Resources: {
  NestedStack: {
@@ -451,9 +453,11 @@ test('readCurrentTemplateWithNestedStacks() with a 3-level nested + sibling stru
  );
 
  // WHEN
- const deployedTemplate = await deployments.readCurrentTemplateWithNestedStacks(rootStack);
+ const nestedStackCount = (await deployments.readCurrentTemplateWithNestedStacks(rootStack)).nestedStackCount;
+ const deployedTemplate = (await deployments.readCurrentTemplateWithNestedStacks(rootStack)).deployedTemplate;
 
  // THEN
+ expect(nestedStackCount).toEqual(3);
  expect(deployedTemplate).toEqual({
  Resources: {
  NestedStack: {
@@ -608,9 +612,11 @@ test('readCurrentTemplateWithNestedStacks() on an undeployed parent stack with a
  });
 
  // WHEN
- const deployedTemplate = await deployments.readCurrentTemplateWithNestedStacks(rootStack);
+ const nestedStackCount = (await deployments.readCurrentTemplateWithNestedStacks(rootStack)).nestedStackCount;
+ const deployedTemplate = (await deployments.readCurrentTemplateWithNestedStacks(rootStack)).deployedTemplate;
 
  // THEN
+ expect(nestedStackCount).toEqual(2);
  expect(deployedTemplate).toEqual({
  Resources: {
  NestedStack: {
@@ -781,9 +787,11 @@ test('readCurrentTemplateWithNestedStacks() succesfully ignores stacks without m
  ));
 
  // WHEN
- const deployedTemplate = await deployments.readCurrentTemplateWithNestedStacks(rootStack);
+ const nestedStackCount = (await deployments.readCurrentTemplateWithNestedStacks(rootStack)).nestedStackCount;
+ const deployedTemplate = (await deployments.readCurrentTemplateWithNestedStacks(rootStack)).deployedTemplate;
 
  // THEN
+ expect(nestedStackCount).toEqual(1);
  expect(deployedTemplate).toEqual({
  Resources: {
  WithMetadata: {

packages/aws-cdk/test/diff.test.ts

@@ -54,9 +54,15 @@ describe('non-nested stacks', () => {
  // Default implementations
  cloudFormation.readCurrentTemplateWithNestedStacks.mockImplementation((stackArtifact: CloudFormationStackArtifact) => {
  if (stackArtifact.stackName === 'D') {
- return Promise.resolve({ resource: 'D' });
+ return Promise.resolve({
+ deployedTemplate: { resource: 'D' },
+ nestedStackCount: 0,
+ });
  }
- return Promise.resolve({});
+ return Promise.resolve({
+ deployedTemplate: {},
+ nestedStackCount: 0,
+ });
  });
  cloudFormation.deployStack.mockImplementation((options) => Promise.resolve({
  noOp: true,
@@ -85,6 +91,43 @@ describe('non-nested stacks', () => {
  expect(exitCode).toBe(0);
  });
 
+ test('diff number of stack diffs, not resource diffs', async () => {
+ // GIVEN
+ cloudExecutable = new MockCloudExecutable({
+ stacks: [{
+ stackName: 'A',
+ template: { resourceA: 'A', resourceB: 'B' },
+ },
+ {
+ stackName: 'B',
+ template: { resourceC: 'C' },
+ }],
+ });
+
+ toolkit = new CdkToolkit({
+ cloudExecutable,
+ deployments: cloudFormation,
+ configuration: cloudExecutable.configuration,
+ sdkProvider: cloudExecutable.sdkProvider,
+ });
+
+ const buffer = new StringWritable();
+
+ // WHEN
+ const exitCode = await toolkit.diff({
+ stackNames: ['A', 'B'],
+ stream: buffer,
+ });
+
+ // THEN
+ const plainTextOutput = buffer.data.replace(/\x1B\[[0-?]*[ -/]*[@-~]/g, '');
+ expect(plainTextOutput).toContain('Stack A');
+ expect(plainTextOutput).toContain('Stack B');
+
+ expect(buffer.data.trim()).toContain('✨ Number of stacks with differences: 2');
+ expect(exitCode).toBe(0);
+ });
+
  test('exits with 1 with diffs and fail set to true', async () => {
  // GIVEN
  const buffer = new StringWritable();
@@ -161,7 +204,7 @@ describe('nested stacks', () => {
  cloudExecutable = new MockCloudExecutable({
  stacks: [{
  stackName: 'Parent',
- template: { },
+ template: {},
  }],
  });
 
@@ -209,41 +252,47 @@ describe('nested stacks', () => {
  },
  };
  return Promise.resolve({
- Resources: {
- AdditionChild: {
- Type: 'AWS::CloudFormation::Stack',
- Resources: {
- SomeResource: {
- Type: 'AWS::Something',
+ deployedTemplate: {
+ Resources: {
+ AdditionChild: {
+ Type: 'AWS::CloudFormation::Stack',
+ Resources: {
+ SomeResource: {
+ Type: 'AWS::Something',
+ },
  },
  },
- },
- DeletionChild: {
- Type: 'AWS::CloudFormation::Stack',
- Resources: {
- SomeResource: {
- Type: 'AWS::Something',
- Properties: {
- Prop: 'value-to-be-removed',
+  DeletionChild: {
+  Type: 'AWS::CloudFormation::Stack',
+  Resources: {
+  SomeResource: {
+  Type: 'AWS::Something',
+  Properties: {
+  Prop: 'value-to-be-removed',
+ },
  },
  },
  },
- },
- ChangedChild: {
- Type: 'AWS::CloudFormation::Stack',
- Resources: {
- SomeResource: {
- Type: 'AWS::Something',
- Properties: {
- Prop: 'old-value',
+  ChangedChild: {
+  Type: 'AWS::CloudFormation::Stack',
+  Resources: {
+  SomeResource: {
+  Type: 'AWS::Something',
+  Properties: {
+  Prop: 'old-value',
+ },
  },
  },
  },
  },
  },
+ nestedStackCount: 3,
  });
  }
- return Promise.resolve({});
+ return Promise.resolve({
+ deployedTemplate: {},
+ nestedStackCount: 0,
+ });
  });
  });
 
@@ -279,7 +328,7 @@ Resources
  └─ [+] new-value
 
 
-✨ Number of stacks with differences: 3`);
+✨ Number of stacks with differences: 4`);
 
  expect(exitCode).toBe(0);
  });