Skip to content

Commit

Permalink
fix(rds): MySQL 8.0 uses wrong Parameter for S3 export (#19775)
Browse files Browse the repository at this point in the history
In the newest version of the MySQL 8.0 Aurora engine,
the S3 import and export Roles need to be combined,
which is the first time this is needed for a Cluster
(previously, only Instances combined these two Roles).
Introduce this concept as a first-class property of a Cluster Engine,
making it `false` by default, and make it `true` in MySQL version 8.0.

Fixes #19735

----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/master/INTEGRATION_TESTS.md)?
	* [x] Did you use `cdk-integ` to deploy the infrastructure and generate the snapshot (i.e. `cdk-integ` without `--dry-run`)?

*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 and otaviomacedo committed Apr 11, 2022
1 parent 7bca49f commit 87ca3ab
Show file tree
Hide file tree
Showing 8 changed files with 1,886 additions and 13 deletions.
39 changes: 33 additions & 6 deletions packages/@aws-cdk/aws-rds/lib/cluster-engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,14 @@ export interface IClusterEngine extends IEngine {
/** The log types that are available with this engine type */
readonly supportedLogTypes: string[];

/**
* Whether the IAM Roles used for data importing and exporting need to be combined for this Engine,
* or can they be kept separate.
*
* @default false
*/
readonly combineImportAndExportRoles?: boolean;

/**
* Method called when the engine is used to create a new cluster.
*/
Expand Down Expand Up @@ -154,11 +162,13 @@ interface MysqlClusterEngineBaseProps {
readonly engineType: string;
readonly engineVersion?: EngineVersion;
readonly defaultMajorVersion: string;
readonly combineImportAndExportRoles?: boolean;
}

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

constructor(props: MysqlClusterEngineBaseProps) {
super({
Expand All @@ -167,6 +177,7 @@ abstract class MySqlClusterEngineBase extends ClusterEngineBase {
multiUserRotationApplication: secretsmanager.SecretRotationApplication.MYSQL_ROTATION_MULTI_USER,
engineVersion: props.engineVersion ? props.engineVersion : { majorVersion: props.defaultMajorVersion },
});
this.combineImportAndExportRoles = props.combineImportAndExportRoles;
}

public bindToCluster(scope: Construct, options: ClusterEngineBindOptions): ClusterEngineConfig {
Expand All @@ -177,14 +188,18 @@ abstract class MySqlClusterEngineBase extends ClusterEngineBase {
})
: config.parameterGroup);
if (options.s3ImportRole) {
// major version 8.0 uses a different name for the S3 import parameter
const s3ImportParam = this.engineVersion?.majorVersion === '8.0'
// versions which combine the import and export Roles (right now, this is only 8.0)
// require a different parameter name (identical for both import and export)
const s3ImportParam = this.combineImportAndExportRoles
? 'aws_default_s3_role'
: 'aurora_load_from_s3_role';
parameterGroup?.addParameter(s3ImportParam, options.s3ImportRole.roleArn);
}
if (options.s3ExportRole) {
parameterGroup?.addParameter('aurora_select_into_s3_role', options.s3ExportRole.roleArn);
const s3ExportParam = this.combineImportAndExportRoles
? 'aws_default_s3_role'
: 'aurora_select_into_s3_role';
parameterGroup?.addParameter(s3ExportParam, options.s3ExportRole.roleArn);
}

return {
Expand Down Expand Up @@ -366,17 +381,28 @@ export class AuroraMysqlEngineVersion {
}

private static builtIn_8_0(minorVersion: string): AuroraMysqlEngineVersion {
return new AuroraMysqlEngineVersion(`8.0.mysql_aurora.${minorVersion}`, '8.0');
// 8.0 of the MySQL engine needs to combine the import and export Roles
return new AuroraMysqlEngineVersion(`8.0.mysql_aurora.${minorVersion}`, '8.0', true);
}

/** The full version string, for example, "5.7.mysql_aurora.1.78.3.6". */
public readonly auroraMysqlFullVersion: string;
/** The major version of the engine. Currently, it's always "5.7". */
/** The major version of the engine. Currently, it's either "5.7", or "8.0". */
public readonly auroraMysqlMajorVersion: string;
/**
* Whether this version requires combining the import and export IAM Roles.
*
* @internal
*/
public readonly _combineImportAndExportRoles?: boolean;

private constructor(auroraMysqlFullVersion: string, auroraMysqlMajorVersion: string = '5.7') {
private constructor(
auroraMysqlFullVersion: string, auroraMysqlMajorVersion: string = '5.7',
combineImportAndExportRoles?: boolean,
) {
this.auroraMysqlFullVersion = auroraMysqlFullVersion;
this.auroraMysqlMajorVersion = auroraMysqlMajorVersion;
this._combineImportAndExportRoles = combineImportAndExportRoles;
}
}

Expand All @@ -400,6 +426,7 @@ class AuroraMysqlClusterEngine extends MySqlClusterEngineBase {
}
: undefined,
defaultMajorVersion: '5.7',
combineImportAndExportRoles: version?._combineImportAndExportRoles,
});
}

Expand Down
9 changes: 7 additions & 2 deletions packages/@aws-cdk/aws-rds/lib/cluster.ts
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,8 @@ abstract class DatabaseClusterNew extends DatabaseClusterBase {
}),
];

let { s3ImportRole, s3ExportRole } = setupS3ImportExport(this, props, /* combineRoles */ false);
const combineRoles = props.engine.combineImportAndExportRoles ?? false;
let { s3ImportRole, s3ExportRole } = setupS3ImportExport(this, props, combineRoles);

if (props.parameterGroup && props.parameters) {
throw new Error('You cannot specify both parameterGroup and parameters');
Expand All @@ -386,7 +387,11 @@ abstract class DatabaseClusterNew extends DatabaseClusterBase {
if (s3ImportRole) {
clusterAssociatedRoles.push({ roleArn: s3ImportRole.roleArn, featureName: clusterEngineBindConfig.features?.s3Import });
}
if (s3ExportRole) {
if (s3ExportRole &&
// only add the second associated Role if it's different than the first
// (duplicates in the associated Roles array are not allowed by the RDS service)
(s3ExportRole !== s3ImportRole ||
clusterEngineBindConfig.features?.s3Import !== clusterEngineBindConfig.features?.s3Export)) {
clusterAssociatedRoles.push({ roleArn: s3ExportRole.roleArn, featureName: clusterEngineBindConfig.features?.s3Export });
}

Expand Down
Loading

0 comments on commit 87ca3ab

Please sign in to comment.