Merge branch 'main' into huijbers/cli-logging

packages/aws-cdk/CONTRIBUTING.md

@@ -257,3 +257,20 @@ will change, and needs to be regenerated. For you, this means that:
 2. When you build the CLI locally, you must ensure your dependencies are up to date by running `yarn install` inside the CLI package.
 Otherwise, you might get an error like so: `aws-cdk: - [bundle/outdated-attributions] THIRD_PARTY_LICENSES is outdated (fixable)`.
 
+## Source Maps
+
+The source map handling is not entirely intuitive, so it bears some description here.
+
+There are 2 steps to producing a CLI build:
+
+- First we compile TypeScript to JavaScript. This step is configured to produce inline sourcemaps.
+- Then we bundle JavaScript -> bundled JavaScript. This removes the inline
+  sourcemaps, and also is configured to *not* emit a fresh sourcemap file.
+
+The upshot is that we don't vend a 30+MB sourcemap to customers that they have no use for,
+and that we don't slow down Node loading those sourcemaps, while if we are locally developing
+and testing the sourcemaps are still present and can be used.
+
+During the CLI initialization, we always enable source map support: if we are developing
+then source maps are present and can be used, while in a production build there will be no
+source maps so there's nothing to load anyway.

packages/aws-cdk/bin/cdk

@@ -1,7 +1,6 @@
 #!/usr/bin/env node
 
 // source maps must be enabled before importing files
-if (process.argv.includes('--debug')) {
-  process.setSourceMapsEnabled(true);
-}
+process.setSourceMapsEnabled(true);
+
 require('./cdk.js');

packages/aws-cdk/jest.config.js

@@ -1,5 +1,6 @@
 const baseConfig = require('@aws-cdk/cdk-build-tools/config/jest.config');
-module.exports = {
+
+const config = {
     ...baseConfig,
     coverageThreshold: {
         global: {
@@ -21,4 +22,18 @@ module.exports = {
     // We have many tests here that commonly time out
     testTimeout: 30_000,
     setupFilesAfterEnv: ["<rootDir>/test/jest-setup-after-env.ts"],
+
+    // Randomize test order: this will catch tests that accidentally pass or
+    // fail because they rely on shared mutable state left by other tests
+    // (files on disk, global mocks, etc).
+    randomize: true,
 };
+
+// Disable coverage running in the VSCode debug terminal: we never want coverage
+// when we're attaching a debugger because the coverage injection messes with
+// the source maps.
+if (process.env.VSCODE_INJECTION) {
+    config.collectCoverage = false;
+}
+
+module.exports = config;

packages/aws-cdk/lib/api/aws-auth/sdk.ts

@@ -953,7 +953,6 @@ export class SDK {
         });
         const command = new GetCallerIdentityCommand({});
         const result = await client.send(command);
-        debug(result.Account!, result.Arn, result.UserId);
         const accountId = result.Account;
         const partition = result.Arn!.split(':')[1];
         if (!accountId) {

packages/aws-cdk/lib/api/deploy-stack.ts

@@ -37,7 +37,7 @@ import { StringWithoutPlaceholders } from './util/placeholders';
 export type DeployStackResult =
   | SuccessfulDeployStackResult
   | NeedRollbackFirstDeployStackResult
-  | ReplacementRequiresNoRollbackStackResult
+  | ReplacementRequiresRollbackStackResult
   ;
 
 /** Successfully deployed a stack */
@@ -52,11 +52,12 @@ export interface SuccessfulDeployStackResult {
 export interface NeedRollbackFirstDeployStackResult {
   readonly type: 'failpaused-need-rollback-first';
   readonly reason: 'not-norollback' | 'replacement';
+  readonly status: string;
 }
 
-/** The upcoming change has a replacement, which requires deploying without --no-rollback */
-export interface ReplacementRequiresNoRollbackStackResult {
-  readonly type: 'replacement-requires-norollback';
+/** The upcoming change has a replacement, which requires deploying with --rollback */
+export interface ReplacementRequiresRollbackStackResult {
+  readonly type: 'replacement-requires-rollback';
 }
 
 export function assertIsSuccessfulDeployStackResult(x: DeployStackResult): asserts x is SuccessfulDeployStackResult {
@@ -512,13 +513,13 @@ class FullCloudFormationDeployment {
     const isPausedFailState = this.cloudFormationStack.stackStatus.isRollbackable;
     const rollback = this.options.rollback ?? true;
     if (isPausedFailState && replacement) {
-      return { type: 'failpaused-need-rollback-first', reason: 'replacement' };
+      return { type: 'failpaused-need-rollback-first', reason: 'replacement', status: this.cloudFormationStack.stackStatus.name };
     }
-    if (isPausedFailState && !rollback) {
-      return { type: 'failpaused-need-rollback-first', reason: 'not-norollback' };
+    if (isPausedFailState && rollback) {
+      return { type: 'failpaused-need-rollback-first', reason: 'not-norollback', status: this.cloudFormationStack.stackStatus.name };
     }
     if (!rollback && replacement) {
-      return { type: 'replacement-requires-norollback' };
+      return { type: 'replacement-requires-rollback' };
     }
 
     return this.executeChangeSet(changeSetDescription);

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

@@ -436,8 +436,8 @@ export class CdkToolkit {
 
             case 'failpaused-need-rollback-first': {
               const motivation = r.reason === 'replacement'
-                ? 'Stack is in a paused fail state and change includes a replacement which cannot be deployed with "--no-rollback"'
-                : 'Stack is in a paused fail state and command line arguments do not include "--no-rollback"';
+                ? `Stack is in a paused fail state (${r.status}) and change includes a replacement which cannot be deployed with "--no-rollback"`
+                : `Stack is in a paused fail state (${r.status}) and command line arguments do not include "--no-rollback"`;
 
               if (options.force) {
                 warning(`${motivation}. Rolling back first (--force).`);
@@ -461,7 +461,7 @@ export class CdkToolkit {
               break;
             }
 
-            case 'replacement-requires-norollback': {
+            case 'replacement-requires-rollback': {
               const motivation = 'Change includes a replacement which cannot be deployed with "--no-rollback"';
 
               if (options.force) {

packages/aws-cdk/lib/cli.ts

@@ -561,6 +561,7 @@ function determineHotswapMode(hotswap?: boolean, hotswapFallback?: boolean, watc
   return hotswapMode;
 }
 
+/* istanbul ignore next: we never call this in unit tests */
 export function cli(args: string[] = process.argv.slice(2)) {
   exec(args)
     .then(async (value) => {
@@ -570,7 +571,10 @@ export function cli(args: string[] = process.argv.slice(2)) {
     })
     .catch((err) => {
       error(err.message);
-      if (err.stack) {
+
+      // Log the stack trace if we're on a developer workstation. Otherwise this will be into a minified
+      // file and the printed code line and stack trace are huge and useless.
+      if (err.stack && version.isDeveloperBuild()) {
         debug(err.stack);
       }
       process.exitCode = 1;

packages/aws-cdk/lib/config.ts

@@ -21,7 +21,7 @@ export function makeConfig(): CliConfig {
       'ignore-errors': { type: 'boolean', default: false, desc: 'Ignores synthesis errors, which will likely produce an invalid output' },
       'json': { type: 'boolean', alias: 'j', desc: 'Use JSON output instead of YAML when templates are printed to STDOUT', default: false },
       'verbose': { type: 'boolean', alias: 'v', desc: 'Show debug logs (specify multiple times to increase verbosity)', default: false, count: true },
-      'debug': { type: 'boolean', desc: 'Enable emission of additional debugging information, such as creation stack traces of tokens', default: false },
+      'debug': { type: 'boolean', desc: 'Debug the CDK app. Log additional information during synthesis, such as creation stack traces of tokens (sets CDK_DEBUG, will slow down synthesis)', default: false },
       'profile': { type: 'string', desc: 'Use the indicated AWS profile as the default environment', requiresArg: true },
       'proxy': { type: 'string', desc: 'Use the indicated proxy. Will read from HTTPS_PROXY environment variable if not specified', requiresArg: true },
       'ca-bundle-path': { type: 'string', desc: 'Path to CA certificate to use when validating HTTPS requests. Will read from AWS_CA_BUNDLE environment variable if not specified', requiresArg: true },

packages/aws-cdk/lib/parse-command-line-arguments.ts

@@ -73,7 +73,7 @@ export function parseCommandLineArguments(
     })
     .option('debug', {
       type: 'boolean',
-      desc: 'Enable emission of additional debugging information, such as creation stack traces of tokens',
+      desc: 'Debug the CDK app. Log additional information during synthesis, such as creation stack traces of tokens (sets CDK_DEBUG, will slow down synthesis)',
       default: false,
     })
     .option('profile', {

packages/aws-cdk/lib/version.ts

@@ -15,6 +15,10 @@ const UPGRADE_DOCUMENTATION_LINKS: Record<number, string> = {
 
 export const DISPLAY_VERSION = `${versionNumber()} (build ${commit()})`;
 
+export function isDeveloperBuild(): boolean {
+  return versionNumber() === '0.0.0';
+}
+
 export function versionNumber(): string {
   // eslint-disable-next-line @typescript-eslint/no-require-imports
   return require(path.join(rootDir(), 'package.json')).version.replace(/\+[0-9a-f]+$/, '');

packages/aws-cdk/package.json

@@ -57,7 +57,6 @@
       "entryPoints": [
         "lib/index.js"
       ],
-      "sourcemap": "linked",
       "minifyWhitespace": true
     }
   },

packages/aws-cdk/test/api/deploy-stack.test.ts

@@ -1127,30 +1127,41 @@ describe('disable rollback', () => {
 });
 
 test.each([
-  [StackStatus.UPDATE_FAILED, 'failpaused-need-rollback-first'],
-  [StackStatus.CREATE_COMPLETE, 'replacement-requires-norollback'],
-])('no-rollback and replacement is disadvised: %p -> %p', async (stackStatus, expectedType) => {
+  // From a failed state, a --no-rollback is possible as long as there is not a replacement
+  [StackStatus.UPDATE_FAILED, 'no-rollback', 'no-replacement', 'did-deploy-stack'],
+  [StackStatus.UPDATE_FAILED, 'no-rollback', 'replacement', 'failpaused-need-rollback-first'],
+  // Any combination of UPDATE_FAILED & rollback always requires a rollback first
+  [StackStatus.UPDATE_FAILED, 'rollback', 'replacement', 'failpaused-need-rollback-first'],
+  [StackStatus.UPDATE_FAILED, 'rollback', 'no-replacement', 'failpaused-need-rollback-first'],
+  // From a stable state, any deployment containing a replacement requires a regular deployment (--rollback)
+  [StackStatus.UPDATE_COMPLETE, 'no-rollback', 'replacement', 'replacement-requires-rollback'],
+] satisfies Array<[StackStatus, 'rollback' | 'no-rollback', 'replacement' | 'no-replacement', string]>)
+('no-rollback and replacement is disadvised: %s %s %s -> %s', async (stackStatus, rollback, replacement, expectedType) => {
   // GIVEN
   givenTemplateIs(FAKE_STACK.template);
   givenStackExists({
-    NotificationARNs: ['arn:aws:sns:bermuda-triangle-1337:123456789012:TestTopic'],
+    // First call
     StackStatus: stackStatus,
+  }, {
+    // Later calls
+    StackStatus: 'UPDATE_COMPLETE',
   });
-  givenChangeSetContainsReplacement();
+  givenChangeSetContainsReplacement(replacement === 'replacement');
 
   // WHEN
   const result = await deployStack({
     ...standardDeployStackArguments(),
     stack: FAKE_STACK,
-    rollback: false,
+    rollback: rollback === 'rollback',
+    force: true, // Bypass 'canSkipDeploy'
   });
 
   // THEN
   expect(result.type).toEqual(expectedType);
 });
 
 test('assertIsSuccessfulDeployStackResult does what it says', () => {
-  expect(() => assertIsSuccessfulDeployStackResult({ type: 'replacement-requires-norollback' })).toThrow();
+  expect(() => assertIsSuccessfulDeployStackResult({ type: 'replacement-requires-rollback' })).toThrow();
 });
 /**
  * Set up the mocks so that it looks like the stack exists to start with
@@ -1162,12 +1173,14 @@ function givenStackExists(...overrides: Array<Partial<Stack>>) {
     overrides = [{}];
   }
 
+  let handler = mockCloudFormationClient.on(DescribeStacksCommand);
+
   for (const override of overrides.slice(0, overrides.length - 1)) {
-    mockCloudFormationClient.on(DescribeStacksCommand).resolvesOnce({
+    handler = handler.resolvesOnce({
       Stacks: [{ ...baseResponse, ...override }],
     });
   }
-  mockCloudFormationClient.on(DescribeStacksCommand).resolves({
+  handler.resolves({
     Stacks: [{ ...baseResponse, ...overrides[overrides.length - 1] }],
   });
 }
@@ -1178,10 +1191,10 @@ function givenTemplateIs(template: any) {
   });
 }
 
-function givenChangeSetContainsReplacement() {
+function givenChangeSetContainsReplacement(replacement: boolean) {
   mockCloudFormationClient.on(DescribeChangeSetCommand).resolves({
     Status: 'CREATE_COMPLETE',
-    Changes: [
+    Changes: replacement ? [
       {
         Type: 'Resource',
         ResourceChange: {
@@ -1205,6 +1218,6 @@ function givenChangeSetContainsReplacement() {
           ],
         },
       },
-    ],
+    ] : [],
   });
 }