feat(cli): always exit with 0 on cdk diff (under feature flag) #4721

jogold · 2019-10-28T14:33:29Z

Use cdk diff --fail to exit with 1 if there's a diff.

Related to #4650 and #4708

BREAKING CHANGE: cdk diffnow exits with 0 even when there's a diff, use --fail to exit with 1. To enable this feature for old projects, add the context key "aws-cdk:diffNoFail": "true" in your cdk.json file.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

Use `cdk diff --no-fail` to exit with 0 even when there is a diff. To avoid a CLI breaking change, the default remains to fail on diff. Related to aws#4650 and aws#4708

mergify · 2019-10-28T14:34:12Z

Thanks so much for taking the time to contribute to the AWS CDK ❤️

We will shortly assign someone to review this pull request and help get it
merged. In the meantime, please take a minute to make sure you follow this
checklist:

PR title type(scope): text
- type: fix, feat, refactor go into CHANGELOG, chore is hidden
- scope: name of module without aws- or cdk- prefix or postfix (e.g. s3 instead of aws-s3-deployment)
- text: use all lower-case, do not end with a period, do not include issue refs
PR Description
- Rationale: describe rationale of change and approach taken
- Issues: indicate issues fixed via: fixes #xxx or closes #xxx
- Breaking?: last paragraph: BREAKING CHANGE: <describe what changed + link for details>
Testing
- Unit test added. Prefer to add a new test rather than modify existing tests
- CLI or init templates change? Re-run/add CLI integration tests
Documentation
- README: update module README to describe new features
- API docs: public APIs must be documented. Copy from official AWS docs when possible
- Design: for significant features, follow design process

mergify · 2019-10-28T14:34:12Z

Thanks so much for taking the time to contribute to the AWS CDK ❤️

We will shortly assign someone to review this pull request and help get it
merged. In the meantime, please take a minute to make sure you follow this
checklist:

PR title type(scope): text
- type: fix, feat, refactor go into CHANGELOG, chore is hidden
- scope: name of module without aws- or cdk- prefix or postfix (e.g. s3 instead of aws-s3-deployment)
- text: use all lower-case, do not end with a period, do not include issue refs
PR Description
- Rationale: describe rationale of change and approach taken
- Issues: indicate issues fixed via: fixes #xxx or closes #xxx
- Breaking?: last paragraph: BREAKING CHANGE: <describe what changed + link for details>
Testing
- Unit test added. Prefer to add a new test rather than modify existing tests
- CLI or init templates change? Re-run/add CLI integration tests
Documentation
- README: update module README to describe new features
- API docs: public APIs must be documented. Copy from official AWS docs when possible
- Design: for significant features, follow design process

aws-cdk-automation · 2019-10-28T15:12:15Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2019-11-11T21:46:37Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

shivlaks · 2019-11-12T01:55:47Z

now that we have the feature-flags I think we should leverage it to introduce the capability you initially wanted to introduce in #4650

thoughts?

…-fail

jogold · 2019-11-12T15:27:52Z

@eladb can you have a look at my first attempt to use the feature flags (as suggested by @shivlaks)?

aws-cdk-automation · 2019-11-12T15:54:06Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

eladb · 2019-11-12T16:25:31Z

packages/aws-cdk/bin/cdk.ts

@@ -185,7 +187,8 @@ async function initCommandLine() {
          exclusively: args.exclusively,
          templatePath: args.template,
          strict: args.strict,
-          contextLines: args.contextLines
+          contextLines: args.contextLines,
+          fail: args.fail || !configuration.context.get(cxapi.ENABLE_DIFF_NO_FAIL),


Will this also work if this is defined in in cdk.context.json?

Yes:

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

Lines 73 to 76 in 6fa4713

this.context = new Context(

this.commandLineContext,

this.projectConfig.subSettings([CONTEXT_KEY]).makeReadOnly(),

this.projectContext);

DISABLE_DIFF_FAIL instead of ENABLE_DIFF_NO_FAIL, I'm not very happy with the name here...

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

mergify · 2019-11-28T06:48:32Z

Thank you for contributing! Your pull request is now being automatically merged.

mergify · 2019-11-28T06:48:32Z

Thank you for contributing! Your pull request is now being automatically merged.

aws-cdk-automation · 2019-11-28T07:18:34Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

…7401) Rationale: we missed this entry from #4721 presumably because there isn't a newline after `BREAKING CHANGE:` before the rest of the text in the commit body

In v1, the original behavior was that `cdk diff` exited 0 on a clean diff (no changes), and exited 1 on changes. #4721 introduced a new feature flag to always have `cdk diff` exit 0. As is standard, the "default" for the flag was `false`, but the future behavior was `true`, meaning any new CDK project created after that release (v1.19.0) defaulted to always exiting 0. In v2, the feature flag was expired, meaning users could no longer set it. Typically, the behavior (`FeatureFlags.isEnabled`) checks and errors if an expired flag is explicitly set, and always returns true for expired flags otherwise. However, the CDK CLI helper function did not contain this functionality. That means for current v2 projects, the v1-ported default value is used (false). This means `cdk diff` *always* exits 1 when there's a diff, and there's no way for a user to disable this behavior. This change updates the CLI behavior to default to true for expired flags, same as any other feature flag check; this means `cdk diff` will return to exiting 0, as v1 apps have been doing for years. The `FeatureFlags.isEnabled` method can't be used, as it requires a Construct node. I also took the liberty of explicitly setting the "default" to true for all expired flags, so any other direct access of the flag (not using the FeatureFlag library) will have the same consistent behavior.

feat(cli): add --no-fail option to diff

62dc470

Use `cdk diff --no-fail` to exit with 0 even when there is a diff. To avoid a CLI breaking change, the default remains to fail on diff. Related to aws#4650 and aws#4708

jogold requested a review from RomainMuller as a code owner October 28, 2019 14:33

jogold mentioned this pull request Oct 28, 2019

fix(cli): always exit with 0 on cdk diff #4650

Merged

SomayaB assigned shivlaks Oct 28, 2019

Merge branch 'master' into diff-no-fail

5eb0b7c

jogold added 3 commits November 12, 2019 10:21

Merge branch 'master' into diff-no-fail

e2a2f62

use feature flag

838e464

Merge branch 'diff-no-fail' of github.com:jogold/aws-cdk into diff-no…

a41f4cd

…-fail

jogold requested a review from eladb as a code owner November 12, 2019 15:26

jogold changed the title ~~feat(cli): add --no-fail option to diff~~ feat(cli): always exit with 0 on cdk diff (under feature flag) Nov 12, 2019

eladb reviewed Nov 12, 2019

View reviewed changes

eladb approved these changes Nov 28, 2019

View reviewed changes

packages/aws-cdk/lib/cdk-toolkit.ts Show resolved Hide resolved

Merge branch 'master' into diff-no-fail

f7dcec8

mergify bot merged commit 3ffd810 into aws:master Nov 28, 2019

jogold deleted the diff-no-fail branch November 28, 2019 08:39

shivlaks mentioned this pull request Dec 12, 2019

Make cdk diff CI/CD friendly #5392

Closed

2 tasks

This was referenced Apr 17, 2020

Running "cdk diff" returns error code 1 #5885

Closed

chore: add missing breaking change entry into CHANGELOG for v1.19.0 #7401

Merged

njlynch mentioned this pull request Dec 6, 2021

fix(aws-cdk): cdk diff always fails on diff #17862

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(cli): always exit with 0 on cdk diff (under feature flag) #4721

feat(cli): always exit with 0 on cdk diff (under feature flag) #4721

jogold commented Oct 28, 2019 •

edited

Loading

mergify bot commented Oct 28, 2019

mergify bot commented Oct 28, 2019

aws-cdk-automation commented Oct 28, 2019

aws-cdk-automation commented Nov 11, 2019

shivlaks commented Nov 12, 2019

jogold commented Nov 12, 2019 •

edited

Loading

aws-cdk-automation commented Nov 12, 2019

eladb Nov 12, 2019

jogold Nov 12, 2019

jogold Nov 12, 2019

mergify bot commented Nov 28, 2019

mergify bot commented Nov 28, 2019

aws-cdk-automation commented Nov 28, 2019

	this.context = new Context(
	this.commandLineContext,
	this.projectConfig.subSettings([CONTEXT_KEY]).makeReadOnly(),
	this.projectContext);

feat(cli): always exit with 0 on cdk diff (under feature flag) #4721

feat(cli): always exit with 0 on cdk diff (under feature flag) #4721

Conversation

jogold commented Oct 28, 2019 • edited Loading

mergify bot commented Oct 28, 2019

mergify bot commented Oct 28, 2019

aws-cdk-automation commented Oct 28, 2019

AWS CodeBuild CI Report

aws-cdk-automation commented Nov 11, 2019

AWS CodeBuild CI Report

shivlaks commented Nov 12, 2019

jogold commented Nov 12, 2019 • edited Loading

aws-cdk-automation commented Nov 12, 2019

AWS CodeBuild CI Report

eladb Nov 12, 2019

Choose a reason for hiding this comment

jogold Nov 12, 2019

Choose a reason for hiding this comment

jogold Nov 12, 2019

Choose a reason for hiding this comment

mergify bot commented Nov 28, 2019

mergify bot commented Nov 28, 2019

aws-cdk-automation commented Nov 28, 2019

AWS CodeBuild CI Report

jogold commented Oct 28, 2019 •

edited

Loading

jogold commented Nov 12, 2019 •

edited

Loading