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

feat(rds): unify ParameterGroup and ClusterParameterGroup #8959

Merged
merged 4 commits into from
Jul 16, 2020
Merged
Show file tree
Hide file tree
Changes from all 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
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({});
skinny85 marked this conversation as resolved.
Show resolved Hide resolved

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
skinny85 marked this conversation as resolved.
Show resolved Hide resolved
*/
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