Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(rds): make rds secret name configurable #13626

Merged
merged 13 commits into from
Mar 19, 2021
11 changes: 11 additions & 0 deletions packages/@aws-cdk/aws-rds/lib/database-secret.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,16 @@ export interface DatabaseSecretProps {
*/
readonly username: string;

/**
* The name of the secret.
*
* For "owned" secrets, this will be the full resource name (secret name + suffix), unless the
* '@aws-cdk/aws-secetsmanager:parseOwnedSecretName' feature flag is set.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have to say, I find this documentation super confusing 🤨.

This is an input property, right? So either you provide the name of the Secret, or it will be generated by CloudFormation.

What does this paragraph mean?

Copy link
Contributor Author

@hedrall hedrall Mar 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry. I did not consider this point very deeply, and used the comments for the properties of Secret itself.

https://github.com/hedrall/aws-cdk/blob/03fba9619d7f3ad4f5a27d8123ff826a0ed26bce/packages/%40aws-cdk/aws-secretsmanager/lib/secret.ts#L33-L39

I would like to look into this more, including how it relates to the Feature flag.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you.

A name for the secret. Note that deleting secrets from SecretsManager does not happen immediately, but after a 7 to * 30 days blackout period. During that period, it is not possible to create another secret that shares the same name.

However, I just noticed that this seems to be an old specification.
I tried destroying the deployed secret and renaming the deployed secret, but in both cases the secret was deleted immediately.
I think that cloud formation use a flag like as cli 'force-delete-without-recovery’.

So, I change comment simply.
( Should I also change the comment of SecretProps? )

*
* @default - A name is generated by CloudFormation.
*/
readonly secretName?: string;

/**
* The KMS key to use to encrypt the secret.
*
Expand Down Expand Up @@ -60,6 +70,7 @@ export class DatabaseSecret extends secretsmanager.Secret {
super(scope, id, {
encryptionKey: props.encryptionKey,
description: `Generated by the CDK for stack: ${Aws.STACK_NAME}`,
secretName: props.secretName,
generateSecretString: {
passwordLength: 30, // Oracle password cannot have more than 30 characters
secretStringTemplate: JSON.stringify({
Expand Down
3 changes: 2 additions & 1 deletion packages/@aws-cdk/aws-rds/lib/private/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ export function renderCredentials(scope: Construct, engine: IEngine, credentials
renderedCredentials = Credentials.fromSecret(
new DatabaseSecret(scope, 'Secret', {
username: renderedCredentials.username,
secretName: renderedCredentials.secretName,
encryptionKey: renderedCredentials.encryptionKey,
excludeCharacters: renderedCredentials.excludeCharacters,
// if username must be referenced as a string we can safely replace the
Expand Down Expand Up @@ -131,4 +132,4 @@ export function helperRemovalPolicy(basePolicy?: RemovalPolicy): RemovalPolicy {
*/
export function renderUnless<A>(value: A, suppressValue: A): A | undefined {
return value === suppressValue ? undefined : value;
}
}
14 changes: 14 additions & 0 deletions packages/@aws-cdk/aws-rds/lib/props.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,13 @@ export interface BackupProps {
* Base options for creating Credentials.
*/
export interface CredentialsBaseOptions {
/**
* The name of the secret.
*
* @default - A name is generated by CloudFormation.
*/
readonly secretName?: string;

/**
* KMS encryption key to encrypt the generated secret.
*
Expand Down Expand Up @@ -231,6 +238,13 @@ export abstract class Credentials {
*/
public abstract readonly username: string;

/**
* The name to use for the Secret if a new Secret is to be generated in SecretsManager for these Credentials.
*
* @default - A name is generated by CloudFormation.
*/
readonly secretName?: string;
hedrall marked this conversation as resolved.
Show resolved Hide resolved

/**
* Whether the username should be referenced as a string and not as a dynamic
* reference to the username in the secret.
Expand Down
48 changes: 47 additions & 1 deletion packages/@aws-cdk/aws-rds/test/cluster.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import * as cxapi from '@aws-cdk/cx-api';
import { testFutureBehavior } from 'cdk-build-tools/lib/feature-flag';
import {
AuroraEngineVersion, AuroraMysqlEngineVersion, AuroraPostgresEngineVersion, CfnDBCluster, Credentials, DatabaseCluster,
DatabaseClusterEngine, DatabaseClusterFromSnapshot, ParameterGroup, PerformanceInsightRetention, SubnetGroup,
DatabaseClusterEngine, DatabaseClusterFromSnapshot, ParameterGroup, PerformanceInsightRetention, SubnetGroup, DatabaseSecret,
} from '../lib';

describe('cluster', () => {
Expand Down Expand Up @@ -1763,6 +1763,52 @@ describe('cluster', () => {

});

test('can set custom name to database secret by fromSecret', () => {
// GIVEN
const stack = testStack();
const vpc = new ec2.Vpc(stack, 'VPC');
const secretName = 'custom-secret-name';
const secret = new DatabaseSecret(stack, 'Secret', {
username: 'admin',
secretName,
} );

// WHEN
new DatabaseCluster(stack, 'Database', {
engine: DatabaseClusterEngine.aurora({ version: AuroraEngineVersion.VER_1_22_2 }),
credentials: Credentials.fromSecret(secret),
instanceProps: {
vpc,
},
});

// THEN
expect(stack).toHaveResourceLike('AWS::SecretsManager::Secret', {
Name: secretName,
});
});

test('can set custom name to database secret by fromGeneratedSecret', () => {
// GIVEN
const stack = testStack();
const vpc = new ec2.Vpc(stack, 'VPC');
const secretName = 'custom-secret-name';

// WHEN
new DatabaseCluster(stack, 'Database', {
engine: DatabaseClusterEngine.aurora({ version: AuroraEngineVersion.VER_1_22_2 }),
credentials: Credentials.fromGeneratedSecret('admin', { secretName }),
instanceProps: {
vpc,
},
});

// THEN
expect(stack).toHaveResourceLike('AWS::SecretsManager::Secret', {
Name: secretName,
});
});

test('can set public accessibility for database cluster with instances in private subnet', () => {
// GIVEN
const stack = testStack();
Expand Down
38 changes: 37 additions & 1 deletion packages/@aws-cdk/aws-rds/test/instance.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1204,8 +1204,44 @@ describe('instance', () => {
MasterUsername: 'postgres', // username is a string
MasterUserPassword: '{{resolve:ssm-secure:/dbPassword:1}}', // reference to SSM
});
});

test('can set custom name to database secret by fromSecret', () => {
// WHEN
const secretName = 'custom-secret-name';
const secret = new rds.DatabaseSecret(stack, 'Secret', {
username: 'admin',
secretName,
} );
new rds.DatabaseInstance(stack, 'Instance', {
engine: rds.DatabaseInstanceEngine.mysql({
version: rds.MysqlEngineVersion.VER_8_0_19,
}),
credentials: rds.Credentials.fromSecret(secret),
vpc,
});

// THEN
expect(stack).toHaveResourceLike('AWS::SecretsManager::Secret', {
Name: secretName,
});
});

test('can set custom name to database secret by fromGeneratedSecret', () => {
// WHEN
const secretName = 'custom-secret-name';
new rds.DatabaseInstance(stack, 'Instance', {
engine: rds.DatabaseInstanceEngine.mysql({
version: rds.MysqlEngineVersion.VER_8_0_19,
}),
credentials: rds.Credentials.fromGeneratedSecret('admin', { secretName }),
vpc,
});

// THEN
expect(stack).toHaveResourceLike('AWS::SecretsManager::Secret', {
Name: secretName,
});
});

test('can set publiclyAccessible to false with public subnets', () => {
Expand Down Expand Up @@ -1274,4 +1310,4 @@ test.each([
DeletionPolicy: subnetValue,
UpdateReplacePolicy: subnetValue,
}, ResourcePart.CompleteDefinition);
});
});