(aws-rds): when @aws-cdk/aws-rds:lowercaseDbIdentifier is true (which is the default in v2), tokens in the clusterIdentifier property get mutated and thus are not replaced during synthesis

[saltman424]

What is the problem?

The issue has to do with these lines (possibly more, but these are the only two that I saw):

aws-cdk/packages/@aws-cdk/aws-rds/lib/cluster.ts

Line 361 in 323281e

? props.clusterIdentifier?.toLowerCase()

aws-cdk/packages/@aws-cdk/aws-rds/lib/instance.ts

Line 716 in 323281e

? props.instanceIdentifier?.toLowerCase()

The .toLowerCase messes up the tokens.

Reproduction Steps

new rds.DatabaseCluster(scope, 'DB', {
  clusterIdentifier: `my-${cdk.Lazy.string({ produce(): string { return 'prod' }})}-db-cluster`,
  engine: rds.DatabaseClusterEngine.auroraPostgres({ version: rds.AuroraPostgresEngineVersion.VER_13_4 }),
  instanceProps: { vpc }
})

What did you expect to happen?

Cluster identifier = my-prod-db-cluster

What actually happened?

Cluster identifier = my-${token[token.1234]}-db-cluster

CDK CLI Version

2.8.0

Framework Version

2.8.0

Node.js Version

16.13.1

OS

Microsoft Windows 10 Enterprise

Language

Typescript

Language Version

4.5.5

Other information

I am not sure how this should be handled, but some options are:

• Simply ignore this feature when there is a token in the cluster identifier by adding && !cdk.Token.isUnresolved(clusterIdentifier) to the condition that checks if the feature flag is enabled
• Print a warning when a token is in the cluster identifier, but still skip the toLowerCase method in that case
• Convert everything besides the token(s) to lower case (I am not sure if there is a simple way to identify which parts of a string are tokens and which aren't)

[skinny85]

Thanks for opening the issue @saltman424!

I am not sure how this should be handled, but some options are:

• Simply ignore this feature when there is a token in the cluster identifier by adding && !cdk.Token.isUnresolved(clusterIdentifier) to the condition that checks if the feature flag is enabled

I think this is the best solution - we can't really lowercase deploy-time values anyway.

Would you consider opening us a PR fixing this? Here's our "Contributing" guide: https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md.

Thanks,
Adam

[github-actions]

⚠️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.