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.63k stars 3.91k forks source link

aws-rds: ServerlessCluster does not resolve tokens in clusterIdentifier if lowercaseDbIdentifier is enabled. #24937

Open awsdiegorad opened 1 year ago

awsdiegorad commented 1 year ago

Describe the bug

When the "@aws-cdk/aws-rds:lowercaseDbIdentifier" feature flag is enabled in the cdk.json file, CDK will automatically make the value assigned to the clusterIdentifier property of the ServerlesCluster construct lower case.

If the value assigned to clusterIdentifier includes any tokens, then they will be made lower case before they are resolved, leading to the output being the literal lower case token as we see in the CX environment.

Expected Behavior

    new CfnParameter(this, 'env', {
    type: 'String',
    default: 'example'
});

  const cluster = new rds.ServerlessCluster(this, `ClusterDB`, {
    //clusterIdentifier: `cluster-${Tokenization.resolve(Fn.ref("env"), {resolver: tokenResolver., scope: this})}`,
    clusterIdentifier: Fn.ref("env"),

Should output


  Cluster:
    Type: AWS::RDS::DBCluster
    Properties:
...
      DBClusterIdentifier:
        Ref: env

Current Behavior

If lowercaseDbIdentifier is set to true, the above code instead outputs

  Cluster:
    Type: AWS::RDS::DBCluster
    Properties:
...
      DBClusterIdentifier: ${token[token.324]}

Reproduction Steps

  1. Set @aws-cdk/aws-rds:lowercaseDbIdentifier to true in the cdk.json file
  2. Create a ServerlessCluster resource with the clusterIdentifier property defined, include a tokenized object in the value for the cluster identifier
  3. Synthesize the CDK application and observe the value assigned to DBClusterIdentifier for the generated AWS::RDS::DBCluster resouce

Possible Solution

This was a known and resolved issue for the RDS DatabaseCluster construct, however the fix has not been implemented in the ServelessCluster construct code.

The code for DatabaseCluster will make the clusterIdentifier lower case if the lower case feature flag is set to true, and if there are no unresolved tokens in the name

const clusterIdentifier = FeatureFlags.of(this).isEnabled(cxapi.RDS_LOWERCASE_DB_IDENTIFIER) && !Token.isUnresolved(props.clusterIdentifier)

However, this is not the case for the ServerlessCluster code, which will set the db cluster identifier to lower case regardless of whether there is a token included.

const clusterIdentifier = FeatureFlags.of(this).isEnabled(cxapi.RDS_LOWERCASE_DB_IDENTIFIER)

Adding the unresolved token checker to the ServlessCluster code will resolve this issue.

const clusterIdentifier = FeatureFlags.of(this).isEnabled(cxapi.RDS_LOWERCASE_DB_IDENTIFIER) && !Token.isUnresolved(props.clusterIdentifier)

Additional Information/Context

No response

CDK CLI Version

2.65.0

Framework Version

No response

Node.js Version

v16.16.0

OS

Amazon Linux 2

Language

Typescript

Language Version

No response

Other information

No response

pahud commented 1 year ago

Thank you for pointing out the bug with all the details and yes we definitely should fix this in ServerlessCluster. Are you interested to submit a PR for that?

awsdiegorad commented 1 year ago

PR Created