Skip to content

Commit

Permalink
fix(rds): instance identifiers and endpoints of a Cluster are blank (#…
Browse files Browse the repository at this point in the history
…14394)

Previously instanceIdentifiers and instanceEndpoints were readonly properties of DatabaseClusterNew, defaulting to empty arrays. However, they they were never set to the correct values after instances were added to the cluster. Those properties are now moved on to the DatabaseCluster and DatabaseClusterFromSnapshot classes where they can now actually be set to correct values.

fixes #14377


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
vilikin authored Apr 29, 2021
1 parent 0e59708 commit 9597d97
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 6 deletions.
14 changes: 10 additions & 4 deletions packages/@aws-cdk/aws-rds/lib/cluster.ts
Original file line number Diff line number Diff line change
Expand Up @@ -298,8 +298,6 @@ abstract class DatabaseClusterNew extends DatabaseClusterBase {
* Never undefined.
*/
public readonly engine?: IClusterEngine;
public readonly instanceIdentifiers: string[] = [];
public readonly instanceEndpoints: Endpoint[] = [];

protected readonly newCfnProps: CfnDBClusterProps;
protected readonly securityGroups: ec2.ISecurityGroup[];
Expand Down Expand Up @@ -481,6 +479,8 @@ export class DatabaseCluster extends DatabaseClusterNew {
public readonly clusterEndpoint: Endpoint;
public readonly clusterReadEndpoint: Endpoint;
public readonly connections: ec2.Connections;
public readonly instanceIdentifiers: string[];
public readonly instanceEndpoints: Endpoint[];

/**
* The secret attached to this cluster
Expand Down Expand Up @@ -533,7 +533,9 @@ export class DatabaseCluster extends DatabaseClusterNew {
}

setLogRetention(this, props);
createInstances(this, props, this.subnetGroup);
const createdInstances = createInstances(this, props, this.subnetGroup);
this.instanceIdentifiers = createdInstances.instanceIdentifiers;
this.instanceEndpoints = createdInstances.instanceEndpoints;
}

/**
Expand Down Expand Up @@ -602,6 +604,8 @@ export class DatabaseClusterFromSnapshot extends DatabaseClusterNew {
public readonly clusterEndpoint: Endpoint;
public readonly clusterReadEndpoint: Endpoint;
public readonly connections: ec2.Connections;
public readonly instanceIdentifiers: string[];
public readonly instanceEndpoints: Endpoint[];

constructor(scope: Construct, id: string, props: DatabaseClusterFromSnapshotProps) {
super(scope, id, props);
Expand All @@ -625,7 +629,9 @@ export class DatabaseClusterFromSnapshot extends DatabaseClusterNew {
cluster.applyRemovalPolicy(props.removalPolicy ?? RemovalPolicy.SNAPSHOT);

setLogRetention(this, props);
createInstances(this, props, this.subnetGroup);
const createdInstances = createInstances(this, props, this.subnetGroup);
this.instanceIdentifiers = createdInstances.instanceIdentifiers;
this.instanceEndpoints = createdInstances.instanceEndpoints;
}
}

Expand Down
48 changes: 46 additions & 2 deletions packages/@aws-cdk/aws-rds/test/cluster.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ describe('cluster', () => {
const vpc = new ec2.Vpc(stack, 'VPC');

// WHEN
new DatabaseCluster(stack, 'Database', {
const cluster = new DatabaseCluster(stack, 'Database', {
engine: DatabaseClusterEngine.AURORA,
instances: 1,
credentials: {
Expand All @@ -97,6 +97,28 @@ describe('cluster', () => {
MasterUserPassword: 'tooshort',
VpcSecurityGroupIds: [{ 'Fn::GetAtt': ['DatabaseSecurityGroup5C91FDCB', 'GroupId'] }],
});

expect(cluster.instanceIdentifiers).toHaveLength(1);
expect(stack.resolve(cluster.instanceIdentifiers[0])).toEqual({
Ref: 'DatabaseInstance1844F58FD',
});

expect(cluster.instanceEndpoints).toHaveLength(1);
expect(stack.resolve(cluster.instanceEndpoints[0])).toEqual({
hostname: {
'Fn::GetAtt': ['DatabaseInstance1844F58FD', 'Endpoint.Address'],
},
port: {
'Fn::GetAtt': ['DatabaseB269D8BB', 'Endpoint.Port'],
},
socketAddress: {
'Fn::Join': ['', [
{ 'Fn::GetAtt': ['DatabaseInstance1844F58FD', 'Endpoint.Address'] },
':',
{ 'Fn::GetAtt': ['DatabaseB269D8BB', 'Endpoint.Port'] },
]],
},
});
});

test('can create a cluster with imported vpc and security group', () => {
Expand Down Expand Up @@ -1635,7 +1657,7 @@ describe('cluster', () => {
const vpc = new ec2.Vpc(stack, 'VPC');

// WHEN
new DatabaseClusterFromSnapshot(stack, 'Database', {
const cluster = new DatabaseClusterFromSnapshot(stack, 'Database', {
engine: DatabaseClusterEngine.aurora({ version: AuroraEngineVersion.VER_1_22_2 }),
instanceProps: {
vpc,
Expand All @@ -1659,6 +1681,28 @@ describe('cluster', () => {
}, ResourcePart.CompleteDefinition);

expect(stack).toCountResources('AWS::RDS::DBInstance', 2);

expect(cluster.instanceIdentifiers).toHaveLength(2);
expect(stack.resolve(cluster.instanceIdentifiers[0])).toEqual({
Ref: 'DatabaseInstance1844F58FD',
});

expect(cluster.instanceEndpoints).toHaveLength(2);
expect(stack.resolve(cluster.instanceEndpoints[0])).toEqual({
hostname: {
'Fn::GetAtt': ['DatabaseInstance1844F58FD', 'Endpoint.Address'],
},
port: {
'Fn::GetAtt': ['DatabaseB269D8BB', 'Endpoint.Port'],
},
socketAddress: {
'Fn::Join': ['', [
{ 'Fn::GetAtt': ['DatabaseInstance1844F58FD', 'Endpoint.Address'] },
':',
{ 'Fn::GetAtt': ['DatabaseB269D8BB', 'Endpoint.Port'] },
]],
},
});
});

test('reuse an existing subnet group', () => {
Expand Down

0 comments on commit 9597d97

Please sign in to comment.