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
fix(rds): proxy for db cluster fails with model validation error #8896
Changes from all commits
e108ca7
84d0a4a
06dc31f
ef3deb5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -408,20 +408,30 @@ 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 ]; | ||
} | ||
|
||
new CfnDBProxyTargetGroup(this, 'ProxyTargetGroup', { | ||
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), | ||
}); | ||
|
||
// 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'); | ||
Comment on lines
+432
to
+434
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Overrides are set for CloudFormation properties that are not yet supported in the CDK. Looks like this is a CloudFormation return value. Why is this being set? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There seems to be a difference between the CloudFormation resource document and what CloudFormation actually does. Currently CloudFormation requires There is no normal way to set The behavior seems to be changed around DB Proxy's GA. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the explanation. |
||
} | ||
|
||
/** | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -554,7 +554,8 @@ | |
{ | ||
"Ref": "dbInstance4076B1EC" | ||
} | ||
] | ||
], | ||
"TargetGroupName": "default" | ||
} | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a unit test for the case that was previously wrong and is fixed in this PR.