From 2ab329f56d06c376f1fa7c23246ce74958a08bac Mon Sep 17 00:00:00 2001 From: Hyeonsoo David Lee Date: Fri, 17 Jul 2020 02:36:41 +0900 Subject: [PATCH 1/3] feat(rds): Allow multiple secrets to be passed to an RDS Proxy (#9103) resolves #9098 BREAKING CHANGE: `DatabaseProxyProps.secret` => `DatabaseProxyProps.secrets[]` ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- packages/@aws-cdk/aws-rds/README.md | 4 +-- packages/@aws-cdk/aws-rds/lib/proxy.ts | 23 ++++++++------ packages/@aws-cdk/aws-rds/test/integ.proxy.ts | 2 +- packages/@aws-cdk/aws-rds/test/test.proxy.ts | 31 +++++++++++++++++-- 4 files changed, 45 insertions(+), 15 deletions(-) diff --git a/packages/@aws-cdk/aws-rds/README.md b/packages/@aws-cdk/aws-rds/README.md index ea3e4dee8b75d..be60c3f0a1a70 100644 --- a/packages/@aws-cdk/aws-rds/README.md +++ b/packages/@aws-cdk/aws-rds/README.md @@ -252,13 +252,13 @@ import * as secrets from '@aws-cdk/aws-secretsmanager'; const vpc: ec2.IVpc = ...; const securityGroup: ec2.ISecurityGroup = ...; -const secret: secrets.ISecret = ...; +const secrets: secrets.ISecret[] = [...]; const dbInstance: rds.IDatabaseInstance = ...; const proxy = dbInstance.addProxy('proxy', { connectionBorrowTimeout: cdk.Duration.seconds(30), maxConnectionsPercent: 50, - secret, + secrets, vpc, }); ``` diff --git a/packages/@aws-cdk/aws-rds/lib/proxy.ts b/packages/@aws-cdk/aws-rds/lib/proxy.ts index 9733056d8ed3b..e6e0ba5d66679 100644 --- a/packages/@aws-cdk/aws-rds/lib/proxy.ts +++ b/packages/@aws-cdk/aws-rds/lib/proxy.ts @@ -235,10 +235,9 @@ export interface DatabaseProxyOptions { /** * The secret that the proxy uses to authenticate to the RDS DB instance or Aurora DB cluster. * These secrets are stored within Amazon Secrets Manager. - * - * @default - no secret + * One or more secrets are required. */ - readonly secret: secretsmanager.ISecret; + readonly secrets: secretsmanager.ISecret[]; /** * One or more VPC security groups to associate with the new proxy. @@ -379,20 +378,26 @@ export class DatabaseProxy extends cdk.Resource assumedBy: new iam.ServicePrincipal('rds.amazonaws.com'), }); - props.secret.grantRead(role); + for (const secret of props.secrets) { + secret.grantRead(role); + } this.connections = new ec2.Connections({ securityGroups: props.securityGroups }); const bindResult = props.proxyTarget.bind(this); + if (props.secrets.length < 1) { + throw new Error('One or more secrets are required.'); + } + this.resource = new CfnDBProxy(this, 'Resource', { - auth: [ - { + auth: props.secrets.map(_ => { + return { authScheme: 'SECRETS', iamAuth: props.iamAuth ? 'REQUIRED' : 'DISABLED', - secretArn: props.secret.secretArn, - }, - ], + secretArn: _.secretArn, + }; + }), dbProxyName: this.physicalName, debugLogging: props.debugLogging, engineFamily: bindResult.engineFamily, diff --git a/packages/@aws-cdk/aws-rds/test/integ.proxy.ts b/packages/@aws-cdk/aws-rds/test/integ.proxy.ts index 22f5535237d77..8c2ef1879787b 100644 --- a/packages/@aws-cdk/aws-rds/test/integ.proxy.ts +++ b/packages/@aws-cdk/aws-rds/test/integ.proxy.ts @@ -17,7 +17,7 @@ const dbInstance = new rds.DatabaseInstance(stack, 'dbInstance', { new rds.DatabaseProxy(stack, 'dbProxy', { borrowTimeout: cdk.Duration.seconds(30), maxConnectionsPercent: 50, - secret: dbInstance.secret!, + secrets: [dbInstance.secret!], proxyTarget: rds.ProxyTarget.fromInstance(dbInstance), vpc, }); diff --git a/packages/@aws-cdk/aws-rds/test/test.proxy.ts b/packages/@aws-cdk/aws-rds/test/test.proxy.ts index 828f1f78bc7e9..3473527054d06 100644 --- a/packages/@aws-cdk/aws-rds/test/test.proxy.ts +++ b/packages/@aws-cdk/aws-rds/test/test.proxy.ts @@ -19,7 +19,7 @@ export = { // WHEN new rds.DatabaseProxy(stack, 'Proxy', { proxyTarget: rds.ProxyTarget.fromInstance(instance), - secret: instance.secret!, + secrets: [instance.secret!], vpc, }); @@ -94,7 +94,7 @@ export = { // WHEN new rds.DatabaseProxy(stack, 'Proxy', { proxyTarget: rds.ProxyTarget.fromCluster(cluster), - secret: cluster.secret!, + secrets: [cluster.secret!], vpc, }); @@ -170,7 +170,7 @@ export = { test.doesNotThrow(() => { new rds.DatabaseProxy(stack, 'Proxy', { proxyTarget: rds.ProxyTarget.fromCluster(cluster), - secret: cluster.secret!, + secrets: [cluster.secret!], vpc, }); }, /Cannot specify both dbInstanceIdentifiers and dbClusterIdentifiers/); @@ -181,4 +181,29 @@ export = { test.done(); }, + + 'One or more secrets are required.'(test: Test) { + // GIVEN + const stack = new cdk.Stack(); + const vpc = new ec2.Vpc(stack, 'VPC'); + const cluster = new rds.DatabaseCluster(stack, 'Database', { + engine: rds.DatabaseClusterEngine.auroraPostgres({ version: '10.7' }), + masterUser: { username: 'admin' }, + instanceProps: { + instanceType: ec2.InstanceType.of(ec2.InstanceClass.BURSTABLE2, ec2.InstanceSize.SMALL), + vpc, + }, + }); + + // WHEN + test.throws(() => { + new rds.DatabaseProxy(stack, 'Proxy', { + proxyTarget: rds.ProxyTarget.fromCluster(cluster), + secrets: [], // No secret + vpc, + }); + }, 'One or more secrets are required.'); + + test.done(); + }, }; From 846a222140984d0aaed948d5bb1f3127a2cc6eb1 Mon Sep 17 00:00:00 2001 From: Jeff Strunk Date: Thu, 16 Jul 2020 14:11:04 -0400 Subject: [PATCH 2/3] feat(aws-stepfunctions-tasks): allow lambda invocations to combine input and function results (#9022) Step Functions allows a third way to invoke Lambda functions that was previously unimplemented in LambdaInvoke by specifying the function ARN as the Resource. As implemented, LambdaInvoke does not allow the combination of the inputPath and the Lambda function result without including extra metadata. A suggestion workaround was to add a `Pass` state to manipulate the output. This change implements that option without requiring an extra state transition. fixes #8943 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../aws-stepfunctions-tasks/README.md | 13 ++ .../lib/lambda/invoke.ts | 45 +++- .../integ.invoke.payload.only.expected.json | 212 ++++++++++++++++++ .../test/lambda/integ.invoke.payload.only.ts | 76 +++++++ .../test/lambda/invoke.test.ts | 98 ++++++++ 5 files changed, 434 insertions(+), 10 deletions(-) create mode 100644 packages/@aws-cdk/aws-stepfunctions-tasks/test/lambda/integ.invoke.payload.only.expected.json create mode 100644 packages/@aws-cdk/aws-stepfunctions-tasks/test/lambda/integ.invoke.payload.only.ts diff --git a/packages/@aws-cdk/aws-stepfunctions-tasks/README.md b/packages/@aws-cdk/aws-stepfunctions-tasks/README.md index 305530f60d0fe..23b9f6660f65e 100644 --- a/packages/@aws-cdk/aws-stepfunctions-tasks/README.md +++ b/packages/@aws-cdk/aws-stepfunctions-tasks/README.md @@ -623,6 +623,19 @@ new tasks.LambdaInvoke(this, 'Invoke and set function response as task output', }); ``` +If you want to combine the input and the Lambda function response you can use +the `payloadResponseOnly` property and specify the `resultPath`. This will put the +Lambda function ARN directly in the "Resource" string, but it conflicts with the +integrationPattern, invocationType, clientContext, and qualifier properties. + +```ts +new tasks.LambdaInvoke(this, 'Invoke and combine function response with task input', { + lambdaFunction: myLambda, + payloadResponseOnly: true, + resultPath: '$.myLambda', +}); +``` + You can have Step Functions pause a task, and wait for an external process to return a task token. Read more about the [callback pattern](https://docs.aws.amazon.com/step-functions/latest/dg/callback-task-sample-sqs.html#call-back-lambda-example) diff --git a/packages/@aws-cdk/aws-stepfunctions-tasks/lib/lambda/invoke.ts b/packages/@aws-cdk/aws-stepfunctions-tasks/lib/lambda/invoke.ts index 90ac0c2f0ab33..6c44c555f80a4 100644 --- a/packages/@aws-cdk/aws-stepfunctions-tasks/lib/lambda/invoke.ts +++ b/packages/@aws-cdk/aws-stepfunctions-tasks/lib/lambda/invoke.ts @@ -46,6 +46,17 @@ export interface LambdaInvokeProps extends sfn.TaskStateBaseProps { * @default - Version or alias inherent to the `lambdaFunction` object. */ readonly qualifier?: string; + + /** + * Invoke the Lambda in a way that only returns the payload response without additional metadata. + * + * The `payloadResponseOnly` property cannot be used if `integrationPattern`, `invocationType`, + * `clientContext`, or `qualifier` are specified. + * It always uses the REQUEST_RESPONSE behavior. + * + * @default false + */ + readonly payloadResponseOnly?: boolean; } /** @@ -76,6 +87,13 @@ export class LambdaInvoke extends sfn.TaskStateBase { throw new Error('Task Token is required in `payload` for callback. Use JsonPath.taskToken to set the token.'); } + if (props.payloadResponseOnly && + (props.integrationPattern || props.invocationType || props.clientContext || props.qualifier)) { + throw new Error( + "The 'payloadResponseOnly' property cannot be used if 'integrationPattern', 'invocationType', 'clientContext', or 'qualifier' are specified.", + ); + } + this.taskMetrics = { metricPrefixSingular: 'LambdaFunction', metricPrefixPlural: 'LambdaFunctions', @@ -100,16 +118,23 @@ export class LambdaInvoke extends sfn.TaskStateBase { * @internal */ protected _renderTask(): any { - return { - Resource: integrationResourceArn('lambda', 'invoke', this.integrationPattern), - Parameters: sfn.FieldUtils.renderObject({ - FunctionName: this.props.lambdaFunction.functionArn, - Payload: this.props.payload ? this.props.payload.value : sfn.TaskInput.fromDataAt('$').value, - InvocationType: this.props.invocationType, - ClientContext: this.props.clientContext, - Qualifier: this.props.qualifier, - }), - }; + if (this.props.payloadResponseOnly) { + return { + Resource: this.props.lambdaFunction.functionArn, + ...this.props.payload && { Parameters: sfn.FieldUtils.renderObject(this.props.payload.value) }, + }; + } else { + return { + Resource: integrationResourceArn('lambda', 'invoke', this.integrationPattern), + Parameters: sfn.FieldUtils.renderObject({ + FunctionName: this.props.lambdaFunction.functionArn, + Payload: this.props.payload ? this.props.payload.value : sfn.TaskInput.fromDataAt('$').value, + InvocationType: this.props.invocationType, + ClientContext: this.props.clientContext, + Qualifier: this.props.qualifier, + }), + }; + } } } diff --git a/packages/@aws-cdk/aws-stepfunctions-tasks/test/lambda/integ.invoke.payload.only.expected.json b/packages/@aws-cdk/aws-stepfunctions-tasks/test/lambda/integ.invoke.payload.only.expected.json new file mode 100644 index 0000000000000..b994f8b20cc5e --- /dev/null +++ b/packages/@aws-cdk/aws-stepfunctions-tasks/test/lambda/integ.invoke.payload.only.expected.json @@ -0,0 +1,212 @@ +{ + "Resources": { + "submitJobLambdaServiceRole4D897ABD": { + "Type": "AWS::IAM::Role", + "Properties": { + "AssumeRolePolicyDocument": { + "Statement": [ + { + "Action": "sts:AssumeRole", + "Effect": "Allow", + "Principal": { + "Service": "lambda.amazonaws.com" + } + } + ], + "Version": "2012-10-17" + }, + "ManagedPolicyArns": [ + { + "Fn::Join": [ + "", + [ + "arn:", + { + "Ref": "AWS::Partition" + }, + ":iam::aws:policy/service-role/AWSLambdaBasicExecutionRole" + ] + ] + } + ] + } + }, + "submitJobLambdaEFB00F3C": { + "Type": "AWS::Lambda::Function", + "Properties": { + "Code": { + "ZipFile": "exports.handler = async () => {\n return {\n statusCode: '200',\n body: 'hello, world!'\n };\n };" + }, + "Handler": "index.handler", + "Role": { + "Fn::GetAtt": [ + "submitJobLambdaServiceRole4D897ABD", + "Arn" + ] + }, + "Runtime": "nodejs10.x" + }, + "DependsOn": [ + "submitJobLambdaServiceRole4D897ABD" + ] + }, + "checkJobStateLambdaServiceRoleB8B57B65": { + "Type": "AWS::IAM::Role", + "Properties": { + "AssumeRolePolicyDocument": { + "Statement": [ + { + "Action": "sts:AssumeRole", + "Effect": "Allow", + "Principal": { + "Service": "lambda.amazonaws.com" + } + } + ], + "Version": "2012-10-17" + }, + "ManagedPolicyArns": [ + { + "Fn::Join": [ + "", + [ + "arn:", + { + "Ref": "AWS::Partition" + }, + ":iam::aws:policy/service-role/AWSLambdaBasicExecutionRole" + ] + ] + } + ] + } + }, + "checkJobStateLambda4618B7B7": { + "Type": "AWS::Lambda::Function", + "Properties": { + "Code": { + "ZipFile": "exports.handler = async function(event, context) {\n return {\n status: event.statusCode === '200' ? 'SUCCEEDED' : 'FAILED'\n };\n };" + }, + "Handler": "index.handler", + "Role": { + "Fn::GetAtt": [ + "checkJobStateLambdaServiceRoleB8B57B65", + "Arn" + ] + }, + "Runtime": "nodejs10.x" + }, + "DependsOn": [ + "checkJobStateLambdaServiceRoleB8B57B65" + ] + }, + "StateMachineRoleB840431D": { + "Type": "AWS::IAM::Role", + "Properties": { + "AssumeRolePolicyDocument": { + "Statement": [ + { + "Action": "sts:AssumeRole", + "Effect": "Allow", + "Principal": { + "Service": { + "Fn::Join": [ + "", + [ + "states.", + { + "Ref": "AWS::Region" + }, + ".amazonaws.com" + ] + ] + } + } + } + ], + "Version": "2012-10-17" + } + } + }, + "StateMachineRoleDefaultPolicyDF1E6607": { + "Type": "AWS::IAM::Policy", + "Properties": { + "PolicyDocument": { + "Statement": [ + { + "Action": "lambda:InvokeFunction", + "Effect": "Allow", + "Resource": { + "Fn::GetAtt": [ + "submitJobLambdaEFB00F3C", + "Arn" + ] + } + }, + { + "Action": "lambda:InvokeFunction", + "Effect": "Allow", + "Resource": { + "Fn::GetAtt": [ + "checkJobStateLambda4618B7B7", + "Arn" + ] + } + } + ], + "Version": "2012-10-17" + }, + "PolicyName": "StateMachineRoleDefaultPolicyDF1E6607", + "Roles": [ + { + "Ref": "StateMachineRoleB840431D" + } + ] + } + }, + "StateMachine2E01A3A5": { + "Type": "AWS::StepFunctions::StateMachine", + "Properties": { + "RoleArn": { + "Fn::GetAtt": [ + "StateMachineRoleB840431D", + "Arn" + ] + }, + "DefinitionString": { + "Fn::Join": [ + "", + [ + "{\"StartAt\":\"Invoke Handler\",\"States\":{\"Invoke Handler\":{\"Next\":\"Check the job state\",\"Type\":\"Task\",\"Resource\":\"", + { + "Fn::GetAtt": [ + "submitJobLambdaEFB00F3C", + "Arn" + ] + }, + "\"},\"Check the job state\":{\"Next\":\"Job Complete?\",\"Type\":\"Task\",\"Resource\":\"", + { + "Fn::GetAtt": [ + "checkJobStateLambda4618B7B7", + "Arn" + ] + }, + "\"},\"Job Complete?\":{\"Type\":\"Choice\",\"Choices\":[{\"Variable\":\"$.status\",\"StringEquals\":\"FAILED\",\"Next\":\"Job Failed\"},{\"Variable\":\"$.status\",\"StringEquals\":\"SUCCEEDED\",\"Next\":\"Final step\"}]},\"Job Failed\":{\"Type\":\"Fail\",\"Error\":\"Received a status that was not 200\",\"Cause\":\"Job Failed\"},\"Final step\":{\"Type\":\"Pass\",\"End\":true}},\"TimeoutSeconds\":30}" + ] + ] + } + }, + "DependsOn": [ + "StateMachineRoleDefaultPolicyDF1E6607", + "StateMachineRoleB840431D" + ] + } + }, + "Outputs": { + "stateMachineArn": { + "Value": { + "Ref": "StateMachine2E01A3A5" + } + } + } +} \ No newline at end of file diff --git a/packages/@aws-cdk/aws-stepfunctions-tasks/test/lambda/integ.invoke.payload.only.ts b/packages/@aws-cdk/aws-stepfunctions-tasks/test/lambda/integ.invoke.payload.only.ts new file mode 100644 index 0000000000000..711a5ceb458f6 --- /dev/null +++ b/packages/@aws-cdk/aws-stepfunctions-tasks/test/lambda/integ.invoke.payload.only.ts @@ -0,0 +1,76 @@ +import { Code, Function, Runtime } from '@aws-cdk/aws-lambda'; +import * as sfn from '@aws-cdk/aws-stepfunctions'; +import * as cdk from '@aws-cdk/core'; +import { LambdaInvoke } from '../../lib'; + +/* + * Creates a state machine with a task state to invoke a Lambda function + * The state machine creates a couple of Lambdas that pass results forward + * and into a Choice state that validates the output. + * + * Stack verification steps: + * The generated State Machine can be executed from the CLI (or Step Functions console) + * and runs with an execution status of `Succeeded`. + * + * -- aws stepfunctions start-execution --state-machine-arn provides execution arn + * -- aws stepfunctions describe-execution --execution-arn returns a status of `Succeeded` + */ +const app = new cdk.App(); +const stack = new cdk.Stack(app, 'aws-stepfunctions-tasks-lambda-invoke-integ'); + +const submitJobLambda = new Function(stack, 'submitJobLambda', { + code: Code.fromInline(`exports.handler = async () => { + return { + statusCode: '200', + body: 'hello, world!' + }; + };`), + runtime: Runtime.NODEJS_10_X, + handler: 'index.handler', +}); + +const submitJob = new LambdaInvoke(stack, 'Invoke Handler', { + lambdaFunction: submitJobLambda, + payloadResponseOnly: true, +}); + +const checkJobStateLambda = new Function(stack, 'checkJobStateLambda', { + code: Code.fromInline(`exports.handler = async function(event, context) { + return { + status: event.statusCode === '200' ? 'SUCCEEDED' : 'FAILED' + }; + };`), + runtime: Runtime.NODEJS_10_X, + handler: 'index.handler', +}); + +const checkJobState = new LambdaInvoke(stack, 'Check the job state', { + lambdaFunction: checkJobStateLambda, + payloadResponseOnly: true, +}); + +const isComplete = new sfn.Choice(stack, 'Job Complete?'); +const jobFailed = new sfn.Fail(stack, 'Job Failed', { + cause: 'Job Failed', + error: 'Received a status that was not 200', +}); +const finalStatus = new sfn.Pass(stack, 'Final step'); + +const chain = sfn.Chain.start(submitJob) + .next(checkJobState) + .next( + isComplete + .when(sfn.Condition.stringEquals('$.status', 'FAILED'), jobFailed) + .when(sfn.Condition.stringEquals('$.status', 'SUCCEEDED'), finalStatus), + ); + +const sm = new sfn.StateMachine(stack, 'StateMachine', { + definition: chain, + timeout: cdk.Duration.seconds(30), +}); + +new cdk.CfnOutput(stack, 'stateMachineArn', { + value: sm.stateMachineArn, +}); + +app.synth(); diff --git a/packages/@aws-cdk/aws-stepfunctions-tasks/test/lambda/invoke.test.ts b/packages/@aws-cdk/aws-stepfunctions-tasks/test/lambda/invoke.test.ts index 0919ddfae535a..dc2cf11ac367b 100644 --- a/packages/@aws-cdk/aws-stepfunctions-tasks/test/lambda/invoke.test.ts +++ b/packages/@aws-cdk/aws-stepfunctions-tasks/test/lambda/invoke.test.ts @@ -177,6 +177,104 @@ describe('LambdaInvoke', () => { }); }); + test('Invoke lambda with payloadResponseOnly', () => { + // WHEN + const task = new LambdaInvoke(stack, 'Task', { + lambdaFunction, + payloadResponseOnly: true, + }); + + // THEN + expect(stack.resolve(task.toStateJson())).toEqual({ + End: true, + Type: 'Task', + Resource: { + 'Fn::GetAtt': [ + 'Fn9270CBC0', + 'Arn', + ], + }, + }); + }); + + test('Invoke lambda with payloadResponseOnly with payload', () => { + // WHEN + const task = new LambdaInvoke(stack, 'Task', { + lambdaFunction, + payloadResponseOnly: true, + payload: sfn.TaskInput.fromObject({ + foo: 'bar', + }), + }); + + // THEN + expect(stack.resolve(task.toStateJson())).toEqual({ + End: true, + Type: 'Task', + Resource: { + 'Fn::GetAtt': [ + 'Fn9270CBC0', + 'Arn', + ], + }, + Parameters: { + foo: 'bar', + }, + }); + }); + + test('fails when integrationPattern used with payloadResponseOnly', () => { + expect(() => { + new LambdaInvoke(stack, 'Task', { + lambdaFunction, + payloadResponseOnly: true, + integrationPattern: sfn.IntegrationPattern.WAIT_FOR_TASK_TOKEN, + payload: sfn.TaskInput.fromObject({ + token: sfn.JsonPath.taskToken, + }), + }); + }).toThrow(/The 'payloadResponseOnly' property cannot be used if 'integrationPattern', 'invocationType', 'clientContext', or 'qualifier' are specified./); + }); + + test('fails when invocationType used with payloadResponseOnly', () => { + expect(() => { + new LambdaInvoke(stack, 'Task', { + lambdaFunction, + payloadResponseOnly: true, + payload: sfn.TaskInput.fromObject({ + foo: 'bar', + }), + invocationType: LambdaInvocationType.REQUEST_RESPONSE, + }); + }).toThrow(/The 'payloadResponseOnly' property cannot be used if 'integrationPattern', 'invocationType', 'clientContext', or 'qualifier' are specified./); + }); + + test('fails when clientContext used with payloadResponseOnly', () => { + expect(() => { + new LambdaInvoke(stack, 'Task', { + lambdaFunction, + payloadResponseOnly: true, + payload: sfn.TaskInput.fromObject({ + foo: 'bar', + }), + clientContext: 'eyJoZWxsbyI6IndvcmxkIn0=', + }); + }).toThrow(/The 'payloadResponseOnly' property cannot be used if 'integrationPattern', 'invocationType', 'clientContext', or 'qualifier' are specified./); + }); + + test('fails when qualifier used with payloadResponseOnly', () => { + expect(() => { + new LambdaInvoke(stack, 'Task', { + lambdaFunction, + payloadResponseOnly: true, + payload: sfn.TaskInput.fromObject({ + foo: 'bar', + }), + qualifier: '1', + }); + }).toThrow(/The 'payloadResponseOnly' property cannot be used if 'integrationPattern', 'invocationType', 'clientContext', or 'qualifier' are specified./); + }); + test('fails when WAIT_FOR_TASK_TOKEN integration pattern is used without supplying a task token in payload', () => { expect(() => { new LambdaInvoke(stack, 'Task', { From 17b690bc4573a9b57de7a0aa6591c4e2f98a3f2e Mon Sep 17 00:00:00 2001 From: Adam Ruka Date: Thu, 16 Jul 2020 11:31:48 -0700 Subject: [PATCH 3/3] feat(rds): unify ParameterGroup and ClusterParameterGroup (#8959) The classes ParameterGroup and ClusterParameterGroup were basically identical; they only differed in the L1 that was created in their scope. Unify the classes by adding a bind()-like protocol between ParameterGroup and cluster/instance, which allows us to only have a single class instead of 2. Fixes #8932 BREAKING CHANGE: the class ClusterParameterGroup has been removed - use ParameterGroup instead ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../@aws-cdk/aws-rds/lib/cluster-engine.ts | 70 +------ packages/@aws-cdk/aws-rds/lib/cluster.ts | 6 +- packages/@aws-cdk/aws-rds/lib/engine.ts | 26 +++ packages/@aws-cdk/aws-rds/lib/index.ts | 1 + .../@aws-cdk/aws-rds/lib/instance-engine.ts | 23 +-- packages/@aws-cdk/aws-rds/lib/instance.ts | 3 +- .../@aws-cdk/aws-rds/lib/parameter-group.ts | 192 ++++++++++-------- .../private/parameter-group-family-mapping.ts | 27 +++ packages/@aws-cdk/aws-rds/package.json | 1 - .../@aws-cdk/aws-rds/test/integ.cluster.ts | 7 +- .../aws-rds/test/integ.instance.lit.ts | 4 +- .../@aws-cdk/aws-rds/test/test.cluster.ts | 14 +- .../@aws-cdk/aws-rds/test/test.instance.ts | 2 +- .../aws-rds/test/test.parameter-group.ts | 69 +++++-- 14 files changed, 259 insertions(+), 186 deletions(-) create mode 100644 packages/@aws-cdk/aws-rds/lib/engine.ts diff --git a/packages/@aws-cdk/aws-rds/lib/cluster-engine.ts b/packages/@aws-cdk/aws-rds/lib/cluster-engine.ts index 7b5ec932e52cd..799cd14d88694 100644 --- a/packages/@aws-cdk/aws-rds/lib/cluster-engine.ts +++ b/packages/@aws-cdk/aws-rds/lib/cluster-engine.ts @@ -1,9 +1,9 @@ import * as iam from '@aws-cdk/aws-iam'; import * as secretsmanager from '@aws-cdk/aws-secretsmanager'; import * as core from '@aws-cdk/core'; -import { ClusterParameterGroup, IParameterGroup, ParameterGroup } from './parameter-group'; -import { ParameterGroupFamilyMapping } from './private/parameter-group-family-mapping'; -import { compare } from './private/version'; +import { IEngine } from './engine'; +import { IParameterGroup, ParameterGroup } from './parameter-group'; +import { calculateParameterGroupFamily, ParameterGroupFamilyMapping } from './private/parameter-group-family-mapping'; /** * The extra options passed to the {@link IClusterEngine.bindToCluster} method. @@ -54,25 +54,7 @@ export interface ClusterEngineConfig { /** * The interface representing a database cluster (as opposed to instance) engine. */ -export interface IClusterEngine { - /** The type of the engine, for example "aurora-mysql". */ - readonly engineType: string; - - /** - * The exact version of a given engine. - * - * @default - default version for the given engine type - */ - readonly engineVersion?: string; - - /** - * The family to use for ParameterGroups using this engine. - * This is usually equal to "", - * but can sometimes be a variation of that. - * You can pass this property when creating new ClusterParameterGroup. - */ - readonly parameterGroupFamily: string; - +export interface IClusterEngine extends IEngine { /** The application used by this engine to perform rotation for a single-user scenario. */ readonly singleUserRotationApplication: secretsmanager.SecretRotationApplication; @@ -101,17 +83,19 @@ abstract class ClusterEngineBase implements IClusterEngine { public readonly singleUserRotationApplication: secretsmanager.SecretRotationApplication; public readonly multiUserRotationApplication: secretsmanager.SecretRotationApplication; - private readonly parameterGroupFamilies?: ParameterGroupFamilyMapping[]; private readonly defaultPort?: number; constructor(props: ClusterEngineBaseProps) { this.engineType = props.engineType; this.singleUserRotationApplication = props.singleUserRotationApplication; this.multiUserRotationApplication = props.multiUserRotationApplication; - this.parameterGroupFamilies = props.parameterGroupFamilies; this.defaultPort = props.defaultPort; this.engineVersion = props.engineVersion; - this.parameterGroupFamily = this.establishParameterGroupFamily(); + const parameterGroupFamily = calculateParameterGroupFamily(props.parameterGroupFamilies, props.engineVersion); + if (parameterGroupFamily === undefined) { + throw new Error(`No parameter group family found for database engine ${this.engineType} with version ${this.engineVersion}.`); + } + this.parameterGroupFamily = parameterGroupFamily; } public bindToCluster(scope: core.Construct, options: ClusterEngineBindOptions): ClusterEngineConfig { @@ -128,46 +112,14 @@ abstract class ClusterEngineBase implements IClusterEngine { * if one wasn't provided by the customer explicitly. */ protected abstract defaultParameterGroup(scope: core.Construct): IParameterGroup | undefined; - - private establishParameterGroupFamily(): string { - const ret = this.calculateParameterGroupFamily(); - if (ret === undefined) { - throw new Error(`No parameter group family found for database engine ${this.engineType} with version ${this.engineVersion}.`); - } - return ret; - } - - /** - * Get the latest parameter group family for this engine. Latest is determined using semver on the engine major version. - * When `engineVersion` is specified, return the parameter group family corresponding to that engine version. - * Return undefined if no parameter group family is defined for this engine or for the requested `engineVersion`. - */ - private calculateParameterGroupFamily(): string | undefined { - if (this.parameterGroupFamilies === undefined) { - return undefined; - } - const engineVersion = this.engineVersion; - if (engineVersion !== undefined) { - const family = this.parameterGroupFamilies.find(x => engineVersion.startsWith(x.engineMajorVersion)); - if (family) { - return family.parameterGroupFamily; - } - } else if (this.parameterGroupFamilies.length > 0) { - const sorted = this.parameterGroupFamilies.slice().sort((a, b) => { - return compare(a.engineMajorVersion, b.engineMajorVersion); - }).reverse(); - return sorted[0].parameterGroupFamily; - } - return undefined; - } } abstract class MySqlClusterEngineBase extends ClusterEngineBase { public bindToCluster(scope: core.Construct, options: ClusterEngineBindOptions): ClusterEngineConfig { const config = super.bindToCluster(scope, options); const parameterGroup = options.parameterGroup ?? (options.s3ImportRole || options.s3ExportRole - ? new ClusterParameterGroup(scope, 'ClusterParameterGroup', { - family: this.parameterGroupFamily, + ? new ParameterGroup(scope, 'ClusterParameterGroup', { + engine: this, }) : config.parameterGroup); if (options.s3ImportRole) { diff --git a/packages/@aws-cdk/aws-rds/lib/cluster.ts b/packages/@aws-cdk/aws-rds/lib/cluster.ts index e3a1d36286d93..b44d7ecc19050 100644 --- a/packages/@aws-cdk/aws-rds/lib/cluster.ts +++ b/packages/@aws-cdk/aws-rds/lib/cluster.ts @@ -415,6 +415,7 @@ export class DatabaseCluster extends DatabaseClusterBase { parameterGroup: props.parameterGroup, }); const clusterParameterGroup = props.parameterGroup ?? clusterEngineBindConfig.parameterGroup; + const clusterParameterGroupConfig = clusterParameterGroup?.bindToCluster({}); const cluster = new CfnDBCluster(this, 'Resource', { // Basic @@ -424,7 +425,7 @@ export class DatabaseCluster extends DatabaseClusterBase { dbSubnetGroupName: subnetGroup.ref, vpcSecurityGroupIds: securityGroups.map(sg => sg.securityGroupId), port: props.port ?? clusterEngineBindConfig.port, - dbClusterParameterGroupName: clusterParameterGroup && clusterParameterGroup.parameterGroupName, + dbClusterParameterGroupName: clusterParameterGroupConfig?.parameterGroupName, associatedRoles: clusterAssociatedRoles.length > 0 ? clusterAssociatedRoles : undefined, // Admin masterUsername: secret ? secret.secretValueFromJson('username').toString() : props.masterUser.username, @@ -483,6 +484,7 @@ export class DatabaseCluster extends DatabaseClusterBase { } const instanceType = props.instanceProps.instanceType ?? ec2.InstanceType.of(ec2.InstanceClass.T3, ec2.InstanceSize.MEDIUM); + const instanceParameterGroupConfig = props.instanceProps.parameterGroup?.bindToInstance({}); for (let i = 0; i < instanceCount; i++) { const instanceIndex = i + 1; @@ -503,7 +505,7 @@ export class DatabaseCluster extends DatabaseClusterBase { publiclyAccessible, // This is already set on the Cluster. Unclear to me whether it should be repeated or not. Better yes. dbSubnetGroupName: subnetGroup.ref, - dbParameterGroupName: props.instanceProps.parameterGroup && props.instanceProps.parameterGroup.parameterGroupName, + dbParameterGroupName: instanceParameterGroupConfig?.parameterGroupName, monitoringInterval: props.monitoringInterval && props.monitoringInterval.toSeconds(), monitoringRoleArn: monitoringRole && monitoringRole.roleArn, }); diff --git a/packages/@aws-cdk/aws-rds/lib/engine.ts b/packages/@aws-cdk/aws-rds/lib/engine.ts new file mode 100644 index 0000000000000..4d6a179be8740 --- /dev/null +++ b/packages/@aws-cdk/aws-rds/lib/engine.ts @@ -0,0 +1,26 @@ +/** + * A common interface for database engines. + * Don't implement this interface directly, + * instead implement one of the known sub-interfaces, + * like IClusterEngine and IInstanceEngine. + */ +export interface IEngine { + /** The type of the engine, for example "mysql". */ + readonly engineType: string; + + /** + * The exact version of the engine that is used, + * for example "5.1.42". + * + * @default - use the default version for this engine type + */ + readonly engineVersion?: string; + + /** + * The family to use for ParameterGroups using this engine. + * This is usually equal to "", + * but can sometimes be a variation of that. + * You can pass this property when creating new ParameterGroup. + */ + readonly parameterGroupFamily: string; +} diff --git a/packages/@aws-cdk/aws-rds/lib/index.ts b/packages/@aws-cdk/aws-rds/lib/index.ts index d5459a3528c1d..b2da8e110654c 100644 --- a/packages/@aws-cdk/aws-rds/lib/index.ts +++ b/packages/@aws-cdk/aws-rds/lib/index.ts @@ -1,3 +1,4 @@ +export * from './engine'; export * from './cluster'; export * from './cluster-ref'; export * from './cluster-engine'; diff --git a/packages/@aws-cdk/aws-rds/lib/instance-engine.ts b/packages/@aws-cdk/aws-rds/lib/instance-engine.ts index 6cd17df3e52ba..ecaf86d1c9055 100644 --- a/packages/@aws-cdk/aws-rds/lib/instance-engine.ts +++ b/packages/@aws-cdk/aws-rds/lib/instance-engine.ts @@ -1,7 +1,8 @@ import * as ec2 from '@aws-cdk/aws-ec2'; import * as secretsmanager from '@aws-cdk/aws-secretsmanager'; import * as core from '@aws-cdk/core'; -import { ParameterGroupFamilyMapping } from './private/parameter-group-family-mapping'; +import { IEngine } from './engine'; +import { calculateParameterGroupFamily, ParameterGroupFamilyMapping } from './private/parameter-group-family-mapping'; /** * The options passed to {@link IInstanceEngine.bind}. @@ -26,18 +27,7 @@ export interface InstanceEngineConfig { /** * Interface representing a database instance (as opposed to cluster) engine. */ -export interface IInstanceEngine { - /** The type of the engine, for example "mysql". */ - readonly engineType: string; - - /** - * The exact version of the engine that is used, - * for example "5.1.42". - * - * @default - use the default version for this engine type - */ - readonly engineVersion?: string; - +export interface IInstanceEngine extends IEngine { /** The application used by this engine to perform rotation for a single-user scenario. */ readonly singleUserRotationApplication: secretsmanager.SecretRotationApplication; @@ -61,6 +51,7 @@ interface InstanceEngineBaseProps { abstract class InstanceEngineBase implements IInstanceEngine { public readonly engineType: string; public readonly engineVersion?: string; + public readonly parameterGroupFamily: string; public readonly singleUserRotationApplication: secretsmanager.SecretRotationApplication; public readonly multiUserRotationApplication: secretsmanager.SecretRotationApplication; @@ -69,6 +60,11 @@ abstract class InstanceEngineBase implements IInstanceEngine { this.singleUserRotationApplication = props.singleUserRotationApplication; this.multiUserRotationApplication = props.multiUserRotationApplication; this.engineVersion = props.version; + const parameterGroupFamily = calculateParameterGroupFamily(props.parameterGroupFamilies, props.version); + if (parameterGroupFamily === undefined) { + throw new Error(`No parameter group family found for database engine ${this.engineType} with version ${this.engineVersion}.`); + } + this.parameterGroupFamily = parameterGroupFamily; } public bindToInstance(_scope: core.Construct, options: InstanceEngineBindOptions): InstanceEngineConfig { @@ -231,6 +227,7 @@ class OracleSe2InstanceEngine extends OracleInstanceEngine { super({ engineType: 'oracle-se2', parameterGroupFamilies: [ + { engineMajorVersion: '11.2', parameterGroupFamily: 'oracle-se2-11.2' }, { engineMajorVersion: '12.1', parameterGroupFamily: 'oracle-se2-12.1' }, { engineMajorVersion: '12.2', parameterGroupFamily: 'oracle-se2-12.2' }, { engineMajorVersion: '18', parameterGroupFamily: 'oracle-se2-18' }, diff --git a/packages/@aws-cdk/aws-rds/lib/instance.ts b/packages/@aws-cdk/aws-rds/lib/instance.ts index 80572edced9a8..c43c2ab395a0d 100644 --- a/packages/@aws-cdk/aws-rds/lib/instance.ts +++ b/packages/@aws-cdk/aws-rds/lib/instance.ts @@ -678,12 +678,13 @@ abstract class DatabaseInstanceSource extends DatabaseInstanceNew implements IDa props.engine.bindToInstance(this, props); this.instanceType = props.instanceType ?? ec2.InstanceType.of(ec2.InstanceClass.M5, ec2.InstanceSize.LARGE); + const instanceParameterGroupConfig = props.parameterGroup?.bindToInstance({}); this.sourceCfnProps = { ...this.newCfnProps, allocatedStorage: props.allocatedStorage ? props.allocatedStorage.toString() : '100', allowMajorVersionUpgrade: props.allowMajorVersionUpgrade, dbName: props.databaseName, - dbParameterGroupName: props.parameterGroup && props.parameterGroup.parameterGroupName, + dbParameterGroupName: instanceParameterGroupConfig?.parameterGroupName, engine: props.engine.engineType, engineVersion: props.engine.engineVersion, licenseModel: props.licenseModel, diff --git a/packages/@aws-cdk/aws-rds/lib/parameter-group.ts b/packages/@aws-cdk/aws-rds/lib/parameter-group.ts index be42fc1cbff13..0c5ead62727c3 100644 --- a/packages/@aws-cdk/aws-rds/lib/parameter-group.ts +++ b/packages/@aws-cdk/aws-rds/lib/parameter-group.ts @@ -1,71 +1,63 @@ import { Construct, IResource, Lazy, Resource } from '@aws-cdk/core'; +import { IEngine } from './engine'; import { CfnDBClusterParameterGroup, CfnDBParameterGroup } from './rds.generated'; /** - * A parameter group + * Options for {@link IParameterGroup.bindToCluster}. + * Empty for now, but can be extended later. */ -export interface IParameterGroup extends IResource { - /** - * The name of this parameter group - * - * @attribute - */ - readonly parameterGroupName: string; +export interface ParameterGroupClusterBindOptions { +} - /** - * Adds a parameter to this group. - * If this is an imported parameter group, - * this method does nothing. - * - * @returns true if the parameter was actually added - * (i.e., this ParameterGroup is not imported), - * false otherwise - */ - addParameter(key: string, value: string): boolean; +/** + * The type returned from {@link IParameterGroup.bindToCluster}. + */ +export interface ParameterGroupClusterConfig { + /** The name of this parameter group. */ + readonly parameterGroupName: string; } /** - * A new cluster or instance parameter group + * Options for {@link IParameterGroup.bindToInstance}. + * Empty for now, but can be extended later. */ -abstract class ParameterGroupBase extends Resource implements IParameterGroup { - /** - * Imports a parameter group - */ - public static fromParameterGroupName(scope: Construct, id: string, parameterGroupName: string): IParameterGroup { - class Import extends Resource implements IParameterGroup { - public readonly parameterGroupName = parameterGroupName; +export interface ParameterGroupInstanceBindOptions { +} - public addParameter(): boolean { return false; } - } - return new Import(scope, id); - } +/** + * The type returned from {@link IParameterGroup.bindToInstance}. + */ +export interface ParameterGroupInstanceConfig { + /** The name of this parameter group. */ + readonly parameterGroupName: string; +} +/** + * A parameter group. + * Represents both a cluster parameter group, + * and an instance parameter group. + */ +export interface IParameterGroup extends IResource { /** - * The name of the parameter group + * Method called when this Parameter Group is used when defining a database cluster. */ - public abstract readonly parameterGroupName: string; + bindToCluster(options: ParameterGroupClusterBindOptions): ParameterGroupClusterConfig; /** - * Parameters of the parameter group + * Method called when this Parameter Group is used when defining a database instance. */ - protected readonly parameters: { [key: string]: string }; - - constructor(scope: Construct, id: string, parameters: { [key: string]: string } | undefined) { - super(scope, id); - - this.parameters = parameters ?? {}; - } + bindToInstance(options: ParameterGroupInstanceBindOptions): ParameterGroupInstanceConfig; /** - * Add a parameter to this parameter group + * Adds a parameter to this group. + * If this is an imported parameter group, + * this method does nothing. * - * @param key The key of the parameter to be added - * @param value The value of the parameter to be added + * @returns true if the parameter was actually added + * (i.e., this ParameterGroup is not imported), + * false otherwise */ - public addParameter(key: string, value: string): boolean { - this.parameters[key] = value; - return true; - } + addParameter(key: string, value: string): boolean; } /** @@ -73,9 +65,9 @@ abstract class ParameterGroupBase extends Resource implements IParameterGroup { */ export interface ParameterGroupProps { /** - * Database family of this parameter group + * The database engine for this parameter group. */ - readonly family: string; + readonly engine: IEngine; /** * Description for this parameter group @@ -93,55 +85,85 @@ export interface ParameterGroupProps { } /** - * A parameter group + * A parameter group. + * Represents both a cluster parameter group, + * and an instance parameter group. * * @resource AWS::RDS::DBParameterGroup */ -export class ParameterGroup extends ParameterGroupBase { +export class ParameterGroup extends Resource implements IParameterGroup { /** - * The name of the parameter group + * Imports a parameter group */ - public readonly parameterGroupName: string; + public static fromParameterGroupName(scope: Construct, id: string, parameterGroupName: string): IParameterGroup { + class Import extends Resource implements IParameterGroup { + public bindToCluster(_options: ParameterGroupClusterBindOptions): ParameterGroupClusterConfig { + return { parameterGroupName }; + } - constructor(scope: Construct, id: string, props: ParameterGroupProps) { - super(scope, id, props.parameters); + public bindToInstance(_options: ParameterGroupInstanceBindOptions): ParameterGroupInstanceConfig { + return { parameterGroupName }; + } - const resource = new CfnDBParameterGroup(this, 'Resource', { - description: props.description || `Parameter group for ${props.family}`, - family: props.family, - parameters: Lazy.anyValue({ produce: () => this.parameters }), - }); + public addParameter(_key: string, _value: string): boolean { + return false; + } + } - this.parameterGroupName = resource.ref; + return new Import(scope, id); } -} -/** - * Construction properties for a ClusterParameterGroup - */ -export interface ClusterParameterGroupProps extends ParameterGroupProps { + private readonly parameters: { [key: string]: string }; + private readonly family: string; + private readonly description?: string; -} -/** - * A cluster parameter group - * - * @resource AWS::RDS::DBClusterParameterGroup - */ -export class ClusterParameterGroup extends ParameterGroupBase { - /** - * The name of the parameter group - */ - public readonly parameterGroupName: string; + private clusterCfnGroup?: CfnDBClusterParameterGroup; + private instanceCfnGroup?: CfnDBParameterGroup; - constructor(scope: Construct, id: string, props: ClusterParameterGroupProps) { - super(scope, id, props.parameters); + constructor(scope: Construct, id: string, props: ParameterGroupProps) { + super(scope, id); - const resource = new CfnDBClusterParameterGroup(this, 'Resource', { - description: props.description || `Cluster parameter group for ${props.family}`, - family: props.family, - parameters: Lazy.anyValue({ produce: () => this.parameters }), - }); + this.family = props.engine.parameterGroupFamily; + this.description = props.description; + this.parameters = props.parameters ?? {}; + } - this.parameterGroupName = resource.ref; + public bindToCluster(_options: ParameterGroupClusterBindOptions): ParameterGroupClusterConfig { + if (!this.clusterCfnGroup) { + const id = this.instanceCfnGroup ? 'ClusterParameterGroup' : 'Resource'; + this.clusterCfnGroup = new CfnDBClusterParameterGroup(this, id, { + description: this.description || `Cluster parameter group for ${this.family}`, + family: this.family, + parameters: Lazy.anyValue({ produce: () => this.parameters }), + }); + } + return { + parameterGroupName: this.clusterCfnGroup.ref, + }; + } + + public bindToInstance(_options: ParameterGroupInstanceBindOptions): ParameterGroupInstanceConfig { + if (!this.instanceCfnGroup) { + const id = this.clusterCfnGroup ? 'InstanceParameterGroup' : 'Resource'; + this.instanceCfnGroup = new CfnDBParameterGroup(this, id, { + description: this.description || `Parameter group for ${this.family}`, + family: this.family, + parameters: Lazy.anyValue({ produce: () => this.parameters }), + }); + } + return { + parameterGroupName: this.instanceCfnGroup.ref, + }; + } + + /** + * Add a parameter to this parameter group + * + * @param key The key of the parameter to be added + * @param value The value of the parameter to be added + */ + public addParameter(key: string, value: string): boolean { + this.parameters[key] = value; + return true; } } diff --git a/packages/@aws-cdk/aws-rds/lib/private/parameter-group-family-mapping.ts b/packages/@aws-cdk/aws-rds/lib/private/parameter-group-family-mapping.ts index ec1eb19a27547..98e83f9536206 100644 --- a/packages/@aws-cdk/aws-rds/lib/private/parameter-group-family-mapping.ts +++ b/packages/@aws-cdk/aws-rds/lib/private/parameter-group-family-mapping.ts @@ -1,3 +1,5 @@ +import { compare } from './version'; + /** * Engine major version and parameter group family pairs. */ @@ -12,3 +14,28 @@ export interface ParameterGroupFamilyMapping { */ readonly parameterGroupFamily: string } + +/** + * Get the latest parameter group family for this engine. Latest is determined using semver on the engine major version. + * When `engineVersion` is specified, return the parameter group family corresponding to that engine version. + * Return undefined if no parameter group family is defined for this engine or for the requested `engineVersion`. + */ +export function calculateParameterGroupFamily( + parameterGroupFamilies: ParameterGroupFamilyMapping[] | undefined, + engineVersion: string | undefined): string | undefined { + if (parameterGroupFamilies === undefined) { + return undefined; + } + if (engineVersion !== undefined) { + const family = parameterGroupFamilies.find(x => engineVersion.startsWith(x.engineMajorVersion)); + if (family) { + return family.parameterGroupFamily; + } + } else if (parameterGroupFamilies.length > 0) { + const sorted = parameterGroupFamilies.slice().sort((a, b) => { + return compare(a.engineMajorVersion, b.engineMajorVersion); + }).reverse(); + return sorted[0].parameterGroupFamily; + } + return undefined; +} diff --git a/packages/@aws-cdk/aws-rds/package.json b/packages/@aws-cdk/aws-rds/package.json index 1568b3346b09b..dd3795b7497cc 100644 --- a/packages/@aws-cdk/aws-rds/package.json +++ b/packages/@aws-cdk/aws-rds/package.json @@ -104,7 +104,6 @@ "awslint": { "exclude": [ "props-physical-name:@aws-cdk/aws-rds.ParameterGroupProps", - "props-physical-name:@aws-cdk/aws-rds.ClusterParameterGroupProps", "props-physical-name:@aws-cdk/aws-rds.DatabaseClusterProps", "props-physical-name:@aws-cdk/aws-rds.DatabaseInstanceProps", "props-physical-name:@aws-cdk/aws-rds.DatabaseInstanceFromSnapshotProps", diff --git a/packages/@aws-cdk/aws-rds/test/integ.cluster.ts b/packages/@aws-cdk/aws-rds/test/integ.cluster.ts index 590a32fd7afb1..a5bdaba4883c9 100644 --- a/packages/@aws-cdk/aws-rds/test/integ.cluster.ts +++ b/packages/@aws-cdk/aws-rds/test/integ.cluster.ts @@ -1,16 +1,15 @@ import * as ec2 from '@aws-cdk/aws-ec2'; import * as kms from '@aws-cdk/aws-kms'; import * as cdk from '@aws-cdk/core'; -import { DatabaseCluster, DatabaseClusterEngine } from '../lib'; -import { ClusterParameterGroup } from '../lib/parameter-group'; +import { DatabaseCluster, DatabaseClusterEngine, ParameterGroup } from '../lib'; const app = new cdk.App(); const stack = new cdk.Stack(app, 'aws-cdk-rds-integ'); const vpc = new ec2.Vpc(stack, 'VPC', { maxAzs: 2 }); -const params = new ClusterParameterGroup(stack, 'Params', { - family: 'aurora5.6', +const params = new ParameterGroup(stack, 'Params', { + engine: DatabaseClusterEngine.AURORA, description: 'A nice parameter group', parameters: { character_set_database: 'utf8mb4', diff --git a/packages/@aws-cdk/aws-rds/test/integ.instance.lit.ts b/packages/@aws-cdk/aws-rds/test/integ.instance.lit.ts index e7ec585de679b..405d1d1be69cb 100644 --- a/packages/@aws-cdk/aws-rds/test/integ.instance.lit.ts +++ b/packages/@aws-cdk/aws-rds/test/integ.instance.lit.ts @@ -18,7 +18,9 @@ class DatabaseInstanceStack extends cdk.Stack { /// !show // Set open cursors with parameter group const parameterGroup = new rds.ParameterGroup(this, 'ParameterGroup', { - family: 'oracle-se1-11.2', + engine: rds.DatabaseInstanceEngine.oracleSe1({ + version: '11.2', + }), parameters: { open_cursors: '2500', }, diff --git a/packages/@aws-cdk/aws-rds/test/test.cluster.ts b/packages/@aws-cdk/aws-rds/test/test.cluster.ts index 2b4bc45259b91..e9b5015e0ba0f 100644 --- a/packages/@aws-cdk/aws-rds/test/test.cluster.ts +++ b/packages/@aws-cdk/aws-rds/test/test.cluster.ts @@ -5,7 +5,7 @@ import * as kms from '@aws-cdk/aws-kms'; import * as s3 from '@aws-cdk/aws-s3'; import * as cdk from '@aws-cdk/core'; import { Test } from 'nodeunit'; -import { ClusterParameterGroup, DatabaseCluster, DatabaseClusterEngine, ParameterGroup } from '../lib'; +import { DatabaseCluster, DatabaseClusterEngine, ParameterGroup } from '../lib'; export = { 'creating a Cluster also creates 2 DB Instances'(test: Test) { @@ -120,8 +120,8 @@ export = { const vpc = new ec2.Vpc(stack, 'VPC'); // WHEN - const group = new ClusterParameterGroup(stack, 'Params', { - family: 'hello', + const group = new ParameterGroup(stack, 'Params', { + engine: DatabaseClusterEngine.AURORA, description: 'bye', parameters: { param: 'value', @@ -263,7 +263,7 @@ export = { const stack = testStack(); const vpc = new ec2.Vpc(stack, 'VPC'); const parameterGroup = new ParameterGroup(stack, 'ParameterGroup', { - family: 'hello', + engine: DatabaseClusterEngine.AURORA, parameters: { key: 'value', }, @@ -888,8 +888,8 @@ export = { const stack = testStack(); const vpc = new ec2.Vpc(stack, 'VPC'); - const parameterGroup = new ClusterParameterGroup(stack, 'ParameterGroup', { - family: 'family', + const parameterGroup = new ParameterGroup(stack, 'ParameterGroup', { + engine: DatabaseClusterEngine.AURORA, parameters: { key: 'value', }, @@ -935,7 +935,7 @@ export = { })); expect(stack).to(haveResource('AWS::RDS::DBClusterParameterGroup', { - Family: 'family', + Family: 'aurora5.6', Parameters: { key: 'value', aurora_load_from_s3_role: { diff --git a/packages/@aws-cdk/aws-rds/test/test.instance.ts b/packages/@aws-cdk/aws-rds/test/test.instance.ts index d83d36d43d9fc..40b5caa91dca8 100644 --- a/packages/@aws-cdk/aws-rds/test/test.instance.ts +++ b/packages/@aws-cdk/aws-rds/test/test.instance.ts @@ -206,7 +206,7 @@ export = { }); const parameterGroup = new rds.ParameterGroup(stack, 'ParameterGroup', { - family: 'hello', + engine: rds.DatabaseInstanceEngine.SQL_SERVER_EE, description: 'desc', parameters: { key: 'value', diff --git a/packages/@aws-cdk/aws-rds/test/test.parameter-group.ts b/packages/@aws-cdk/aws-rds/test/test.parameter-group.ts index 4de1a34310768..f4a21db1fdec0 100644 --- a/packages/@aws-cdk/aws-rds/test/test.parameter-group.ts +++ b/packages/@aws-cdk/aws-rds/test/test.parameter-group.ts @@ -1,26 +1,47 @@ -import { expect, haveResource } from '@aws-cdk/assert'; +import { countResources, expect, haveResource } from '@aws-cdk/assert'; import * as cdk from '@aws-cdk/core'; import { Test } from 'nodeunit'; -import { ClusterParameterGroup, ParameterGroup } from '../lib'; +import { DatabaseClusterEngine, ParameterGroup } from '../lib'; export = { - 'create a parameter group'(test: Test) { + "does not create a parameter group if it wasn't bound to a cluster or instance"(test: Test) { // GIVEN const stack = new cdk.Stack(); // WHEN new ParameterGroup(stack, 'Params', { - family: 'hello', + engine: DatabaseClusterEngine.AURORA, description: 'desc', parameters: { key: 'value', }, }); + // THEN + expect(stack).to(countResources('AWS::RDS::DBParameterGroup', 0)); + expect(stack).to(countResources('AWS::RDS::DBClusterParameterGroup', 0)); + + test.done(); + }, + + 'create a parameter group when bound to an instance'(test: Test) { + // GIVEN + const stack = new cdk.Stack(); + + // WHEN + const parameterGroup = new ParameterGroup(stack, 'Params', { + engine: DatabaseClusterEngine.AURORA, + description: 'desc', + parameters: { + key: 'value', + }, + }); + parameterGroup.bindToInstance({}); + // THEN expect(stack).to(haveResource('AWS::RDS::DBParameterGroup', { Description: 'desc', - Family: 'hello', + Family: 'aurora5.6', Parameters: { key: 'value', }, @@ -29,23 +50,24 @@ export = { test.done(); }, - 'create a cluster parameter group'(test: Test) { + 'create a parameter group when bound to a cluster'(test: Test) { // GIVEN const stack = new cdk.Stack(); // WHEN - new ClusterParameterGroup(stack, 'Params', { - family: 'hello', + const parameterGroup = new ParameterGroup(stack, 'Params', { + engine: DatabaseClusterEngine.AURORA, description: 'desc', parameters: { key: 'value', }, }); + parameterGroup.bindToCluster({}); // THEN expect(stack).to(haveResource('AWS::RDS::DBClusterParameterGroup', { Description: 'desc', - Family: 'hello', + Family: 'aurora5.6', Parameters: { key: 'value', }, @@ -54,25 +76,48 @@ export = { test.done(); }, + 'creates 2 parameter groups when bound to a cluster and an instance'(test: Test) { + // GIVEN + const stack = new cdk.Stack(); + + // WHEN + const parameterGroup = new ParameterGroup(stack, 'Params', { + engine: DatabaseClusterEngine.AURORA, + description: 'desc', + parameters: { + key: 'value', + }, + }); + parameterGroup.bindToCluster({}); + parameterGroup.bindToInstance({}); + + // THEN + expect(stack).to(countResources('AWS::RDS::DBParameterGroup', 1)); + expect(stack).to(countResources('AWS::RDS::DBClusterParameterGroup', 1)); + + test.done(); + }, + 'Add an additional parameter to an existing parameter group'(test: Test) { // GIVEN const stack = new cdk.Stack(); // WHEN - const clusterParameterGroup = new ClusterParameterGroup(stack, 'Params', { - family: 'hello', + const clusterParameterGroup = new ParameterGroup(stack, 'Params', { + engine: DatabaseClusterEngine.AURORA, description: 'desc', parameters: { key1: 'value1', }, }); + clusterParameterGroup.bindToCluster({}); clusterParameterGroup.addParameter('key2', 'value2'); // THEN expect(stack).to(haveResource('AWS::RDS::DBClusterParameterGroup', { Description: 'desc', - Family: 'hello', + Family: 'aurora5.6', Parameters: { key1: 'value1', key2: 'value2',