Skip to content

Commit

Permalink
refactor: replace cc parser with split functions
Browse files Browse the repository at this point in the history
  • Loading branch information
Almog Ben-David committed Aug 17, 2021
1 parent 707801d commit 1ed7d11
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 33 deletions.
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@
"dependencies": {
"@open-policy-agent/opa-wasm": "^1.2.0",
"@snyk/cli-interface": "2.11.0",
"@snyk/cloud-config-parser": "^1.9.2",
"@snyk/cloud-config-parser": "^1.10.2",
"@snyk/code-client": "4.0.0",
"@snyk/dep-graph": "^1.27.1",
"@snyk/fix": "1.650.0",
Expand Down Expand Up @@ -147,7 +147,7 @@
"devDependencies": {
"@types/cross-spawn": "^6.0.2",
"@types/fs-extra": "^9.0.11",
"@types/jest": "^26.0.20",
"@types/jest": "^27.0.1",
"@types/lodash": "^4.14.161",
"@types/needle": "^2.0.4",
"@types/node": "^14.14.31",
Expand Down
15 changes: 6 additions & 9 deletions src/cli/commands/test/iac-local-execution/extract-line-number.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,16 @@ import { IaCErrorCodes } from './types';
import { CustomError } from '../../../../lib/errors';
import {
CloudConfigFileTypes,
issuesToLineNumbers,
MapsDocIdToTree,
getLineNumber,
} from '@snyk/cloud-config-parser';
import { UnsupportedFileTypeError } from './file-parser';
import * as analytics from '../../../../lib/analytics';
import * as Debug from 'debug';
import { getErrorStringCode } from './error-utils';
const debug = Debug('iac-extract-line-number');

function getFileTypeForLineNumber(fileType: string): CloudConfigFileTypes {
export function getFileTypeForParser(fileType: string): CloudConfigFileTypes {
switch (fileType) {
case 'yaml':
case 'yml':
Expand All @@ -25,16 +26,12 @@ function getFileTypeForLineNumber(fileType: string): CloudConfigFileTypes {
}

export function extractLineNumber(
fileContent: string,
fileType: string,
cloudConfigPath: string[],
fileType: CloudConfigFileTypes,
treeByDocId: MapsDocIdToTree,
): number {
try {
return issuesToLineNumbers(
fileContent,
getFileTypeForLineNumber(fileType),
cloudConfigPath,
);
return getLineNumber(cloudConfigPath, fileType, treeByDocId);
} catch {
const err = new FailedToExtractLineNumberError();
analytics.add('error-code', err.code);
Expand Down
30 changes: 14 additions & 16 deletions src/cli/commands/test/iac-local-execution/results-formatter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,10 @@ import * as path from 'path';
import { SEVERITY } from '../../../../lib/snyk-test/common';
import { IacProjectType } from '../../../../lib/iac/constants';
import { CustomError } from '../../../../lib/errors';
import { extractLineNumber } from './extract-line-number';
import { extractLineNumber, getFileTypeForParser } from './extract-line-number';
import { getErrorStringCode } from './error-utils';
import { isLocalFolder } from '../../../../lib/detect';
import { MapsDocIdToTree, getTrees } from '@snyk/cloud-config-parser';

const SEVERITIES = [SEVERITY.LOW, SEVERITY.MEDIUM, SEVERITY.HIGH];

Expand Down Expand Up @@ -53,27 +54,24 @@ function formatScanResult(
meta: TestMeta,
options: IaCTestFlags,
): FormattedResult {
const fileType = getFileTypeForParser(scanResult.fileType);
let treeByDocId: MapsDocIdToTree;
try {
treeByDocId = getTrees(fileType, scanResult.fileContent);
} catch (err) {
// we do nothing intentionally.
// Even if the building of the tree fails in the external parser,
// we still pass an undefined tree and not calculated line number for those
}

const formattedIssues = scanResult.violatedPolicies.map((policy) => {
const cloudConfigPath =
scanResult.docId !== undefined
? [`[DocId: ${scanResult.docId}]`].concat(parsePath(policy.msg))
: policy.msg.split('.');

const flagsRequiringLineNumber = [
'json',
'sarif',
'json-file-output',
'sarif-file-output',
];
const shouldExtractLineNumber = flagsRequiringLineNumber.some(
(flag) => options[flag],
);
const lineNumber: number = shouldExtractLineNumber
? extractLineNumber(
scanResult.fileContent,
scanResult.fileType,
cloudConfigPath,
)
const lineNumber: number = treeByDocId
? extractLineNumber(cloudConfigPath, fileType, treeByDocId)
: -1;

return {
Expand Down
58 changes: 52 additions & 6 deletions test/jest/unit/iac-unit-tests/results-formatter.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,18 @@ import {
policyStub,
generateScanResults,
} from './results-formatter.fixtures';
import { issuesToLineNumbers } from '@snyk/cloud-config-parser';
import * as cloudConfigParserModule from '@snyk/cloud-config-parser';
import { PolicyMetadata } from '../../../../src/cli/commands/test/iac-local-execution/types';

jest.mock('@snyk/cloud-config-parser');

jest.mock('@snyk/cloud-config-parser', () => ({
...jest.requireActual('@snyk/cloud-config-parser'),
}));
const validTree = { '0': { nodes: [] } };
describe('formatScanResults', () => {
it.each([
[
{ severityThreshold: SEVERITY.HIGH },
expectedFormattedResultsWithoutLineNumber,
expectedFormattedResultsWithLineNumber,
],
[
{ severityThreshold: SEVERITY.HIGH, sarif: true },
Expand All @@ -40,7 +42,10 @@ describe('formatScanResults', () => {
])(
'given %p options object, returns the expected results',
(optionsObject, expectedResult) => {
(issuesToLineNumbers as jest.Mock).mockReturnValue(3);
jest
.spyOn(cloudConfigParserModule, 'getTrees')
.mockReturnValue(validTree);
jest.spyOn(cloudConfigParserModule, 'getLineNumber').mockReturnValue(3);
const formattedResults = formatScanResults(
generateScanResults(),
optionsObject,
Expand All @@ -51,9 +56,50 @@ describe('formatScanResults', () => {
expect(formattedResults[0]).toEqual(expectedResult);
},
);
// TODO: add tests for the multi-doc yaml grouping
});

describe('parser failures should return -1 for lineNumber', () => {
beforeEach(async () => {
jest.restoreAllMocks();
});

it('creates a valid tree, but the getLineNumber() fails', () => {
jest.spyOn(cloudConfigParserModule, 'getTrees').mockReturnValue(validTree);
jest
.spyOn(cloudConfigParserModule, 'getLineNumber')
.mockImplementation(() => {
throw new Error();
});
const formattedResults = formatScanResults(
generateScanResults(),
{ severityThreshold: SEVERITY.HIGH },
meta,
);

expect(formattedResults.length).toEqual(1);
expect(formattedResults[0]).toEqual(
expectedFormattedResultsWithoutLineNumber,
);
});

it('sends an invalid tree and getLineNumber() fails', () => {
jest
.spyOn(cloudConfigParserModule, 'getTrees')
.mockReturnValue(null as any);
const formattedResults = formatScanResults(
generateScanResults(),
{ severityThreshold: SEVERITY.HIGH },
meta,
);

expect(formattedResults.length).toEqual(1);
expect(formattedResults[0]).toEqual(
expectedFormattedResultsWithoutLineNumber,
);
});
});

// TODO: add tests for the multi-doc yaml grouping
describe('filterPoliciesBySeverity', () => {
it('returns the formatted results filtered by severity - no default threshold', () => {
const scanResults = generateScanResults();
Expand Down

0 comments on commit 1ed7d11

Please sign in to comment.