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): allow creating Proxies for imported resources #10488

Merged
merged 3 commits into from
Sep 23, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
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
12 changes: 12 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,16 @@ 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.
skinny85 marked this conversation as resolved.
Show resolved Hide resolved
* 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 @@ -783,6 +798,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 @@ -799,6 +815,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
47 changes: 25 additions & 22 deletions packages/@aws-cdk/aws-rds/lib/proxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@ 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 { CfnDBProxy, CfnDBProxyTargetGroup } from './rds.generated';

/**
* SessionPinningFilter
Expand Down Expand Up @@ -47,7 +48,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 +60,34 @@ 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) {
}
njlynch marked this conversation as resolved.
Show resolved Hide resolved

/**
* 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;
let engine: IEngine | undefined;
if (this.dbCluster) {
engine = this.dbCluster.engine;
} else if (this.dbInstance) {
engine = (this.dbInstance.node.defaultChild as CfnDBInstance).engine;
engine = this.dbInstance.engine;
}
skinny85 marked this conversation as resolved.
Show resolved Hide resolved

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) {
const engineVersion = engine.engineVersion?.fullVersion
? ` (version: ${engine.engineVersion.fullVersion})`
: '';
throw new Error(`Engine '${engine.engineType}'${engineVersion} does not support proxies`);
skinny85 marked this conversation as resolved.
Show resolved Hide resolved
}

return {
Expand All @@ -105,12 +106,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