Skip to content

Commit 4a87d39

Browse files
authored
fix(rds): clusters created from snapshots generate incorrect passwords (#20504)
Deprecate `credentials` and explain how it is broken. Replace it with `snapshotCredentials` that offer the expected behavior. Fixes #20434 Closes #20473 ---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/master/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent ade7af5 commit 4a87d39

File tree

18 files changed

+6075
-5
lines changed

18 files changed

+6075
-5
lines changed

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

+51-3
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,11 @@ import * as cxapi from '@aws-cdk/cx-api';
99
import { Construct } from 'constructs';
1010
import { IClusterEngine } from './cluster-engine';
1111
import { DatabaseClusterAttributes, IDatabaseCluster } from './cluster-ref';
12+
import { DatabaseSecret } from './database-secret';
1213
import { Endpoint } from './endpoint';
1314
import { IParameterGroup, ParameterGroup } from './parameter-group';
1415
import { applyDefaultRotationOptions, defaultDeletionProtection, renderCredentials, setupS3ImportExport, helperRemovalPolicy, renderUnless } from './private/util';
15-
import { BackupProps, Credentials, InstanceProps, PerformanceInsightRetention, RotationSingleUserOptions, RotationMultiUserOptions } from './props';
16+
import { BackupProps, Credentials, InstanceProps, PerformanceInsightRetention, RotationSingleUserOptions, RotationMultiUserOptions, SnapshotCredentials } from './props';
1617
import { DatabaseProxy, DatabaseProxyOptions, ProxyTarget } from './proxy';
1718
import { CfnDBCluster, CfnDBClusterProps, CfnDBInstance } from './rds.generated';
1819
import { ISubnetGroup, SubnetGroup } from './subnet-group';
@@ -661,9 +662,27 @@ export interface DatabaseClusterFromSnapshotProps extends DatabaseClusterBasePro
661662
/**
662663
* Credentials for the administrative user
663664
*
665+
* Note - using this prop only works with `Credentials.fromPassword()` with the
666+
* username of the snapshot, `Credentials.fromUsername()` with the username and
667+
* password of the snapshot or `Credentials.fromSecret()` with a secret containing
668+
* the username and password of the snapshot.
669+
*
664670
* @default - A username of 'admin' (or 'postgres' for PostgreSQL) and SecretsManager-generated password
671+
* that **will not be applied** to the cluster, use `snapshotCredentials` for the correct behavior.
672+
*
673+
* @deprecated use `snapshotCredentials` which allows to generate a new password
665674
*/
666675
readonly credentials?: Credentials;
676+
677+
/**
678+
* Master user credentials.
679+
*
680+
* Note - It is not possible to change the master username for a snapshot;
681+
* however, it is possible to provide (or generate) a new password.
682+
*
683+
* @default - The existing username and password from the snapshot will be used.
684+
*/
685+
readonly snapshotCredentials?: SnapshotCredentials;
667686
}
668687

669688
/**
@@ -687,12 +706,34 @@ export class DatabaseClusterFromSnapshot extends DatabaseClusterNew {
687706
constructor(scope: Construct, id: string, props: DatabaseClusterFromSnapshotProps) {
688707
super(scope, id, props);
689708

690-
const credentials = renderCredentials(this, props.engine, props.credentials);
691-
const secret = credentials.secret;
709+
if (props.credentials && !props.credentials.password && !props.credentials.secret) {
710+
Annotations.of(this).addWarning('Use `snapshotCredentials` to modify password of a cluster created from a snapshot.');
711+
}
712+
if (!props.credentials && !props.snapshotCredentials) {
713+
Annotations.of(this).addWarning('Generated credentials will not be applied to cluster. Use `snapshotCredentials` instead. `addRotationSingleUser()` and `addRotationMultiUser()` cannot be used on tbis cluster.');
714+
}
715+
const deprecatedCredentials = renderCredentials(this, props.engine, props.credentials);
716+
717+
let credentials = props.snapshotCredentials;
718+
let secret = credentials?.secret;
719+
if (!secret && credentials?.generatePassword) {
720+
if (!credentials.username) {
721+
throw new Error('`snapshotCredentials` `username` must be specified when `generatePassword` is set to true');
722+
}
723+
724+
secret = new DatabaseSecret(this, 'SnapshotSecret', {
725+
username: credentials.username,
726+
encryptionKey: credentials.encryptionKey,
727+
excludeCharacters: credentials.excludeCharacters,
728+
replaceOnPasswordCriteriaChanges: credentials.replaceOnPasswordCriteriaChanges,
729+
replicaRegions: credentials.replicaRegions,
730+
});
731+
}
692732

693733
const cluster = new CfnDBCluster(this, 'Resource', {
694734
...this.newCfnProps,
695735
snapshotIdentifier: props.snapshotIdentifier,
736+
masterUserPassword: secret?.secretValueFromJson('password')?.unsafeUnwrap() ?? credentials?.password?.unsafeUnwrap(), // Safe usage
696737
});
697738

698739
this.clusterIdentifier = cluster.ref;
@@ -701,6 +742,13 @@ export class DatabaseClusterFromSnapshot extends DatabaseClusterNew {
701742
this.secret = secret.attach(this);
702743
}
703744

745+
if (deprecatedCredentials.secret) {
746+
const deprecatedSecret = deprecatedCredentials.secret.attach(this);
747+
if (!this.secret) {
748+
this.secret = deprecatedSecret;
749+
}
750+
}
751+
704752
// create a number token that represents the port of the cluster
705753
const portAttribute = Token.asNumber(cluster.attrEndpointPort);
706754
this.clusterEndpoint = new Endpoint(cluster.attrEndpointAddress, portAttribute);

packages/@aws-cdk/aws-rds/package.json

+1
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@
8484
"@aws-cdk/aws-events-targets": "0.0.0",
8585
"@aws-cdk/aws-lambda": "0.0.0",
8686
"@aws-cdk/cdk-build-tools": "0.0.0",
87+
"@aws-cdk/custom-resources": "0.0.0",
8788
"@aws-cdk/integ-runner": "0.0.0",
8889
"@aws-cdk/cfn2ts": "0.0.0",
8990
"@aws-cdk/cx-api": "0.0.0",
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
import type { IsCompleteRequest, IsCompleteResponse, OnEventRequest, OnEventResponse } from '@aws-cdk/custom-resources/lib/provider-framework/types';
2+
export declare function onEventHandler(event: OnEventRequest): Promise<OnEventResponse>;
3+
export declare function isCompleteHandler(event: IsCompleteRequest): Promise<IsCompleteResponse>;

packages/@aws-cdk/aws-rds/test/cluster-snapshot.integ.snapshot/asset.1e025324752b3133dc230c4b8b8752f666b63c09cd4aa605ec2b322cc40def2e/index.js

+60
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
/* eslint-disable no-console */
2+
import type { IsCompleteRequest, IsCompleteResponse, OnEventRequest, OnEventResponse } from '@aws-cdk/custom-resources/lib/provider-framework/types';
3+
import { RDS } from 'aws-sdk'; // eslint-disable-line import/no-extraneous-dependencies
4+
5+
export async function onEventHandler(event: OnEventRequest): Promise<OnEventResponse> {
6+
console.log('Event: %j', event);
7+
8+
const rds = new RDS();
9+
10+
const physicalResourceId = `${event.ResourceProperties.DBClusterIdentifier}-${event.ResourceProperties.DBClusterIdentifier}`;
11+
12+
if (event.RequestType === 'Create' || event.RequestType === 'Update') {
13+
const data = await rds.createDBClusterSnapshot({
14+
DBClusterIdentifier: event.ResourceProperties.DBClusterIdentifier,
15+
DBClusterSnapshotIdentifier: event.ResourceProperties.DBClusterSnapshotIdentifier,
16+
}).promise();
17+
return {
18+
PhysicalResourceId: physicalResourceId,
19+
Data: {
20+
DBClusterSnapshotArn: data.DBClusterSnapshot?.DBClusterSnapshotArn,
21+
},
22+
};
23+
}
24+
25+
if (event.RequestType === 'Delete') {
26+
await rds.deleteDBClusterSnapshot({
27+
DBClusterSnapshotIdentifier: event.ResourceProperties.DBClusterSnapshotIdentifier,
28+
}).promise();
29+
}
30+
31+
return {
32+
PhysicalResourceId: `${event.ResourceProperties.DBClusterIdentifier}-${event.ResourceProperties.DBClusterIdentifier}`,
33+
};
34+
}
35+
36+
export async function isCompleteHandler(event: IsCompleteRequest): Promise<IsCompleteResponse> {
37+
console.log('Event: %j', event);
38+
39+
const snapshotStatus = await tryGetClusterSnapshotStatus(event.ResourceProperties.DBClusterSnapshotIdentifier);
40+
41+
switch (event.RequestType) {
42+
case 'Create':
43+
case 'Update':
44+
return { IsComplete: snapshotStatus === 'available' };
45+
case 'Delete':
46+
return { IsComplete: snapshotStatus === undefined };
47+
}
48+
}
49+
50+
async function tryGetClusterSnapshotStatus(identifier: string): Promise<string | undefined> {
51+
try {
52+
const rds = new RDS();
53+
const data = await rds.describeDBClusterSnapshots({
54+
DBClusterSnapshotIdentifier: identifier,
55+
}).promise();
56+
return data.DBClusterSnapshots?.[0].Status;
57+
} catch (err) {
58+
if (err.code === 'DBClusterSnapshotNotFoundFault') {
59+
return undefined;
60+
}
61+
throw err;
62+
}
63+
}

0 commit comments

Comments
 (0)