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

(cli): cdk diff pessimizes replacement if expression changes #21164

Open
asnaseer-resilient opened this issue Jul 15, 2022 · 1 comment
Open
Labels
@aws-cdk/core Related to core CDK functionality bug This issue is a bug. effort/medium Medium work item – several days of effort p2

Comments

@asnaseer-resilient
Copy link

asnaseer-resilient commented Jul 15, 2022

Describe the bug

I am trying to use cloudformation_include to import a serverless framework deployed RDS stack into the CDK v2.25.0. I followed the instructions in this article https://snow-dev.com/posts/migrate-from-serverless-to-cdk.html

However, when I do a cdk diff on this I seem to get an indication that the DB will be replaced, i.e. I get this within the output:

Resources
[~] AWS::RDS::DBCluster RdsTemplate/InsightsDbCluster InsightsDbCluster replace
 └─ [~] DatabaseName (requires replacement)
     └─ [~] .Fn::Join:
         └─ @@ -1,9 +1,11 @@
            [ ] [
            [ ]   "_",
            [-]   {
            [-]     "Fn::Split": [
            [-]       "-",
            [-]       "sn-control-insights-rds-dev-insights"
            [-]     ]
            [-]   }
            [+]   [
            [+]     "sn",
            [+]     "control",
            [+]     "insights",
            [+]     "rds",
            [+]     "dev",
            [+]     "insights"
            [+]   ]
            [ ] ]
[~] AWS::RDS::DBInstance RdsTemplate/InsightsDbInstance1 InsightsDbInstance1 replace
 └─ [~] DBClusterIdentifier (requires replacement)
     └─ [~] .Ref:
         ├─ [-] InsightsDbCluster
         └─ [+] InsightsDbCluster (replaced)
[~] AWS::RDS::DBInstance RdsTemplate/InsightsDbInstance2 InsightsDbInstance2 replace
 └─ [~] DBClusterIdentifier (requires replacement)
     └─ [~] .Ref:
         ├─ [-] InsightsDbCluster
         └─ [+] InsightsDbCluster (replaced)

The template JSON file that I supplied defines the DatabaseName as:

        "DatabaseName": {
          "Fn::Join": [
            "_",
            {
              "Fn::Split": [
                "-",
                "sn-control-insights-rds-dev-insights"
              ]
            }
          ]
        },

It looks like the CDK synthesises this as:

      DatabaseName:
        Fn::Join:
          - _
          - - sn
            - control
            - insights
            - rds
            - dev
            - insights

which (I think) then causes it to incorrectly believe that the RDS will need to be redeployed.

If I run cdk deploy it correctly preserves the RDS. So it looks like it is only cdk diff that is reporting this incorrectly.

Expected Behavior

I expected cdk diff to not report that the RDS will be replaced.

Current Behavior

cdk diff incorrectly reports that the RDS will be replaced (as described above). However, cdk deploy behaves correctly in that it preserves the existing RDS and all the data within it.

Reproduction Steps

All details are provided above.

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.25.0

Framework Version

No response

Node.js Version

v14.17.5

OS

macOS Monterey Version 12.4

Language

Typescript

Language Version

TypeScript v4.6.4

Other information

Originally reported in cdk slack channel - https://cdk-dev.slack.com/archives/C018XT6REKT/p1657784736307229

Adam Ruka said this in this slack channel:
I've seen this with Fn::Select before, and we fixed it in #19680, but looks like we need to do the same for Fn::Split!

@asnaseer-resilient asnaseer-resilient added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jul 15, 2022
@github-actions github-actions bot added the @aws-cdk/aws-rds Related to Amazon Relational Database label Jul 15, 2022
@corymhall corymhall assigned rix0rrr and unassigned corymhall Jul 15, 2022
@corymhall corymhall added @aws-cdk/core Related to core CDK functionality and removed @aws-cdk/aws-rds Related to Amazon Relational Database labels Jul 15, 2022
@skinny85
Copy link
Contributor

This is very similar to the problem we had with CDK short-circuiting Fn::Select in #19680, but this time with Fn::Split. The solution will also probably be similar.

@rix0rrr rix0rrr changed the title Fn::Split: Incorrectly short-circuits complex expressions (cli): cdk diff pessimizes replacement if expression changes Jul 25, 2022
@rix0rrr rix0rrr added effort/medium Medium work item – several days of effort p2 and removed needs-triage This issue or PR still needs to be triaged. labels Jul 25, 2022
@rix0rrr rix0rrr removed their assignment Jul 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/core Related to core CDK functionality bug This issue is a bug. effort/medium Medium work item – several days of effort p2
Projects
None yet
Development

No branches or pull requests

4 participants