refactor(cli): stack/security diff no longer prints directly to a stream (#264)

part of https://github.com/orgs/aws/projects/257

this refactor removes direct prints to `stderr` or `stdout` for the diff
cli command. the diff result will instead be printed through the global
`CliIoHost`.

the diff format is determined by the `Formatter` class, which takes in a
`stream` that previously was `stdout` (or `stderr`). now, we are sending
our own stream into `Formatter`, capturing what was previously
immediately printed and instead returning it as a string. we then print
the resulting string through `CliIoHost`. it should have no functional
change to how the diff cli works today.

this effort will help support diff in the toolkit lib because it allows
the `IoHost` governance of the diff print.

as this is a pure refactor, successful existing tests is enough to
ensure that this works.

---
By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache-2.0 license

Author: kaizencc

packages/aws-cdk/lib/api/deployments/cfn-api.ts

@@ -140,7 +140,6 @@ export type PrepareChangeSetOptions = {
   uuid: string;
   willExecute: boolean;
   sdkProvider: SdkProvider;
-  stream: NodeJS.WritableStream;
   parameters: { [name: string]: string | undefined };
   resourcesToImport?: ResourcesToImport;
 }
@@ -224,9 +223,9 @@ async function uploadBodyParameterAndCreateChangeSet(
     const exists = (await CloudFormationStack.lookup(cfn, options.stack.stackName, false)).exists;
 
     const executionRoleArn = await env.replacePlaceholders(options.stack.cloudFormationExecutionRoleArn);
-    options.stream.write(
+    await ioHelper.notify(IO.DEFAULT_TOOLKIT_INFO.msg(
       'Hold on while we create a read-only change set to get a diff with accurate replacement information (use --no-change-set to use a less accurate but faster template-only diff)\n',
-    );
+    ));
 
     return await createChangeSet(ioHelper, {
       cfn,
@@ -242,9 +241,9 @@ async function uploadBodyParameterAndCreateChangeSet(
     });
   } catch (e: any) {
     await ioHelper.notify(IO.DEFAULT_TOOLKIT_DEBUG.msg(e));
-    options.stream.write(
+    await ioHelper.notify(IO.DEFAULT_TOOLKIT_INFO.msg(
       'Could not create a change set, will base the diff on template differences (run again with -v to see the reason)\n',
-    );
+    ));
 
     return undefined;
   }

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

@@ -36,7 +36,7 @@ import { tagsForStack, type Tag } from '../api/tags';
 import type { AssetBuildNode, AssetPublishNode, Concurrency, StackNode, WorkGraph } from '../api/work-graph';
 import { WorkGraphBuilder } from '../api/work-graph/work-graph-builder';
 import { StackActivityProgress } from '../commands/deploy';
-import { printSecurityDiff, printStackDiff, RequireApproval } from '../commands/diff';
+import { formatSecurityDiff, formatStackDiff, RequireApproval } from '../commands/diff';
 import { listStacks } from '../commands/list-stacks';
 import type {
   FromScan,
@@ -60,7 +60,7 @@ import {
 } from '../commands/migrate';
 import { result as logResult, debug, error, highlight, info, success, warning } from '../logging';
 import { CliIoHost } from './io-host';
-import { numberFromBool, partition, validateSnsTopicArn, formatErrorMessage, deserializeStructure, obscureTemplate, serializeStructure, formatTime } from '../util';
+import { partition, validateSnsTopicArn, formatErrorMessage, deserializeStructure, obscureTemplate, serializeStructure, formatTime } from '../util';
 
 // Must use a require() otherwise esbuild complains about calling a namespace
 // eslint-disable-next-line @typescript-eslint/no-require-imports,@typescript-eslint/consistent-type-imports
@@ -177,7 +177,6 @@ export class CdkToolkit {
 
     const strict = !!options.strict;
     const contextLines = options.contextLines || 3;
-    const stream = options.stream || process.stderr;
     const quiet = options.quiet || false;
 
     let diffs = 0;
@@ -196,9 +195,31 @@ export class CdkToolkit {
       }
 
       const template = deserializeStructure(await fs.readFile(options.templatePath, { encoding: 'UTF-8' }));
-      diffs = options.securityOnly
-        ? numberFromBool(printSecurityDiff(template, stacks.firstStack, RequireApproval.Broadening, quiet))
-        : printStackDiff(template, stacks.firstStack, strict, contextLines, quiet, undefined, undefined, false, stream);
+
+      if (options.securityOnly) {
+        const securityDiff = formatSecurityDiff(
+          template,
+          stacks.firstStack,
+          RequireApproval.Broadening,
+        );
+        if (securityDiff.formattedDiff) {
+          info(securityDiff.formattedDiff);
+          diffs += 1;
+        }
+      } else {
+        const diff = formatStackDiff(
+          template,
+          stacks.firstStack,
+          strict,
+          contextLines,
+          quiet,
+          undefined,
+          undefined,
+          false,
+        );
+        diffs = diff.numStacksWithChanges;
+        info(diff.formattedDiff);
+      }
     } else {
       // Compare N stacks against deployed templates
       for (const stack of stacks.stackArtifacts) {
@@ -231,7 +252,7 @@ export class CdkToolkit {
           } catch (e: any) {
             debug(formatErrorMessage(e));
             if (!quiet) {
-              stream.write(
+              info(
                 `Checking if the stack ${stack.stackName} exists before creating the changeset has failed, will base the diff on template differences (run again with -v to see the reason)\n`,
               );
             }
@@ -247,7 +268,6 @@ export class CdkToolkit {
               sdkProvider: this.props.sdkProvider,
               parameters: Object.assign({}, parameterMap['*'], parameterMap[stack.stackName]),
               resourcesToImport,
-              stream,
             });
           } else {
             debug(
@@ -256,18 +276,20 @@ export class CdkToolkit {
           }
         }
 
-        const stackCount = options.securityOnly
-          ? numberFromBool(
-            printSecurityDiff(
-              currentTemplate,
-              stack,
-              RequireApproval.Broadening,
-              quiet,
-              stack.displayName,
-              changeSet,
-            ),
-          )
-          : printStackDiff(
+        if (options.securityOnly) {
+          const securityDiff = formatSecurityDiff(
+            currentTemplate,
+            stack,
+            RequireApproval.Broadening,
+            stack.displayName,
+            changeSet,
+          );
+          if (securityDiff.formattedDiff) {
+            info(securityDiff.formattedDiff);
+            diffs += 1;
+          }
+        } else {
+          const diff = formatStackDiff(
             currentTemplate,
             stack,
             strict,
@@ -276,15 +298,15 @@ export class CdkToolkit {
             stack.displayName,
             changeSet,
             !!resourcesToImport,
-            stream,
             nestedStacks,
           );
-
-        diffs += stackCount;
+          info(diff.formattedDiff);
+          diffs += diff.numStacksWithChanges;
+        }
       }
     }
 
-    stream.write(format('\n✨  Number of stacks with differences: %s\n', diffs));
+    info(format('\n✨  Number of stacks with differences: %s\n', diffs));
 
     return diffs && options.fail ? 1 : 0;
   }
@@ -401,7 +423,9 @@ export class CdkToolkit {
 
       if (requireApproval !== RequireApproval.Never) {
         const currentTemplate = await this.props.deployments.readCurrentTemplate(stack);
-        if (printSecurityDiff(currentTemplate, stack, requireApproval)) {
+        const securityDiff = formatSecurityDiff(currentTemplate, stack, requireApproval);
+        if (securityDiff.formattedDiff) {
+          info(securityDiff.formattedDiff);
           await askUserConfirmation(
             this.ioHost,
             concurrency,
@@ -1360,13 +1384,6 @@ export interface DiffOptions {
    */
   contextLines?: number;
 
-  /**
-   * Where to write the default
-   *
-   * @default stderr
-   */
-  stream?: NodeJS.WritableStream;
-
   /**
    * Whether to fail with exit code 1 in case of diff
    *

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

@@ -274,7 +274,6 @@ export async function exec(args: string[], synthesizer?: Synthesizer): Promise<n
           contextLines: args.contextLines,
           securityOnly: args.securityOnly,
           fail: args.fail != null ? args.fail : !enableDiffNoFail,
-          stream: args.ci ? process.stdout : undefined,
           compareAgainstProcessedTemplate: args.processed,
           quiet: args.quiet,
           changeSet: args['change-set'],