Skip to content
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): fail with a descriptive error if Cluster's instance count is a deploy-time value #13765

Merged
merged 5 commits into from
Mar 26, 2021
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions packages/@aws-cdk/aws-rds/lib/cluster.ts
Original file line number Diff line number Diff line change
Expand Up @@ -651,6 +651,9 @@ interface InstanceConfig {
*/
function createInstances(cluster: DatabaseClusterNew, props: DatabaseClusterBaseProps, subnetGroup: ISubnetGroup): InstanceConfig {
const instanceCount = props.instances != null ? props.instances : 2;
if (Token.isUnresolved(instanceCount)) {
throw new Error('The number of instances an RDS Cluster consists of cannot be provided as a deploy-time only value!');
}
if (instanceCount < 1) {
throw new Error('At least one instance is required');
}
Expand Down
51 changes: 15 additions & 36 deletions packages/@aws-cdk/aws-rds/test/cluster.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,23 @@ describe('cluster', () => {
DeletionPolicy: 'Delete',
UpdateReplacePolicy: 'Delete',
}, ResourcePart.CompleteDefinition);
});

test('validates that the number of instances is not a deploy-time value', () => {
const stack = testStack();
const vpc = new ec2.Vpc(stack, 'VPC');
const parameter = new cdk.CfnParameter(stack, 'Param');

expect(() => {
new DatabaseCluster(stack, 'Database', {
instances: parameter.valueAsNumber,
engine: DatabaseClusterEngine.AURORA,
instanceProps: {
instanceType: ec2.InstanceType.of(ec2.InstanceClass.BURSTABLE2, ec2.InstanceSize.SMALL),
vpc,
},
});
}).toThrow('The number of instances an RDS Cluster consists of cannot be provided as a deploy-time only value!');
});

test('can create a cluster with a single instance', () => {
Expand Down Expand Up @@ -81,8 +96,6 @@ describe('cluster', () => {
MasterUserPassword: 'tooshort',
VpcSecurityGroupIds: [{ 'Fn::GetAtt': ['DatabaseSecurityGroup5C91FDCB', 'GroupId'] }],
});


});

test('can create a cluster with imported vpc and security group', () => {
Expand Down Expand Up @@ -116,8 +129,6 @@ describe('cluster', () => {
MasterUserPassword: 'tooshort',
VpcSecurityGroupIds: ['SecurityGroupId12345'],
});


});

test('cluster with parameter group', () => {
Expand Down Expand Up @@ -150,8 +161,6 @@ describe('cluster', () => {
expect(stack).toHaveResource('AWS::RDS::DBCluster', {
DBClusterParameterGroupName: { Ref: 'ParamsA8366201' },
});


});

test("sets the retention policy of the SubnetGroup to 'Retain' if the Cluster is created with 'Retain'", () => {
Expand All @@ -172,8 +181,6 @@ describe('cluster', () => {
DeletionPolicy: 'Retain',
UpdateReplacePolicy: 'Retain',
}, ResourcePart.CompleteDefinition);


});

test('creates a secret when master credentials are not specified', () => {
Expand Down Expand Up @@ -230,8 +237,6 @@ describe('cluster', () => {
SecretStringTemplate: '{"username":"admin"}',
},
});


});

test('create an encrypted cluster with custom KMS key', () => {
Expand Down Expand Up @@ -261,8 +266,6 @@ describe('cluster', () => {
],
},
});


});

test('cluster with instance parameter group', () => {
Expand Down Expand Up @@ -294,8 +297,6 @@ describe('cluster', () => {
Ref: 'ParameterGroup5E32DECB',
},
});


});

describe('performance insights', () => {
Expand Down Expand Up @@ -323,8 +324,6 @@ describe('cluster', () => {
PerformanceInsightsRetentionPeriod: 731,
PerformanceInsightsKMSKeyId: { 'Fn::GetAtt': ['Key961B73FD', 'Arn'] },
});


});

test('setting performance insights fields enables performance insights', () => {
Expand All @@ -348,8 +347,6 @@ describe('cluster', () => {
EnablePerformanceInsights: true,
PerformanceInsightsRetentionPeriod: 731,
});


});

test('throws if performance insights fields are set but performance insights is disabled', () => {
Expand All @@ -370,8 +367,6 @@ describe('cluster', () => {
},
});
}).toThrow(/`enablePerformanceInsights` disabled, but `performanceInsightRetention` or `performanceInsightEncryptionKey` was set/);


});
});

Expand All @@ -392,8 +387,6 @@ describe('cluster', () => {
expect(stack).toHaveResource('AWS::RDS::DBInstance', {
AutoMinorVersionUpgrade: false,
});


});

test('cluster with allow upgrade of major version', () => {
Expand All @@ -413,8 +406,6 @@ describe('cluster', () => {
expect(stack).toHaveResourceLike('AWS::RDS::DBInstance', {
AllowMajorVersionUpgrade: true,
});


});

test('cluster with disallow remove backups', () => {
Expand All @@ -434,8 +425,6 @@ describe('cluster', () => {
expect(stack).toHaveResourceLike('AWS::RDS::DBInstance', {
DeleteAutomatedBackups: false,
});


});

test('create a cluster using a specific version of MySQL', () => {
Expand All @@ -462,8 +451,6 @@ describe('cluster', () => {
Engine: 'aurora-mysql',
EngineVersion: '5.7.mysql_aurora.2.04.4',
});


});

test('create a cluster using a specific version of Postgresql', () => {
Expand Down Expand Up @@ -513,8 +500,6 @@ describe('cluster', () => {

// THEN
expect(stack.resolve(cluster.clusterEndpoint)).not.toEqual(stack.resolve(cluster.clusterReadEndpoint));


});

test('imported cluster with imported security group honors allowAllOutbound', () => {
Expand All @@ -540,8 +525,6 @@ describe('cluster', () => {
expect(stack).toHaveResource('AWS::EC2::SecurityGroupEgress', {
GroupId: 'sg-123456789',
});


});

test('can import a cluster with minimal attributes', () => {
Expand All @@ -567,8 +550,6 @@ describe('cluster', () => {
expect(() => cluster.clusterReadEndpoint).toThrow(/Cannot access `clusterReadEndpoint` of an imported cluster/);
expect(() => cluster.instanceIdentifiers).toThrow(/Cannot access `instanceIdentifiers` of an imported cluster/);
expect(() => cluster.instanceEndpoints).toThrow(/Cannot access `instanceEndpoints` of an imported cluster/);


});

test('imported cluster can access properties if attributes are provided', () => {
Expand All @@ -590,8 +571,6 @@ describe('cluster', () => {
expect(cluster.clusterReadEndpoint.socketAddress).toEqual('reader-address:3306');
expect(cluster.instanceIdentifiers).toEqual(['identifier']);
expect(cluster.instanceEndpoints.map(endpoint => endpoint.socketAddress)).toEqual(['instance-addr:3306']);


});

test('cluster supports metrics', () => {
Expand Down