From 9065b25a6a3812cf186aaddcbc5466d422a40424 Mon Sep 17 00:00:00 2001 From: Luca Pizzini Date: Fri, 30 Jun 2023 13:24:00 +0200 Subject: [PATCH] fix(rds): monitoring role is not created by default when using readers and writers (#26006) The [`_createInstances`](https://github.com/aws/aws-cdk/blob/4c9016a264c2fec9c0e0e3fae1d7c4216c964b31/packages/aws-cdk-lib/aws-rds/lib/cluster.ts#L635) function was not providing a default `monitoringRole` value with enabled monitoring. This fix creates a default role as done by the [legacy code](https://github.com/aws/aws-cdk/blob/4c9016a264c2fec9c0e0e3fae1d7c4216c964b31/packages/aws-cdk-lib/aws-rds/lib/cluster.ts#L1228). Closes #25941. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- packages/aws-cdk-lib/aws-rds/lib/cluster.ts | 21 ++++++-- .../aws-cdk-lib/aws-rds/test/cluster.test.ts | 54 ++++++++++++++++++- 2 files changed, 69 insertions(+), 6 deletions(-) diff --git a/packages/aws-cdk-lib/aws-rds/lib/cluster.ts b/packages/aws-cdk-lib/aws-rds/lib/cluster.ts index 1f7acddde80ae..467df41b2fa21 100644 --- a/packages/aws-cdk-lib/aws-rds/lib/cluster.ts +++ b/packages/aws-cdk-lib/aws-rds/lib/cluster.ts @@ -632,14 +632,25 @@ abstract class DatabaseClusterNew extends DatabaseClusterBase { * * @internal */ - protected _createInstances(props: DatabaseClusterProps): InstanceConfig { + protected _createInstances(cluster: DatabaseClusterNew, props: DatabaseClusterProps): InstanceConfig { const instanceEndpoints: Endpoint[] = []; const instanceIdentifiers: string[] = []; const readers: IAuroraClusterInstance[] = []; + + let monitoringRole = props.monitoringRole; + if (!props.monitoringRole && props.monitoringInterval && props.monitoringInterval.toSeconds()) { + monitoringRole = new Role(cluster, 'MonitoringRole', { + assumedBy: new ServicePrincipal('monitoring.rds.amazonaws.com'), + managedPolicies: [ + ManagedPolicy.fromAwsManagedPolicyName('service-role/AmazonRDSEnhancedMonitoringRole'), + ], + }); + } + // need to create the writer first since writer is determined by what instance is first const writer = props.writer!.bind(this, this, { monitoringInterval: props.monitoringInterval, - monitoringRole: props.monitoringRole, + monitoringRole: monitoringRole, removalPolicy: props.removalPolicy ?? RemovalPolicy.SNAPSHOT, subnetGroup: this.subnetGroup, promotionTier: 0, // override the promotion tier so that writers are always 0 @@ -647,7 +658,7 @@ abstract class DatabaseClusterNew extends DatabaseClusterBase { (props.readers ?? []).forEach(instance => { const clusterInstance = instance.bind(this, this, { monitoringInterval: props.monitoringInterval, - monitoringRole: props.monitoringRole, + monitoringRole: monitoringRole, removalPolicy: props.removalPolicy ?? RemovalPolicy.SNAPSHOT, subnetGroup: this.subnetGroup, }); @@ -988,7 +999,7 @@ export class DatabaseCluster extends DatabaseClusterNew { throw new Error('writer must be provided'); } - const createdInstances = props.writer ? this._createInstances(props) : legacyCreateInstances(this, props, this.subnetGroup); + const createdInstances = props.writer ? this._createInstances(this, props) : legacyCreateInstances(this, props, this.subnetGroup); this.instanceIdentifiers = createdInstances.instanceIdentifiers; this.instanceEndpoints = createdInstances.instanceEndpoints; } @@ -1185,7 +1196,7 @@ export class DatabaseClusterFromSnapshot extends DatabaseClusterNew { if ((props.writer || props.readers) && (props.instances || props.instanceProps)) { throw new Error('Cannot provide clusterInstances if instances or instanceProps are provided'); } - const createdInstances = props.writer ? this._createInstances(props) : legacyCreateInstances(this, props, this.subnetGroup); + const createdInstances = props.writer ? this._createInstances(this, props) : legacyCreateInstances(this, props, this.subnetGroup); this.instanceIdentifiers = createdInstances.instanceIdentifiers; this.instanceEndpoints = createdInstances.instanceEndpoints; } diff --git a/packages/aws-cdk-lib/aws-rds/test/cluster.test.ts b/packages/aws-cdk-lib/aws-rds/test/cluster.test.ts index 298dfec3352e0..afb8472295e2b 100644 --- a/packages/aws-cdk-lib/aws-rds/test/cluster.test.ts +++ b/packages/aws-cdk-lib/aws-rds/test/cluster.test.ts @@ -1527,7 +1527,7 @@ describe('cluster', () => { }); }); - test('cluster with enabled monitoring', () => { + test('cluster with enabled monitoring (legacy)', () => { // GIVEN const stack = testStack(); const vpc = new ec2.Vpc(stack, 'VPC'); @@ -1584,6 +1584,58 @@ describe('cluster', () => { }); }); + test('cluster with enabled monitoring should create default role with new api', () => { + // GIVEN + const stack = testStack(); + const vpc = new ec2.Vpc(stack, 'VPC'); + + // WHEN + new DatabaseCluster(stack, 'Database', { + engine: DatabaseClusterEngine.AURORA, + vpc, + writer: ClusterInstance.serverlessV2('writer'), + iamAuthentication: true, + monitoringInterval: cdk.Duration.minutes(1), + }); + + // THEN + Template.fromStack(stack).hasResourceProperties('AWS::RDS::DBInstance', { + MonitoringInterval: 60, + MonitoringRoleArn: { + 'Fn::GetAtt': ['DatabaseMonitoringRole576991DA', 'Arn'], + }, + }); + + Template.fromStack(stack).hasResourceProperties('AWS::IAM::Role', { + AssumeRolePolicyDocument: { + Statement: [ + { + Action: 'sts:AssumeRole', + Effect: 'Allow', + Principal: { + Service: 'monitoring.rds.amazonaws.com', + }, + }, + ], + Version: '2012-10-17', + }, + ManagedPolicyArns: [ + { + 'Fn::Join': [ + '', + [ + 'arn:', + { + Ref: 'AWS::Partition', + }, + ':iam::aws:policy/service-role/AmazonRDSEnhancedMonitoringRole', + ], + ], + }, + ], + }); + }); + test('create a cluster with imported monitoring role', () => { // GIVEN const stack = testStack();