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

aws-cdk: stack difference count on diff is incorrect #27238

Closed
jchli opened this issue Sep 21, 2023 · 13 comments
Closed

aws-cdk: stack difference count on diff is incorrect #27238

jchli opened this issue Sep 21, 2023 · 13 comments
Assignees
Labels
@aws-cdk/cloudformation-diff bug This issue is a bug. closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. p1 package/tools Related to AWS CDK Tools or CLI

Comments

@jchli
Copy link

jchli commented Sep 21, 2023

Describe the bug

The reported number of stack differences on cdk diff is incorrect

Expected Behavior

A stack with no diffs (e.g. just deployed cleanly) should report 0 differences

Current Behavior

% AWS_PROFILE=non-prod npm run cdk diff my-app -- --exclusively --context myenv=stage --fail

> aws-cdk-infra@0.1.0 cdk
> cdk diff my-app --exclusively --context myenv=stage --fail

[WARNING] aws-cdk-lib.aws_rds.DatabaseClusterProps#instanceProps is deprecated.
  - use writer and readers instead
  This API will be removed in the next major release.
Stack my-app (my-app-stage)
There were no differences

✨  Number of stacks with differences: 2

Reproduction Steps

We're running aws-cdk and aws-cdk-lib on 2.96.2

Our my-app stack includes nested stacks such as aurora postgres, ECR, s3, ECS fargate. I can provide more detailed/redacted CDK code if you aren't able to reproduce

Possible Solution

It seems like this might be related to this: #26796, which was seemingly fixed in 2.93.0.

More precisely, likely something to do with this line which adds up stacks and nested stacks: https://github.com/aws/aws-cdk/pull/26796/files#diff-8cff125f4b53b025add8740ab74f39ee6c20221c0d3ac5ab7db184a6bb077398R154

Additional Information/Context

I just ran into this bug on 2.96.2 but also tested 2.93.0 with the same results.

CDK CLI Version

2.96.2

Framework Version

No response

Node.js Version

18.17.1

OS

Darwin jasonliu macbook 22.4.0 Darwin Kernel Version 22.4.0: Mon Mar 6 21:00:17 PST 2023; root:xnu-8796.101.5~3/RELEASE_X86_64 x86_64

Language

Typescript

Language Version

typescript 5.1.6

Other information

No response

@jchli jchli added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Sep 21, 2023
@github-actions github-actions bot added the package/tools Related to AWS CDK Tools or CLI label Sep 21, 2023
@indrora indrora added p1 @aws-cdk/cloudformation-diff and removed needs-triage This issue or PR still needs to be triaged. labels Sep 21, 2023
@indrora
Copy link
Contributor

indrora commented Sep 21, 2023

Thank you for the report.

@indrora
Copy link
Contributor

indrora commented Sep 27, 2023

@jchli I can partially recreate this. Do you have a minimal example that reliably reproduces this for you? When you pass --fail to cdk diff, do you get a nonzero exit code?

@ChaseWagoner
Copy link
Contributor

@indrora I generated a sample app with npx cdk init sample-app --language=typescript, then modified it to reproduce the issue:

// cdk-test-stack.ts
import { NestedStack, Stack, StackProps } from 'aws-cdk-lib';
import { Construct } from 'constructs';

export class CdkTestStack extends Stack {
  constructor(scope: Construct, id: string, props?: StackProps) {
    super(scope, id, props);
    new TestNestedStack(this, 'CdkTestNestedStack');
  }
}

class TestNestedStack extends NestedStack {
  constructor(scope: Construct, id: string, props?: StackProps) {
    super(scope, id, props);
  }
}

Testing:

$ cdk deploy CdkTestStack # Succeeds, creating both stacks

$ cdk diff --fail
Stack CdkTestStack
There were no differences

✨  Number of stacks with differences: 1

$ echo $?
1

@jortkoopmans
Copy link

I have the same issue, which can be reproduced with the steps described by @ChaseWagoner. One minor fix to the reproduction steps: you need to modify the import at the bin/cdk-testapp.ts to point to correctly point to CdkTestStack (instead of default CdkTestappStack). Or you could alternatively change the class name to match. Then it should synth/deploy OK.

I'm (also) quite certain it is directly related to issue #26818 with PR #26796 , as I have successfully tested with version 2.92.0 where this issue does not occur. As soon as you flip to 2.93.0 or higher (up until 2.104.0), it will be broken.

@jchli
Copy link
Author

jchli commented Dec 4, 2023

hey @SankyRed - any movement on this?

@SankyRed
Copy link
Contributor

SankyRed commented Jan 8, 2024

After testing the issue locally seems like the issue has been fixed. The below image shows when a nested stack has a change which shows the diff as 1.
Screenshot 2024-01-08 at 2 45 58 PM
Whereas after deploying the stack, when we run cdk diff it shows the diff count as 0 and works as expected.
Screenshot 2024-01-08 at 2 46 06 PM
Closing the issue. Thank you!

@SankyRed SankyRed closed this as completed Jan 8, 2024
Copy link

github-actions bot commented Jan 8, 2024

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

@ChaseWagoner
Copy link
Contributor

@SankyRed Out of curiosity, which version of the CDK packages did you use for testing? The minimal test from before still shows an incorrect diff stack count. (I also reproduced the incorrect diff count with a realistic stack containing various resources and nested stacks.)

I still see the suspect line in main branch, which adds the nested stack count as "diffs," seemingly without regard for whether those stacks actually contain a diff:

diffs += stackCount + nestedStackCount;

Setup

  • npx cdk init sample-app --language=typescript
  • Modify cdk-test-stack.ts to match my previous comment

Test 2.92.0

npm i aws-cdk@2.92.0
npm i -D aws-cdk-lib@2.92.0
npm run cdk synth
npm run cdk deploy
npm run cdk diff -- --fail
echo $?

Output shows no diff, as expected:

> cdk-test@0.1.0 cdk
> cdk diff --fail

Stack CdkTestStack
There were no differences

✨  Number of stacks with differences: 0

0 # exit code

Test 2.118.0

npm i aws-cdk@latest
npm i -D aws-cdk-lib@latest
npm ls aws-cdk aws-cdk-lib
# cdk-test@0.1.0
# ├── aws-cdk-lib@2.118.0
# └── aws-cdk@2.118.0
npm run cdk synth
npm run cdk deploy
npm run cdk diff -- --fail
echo $?

Output shows an incorrect diff, including every nested stack in the diff count (even those without diffs):

> cdk-test@0.1.0 cdk
> cdk diff --fail

Stack CdkTestStack
There were no differences

✨  Number of stacks with differences: 1

1 # exit code

@SankyRed
Copy link
Contributor

SankyRed commented Jan 8, 2024

@ChaseWagoner Apologies on that, tested the dependent stack instead of nested stack earlier. When run cdk diff with stacks containing nested stacks with no changes the issue seems to persist. Thanks for calling it out.
Screenshot 2024-01-08 at 4 09 33 PM

@SankyRed SankyRed reopened this Jan 8, 2024
@danny-fairly
Copy link

@SankyRed +1 on feeling the pain on this. Impact is that it makes it difficult to detect a clean diff in CI/CD (e.g. by inspecting the exit code) if there is a nested stack.

@Rondineli
Copy link

I still facing this issue, did anybody manage to fix/bypass it? Running on latest version (2.127.0).

@jchli
Copy link
Author

jchli commented Mar 6, 2024

Hey @SankyRed - any news on this? This blocks us from upgrading to a newer version since we use the diff count as part of our release process.

@SankyRed
Copy link
Contributor

The latest version(v2.133.0) fixes the issue. Please upgrade onto the latest version to apply the necessary fix. Adding a closing-soon tag to give visibility to the community before closing out the issue. Thank you!

Test 2.132.0

npm i aws-cdk@2.132.0
npm i aws-cdk-lib@2.132.0
npm ls aws-cdk aws-cdk-lib
# cdk_test@0.1.0
#  |----- aws-cdk-lib@2.132.0
#  |----- aws-cdk@2.132.0
npm run cdk synth
npm run cdk deploy
npm run cdk diff

Screenshot 2024-03-18 at 11 15 13 AM

Test 2.133.0

npm i aws-cdk@latest
npm i aws-cdk-lib@latest
npm ls aws-cdk aws-cdk-lib
# cdk_test@0.1.0
#  |----- aws-cdk-lib@2.133.0
#  |----- aws-cdk@2.133.0
npm run cdk synth
npm run cdk deploy
npm run cdk diff

Screenshot 2024-03-18 at 11 18 19 AM

@SankyRed SankyRed added the closing-soon This issue will automatically close in 4 days unless further comments are made. label Mar 18, 2024
@github-actions github-actions bot removed the closing-soon This issue will automatically close in 4 days unless further comments are made. label Mar 23, 2024
@github-actions github-actions bot added the closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. label Mar 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/cloudformation-diff bug This issue is a bug. closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. p1 package/tools Related to AWS CDK Tools or CLI
Projects
None yet
Development

No branches or pull requests

8 participants