Skip to content

Commit 5a895a3

Browse files
authored
fix(rds): MySQL 8.0 uses wrong Parameter for S3 export (#19775)
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*
1 parent 95cb3f3 commit 5a895a3

File tree

8 files changed

+1886
-13
lines changed

8 files changed

+1886
-13
lines changed

packages/@aws-cdk/aws-rds/lib/cluster-engine.ts

+33-6
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,14 @@ export interface IClusterEngine extends IEngine {
9797
/** The log types that are available with this engine type */
9898
readonly supportedLogTypes: string[];
9999

100+
/**
101+
* Whether the IAM Roles used for data importing and exporting need to be combined for this Engine,
102+
* or can they be kept separate.
103+
*
104+
* @default false
105+
*/
106+
readonly combineImportAndExportRoles?: boolean;
107+
100108
/**
101109
* Method called when the engine is used to create a new cluster.
102110
*/
@@ -154,11 +162,13 @@ interface MysqlClusterEngineBaseProps {
154162
readonly engineType: string;
155163
readonly engineVersion?: EngineVersion;
156164
readonly defaultMajorVersion: string;
165+
readonly combineImportAndExportRoles?: boolean;
157166
}
158167

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

163173
constructor(props: MysqlClusterEngineBaseProps) {
164174
super({
@@ -167,6 +177,7 @@ abstract class MySqlClusterEngineBase extends ClusterEngineBase {
167177
multiUserRotationApplication: secretsmanager.SecretRotationApplication.MYSQL_ROTATION_MULTI_USER,
168178
engineVersion: props.engineVersion ? props.engineVersion : { majorVersion: props.defaultMajorVersion },
169179
});
180+
this.combineImportAndExportRoles = props.combineImportAndExportRoles;
170181
}
171182

172183
public bindToCluster(scope: Construct, options: ClusterEngineBindOptions): ClusterEngineConfig {
@@ -177,14 +188,18 @@ abstract class MySqlClusterEngineBase extends ClusterEngineBase {
177188
})
178189
: config.parameterGroup);
179190
if (options.s3ImportRole) {
180-
// major version 8.0 uses a different name for the S3 import parameter
181-
const s3ImportParam = this.engineVersion?.majorVersion === '8.0'
191+
// versions which combine the import and export Roles (right now, this is only 8.0)
192+
// require a different parameter name (identical for both import and export)
193+
const s3ImportParam = this.combineImportAndExportRoles
182194
? 'aws_default_s3_role'
183195
: 'aurora_load_from_s3_role';
184196
parameterGroup?.addParameter(s3ImportParam, options.s3ImportRole.roleArn);
185197
}
186198
if (options.s3ExportRole) {
187-
parameterGroup?.addParameter('aurora_select_into_s3_role', options.s3ExportRole.roleArn);
199+
const s3ExportParam = this.combineImportAndExportRoles
200+
? 'aws_default_s3_role'
201+
: 'aurora_select_into_s3_role';
202+
parameterGroup?.addParameter(s3ExportParam, options.s3ExportRole.roleArn);
188203
}
189204

190205
return {
@@ -366,17 +381,28 @@ export class AuroraMysqlEngineVersion {
366381
}
367382

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

372388
/** The full version string, for example, "5.7.mysql_aurora.1.78.3.6". */
373389
public readonly auroraMysqlFullVersion: string;
374-
/** The major version of the engine. Currently, it's always "5.7". */
390+
/** The major version of the engine. Currently, it's either "5.7", or "8.0". */
375391
public readonly auroraMysqlMajorVersion: string;
392+
/**
393+
* Whether this version requires combining the import and export IAM Roles.
394+
*
395+
* @internal
396+
*/
397+
public readonly _combineImportAndExportRoles?: boolean;
376398

377-
private constructor(auroraMysqlFullVersion: string, auroraMysqlMajorVersion: string = '5.7') {
399+
private constructor(
400+
auroraMysqlFullVersion: string, auroraMysqlMajorVersion: string = '5.7',
401+
combineImportAndExportRoles?: boolean,
402+
) {
378403
this.auroraMysqlFullVersion = auroraMysqlFullVersion;
379404
this.auroraMysqlMajorVersion = auroraMysqlMajorVersion;
405+
this._combineImportAndExportRoles = combineImportAndExportRoles;
380406
}
381407
}
382408

@@ -400,6 +426,7 @@ class AuroraMysqlClusterEngine extends MySqlClusterEngineBase {
400426
}
401427
: undefined,
402428
defaultMajorVersion: '5.7',
429+
combineImportAndExportRoles: version?._combineImportAndExportRoles,
403430
});
404431
}
405432

packages/@aws-cdk/aws-rds/lib/cluster.ts

+7-2
Original file line numberDiff line numberDiff line change
@@ -362,7 +362,8 @@ abstract class DatabaseClusterNew extends DatabaseClusterBase {
362362
}),
363363
];
364364

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

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

0 commit comments

Comments
 (0)