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

DatabaseCluster: MonitoringRole not created by default when using readers and writers #25941

Closed
hertino76 opened this issue Jun 12, 2023 · 4 comments · Fixed by #26006
Closed
Labels
@aws-cdk/aws-rds Related to Amazon Relational Database bug This issue is a bug. effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md p1

Comments

@hertino76
Copy link

hertino76 commented Jun 12, 2023

Describe the bug

I've a Typescript app where I developed a library to create an Aurora Mysql database, this library was created with aws-cdk v2.68, we used this to create a database in production using azure DevOps and cloudFormation (we've a functional database), we used the InstanceProps property, and then we saw a message that this element is deprecated in a new version.

Then we updated to aws-cdk v2.82, and I replaced instanceProps according to the migration recommendations just changing the location of the properties as the code below: (https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_rds-readme.html#migrating-from-instanceprops)

Although no other change was made, there is no way to preserve the Instance IDs when we migrated to using "Writer and Reader" elements and also use the monitoringInterval element.

` Code :

 let cluster = new rds.DatabaseCluster(scope, 'DBCluster', {
  clusterIdentifier: props.nomCluster,
  engine: rds.DatabaseClusterEngine.auroraMysql({ version: varVersion }),
  defaultDatabaseName: props.nomDB,
  credentials: rds.Credentials.fromSecret(this.dbSecret),
  parameterGroup: this.parameterGroupCluster,
  monitoringInterval: Duration.seconds(60),
  copyTagsToSnapshot: false,
  storageEncrypted: true,
  backtrackWindow: Duration.hours(6),
  backup: {
    retention: Duration.days(RetentionDays.ONE_WEEK),
    preferredWindow: '01:00-03:00',
  },
  cloudwatchLogsExports: ['audit'],
  preferredMaintenanceWindow: 'Sat:03:00-Sun:11:00',
  deletionProtection: true,
  removalPolicy: RemovalPolicy.RETAIN,
  vpc: props.vpc,
  vpcSubnets: props.vpcSubnetsData,
  securityGroups: [props.securityGroup],

  writer : rds.ClusterInstance.serverlessV2('instance1', {
    parameterGroup: this.parameterGroupInst,
    publiclyAccessible: false,
    autoMinorVersionUpgrade: true,
    allowMajorVersionUpgrade: false,
    enablePerformanceInsights: true
  }),  ...`

Then we compiled the code, and when we tried to deploy the infrastructure we got several differences with the infra deployed before, and there shouldn't be any differences, in the photo below you can see the differences show, I consider that the one that causes the problem is the first element, the MonitoringRole element which will be destroyed, even though this element was never uses in the code, which means that the application must re-create the instance when deploying and, as a result, deestroy our functional implementation.

aws diff results:

image

Expected Behavior

aws diff shows zero differences when I upgrade to Writer and Reader elements

Current Behavior

aws diff show theses differences :

image

Reproduction Steps

  1. Create a library app (appA-Lib) with Typescript, npm and aws-cdk.
  2. First version of "Lib" application with InstanceProps element:
import { Environment, Duration, RemovalPolicy, } from 'aws-cdk-lib'
import { RetentionDays } from 'aws-cdk-lib/aws-logs'

import * as sm from 'aws-cdk-lib/aws-secretsmanager';
import * as rds from 'aws-cdk-lib/aws-rds';
import * as ec2 from 'aws-cdk-lib/aws-ec2';
import { Construct } from 'constructs';

export interface IAuroraDbProps {
  environment?: any,
  nomCluster: string,
  nomBD: string,
  parameterGroupCluster: any,
  parameterGroupInst: any,
  SG: any, 
  vpc: any
}

var varVersion: rds.AuroraMysqlEngineVersion;

export class AuroraDb extends Construct {
  public readonly dbSecret: sm.Secret;
  public readonly cluster: rds.DatabaseCluster;

  constructor(scope: Construct, id: string, props: IAuroraDbProps) {
    super(scope, id);

    this.dbSecret = new sm.Secret(scope, 'Secret', {
      description: 'Secret DB',
      generateSecretString: {
        secretStringTemplate: JSON.stringify({ username: 'admin2' }),
        excludePunctuation: true,
        includeSpace: false,
        generateStringKey: 'password',
        excludeCharacters: '/@"\' '      }
    });

    varVersion = rds.AuroraMysqlEngineVersion.VER_3_02_1;
    
    let cluster = new rds.DatabaseCluster(scope, 'ClusterDB', {
      clusterIdentifier: `${props.nomCluster}`,
      engine: rds.DatabaseClusterEngine.auroraMysql({ version: varVersion }),
      defaultDatabaseName: props.nomBD,
      instances: 2,
      credentials: rds.Credentials.fromSecret(this.dbSecret),
      parameterGroup: this.parameterGroupCluster,

      monitoringInterval: Duration.seconds(60),
      copyTagsToSnapshot: false,
      storageEncrypted: true,
      backtrackWindow: Duration.hours(6),
      backup: {
        retention: Duration.days(RetentionDays.ONE_WEEK),
        preferredWindow: '01:00-03:00',
      },
      cloudwatchLogsExports: ['audit'],
      preferredMaintenanceWindow: 'Sat:03:00-Sun:11:00',,
      deletionProtection: true,
      removalPolicy: RemovalPolicy.RETAIN,

      instanceProps: {
        instanceType: new ec2.InstanceType('serverless'),
        vpc: props.vpc,
        vpcSubnets: props.vpcSubnetsData,
        securityGroups: [props.securityGroup],
        parameterGroup: this.parameterGroupInst,
        publiclyAccessible: false,
        autoMinorVersionUpgrade: true,
        allowMajorVersionUpgrade: false,
        enablePerformanceInsights: true        
      }
    });
}
}
  1. Build the library version.
  2. Create a Typescript applicaton (appB) using the created library and deploy a database with this code.
  3. Upgate to aws-cdk v2.82 (although I believe that the initial version may be v2.82)
  4. Second version of the Library application using Writer and Reader elements:
import { Environment, Duration, RemovalPolicy, } from 'aws-cdk-lib'
import { RetentionDays } from 'aws-cdk-lib/aws-logs'

import * as sm from 'aws-cdk-lib/aws-secretsmanager';
import * as rds from 'aws-cdk-lib/aws-rds';
import * as ec2 from 'aws-cdk-lib/aws-ec2';
import { Construct } from 'constructs';

export interface IAuroraDbProps {
  environment?: any,
  nomCluster: string,
  nomBD: string,
  parameterGroupCluster: any,
  parameterGroupInst: any,
  SG: any, 
  vpc: any
}

var varVersion: rds.AuroraMysqlEngineVersion;

export class AuroraDb extends Construct {
  public readonly dbSecret: sm.Secret;
  public readonly cluster: rds.DatabaseCluster;

  constructor(scope: Construct, id: string, props: IAuroraDbProps) {
    super(scope, id);

    this.dbSecret = new sm.Secret(scope, 'Secret', {
      description: 'Secret DB',
      generateSecretString: {
        secretStringTemplate: JSON.stringify({ username: 'admin2' }),
        excludePunctuation: true,
        includeSpace: false,
        generateStringKey: 'password',
        excludeCharacters: '/@"\' '      }
    });

    varVersion = rds.AuroraMysqlEngineVersion.VER_3_02_1;
    
    let cluster = new rds.DatabaseCluster(scope, 'ClusterDB', {
      clusterIdentifier: `${props.nomCluster}`,
      engine: rds.DatabaseClusterEngine.auroraMysql({ version: varVersion }),
      defaultDatabaseName: props.nomBD,
      credentials: rds.Credentials.fromSecret(this.dbSecret),
      parameterGroup: this.parameterGroupCluster,

      monitoringInterval: Duration.seconds(60),
      copyTagsToSnapshot: false,
      storageEncrypted: true,
      backtrackWindow: Duration.hours(6),
      backup: {
        retention: Duration.days(RetentionDays.ONE_WEEK),
        preferredWindow: '01:00-03:00',
      },
      cloudwatchLogsExports: ['audit'],
      preferredMaintenanceWindow: 'Sat:03:00-Sun:11:00',,
      deletionProtection: true,
      removalPolicy: RemovalPolicy.RETAIN,

      writer : rds.ClusterInstance.serverlessV2(`${props.nomCluster}` + 'instance1', {
        parameterGroup: this.parameterGroupInst,
        publiclyAccessible: false,
        autoMinorVersionUpgrade: true,
        allowMajorVersionUpgrade: false,
        enablePerformanceInsights: true
      }),
    readers : [
        rds.ClusterInstance.serverlessV2(`${props.nomCluster}` + 'instance2', {scaleWithWriter: true}),
      ],
      serverlessV2MinCapacity: minCap,
      serverlessV2MaxCapacity: maxCap,

    });
}
}
  1. Build the library version.
  2. Update the application (appB) using the new version of the library.
  3. Try to deploy a Aurora MySql database with this code --> you will get Differences

Possible Solution

Check why aws-cdk finds differences in the MonitoringRole element when we update these elements and we use monitoringInterval. The problem isn't version 2.82 itself, because I did a test with this version without changing the InstanceProps element and the deployment works fine, the error doesn't show up until I change the element to Writer.

Additional Information/Context

No response

CDK CLI Version

2.82.0 (build 3a8648a)

Framework Version

No response

Node.js Version

npm: '9.3.1', node: '18.14.0',

OS

Windows 10 Entreprise 21H2

Language

Typescript

Language Version

Version 5.0.4

Other information

No response

@hertino76 hertino76 added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jun 12, 2023
@github-actions github-actions bot added the @aws-cdk/aws-rds Related to Amazon Relational Database label Jun 12, 2023
@peterwoodworth
Copy link
Contributor

I think that you can use the instanceIdentifiers property to make sure the instance logical IDs don't get replaced.

Regards to the monitoring role - it appears that one is not automatically created when using the new API. In the modern _createInstances method, we only pass in the monitoring role that was passed in as a prop - the bind method won't automatically create a monitoring role under the hood

monitoringRole: props.monitoringRole,

To contrast, in the legacy code the role gets created if not passed in

monitoringRole = props.monitoringRole || new Role(cluster, 'MonitoringRole', {

Thanks for reporting!

@peterwoodworth peterwoodworth added p1 effort/small Small work item – less than a day of effort and removed needs-triage This issue or PR still needs to be triaged. labels Jun 12, 2023
@peterwoodworth peterwoodworth changed the title DatabaseCluster: Destroy MonitoringRole when replacing InstanceProps with Writer and Reader DatabaseCluster: MonitoringRole not created by default when using readers and writers Jun 12, 2023
@hertino76
Copy link
Author

I could clarify that when we first deployed the database with cdk before using writers and readers, the application created a default Monitoring role, with the new implementation it tries to destroy it.

@hertino76
Copy link
Author

Please let me know if this issue is fixed and if it is implemented in the "Bug Fix" version of aws-cdk, because I saw something that looks like a bug in the Pull Request.

lpizzinidev added a commit to lpizzinidev/aws-cdk that referenced this issue Jun 26, 2023
lpizzinidev added a commit to lpizzinidev/aws-cdk that referenced this issue Jun 27, 2023
lpizzinidev added a commit to lpizzinidev/aws-cdk that referenced this issue Jun 27, 2023
corymhall added a commit to lpizzinidev/aws-cdk that referenced this issue Jun 30, 2023
mergify bot added a commit to lpizzinidev/aws-cdk that referenced this issue Jun 30, 2023
@mergify mergify bot closed this as completed in #26006 Jun 30, 2023
mergify bot pushed a commit that referenced this issue Jun 30, 2023
…s 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*
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-rds Related to Amazon Relational Database bug This issue is a bug. effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md p1
Projects
None yet
2 participants