fix(rds): clusters created from snapshots generate incorrect passwords

packages/@aws-cdk/aws-rds/lib/cluster.ts

@@ -12,7 +12,7 @@ import { DatabaseClusterAttributes, IDatabaseCluster } from './cluster-ref';
 import { Endpoint } from './endpoint';
 import { IParameterGroup, ParameterGroup } from './parameter-group';
 import { applyDefaultRotationOptions, defaultDeletionProtection, renderCredentials, setupS3ImportExport, helperRemovalPolicy, renderUnless, renderClusterSnapshotCredentials } from './private/util';
-import { BackupProps, Credentials, InstanceProps, PerformanceInsightRetention, RotationSingleUserOptions, RotationMultiUserOptions, SnapshotCredentials } from './props';
+import { BackupProps, Credentials, InstanceProps, PerformanceInsightRetention, RotationSingleUserOptions, RotationMultiUserOptions, ClusterSnapshotCredentials } from './props';
 import { DatabaseProxy, DatabaseProxyOptions, ProxyTarget } from './proxy';
 import { CfnDBCluster, CfnDBClusterProps, CfnDBInstance } from './rds.generated';
 import { ISubnetGroup, SubnetGroup } from './subnet-group';
@@ -661,7 +661,7 @@ export interface DatabaseClusterFromSnapshotProps extends DatabaseClusterBasePro
   /**
    * Credentials for the administrative user
    *
-   * @deprecated - use `snapshotCredentials` instead
+   * @deprecated - use {@link DatabaseClusterFromSnapshotProps.snapshotCredentials} instead
    *
    * @default - A username of 'admin' (or 'postgres' for PostgreSQL) and SecretsManager-generated password
    */
@@ -672,7 +672,7 @@ export interface DatabaseClusterFromSnapshotProps extends DatabaseClusterBasePro
    *
    * @default - No credentials are generated
    */
-  readonly snapshotCredentials?: SnapshotCredentials;
+  readonly snapshotCredentials?: ClusterSnapshotCredentials;
 }
 
 /**
@@ -702,7 +702,12 @@ export class DatabaseClusterFromSnapshot extends DatabaseClusterNew {
     const credentials = renderCredentials(this, props.engine, props.credentials);
 
     const snapshotCredentials = renderClusterSnapshotCredentials(this, props.snapshotCredentials);
-    const secret = snapshotCredentials.secret || credentials.secret;
+    let secret;
+    if (snapshotCredentials) {
+      secret = snapshotCredentials.secret;
+    } else if (credentials) {
+      secret = credentials.secret;
+    }
 
     const cluster = new CfnDBCluster(this, 'Resource', {
       ...this.newCfnProps,

packages/@aws-cdk/aws-rds/lib/private/util.ts

@@ -2,10 +2,10 @@ import * as ec2 from '@aws-cdk/aws-ec2';
 import * as iam from '@aws-cdk/aws-iam';
 import * as s3 from '@aws-cdk/aws-s3';
 import * as secretsmanager from '@aws-cdk/aws-secretsmanager';
-import { Annotations, RemovalPolicy } from '@aws-cdk/core';
+import { Aws, RemovalPolicy } from '@aws-cdk/core';
 import { DatabaseSecret } from '../database-secret';
 import { IEngine } from '../engine';
-import { CommonRotationUserOptions, Credentials, SnapshotCredentials } from '../props';
+import { ClusterSnapshotCredentials, CommonRotationUserOptions, Credentials } from '../props';
 
 // keep this import separate from other imports to reduce chance for merge conflicts with v2-main
 // eslint-disable-next-line no-duplicate-imports, import/order
@@ -115,22 +115,18 @@ export function renderCredentials(scope: Construct, engine: IEngine, credentials
 /**
  * Renders credentials for a cluster from snapshot
  */
-export function renderClusterSnapshotCredentials(scope: Construct, credentials?: SnapshotCredentials): SnapshotCredentials {
+export function renderClusterSnapshotCredentials(scope: Construct, credentials?: ClusterSnapshotCredentials): ClusterSnapshotCredentials | undefined {
   let renderedCredentials = credentials;
 
-  if (!renderedCredentials) return { generatePassword: false };
+  if (renderedCredentials?.secret) return renderedCredentials;
 
-  if (renderedCredentials.generatePassword) Annotations.of(scope).addWarning('Cannot generate new password for clusters created from snapshot. Use `fromSecret()` or `fromPassword()` instead');
-
-  if (renderedCredentials.secret) return renderedCredentials;
-
-  if (renderedCredentials.password) {
-    renderedCredentials = SnapshotCredentials.fromSecret(
+  if (renderedCredentials?.password) {
+    renderedCredentials = ClusterSnapshotCredentials.fromSecret(
       new secretsmanager.Secret(scope, 'Secret', {
-        //username: renderedCredentials.username,
-        secretName: renderedCredentials.secretName,
+        description: `Generated by the CDK for stack: ${Aws.STACK_NAME}`,
         encryptionKey: renderedCredentials.encryptionKey,
         replicaRegions: renderedCredentials.replicaRegions,
+        secretName: renderedCredentials.secretName,
         secretStringValue: renderedCredentials.password,
       }),
     );

packages/@aws-cdk/aws-rds/lib/props.ts

@@ -381,9 +381,8 @@ export abstract class SnapshotCredentials {
   /**
    * Update the snapshot login with an existing password.
    */
-  public static fromPassword(password: SecretValue, options: GeneratedSecretForSnapshotCredentialsOptions = {}): SnapshotCredentials {
+  public static fromPassword(password: SecretValue): SnapshotCredentials {
     return {
-      ...options,
       generatePassword: false,
       password,
     };
@@ -476,6 +475,80 @@ export abstract class SnapshotCredentials {
   public abstract readonly replicaRegions?: secretsmanager.ReplicaRegion[];
 }
 
+/**
+ * Credentials to create or reference a secret for a cluster created from snapshot
+ *
+ * The login credentials of the cluster are inherited from the snapshot and cannot be changed
+ */
+export abstract class ClusterSnapshotCredentials {
+  /**
+   * Create a new secret with the appropriate credentials
+   *
+   * **NOTE:** Do NOT store your password directly in CDK code
+   */
+  // eslint-disable-next-line max-len
+  public static fromPassword(username: string, password: SecretValue, options: GeneratedSecretForSnapshotCredentialsOptions = {}): ClusterSnapshotCredentials {
+    return {
+      ...options,
+      username,
+      password,
+    };
+  }
+
+  /**
+   * Reference a secret to attach to your cluster
+   */
+  public static fromSecret(secret: secretsmanager.ISecret, options: GeneratedSecretForSnapshotCredentialsOptions = {}): ClusterSnapshotCredentials {
+    return {
+      ...options,
+      secret,
+    };
+  }
+
+  /**
+   * KMS encryption key to encrypt the generated secret.
+   *
+   * @default - default master key
+   */
+  public abstract readonly encryptionKey?: kms.IKey;
+
+  /**
+   * The master user password.
+   *
+   * **NOTE:** Do NOT store your password directly in CDK code
+   */
+  public abstract readonly password?: SecretValue;
+
+  /**
+   * Secret used to instantiate this Login.
+   *
+   * @default - none
+   */
+  public abstract readonly secret?: secretsmanager.ISecret;
+
+  /**
+   * Name of secret to create
+   *
+   * @default - default name is generated
+   */
+  public abstract readonly secretName?: string;
+
+  /**
+   * A list of regions where to replicate the generated secret.
+   *
+   * @default - Secret is not replicated
+   */
+  public abstract readonly replicaRegions?: secretsmanager.ReplicaRegion[];
+
+  /**
+   * The master user name.
+   *
+   * Must be the **current** master user name of the snapshot.
+   * It is not possible to change the master user name of a RDS instance.
+   */
+  public abstract readonly username?: string;
+}
+
 /**
  * Properties common to single-user and multi-user rotation options.
  */

packages/@aws-cdk/aws-rds/lib/serverless-cluster.ts

@@ -6,11 +6,12 @@ import { Resource, Duration, Token, Annotations, RemovalPolicy, IResource, Stack
 import * as cxapi from '@aws-cdk/cx-api';
 import { Construct } from 'constructs';
 import { IClusterEngine } from './cluster-engine';
+import { DatabaseSecret } from './database-secret';
 import { Endpoint } from './endpoint';
 import { IParameterGroup } from './parameter-group';
 import { DATA_API_ACTIONS } from './perms';
 import { applyDefaultRotationOptions, defaultDeletionProtection, renderClusterSnapshotCredentials, renderCredentials } from './private/util';
-import { Credentials, RotationMultiUserOptions, RotationSingleUserOptions, SnapshotCredentials } from './props';
+import { ClusterSnapshotCredentials, Credentials, RotationMultiUserOptions, RotationSingleUserOptions, SnapshotCredentials } from './props';
 import { CfnDBCluster, CfnDBClusterProps } from './rds.generated';
 import { ISubnetGroup, SubnetGroup } from './subnet-group';
 
@@ -651,9 +652,20 @@ export interface ServerlessClusterFromSnapshotProps extends ServerlessClusterNew
    * Note - It is not possible to change the master username for a snapshot;
    * however, it is possible to provide (or generate) a new password.
    *
+   * @deprecated - Use {@link ServerlessClusterFromSnapshotProps.snapshotCredentials} instead
+   *
    * @default - The existing username and password from the snapshot will be used.
    */
   readonly credentials?: SnapshotCredentials;
+
+  /**
+   * Credentials to create or reference a secret for a cluster created from snapshot
+   *
+   * The login credentials of the cluster are inherited from the snapshot and cannot be changed
+   *
+   * @default - no credentials are created
+   */
+  readonly snapshotCredentials?: ClusterSnapshotCredentials;
 }
 
 /**
@@ -672,8 +684,27 @@ export class ServerlessClusterFromSnapshot extends ServerlessClusterNew {
 
     this.enableDataApi = props.enableDataApi;
 
-    const credentials = renderClusterSnapshotCredentials(this, props.credentials);
-    const secret = credentials.secret;
+    const credentials = props.credentials;
+    let secret;
+
+    const snapshotCredentials = renderClusterSnapshotCredentials(this, props.snapshotCredentials);
+    if (snapshotCredentials) secret = snapshotCredentials.secret;
+
+    if (!secret && credentials?.generatePassword) {
+      if (!credentials.username) {
+        throw new Error('`credentials` `username` must be specified when `generatePassword` is set to true');
+      }
+
+      Annotations.of(this).addWarning('Generating a password for a cluster created from a snapshot will result in creating a secret with an incorrect password. Use `snapshotCredentials` instead.');
+
+      secret = new DatabaseSecret(this, 'Secret', {
+        username: credentials.username,
+        encryptionKey: credentials.encryptionKey,
+        excludeCharacters: credentials.excludeCharacters,
+        replaceOnPasswordCriteriaChanges: credentials.replaceOnPasswordCriteriaChanges,
+        replicaRegions: credentials.replicaRegions,
+      });
+    }
 
     const cluster = new CfnDBCluster(this, 'Resource', {
       ...this.newCfnProps,

packages/@aws-cdk/aws-rds/test/serverless-cluster-from-snapshot.test.ts

@@ -1,8 +1,7 @@
-import { Match, Template } from '@aws-cdk/assertions';
+import { Template } from '@aws-cdk/assertions';
 import * as ec2 from '@aws-cdk/aws-ec2';
-import * as kms from '@aws-cdk/aws-kms';
 import * as cdk from '@aws-cdk/core';
-import { DatabaseClusterEngine, DatabaseSecret, ServerlessClusterFromSnapshot, SnapshotCredentials } from '../lib';
+import { ClusterSnapshotCredentials, DatabaseClusterEngine, ServerlessClusterFromSnapshot } from '../lib';
 
 describe('serverless cluster from snapshot', () => {
   test('create a serverless cluster from a snapshot', () => {
@@ -41,122 +40,22 @@ describe('serverless cluster from snapshot', () => {
     });
   });
 
-  test('can generate a new snapshot password', () => {
+  test('can create new secret for snapshot using password from an existing SecretValue', () => {
     const stack = testStack();
     const vpc = new ec2.Vpc(stack, 'VPC');
+    const secretValue = cdk.SecretValue.secretsManager('mysecretid');
 
     // WHEN
     new ServerlessClusterFromSnapshot(stack, 'ServerlessDatabase', {
       engine: DatabaseClusterEngine.AURORA_MYSQL,
       vpc,
       snapshotIdentifier: 'mySnapshot',
-      credentials: SnapshotCredentials.fromGeneratedSecret('admin', {
-        excludeCharacters: '"@/\\',
-      }),
-    });
-
-    // THEN
-    Template.fromStack(stack).hasResourceProperties('AWS::RDS::DBCluster', {
-      MasterUsername: Match.absent(),
-      MasterUserPassword: {
-        'Fn::Join': ['', [
-          '{{resolve:secretsmanager:',
-          { Ref: 'ServerlessDatabaseSecret813910E98ee0a797cad8a68dbeb85f8698cdb5bb' },
-          ':SecretString:password::}}',
-        ]],
-      },
-    });
-    Template.fromStack(stack).hasResourceProperties('AWS::SecretsManager::Secret', {
-      Description: {
-        'Fn::Join': ['', ['Generated by the CDK for stack: ', { Ref: 'AWS::StackName' }]],
-      },
-      GenerateSecretString: {
-        ExcludeCharacters: '\"@/\\',
-        GenerateStringKey: 'password',
-        PasswordLength: 30,
-        SecretStringTemplate: '{"username":"admin"}',
-      },
-    });
-  });
-
-  test('fromGeneratedSecret with replica regions', () => {
-    const stack = testStack();
-    const vpc = new ec2.Vpc(stack, 'VPC');
-
-    // WHEN
-    new ServerlessClusterFromSnapshot(stack, 'ServerlessDatabase', {
-      engine: DatabaseClusterEngine.AURORA_MYSQL,
-      vpc,
-      snapshotIdentifier: 'mySnapshot',
-      credentials: SnapshotCredentials.fromGeneratedSecret('admin', {
-        replicaRegions: [{ region: 'eu-west-1' }],
-      }),
+      snapshotCredentials: ClusterSnapshotCredentials.fromPassword('admin', secretValue),
     });
 
     // THEN
     Template.fromStack(stack).hasResourceProperties('AWS::SecretsManager::Secret', {
-      ReplicaRegions: [
-        {
-          Region: 'eu-west-1',
-        },
-      ],
-    });
-  });
-
-  test('throws if generating a new password without a username', () => {
-    const stack = testStack();
-    const vpc = new ec2.Vpc(stack, 'VPC');
-
-    // WHEN
-    expect(() => new ServerlessClusterFromSnapshot(stack, 'ServerlessDatabase', {
-      engine: DatabaseClusterEngine.AURORA_MYSQL,
-      vpc,
-      snapshotIdentifier: 'mySnapshot',
-      credentials: { generatePassword: true },
-    })).toThrow(/`credentials` `username` must be specified when `generatePassword` is set to true/);
-  });
-
-  test('can set a new snapshot password from an existing SecretValue', () => {
-    const stack = testStack();
-    const vpc = new ec2.Vpc(stack, 'VPC');
-
-    // WHEN
-    new ServerlessClusterFromSnapshot(stack, 'ServerlessDatabase', {
-      engine: DatabaseClusterEngine.AURORA_MYSQL,
-      vpc,
-      snapshotIdentifier: 'mySnapshot',
-      credentials: SnapshotCredentials.fromPassword(cdk.SecretValue.unsafePlainText('mysecretpassword')),
-    });
-
-    // THEN
-    Template.fromStack(stack).hasResourceProperties('AWS::RDS::DBCluster', {
-      MasterUsername: Match.absent(),
-      MasterUserPassword: 'mysecretpassword',
-    });
-  });
-
-  test('can set a new snapshot password from an existing Secret', () => {
-    const stack = testStack();
-    const vpc = new ec2.Vpc(stack, 'VPC');
-
-    // WHEN
-    const secret = new DatabaseSecret(stack, 'DBSecret', {
-      username: 'admin',
-      encryptionKey: new kms.Key(stack, 'PasswordKey'),
-    });
-    new ServerlessClusterFromSnapshot(stack, 'ServerlessDatabase', {
-      engine: DatabaseClusterEngine.AURORA_MYSQL,
-      vpc,
-      snapshotIdentifier: 'mySnapshot',
-      credentials: SnapshotCredentials.fromSecret(secret),
-    });
-
-    // THEN
-    Template.fromStack(stack).hasResourceProperties('AWS::RDS::DBCluster', {
-      MasterUsername: Match.absent(),
-      MasterUserPassword: {
-        'Fn::Join': ['', ['{{resolve:secretsmanager:', { Ref: 'DBSecretD58955BC' }, ':SecretString:password::}}']],
-      },
+      SecretString: '{{resolve:secretsmanager:mysecretid:SecretString:::}}',
     });
   });
 });