rds.DatabaseInstance: update CDK version from 1.59.0 -> 1.115.0 cause the replacement of RDS DB instance resource as property 'PubliclyAccessible' is added.

Having an issue after updating the CDK version from 1.59.0 -> 1.115.0 Previously, we defined the vpc with public subnet only, a RDS DB instance was created in the VPC. Script like below:

        vpc = ec2.Vpc(self, "vpc", 
            subnet_configuration = [
                ec2.SubnetConfiguration(name="Public", subnet_type=ec2.SubnetType.PUBLIC),
                # ec2.SubnetConfiguration(name="Private", subnet_type=ec2.SubnetType.PRIVATE)
                ]
            )

        instance = rds.DatabaseInstance(self, "Instance",
            engine=rds.DatabaseInstanceEngine.mysql(
                version=rds.MysqlEngineVersion.VER_8_0_19
            ),

            instance_type=ec2.InstanceType.of(ec2.InstanceClass.BURSTABLE3, ec2.InstanceSize.SMALL),
            vpc=vpc,
            credentials=rds.Credentials.from_secret(my_user_secret),
            license_model = rds.LicenseModel.GENERAL_PUBLIC_LICENSE
        )

However after updating the CDK to the most recent version, we found the RDS will be create in a private subnet if not specify a vpc_subnet parameter. After adding the vpc_subnets = {"subnet_type": ec2.SubnetType.PUBLIC} to RDS DB instance construct, it automatically add PubliclyAccessible property to CFN template, and that is causing the replacement.

        instance = rds.DatabaseInstance(self, "Instance",
            engine=rds.DatabaseInstanceEngine.mysql(
                version=rds.MysqlEngineVersion.VER_8_0_19
            ),

            instance_type=ec2.InstanceType.of(ec2.InstanceClass.BURSTABLE3, ec2.InstanceSize.SMALL),
            vpc=vpc,
            credentials=rds.Credentials.from_secret(my_user_secret),
            # publicly_accessible=False,
            # optional, defaults to m5.large
            vpc_subnets={
                 "subnet_type": ec2.SubnetType.PUBLIC
            },
            subnet_group = my_subnet_group,
            license_model = rds.LicenseModel.GENERAL_PUBLIC_LICENSE
        )

Reproduction Steps

with below code, command cdk synth will generate the RDS DB instance with property PubliclyAccessible, as in previous version, this property doesn't present, add this property will cause the replacement.

from aws_cdk import core as cdk
import aws_cdk.aws_rds as rds
import aws_cdk.aws_ec2 as ec2 
import aws_cdk.aws_secretsmanager as secretsmanager

class RdsStack(cdk.Stack):

    def __init__(self, scope: cdk.Construct, construct_id: str, **kwargs) -> None:
        super().__init__(scope, construct_id, **kwargs)
        #vpc = ec2.Vpc(self, "vpc")
        vpc = ec2.Vpc(self, "vpc", 
            subnet_configuration = [
                ec2.SubnetConfiguration(name="Public", subnet_type=ec2.SubnetType.PUBLIC),
                ec2.SubnetConfiguration(name="Private", subnet_type=ec2.SubnetType.PRIVATE)
                ]
            )

        my_user_secret = rds.DatabaseSecret(self, "MyUserSecret",
            username="myuser",
            secret_name="my-user-secret" # optional, defaults to a CloudFormation-generated name
        )        
        my_subnet_group = rds.SubnetGroup(self, "RDSSubnetGroup",
            description = "my rds subnet public group",
            vpc = vpc,
            vpc_subnets = {
                "subnet_type": ec2.SubnetType.PUBLIC
            }
        )
        rds_instance = rds.DatabaseInstance(self, "Instance",
            engine=rds.DatabaseInstanceEngine.mysql(
                version=rds.MysqlEngineVersion.VER_8_0_19
            ),

            instance_type=ec2.InstanceType.of(ec2.InstanceClass.BURSTABLE3, ec2.InstanceSize.SMALL),
            vpc=vpc,
            credentials=rds.Credentials.from_secret(my_user_secret),
            # publicly_accessible=False,
            # optional, defaults to m5.large
            vpc_subnets={
                 "subnet_type": ec2.SubnetType.PUBLIC
            },
            subnet_group = my_subnet_group,
            license_model = rds.LicenseModel.GENERAL_PUBLIC_LICENSE
        )

What did you expect to happen?

the update of CDK version should not affect critical resource, such as RDS DB instance, replaced.

What actually happened?

The new version of CDK will try to add PubliclyAccessible as long as you set the subnet_group parameter in CDK.

Workaround

        # Add the following to remove the new property. 
        cfn_rds_instance = rds_instance.node.default_child
        cfn_rds_instance.add_property_deletion_override('PubliclyAccessible')

Environment

CDK CLI Version : 1.59.0 -> 1.115.0
Framework Version:
Node.js Version:
OS : AL2
Language (Version): Python (3.7.3)

This is :bug: Bug Report

Current workaround will be using escape hatch to remove the property:

cfn_rds_instance = rds_instance.node.default_child 
cfn_rds_instance.add_property_deletion_override('PubliclyAccessible')

This kind of issue will impact the critical resource. Please fix this with higher priority.

There are two issues at play here:

Different default subnet (PRIVATE instead of PUBLIC)
PubliclyAccessible is set to true (leading to replacement)

Different default subnet

Relevant code in 1.59.0:

https://github.com/aws/aws-cdk/blob/1d082f4d42467885122d4d0cedab1911d064de72/packages/@aws-cdk/aws-rds/lib/instance.ts#L503

Relevant code in 1.115.0:

https://github.com/aws/aws-cdk/blob/04b8d400b2653aff4f48709e8b420c6adb996ef5/packages/%40aws-cdk/aws-rds/lib/subnet-group.ts#L75

As you can see, the new code applies a default that the old code did not. The new code should (probably?) not be applying that default, the VPC class has its own defaults.

't Were @njlynch who changes this behavior in https://github.com/aws/aws-cdk/pull/10391. Looks to be an oversight, but who knows. Nick?

PubliclyAccessible=True

Comes from this line:

https://github.com/aws/aws-cdk/blob/04b8d400b2653aff4f48709e8b420c6adb996ef5/packages/%40aws-cdk/aws-rds/lib/instance.ts#L746

Not sure that I entirely agree with the logic: what if you didn't pass in PUBLIC by default, but selected a subnet that happens to be public by some other means? By name, by direct reference, or by defaulting through all the available subnet types? In those cases the flag WOULDN'T be flipped.

It's going to be slightly hard to change this logic though, especially since this flag apparently has implications for replacement of the instance. But it's making me slightly uncomfortable.

Feature was added by random contributor: https://github.com/aws/aws-cdk/pull/12164

I'm in favor of changing the logic for both of these:

SubnetGroup should not be imposing a default of PRIVATE, it should just be falling back to what Vpc does by default.
We should be defaulting publiclyAccessible based off of the actually selected subnets, not off of the declared selection mechanism (in fact selectedSubnets has a boolean flag indicating whether or not it matched public subnets).

However both of these fixes probably need to be gated behind feature flags that will REMAIN for CDKv2, as they are potentially backwards breaking (but need to be able to be flipped to old behavior).

In any case, @fanhongy, to unblock yourself you don't need to resort to overrides. You can just pass publicly_accessible=False to the constructor to switch it back off.

As you can see, the new code applies a default that the old code did not. The new code should (probably?) not be applying that default, the VPC class has its own defaults. njlynch changes this behavior in #10391. Looks to be an oversight, but who knows. Nick?

To be honest, I cannot recall why I decided to default to private subnets here. Likely a security mentality of private === more secure than public, combined with an ignorance of the fact that the VPC class providing its own defaults. Agreed that unfortunately changing it now would require a feature flag.

In any case, @fanhongy, to unblock yourself you don't need to resort to overrides. You can just pass publicly_accessible=False to the constructor to switch it back off.

I think the reason an override was added is that CFN triggers an update if PubliclyAccessible goes from unset to set, regardless of whether the set state is the same as the default state.

This issue has not received any attention in 1 year. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

aws / aws-cdk