From 571d3b8f6174018d53d859e0a7293da934140e9b Mon Sep 17 00:00:00 2001 From: Ilianna Papastefanou Date: Fri, 13 May 2022 19:01:03 +0100 Subject: [PATCH] feat: include thrown errors in IaC failures section When we have mixed results, eg some paths were successful, but some threw errors, we are now displaying them in the Test Failures section. --- src/cli/commands/test/iac/index.ts | 25 ++-- .../formatters/iac-output/v2/failures/list.ts | 7 +- src/lib/formatters/iac-output/v2/types.ts | 5 + test/jest/acceptance/iac/iac-output.spec.ts | 118 ++---------------- .../v2/failures/fixtures/test-failures.json | 7 +- .../iac-output/v2/failures/list.spec.ts | 8 +- 6 files changed, 41 insertions(+), 129 deletions(-) diff --git a/src/cli/commands/test/iac/index.ts b/src/cli/commands/test/iac/index.ts index dcfb8179fa4..9486da2cac4 100644 --- a/src/cli/commands/test/iac/index.ts +++ b/src/cli/commands/test/iac/index.ts @@ -58,6 +58,7 @@ import { import config from '../../../../lib/config'; import { UnsupportedEntitlementError } from '../../../../lib/errors/unsupported-entitlement-error'; import * as ora from 'ora'; +import { IaCTestFailure } from '../../../../lib/formatters/iac-output/v2/types'; const debug = Debug('snyk-test'); const SEPARATOR = '\n-------------------------------------------------------\n'; @@ -81,7 +82,7 @@ export default async function( const results = [] as any[]; // Holds an array of scanned file metadata for output. - let iacScanFailures: IacFileInDirectory[] | undefined; + let iacScanFailures: IacFileInDirectory[] = []; let iacIgnoredIssuesCount = 0; let iacOutputMeta: IacOutputMeta | undefined; @@ -130,20 +131,16 @@ export default async function( }; res = results; - iacScanFailures = failures; + iacScanFailures = failures ? failures : []; iacIgnoredIssuesCount += ignoreCount; } catch (error) { - // TODO: Check if we really need this anymore. - // All errors that we throw are of Error type, so we shouldn't be needing this check/conversion. res = formatTestError(error); - // we can store all errors here for printing the paths and errors later in the new output. } // Not all test results are arrays in order to be backwards compatible // with scripts that use a callback with test. Coerce results/errors to be arrays // and add the result options to each to be displayed // TODO: Similarly to above, do we actually need to convert this to an array? - // I think we do not need this in IaC const resArray: any[] = Array.isArray(res) ? res : [res]; for (let i = 0; i < resArray.length; i++) { @@ -273,11 +270,23 @@ export default async function( let summaryMessage = ''; let errorResultsLength = errorResults.length; - if (iacScanFailures?.length) { + if (iacScanFailures.length || hasErrors) { errorResultsLength = iacScanFailures.length || errorResults.length; + const thrownErrors: IaCTestFailure[] = errorResults.map((err) => ({ + filePath: err.path, + failureReason: err.message, + })); + + const allTestFailures: IaCTestFailure[] = iacScanFailures + .map((f) => ({ + filePath: f.filePath, + failureReason: f.failureReason, + })) + .concat(thrownErrors); + response += isNewIacOutputSupported - ? EOL.repeat(2) + formatIacTestFailures(iacScanFailures) + ? EOL.repeat(2) + formatIacTestFailures(allTestFailures) : iacScanFailures .map((reason) => chalk.bold.red(getIacDisplayErrorFileOutput(reason))) .join(''); diff --git a/src/lib/formatters/iac-output/v2/failures/list.ts b/src/lib/formatters/iac-output/v2/failures/list.ts index 33fdc4dcee6..b813470446e 100644 --- a/src/lib/formatters/iac-output/v2/failures/list.ts +++ b/src/lib/formatters/iac-output/v2/failures/list.ts @@ -2,8 +2,9 @@ import { EOL } from 'os'; import { IacFileInDirectory } from '../../../../types'; import { colors, contentPadding } from '../utils'; +import { IaCTestFailure } from '../types'; -export function formatIacTestFailures(testFailures: IacFileInDirectory[]) { +export function formatIacTestFailures(testFailures: IaCTestFailure[]): string { const sectionComponents: string[] = []; const titleOutput = colors.title(`Test Failures`); @@ -20,7 +21,7 @@ interface TestFailuresByFailureReason { } function groupTestFailuresByFailureReason( - testFailures: IacFileInDirectory[], + testFailures: IaCTestFailure[], ): TestFailuresByFailureReason { return testFailures.reduce((groupedFailures, failure) => { const reason = failure.failureReason; @@ -36,7 +37,7 @@ function groupTestFailuresByFailureReason( }, {}); } -function formatFailuresList(testFailures: IacFileInDirectory[]) { +function formatFailuresList(testFailures: IaCTestFailure[]) { const testFailuresByReason = groupTestFailuresByFailureReason(testFailures); return Object.entries(testFailuresByReason) .map(([failureReason, testFailures]) => diff --git a/src/lib/formatters/iac-output/v2/types.ts b/src/lib/formatters/iac-output/v2/types.ts index 90acb2dfb57..cc2bf5b5ebf 100644 --- a/src/lib/formatters/iac-output/v2/types.ts +++ b/src/lib/formatters/iac-output/v2/types.ts @@ -6,3 +6,8 @@ export interface IacTestData { results: IacTestResponse[]; failures?: IacFileInDirectory[]; } + +export type IaCTestFailure = { + filePath: string; + failureReason: string | undefined; +}; diff --git a/test/jest/acceptance/iac/iac-output.spec.ts b/test/jest/acceptance/iac/iac-output.spec.ts index 412b62b3630..7e11b19c962 100644 --- a/test/jest/acceptance/iac/iac-output.spec.ts +++ b/test/jest/acceptance/iac/iac-output.spec.ts @@ -39,37 +39,25 @@ describe('iac test output', () => { }); it('should show the IaC test title', async () => { - // Arrange const dirPath = './iac/terraform'; - - // Act const { stdout } = await run(`snyk iac test ${dirPath}`); - // Assert expect(stdout).toContain('Snyk Infrastructure as Code'); }); it('should show the spinner message', async () => { - // Arrange const dirPath = './iac/terraform'; - - // Act const { stdout } = await run(`snyk iac test ${dirPath}`); - // Arrange expect(stdout).toContain( 'Snyk testing Infrastructure as Code configuration issues.', ); }); it('should show the test completion message', async () => { - // Arrange const dirPath = './iac/terraform'; - - // Act const { stdout } = await run(`snyk iac test ${dirPath}`); - // Assert expect(stdout).toContain('Test completed.'); }); @@ -96,11 +84,8 @@ describe('iac test output', () => { }); it('should show the test summary section with correct values', async () => { - // Arrange const dirPath = 'iac/kubernetes'; const policyPath = `iac/policy/.snyk`; - - // Act const { stdout } = await run( `snyk iac test ${dirPath} --policy-path=${policyPath}`, ); @@ -122,37 +107,24 @@ describe('iac test output', () => { }); it('should not show the file meta sections', async () => { - // Arrange const dirPath = 'iac/arm'; - - // Act const { stdout } = await run(`snyk iac test ${dirPath}`); - - // Assert expect(stdout).not.toContain(` Type: ARM Target file: ${dirPath}/`); }); it('should not show the file summary messages', async () => { - // Arrange const dirPath = 'iac/terraform'; - - // Act const { stdout } = await run(`snyk iac test ${dirPath}`); - // Assert expect(stdout).not.toContain(`Tested ${dirPath} for known issues`); }); it('should not show the test failures section', async () => { - // Arrange const dirPath = 'iac/only-valid'; - - // Act const { stdout } = await run(`snyk iac test ${dirPath}`); - // Assert expect(stdout).not.toContain('Test Failures'); }); @@ -176,13 +148,11 @@ Target file: ${dirPath}/`); describe('with multiple test results', () => { describe('with test failures', () => { it('should show the failures list section with the correct values', async () => { - // Arrange const dirPath = 'iac'; + const { stdout } = await run( + `snyk iac test ${dirPath} my-imaginary-file.tf my-imaginary-directory/`, + ); - // Act - const { stdout } = await run(`snyk iac test ${dirPath}`); - - // Assert expect(stdout).toContain( ' Failed to parse JSON file' + EOL + @@ -218,18 +188,22 @@ Target file: ${dirPath}/`); 'iac', 'terraform', 'sg_open_ssh_invalid_hcl2.tf', - )}`, + )}` + + EOL.repeat(2) + + ' Failed to load file content' + + EOL + + ` Path: my-imaginary-file.tf` + + EOL.repeat(2) + + ' Could not find any valid IaC files' + + EOL + + ` Path: my-imaginary-directory/`, ); }); it('should include user tip for test failures', async () => { - // Arrange const dirPath = 'iac/terraform'; - - // Act const { stdout } = await run(`snyk iac test ${dirPath}`); - // Assert expect(stdout).toContain( 'Tip: Re-run in debug mode to see more information: DEBUG=*snyk* ' + EOL + @@ -241,13 +215,9 @@ Target file: ${dirPath}/`); describe('with only test failures', () => { it('should display the failure reason for the first failed test', async () => { - // Arrange const dirPath = 'iac/only-invalid'; - - // Act const { stdout } = await run(`snyk iac test ${dirPath}`); - // Assert expect(stdout).toContain( `Could not find any valid infrastructure as code files. Supported file extensions are tf, yml, yaml & json. More information can be found by running \`snyk iac test --help\` or through our documentation: @@ -257,52 +227,36 @@ https://support.snyk.io/hc/en-us/articles/360013723877-Test-your-Terraform-files }); it('should not show the issues section', async () => { - // Arrange const dirPath = 'iac/only-invalid'; - - // Act const { stdout } = await run(`snyk iac test ${dirPath}`); - // Assert expect(stdout).not.toContain('Issues'); }); it('should not show the test summary section', async () => { - // Arrange const dirPath = 'iac/only-invalid'; - - // Act const { stdout } = await run(`snyk iac test ${dirPath}`); - // Assert expect(stdout).not.toContain('Test Summary'); }); }); describe('with no issues', () => { it('should display an appropriate message in the issues section', async () => { - // Arrange const filePath = 'iac/terraform/vars.tf'; - - // Act const { stdout } = await run(`snyk iac test ${filePath}`); - // Assert expect(stdout).toContain('No vulnerable paths were found!'); }); }); describe('with issues generated by custom rules', () => { it('should include the public custom rule IDs', async () => { - // Arrange const filePath = 'iac/terraform/sg_open_ssh.tf '; - - // Act const { stdout } = await run( `snyk iac test ${filePath} --rules=./iac/custom-rules/custom.tar.gz`, ); - // Assert expect(stdout).toContain(`Rule: custom rule CUSTOM-1`); }); }); @@ -310,13 +264,9 @@ https://support.snyk.io/hc/en-us/articles/360013723877-Test-your-Terraform-files describe(`without the '${IAC_CLI_OUTPUT_FF}' feature flag`, () => { it('should show the file meta sections for each file', async () => { - // Arrange const filePath = 'iac/arm/rule_test.json'; - - // Act const { stdout } = await run(`snyk iac test ${filePath}`); - // Assert expect(stdout).toContain(` Organization: test-org Type: ARM @@ -328,13 +278,9 @@ Project path: ${filePath} }); it('should show the file summary messages', async () => { - // Arrange const dirPath = 'iac/terraform'; - - // Act const { stdout } = await run(`snyk iac test ${dirPath}`); - // Assert expect(stdout).toContain( 'Tested sg_open_ssh.tf for known issues, found 1 issues', ); @@ -370,13 +316,9 @@ Project path: ${filePath} }); it('should show the test summary message', async () => { - // Arrange const dirPath = 'iac/kubernetes'; - - // Act const { stdout } = await run(`snyk iac test ${dirPath}`); - // Assert expect(stdout).toContain( 'Tested 3 projects, 3 contained issues. Failed to test 1 project.', ); @@ -396,48 +338,32 @@ Project path: ${filePath} }); it('should not show the test summary section', async () => { - // Arrange const filePath = 'iac/kubernetes/pod-valid.json'; - - // Act const { stdout } = await run(`snyk iac test ${filePath}`); - // Assert expect(stdout).not.toContain('Test Summary'); }); it('should not show the test failures section', async () => { - // Arrange const dirPath = 'iac/only-valid'; - - // Act const { stdout } = await run(`snyk iac test ${dirPath}`); - // Assert expect(stdout).not.toContain('Invalid Files'); }); describe('with multiple test results', () => { it('should show the test summary message', async () => { - // Arrange const dirPath = 'iac/kubernetes'; - - // Act const { stdout } = await run(`snyk iac test ${dirPath}`); - // Assert expect(stdout).toContain('Tested 3 projects, 3 contained issues.'); }); describe('with test failures', () => { it('should show the failure reasons per file', async () => { - // Arrange const dirPath = 'iac/terraform'; - - // Act const { stdout } = await run(`snyk iac test ${dirPath}`); - // Assert expect(stdout).toContain( `Testing sg_open_ssh_invalid_go_templates.tf... @@ -452,26 +378,18 @@ Failed to parse Terraform file`, }); it('should include the failures count in the test summary message', async () => { - // Arrange const dirPath = 'iac/kubernetes'; - - // Act const { stdout } = await run(`snyk iac test ${dirPath}`); - // Assert expect(stdout).toContain( 'Tested 3 projects, 3 contained issues. Failed to test 1 project.', ); }); it('should include user tip for test failures', async () => { - // Arrange const dirPath = 'iac/terraform'; - - // Act const { stdout } = await run(`snyk iac test ${dirPath}`); - // Assert expect(stdout).toContain( `Tip: Re-run in debug mode to see more information: DEBUG=*snyk* If the issue persists contact support@snyk.io`, @@ -482,13 +400,9 @@ If the issue persists contact support@snyk.io`, describe('with only test failures', () => { it('should display the failure reason for the first failed test', async () => { - // Arrange const dirPath = 'iac/only-invalid'; - - // Act const { stdout } = await run(`snyk iac test ${dirPath}`); - // Assert expect(stdout).toContain( `Could not find any valid infrastructure as code files. Supported file extensions are tf, yml, yaml & json. More information can be found by running \`snyk iac test --help\` or through our documentation: @@ -498,24 +412,16 @@ https://support.snyk.io/hc/en-us/articles/360013723877-Test-your-Terraform-files }); it('should not show file issue lists', async () => { - // Arrange const dirPath = 'iac/only-invalid'; - - // Act const { stdout } = await run(`snyk iac test ${dirPath}`); - // Assert expect(stdout).not.toContain('Infrastructure as code issues'); }); it('should not show the test summary section', async () => { - // Arrange const dirPath = 'iac/only-invalid'; - - // Act const { stdout } = await run(`snyk iac test ${dirPath}`); - // Assert expect(stdout).not.toContain('Test Summary'); }); }); diff --git a/test/jest/unit/lib/formatters/iac-output/v2/failures/fixtures/test-failures.json b/test/jest/unit/lib/formatters/iac-output/v2/failures/fixtures/test-failures.json index 92b2709cb69..6b4721b6701 100644 --- a/test/jest/unit/lib/formatters/iac-output/v2/failures/fixtures/test-failures.json +++ b/test/jest/unit/lib/formatters/iac-output/v2/failures/fixtures/test-failures.json @@ -1,27 +1,22 @@ [ { "filePath": "test/fixtures/iac/arm/invalid_rule_test.json", - "fileType": "json", "failureReason": "Failed to parse JSON file" }, { "filePath": "test/fixtures/iac/cloudformation/invalid-cfn.yml", - "fileType": "yml", "failureReason": "Failed to parse YAML file" }, { "filePath": "test/fixtures/iac/kubernetes/helm-config.yaml", - "fileType": "yaml", "failureReason": "Failed to parse YAML file" }, { "filePath": "test/fixtures/iac/terraform/sg_open_ssh_invalid_go_templates.tf", - "fileType": "tf", "failureReason": "Failed to parse Terraform file" }, { "filePath": "test/fixtures/iac/terraform/sg_open_ssh_invalid_hcl2.tf", - "fileType": "tf", "failureReason": "Failed to parse Terraform file" } -] \ No newline at end of file +] diff --git a/test/jest/unit/lib/formatters/iac-output/v2/failures/list.spec.ts b/test/jest/unit/lib/formatters/iac-output/v2/failures/list.spec.ts index 0bf949dd447..d90bf2a7f29 100644 --- a/test/jest/unit/lib/formatters/iac-output/v2/failures/list.spec.ts +++ b/test/jest/unit/lib/formatters/iac-output/v2/failures/list.spec.ts @@ -3,9 +3,9 @@ import * as pathLib from 'path'; import { formatIacTestFailures } from '../../../../../../../../src/lib/formatters/iac-output'; import { colors } from '../../../../../../../../src/lib/formatters/iac-output/v2/utils'; -import { IacFileInDirectory } from '../../../../../../../../src/lib/types'; +import { IaCTestFailure } from '../../../../../../../../src/lib/formatters/iac-output/v2/types'; -const testFailureFixtures: IacFileInDirectory[] = JSON.parse( +const testFailureFixtures: IaCTestFailure[] = JSON.parse( fs.readFileSync( pathLib.join(__dirname, 'fixtures', 'test-failures.json'), 'utf-8', @@ -14,15 +14,11 @@ const testFailureFixtures: IacFileInDirectory[] = JSON.parse( describe('formatIacTestFailures', () => { it('should include the "Invalid files: X" title with the correct value', () => { - // Act const result = formatIacTestFailures(testFailureFixtures); - - // Assert expect(result).toContain(colors.info.bold(`Test Failures`)); }); it('should include the failures list with the correct values', () => { - // Act const result = formatIacTestFailures(testFailureFixtures); // Assert