fix(toolkit-lib)!: diff message payloads are incomplete

packages/@aws-cdk/toolkit-lib/docs/message-registry.md

@@ -71,7 +71,8 @@ Please let us know by [opening an issue](https://github.com/aws/aws-cdk-cli/issu
 | `CDK_TOOLKIT_I3110` | Additional information is needed to identify a resource | `info` | {@link ResourceIdentificationRequest} |
 | `CDK_TOOLKIT_E3900` | Resource import failed | `error` | {@link ErrorPayload} |
 | `CDK_TOOLKIT_I4000` | Diff stacks is starting | `trace` | {@link StackSelectionDetails} |
-| `CDK_TOOLKIT_I4001` | Output of the diff command | `info` | {@link DiffResult} |
+| `CDK_TOOLKIT_I4001` | Output of the diff command | `result` | {@link DiffResult} |
+| `CDK_TOOLKIT_I4002` | The diff for a single stack | `result` | {@link StackDiff} |
 | `CDK_TOOLKIT_I4590` | Results of the drift command | `result` | {@link DriftResultPayload} |
 | `CDK_TOOLKIT_I4591` | Missing drift result fort a stack. | `warn` | {@link SingleStack} |
 | `CDK_TOOLKIT_I5000` | Provides deployment times | `info` | {@link Duration} |

packages/@aws-cdk/toolkit-lib/lib/actions/diff/index.ts

@@ -89,7 +89,7 @@ export class DiffMethod {
 }
 
 /**
- * Optins for the diff method
+ * Options for the diff method
  */
 export interface DiffOptions {
   /**
@@ -129,13 +129,4 @@ export interface DiffOptions {
    * @default 3
    */
   readonly contextLines?: number;
-
-  /**
-   * Only include broadened security changes in the diff
-   *
-   * @default false
-   *
-   * @deprecated implement in IoHost
-   */
-  readonly securityOnly?: boolean;
 }

packages/@aws-cdk/toolkit-lib/lib/api/diff/diff-formatter.ts

@@ -71,7 +71,7 @@ interface FormatStackDiffOptions {
    *
    * @default 3
    */
-  readonly context?: number;
+  readonly contextLines?: number;
 
   /**
    * silences \'There were no differences\' messages
@@ -157,7 +157,7 @@ export class DiffFormatter {
   /**
    * Get or creates the diff of a stack.
    * If it creates the diff, it stores the result in a map for
-   * easier retreval later.
+   * easier retrieval later.
    */
   private diff(stackName?: string, oldTemplate?: any) {
     const realStackName = stackName ?? this.stackName;
@@ -178,8 +178,8 @@ export class DiffFormatter {
    *
    * If no stackName is given, then the root stack name is used.
    */
-  private permissionType(stackName?: string): PermissionChangeType {
-    const diff = this.diff(stackName);
+  private permissionType(): PermissionChangeType {
+    const diff = this.diff();
 
     if (diff.permissionsBroadened) {
       return PermissionChangeType.BROADENING;
@@ -211,7 +211,7 @@ export class DiffFormatter {
     let diff = this.diff(stackName, oldTemplate);
 
     // The stack diff is formatted via `Formatter`, which takes in a stream
-    // and sends its output directly to that stream. To faciliate use of the
+    // and sends its output directly to that stream. To facilitate use of the
     // global CliIoHost, we create our own stream to capture the output of
     // `Formatter` and return the output as a string for the consumer of
     // `formatStackDiff` to decide what to do with it.
@@ -253,7 +253,7 @@ export class DiffFormatter {
         formatDifferences(stream, diff, {
           ...logicalIdMapFromTemplate(this.oldTemplate),
           ...buildLogicalToPathMap(this.newTemplate),
-        }, options.context);
+        }, options.contextLines);
       } else if (!options.quiet) {
         stream.write(chalk.green('There were no differences\n'));
       }

packages/@aws-cdk/toolkit-lib/lib/api/io/private/messages.ts

@@ -1,7 +1,7 @@
 import type * as cxapi from '@aws-cdk/cx-api';
 import * as make from './message-maker';
 import type { SpanDefinition } from './span';
-import type { DiffResult } from '../../../payloads';
+import type { StackDiff, DiffResult } from '../../../payloads';
 import type { BootstrapEnvironmentProgress } from '../../../payloads/bootstrap-environment-progress';
 import type { MissingContext, UpdatedContext } from '../../../payloads/context';
 import type { BuildAsset, DeployConfirmationRequest, PublishAsset, StackDeployProgress, SuccessfulDeployStackResult } from '../../../payloads/deploy';
@@ -85,11 +85,16 @@ export const IO = {
     description: 'Diff stacks is starting',
     interface: 'StackSelectionDetails',
   }),
-  CDK_TOOLKIT_I4001: make.info<DiffResult>({
+  CDK_TOOLKIT_I4001: make.result<DiffResult>({
     code: 'CDK_TOOLKIT_I4001',
     description: 'Output of the diff command',
     interface: 'DiffResult',
   }),
+  CDK_TOOLKIT_I4002: make.result<StackDiff>({
+    code: 'CDK_TOOLKIT_I4002',
+    description: 'The diff for a single stack',
+    interface: 'StackDiff',
+  }),
 
   // 4: Drift (45xx - 49xx)
   CDK_TOOLKIT_I4590: make.result<DriftResultPayload>({

packages/@aws-cdk/toolkit-lib/lib/payloads/diff.ts

@@ -1,4 +1,5 @@
-import type { Duration } from './types';
+import type { ITemplateDiff } from '@aws-cdk/cloudformation-diff';
+import type { Duration, SingleStack } from './types';
 
 /**
  * Different types of permission related changes in a diff
@@ -20,17 +21,57 @@ export enum PermissionChangeType {
   NON_BROADENING = 'non-broadening',
 }
 
+/**
+ * The diff formatted as different types of output
+ */
+export interface FormattedDiff {
+  /**
+   * The stack diff formatted as a string
+   */
+  readonly diff: string;
+  /**
+   * The security diff formatted as a string, if any
+   */
+  readonly security?: string;
+}
+
+/**
+ * Diff information for a single stack
+ */
+export interface StackDiff extends SingleStack {
+  /**
+   * Total number of stacks that have changes
+   * Can be higher than `1` if the stack has nested stacks.
+   */
+  readonly numStacksWithChanges: number;
+
+  /**
+   * Structural diff of the stack
+   * Can include more than a single diff if the stack has nested stacks.
+   */
+  readonly diffs: { [name: string]: ITemplateDiff };
+
+  /**
+   * The formatted diff
+   */
+  readonly formattedDiff: FormattedDiff;
+
+  /**
+   * Does the diff contain changes to permissions and what kind
+   */
+  readonly permissionChanges: PermissionChangeType;
+}
+
 /**
  * Output of the diff command
  */
 export interface DiffResult extends Duration {
   /**
-   * Stack diff formatted as a string
+   * Total number of stacks that have changes
    */
-  readonly formattedStackDiff: string;
-
+  readonly numStacksWithChanges: number;
   /**
-   * Security diff formatted as a string
+   * Structural diff of all selected stacks
    */
-  readonly formattedSecurityDiff: string;
+  readonly diffs: { [name: string]: ITemplateDiff };
 }

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

@@ -339,39 +339,41 @@ export class Toolkit extends CloudAssemblySourceBuilder {
     const contextLines = options.contextLines || 3;
 
     let diffs = 0;
-    let formattedSecurityDiff = '';
-    let formattedStackDiff = '';
 
     const templateInfos = await prepareDiff(ioHelper, stacks, deployments, await this.sdkProvider('diff'), options);
     const templateDiffs: { [name: string]: TemplateDiff } = {};
     for (const templateInfo of templateInfos) {
-      const formatter = new DiffFormatter({
-        templateInfo,
-      });
+      const formatter = new DiffFormatter({ templateInfo });
+      const stackDiff = formatter.formatStackDiff({ strict, contextLines });
 
-      if (options.securityOnly) {
-        const securityDiff = formatter.formatSecurityDiff();
-        // In Diff, we only care about BROADENING security diffs
-        if (securityDiff.permissionChangeType == PermissionChangeType.BROADENING) {
-          const warningMessage = 'This deployment will make potentially sensitive changes according to your current security approval level.\nPlease confirm you intend to make the following modifications:\n';
-          await ioHelper.defaults.warn(warningMessage);
-          formattedSecurityDiff = securityDiff.formattedDiff;
-          diffs = securityDiff.formattedDiff ? diffs + 1 : diffs;
-        }
-      } else {
-        const diff = formatter.formatStackDiff({
-          strict,
-          context: contextLines,
-        });
-        formattedStackDiff = diff.formattedDiff;
-        diffs = diff.numStacksWithChanges;
+      // Security Diff
+      const securityDiff = formatter.formatSecurityDiff();
+      const formattedSecurityDiff = securityDiff.permissionChangeType !== PermissionChangeType.NONE ? stackDiff.formattedDiff : undefined;
+      // We only warn about BROADENING changes
+      if (securityDiff.permissionChangeType == PermissionChangeType.BROADENING) {
+        const warningMessage = 'This deployment will make potentially sensitive changes according to your current security approval level.\nPlease confirm you intend to make the following modifications:\n';
+        await diffSpan.notifyDefault('warn', warningMessage);
+        await diffSpan.notifyDefault('info', securityDiff.formattedDiff);
       }
+
+      // Stack Diff
+      diffs += stackDiff.numStacksWithChanges;
       appendObject(templateDiffs, formatter.diffs);
+      await diffSpan.notify(IO.CDK_TOOLKIT_I4002.msg(stackDiff.formattedDiff, {
+        stack: templateInfo.newTemplate,
+        diffs: formatter.diffs,
+        numStacksWithChanges: stackDiff.numStacksWithChanges,
+        permissionChanges: securityDiff.permissionChangeType,
+        formattedDiff: {
+          diff: stackDiff.formattedDiff,
+          security: formattedSecurityDiff,
+        },
+      }));
     }
 
     await diffSpan.end(`✨ Number of stacks with differences: ${diffs}`, {
-      formattedSecurityDiff,
-      formattedStackDiff,
+      numStacksWithChanges: diffs,
+      diffs: templateDiffs,
     });
 
     return templateDiffs;

packages/@aws-cdk/toolkit-lib/test/actions/diff.test.ts

@@ -51,11 +51,14 @@ describe('diff', () => {
     // THEN
     expect(ioHost.notifySpy).toHaveBeenCalledWith(expect.objectContaining({
       action: 'diff',
-      level: 'info',
+      level: 'result',
       code: 'CDK_TOOLKIT_I4001',
       message: expect.stringContaining('✨ Number of stacks with differences: 1'),
       data: expect.objectContaining({
-        formattedStackDiff: expect.stringContaining((chalk.bold('Stack1'))),
+        numStacksWithChanges: 1,
+        diffs: expect.objectContaining({
+          Stack1: expect.anything(),
+        }),
       }),
     }));
   });
@@ -131,12 +134,11 @@ describe('diff', () => {
     }));
   });
 
-  test('only security diff', async () => {
+  test('security diff', async () => {
     // WHEN
     const cx = await builderFixture(toolkit, 'stack-with-role');
     const result = await toolkit.diff(cx, {
       stacks: { strategy: StackSelectionStrategy.PATTERN_MUST_MATCH_SINGLE, patterns: ['Stack1'] },
-      securityOnly: true,
       method: DiffMethod.TemplateOnly({ compareAgainstProcessedTemplate: true }),
     });
 
@@ -148,11 +150,12 @@ describe('diff', () => {
     }));
     expect(ioHost.notifySpy).toHaveBeenCalledWith(expect.objectContaining({
       action: 'diff',
-      level: 'info',
-      code: 'CDK_TOOLKIT_I4001',
-      message: expect.stringContaining('✨ Number of stacks with differences: 1'),
+      level: 'result',
+      code: 'CDK_TOOLKIT_I4002',
       data: expect.objectContaining({
-        formattedSecurityDiff: expect.stringContaining((chalk.underline(chalk.bold('IAM Statement Changes')))),
+        formattedDiff: expect.objectContaining({
+          security: expect.stringContaining((chalk.underline(chalk.bold('IAM Statement Changes')))),
+        }),
       }),
     }));
 
@@ -182,17 +185,17 @@ describe('diff', () => {
     const cx = await builderFixture(toolkit, 'two-empty-stacks');
     await toolkit.diff(cx, {
       stacks: { strategy: StackSelectionStrategy.ALL_STACKS },
-      securityOnly: true,
     });
 
     // THEN
     expect(ioHost.notifySpy).toHaveBeenCalledWith(expect.objectContaining({
       action: 'diff',
-      level: 'info',
-      code: 'CDK_TOOLKIT_I4001',
-      message: expect.stringContaining('✨ Number of stacks with differences: 0'),
+      level: 'result',
+      code: 'CDK_TOOLKIT_I4002',
       data: expect.objectContaining({
-        formattedSecurityDiff: '',
+        formattedDiff: expect.objectContaining({
+          security: undefined,
+        }),
       }),
     }));
   });
@@ -207,19 +210,13 @@ describe('diff', () => {
 
     // THEN
     expect(ioHost.notifySpy).not.toHaveBeenCalledWith(expect.objectContaining({
-      action: 'diff',
-      level: 'info',
-      code: 'CDK_TOOLKIT_I0000',
       message: expect.stringContaining('Could not create a change set'),
     }));
     expect(ioHost.notifySpy).toHaveBeenCalledWith(expect.objectContaining({
       action: 'diff',
-      level: 'info',
+      level: 'result',
       code: 'CDK_TOOLKIT_I4001',
       message: expect.stringContaining('✨ Number of stacks with differences: 1'),
-      data: expect.objectContaining({
-        formattedStackDiff: expect.stringContaining(chalk.bold('Stack1')),
-      }),
     }));
   });
 
@@ -344,7 +341,6 @@ describe('diff', () => {
       const cx = await builderFixture(toolkit, 'stack-with-role');
       const result = await toolkit.diff(cx, {
         stacks: { strategy: StackSelectionStrategy.ALL_STACKS },
-        securityOnly: true,
         method: DiffMethod.LocalFile(path.join(__dirname, '..', '_fixtures', 'two-empty-stacks', 'cdk.out', 'Stack1.template.json')),
       });
 

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

@@ -241,7 +241,7 @@ export class CdkToolkit {
       } else {
         const diff = formatter.formatStackDiff({
           strict,
-          context: contextLines,
+          contextLines,
           quiet,
         });
         diffs = diff.numStacksWithChanges;
@@ -325,7 +325,7 @@ export class CdkToolkit {
         } else {
           const diff = formatter.formatStackDiff({
             strict,
-            context: contextLines,
+            contextLines,
             quiet,
           });
           info(diff.formattedDiff);