Skip to content

feat(core): failSynthOnValidationErrors context key to suppress console output and exit code#37909

Merged
mergify[bot] merged 3 commits into
mainfrom
conroy/suppress-validation-exit
May 20, 2026
Merged

feat(core): failSynthOnValidationErrors context key to suppress console output and exit code#37909
mergify[bot] merged 3 commits into
mainfrom
conroy/suppress-validation-exit

Conversation

@kaizencc
Copy link
Copy Markdown
Contributor

@kaizencc kaizencc commented May 18, 2026

Reason for this change

The CDK CLI's validate command runs synthesis and then reads the validation report JSON from the cloud assembly to handle display and failure reporting itself. Currently, synthesis both prints validation results to stderr and sets process.exitCode = 1 on failure — duplicating what the CLI does and causing premature process exit.

The CLI needs a way to tell synthesis: "just write the report, I'll handle the rest."

Description of changes

Introduces @aws-cdk/core:failSynthOnValidationErrors context key in cx-api (so the CLI can depend on it):

  • Default (unset or true): Writes JSON report to cloud assembly, prints human-readable report to stderr, sets process.exitCode = 1 on failure.
  • false: Only writes the JSON report to the cloud assembly. No console output. No exit code. The caller handles reporting and failure.

Other changes:

  • Removes the "Policy Validation Successful!" message — no news is good news.
  • Updates the failure message to: Validation failed. A copy of this report can be found in '<path>'

The JSON report shape is compatible with the new PolicyValidationReportJson types defined in aws/aws-cdk-cli#1515.

Describe any new or updated permissions being added

N/A

Description of how you validated changes

  • All 45 validation unit tests pass
  • Added tests for failSynthOnValidationErrors=false behavior (JSON written, no exit code, no console output)

Checklist


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

@kaizencc kaizencc requested a review from a team as a code owner May 18, 2026 17:56
@mergify mergify Bot added the contribution/core This is a PR that came from AWS. label May 18, 2026
@mergify mergify Bot temporarily deployed to automation May 18, 2026 17:57 Inactive
@github-actions github-actions Bot added the p2 label May 18, 2026
@mergify mergify Bot temporarily deployed to automation May 18, 2026 17:57 Inactive
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 18, 2026

👋 It looks like your PR description references an issue, but not in the expected location.

The issue number must appear in the first section of the description (the first two lines), following the template format:

### Issue # (if applicable)

Closes #123.

Please move your issue reference to the top of the description.

Copy link
Copy Markdown
Collaborator

@aws-cdk-automation aws-cdk-automation left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(This review is outdated)


const POLICY_VALIDATION_FILE_PATH = 'policy-validation-report.json';
const VALIDATION_REPORT_PRETTY_CONTEXT = '@aws-cdk/core:validationReportPrettyPrint';
const VALIDATION_REPORT_ONLY_CONTEXT = '@aws-cdk/core:validationReportOnly';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const VALIDATION_REPORT_ONLY_CONTEXT = '@aws-cdk/core:validationReportOnly';
const FAIL_SYNTH_ON_VALIDATION_ERRORS_CONTEXT = '@aws-cdk/core:failSynthOnValidationErrors';

?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put this in cx-api so that the CLI can officially depend on it.

process.exitCode = 1;
} else {
// eslint-disable-next-line no-console
console.error('Policy Validation Successful!');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not necessary. No news is good news.

Btw, why stderr?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is another instance of me trying to preserve what existed before. It's also the second time you've flagged it, so I'll just remove it :)

const failed = reports.some(r => !r.success);
if (failed) {
// eslint-disable-next-line no-console
console.error(`Validation failed. See the validation report in '${reportFile}' and above for details`);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
console.error(`Validation failed. See the validation report in '${reportFile}' and above for details`);
console.error(`Validation failed. A copy of this report can be found in '${reportFile}'`);

Base automatically changed from conroy/env-var to main May 19, 2026 10:28
@mergify mergify Bot temporarily deployed to automation May 19, 2026 10:28 Inactive
@kaizencc kaizencc added pr-linter/exempt-readme The PR linter will not require README changes pr-linter/exempt-integ-test The PR linter will not require integ test changes labels May 19, 2026
@aws-cdk-automation aws-cdk-automation dismissed their stale review May 19, 2026 18:00

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

Copy link
Copy Markdown
Contributor Author

@kaizencc kaizencc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a test to show with the flag unset and validation errors the resulting error code i 1?

});

test('JSON format only', () => {
test('reportOnly writes JSON but does not print or fail', () => {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not called reportOnly anymore

});

test('JSON report is always written regardless of context', () => {
test('reportOnly succeeds even with validation failures', () => {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not called reportOnly anymore

…onsole output and exit code

Move the validation report context key to cx-api as
FAIL_SYNTH_ON_VALIDATION_ERRORS_CONTEXT so the CLI can depend on it.
When set to false, validation failures only write the JSON report
without printing to stderr or setting a non-zero exit code.

Always write the JSON report to the cloud assembly regardless of the
context key value. Remove the success message ("Policy Validation
Successful!") — no news is good news.
@kaizencc kaizencc force-pushed the conroy/suppress-validation-exit branch from 0c53ea7 to 9deb460 Compare May 19, 2026 18:05
@kaizencc kaizencc changed the title feat(core): validationReportOnly context key to suppress console output and exit failures feat(core): failSynthOnValidationErrors context key to suppress console output and exit code May 19, 2026
@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented May 20, 2026

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented May 20, 2026

Merge Queue Status

  • Entered queue2026-05-20 06:59 UTC · Rule: default-squash
  • Checks passed · in-place
  • Merged2026-05-20 07:46 UTC · at e2564173461cc7f24e5562ae3dd3bb1bd920616c · squash

This pull request spent 47 minutes 45 seconds in the queue, including 47 minutes 22 seconds running CI.

Required conditions to merge

@mergify mergify Bot temporarily deployed to automation May 20, 2026 06:59 Inactive
@mergify mergify Bot temporarily deployed to automation May 20, 2026 06:59 Inactive
@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented May 20, 2026

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@mergify mergify Bot merged commit deb968f into main May 20, 2026
18 of 19 checks passed
@mergify mergify Bot deleted the conroy/suppress-validation-exit branch May 20, 2026 07:46
@github-actions
Copy link
Copy Markdown
Contributor

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

@github-actions github-actions Bot locked as resolved and limited conversation to collaborators May 20, 2026
@aws-cdk-automation aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label May 20, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

contribution/core This is a PR that came from AWS. p2 pr-linter/exempt-integ-test The PR linter will not require integ test changes pr-linter/exempt-readme The PR linter will not require README changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants