Skip to content

Commit

Permalink
fix(rds): allow creating Proxies for imported resources (aws#10488)
Browse files Browse the repository at this point in the history
The current ProxyTarget relied on the underlying L1s to get the engine type
for a given Cluster/Instance.
Change IDatabaseCluster and IInstanceEngine to add an (optional)
`engine` property that is used instead.
Allow the user to specify the engine when importing a Cluster or Instance.

Also move the logic of determining the engine family into `IEngine`.

Fixes aws#9195

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
skinny85 authored Sep 23, 2020
1 parent c7c7851 commit c502114
Show file tree
Hide file tree
Showing 9 changed files with 221 additions and 144 deletions.
2 changes: 2 additions & 0 deletions packages/@aws-cdk/aws-rds/lib/cluster-engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ interface MysqlClusterEngineBaseProps {
}

abstract class MySqlClusterEngineBase extends ClusterEngineBase {
public readonly engineFamily = 'MYSQL';
public readonly supportedLogTypes: string[] = ['error', 'general', 'slowquery', 'audit'];

constructor(props: MysqlClusterEngineBaseProps) {
Expand Down Expand Up @@ -493,6 +494,7 @@ class AuroraPostgresClusterEngine extends ClusterEngineBase {
*/
private static readonly S3_EXPORT_FEATURE_NAME = 's3Export';

public readonly engineFamily = 'POSTGRESQL';
public readonly supportedLogTypes: string[] = ['postgresql'];

constructor(version?: AuroraPostgresEngineVersion) {
Expand Down
14 changes: 14 additions & 0 deletions packages/@aws-cdk/aws-rds/lib/cluster-ref.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import * as ec2 from '@aws-cdk/aws-ec2';
import * as secretsmanager from '@aws-cdk/aws-secretsmanager';
import { IResource } from '@aws-cdk/core';
import { IClusterEngine } from './cluster-engine';
import { Endpoint } from './endpoint';
import { DatabaseProxy, DatabaseProxyOptions } from './proxy';

Expand Down Expand Up @@ -35,6 +36,12 @@ export interface IDatabaseCluster extends IResource, ec2.IConnectable, secretsma
*/
readonly instanceEndpoints: Endpoint[];

/**
* The engine of this Cluster.
* May be not known for imported Clusters if it wasn't provided explicitly.
*/
readonly engine?: IClusterEngine;

/**
* Add a new db proxy to this cluster.
*/
Expand Down Expand Up @@ -92,4 +99,11 @@ export interface DatabaseClusterAttributes {
* @default - no instance endpoints
*/
readonly instanceEndpointAddresses?: string[];

/**
* The engine of the existing Cluster.
*
* @default - the imported Cluster's engine is unknown
*/
readonly engine?: IClusterEngine;
}
9 changes: 8 additions & 1 deletion packages/@aws-cdk/aws-rds/lib/cluster.ts
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,11 @@ export abstract class DatabaseClusterBase extends Resource implements IDatabaseC
* Abstract base for ``DatabaseCluster`` and ``DatabaseClusterFromSnapshot``
*/
abstract class DatabaseClusterNew extends DatabaseClusterBase {

/**
* The engine for this Cluster.
* Never undefined.
*/
public readonly engine?: IClusterEngine;
public readonly instanceIdentifiers: string[] = [];
public readonly instanceEndpoints: Endpoint[] = [];

Expand Down Expand Up @@ -331,6 +335,7 @@ abstract class DatabaseClusterNew extends DatabaseClusterBase {

const clusterParameterGroup = props.parameterGroup ?? clusterEngineBindConfig.parameterGroup;
const clusterParameterGroupConfig = clusterParameterGroup?.bindToCluster({});
this.engine = props.engine;

this.newCfnProps = {
// Basic
Expand Down Expand Up @@ -359,6 +364,7 @@ abstract class DatabaseClusterNew extends DatabaseClusterBase {
class ImportedDatabaseCluster extends DatabaseClusterBase implements IDatabaseCluster {
public readonly clusterIdentifier: string;
public readonly connections: ec2.Connections;
public readonly engine?: IClusterEngine;

private readonly _clusterEndpoint?: Endpoint;
private readonly _clusterReadEndpoint?: Endpoint;
Expand All @@ -375,6 +381,7 @@ class ImportedDatabaseCluster extends DatabaseClusterBase implements IDatabaseCl
securityGroups: attrs.securityGroups,
defaultPort,
});
this.engine = attrs.engine;

this._clusterEndpoint = (attrs.clusterEndpointAddress && attrs.port) ? new Endpoint(attrs.clusterEndpointAddress, attrs.port) : undefined;
this._clusterReadEndpoint = (attrs.readerEndpointAddress && attrs.port) ? new Endpoint(attrs.readerEndpointAddress, attrs.port) : undefined;
Expand Down
11 changes: 11 additions & 0 deletions packages/@aws-cdk/aws-rds/lib/engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,15 @@ export interface IEngine {
* (which means the major version of the engine is also not known)
*/
readonly parameterGroupFamily?: string;

/**
* The family this engine belongs to,
* like "MYSQL", or "POSTGRESQL".
* This property is used when creating a Database Proxy.
* Most engines don't belong to any family
* (and because of that, you can't create Database Proxies for their Clusters or Instances).
*
* @default - the engine doesn't belong to any family
*/
readonly engineFamily?: string;
}
5 changes: 5 additions & 0 deletions packages/@aws-cdk/aws-rds/lib/instance-engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ interface InstanceEngineBaseProps {
readonly multiUserRotationApplication: secretsmanager.SecretRotationApplication;
readonly version?: EngineVersion;
readonly parameterGroupFamily?: string;
readonly engineFamily?: string;
readonly features?: InstanceEngineFeatures;
}

Expand All @@ -118,6 +119,7 @@ abstract class InstanceEngineBase implements IInstanceEngine {
public readonly parameterGroupFamily?: string;
public readonly singleUserRotationApplication: secretsmanager.SecretRotationApplication;
public readonly multiUserRotationApplication: secretsmanager.SecretRotationApplication;
public readonly engineFamily?: string;

private readonly features?: InstanceEngineFeatures;

Expand All @@ -129,6 +131,7 @@ abstract class InstanceEngineBase implements IInstanceEngine {
this.engineVersion = props.version;
this.parameterGroupFamily = props.parameterGroupFamily ??
(this.engineVersion ? `${this.engineType}${this.engineVersion.majorVersion}` : undefined);
this.engineFamily = props.engineFamily;
}

public bindToInstance(_scope: core.Construct, options: InstanceEngineBindOptions): InstanceEngineConfig {
Expand Down Expand Up @@ -391,6 +394,7 @@ class MySqlInstanceEngine extends InstanceEngineBase {
majorVersion: version.mysqlMajorVersion,
}
: undefined,
engineFamily: 'MYSQL',
});
}
}
Expand Down Expand Up @@ -586,6 +590,7 @@ class PostgresInstanceEngine extends InstanceEngineBase {
}
: undefined,
features: version ? version?._features : { s3Import: 's3Import' },
engineFamily: 'POSTGRESQL',
});
}
}
Expand Down
17 changes: 17 additions & 0 deletions packages/@aws-cdk/aws-rds/lib/instance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,13 @@ export interface IDatabaseInstance extends IResource, ec2.IConnectable, secretsm
*/
readonly instanceEndpoint: Endpoint;

/**
* The engine of this database Instance.
* May be not known for imported Instances if it wasn't provided explicitly,
* or for read replicas.
*/
readonly engine?: IInstanceEngine;

/**
* Add a new db proxy to this instance.
*/
Expand Down Expand Up @@ -90,6 +97,13 @@ export interface DatabaseInstanceAttributes {
* The security groups of the instance.
*/
readonly securityGroups: ec2.ISecurityGroup[];

/**
* The engine of the existing database Instance.
*
* @default - the imported Instance's engine is unknown
*/
readonly engine?: IInstanceEngine;
}

/**
Expand All @@ -110,6 +124,7 @@ export abstract class DatabaseInstanceBase extends Resource implements IDatabase
public readonly dbInstanceEndpointAddress = attrs.instanceEndpointAddress;
public readonly dbInstanceEndpointPort = attrs.port.toString();
public readonly instanceEndpoint = new Endpoint(attrs.instanceEndpointAddress, attrs.port);
public readonly engine = attrs.engine;
protected enableIamAuthentication = true;
}

Expand Down Expand Up @@ -769,6 +784,7 @@ export interface DatabaseInstanceSourceProps extends DatabaseInstanceNewProps {
* A new source database instance (not a read replica)
*/
abstract class DatabaseInstanceSource extends DatabaseInstanceNew implements IDatabaseInstance {
public readonly engine?: IInstanceEngine;
/**
* The AWS Secrets Manager secret attached to the instance.
*/
Expand All @@ -785,6 +801,7 @@ abstract class DatabaseInstanceSource extends DatabaseInstanceNew implements IDa

this.singleUserRotationApplication = props.engine.singleUserRotationApplication;
this.multiUserRotationApplication = props.engine.multiUserRotationApplication;
this.engine = props.engine;

let { s3ImportRole, s3ExportRole } = setupS3ImportExport(this, props, true);
const engineConfig = props.engine.bindToInstance(this, {
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-rds/lib/private/util.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as iam from '@aws-cdk/aws-iam';
import * as s3 from '@aws-cdk/aws-s3';
import { Construct, CfnDeletionPolicy, CfnResource, RemovalPolicy } from '@aws-cdk/core';
import { IInstanceEngine } from '../instance-engine';
import { IEngine } from '../engine';

/** Common base of `DatabaseInstanceProps` and `DatabaseClusterBaseProps` that has only the S3 props */
export interface DatabaseS3ImportExportProps {
Expand Down Expand Up @@ -56,7 +56,7 @@ export function setupS3ImportExport(
return { s3ImportRole, s3ExportRole };
}

export function engineDescription(engine: IInstanceEngine) {
export function engineDescription(engine: IEngine) {
return engine.engineType + (engine.engineVersion?.fullVersion ? `-${engine.engineVersion.fullVersion}` : '');
}

Expand Down
42 changes: 19 additions & 23 deletions packages/@aws-cdk/aws-rds/lib/proxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@ import * as iam from '@aws-cdk/aws-iam';
import * as secretsmanager from '@aws-cdk/aws-secretsmanager';
import * as cdk from '@aws-cdk/core';
import { IDatabaseCluster } from './cluster-ref';
import { IEngine } from './engine';
import { IDatabaseInstance } from './instance';
import { CfnDBCluster, CfnDBInstance, CfnDBProxy, CfnDBProxyTargetGroup } from './rds.generated';
import { engineDescription } from './private/util';
import { CfnDBProxy, CfnDBProxyTargetGroup } from './rds.generated';

/**
* SessionPinningFilter
Expand Down Expand Up @@ -47,7 +49,7 @@ export class ProxyTarget {
* @param instance RDS database instance
*/
public static fromInstance(instance: IDatabaseInstance): ProxyTarget {
return new ProxyTarget(instance);
return new ProxyTarget(instance, undefined);
}

/**
Expand All @@ -59,34 +61,26 @@ export class ProxyTarget {
return new ProxyTarget(undefined, cluster);
}

private constructor(private readonly dbInstance?: IDatabaseInstance, private readonly dbCluster?: IDatabaseCluster) {}
private constructor(
private readonly dbInstance: IDatabaseInstance | undefined,
private readonly dbCluster: IDatabaseCluster | undefined) {
}

/**
* Bind this target to the specified database proxy.
*/
public bind(_: DatabaseProxy): ProxyTargetConfig {
let engine: string | undefined;
if (this.dbCluster && this.dbInstance) {
throw new Error('Proxy cannot target both database cluster and database instance.');
} else if (this.dbCluster) {
engine = (this.dbCluster.node.defaultChild as CfnDBCluster).engine;
} else if (this.dbInstance) {
engine = (this.dbInstance.node.defaultChild as CfnDBInstance).engine;
const engine: IEngine | undefined = this.dbInstance?.engine ?? this.dbCluster?.engine;

if (!engine) {
const errorResource = this.dbCluster ?? this.dbInstance;
throw new Error(`Could not determine engine for proxy target '${errorResource?.node.path}'. ` +
'Please provide it explicitly when importing the resource');
}

let engineFamily;
switch (engine) {
case 'aurora':
case 'aurora-mysql':
case 'mysql':
engineFamily = 'MYSQL';
break;
case 'aurora-postgresql':
case 'postgres':
engineFamily = 'POSTGRESQL';
break;
default:
throw new Error(`Unsupported engine type - ${engine}`);
const engineFamily = engine.engineFamily;
if (!engineFamily) {
throw new Error(`Engine '${engineDescription(engine)}' does not support proxies`);
}

return {
Expand All @@ -105,12 +99,14 @@ export interface ProxyTargetConfig {
* The engine family of the database instance or cluster this proxy connects with.
*/
readonly engineFamily: string;

/**
* The database instances to which this proxy connects.
* Either this or `dbClusters` will be set and the other `undefined`.
* @default - `undefined` if `dbClusters` is set.
*/
readonly dbInstances?: IDatabaseInstance[];

/**
* The database clusters to which this proxy connects.
* Either this or `dbInstances` will be set and the other `undefined`.
Expand Down
Loading

0 comments on commit c502114

Please sign in to comment.