From e108ca7d7ec2306176d508bbce2091990b2de32a Mon Sep 17 00:00:00 2001 From: Hyeonsoo David Lee Date: Sat, 4 Jul 2020 10:22:00 +0900 Subject: [PATCH 1/3] fix(rds): Explicitly set 'default' as TargetGroupName of DatabaseProxy fixes #8885 --- packages/@aws-cdk/aws-rds/lib/proxy.ts | 6 +++++- packages/@aws-cdk/aws-rds/test/integ.proxy.expected.json | 3 ++- packages/@aws-cdk/aws-rds/test/test.proxy.ts | 2 ++ 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/@aws-cdk/aws-rds/lib/proxy.ts b/packages/@aws-cdk/aws-rds/lib/proxy.ts index 4e8112ed9ffbc..719c2bc6fa7bf 100644 --- a/packages/@aws-cdk/aws-rds/lib/proxy.ts +++ b/packages/@aws-cdk/aws-rds/lib/proxy.ts @@ -416,12 +416,16 @@ export class DatabaseProxy extends cdk.Resource dbInstanceIdentifiers = [ bindResult.dbInstances[0].instanceIdentifier ]; } - new CfnDBProxyTargetGroup(this, 'ProxyTargetGroup', { + const proxyTargetGroup = new CfnDBProxyTargetGroup(this, 'ProxyTargetGroup', { dbProxyName: this.dbProxyName, dbInstanceIdentifiers, dbClusterIdentifiers: bindResult.dbClusters?.map((c) => c.clusterIdentifier), connectionPoolConfigurationInfo: toConnectionPoolConfigurationInfo(props), }); + + // Currently(2020-07-04), this property must be set to default. + // https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-rds-dbproxytargetgroup.html#TargetGroupName-fn::getatt + proxyTargetGroup.addOverride('Properties.TargetGroupName', 'default'); } /** diff --git a/packages/@aws-cdk/aws-rds/test/integ.proxy.expected.json b/packages/@aws-cdk/aws-rds/test/integ.proxy.expected.json index d02477bf1a86a..10e7b2afb02f8 100644 --- a/packages/@aws-cdk/aws-rds/test/integ.proxy.expected.json +++ b/packages/@aws-cdk/aws-rds/test/integ.proxy.expected.json @@ -554,7 +554,8 @@ { "Ref": "dbInstance4076B1EC" } - ] + ], + "TargetGroupName": "default" } } } diff --git a/packages/@aws-cdk/aws-rds/test/test.proxy.ts b/packages/@aws-cdk/aws-rds/test/test.proxy.ts index f0b6bfbf0c91f..7baef51001985 100644 --- a/packages/@aws-cdk/aws-rds/test/test.proxy.ts +++ b/packages/@aws-cdk/aws-rds/test/test.proxy.ts @@ -67,6 +67,7 @@ export = { Ref: 'InstanceC1063A87', }, ], + TargetGroupName: 'default', }, }, ResourcePart.CompleteDefinition)); @@ -148,6 +149,7 @@ export = { Ref: 'DatabaseInstance2AA380DEE', }, ], + TargetGroupName: 'default', }, }, ResourcePart.CompleteDefinition)); From 84d0a4a05d80916551cab880e25fa2feea0d04f0 Mon Sep 17 00:00:00 2001 From: Hyeonsoo David Lee Date: Sun, 5 Jul 2020 23:47:20 +0900 Subject: [PATCH 2/3] fix(rds): Add validation for CfnDBProxyTargetGroup props --- packages/@aws-cdk/aws-rds/lib/proxy.ts | 16 +++++++++++----- packages/@aws-cdk/aws-rds/test/test.proxy.ts | 8 -------- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/packages/@aws-cdk/aws-rds/lib/proxy.ts b/packages/@aws-cdk/aws-rds/lib/proxy.ts index 719c2bc6fa7bf..9733056d8ed3b 100644 --- a/packages/@aws-cdk/aws-rds/lib/proxy.ts +++ b/packages/@aws-cdk/aws-rds/lib/proxy.ts @@ -408,18 +408,24 @@ export class DatabaseProxy extends cdk.Resource this.endpoint = this.resource.attrEndpoint; let dbInstanceIdentifiers: string[] | undefined; - if (bindResult.dbClusters) { - // support for only instances of a single cluster - dbInstanceIdentifiers = bindResult.dbClusters[0].instanceIdentifiers; - } else if (bindResult.dbInstances) { + if (bindResult.dbInstances) { // support for only single instance dbInstanceIdentifiers = [ bindResult.dbInstances[0].instanceIdentifier ]; } + let dbClusterIdentifiers: string[] | undefined; + if (bindResult.dbClusters) { + dbClusterIdentifiers = bindResult.dbClusters.map((c) => c.clusterIdentifier); + } + + if (!!dbInstanceIdentifiers && !!dbClusterIdentifiers) { + throw new Error('Cannot specify both dbInstanceIdentifiers and dbClusterIdentifiers'); + } + const proxyTargetGroup = new CfnDBProxyTargetGroup(this, 'ProxyTargetGroup', { dbProxyName: this.dbProxyName, dbInstanceIdentifiers, - dbClusterIdentifiers: bindResult.dbClusters?.map((c) => c.clusterIdentifier), + dbClusterIdentifiers, connectionPoolConfigurationInfo: toConnectionPoolConfigurationInfo(props), }); diff --git a/packages/@aws-cdk/aws-rds/test/test.proxy.ts b/packages/@aws-cdk/aws-rds/test/test.proxy.ts index 7baef51001985..ab89e8b534b45 100644 --- a/packages/@aws-cdk/aws-rds/test/test.proxy.ts +++ b/packages/@aws-cdk/aws-rds/test/test.proxy.ts @@ -141,14 +141,6 @@ export = { Ref: 'DatabaseB269D8BB', }, ], - DBInstanceIdentifiers: [ - { - Ref: 'DatabaseInstance1844F58FD', - }, - { - Ref: 'DatabaseInstance2AA380DEE', - }, - ], TargetGroupName: 'default', }, }, ResourcePart.CompleteDefinition)); From 06dc31f275a816e747f00cd760fbd452c72735c6 Mon Sep 17 00:00:00 2001 From: Hyeonsoo Lee Date: Mon, 6 Jul 2020 19:28:05 +0900 Subject: [PATCH 3/3] test(rds): add an assertion to DatabaseProxy --- packages/@aws-cdk/aws-rds/test/test.proxy.ts | 34 +++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/packages/@aws-cdk/aws-rds/test/test.proxy.ts b/packages/@aws-cdk/aws-rds/test/test.proxy.ts index ab89e8b534b45..2ec6d93f6eb0a 100644 --- a/packages/@aws-cdk/aws-rds/test/test.proxy.ts +++ b/packages/@aws-cdk/aws-rds/test/test.proxy.ts @@ -1,4 +1,4 @@ -import { expect, haveResource, ResourcePart } from '@aws-cdk/assert'; +import { ABSENT, expect, haveResource, ResourcePart } from '@aws-cdk/assert'; import * as ec2 from '@aws-cdk/aws-ec2'; import * as cdk from '@aws-cdk/core'; import { Test } from 'nodeunit'; @@ -147,4 +147,36 @@ export = { test.done(); }, + + 'Cannot specify both dbInstanceIdentifiers and dbClusterIdentifiers'(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.AURORA_POSTGRESQL, + engineVersion: '10.7', + masterUser: { + username: 'admin', + }, + instanceProps: { + instanceType: ec2.InstanceType.of(ec2.InstanceClass.BURSTABLE2, ec2.InstanceSize.SMALL), + vpc, + }, + }); + + // WHEN + test.doesNotThrow(() => { + new rds.DatabaseProxy(stack, 'Proxy', { + proxyTarget: rds.ProxyTarget.fromCluster(cluster), + secret: cluster.secret!, + vpc, + }); + }, /Cannot specify both dbInstanceIdentifiers and dbClusterIdentifiers/); + + expect(stack).to(haveResource('AWS::RDS::DBProxyTargetGroup', { + DBInstanceIdentifiers: ABSENT, + }, ResourcePart.Properties)); + + test.done(); + }, };