Skip to content

Commit

Permalink
Merge branch 'master' into chore/gitpod-prereqs-cjeck-off
Browse files Browse the repository at this point in the history
  • Loading branch information
mergify[bot] authored Jul 16, 2020
2 parents c0d7449 + 8d470b2 commit 216c19c
Show file tree
Hide file tree
Showing 21 changed files with 693 additions and 241 deletions.
4 changes: 0 additions & 4 deletions packages/@aws-cdk/aws-ec2/lib/volume.ts
Original file line number Diff line number Diff line change
Expand Up @@ -642,10 +642,6 @@ export class Volume extends VolumeBase {
}

protected validateProps(props: VolumeProps) {
if (!Token.isUnresolved(props.availabilityZone) && !/^[a-z]{2}-[a-z]+-[1-9]+[a-z]$/.test(props.availabilityZone)) {
throw new Error('`availabilityZone` is a region followed by a letter (ex: `us-east-1a`), or a token');
}

if (!(props.size || props.snapshotId)) {
throw new Error('Must provide at least one of `size` or `snapshotId`');
}
Expand Down
41 changes: 0 additions & 41 deletions packages/@aws-cdk/aws-ec2/test/volume.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1251,47 +1251,6 @@ nodeunitShim({
test.done();
},

'validation availabilityZone'(test: Test) {
// GIVEN
const stack = new cdk.Stack();
const volume = new Volume(stack, 'ForToken', {
availabilityZone: 'us-east-1a',
size: cdk.Size.gibibytes(8),
});
let idx: number = 0;

// THEN
test.doesNotThrow(() => {
// Should not throw if we provide a token for the AZ
new Volume(stack, `Volume${idx++}`, {
availabilityZone: volume.volumeId,
size: cdk.Size.gibibytes(8),
});
});
test.throws(() => {
new Volume(stack, `Volume${idx++}`, {
availabilityZone: 'us-east-1',
});
}, '`availabilityZone` is a region followed by a letter (ex: `us-east-1a`), or a token');
test.throws(() => {
new Volume(stack, `Volume${idx++}`, {
availabilityZone: 'Virginia',
});
}, '`availabilityZone` is a region followed by a letter (ex: `us-east-1a`), or a token');
test.throws(() => {
new Volume(stack, `Volume${idx++}`, {
availabilityZone: ' us-east-1a', // leading character(s)
});
}, '`availabilityZone` is a region followed by a letter (ex: `us-east-1a`), or a token');
test.throws(() => {
new Volume(stack, `Volume${idx++}`, {
availabilityZone: 'us-east-1a ', // trailing character(s)
});
}, '`availabilityZone` is a region followed by a letter (ex: `us-east-1a`), or a token');

test.done();
},

'validation snapshotId'(test: Test) {
// GIVEN
const stack = new cdk.Stack();
Expand Down
70 changes: 11 additions & 59 deletions packages/@aws-cdk/aws-rds/lib/cluster-engine.ts
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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 "<engineType><engineMajorVersion>",
* 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;

Expand Down Expand Up @@ -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 {
Expand All @@ -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) {
Expand Down
6 changes: 4 additions & 2 deletions packages/@aws-cdk/aws-rds/lib/cluster.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -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;

Expand All @@ -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,
});
Expand Down
26 changes: 26 additions & 0 deletions packages/@aws-cdk/aws-rds/lib/engine.ts
Original file line number Diff line number Diff line change
@@ -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 "<engineType><engineMajorVersion>",
* but can sometimes be a variation of that.
* You can pass this property when creating new ParameterGroup.
*/
readonly parameterGroupFamily: string;
}
1 change: 1 addition & 0 deletions packages/@aws-cdk/aws-rds/lib/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
export * from './engine';
export * from './cluster';
export * from './cluster-ref';
export * from './cluster-engine';
Expand Down
23 changes: 10 additions & 13 deletions packages/@aws-cdk/aws-rds/lib/instance-engine.ts
Original file line number Diff line number Diff line change
@@ -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}.
Expand All @@ -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;

Expand All @@ -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;

Expand All @@ -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 {
Expand Down Expand Up @@ -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' },
Expand Down
3 changes: 2 additions & 1 deletion packages/@aws-cdk/aws-rds/lib/instance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading

0 comments on commit 216c19c

Please sign in to comment.