Skip to content

Commit 016e206

Browse files
authored
chore: in case of policy validation failures, don't throw (#24872)
Instead, print the report normally and exit with status code 1. This makes for a cleaner output to the user. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 96f6747 commit 016e206

File tree

2 files changed

+32
-30
lines changed

2 files changed

+32
-30
lines changed

packages/@aws-cdk/core/lib/private/synthesis.ts

+9-2
Original file line numberDiff line numberDiff line change
@@ -139,15 +139,22 @@ function invokeValidationPlugins(root: IConstruct, outdir: string, assembly: Clo
139139
? formatter.formatJson(reports)
140140
: formatter.formatPrettyPrinted(reports);
141141

142+
const reportFile = path.join(assembly.directory, POLICY_VALIDATION_FILE_PATH);
142143
if (formatJson) {
143-
fs.writeFileSync(path.join(assembly.directory, POLICY_VALIDATION_FILE_PATH), JSON.stringify(output, undefined, 2));
144+
fs.writeFileSync(reportFile, JSON.stringify(output, undefined, 2));
144145
} else {
145146
// eslint-disable-next-line no-console
146147
console.error(output);
147148
}
148149
const failed = reports.some(r => !r.success);
149150
if (failed) {
150-
throw new Error('Validation failed. See the validation report above for details');
151+
const message = formatJson
152+
? `Validation failed. See the validation report in '${reportFile}' for details`
153+
: 'Validation failed. See the validation report above for details';
154+
155+
// eslint-disable-next-line no-console
156+
console.log(message);
157+
process.exitCode = 1;
151158
} else {
152159
// eslint-disable-next-line no-console
153160
console.log('Policy Validation Successful!');

packages/@aws-cdk/core/test/validation/validation.test.ts

+23-28
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ let consoleLogMock: jest.SpyInstance;
1212
beforeEach(() => {
1313
consoleErrorMock = jest.spyOn(console, 'error').mockImplementation(() => { return true; });
1414
consoleLogMock = jest.spyOn(console, 'log').mockImplementation(() => { return true; });
15+
process.exitCode = undefined;
1516
});
1617

1718
afterEach(() => {
@@ -45,9 +46,9 @@ describe('validations', () => {
4546
result: 'failure',
4647
},
4748
});
48-
expect(() => {
49-
app.synth();
50-
}).toThrow(/Validation failed/);
49+
50+
app.synth();
51+
expect(process.exitCode).toEqual(1);
5152

5253
expect(consoleErrorMock.mock.calls[0][0].split('\n')).toEqual(expect.arrayContaining(validationReport([{
5354
templatePath: '/path/to/Default.template.json',
@@ -88,9 +89,10 @@ describe('validations', () => {
8889
result: 'success',
8990
},
9091
});
91-
expect(() => {
92-
app.synth();
93-
}).not.toThrow(/Validation failed/);
92+
93+
app.synth();
94+
expect(process.exitCode).toBeUndefined();
95+
9496
expect(consoleLogMock.mock.calls).toEqual([
9597
[
9698
expect.stringMatching(/Performing Policy Validations/),
@@ -144,9 +146,9 @@ Policy Validation Report Summary
144146
result: 'failure',
145147
},
146148
});
147-
expect(() => {
148-
app.synth();
149-
}).toThrow(/Validation failed/);
149+
150+
app.synth();
151+
expect(process.exitCode).toEqual(1);
150152

151153
const report = consoleErrorMock.mock.calls[0][0];
152154
// Assuming the rest of the report's content is checked by another test
@@ -228,9 +230,9 @@ Policy Validation Report Summary
228230
result: 'failure',
229231
},
230232
});
231-
expect(() => {
232-
app.synth();
233-
}).toThrow(/Validation failed/);
233+
234+
app.synth();
235+
expect(process.exitCode).toEqual(1);
234236

235237
const report = consoleErrorMock.mock.calls[0][0].split('\n');
236238
// Assuming the rest of the report's content is checked by another test
@@ -422,9 +424,8 @@ Policy Validation Report Summary
422424
const stack = new core.Stack(app);
423425
new LevelTwoConstruct(stack, 'SomeResource');
424426
new LevelTwoConstruct(stack, 'AnotherResource');
425-
expect(() => {
426-
app.synth();
427-
}).toThrow(/Validation failed/);
427+
app.synth();
428+
expect(process.exitCode).toEqual(1);
428429

429430
const report = consoleErrorMock.mock.calls[0][0];
430431
// Assuming the rest of the report's content is checked by another test
@@ -462,9 +463,8 @@ Policy Validation Report Summary
462463
result: 'failure',
463464
},
464465
});
465-
expect(() => {
466-
app.synth();
467-
}).toThrow(/Validation failed/);
466+
app.synth();
467+
expect(process.exitCode).toEqual(1);
468468

469469
const report = consoleErrorMock.mock.calls[0][0].split('\n');
470470
expect(report).toEqual(expect.arrayContaining(
@@ -523,9 +523,7 @@ Policy Validation Report Summary
523523
result: 'failure',
524524
},
525525
});
526-
expect(() => {
527-
app.synth();
528-
}).toThrow(/Validation failed/);
526+
app.synth();
529527

530528
const report = consoleErrorMock.mock.calls[0][0].split('\n');
531529
expect(report).toEqual(expect.arrayContaining(
@@ -588,9 +586,8 @@ Policy Validation Report Summary
588586
},
589587
});
590588

591-
expect(() => {
592-
app.synth();
593-
}).toThrow(/Validation failed/);
589+
app.synth();
590+
expect(process.exitCode).toEqual(1);
594591

595592
const report = consoleErrorMock.mock.calls[0][0];
596593
expect(report).toContain('error: Validation plugin \'broken-plugin\' failed: Something went wrong');
@@ -640,9 +637,8 @@ Policy Validation Report Summary
640637
result: 'failure',
641638
},
642639
});
643-
expect(() => {
644-
app.synth();
645-
}).toThrow(/Validation failed/);
640+
app.synth();
641+
expect(process.exitCode).toEqual(1);
646642

647643
const report = fs.readFileSync(path.join(app.outdir, 'policy-validation-report.json')).toString('utf-8');
648644
expect(JSON.parse(report)).toEqual(expect.objectContaining({
@@ -785,7 +781,6 @@ const validationReport = (data: ValidationReportData[]) => {
785781
expect.stringMatching(new RegExp(' > test-location')),
786782
expect.stringMatching(new RegExp(` Description: ${d.description ?? 'test recommendation'}`)),
787783
...d.ruleMetadata ? [expect.stringMatching(' Rule Metadata:'), ...Object.entries(d.ruleMetadata).flatMap(([key, value]) => expect.stringMatching(`${key}: ${value}`))] : [],
788-
// new RegExp(''),
789784
];
790785
}
791786
return [];

0 commit comments

Comments
 (0)