From 047998ec70915a480d3d030ccf260c04eb19fa06 Mon Sep 17 00:00:00 2001 From: Konstantin Yegupov Date: Tue, 13 Aug 2019 20:07:20 +0100 Subject: [PATCH] fix: reinstate --all-sub-project support for Gradle --- src/cli/commands/monitor.ts | 39 ++++++++++----------- src/lib/module-info/index.ts | 4 +-- src/lib/monitor.ts | 9 ++--- src/lib/plugins/index.ts | 15 -------- src/lib/plugins/types.ts | 4 +-- src/lib/print-deps.ts | 5 +-- src/lib/snyk-test/run-test.ts | 31 +++++++++-------- src/lib/types.ts | 47 +++----------------------- test/acceptance/cli.acceptance.test.ts | 44 ++++++++++++------------ 9 files changed, 72 insertions(+), 126 deletions(-) diff --git a/src/cli/commands/monitor.ts b/src/cli/commands/monitor.ts index 353352e5c4..822e85843a 100644 --- a/src/cli/commands/monitor.ts +++ b/src/cli/commands/monitor.ts @@ -16,9 +16,6 @@ import * as detect from '../../lib/detect'; import * as plugins from '../../lib/plugins'; import {ModuleInfo} from '../../lib/module-info'; // TODO(kyegupov): fix import import { - SingleDepRootResult, - MultiDepRootsResult, - isMultiResult, MonitorOptions, MonitorMeta, MonitorResult, @@ -30,6 +27,7 @@ import { MonitorError, UnsupportedFeatureFlagError, } from '../../lib/errors'; +import { legacyPlugin as pluginApi } from '@snyk/cli-interface'; const SEPARATOR = '\n-------------------------------------------------------\n'; @@ -78,7 +76,7 @@ async function monitor(...args0: MethodArgs): Promise { snyk.id = options.id; } - if (options['all-sub-projects'] && options['project-name']) { + if (options.allSubProjects && options['project-name']) { throw new Error('`--all-sub-projects` is currently not compatible with `--project-name`'); } @@ -126,13 +124,12 @@ async function monitor(...args0: MethodArgs): Promise { // Scan the project dependencies via a plugin - const pluginOptions = plugins.getPluginOptions(packageManager, options); analytics.add('packageManager', packageManager); - analytics.add('pluginOptions', pluginOptions); + analytics.add('pluginOptions', options); - // TODO: the type should depend on multiDepRoots flag - const inspectResult: SingleDepRootResult|MultiDepRootsResult = await promiseOrCleanup( - moduleInfo.inspect(path, targetFile, { ...options, ...pluginOptions }), + // TODO: the type should depend on allSubProjects flag + const inspectResult: pluginApi.InspectResult = await promiseOrCleanup( + moduleInfo.inspect(path, targetFile, { ...options }), spinner.clear(analyzingDepsSpinnerLabel)); analytics.add('pluginName', inspectResult.plugin.name); @@ -155,14 +152,14 @@ async function monitor(...args0: MethodArgs): Promise { // We send results from "all-sub-projects" scanning as different Monitor objects - // SingleDepRootResult is a legacy format understood by Registry, so we have to convert - // a MultiDepRootsResult to an array of these. + // SinglePackageResult is a legacy format understood by Registry, so we have to convert + // a MultiProjectResult to an array of these. - let perDepRootResults: SingleDepRootResult[] = []; + let perSubProjectResults: pluginApi.SinglePackageResult[] = []; let advertiseSubprojectsCount: number | null = null; - if (isMultiResult(inspectResult)) { - perDepRootResults = inspectResult.depRoots.map( - (depRoot) => ({plugin: inspectResult.plugin, package: depRoot.depTree})); + if (pluginApi.isMultiResult(inspectResult)) { + perSubProjectResults = inspectResult.scannedProjects.map( + (scannedProject) => ({plugin: inspectResult.plugin, package: scannedProject.depTree})); } else { if (!options['gradle-sub-project'] && inspectResult.plugin.meta @@ -170,15 +167,15 @@ async function monitor(...args0: MethodArgs): Promise { && inspectResult.plugin.meta.allSubProjectNames.length > 1) { advertiseSubprojectsCount = inspectResult.plugin.meta.allSubProjectNames.length; } - perDepRootResults = [inspectResult]; + perSubProjectResults = [inspectResult]; } // Post the project dependencies to the Registry - for (const depRootDeps of perDepRootResults) { - maybePrintDeps(options, depRootDeps.package); + for (const subProjDeps of perSubProjectResults) { + maybePrintDeps(options, subProjDeps.package); const res = await promiseOrCleanup( - snykMonitor(path, meta, depRootDeps, targetFile), + snykMonitor(path, meta, subProjDeps, targetFile), spinner.clear(postingMonitorSpinnerLabel)); await spinner.clear(postingMonitorSpinnerLabel)(res); @@ -193,8 +190,8 @@ async function monitor(...args0: MethodArgs): Promise { const manageUrl = url.format(endpoint); endpoint.pathname = leader + '/monitor/' + res.id; - const subProjectName = isMultiResult(inspectResult) - ? depRootDeps.package.name + const subProjectName = pluginApi.isMultiResult(inspectResult) + ? subProjDeps.package.name : undefined; const monOutput = formatMonitorOutput( packageManager, diff --git a/src/lib/module-info/index.ts b/src/lib/module-info/index.ts index 382df78785..b424381f13 100644 --- a/src/lib/module-info/index.ts +++ b/src/lib/module-info/index.ts @@ -1,12 +1,12 @@ import * as _ from 'lodash'; import * as Debug from 'debug'; -import { SingleDepRootResult } from '../types'; +import { legacyPlugin as pluginApi } from '@snyk/cli-interface'; const debug = Debug('snyk-module-info'); export function ModuleInfo(plugin, policy) { return { - async inspect(root, targetFile, options): Promise { + async inspect(root, targetFile, options): Promise { const pluginOptions = _.merge({ args: options._doubleDashArgs, }, options); diff --git a/src/lib/monitor.ts b/src/lib/monitor.ts index 67fbaf3320..1b2d74be25 100644 --- a/src/lib/monitor.ts +++ b/src/lib/monitor.ts @@ -8,12 +8,13 @@ import * as os from 'os'; import * as _ from 'lodash'; import {isCI} from './is-ci'; import * as analytics from './analytics'; -import { SingleDepRootResult, DepTree, MonitorMeta, MonitorResult } from './types'; +import { DepTree, MonitorMeta, MonitorResult } from './types'; import * as projectMetadata from './project-metadata'; import * as path from 'path'; import {MonitorError, ConnectionTimeoutError} from './errors'; import { countPathsToGraphRoot, pruneGraph } from './prune'; import { GRAPH_SUPPORTED_PACKAGE_MANAGERS } from './package-managers'; +import { legacyPlugin as pluginApi } from '@snyk/cli-interface'; const debug = Debug('snyk'); @@ -104,7 +105,7 @@ function filterOutMissingDeps(depTree: DepTree): FilteredDepTree { for (const depKey of Object.keys(depTree.dependencies)) { const dep = depTree.dependencies[depKey]; - if (dep.missingLockFileEntry) { + if ((dep as any).missingLockFileEntry) { // TODO(kyegupov): add field to the type missingDeps.push(`${dep.name}@${dep.version}`); } else { filteredDeps[depKey] = dep; @@ -124,7 +125,7 @@ function filterOutMissingDeps(depTree: DepTree): FilteredDepTree { export async function monitor( root: string, meta: MonitorMeta, - info: SingleDepRootResult, + info: pluginApi.SinglePackageResult, targetFile?: string, ): Promise { apiTokenExists(); @@ -231,7 +232,7 @@ export async function monitor( export async function monitorGraph( root: string, meta: MonitorMeta, - info: SingleDepRootResult, + info: pluginApi.SinglePackageResult, targetFile?: string, ): Promise { const packageManager = meta.packageManager; diff --git a/src/lib/plugins/index.ts b/src/lib/plugins/index.ts index a7577f01bf..84bfbd9ebd 100644 --- a/src/lib/plugins/index.ts +++ b/src/lib/plugins/index.ts @@ -59,18 +59,3 @@ export function loadPlugin(packageManager: SupportedPackageManagers, } } } - -export function getPluginOptions(packageManager: string, options: types.Options): types.Options { - const pluginOptions: types.Options = {}; - switch (packageManager) { - case 'gradle': { - if (options['all-sub-projects']) { - pluginOptions.multiDepRoots = true; - } - return pluginOptions; - } - default: { - return pluginOptions; - } - } -} diff --git a/src/lib/plugins/types.ts b/src/lib/plugins/types.ts index 5cd1f04120..d54cbc9c6e 100644 --- a/src/lib/plugins/types.ts +++ b/src/lib/plugins/types.ts @@ -4,7 +4,7 @@ export interface InspectResult { runtime?: string; }; package?: any; - depRoots?: any; + scannedProjects?: any; } export interface Options { @@ -13,7 +13,7 @@ export interface Options { traverseNodeModules?: boolean; dev?: boolean; strictOutOfSync?: boolean | 'true' | 'false'; - multiDepRoots?: boolean; + allSubProjects?: boolean; debug?: boolean; packageManager?: string; composerIsFine?: boolean; diff --git a/src/lib/print-deps.ts b/src/lib/print-deps.ts index 1e82adfb2d..9bf6103b3c 100644 --- a/src/lib/print-deps.ts +++ b/src/lib/print-deps.ts @@ -1,8 +1,9 @@ -import { DepDict, Options, MonitorOptions, DepTree } from './types'; +import { DepDict, Options, MonitorOptions } from './types'; +import { legacyCommon as legacyApi } from '@snyk/cli-interface'; // This option is still experimental and might be deprecated. // It might be a better idea to convert it to a command (i.e. do not perform test/monitor). -export function maybePrintDeps(options: Options | MonitorOptions, rootPackage: DepTree) { +export function maybePrintDeps(options: Options | MonitorOptions, rootPackage: legacyApi.DepTree) { if (options['print-deps']) { if (options.json) { // Will produce 2 JSON outputs, one for the deps, one for the vuln scan. diff --git a/src/lib/snyk-test/run-test.ts b/src/lib/snyk-test/run-test.ts index ed79154c90..c91085f9c2 100644 --- a/src/lib/snyk-test/run-test.ts +++ b/src/lib/snyk-test/run-test.ts @@ -16,7 +16,7 @@ import common = require('./common'); import {DepTree, TestOptions} from '../types'; import gemfileLockToDependencies = require('../../lib/plugins/rubygems/gemfile-lock-to-dependencies'); import {convertTestDepGraphResultToLegacy, AnnotatedIssue, LegacyVulnApiResult, TestDepGraphResponse} from './legacy'; -import {SingleDepRootResult, MultiDepRootsResult, isMultiResult, Options} from '../types'; +import {Options} from '../types'; import { NoSupportedManifestsFoundError, InternalServerError, @@ -26,6 +26,7 @@ import { import { maybePrintDeps } from '../print-deps'; import { SupportedPackageManagers } from '../package-managers'; import { countPathsToGraphRoot, pruneGraph } from '../prune'; +import { legacyPlugin as pluginApi } from '@snyk/cli-interface'; // tslint:disable-next-line:no-var-requires const debug = require('debug')('snyk'); @@ -207,23 +208,22 @@ function assemblePayloads(root: string, options: Options & TestOptions): Promise return assembleRemotePayloads(root, options); } -// Force getDepsFromPlugin to return depRoots for processing in assembleLocalPayload -async function getDepsFromPlugin(root, options: Options): Promise { +// Force getDepsFromPlugin to return scannedProjects for processing in assembleLocalPayload +async function getDepsFromPlugin(root, options: Options): Promise { options.file = options.file || detect.detectPackageFile(root); if (!options.docker && !(options.file || options.packageManager)) { throw NoSupportedManifestsFoundError([...root]); } const plugin = plugins.loadPlugin(options.packageManager, options); const moduleInfo = ModuleInfo(plugin, options.policy); - const pluginOptions = plugins.getPluginOptions(options.packageManager, options); - const inspectRes: SingleDepRootResult | MultiDepRootsResult = - await moduleInfo.inspect(root, options.file, { ...options, ...pluginOptions }); + const inspectRes: pluginApi.InspectResult = + await moduleInfo.inspect(root, options.file, { ...options }); - if (!isMultiResult(inspectRes)) { + if (!pluginApi.isMultiResult(inspectRes)) { if (!inspectRes.package) { // something went wrong if both are not present... throw Error(`error getting dependencies from ${options.packageManager} ` + - 'plugin: neither \'package\' nor \'depRoots\' were found'); + 'plugin: neither \'package\' nor \'scannedProjects\' were found'); } if (!inspectRes.package.targetFile && inspectRes.plugin) { inspectRes.package.targetFile = inspectRes.plugin.targetFile; @@ -238,13 +238,13 @@ async function getDepsFromPlugin(root, options: Options): Promise depRoot.depTree.name); + (options as any).subProjectNames = inspectRes.scannedProjects.map((scannedProject) => scannedProject.depTree.name); return inspectRes; } } @@ -263,14 +263,14 @@ async function assembleLocalPayloads(root, options: Options & TestOptions): Prom const deps = await getDepsFromPlugin(root, options); analytics.add('pluginName', deps.plugin.name); - for (const depRoot of deps.depRoots) { - const pkg = depRoot.depTree; + for (const scannedProject of deps.scannedProjects) { + const pkg = scannedProject.depTree; if (options['print-deps']) { await spinner.clear(spinnerLbl)(); maybePrintDeps(options, pkg); } if (deps.plugin && deps.plugin.packageManager) { - options.packageManager = deps.plugin.packageManager; + (options as any).packageManager = deps.plugin.packageManager; } if (pkg.docker) { @@ -332,7 +332,10 @@ async function assembleLocalPayloads(root, options: Options & TestOptions): Prom } else { // Graphs are more compact and robust representations. // Legacy parts of the code are still using trees, but will eventually be fully migrated. - debug('converting dep-tree to dep-graph', {name: pkg.name, targetFile: depRoot.targetFile || options.file}); + debug('converting dep-tree to dep-graph', { + name: pkg.name, + targetFile: scannedProject.targetFile || options.file, + }); let depGraph = await depGraphLib.legacy.depTreeToGraph( pkg, options.packageManager); diff --git a/src/lib/types.ts b/src/lib/types.ts index c1a14d30c4..4bc718322d 100644 --- a/src/lib/types.ts +++ b/src/lib/types.ts @@ -1,6 +1,5 @@ import { SupportedPackageManagers } from './package-managers'; - -// TODO(kyegupov): use a shared repository snyk-cli-interface +import { legacyCommon as legacyApi } from '@snyk/cli-interface'; export interface PluginMetadata { name: string; @@ -19,45 +18,7 @@ export interface DepDict { [name: string]: DepTree; } -export interface DepTree { - name: string; - version: string; - dependencies?: DepDict; - packageFormatVersion?: string; - docker?: any; - files?: any; - targetFile?: string; - missingLockFileEntry?: boolean; - - labels?: { - [key: string]: string; - - // Known keys: - // pruned: identical subtree already presents in the parent node. - // See --prune-repeated-subdependencies flag. - }; -} - -export interface DepRoot { - depTree: DepTree; // to be soon replaced with depGraph - targetFile?: string; -} - -// Legacy result type. Will be deprecated soon. -export interface SingleDepRootResult { - plugin: PluginMetadata; - package: DepTree; -} - -export interface MultiDepRootsResult { - plugin: PluginMetadata; - depRoots: DepRoot[]; -} - -// https://www.typescriptlang.org/docs/handbook/advanced-types.html#user-defined-type-guards -export function isMultiResult(pet: SingleDepRootResult | MultiDepRootsResult): pet is MultiDepRootsResult { - return !!(pet as MultiDepRootsResult).depRoots; -} +export type DepTree = legacyApi.DepTree; export interface TestOptions { traverseNodeModules: boolean; @@ -81,7 +42,7 @@ export interface Options { 'ignore-policy'?: boolean; 'trust-policies'?: boolean; // used in snyk/policy lib 'policy-path'?: boolean; - 'all-sub-projects'?: boolean; // Corresponds to multiDepRoot in plugins + allSubProjects?: boolean; 'project-name'?: string; 'show-vulnerable-paths'?: string; showVulnPaths?: boolean; @@ -100,7 +61,7 @@ export interface MonitorOptions { file?: string; policy?: string; json?: boolean; - 'all-sub-projects'?: boolean; // Corresponds to multiDepRoot in plugins + allSubProjects?: boolean; 'project-name'?: string; 'print-deps'?: boolean; 'experimental-dep-graph'?: boolean; diff --git a/test/acceptance/cli.acceptance.test.ts b/test/acceptance/cli.acceptance.test.ts index 5274e40570..1893260d23 100644 --- a/test/acceptance/cli.acceptance.test.ts +++ b/test/acceptance/cli.acceptance.test.ts @@ -31,6 +31,7 @@ const after = tap.runOnly ? only : test; // Should be after `process.env` setup. import * as plugins from '../../src/lib/plugins'; +import { legacyPlugin as pluginApi } from '@snyk/cli-interface'; // @later: remove this config stuff. // Was copied straight from ../src/cli-server.js @@ -575,7 +576,7 @@ test('`test gradle-app` returns correct meta', async (t) => { const res = await cli.test('gradle-app'); const meta = res.slice(res.indexOf('Organization:')).split('\n'); - t.false(((spyPlugin.args[0] as any)[2] as any).multiDepRoots, '`multiDepRoots` option is not sent'); + t.false(((spyPlugin.args[0] as any)[2] as any).allSubProjects, '`allSubProjects` option is not sent'); t.match(meta[0], /Organization:\s+test-org/, 'organization displayed'); t.match(meta[1], /Package manager:\s+gradle/, 'package manager displayed'); @@ -586,7 +587,7 @@ test('`test gradle-app` returns correct meta', async (t) => { 'local policy not displayed'); }); -test('`test gradle-app --all-sub-projects` sends `multiDepRoots` argument to plugin', async (t) => { +test('`test gradle-app --all-sub-projects` sends `allSubProjects` argument to plugin', async (t) => { chdirWorkspaces(); const plugin = { async inspect() { @@ -599,12 +600,12 @@ test('`test gradle-app --all-sub-projects` sends `multiDepRoots` argument to plu loadPlugin.withArgs('gradle').returns(plugin); await cli.test('gradle-app', { - 'all-sub-projects': true, + allSubProjects: true, }); - t.true(((spyPlugin.args[0] as any)[2] as any).multiDepRoots); + t.true(((spyPlugin.args[0] as any)[2] as any).allSubProjects); }); -test('`test gradle-app` plugin fails to return package or depRoots', async (t) => { +test('`test gradle-app` plugin fails to return package or scannedProjects', async (t) => { chdirWorkspaces(); const plugin = { async inspect() { @@ -621,7 +622,7 @@ test('`test gradle-app` plugin fails to return package or depRoots', async (t) = t.fail('expected error'); } catch (error) { t.match(error, - /error getting dependencies from gradle plugin: neither 'package' nor 'depRoots' were found/, + /error getting dependencies from gradle plugin: neither 'package' nor 'scannedProjects' were found/, 'error found'); } }); @@ -629,10 +630,10 @@ test('`test gradle-app` plugin fails to return package or depRoots', async (t) = test('`test gradle-app --all-sub-projects` returns correct multi tree meta', async (t) => { chdirWorkspaces(); const plugin = { - async inspect() { + async inspect(): Promise { return { plugin: {name: 'gradle'}, - depRoots: [ + scannedProjects: [ { depTree: { name: 'tree0', @@ -655,8 +656,8 @@ test('`test gradle-app --all-sub-projects` returns correct multi tree meta', asy t.teardown(loadPlugin.restore); loadPlugin.withArgs('gradle').returns(plugin); - const res = await cli.test('gradle-app', {'all-sub-projects': true}); - t.true(((spyPlugin.args[0] as any)[2] as any).multiDepRoots, '`multiDepRoots` option is sent'); + const res = await cli.test('gradle-app', {allSubProjects: true}); + t.true(((spyPlugin.args[0] as any)[2] as any).allSubProjects, '`allSubProjects` option is sent'); const tests = res.split('Testing gradle-app...').filter((s) => !!s.trim()); t.equals(tests.length, 2, 'two projects tested independently'); @@ -2659,8 +2660,8 @@ test('`monitor gradle-app --all-sub-projects`', async (t) => { t.teardown(loadPlugin.restore); loadPlugin.withArgs('gradle').returns(plugin); - await cli.monitor('gradle-app', {'all-sub-projects': true}); - t.true(((spyPlugin.args[0] as any)[2] as any).multiDepRoots); + await cli.monitor('gradle-app', {allSubProjects: true}); + t.true(((spyPlugin.args[0] as any)[2] as any).allSubProjects); const req = server.popRequest(); t.equal(req.method, 'PUT', 'makes PUT request'); @@ -2668,8 +2669,7 @@ test('`monitor gradle-app --all-sub-projects`', async (t) => { t.match(req.url, '/monitor/gradle', 'puts at correct url'); t.same(spyPlugin.getCall(0).args, ['gradle-app', 'build.gradle', { - 'all-sub-projects': true, - 'multiDepRoots': true, + 'allSubProjects': true, 'args': null, }], 'calls gradle plugin'); }); @@ -2691,8 +2691,8 @@ test('`monitor gradle-app pip-app --all-sub-projects`', async (t) => { loadPlugin.withArgs('gradle').returns(plugin); loadPlugin.withArgs('pip').returns(plugin); - await cli.monitor('gradle-app', 'pip-app', {'all-sub-projects': true}); - t.true(((spyPlugin.args[0] as any)[2] as any).multiDepRoots); + await cli.monitor('gradle-app', 'pip-app', {allSubProjects: true}); + t.true(((spyPlugin.args[0] as any)[2] as any).allSubProjects); let req = server.popRequest(); t.equal(req.method, 'PUT', 'makes PUT request for pip'); @@ -2705,15 +2705,13 @@ test('`monitor gradle-app pip-app --all-sub-projects`', async (t) => { t.same(spyPlugin.getCall(0).args, ['gradle-app', 'build.gradle', { - 'all-sub-projects': true, - 'multiDepRoots': true, - 'args': null, + allSubProjects: true, + args: null, }], 'calls plugin for the 1st path'); t.same(spyPlugin.getCall(1).args, ['pip-app', 'requirements.txt', { - 'all-sub-projects': true, - // No multiDepRoots, because only Gradle plugin loader sets it - 'args': null, + allSubProjects: true, + args: null, }], 'calls plugin for the 2nd path'); }); @@ -2734,7 +2732,7 @@ test('`monitor gradle-app --all-sub-projects --project-name`', async (t) => { loadPlugin.withArgs('gradle').returns(plugin); try { - await cli.monitor('gradle-app', {'all-sub-projects': true, 'project-name': 'frumpus'}); + await cli.monitor('gradle-app', {allSubProjects: true, 'project-name': 'frumpus'}); } catch (e) { t.contains(e, /is currently not compatible/); }