Skip to content

Commit a5967b7

Browse files
woodwoop-ospowoodwoop-ospo
committed
fix(rds): clusters created from snapshots generate incorrect passwords
1 parent da4f380 commit a5967b7

5 files changed

Lines changed: 132 additions & 128 deletions

File tree

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

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import { DatabaseClusterAttributes, IDatabaseCluster } from './cluster-ref';
1212
import { Endpoint } from './endpoint';
1313
import { IParameterGroup, ParameterGroup } from './parameter-group';
1414
import { applyDefaultRotationOptions, defaultDeletionProtection, renderCredentials, setupS3ImportExport, helperRemovalPolicy, renderUnless, renderClusterSnapshotCredentials } from './private/util';
15-
import { BackupProps, Credentials, InstanceProps, PerformanceInsightRetention, RotationSingleUserOptions, RotationMultiUserOptions, SnapshotCredentials } from './props';
15+
import { BackupProps, Credentials, InstanceProps, PerformanceInsightRetention, RotationSingleUserOptions, RotationMultiUserOptions, ClusterSnapshotCredentials } from './props';
1616
import { DatabaseProxy, DatabaseProxyOptions, ProxyTarget } from './proxy';
1717
import { CfnDBCluster, CfnDBClusterProps, CfnDBInstance } from './rds.generated';
1818
import { ISubnetGroup, SubnetGroup } from './subnet-group';
@@ -661,7 +661,7 @@ export interface DatabaseClusterFromSnapshotProps extends DatabaseClusterBasePro
661661
/**
662662
* Credentials for the administrative user
663663
*
664-
* @deprecated - use `snapshotCredentials` instead
664+
* @deprecated - use {@link DatabaseClusterFromSnapshotProps.snapshotCredentials} instead
665665
*
666666
* @default - A username of 'admin' (or 'postgres' for PostgreSQL) and SecretsManager-generated password
667667
*/
@@ -672,7 +672,7 @@ export interface DatabaseClusterFromSnapshotProps extends DatabaseClusterBasePro
672672
*
673673
* @default - No credentials are generated
674674
*/
675-
readonly snapshotCredentials?: SnapshotCredentials;
675+
readonly snapshotCredentials?: ClusterSnapshotCredentials;
676676
}
677677

678678
/**
@@ -702,7 +702,12 @@ export class DatabaseClusterFromSnapshot extends DatabaseClusterNew {
702702
const credentials = renderCredentials(this, props.engine, props.credentials);
703703

704704
const snapshotCredentials = renderClusterSnapshotCredentials(this, props.snapshotCredentials);
705-
const secret = snapshotCredentials.secret || credentials.secret;
705+
let secret;
706+
if (snapshotCredentials) {
707+
secret = snapshotCredentials.secret;
708+
} else if (credentials) {
709+
secret = credentials.secret;
710+
}
706711

707712
const cluster = new CfnDBCluster(this, 'Resource', {
708713
...this.newCfnProps,

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

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,10 @@ import * as ec2 from '@aws-cdk/aws-ec2';
22
import * as iam from '@aws-cdk/aws-iam';
33
import * as s3 from '@aws-cdk/aws-s3';
44
import * as secretsmanager from '@aws-cdk/aws-secretsmanager';
5-
import { Annotations, RemovalPolicy } from '@aws-cdk/core';
5+
import { Aws, RemovalPolicy } from '@aws-cdk/core';
66
import { DatabaseSecret } from '../database-secret';
77
import { IEngine } from '../engine';
8-
import { CommonRotationUserOptions, Credentials, SnapshotCredentials } from '../props';
8+
import { ClusterSnapshotCredentials, CommonRotationUserOptions, Credentials } from '../props';
99

1010
// keep this import separate from other imports to reduce chance for merge conflicts with v2-main
1111
// eslint-disable-next-line no-duplicate-imports, import/order
@@ -115,22 +115,18 @@ export function renderCredentials(scope: Construct, engine: IEngine, credentials
115115
/**
116116
* Renders credentials for a cluster from snapshot
117117
*/
118-
export function renderClusterSnapshotCredentials(scope: Construct, credentials?: SnapshotCredentials): SnapshotCredentials {
118+
export function renderClusterSnapshotCredentials(scope: Construct, credentials?: ClusterSnapshotCredentials): ClusterSnapshotCredentials | undefined {
119119
let renderedCredentials = credentials;
120120

121-
if (!renderedCredentials) return { generatePassword: false };
121+
if (renderedCredentials?.secret) return renderedCredentials;
122122

123-
if (renderedCredentials.generatePassword) Annotations.of(scope).addWarning('Cannot generate new password for clusters created from snapshot. Use `fromSecret()` or `fromPassword()` instead');
124-
125-
if (renderedCredentials.secret) return renderedCredentials;
126-
127-
if (renderedCredentials.password) {
128-
renderedCredentials = SnapshotCredentials.fromSecret(
123+
if (renderedCredentials?.password) {
124+
renderedCredentials = ClusterSnapshotCredentials.fromSecret(
129125
new secretsmanager.Secret(scope, 'Secret', {
130-
//username: renderedCredentials.username,
131-
secretName: renderedCredentials.secretName,
126+
description: `Generated by the CDK for stack: ${Aws.STACK_NAME}`,
132127
encryptionKey: renderedCredentials.encryptionKey,
133128
replicaRegions: renderedCredentials.replicaRegions,
129+
secretName: renderedCredentials.secretName,
134130
secretStringValue: renderedCredentials.password,
135131
}),
136132
);

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

Lines changed: 75 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -381,9 +381,8 @@ export abstract class SnapshotCredentials {
381381
/**
382382
* Update the snapshot login with an existing password.
383383
*/
384-
public static fromPassword(password: SecretValue, options: GeneratedSecretForSnapshotCredentialsOptions = {}): SnapshotCredentials {
384+
public static fromPassword(password: SecretValue): SnapshotCredentials {
385385
return {
386-
...options,
387386
generatePassword: false,
388387
password,
389388
};
@@ -476,6 +475,80 @@ export abstract class SnapshotCredentials {
476475
public abstract readonly replicaRegions?: secretsmanager.ReplicaRegion[];
477476
}
478477

478+
/**
479+
* Credentials to create or reference a secret for a cluster created from snapshot
480+
*
481+
* The login credentials of the cluster are inherited from the snapshot and cannot be changed
482+
*/
483+
export abstract class ClusterSnapshotCredentials {
484+
/**
485+
* Create a new secret with the appropriate credentials
486+
*
487+
* **NOTE:** Do NOT store your password directly in CDK code
488+
*/
489+
// eslint-disable-next-line max-len
490+
public static fromPassword(username: string, password: SecretValue, options: GeneratedSecretForSnapshotCredentialsOptions = {}): ClusterSnapshotCredentials {
491+
return {
492+
...options,
493+
username,
494+
password,
495+
};
496+
}
497+
498+
/**
499+
* Reference a secret to attach to your cluster
500+
*/
501+
public static fromSecret(secret: secretsmanager.ISecret, options: GeneratedSecretForSnapshotCredentialsOptions = {}): ClusterSnapshotCredentials {
502+
return {
503+
...options,
504+
secret,
505+
};
506+
}
507+
508+
/**
509+
* KMS encryption key to encrypt the generated secret.
510+
*
511+
* @default - default master key
512+
*/
513+
public abstract readonly encryptionKey?: kms.IKey;
514+
515+
/**
516+
* The master user password.
517+
*
518+
* **NOTE:** Do NOT store your password directly in CDK code
519+
*/
520+
public abstract readonly password?: SecretValue;
521+
522+
/**
523+
* Secret used to instantiate this Login.
524+
*
525+
* @default - none
526+
*/
527+
public abstract readonly secret?: secretsmanager.ISecret;
528+
529+
/**
530+
* Name of secret to create
531+
*
532+
* @default - default name is generated
533+
*/
534+
public abstract readonly secretName?: string;
535+
536+
/**
537+
* A list of regions where to replicate the generated secret.
538+
*
539+
* @default - Secret is not replicated
540+
*/
541+
public abstract readonly replicaRegions?: secretsmanager.ReplicaRegion[];
542+
543+
/**
544+
* The master user name.
545+
*
546+
* Must be the **current** master user name of the snapshot.
547+
* It is not possible to change the master user name of a RDS instance.
548+
*/
549+
public abstract readonly username?: string;
550+
}
551+
479552
/**
480553
* Properties common to single-user and multi-user rotation options.
481554
*/

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

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,12 @@ import { Resource, Duration, Token, Annotations, RemovalPolicy, IResource, Stack
66
import * as cxapi from '@aws-cdk/cx-api';
77
import { Construct } from 'constructs';
88
import { IClusterEngine } from './cluster-engine';
9+
import { DatabaseSecret } from './database-secret';
910
import { Endpoint } from './endpoint';
1011
import { IParameterGroup } from './parameter-group';
1112
import { DATA_API_ACTIONS } from './perms';
1213
import { applyDefaultRotationOptions, defaultDeletionProtection, renderClusterSnapshotCredentials, renderCredentials } from './private/util';
13-
import { Credentials, RotationMultiUserOptions, RotationSingleUserOptions, SnapshotCredentials } from './props';
14+
import { ClusterSnapshotCredentials, Credentials, RotationMultiUserOptions, RotationSingleUserOptions, SnapshotCredentials } from './props';
1415
import { CfnDBCluster, CfnDBClusterProps } from './rds.generated';
1516
import { ISubnetGroup, SubnetGroup } from './subnet-group';
1617

@@ -651,9 +652,20 @@ export interface ServerlessClusterFromSnapshotProps extends ServerlessClusterNew
651652
* Note - It is not possible to change the master username for a snapshot;
652653
* however, it is possible to provide (or generate) a new password.
653654
*
655+
* @deprecated - Use {@link ServerlessClusterFromSnapshotProps.snapshotCredentials} instead
656+
*
654657
* @default - The existing username and password from the snapshot will be used.
655658
*/
656659
readonly credentials?: SnapshotCredentials;
660+
661+
/**
662+
* Credentials to create or reference a secret for a cluster created from snapshot
663+
*
664+
* The login credentials of the cluster are inherited from the snapshot and cannot be changed
665+
*
666+
* @default - no credentials are created
667+
*/
668+
readonly snapshotCredentials?: ClusterSnapshotCredentials;
657669
}
658670

659671
/**
@@ -672,8 +684,27 @@ export class ServerlessClusterFromSnapshot extends ServerlessClusterNew {
672684

673685
this.enableDataApi = props.enableDataApi;
674686

675-
const credentials = renderClusterSnapshotCredentials(this, props.credentials);
676-
const secret = credentials.secret;
687+
const credentials = props.credentials;
688+
let secret;
689+
690+
const snapshotCredentials = renderClusterSnapshotCredentials(this, props.snapshotCredentials);
691+
if (snapshotCredentials) secret = snapshotCredentials.secret;
692+
693+
if (!secret && credentials?.generatePassword) {
694+
if (!credentials.username) {
695+
throw new Error('`credentials` `username` must be specified when `generatePassword` is set to true');
696+
}
697+
698+
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.');
699+
700+
secret = new DatabaseSecret(this, 'Secret', {
701+
username: credentials.username,
702+
encryptionKey: credentials.encryptionKey,
703+
excludeCharacters: credentials.excludeCharacters,
704+
replaceOnPasswordCriteriaChanges: credentials.replaceOnPasswordCriteriaChanges,
705+
replicaRegions: credentials.replicaRegions,
706+
});
707+
}
677708

678709
const cluster = new CfnDBCluster(this, 'Resource', {
679710
...this.newCfnProps,

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

Lines changed: 6 additions & 107 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
1-
import { Match, Template } from '@aws-cdk/assertions';
1+
import { Template } from '@aws-cdk/assertions';
22
import * as ec2 from '@aws-cdk/aws-ec2';
3-
import * as kms from '@aws-cdk/aws-kms';
43
import * as cdk from '@aws-cdk/core';
5-
import { DatabaseClusterEngine, DatabaseSecret, ServerlessClusterFromSnapshot, SnapshotCredentials } from '../lib';
4+
import { ClusterSnapshotCredentials, DatabaseClusterEngine, ServerlessClusterFromSnapshot } from '../lib';
65

76
describe('serverless cluster from snapshot', () => {
87
test('create a serverless cluster from a snapshot', () => {
@@ -41,122 +40,22 @@ describe('serverless cluster from snapshot', () => {
4140
});
4241
});
4342

44-
test('can generate a new snapshot password', () => {
43+
test('can create new secret for snapshot using password from an existing SecretValue', () => {
4544
const stack = testStack();
4645
const vpc = new ec2.Vpc(stack, 'VPC');
46+
const secretValue = cdk.SecretValue.secretsManager('mysecretid');
4747

4848
// WHEN
4949
new ServerlessClusterFromSnapshot(stack, 'ServerlessDatabase', {
5050
engine: DatabaseClusterEngine.AURORA_MYSQL,
5151
vpc,
5252
snapshotIdentifier: 'mySnapshot',
53-
credentials: SnapshotCredentials.fromGeneratedSecret('admin', {
54-
excludeCharacters: '"@/\\',
55-
}),
56-
});
57-
58-
// THEN
59-
Template.fromStack(stack).hasResourceProperties('AWS::RDS::DBCluster', {
60-
MasterUsername: Match.absent(),
61-
MasterUserPassword: {
62-
'Fn::Join': ['', [
63-
'{{resolve:secretsmanager:',
64-
{ Ref: 'ServerlessDatabaseSecret813910E98ee0a797cad8a68dbeb85f8698cdb5bb' },
65-
':SecretString:password::}}',
66-
]],
67-
},
68-
});
69-
Template.fromStack(stack).hasResourceProperties('AWS::SecretsManager::Secret', {
70-
Description: {
71-
'Fn::Join': ['', ['Generated by the CDK for stack: ', { Ref: 'AWS::StackName' }]],
72-
},
73-
GenerateSecretString: {
74-
ExcludeCharacters: '\"@/\\',
75-
GenerateStringKey: 'password',
76-
PasswordLength: 30,
77-
SecretStringTemplate: '{"username":"admin"}',
78-
},
79-
});
80-
});
81-
82-
test('fromGeneratedSecret with replica regions', () => {
83-
const stack = testStack();
84-
const vpc = new ec2.Vpc(stack, 'VPC');
85-
86-
// WHEN
87-
new ServerlessClusterFromSnapshot(stack, 'ServerlessDatabase', {
88-
engine: DatabaseClusterEngine.AURORA_MYSQL,
89-
vpc,
90-
snapshotIdentifier: 'mySnapshot',
91-
credentials: SnapshotCredentials.fromGeneratedSecret('admin', {
92-
replicaRegions: [{ region: 'eu-west-1' }],
93-
}),
53+
snapshotCredentials: ClusterSnapshotCredentials.fromPassword('admin', secretValue),
9454
});
9555

9656
// THEN
9757
Template.fromStack(stack).hasResourceProperties('AWS::SecretsManager::Secret', {
98-
ReplicaRegions: [
99-
{
100-
Region: 'eu-west-1',
101-
},
102-
],
103-
});
104-
});
105-
106-
test('throws if generating a new password without a username', () => {
107-
const stack = testStack();
108-
const vpc = new ec2.Vpc(stack, 'VPC');
109-
110-
// WHEN
111-
expect(() => new ServerlessClusterFromSnapshot(stack, 'ServerlessDatabase', {
112-
engine: DatabaseClusterEngine.AURORA_MYSQL,
113-
vpc,
114-
snapshotIdentifier: 'mySnapshot',
115-
credentials: { generatePassword: true },
116-
})).toThrow(/`credentials` `username` must be specified when `generatePassword` is set to true/);
117-
});
118-
119-
test('can set a new snapshot password from an existing SecretValue', () => {
120-
const stack = testStack();
121-
const vpc = new ec2.Vpc(stack, 'VPC');
122-
123-
// WHEN
124-
new ServerlessClusterFromSnapshot(stack, 'ServerlessDatabase', {
125-
engine: DatabaseClusterEngine.AURORA_MYSQL,
126-
vpc,
127-
snapshotIdentifier: 'mySnapshot',
128-
credentials: SnapshotCredentials.fromPassword(cdk.SecretValue.unsafePlainText('mysecretpassword')),
129-
});
130-
131-
// THEN
132-
Template.fromStack(stack).hasResourceProperties('AWS::RDS::DBCluster', {
133-
MasterUsername: Match.absent(),
134-
MasterUserPassword: 'mysecretpassword',
135-
});
136-
});
137-
138-
test('can set a new snapshot password from an existing Secret', () => {
139-
const stack = testStack();
140-
const vpc = new ec2.Vpc(stack, 'VPC');
141-
142-
// WHEN
143-
const secret = new DatabaseSecret(stack, 'DBSecret', {
144-
username: 'admin',
145-
encryptionKey: new kms.Key(stack, 'PasswordKey'),
146-
});
147-
new ServerlessClusterFromSnapshot(stack, 'ServerlessDatabase', {
148-
engine: DatabaseClusterEngine.AURORA_MYSQL,
149-
vpc,
150-
snapshotIdentifier: 'mySnapshot',
151-
credentials: SnapshotCredentials.fromSecret(secret),
152-
});
153-
154-
// THEN
155-
Template.fromStack(stack).hasResourceProperties('AWS::RDS::DBCluster', {
156-
MasterUsername: Match.absent(),
157-
MasterUserPassword: {
158-
'Fn::Join': ['', ['{{resolve:secretsmanager:', { Ref: 'DBSecretD58955BC' }, ':SecretString:password::}}']],
159-
},
58+
SecretString: '{{resolve:secretsmanager:mysecretid:SecretString:::}}',
16059
});
16160
});
16261
});

0 commit comments

Comments
 (0)