Skip to content

Commit

Permalink
feat: multi level auto detection with flag
Browse files Browse the repository at this point in the history
Add --detection-depth arg.
Add tests for detection-depth arg to test and monitor.
Ensure targetFile is not sent when plugin does not require it (retains
project name on existing projects).
  • Loading branch information
orsagie authored and gitphill committed Jan 7, 2020
1 parent 758eca5 commit 5eccd37
Show file tree
Hide file tree
Showing 21 changed files with 297 additions and 88 deletions.
1 change: 1 addition & 0 deletions src/cli/args.ts
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ export function args(rawArgv: string[]): Args {
'scan-all-unmanaged',
'fail-on',
'all-projects',
'detection-depth',
]) {
if (argv[dashedArg]) {
const camelCased = dashToCamelCase(dashedArg);
Expand Down
2 changes: 1 addition & 1 deletion src/cli/commands/test/formatters/format-test-meta.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ export function formatTestMeta(
): string {
const padToLength = 19; // chars to align
const packageManager = res.packageManager || options.packageManager;
const targetFile = res.targetFile || options.file;
const targetFile = res.targetFile || res.displayTargetFile || options.file;
const openSource = res.isPrivate ? 'no' : 'yes';
const meta = [
chalk.bold(rightPadWithSpaces('Organization: ', padToLength)) + res.org,
Expand Down
1 change: 0 additions & 1 deletion src/cli/commands/test/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import {
} from '../../../lib/package-managers';

import * as analytics from '../../../lib/analytics';
import { isFeatureFlagSupportedForOrg } from '../../../lib/feature-flags';
import { FailOnError } from '../../../lib/errors/fail-on-error.ts';
import {
summariseVulnerableResults,
Expand Down
29 changes: 21 additions & 8 deletions src/cli/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -158,18 +158,31 @@ async function main() {
]);
}

if (
(args.options['project-name'] ||
args.options.file ||
args.options.packageManager ||
args.options.docker) &&
args.options.allProjects
) {
if (args.options['project-name'] && args.options.allProjects) {
throw new UnsupportedOptionCombinationError([
'project-name',
'all-projects',
]);
}
if (args.options.file && args.options.allProjects) {
throw new UnsupportedOptionCombinationError(['file', 'all-projects']);
}
if (args.options.packageManager && args.options.allProjects) {
throw new UnsupportedOptionCombinationError([
'project-name or file or package-manager or docker',
'package-manager',
'all-projects',
]);
}
if (args.options.docker && args.options.allProjects) {
throw new UnsupportedOptionCombinationError(['docker', 'all-projects']);
}
if (args.options.allSubProjects && args.options.allProjects) {
throw new UnsupportedOptionCombinationError([
'all-sub-projects',
'all-projects',
]);
}

if (
args.options.file &&
typeof args.options.file === 'string' &&
Expand Down
2 changes: 1 addition & 1 deletion src/lib/plugins/get-deps-from-plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export async function getDepsFromPlugin(
let inspectRes: pluginApi.InspectResult;

if (options.allProjects) {
const levelsDeep = 1; // TODO: auto-detect only one-level deep for now
const levelsDeep = options.detectionDepth || 1; // default to 1 level deep
const targetFiles = await find(root, [], AUTO_DETECTABLE_FILES, levelsDeep);
debug(
`auto detect manifest files, found ${targetFiles.length}`,
Expand Down
4 changes: 2 additions & 2 deletions src/lib/plugins/get-multi-plugin-result.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ export async function getMultiPluginResult(
const allResults: ScannedProjectCustom[] = [];
for (const targetFile of targetFiles) {
const optionsClone = _.cloneDeep(options);
optionsClone.file = path.basename(targetFile);
optionsClone.file = path.relative(root, targetFile);
optionsClone.packageManager = detectPackageManagerFromFile(
optionsClone.file,
path.basename(targetFile),
);
try {
const inspectRes = await getSinglePluginResult(
Expand Down
17 changes: 12 additions & 5 deletions src/lib/snyk-test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,19 +27,20 @@ async function test(root, options, callback) {

function executeTest(root, options) {
try {
const packageManager = detect.detectPackageManager(root, options);
options.packageManager = packageManager;
if (!options.allProjects) {
options.packageManager = detect.detectPackageManager(root, options);
}
return run(root, options).then((results) => {
for (const res of results) {
if (!res.packageManager) {
res.packageManager = packageManager;
res.packageManager = options.packageManager;
}
}
if (results.length === 1) {
// Return only one result if only one found as this is the default usecase
return results[0];
}
// For gradle and yarnWorkspaces we may be returning more than one result
// For gradle, yarnWorkspaces, allProjects we may be returning more than one result
return results;
});
} catch (error) {
Expand All @@ -49,7 +50,13 @@ function executeTest(root, options) {

function run(root, options) {
const packageManager = options.packageManager;
if (!(options.docker || pm.SUPPORTED_PACKAGE_MANAGER_NAME[packageManager])) {
if (
!(
options.docker ||
options.allProjects ||
pm.SUPPORTED_PACKAGE_MANAGER_NAME[packageManager]
)
) {
throw new UnsupportedPackageManagerError(packageManager);
}
return runTest(packageManager, root, options);
Expand Down
1 change: 1 addition & 0 deletions src/lib/snyk-test/legacy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ export interface BaseImageRemediationAdvice {
export interface TestResult extends LegacyVulnApiResult {
targetFile?: string;
projectName?: string;
displayTargetFile?: string; // used for display only
}

interface UpgradePathItem {
Expand Down
15 changes: 12 additions & 3 deletions src/lib/snyk-test/run-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ interface PayloadBody {
originalProjectName?: string; // used only for display
foundProjectCount?: number; // used only for display
docker?: any;
displayTargetFile?: string;
target?: GitTarget | null;
}

Expand Down Expand Up @@ -93,6 +94,8 @@ async function runTest(
_.get(payload, 'body.projectNameOverride') ||
_.get(payload, 'body.originalProjectName');
const foundProjectCount = _.get(payload, 'body.foundProjectCount');
const displayTargetFile = _.get(payload, 'body.displayTargetFile');

let dockerfilePackages;
if (
payload.body &&
Expand Down Expand Up @@ -190,6 +193,7 @@ async function runTest(
targetFile,
projectName,
foundProjectCount,
displayTargetFile,
};
results.push(result);
}
Expand Down Expand Up @@ -277,7 +281,9 @@ async function assembleLocalPayloads(
root,
options: Options & TestOptions,
): Promise<Payload[]> {
const analysisType = options.docker ? 'docker' : options.packageManager;
// For --all-projects packageManager is yet undefined here. Use 'all'
const analysisType =
(options.docker ? 'docker' : options.packageManager) || 'all';
const spinnerLbl =
'Analyzing ' +
analysisType +
Expand Down Expand Up @@ -344,14 +350,17 @@ async function assembleLocalPayloads(
}

// todo: normalize what target file gets used across plugins and functions
const targetFile = scannedProject.targetFile || deps.plugin.targetFile;
const targetFile =
scannedProject.targetFile || deps.plugin.targetFile || options.file;

let body: PayloadBody = {
targetFile,
// WARNING: be careful changing this as it affects project uniqueness
targetFile: deps.plugin.targetFile,
projectNameOverride: options.projectName,
originalProjectName: pkg.name,
policy: policy && policy.toString(),
foundProjectCount: getSubProjectCount(deps),
displayTargetFile: targetFile,
docker: pkg.docker,
hasDevDependencies: (pkg as any).hasDevDependencies,
target: await projectMetadata.getInfo(pkg, options),
Expand Down
5 changes: 2 additions & 3 deletions src/lib/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ export interface TestOptions {
'prune-repeated-subdependencies'?: boolean;
showVulnPaths: ShowVulnPaths;
failOn?: FailOn;
allProjects?: boolean;
}
export interface ProtectOptions {
loose: boolean;
Expand Down Expand Up @@ -64,6 +63,8 @@ export interface Options {
'print-deps'?: boolean;
'remote-repo-url'?: string;
scanAllUnmanaged?: boolean;
allProjects?: boolean;
detectionDepth?: number;
}

// TODO(kyegupov): catch accessing ['undefined-properties'] via noImplicitAny
Expand All @@ -78,8 +79,6 @@ export interface MonitorOptions {
'print-deps'?: boolean;
'experimental-dep-graph'?: boolean;
scanAllUnmanaged?: boolean;
allProjects?: boolean;

// An experimental flag to allow monitoring of bigtrees (with degraded deps info and remediation advice).
'prune-repeated-subdependencies'?: boolean;
}
Expand Down
64 changes: 18 additions & 46 deletions test/acceptance/cli-args.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,64 +78,36 @@ test('`test --file=blah --scan-all-unmanaged`', (t) => {
});
});

test('`test --file=blah and --all-projects`', (t) => {
t.plan(1);
exec(`node ${main} test --file=blah --all-projects`, (err, stdout) => {
if (err) {
throw err;
}
t.match(
stdout.trim(),
'The following option combination is not currently supported: project-name or file or package-manager or docker + all-projects',
'correct error output',
);
});
});
const argsNotAllowedWithAllProjects = [
'file',
'package-manager',
'project-name',
'docker',
'all-sub-projects',
];

test('`test --package-manager and --all-projects`', (t) => {
t.plan(1);
exec(
`node ${main} test --package-manager=npm --all-projects`,
(err, stdout) => {
argsNotAllowedWithAllProjects.forEach((arg) => {
test(`using --${arg} and --all-projects throws exception`, (t) => {
t.plan(2);
exec(`node ${main} test --${arg} --all-projects`, (err, stdout) => {
if (err) {
throw err;
}
t.match(
stdout.trim(),
'The following option combination is not currently supported: project-name or file or package-manager or docker + all-projects',
'correct error output',
`The following option combination is not currently supported: ${arg} + all-projects`,
'when using test',
);
},
);
});

test('`test --project-name and --all-projects`', (t) => {
t.plan(1);
exec(
`node ${main} test --project-name=my-monorepo --all-projects`,
(err, stdout) => {
});
exec(`node ${main} monitor --${arg} --all-projects`, (err, stdout) => {
if (err) {
throw err;
}
t.match(
stdout.trim(),
'The following option combination is not currently supported: project-name or file or package-manager or docker + all-projects',
'correct error output',
`The following option combination is not currently supported: ${arg} + all-projects`,
'when using monitor',
);
},
);
});

test('`test --docker and --all-projects`', (t) => {
t.plan(1);
exec(`node ${main} test --docker --all-projects`, (err, stdout) => {
if (err) {
throw err;
}
t.match(
stdout.trim(),
'The following option combination is not currently supported: project-name or file or package-manager or docker + all-projects',
'correct error output',
);
});
});
});
35 changes: 34 additions & 1 deletion test/acceptance/cli-monitor/cli-monitor.acceptance.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import { fakeServer } from '../fake-server';
import * as subProcess from '../../../src/lib/sub-process';
import * as version from '../../../src/lib/version';
import * as userConfig from '../../../src/lib/user-config';
import { chdirWorkspaces, getWorkspaceJSON } from '../workspace-helper';
import * as _ from 'lodash';

// ensure this is required *after* the demo server, since this will
// configure our fake configuration too
Expand All @@ -31,7 +33,6 @@ const after = tap.runOnly ? only : test;

// Should be after `process.env` setup.
import * as plugins from '../../../src/lib/plugins/index';
import { chdirWorkspaces } from '../workspace-helper';

// @later: remove this config stuff.
// Was copied straight from ../src/cli-server.js
Expand Down Expand Up @@ -1419,6 +1420,38 @@ test('`monitor foo:latest --docker` with custom policy path', async (t) => {
t.deepEqual(policyString, expected, 'sends correct policy');
});

test('monitor --json multiple folders', async (t) => {
chdirWorkspaces('fail-on');

const noFixableResult = getWorkspaceJSON(
'fail-on',
'no-fixable',
'vulns-result.json',
);
server.setNextResponse(noFixableResult);
try {
const response = await cli.monitor('upgradable', 'no-fixable', {
json: true,
});
const res = JSON.parse(response);
if (_.isObject(res)) {
t.pass('monitor outputted JSON');
} else {
t.fail('Failed parsing monitor JSON output');
}
const keyList = ['packageManager', 'manageUrl'];
t.true(Array.isArray(res), 'Response is an array');
t.equal(res.length, 2, 'Two monitor responses in the array');
res.forEach((project) => {
keyList.forEach((k) => {
!_.get(project, k) ? t.fail(k + 'not found') : t.pass(k + ' found');
});
});
} catch (error) {
t.fail('should not have failed');
}
});

/**
* We can't expect all test environments to have Maven installed
* So, hijack the system exec call and return the expected output
Expand Down
Loading

0 comments on commit 5eccd37

Please sign in to comment.