aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.41k stars 3.8k forks source link

(rds): add auto_minor_version_upgrade to DatabaseCluster constructor #21894

Open rittneje opened 1 year ago

rittneje commented 1 year ago

Describe the feature

CloudFormation AWS::RDS::DBCluster has an AutoMinorVersionUpgrade field. But this field is not mapped into aws_rds.DatabaseCluster so we cannot set it. Please add it.

Use Case

Auto-minor version upgrades are a problem when the stack itself is specifying the minor version. (See #15475.) We want to be able to disable it so we can properly control the version.

Proposed Solution

No response

Other Information

No response

Acknowledgements

CDK version used

2.28.0 (build ba233f0)

Environment details (OS name and version, etc.)

Alpine 3.16, Python 3.10.5

peterwoodworth commented 1 year ago

Thanks for submitting this, we accept contributions! Check out our contributing guide if you're interested - there's a low chance the team will be able to address this soon but we'd be happy to review a PR 🙂

sreenath-tm commented 1 year ago

I'm interested in contributing to this issue, so before I start working it, would you mind sparing your time explaining what the fix will be like and pointing me to some resources to get started.

daschaa commented 1 year ago

@rittneje I don't see this field in the CloudFormation resource. See here: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-rds-dbcluster.html

Could you please specify from which documentation you took the information that the DBCluster resource has the field AutoMinorVersionUpgrade.

peterwoodworth commented 1 year ago

@daschaa I assume OP was referring to DBInstance

rittneje commented 1 year ago

@daschaa In the console I can create a cluster with auto-minor version upgrades disabled. I don't know if this maps to an per-instance property or not. So there may also be a CloudFormation issue where that property is not exposed at the cluster level.

rgoltz commented 1 year ago

As far I as known, AWS::RDS::DBCluster creates an Aurora database only. On the other hand via AWS::RDS::DBInstance you could create SQLServer, PostgreSQL, Oracle, etc (but no Aurora database). The option to disable auto_minor_version_upgrade in terms of AWS::RDS::DBInstance would also help to avoid Stack Drifts on CloudFormation stack-level, as reported via https://github.com/aws-cloudformation/cloudformation-coverage-roadmap/issues/1309

Since auto_minor_version_upgrade is enabled by default (even not set via CloudFormation template output) the exact version of your database would differ quite fast compaired to your IaC/CDK defintion. Having this said, I agree with @rittneje that a easy way to disable it in order to properly control the version of your QLServer, PostgreSQL, Oracle, etc database.

daschaa commented 1 year ago

@peterwoodworth For the instance level you can pass the autoMinorVersionUpgrade via the instanceProps when instantiating the Cluster if I understand it correctly? Is this maybe the thing which is done via the AWS Console?

const cluster = new rds.DatabaseCluster(stack, 'Database', {
  engine: rds.DatabaseClusterEngine.AURORA,
  instanceProps: {
    ...otherInstanceProps,
    autoMinorVersionUpgrade: false,
  },
});
rittneje commented 1 year ago

@rgoltz Aurora also uses AWS::RDS::DBInstance to define the individual instances. The DBClusterIdentifier ties these back to the cluster.

rgoltz commented 1 year ago

@rgoltz Aurora also uses AWS::RDS::DBInstance to define the individual instances. The DBClusterIdentifier ties these back to the cluster.

Thanks for clarification and correction.

s0enke commented 1 year ago

Current workaround:

        // disable auto minor version upgrade for all instances
        for (let i = 0; i < instanceCount; i++) {
            const cfnDbInstance = dbCluster.node.findChild(`Instance${i + 1}`) as aws_rds.CfnDBInstance;
            cfnDbInstance.autoMinorVersionUpgrade = false;
        }