Skip to content

Commit

Permalink
refactor: Remove use of options.iacDirFiles
Browse files Browse the repository at this point in the history
We were only using these for the display output in the test/index.ts
file, this change replaces this usage with a local variable. Now the
only place using the iacDirFiles property is the TestOptions and the
legacy remote execution test flow.

One change from this PR is that we will now always use the file basename
in the CLI output rather than the basename for directory scanning and
the full path for the single file scanning.
  • Loading branch information
aron committed Mar 23, 2021
1 parent 98e4ddb commit e08c298
Show file tree
Hide file tree
Showing 11 changed files with 60 additions and 45 deletions.
9 changes: 5 additions & 4 deletions src/cli/commands/test/iac-local-execution/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ export async function test(
options: IaCTestFlags,
): Promise<{
results: TestResult | TestResult[];
files: IacFileInDirectory[] | null;
/** All files scanned by IaC with parse errors */
failures?: IacFileInDirectory[];
}> {
await initLocalCache();
const filesToParse = await loadFiles(pathToScan);
Expand All @@ -36,9 +37,9 @@ export async function test(
return {
results: (formattedResults as unknown) as TestResult[],
// NOTE: No file or parsed file data should leave this function.
files: isLocalFolder(pathToScan)
? [...parsedFiles, ...failedFiles].map(removeFileContent)
: null,
failures: isLocalFolder(pathToScan)
? failedFiles.map(removeFileContent)
: undefined,
};
}

Expand Down
1 change: 1 addition & 0 deletions src/cli/commands/test/iac-local-execution/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ export type IaCTestFlags = Pick<
// that are added at runtime and not part of the parsed
// CLI flags.
export type IaCTestOptions = IaCTestFlags & {
/** @deprecated Only used by the legacy `iac test` flow remove once local exec path is GA */
iacDirFiles?: Array<IacFileInDirectory>;
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export async function test(
options: IaCTestOptions,
): Promise<{
results: TestResult | TestResult[];
files: IacFileInDirectory[] | null;
failures?: IacFileInDirectory[];
}> {
// Ensure that all flags are correct. We do this to ensure that the
// caller doesn't accidentally mistype --experimental and send their
Expand All @@ -30,5 +30,8 @@ export async function test(
}

const results = await legacyTest(pathToScan, options);
return { files: options.iacDirFiles ?? null, results };
return {
failures: options.iacDirFiles?.filter((file) => !!file.failureReason),
results,
};
}
30 changes: 13 additions & 17 deletions src/cli/commands/test/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { isCI } from '../../../lib/is-ci';
import * as Debug from 'debug';
import * as pathLib from 'path';
import {
IacFileInDirectory,
Options,
SupportedProjectTypes,
TestOptions,
Expand Down Expand Up @@ -48,7 +49,7 @@ import {
getDisplayedOutput,
} from './formatters/format-test-results';

import { test as iacTest } from './iac-test';
import { test as iacTest } from './iac-test-shim';
import { validateCredentials } from './validate-credentials';
import { generateSnykTestError } from './generate-snyk-test-error';
import { validateTestOptions } from './validate-test-options';
Expand Down Expand Up @@ -83,6 +84,9 @@ async function test(...args: MethodArgs): Promise<TestCommandResult> {
const resultOptions: Array<Options & TestOptions> = [];
const results = [] as any[];

// Holds an array of scanned file metadata for output.
let iacScanFailures: IacFileInDirectory[] | undefined;

// Promise waterfall to test all other paths sequentially
for (const path of paths) {
// Create a copy of the options so a specific test can
Expand All @@ -93,16 +97,13 @@ async function test(...args: MethodArgs): Promise<TestCommandResult> {
testOpts.projectName = testOpts['project-name'];

let res: (TestResult | TestResult[]) | Error;

try {
if (options.iac) {
// this path is an experimental feature feature for IaC which does issue scanning locally without sending files to our Backend servers.
// once ready for GA, it is aimed to deprecate our remote-processing model, so IaC file scanning in the CLI is done locally.
const { results, files } = await iacTest(path, testOpts);
const { results, failures } = await iacTest(path, testOpts);
res = results;
if (files) {
options.iacDirFiles = files;
}
iacScanFailures = failures;
} else {
res = await snyk.test(path, testOpts);
}
Expand Down Expand Up @@ -231,16 +232,11 @@ async function test(...args: MethodArgs): Promise<TestCommandResult> {
let summaryMessage = '';
let errorResultsLength = errorResults.length;

if (options.iac && options.iacDirFiles) {
const iacDirFilesErrors = options.iacDirFiles?.filter(
(iacFile) => iacFile.failureReason,
);
errorResultsLength = iacDirFilesErrors?.length || errorResults.length;
if (options.iac && iacScanFailures) {
errorResultsLength = iacScanFailures.length || errorResults.length;

if (iacDirFilesErrors) {
for (const iacFileError of iacDirFilesErrors) {
response += chalk.bold.red(getIacDisplayErrorFileOutput(iacFileError));
}
for (const reason of iacScanFailures) {
response += chalk.bold.red(getIacDisplayErrorFileOutput(reason));
}
}

Expand Down Expand Up @@ -326,7 +322,7 @@ function displayResult(
(res.packageManager as SupportedProjectTypes) || options.packageManager;
const localPackageTest = isLocalFolder(options.path);
let testingPath = options.path;
if (options.iac && options.iacDirFiles && res.targetFile) {
if (options.iac && res.targetFile) {
testingPath = pathLib.basename(res.targetFile);
}
const prefix = chalk.bold.white('\nTesting ' + testingPath + '...\n\n');
Expand All @@ -344,7 +340,7 @@ function displayResult(

if (res.dependencyCount) {
pathOrDepsText += res.dependencyCount + ' dependencies';
} else if (options.iacDirFiles && res.targetFile) {
} else if (options.iac && res.targetFile) {
pathOrDepsText += pathLib.basename(res.targetFile);
} else {
pathOrDepsText += options.path;
Expand Down
1 change: 1 addition & 0 deletions src/lib/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ export interface TestOptions {
yarnWorkspaces?: boolean;
testDepGraphDockerEndpoint?: string | null;
isDockerUser?: boolean;
/** @deprecated Only used by the legacy `iac test` flow remove once local exec path is GA */
iacDirFiles?: IacFileInDirectory[];
}

Expand Down
2 changes: 1 addition & 1 deletion test/acceptance/cli-test/iac/cli-test.iac-k8s.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ export const IacK8sTests: AcceptanceTests = {

t.match(
res,
'Tested iac-kubernetes/multi-file.yaml for known issues, found 3 issues',
'Tested multi-file.yaml for known issues, found 3 issues',
'3 issue',
);

Expand Down
5 changes: 4 additions & 1 deletion test/acceptance/cli-test/iac/cli-test.iac-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
IacTestResponse,
} from '../../../../src/lib/snyk-test/iac-test-result';
import { Log, Run, Result } from 'sarif';
import { basename } from 'path';

export enum IacAcceptanceTestType {
SINGLE_K8S_FILE = 1,
Expand Down Expand Up @@ -159,7 +160,9 @@ export async function iacTest(
const res = testableObject.message;
t.match(
res,
`Tested ${testParams[testType].displayFilePath} for known issues, found ${numOfIssues} issues`,
`Tested ${basename(
testParams[testType].displayFilePath,
)} for known issues, found ${numOfIssues} issues`,
`${numOfIssues} issue`,
);
iacTestMetaAssertions(t, res, testType);
Expand Down
20 changes: 15 additions & 5 deletions test/jest/unit/iac-unit-tests/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,16 @@ jest.mock(
EngineType,
} = require('../../../../src/cli/commands/test/iac-local-execution/types');
const parsedFiles: IacFileParsed[] = [
{
engineType: EngineType.Terraform,
fileContent: 'FAKE_FILE_CONTENT',
jsonContent: {},
filePath: './storage/storage.tf',
fileType: 'tf',
projectType: IacProjectType.TERRAFORM,
},
];
const failedFiles: IacFileParsed[] = [
{
engineType: EngineType.Terraform,
fileContent: 'FAKE_FILE_CONTENT',
Expand All @@ -19,7 +29,7 @@ jest.mock(
},
];
return {
parseFiles: async () => ({ parsedFiles, failedFiles: [] }),
parseFiles: async () => ({ parsedFiles, failedFiles }),
};
},
);
Expand All @@ -43,19 +53,19 @@ import {
import { IacProjectType } from '../../../../src/lib/iac/constants';

describe('test()', () => {
it('extends the options object with iacDirFiles when a local directory is provided', async () => {
it('returns the unparsable files excluding content', async () => {
const opts: IaCTestFlags = {};
const { files } = await test('./storage/', opts);
const { failures } = await test('./storage/', opts);

expect(files).toEqual([
expect(failures).toEqual([
{
filePath: './storage/storage.tf',
fileType: 'tf',
failureReason: 'Mock Test',
projectType: IacProjectType.TERRAFORM,
},
]);
expect(files).not.toEqual(
expect(failures).not.toEqual(
expect.arrayContaining([
{
fileContent: 'FAKE_FILE_CONTENT',
Expand Down
8 changes: 4 additions & 4 deletions test/smoke/spec/iac/snyk_test_k8s_spec.sh
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ Describe "Snyk iac test command"
It "finds issues in k8s file"
When run snyk iac test ../fixtures/iac/kubernetes/pod-privileged.yaml
The status should be failure # issues found
The output should include "Testing ../fixtures/iac/kubernetes/pod-privileged.yaml..."
The output should include "Testing pod-privileged.yaml..."

# Outputs issues
The output should include "Infrastructure as code issues:"
Expand All @@ -33,13 +33,13 @@ Describe "Snyk iac test command"
The output should include "Project name: kubernetes"
The output should include "Open source: no"
The output should include "Project path: ../fixtures/iac/kubernetes/pod-privileged.yaml"
The output should include "Tested ../fixtures/iac/kubernetes/pod-privileged.yaml for known issues, found"
The output should include "Tested pod-privileged.yaml for known issues, found"
End

It "filters out issues when using severity threshold"
When run snyk iac test ../fixtures/iac/kubernetes/pod-privileged.yaml --severity-threshold=high
The status should be failure # one issue found
The output should include "Testing ../fixtures/iac/kubernetes/pod-privileged.yaml..."
The output should include "Testing pod-privileged.yaml..."

The output should include "Infrastructure as code issues:"
The output should include "✗ Container is running in privileged mode [High Severity] [SNYK-CC-K8S-1] in Deployment"
Expand All @@ -51,7 +51,7 @@ Describe "Snyk iac test command"
The output should include "Project name: kubernetes"
The output should include "Open source: no"
The output should include "Project path: ../fixtures/iac/kubernetes/pod-privileged.yaml"
The output should include "Tested ../fixtures/iac/kubernetes/pod-privileged.yaml for known issues, found"
The output should include "Tested pod-privileged.yaml for known issues, found"
End

It "outputs an error for files with no valid k8s objects"
Expand Down
14 changes: 7 additions & 7 deletions test/smoke/spec/iac/snyk_test_local_exec_spec.sh
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ Describe "Snyk iac test --experimental command"
It "finds issues in k8s file"
When run snyk iac test ../fixtures/iac/kubernetes/pod-privileged.yaml --experimental
The status should be failure # issues found
The output should include "Testing ../fixtures/iac/kubernetes/pod-privileged.yaml..."
The output should include "Testing pod-privileged.yaml..."

# Outputs issues
The output should include "Infrastructure as code issues:"
Expand All @@ -41,7 +41,7 @@ Describe "Snyk iac test --experimental command"
It "filters out issues when using severity threshold"
When run snyk iac test ../fixtures/iac/kubernetes/pod-privileged.yaml --experimental --severity-threshold=high
The status should be failure # one issue found
The output should include "Testing ../fixtures/iac/kubernetes/pod-privileged.yaml..."
The output should include "Testing pod-privileged.yaml..."

The output should include "Infrastructure as code issues:"
The output should include "✗ Container is running in privileged mode [High Severity] [SNYK-CC-K8S-1] in Deployment"
Expand Down Expand Up @@ -74,7 +74,7 @@ Describe "Snyk iac test --experimental command"
It "finds issues in terraform file"
When run snyk iac test ../fixtures/iac/terraform/sg_open_ssh.tf --experimental
The status should be failure # issues found
The output should include "Testing ../fixtures/iac/terraform/sg_open_ssh.tf..."
The output should include "Testing sg_open_ssh.tf..."

# Outputs issues
The output should include "Infrastructure as code issues:"
Expand All @@ -85,10 +85,10 @@ Describe "Snyk iac test --experimental command"
It "filters out issues when using severity threshold"
When run snyk iac test ../fixtures/iac/terraform/sg_open_ssh.tf --experimental --severity-threshold=high
The status should be success # no issues found
The output should include "Testing ../fixtures/iac/terraform/sg_open_ssh.tf..."
The output should include "Testing sg_open_ssh.tf..."

The output should include "Infrastructure as code issues:"
The output should include "Tested ../fixtures/iac/terraform/sg_open_ssh.tf for known issues, found 0 issues"
The output should include "Tested sg_open_ssh.tf for known issues, found 0 issues"
End

# TODO: currently skipped because the parser we're using doesn't fail on invalid terraform
Expand Down Expand Up @@ -157,7 +157,7 @@ Describe "Snyk iac test --experimental command"
It "finds issues in a Terraform plan file"
When run snyk iac test ../fixtures/iac/terraform-plan/tf-plan.json --experimental
The status should be failure # issues found
The output should include "Testing ../fixtures/iac/terraform-plan/tf-plan.json"
The output should include "Testing tf-plan.json"

# Outputs issues
The output should include "Infrastructure as code issues:"
Expand All @@ -168,7 +168,7 @@ Describe "Snyk iac test --experimental command"
The output should include "✗ Security Group allows open ingress [Medium Severity] [SNYK-CC-TF-1] in Security Group"
The output should include " introduced by resource > aws_security_group[CHILD_MODULE_terra_ci_allow_outband_0] > ingress"

The output should include "../fixtures/iac/terraform-plan/tf-plan.json for known issues, found 2 issues"
The output should include "tf-plan.json for known issues, found 2 issues"
End
End
End
8 changes: 4 additions & 4 deletions test/smoke/spec/iac/snyk_test_terraform_spec.sh
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ Describe "Snyk iac test command"
It "finds issues in terraform file"
When run snyk iac test ../fixtures/iac/terraform/sg_open_ssh.tf
The status should be failure # issues found
The output should include "Testing ../fixtures/iac/terraform/sg_open_ssh.tf..."
The output should include "Testing sg_open_ssh.tf..."
# Outputs issues
The output should include "Infrastructure as code issues:"
The output should include "✗ Security Group allows open ingress [Medium Severity] [SNYK-CC-TF-1] in Security Group"
Expand All @@ -22,13 +22,13 @@ Describe "Snyk iac test command"
The output should include "Project name: terraform"
The output should include "Open source: no"
The output should include "Project path: ../fixtures/iac/terraform/sg_open_ssh.tf"
The output should include "Tested ../fixtures/iac/terraform/sg_open_ssh.tf for known issues, found"
The output should include "Tested sg_open_ssh.tf for known issues, found"
End

It "filters out issues when using severity threshold"
When run snyk iac test ../fixtures/iac/terraform/sg_open_ssh.tf --severity-threshold=high
The status should be success # no issues found
The output should include "Testing ../fixtures/iac/terraform/sg_open_ssh.tf..."
The output should include "Testing sg_open_ssh.tf..."
# Outputs issues
The output should include "Infrastructure as code issues:"

Expand All @@ -39,7 +39,7 @@ Describe "Snyk iac test command"
The output should include "Project name: terraform"
The output should include "Open source: no"
The output should include "Project path: ../fixtures/iac/terraform/sg_open_ssh.tf"
The output should include "Tested ../fixtures/iac/terraform/sg_open_ssh.tf for known issues, found"
The output should include "Tested sg_open_ssh.tf for known issues, found"
End

It "outputs an error for invalid hcl2 tf files"
Expand Down

0 comments on commit e08c298

Please sign in to comment.