Merge branch 'master' into rybickic/revert-ecs-patterns-flag

packages/@aws-cdk/aws-rds/README.md

@@ -185,7 +185,7 @@ const rule = instance.onEvent('InstanceEvent', { target: new targets.LambdaFunct
 
 ## Login credentials
 
-By default, database instances and clusters will have `admin` user with an auto-generated password.
+By default, database instances and clusters (with the exception of `DatabaseInstanceFromSnapshot` and `ServerlessClusterFromSnapshot`) will have `admin` user with an auto-generated password.
 An alternative username (and password) may be specified for the admin user instead of the default.
 
 The following examples use a `DatabaseInstance`, but the same usage is applicable to `DatabaseCluster`.
@@ -232,6 +232,27 @@ new rds.DatabaseInstance(this, 'InstanceWithCustomizedSecret', {
 });
 ```
 
+### Snapshot credentials
+
+As noted above, Databases created with `DatabaseInstanceFromSnapshot` or `ServerlessClusterFromSnapshot` will not create user and auto-generated password by default because it's not possible to change the master username for a snapshot. Instead, they will use the existing username and password from the snapshot. You can still generate a new password - to generate a secret similarly to the other constructs, pass in credentials with `fromGeneratedSecret()` or `fromGeneratedPassword()`.
+
+```ts
+declare const vpc: ec2.Vpc;
+const engine = rds.DatabaseInstanceEngine.postgres({ version: rds.PostgresEngineVersion.VER_12_3 });
+const myKey = new kms.Key(this, 'MyKey');
+
+new rds.DatabaseInstanceFromSnapshot(this, 'InstanceFromSnapshotWithCustomizedSecret', {
+  engine,
+  vpc,
+  snapshotIdentifier: 'mySnapshot',
+  credentials: rds.SnapshotCredentials.fromGeneratedSecret('username', {
+    encryptionKey: myKey,
+    excludeCharacters: '!&*^#@()',
+    replicaRegions: [{ region: 'eu-west-1' }, { region: 'eu-west-2' }],
+  }),
+});
+```
+
 ## Connecting
 
 To control who can access the cluster or instance, use the `.connections` attribute. RDS databases have

packages/@aws-cdk/core/lib/cfn-element.ts

@@ -1,4 +1,5 @@
 import * as cxschema from '@aws-cdk/cloud-assembly-schema';
+import * as cxapi from '@aws-cdk/cx-api';
 import { Construct, Node } from 'constructs';
 
 // v2 - keep this import as a separate section to reduce merge conflict when forward merging with the v2 branch.
@@ -64,7 +65,9 @@ export abstract class CfnElement extends CoreConstruct {
       displayHint: `${notTooLong(Node.of(this).path)}.LogicalID`,
     });
 
-    Node.of(this).addMetadata(cxschema.ArtifactMetadataEntryType.LOGICAL_ID, this.logicalId, this.constructor);
+    if (!this.node.tryGetContext(cxapi.DISABLE_LOGICAL_ID_METADATA)) {
+      Node.of(this).addMetadata(cxschema.ArtifactMetadataEntryType.LOGICAL_ID, this.logicalId, this.constructor);
+    }
   }
 
   /**

packages/@aws-cdk/core/test/synthesis.test.ts

@@ -3,6 +3,7 @@ import * as os from 'os';
 import * as path from 'path';
 import { testDeprecated } from '@aws-cdk/cdk-build-tools';
 import * as cxschema from '@aws-cdk/cloud-assembly-schema';
+import * as cxapi from '@aws-cdk/cx-api';
 import * as cdk from '../lib';
 
 function createModernApp() {
@@ -36,7 +37,6 @@ describe('synthesis', () => {
         },
       }),
     });
-
   });
 
   test('synthesis respects disabling tree metadata', () => {
@@ -45,7 +45,52 @@ describe('synthesis', () => {
     });
     const assembly = app.synth();
     expect(list(assembly.directory)).toEqual(['cdk.out', 'manifest.json']);
+  });
+
+  test('synthesis respects disabling logicalId metadata', () => {
+    const app = new cdk.App({
+      context: {
+        [cxapi.DISABLE_LOGICAL_ID_METADATA]: true,
+      },
+    });
+    const stack = new cdk.Stack(app, 'one-stack');
+    new cdk.CfnResource(stack, 'MagicResource', { type: 'Resource::Type' });
+
+    // WHEN
+    const session = app.synth();
+
+    // THEN
+    expect(Object.keys((session.manifest.artifacts ?? {})['one-stack'])).not.toContain('metadata');
+  });
+
+  test('synthesis respects disabling logicalId metadata, and does not disable other metadata', () => {
+    const app = new cdk.App({
+      context: {
+        [cxapi.DISABLE_LOGICAL_ID_METADATA]: true,
+      },
+      stackTraces: false,
+    });
+    const stack = new cdk.Stack(app, 'one-stack', { tags: { boomTag: 'BOOM' } });
+    new cdk.CfnResource(stack, 'MagicResource', { type: 'Resource::Type' });
 
+    // WHEN
+    const session = app.synth();
+
+    // THEN
+    expect(session.manifest.artifacts?.['one-stack'].metadata).toEqual({
+      '/one-stack': [
+        {
+          type: 'aws:cdk:stack-tags',
+          data: [
+            {
+              key: 'boomTag',
+              value: 'BOOM',
+            },
+          ],
+        },
+      ],
+      // no logicalId entry
+    });
   });
 
   test('single empty stack', () => {
@@ -58,7 +103,6 @@ describe('synthesis', () => {
 
     // THEN
     expect(list(session.directory).includes('one-stack.template.json')).toEqual(true);
-
   });
 
   test('some random construct implements "synthesize"', () => {
@@ -112,7 +156,6 @@ describe('synthesis', () => {
         },
       },
     });
-
   });
 
   test('random construct uses addCustomSynthesis', () => {
@@ -172,7 +215,6 @@ describe('synthesis', () => {
         },
       },
     });
-
   });
 
   testDeprecated('it should be possible to synthesize without an app', () => {
@@ -220,7 +262,6 @@ describe('synthesis', () => {
     expect(stack.templateFile).toEqual('hey.json');
     expect(stack.parameters).toEqual({ paramId: 'paramValue', paramId2: 'paramValue2' });
     expect(stack.environment).toEqual({ region: 'us-east-1', account: 'unknown-account', name: 'aws://unknown-account/us-east-1' });
-
   });
 });
 

packages/@aws-cdk/cx-api/lib/app.ts

@@ -39,6 +39,12 @@ export const DISABLE_ASSET_STAGING_CONTEXT = 'aws:cdk:disable-asset-staging';
  */
 export const DISABLE_METADATA_STACK_TRACE = 'aws:cdk:disable-stack-trace';
 
+/**
+ * If this context key is set, the CDK will not store logical ID
+ * metadata in the manifest.
+ */
+export const DISABLE_LOGICAL_ID_METADATA = 'aws:cdk:disable-logicalId-metadata';
+
 /**
  * Run bundling for stacks specified in this context key
  */

packages/@aws-cdk/integ-runner/lib/cli.ts

@@ -27,7 +27,9 @@ async function main() {
     .options('max-workers', { type: 'number', desc: 'The max number of workerpool workers to use when running integration tests in parallel', default: 16 })
     .options('exclude', { type: 'boolean', desc: 'All tests should be run, except for the list of tests provided', default: false })
     .options('from-file', { type: 'string', desc: 'Import tests to include or exclude from a file' })
+    .option('inspect-failures', { type: 'boolean', desc: 'Keep the integ test cloud assembly if a failure occurs for inspection', default: false })
     .option('disable-update-workflow', { type: 'boolean', default: false, desc: 'If this is "true" then the stack update workflow will be disabled' })
+    .strict()
     .argv;
 
   const pool = workerpool.pool(path.join(__dirname, '../lib/workers/extract/index.js'), {
@@ -70,7 +72,10 @@ async function main() {
     // always run snapshot tests, but if '--force' is passed then
     // run integration tests on all failed tests, not just those that
     // failed snapshot tests
-    failedSnapshots = await runSnapshotTests(pool, testsFromArgs);
+    failedSnapshots = await runSnapshotTests(pool, testsFromArgs, {
+      retain: argv['inspect-failures'],
+      verbose: argv.verbose,
+    });
     for (const failure of failedSnapshots) {
       destructiveChanges.push(...failure.destructiveChanges ?? []);
     }

packages/@aws-cdk/integ-runner/lib/runner/runner-base.ts

@@ -129,6 +129,8 @@ export abstract class IntegRunner {
 
   protected readonly profile?: string;
 
+  protected readonly cdkExecutable: string;
+
   protected _destructiveChanges?: DestructiveChange[];
   private legacyContext?: Record<string, any>;
 
@@ -150,8 +152,10 @@ export abstract class IntegRunner {
     this.relativeSnapshotDir = `${testName}.integ.snapshot`;
     this.sourceFilePath = path.join(this.directory, parsed.base);
     this.cdkContextPath = path.join(this.directory, 'cdk.context.json');
+
+    this.cdkExecutable = require.resolve('aws-cdk/bin/cdk');
     this.cdk = options.cdk ?? new CdkCliWrapper({
-      cdkExecutable: require.resolve('aws-cdk/bin/cdk'),
+      cdkExecutable: this.cdkExecutable,
       directory: this.directory,
       env: {
         ...options.env,

packages/@aws-cdk/integ-runner/lib/runner/snapshot-test-runner.ts

@@ -2,7 +2,7 @@ import * as path from 'path';
 import { Writable, WritableOptions } from 'stream';
 import { StringDecoder, NodeStringDecoder } from 'string_decoder';
 import { diffTemplate, formatDifferences, ResourceDifference, ResourceImpact } from '@aws-cdk/cloudformation-diff';
-import { Diagnostic, DiagnosticReason, DestructiveChange } from '../workers/common';
+import { Diagnostic, DiagnosticReason, DestructiveChange, SnapshotVerificationOptions } from '../workers/common';
 import { canonicalizeTemplate } from './private/canonicalize-assets';
 import { AssemblyManifestReader } from './private/cloud-assembly';
 import { IntegRunnerOptions, IntegRunner, DEFAULT_SYNTH_OPTIONS } from './runner-base';
@@ -22,7 +22,8 @@ export class IntegSnapshotRunner extends IntegRunner {
    *
    * @returns any diagnostics and any destructive changes
    */
-  public testSnapshot(): { diagnostics: Diagnostic[], destructiveChanges: DestructiveChange[] } {
+  public testSnapshot(options: SnapshotVerificationOptions = {}): { diagnostics: Diagnostic[], destructiveChanges: DestructiveChange[] } {
+    let doClean = true;
     try {
       // read the existing snapshot
       const expectedStacks = this.readAssembly(this.snapshotDir);
@@ -39,17 +40,19 @@ export class IntegSnapshotRunner extends IntegRunner {
       // the cdkOutDir exists already, but for some reason generateActualSnapshot
       // generates an incorrect snapshot and I have no idea why so synth again here
       // to produce the "correct" snapshot
+      const env = {
+        ...DEFAULT_SYNTH_OPTIONS.env,
+        CDK_CONTEXT_JSON: JSON.stringify(this.getContext()),
+      };
       this.cdk.synthFast({
         execCmd: this.cdkApp.split(' '),
-        env: {
-          ...DEFAULT_SYNTH_OPTIONS.env,
-          CDK_CONTEXT_JSON: JSON.stringify(this.getContext()),
-        },
+        env,
         output: this.cdkOutDir,
       });
 
       // read the "actual" snapshot
-      const actualStacks = this.readAssembly(path.join(this.directory, this.cdkOutDir));
+      const actualDir = path.join(this.directory, this.cdkOutDir);
+      const actualStacks = this.readAssembly(actualDir);
       // only diff stacks that are part of the test case
       const actualStacksToDiff: Record<string, any> = {};
       for (const [stackName, template] of Object.entries(actualStacks)) {
@@ -60,11 +63,45 @@ export class IntegSnapshotRunner extends IntegRunner {
 
       // diff the existing snapshot (expected) with the integration test (actual)
       const diagnostics = this.diffAssembly(expectedStacksToDiff, actualStacksToDiff);
+
+      if (diagnostics.diagnostics.length) {
+        // Attach additional messages to the first diagnostic
+        const additionalMessages: string[] = [];
+
+        if (options.retain) {
+          additionalMessages.push(
+            `(Failure retained) Expected: ${path.relative(process.cwd(), this.snapshotDir)}`,
+            `                   Actual:   ${path.relative(process.cwd(), actualDir)}`,
+          ),
+          doClean = false;
+        }
+
+        if (options.verbose) {
+          // Show the command necessary to repro this
+          const envSet = Object.entries(env)
+            .filter(([k, _]) => k !== 'CDK_CONTEXT_JSON')
+            .map(([k, v]) => `${k}='${v}'`);
+          const envCmd = envSet.length > 0 ? ['env', ...envSet] : [];
+
+          additionalMessages.push(
+            'Repro:',
+            `  ${[...envCmd, 'cdk synth', `-a '${this.cdkApp}'`, `-o '${this.cdkOutDir}'`, ...Object.entries(this.getContext()).flatMap(([k, v]) => typeof v !== 'object' ? [`-c '${k}=${v}'`] : [])].join(' ')}`,
+          );
+        }
+
+        diagnostics.diagnostics[0] = {
+          ...diagnostics.diagnostics[0],
+          additionalMessages,
+        };
+      }
+
       return diagnostics;
     } catch (e) {
       throw e;
     } finally {
-      this.cleanup();
+      if (doClean) {
+        this.cleanup();
+      }
     }
   }
 

packages/@aws-cdk/integ-runner/lib/workers/common.ts

@@ -89,6 +89,22 @@ export interface IntegRunnerMetrics {
   readonly profile?: string;
 }
 
+export interface SnapshotVerificationOptions {
+  /**
+   * Retain failed snapshot comparisons
+   *
+   * @default false
+   */
+  readonly retain?: boolean;
+
+  /**
+   * Verbose mode
+   *
+   * @default false
+   */
+  readonly verbose?: boolean;
+}
+
 /**
  * Integration test results
  */
@@ -208,6 +224,11 @@ export interface Diagnostic {
    * The reason for the diagnostic
    */
   readonly reason: DiagnosticReason;
+
+  /**
+   * Additional messages to print
+   */
+  readonly additionalMessages?: string[];
 }
 
 export function printSummary(total: number, failed: number): void {
@@ -251,4 +272,7 @@ export function printResults(diagnostic: Diagnostic): void {
     case DiagnosticReason.ASSERTION_FAILED:
       logger.error('   %s - Assertions Failed! %s\n%s', diagnostic.testName, chalk.gray(`${diagnostic.duration}s`), diagnostic.message);
   }
+  for (const addl of diagnostic.additionalMessages ?? []) {
+    logger.print(`      ${addl}`);
+  }
 }

packages/@aws-cdk/integ-runner/lib/workers/extract/extract_worker.ts

@@ -1,7 +1,7 @@
 import * as workerpool from 'workerpool';
 import { IntegSnapshotRunner, IntegTestRunner } from '../../runner';
 import { IntegTestConfig } from '../../runner/integration-tests';
-import { DiagnosticReason, IntegTestWorkerConfig, formatAssertionResults } from '../common';
+import { DiagnosticReason, IntegTestWorkerConfig, SnapshotVerificationOptions, Diagnostic, formatAssertionResults } from '../common';
 import { IntegTestBatchRequest } from '../integ-test-worker';
 
 /**
@@ -84,7 +84,7 @@ export function integTestWorker(request: IntegTestBatchRequest): IntegTestWorker
  * if there is an existing snapshot, and if there is will
  * check if there are any changes
  */
-export function snapshotTestWorker(test: IntegTestConfig): IntegTestWorkerConfig[] {
+export function snapshotTestWorker(test: IntegTestConfig, options: SnapshotVerificationOptions = {}): IntegTestWorkerConfig[] {
   const failedTests = new Array<IntegTestWorkerConfig>();
   const runner = new IntegSnapshotRunner({ fileName: test.fileName, directory: test.directory });
   const start = Date.now();
@@ -98,12 +98,12 @@ export function snapshotTestWorker(test: IntegTestConfig): IntegTestWorkerConfig
       });
       failedTests.push(test);
     } else {
-      const { diagnostics, destructiveChanges } = runner.testSnapshot();
+      const { diagnostics, destructiveChanges } = runner.testSnapshot(options);
       if (diagnostics.length > 0) {
         diagnostics.forEach(diagnostic => workerpool.workerEmit({
           ...diagnostic,
           duration: (Date.now() - start) / 1000,
-        }));
+        } as Diagnostic));
         failedTests.push({
           fileName: test.fileName,
           directory: test.directory,
@@ -115,7 +115,7 @@ export function snapshotTestWorker(test: IntegTestConfig): IntegTestWorkerConfig
           testName: runner.testName,
           message: 'Success',
           duration: (Date.now() - start) / 1000,
-        });
+        } as Diagnostic);
       }
     }
   } catch (e) {
@@ -125,7 +125,7 @@ export function snapshotTestWorker(test: IntegTestConfig): IntegTestWorkerConfig
       testName: runner.testName,
       reason: DiagnosticReason.SNAPSHOT_FAILED,
       duration: (Date.now() - start) / 1000,
-    });
+    } as Diagnostic);
   }
 
   return failedTests;