Merge branch 'main' into evzz-docs-update-s3-bucket-physical-names

packages/@aws-cdk-testing/cli-integ/lib/with-cdk-app.ts

@@ -13,7 +13,7 @@ import { shell, ShellOptions, ShellHelper, rimraf } from './shell';
 import { AwsContext, withAws } from './with-aws';
 import { withTimeout } from './with-timeout';
 
-export const DEFAULT_TEST_TIMEOUT_S = 10 * 60;
+export const DEFAULT_TEST_TIMEOUT_S = 20 * 60;
 export const EXTENDED_TEST_TIMEOUT_S = 30 * 60;
 
 /**

packages/@aws-cdk-testing/cli-integ/resources/cli-regression-patches/v2.160.0/skip-tests.txt

@@ -0,0 +1,2 @@
+# This test asserts too much about the output of the CLI.
+security related changes without a CLI are expected to fail when approval is required

packages/@aws-cdk-testing/cli-integ/skip-tests.txt

@@ -1,7 +1,7 @@
 # This file is empty on purpose. Leave it here as documentation
 # and an example.
 #
-# Copy this file to cli-regression-patches/vX.Y.Z/skip-tests.txt
+# Copy this file to resources/cli-regression-patches/vX.Y.Z/skip-tests.txt
 # and edit it there if you want to exclude certain tests from running
 # when performing a certain version's regression tests.
 #

packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli-lib.integtest.ts

@@ -75,7 +75,7 @@ integTest(
       'This deployment will make potentially sensitive changes according to your current security approval level',
     );
     expect(stdErr).toContain(
-      'Deployment failed: Error: "--require-approval" is enabled and stack includes security-sensitive updates',
+      '"--require-approval" is enabled and stack includes security-sensitive updates',
     );
 
     // Ensure stack was not deployed

packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts

@@ -19,7 +19,6 @@ import {
 import { InvokeCommand } from '@aws-sdk/client-lambda';
 import { CreateTopicCommand, DeleteTopicCommand } from '@aws-sdk/client-sns';
 import { AssumeRoleCommand, GetCallerIdentityCommand } from '@aws-sdk/client-sts';
-import * as chalk from 'chalk';
 import {
   integTest,
   cloneDirectory,
@@ -2198,8 +2197,7 @@ integTest(
       allowErrExit: true,
     });
 
-    const stackName = `${fixture.stackNamePrefix}-ecs-hotswap`;
-    const expectedSubstring = `❌  ${chalk.bold(stackName)} failed: ResourceNotReady: Resource is not in the state deploymentCompleted`;
+    const expectedSubstring = 'Resource is not in the state deploymentCompleted';
 
     expect(deployOutput).toContain(expectedSubstring);
     expect(deployOutput).not.toContain('hotswapped!');

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

@@ -1,6 +1,7 @@
 import * as cxapi from '@aws-cdk/cx-api';
 import * as cdk_assets from 'cdk-assets';
 import { AssetManifest, IManifestEntry } from 'cdk-assets';
+import * as chalk from 'chalk';
 import { Tag } from '../cdk-toolkit';
 import { debug, warning, error } from '../logging';
 import { Mode } from './aws-auth/credentials';
@@ -700,7 +701,7 @@ export class Deployments {
     if (existing) {
       return existing;
     }
-    const prefix = stackName ? `${stackName}: ` : '';
+    const prefix = stackName ? `${chalk.bold(stackName)}: ` : '';
     const publisher = new cdk_assets.AssetPublishing(assetManifest, {
       aws: new PublishingAws(this.sdkProvider, env),
       progressListener: new ParallelSafeAssetProgress(prefix, this.props.quiet ?? false),
@@ -719,7 +720,7 @@ class ParallelSafeAssetProgress implements cdk_assets.IPublishProgressListener {
 
   public onPublishEvent(type: cdk_assets.EventType, event: cdk_assets.IPublishProgress): void {
     const handler = this.quiet && type !== 'fail' ? debug : EVENT_TO_LOGGER[type];
-    handler(`${this.prefix} ${type}: ${event.message}`);
+    handler(`${this.prefix}${type}: ${event.message}`);
   }
 }
 

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

@@ -138,15 +138,11 @@ export class CdkToolkit {
 
       const template = deserializeStructure(await fs.readFile(options.templatePath, { encoding: 'UTF-8' }));
       diffs = options.securityOnly
-        ? numberFromBool(printSecurityDiff(template, stacks.firstStack, RequireApproval.Broadening, undefined))
-        : printStackDiff(template, stacks.firstStack, strict, contextLines, quiet, undefined, false, stream);
+        ? numberFromBool(printSecurityDiff(template, stacks.firstStack, RequireApproval.Broadening, quiet))
+        : printStackDiff(template, stacks.firstStack, strict, contextLines, quiet, undefined, undefined, false, stream);
     } else {
       // Compare N stacks against deployed templates
       for (const stack of stacks.stackArtifacts) {
-        if (!quiet) {
-          stream.write(format('Stack %s\n', chalk.bold(stack.displayName)));
-        }
-
         const templateWithNestedStacks = await this.props.deployments.readCurrentTemplateWithNestedStacks(
           stack, options.compareAgainstProcessedTemplate,
         );
@@ -170,7 +166,9 @@ export class CdkToolkit {
             });
           } 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');
+            if (!quiet) {
+              stream.write(`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`);
+            }
             stackExists = false;
           }
 
@@ -190,14 +188,12 @@ export class CdkToolkit {
           }
         }
 
-        if (resourcesToImport) {
-          stream.write('Parameters and rules created during migration do not affect resource configuration.\n');
-        }
-
         const stackCount =
         options.securityOnly
-          ? (numberFromBool(printSecurityDiff(currentTemplate, stack, RequireApproval.Broadening, changeSet)))
-          : (printStackDiff(currentTemplate, stack, strict, contextLines, quiet, changeSet, !!resourcesToImport, stream, nestedStacks));
+          ? (numberFromBool(printSecurityDiff(currentTemplate, stack, RequireApproval.Broadening, quiet, stack.displayName, changeSet)))
+          : (printStackDiff(
+            currentTemplate, stack, strict, contextLines, quiet, stack.displayName, changeSet, !!resourcesToImport, stream, nestedStacks,
+          ));
 
         diffs += stackCount;
       }
@@ -375,9 +371,14 @@ export class CdkToolkit {
         print('Stack ARN:');
 
         data(result.stackArn);
-      } catch (e) {
-        error('\n ❌  %s failed: %s', chalk.bold(stack.displayName), e);
-        throw e;
+      } catch (e: any) {
+        // It has to be exactly this string because an integration test tests for
+        // "bold(stackname) failed: ResourceNotReady: <error>"
+        throw new Error([
+          `❌  ${chalk.bold(stack.stackName)} failed:`,
+          ...e.code ? [`${e.code}:`] : [],
+          e.message,
+        ].join(' '));
       } finally {
         if (options.cloudWatchLogMonitor) {
           const foundLogGroupsResult = await findCloudWatchLogGroups(this.props.sdkProvider, stack);
@@ -405,33 +406,28 @@ export class CdkToolkit {
       warning('⚠️ The --concurrency flag only supports --progress "events". Switching to "events".');
     }
 
-    try {
-      const stacksAndTheirAssetManifests = stacks.flatMap(stack => [
-        stack,
-        ...stack.dependencies.filter(cxapi.AssetManifestArtifact.isAssetManifestArtifact),
-      ]);
-      const workGraph = new WorkGraphBuilder(prebuildAssets).build(stacksAndTheirAssetManifests);
-
-      // Unless we are running with '--force', skip already published assets
-      if (!options.force) {
-        await this.removePublishedAssets(workGraph, options);
-      }
+    const stacksAndTheirAssetManifests = stacks.flatMap(stack => [
+      stack,
+      ...stack.dependencies.filter(cxapi.AssetManifestArtifact.isAssetManifestArtifact),
+    ]);
+    const workGraph = new WorkGraphBuilder(prebuildAssets).build(stacksAndTheirAssetManifests);
 
-      const graphConcurrency: Concurrency = {
-        'stack': concurrency,
-        'asset-build': 1, // This will be CPU-bound/memory bound, mostly matters for Docker builds
-        'asset-publish': (options.assetParallelism ?? true) ? 8 : 1, // This will be I/O-bound, 8 in parallel seems reasonable
-      };
-
-      await workGraph.doParallel(graphConcurrency, {
-        deployStack,
-        buildAsset,
-        publishAsset,
-      });
-    } catch (e) {
-      error('\n ❌ Deployment failed: %s', e);
-      throw e;
+    // Unless we are running with '--force', skip already published assets
+    if (!options.force) {
+      await this.removePublishedAssets(workGraph, options);
     }
+
+    const graphConcurrency: Concurrency = {
+      'stack': concurrency,
+      'asset-build': 1, // This will be CPU-bound/memory bound, mostly matters for Docker builds
+      'asset-publish': (options.assetParallelism ?? true) ? 8 : 1, // This will be I/O-bound, 8 in parallel seems reasonable
+    };
+
+    await workGraph.doParallel(graphConcurrency, {
+      deployStack,
+      buildAsset,
+      publishAsset,
+    });
   }
 
   public async watch(options: WatchOptions) {

packages/aws-cdk/lib/diff.ts

@@ -31,13 +31,22 @@ export function printStackDiff(
   strict: boolean,
   context: number,
   quiet: boolean,
+  stackName?: string,
   changeSet?: DescribeChangeSetOutput,
   isImport?: boolean,
   stream: FormatStream = process.stderr,
   nestedStackTemplates?: { [nestedStackLogicalId: string]: NestedStackTemplates }): number {
-
   let diff = fullDiff(oldTemplate, newTemplate.template, changeSet, isImport);
 
+  // must output the stack name if there are differences, even if quiet
+  if (stackName && (!quiet || !diff.isEmpty)) {
+    stream.write(format('Stack %s\n', chalk.bold(stackName)));
+  }
+
+  if (!quiet && isImport) {
+    stream.write('Parameters and rules created during migration do not affect resource configuration.\n');
+  }
+
   // detect and filter out mangled characters from the diff
   let filteredChangesCount = 0;
   if (diff.differenceCount && !strict) {
@@ -74,9 +83,6 @@ export function printStackDiff(
       break;
     }
     const nestedStack = nestedStackTemplates[nestedStackLogicalId];
-    if (!quiet) {
-      stream.write(format('Stack %s\n', chalk.bold(nestedStack.physicalName ?? nestedStackLogicalId)));
-    }
 
     (newTemplate as any)._template = nestedStack.generatedTemplate;
     stackDiffCount += printStackDiff(
@@ -85,6 +91,7 @@ export function printStackDiff(
       strict,
       context,
       quiet,
+      nestedStack.physicalName ?? nestedStackLogicalId,
       undefined,
       isImport,
       stream,
@@ -112,10 +119,18 @@ export function printSecurityDiff(
   oldTemplate: any,
   newTemplate: cxapi.CloudFormationStackArtifact,
   requireApproval: RequireApproval,
+  quiet?: boolean,
+  stackName?: string,
   changeSet?: DescribeChangeSetOutput,
+  stream: FormatStream = process.stderr,
 ): boolean {
   const diff = fullDiff(oldTemplate, newTemplate.template, changeSet);
 
+  // must output the stack name if there are differences, even if quiet
+  if (!quiet || !diff.isEmpty) {
+    stream.write(format('Stack %s\n', chalk.bold(stackName)));
+  }
+
   if (difRequiresApproval(diff, requireApproval)) {
     // eslint-disable-next-line max-len
     warning(`This deployment will make potentially sensitive changes according to your current security approval level (--require-approval ${requireApproval}).`);