From 327dee31d0167136dd43c23d1317065498cd4702 Mon Sep 17 00:00:00 2001 From: aereal Date: Wed, 20 Mar 2019 13:08:55 +0900 Subject: [PATCH 1/8] feat(rds): Add InstanceParameterGroup --- packages/@aws-cdk/aws-rds/lib/index.ts | 1 + .../lib/instance-parameter-group-ref.ts | 44 ++++++ .../aws-rds/lib/instance-parameter-group.ts | 130 ++++++++++++++++++ 3 files changed, 175 insertions(+) create mode 100644 packages/@aws-cdk/aws-rds/lib/instance-parameter-group-ref.ts create mode 100644 packages/@aws-cdk/aws-rds/lib/instance-parameter-group.ts diff --git a/packages/@aws-cdk/aws-rds/lib/index.ts b/packages/@aws-cdk/aws-rds/lib/index.ts index 290e0153e4c74..242987ebf0d8a 100644 --- a/packages/@aws-cdk/aws-rds/lib/index.ts +++ b/packages/@aws-cdk/aws-rds/lib/index.ts @@ -5,6 +5,7 @@ export * from './props'; export * from './cluster-parameter-group'; export * from './rotation-single-user'; export * from './database-secret'; +export * from './instance-parameter-group'; // AWS::RDS CloudFormation Resources: export * from './rds.generated'; diff --git a/packages/@aws-cdk/aws-rds/lib/instance-parameter-group-ref.ts b/packages/@aws-cdk/aws-rds/lib/instance-parameter-group-ref.ts new file mode 100644 index 0000000000000..3854eeec81a1a --- /dev/null +++ b/packages/@aws-cdk/aws-rds/lib/instance-parameter-group-ref.ts @@ -0,0 +1,44 @@ +import { CfnOutput, Construct } from "@aws-cdk/cdk"; + +/** + * A instance parameter group + */ +export abstract class InstanceParameterGroupRef extends Construct { + /** + * Import a parameter group + */ + public static import(scope: Construct, id: string, props: InstanceParameterGroupRefProps): InstanceParameterGroupRef { + return new ImportedInstanceParameterGroup(scope, id, props); + } + + /** + * Name of this parameter group + */ + public abstract readonly parameterGroupName: string; + + /** + * Export this parameter group + */ + public export(): InstanceParameterGroupRefProps { + return { + parameterGroupName: new CfnOutput(this, 'ParameterGroupName', { value: this.parameterGroupName}).makeImportValue().toString(), + }; + } +} + +/** + * Properties to reference a instance parameter group + */ +export interface InstanceParameterGroupRefProps { + parameterGroupName: string +} + +class ImportedInstanceParameterGroup extends InstanceParameterGroupRef { + public readonly parameterGroupName: string; + + constructor(scope: Construct, id: string, props: InstanceParameterGroupRefProps) { + super(scope, id); + + this.parameterGroupName = props.parameterGroupName; + } +} diff --git a/packages/@aws-cdk/aws-rds/lib/instance-parameter-group.ts b/packages/@aws-cdk/aws-rds/lib/instance-parameter-group.ts new file mode 100644 index 0000000000000..25c4f037e1489 --- /dev/null +++ b/packages/@aws-cdk/aws-rds/lib/instance-parameter-group.ts @@ -0,0 +1,130 @@ +import { CfnOutput, Construct, IConstruct, Token } from '@aws-cdk/cdk'; +import { Parameters } from './props'; +import { CfnDBParameterGroup } from './rds.generated'; + +/** + * A instance parameter group + */ +export interface IInstanceParameterGroup extends IConstruct { + /** + * Name of this parameter group + */ + readonly parameterGroupName: string; + + /** + * Export this parameter group + */ + export(): InstanceParameterGroupImportProps; +} + +/** + * Properties to reference a instance parameter group + */ +export interface InstanceParameterGroupImportProps { + readonly parameterGroupName: string; +} + +/** + * Properties for a instance parameter group + */ +export interface InstanceParameterGroupProps { + /** + * Database family of this parameter group + */ + family: string; + + /** + * Description for this parameter group + */ + description: string; + + /** + * The parameters in this parameter group + */ + parameters?: Parameters; +} + +/** + * Defina a instance parameter group + */ +export class InstanceParameterGroup extends Construct implements IInstanceParameterGroup { + /** + * Import a parameter group + */ + public static import(scope: Construct, id: string, props: InstanceParameterGroupImportProps): IInstanceParameterGroup { + return new ImportedInstanceParameterGroup(scope, id, props); + } + + public readonly parameterGroupName: string; + private readonly parameters: Parameters = {}; + + constructor(scope: Construct, id: string, props: InstanceParameterGroupProps) { + super(scope, id); + + const { description, family, parameters } = props; + + const resource = new CfnDBParameterGroup(this, 'Resource', { + description, + family, + parameters: new Token(() => this.parameters) + }); + + for (const [key, value] of Object.entries(parameters || {})) { + this.setParameter(key, value); + } + + this.parameterGroupName = resource.ref; + } + + /** + * Export this parameter group + */ + public export(): InstanceParameterGroupImportProps { + return { + parameterGroupName: new CfnOutput(this, 'ParameterGroupName', { value: this.parameterGroupName }).makeImportValue().toString(), + }; + } + + /** + * Set a single parameter in this parameter group + */ + public setParameter(key: string, value: string | undefined) { + if (value === undefined && key in this.parameters) { + delete this.parameters[key]; + } + if (value !== undefined) { + this.parameters[key] = value; + } + } + + /** + * Remove a previously-set parameter from this parameter group + */ + public removeParameter(key: string) { + this.setParameter(key, undefined); + } + + /** + * Validate this construct + */ + protected validate(): string[] { + if (Object.keys(this.parameters).length === 0) { + return ['At least one parameter required, call setParameter().']; + } + return []; + } +} + +class ImportedInstanceParameterGroup extends Construct implements IInstanceParameterGroup { + public readonly parameterGroupName: string; + + constructor(scope: Construct, id: string, private readonly props: InstanceParameterGroupImportProps) { + super(scope, id); + + this.parameterGroupName = props.parameterGroupName; + } + + public export() { + return this.props; + } +} From a59d9adf3b296333996725ec1c180f6e600dd0e3 Mon Sep 17 00:00:00 2001 From: aereal Date: Wed, 20 Mar 2019 13:13:27 +0900 Subject: [PATCH 2/8] feat(rds): pass InstanceParameterGroup to CfnDBInstance --- packages/@aws-cdk/aws-rds/lib/cluster.ts | 3 +++ packages/@aws-cdk/aws-rds/lib/props.ts | 8 ++++++++ 2 files changed, 11 insertions(+) diff --git a/packages/@aws-cdk/aws-rds/lib/cluster.ts b/packages/@aws-cdk/aws-rds/lib/cluster.ts index 39465a1231a5c..b4b208aa2f43c 100644 --- a/packages/@aws-cdk/aws-rds/lib/cluster.ts +++ b/packages/@aws-cdk/aws-rds/lib/cluster.ts @@ -291,6 +291,8 @@ export class DatabaseCluster extends DatabaseClusterBase implements IDatabaseClu const publiclyAccessible = props.instanceProps.vpcSubnets && props.instanceProps.vpcSubnets.subnetType === ec2.SubnetType.Public; + const instanceParameterGroupName = props.instanceProps.parameterGroup && props.instanceProps.parameterGroup.parameterGroupName; + const instance = new CfnDBInstance(this, `Instance${instanceIndex}`, { // Link to cluster engine: props.engine, @@ -301,6 +303,7 @@ export class DatabaseCluster extends DatabaseClusterBase implements IDatabaseClu publiclyAccessible, // This is already set on the Cluster. Unclear to me whether it should be repeated or not. Better yes. dbSubnetGroupName: subnetGroup.ref, + dbParameterGroupName: instanceParameterGroupName, }); // We must have a dependency on the NAT gateway provider here to create diff --git a/packages/@aws-cdk/aws-rds/lib/props.ts b/packages/@aws-cdk/aws-rds/lib/props.ts index 4816f249cc083..e132d6908744e 100644 --- a/packages/@aws-cdk/aws-rds/lib/props.ts +++ b/packages/@aws-cdk/aws-rds/lib/props.ts @@ -1,5 +1,6 @@ import ec2 = require('@aws-cdk/aws-ec2'); import kms = require('@aws-cdk/aws-kms'); +import { IInstanceParameterGroup } from './instance-parameter-group'; /** * The engine for the database cluster @@ -36,6 +37,13 @@ export interface InstanceProps { * Security group. If not specified a new one will be created. */ securityGroup?: ec2.ISecurityGroup; + + /** + * Additional parameters to pass to the database instance + * + * @default No parameter group + */ + parameterGroup?: IInstanceParameterGroup; } /** From 46b75580e44de853101a8c411f6634f1ad5cfcc4 Mon Sep 17 00:00:00 2001 From: aereal Date: Fri, 22 Mar 2019 16:19:30 +0900 Subject: [PATCH 3/8] feat(rds): add tests for InstanceParameterGroup --- .../@aws-cdk/aws-rds/test/test.cluster.ts | 57 ++++++++++++++++++- 1 file changed, 56 insertions(+), 1 deletion(-) diff --git a/packages/@aws-cdk/aws-rds/test/test.cluster.ts b/packages/@aws-cdk/aws-rds/test/test.cluster.ts index cda81e806128f..c3829bb57a144 100644 --- a/packages/@aws-cdk/aws-rds/test/test.cluster.ts +++ b/packages/@aws-cdk/aws-rds/test/test.cluster.ts @@ -2,7 +2,7 @@ import { expect, haveResource } from '@aws-cdk/assert'; import ec2 = require('@aws-cdk/aws-ec2'); import cdk = require('@aws-cdk/cdk'); import { Test } from 'nodeunit'; -import { ClusterParameterGroup, DatabaseCluster, DatabaseClusterEngine } from '../lib'; +import { ClusterParameterGroup, DatabaseCluster, DatabaseClusterEngine, InstanceParameterGroup } from '../lib'; export = { 'check that instantiation works'(test: Test) { @@ -160,6 +160,43 @@ export = { test.done(); }, + 'cluster with instance parameter group'(test: Test) { + // GIVEN + const stack = testStack(); + const vpc = new ec2.VpcNetwork(stack, 'VPC'); + + // WHEN + const instanceParameterGroup = new InstanceParameterGroup(stack, 'Params', { + family: 'mysql', + description: 'bye', + }); + instanceParameterGroup.setParameter('param1', 'value1'); + instanceParameterGroup.setParameter('param2', 'value2'); + instanceParameterGroup.removeParameter('param2'); + new DatabaseCluster(stack, 'Database', { + engine: DatabaseClusterEngine.Aurora, + masterUser: { + username: 'admin', + password: 'tooshort', + }, + instances: 1, + instanceProps: { + instanceType: new ec2.InstanceTypePair(ec2.InstanceClass.Burstable2, ec2.InstanceSize.Small), + parameterGroup: instanceParameterGroup, + vpc + }, + }); + + // THEN + expect(stack).to(haveResource('AWS::RDS::DBInstance', { + DBParameterGroupName: { + Ref: 'ParamsA8366201', + }, + })); + + test.done(); + }, + 'import/export cluster parameter group'(test: Test) { // GIVEN const stack = testStack(); @@ -178,6 +215,24 @@ export = { test.done(); }, + 'import/export instance parameter group'(test: Test) { + // GIVEN + const stack = testStack(); + const group = new InstanceParameterGroup(stack, 'Params', { + family: 'hello', + description: 'desc' + }); + + // WHEN + const exported = group.export(); + const imported = InstanceParameterGroup.import(stack, 'ImportParams', exported); + + // THEN + test.deepEqual(stack.node.resolve(exported), { parameterGroupName: { 'Fn::ImportValue': 'Stack:ParamsParameterGroupNameA6B808D7' } }); + test.deepEqual(stack.node.resolve(imported.parameterGroupName), { 'Fn::ImportValue': 'Stack:ParamsParameterGroupNameA6B808D7' }); + test.done(); + }, + 'creates a secret when master credentials are not specified'(test: Test) { // GIVEN const stack = testStack(); From ad3d753ef84a3cb915332ff2b4b3ed44a2994d22 Mon Sep 17 00:00:00 2001 From: aereal Date: Sun, 24 Mar 2019 23:43:47 +0900 Subject: [PATCH 4/8] fix(rds): Make `parameters` required --- packages/@aws-cdk/aws-rds/lib/instance-parameter-group.ts | 2 +- packages/@aws-cdk/aws-rds/test/test.cluster.ts | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/@aws-cdk/aws-rds/lib/instance-parameter-group.ts b/packages/@aws-cdk/aws-rds/lib/instance-parameter-group.ts index 25c4f037e1489..e59ee6f9aad4d 100644 --- a/packages/@aws-cdk/aws-rds/lib/instance-parameter-group.ts +++ b/packages/@aws-cdk/aws-rds/lib/instance-parameter-group.ts @@ -41,7 +41,7 @@ export interface InstanceParameterGroupProps { /** * The parameters in this parameter group */ - parameters?: Parameters; + parameters: Parameters; } /** diff --git a/packages/@aws-cdk/aws-rds/test/test.cluster.ts b/packages/@aws-cdk/aws-rds/test/test.cluster.ts index c3829bb57a144..d180173124d58 100644 --- a/packages/@aws-cdk/aws-rds/test/test.cluster.ts +++ b/packages/@aws-cdk/aws-rds/test/test.cluster.ts @@ -169,6 +169,7 @@ export = { const instanceParameterGroup = new InstanceParameterGroup(stack, 'Params', { family: 'mysql', description: 'bye', + parameters: {}, }); instanceParameterGroup.setParameter('param1', 'value1'); instanceParameterGroup.setParameter('param2', 'value2'); @@ -220,7 +221,8 @@ export = { const stack = testStack(); const group = new InstanceParameterGroup(stack, 'Params', { family: 'hello', - description: 'desc' + description: 'desc', + parameters: {}, }); // WHEN From 8a22d7c4034c541d586d88bc6b0d30f510ff4a9f Mon Sep 17 00:00:00 2001 From: aereal Date: Sun, 24 Mar 2019 23:45:51 +0900 Subject: [PATCH 5/8] fix(rds): Use `dbParameterGroupName` instead of `ref` --- packages/@aws-cdk/aws-rds/lib/instance-parameter-group.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@aws-cdk/aws-rds/lib/instance-parameter-group.ts b/packages/@aws-cdk/aws-rds/lib/instance-parameter-group.ts index e59ee6f9aad4d..9fd24266be97e 100644 --- a/packages/@aws-cdk/aws-rds/lib/instance-parameter-group.ts +++ b/packages/@aws-cdk/aws-rds/lib/instance-parameter-group.ts @@ -73,7 +73,7 @@ export class InstanceParameterGroup extends Construct implements IInstanceParame this.setParameter(key, value); } - this.parameterGroupName = resource.ref; + this.parameterGroupName = resource.dbParameterGroupName; } /** From db1c8f3d461687a5e11a97d8c63cb9de2b6fa9a8 Mon Sep 17 00:00:00 2001 From: aereal Date: Sun, 24 Mar 2019 23:48:50 +0900 Subject: [PATCH 6/8] fix(rds): removed `InstanceParameterGroupRef` we already introduced `ImportedInstanceParameterGroup` --- .../lib/instance-parameter-group-ref.ts | 44 ------------------- 1 file changed, 44 deletions(-) delete mode 100644 packages/@aws-cdk/aws-rds/lib/instance-parameter-group-ref.ts diff --git a/packages/@aws-cdk/aws-rds/lib/instance-parameter-group-ref.ts b/packages/@aws-cdk/aws-rds/lib/instance-parameter-group-ref.ts deleted file mode 100644 index 3854eeec81a1a..0000000000000 --- a/packages/@aws-cdk/aws-rds/lib/instance-parameter-group-ref.ts +++ /dev/null @@ -1,44 +0,0 @@ -import { CfnOutput, Construct } from "@aws-cdk/cdk"; - -/** - * A instance parameter group - */ -export abstract class InstanceParameterGroupRef extends Construct { - /** - * Import a parameter group - */ - public static import(scope: Construct, id: string, props: InstanceParameterGroupRefProps): InstanceParameterGroupRef { - return new ImportedInstanceParameterGroup(scope, id, props); - } - - /** - * Name of this parameter group - */ - public abstract readonly parameterGroupName: string; - - /** - * Export this parameter group - */ - public export(): InstanceParameterGroupRefProps { - return { - parameterGroupName: new CfnOutput(this, 'ParameterGroupName', { value: this.parameterGroupName}).makeImportValue().toString(), - }; - } -} - -/** - * Properties to reference a instance parameter group - */ -export interface InstanceParameterGroupRefProps { - parameterGroupName: string -} - -class ImportedInstanceParameterGroup extends InstanceParameterGroupRef { - public readonly parameterGroupName: string; - - constructor(scope: Construct, id: string, props: InstanceParameterGroupRefProps) { - super(scope, id); - - this.parameterGroupName = props.parameterGroupName; - } -} From 9c77fbf772bfe19185dcc7aa9a6e24846d5dd491 Mon Sep 17 00:00:00 2001 From: aereal Date: Mon, 25 Mar 2019 00:14:18 +0900 Subject: [PATCH 7/8] fix(rds): add test for InstanceParameterGroup#validate --- packages/@aws-cdk/aws-rds/test/test.cluster.ts | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/packages/@aws-cdk/aws-rds/test/test.cluster.ts b/packages/@aws-cdk/aws-rds/test/test.cluster.ts index d180173124d58..82814f9b5425b 100644 --- a/packages/@aws-cdk/aws-rds/test/test.cluster.ts +++ b/packages/@aws-cdk/aws-rds/test/test.cluster.ts @@ -290,7 +290,21 @@ export = { })); test.done(); - } + }, + + 'empty instance parameter group is not valid'(test: Test) { + const stack = testStack(); + new InstanceParameterGroup(stack, 'Params', { + family: 'hello', + description: 'desc', + parameters: {}, + }); + + const errorMessages = stack.node.validateTree().map(e => e.message); + test.deepEqual(errorMessages, ['At least one parameter required, call setParameter().']); + + test.done(); + }, }; function testStack() { From 843516477134293922439c66d8f0c3074212961c Mon Sep 17 00:00:00 2001 From: aereal Date: Mon, 25 Mar 2019 00:22:01 +0900 Subject: [PATCH 8/8] fix(rds): add more detailed docs for `InstanceParameterGroupProps.family` --- packages/@aws-cdk/aws-rds/lib/instance-parameter-group.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/@aws-cdk/aws-rds/lib/instance-parameter-group.ts b/packages/@aws-cdk/aws-rds/lib/instance-parameter-group.ts index 9fd24266be97e..93fbcaa36341b 100644 --- a/packages/@aws-cdk/aws-rds/lib/instance-parameter-group.ts +++ b/packages/@aws-cdk/aws-rds/lib/instance-parameter-group.ts @@ -30,6 +30,10 @@ export interface InstanceParameterGroupImportProps { export interface InstanceParameterGroupProps { /** * Database family of this parameter group + * + * @see https://docs.aws.amazon.com/AmazonRDS/latest/APIReference/API_CreateDBParameterGroup.html#API_CreateDBParameterGroup_RequestParameters + * @example + * { family: 'MySQL5.5' } */ family: string;