when using npm run cdk ... parameters are eaten by npm unless npm run cdk -- ... is used.

[l0b0]

Describe the bug

cdk diff --fail doesn't do anything, whether it's set to true, false, or nothing.

Expected Behavior

--fail should work exactly like git diff --exit-code - if there is a diff, the exit code should be non-zero.

Current Behavior

The exit code is always zero, making the flag useless.

Reproduction Steps

See log below.

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.93.0 (build 724bd01)

Framework Version

No response

Node.js Version

v18.17.1

OS

NixOS

Language

Typescript

Language Version

5.2.2

Other information

Log:

$ npm run cdk diff --fail=true

> @linz/hydro-aws-ci-infrastructure@0.1.0 cdk
> cdk diff

Stack ContinuousIntegration
Resources
[~] AWS::IAM::ManagedPolicy CiExecPolicy CiExecPolicyD0969051 
 └─ [~] PolicyDocument
     └─ [~] .Statement:
         └─ @@ -33,7 +33,6 @@
            [ ] "sqs:*",
            [ ] "ssm:*",
            [ ] "states:*",
            [-] "tag:*",
            [ ] "waf-regional:*",
            [ ] "waf:*",
            [ ] "wafv2:*"


✨  Number of stacks with differences: 1

$ echo $?
0
$ npm run cdk diff --fail=false

> @linz/hydro-aws-ci-infrastructure@0.1.0 cdk
> cdk diff

Stack ContinuousIntegration
Resources
[~] AWS::IAM::ManagedPolicy CiExecPolicy CiExecPolicyD0969051 
 └─ [~] PolicyDocument
     └─ [~] .Statement:
         └─ @@ -33,7 +33,6 @@
            [ ] "sqs:*",
            [ ] "ssm:*",
            [ ] "states:*",
            [-] "tag:*",
            [ ] "waf-regional:*",
            [ ] "waf:*",
            [ ] "wafv2:*"


✨  Number of stacks with differences: 1

$ echo $?
0

[indrora]

This is a pretty clear issue. Thanks for reporting.

[indrora]

Doing a deep dive on this, there looks to be a deeper problem here.

The actual CLI call into diff is made at

aws-cdk/packages/aws-cdk/lib/cli.ts

Line 485 in a2a4f68

case 'diff':

This is partly controlled by the feature flag aws-cdk:enabledDiffNoFail

aws-cdk/packages/aws-cdk-lib/cx-api/lib/features.ts

Line 115 in a2a4f68

[ENABLE_DIFF_NO_FAIL_CONTEXT]: {

This defaults to 'true'. This gets a weird ternary dance in the configuration of cdk diff:

 fail: args.fail != null ? args.fail : !enableDiffNoFail,

I'm assuming that passing --fail sets this to some value while --no-fail sets it to some other value? False? NaN? Undefined? FileNotFound?

I don't know how falsy/truthy null vs other values can be in Typescript, but it appears that the rest of the codebase compares against undefined rather than null and this is causing some level of type confusion?

[l0b0]

Actually, this flag has a lot of UX issues! I might file separate issues for these, but I'll just make sure to record them here for reference.

Why is a nonzero exit code not the default if diff finds a difference (reported)? It's literally more information for the end user, and anyone who doesn't care about the exit code could just do || [ $? -eq 1 ] (except they can't, see below). It's also the default in the most ubiquitous diffing command in existence, diff itself.

cdk diff --fail does not have its own exit code (reported). For example:

$ ./node_modules/.bin/cdk diff --fail
[omitted, no diff printed here]
The security token included in the request is expired
$ echo $?
1

Even passing a non-existing flag results in the same exit code (reported):

$ ./node_modules/.bin/cdk diff --wtf
[no mention of the `--wtf` flag here!]
The security token included in the request is expired

This means anyone who wants to react to an actual diff has to parse the output instead of checking the exit code. An exit code of 1 could mean basically anything, making the flag almost completely useless.

--fail is IMO a terrible name. Any discussion around this flag is basically guaranteed to have a bunch of double negatives. A more explicit flag would be helpful to discover and discuss this feature.

[l0b0]

What a crock. It turns out that

1. npm run needs a -- between the command and hyphenated options. (Otherwise it presumably parses them as its own?) You
2. npm run silently ignores flags it doesn't know about.

The end result is that npm run cdk diff --fail is probably equivalent to npm run --fail cdk diff. WTF?!

You can see the core of the issue by observing that the command is printed without the flag:

> cdk diff

npm run cdk diff -- --fail does do the right thing, actually returning a nonzero exit code in case of a difference. It still has terrible UX though, so I'll make sure to report those.

[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.

[indrora]

I'm going to leave this open so that we have a tracking issue at least.

This may be a known interaction with npm, but we should at least be aware of it since we do suggest the use of npx and similar, so this warrants us at least looking into this issue to make sure our customers know about it. Thank you @l0b0 for doing the extensive legwork.