-
Notifications
You must be signed in to change notification settings - Fork 578
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feat: snyk fix v1 (Python) in behind FF #1707
Changes from all commits
626ff6d
cbeeeaf
34df01d
4ef96f5
23bd100
c68c7da
ca508ac
77e6665
3d872fb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
import { DepGraphData } from '@snyk/dep-graph'; | ||
import { TestResult } from '../../../lib/ecosystems/types'; | ||
import { TestResult as LegacyTestResult } from '../../../lib/snyk-test/legacy'; | ||
|
||
export function convertLegacyTestResultToNew( | ||
testResult: LegacyTestResult, | ||
): TestResult { | ||
return { | ||
issuesData: {} as any, // TODO: add converter | ||
issues: [], // TODO: add converter | ||
remediation: testResult.remediation, | ||
// TODO: grab this once Ecosystems flow starts sending back ScanResult | ||
depGraphData: {} as DepGraphData, | ||
}; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
import { ScanResult } from '../../../lib/ecosystems/types'; | ||
import { TestResult } from '../../../lib/snyk-test/legacy'; | ||
|
||
export function convertLegacyTestResultToScanResult( | ||
testResult: TestResult, | ||
): ScanResult { | ||
if (!testResult.packageManager) { | ||
throw new Error( | ||
'Only results with packageManagers are supported for conversion', | ||
); | ||
} | ||
return { | ||
identity: { | ||
type: testResult.packageManager, | ||
// this is because not all plugins send it back today, but we should always have it | ||
targetFile: testResult.targetFile || testResult.displayTargetFile, | ||
}, | ||
name: testResult.projectName, | ||
// TODO: grab this once Ecosystems flow starts sending back ScanResult | ||
facts: [], | ||
policy: testResult.policy, | ||
// TODO: grab this once Ecosystems flow starts sending back ScanResult | ||
target: {} as any, | ||
}; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
import * as fs from 'fs'; | ||
import * as pathLib from 'path'; | ||
import { convertLegacyTestResultToNew } from './convert-legacy-test-result-to-new'; | ||
import { convertLegacyTestResultToScanResult } from './convert-legacy-test-result-to-scan-result'; | ||
import { TestResult } from '../../../lib/snyk-test/legacy'; | ||
|
||
export function convertLegacyTestResultToFixEntities( | ||
testResults: (TestResult | TestResult[]) | Error, | ||
root: string, | ||
): any { | ||
if (testResults instanceof Error) { | ||
return []; | ||
} | ||
const oldResults = Array.isArray(testResults) ? testResults : [testResults]; | ||
return oldResults.map((res) => ({ | ||
workspace: { | ||
readFile: async (path: string) => { | ||
return fs.readFileSync(pathLib.resolve(root, path), 'utf8'); | ||
}, | ||
writeFile: async (path: string, content: string) => { | ||
return fs.writeFileSync(pathLib.resolve(root, path), content, 'utf8'); | ||
}, | ||
}, | ||
scanResult: convertLegacyTestResultToScanResult(res), | ||
testResult: convertLegacyTestResultToNew(res), | ||
})); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
import * as pathLib from 'path'; | ||
|
||
import { isLocalFolder } from '../../../lib/detect'; | ||
|
||
export function getDisplayPath(path: string): string { | ||
if (!isLocalFolder(path)) { | ||
return path; | ||
} | ||
if (path === process.cwd()) { | ||
return '.'; | ||
} | ||
return pathLib.relative(process.cwd(), path); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,109 @@ | ||
export = fix; | ||
|
||
import * as Debug from 'debug'; | ||
import * as snykFix from '@snyk/fix'; | ||
import * as ora from 'ora'; | ||
|
||
import { MethodArgs } from '../../args'; | ||
import * as snyk from '../../../lib'; | ||
import { TestResult } from '../../../lib/snyk-test/legacy'; | ||
|
||
import { convertLegacyTestResultToFixEntities } from './convert-legacy-tests-results-to-fix-entities'; | ||
import { formatTestError } from '../test/format-test-error'; | ||
import { processCommandArgs } from '../process-command-args'; | ||
import { validateCredentials } from '../test/validate-credentials'; | ||
import { validateTestOptions } from '../test/validate-test-options'; | ||
import { setDefaultTestOptions } from '../test/set-default-test-options'; | ||
import { validateFixCommandIsSupported } from './validate-fix-command-is-supported'; | ||
import { Options, TestOptions } from '../../../lib/types'; | ||
import { getDisplayPath } from './get-display-path'; | ||
|
||
const debug = Debug('snyk-fix'); | ||
const snykFixFeatureFlag = 'cliSnykFix'; | ||
|
||
interface FixOptions { | ||
dryRun?: boolean; | ||
quiet?: boolean; | ||
} | ||
async function fix(...args: MethodArgs): Promise<string> { | ||
const { options: rawOptions, paths } = await processCommandArgs<FixOptions>( | ||
...args, | ||
); | ||
const options = setDefaultTestOptions<FixOptions>(rawOptions); | ||
debug(options); | ||
await validateFixCommandIsSupported(options); | ||
validateTestOptions(options); | ||
validateCredentials(options); | ||
const results: snykFix.EntityToFix[] = []; | ||
results.push(...(await runSnykTestLegacy(options, paths))); | ||
|
||
// fix | ||
debug( | ||
`Organization has ${snykFixFeatureFlag} feature flag enabled for experimental Snyk fix functionality`, | ||
); | ||
const { dryRun, quiet } = options; | ||
const { fixSummary, meta } = await snykFix.fix(results, { dryRun, quiet }); | ||
if (meta.fixed === 0) { | ||
throw new Error(fixSummary); | ||
} | ||
return fixSummary; | ||
} | ||
|
||
/* @deprecated | ||
* TODO: once project envelope is default all code below will be deleted | ||
* we should be calling test via new Ecosystems instead | ||
*/ | ||
async function runSnykTestLegacy( | ||
options: Options & TestOptions & FixOptions, | ||
paths: string[], | ||
): Promise<snykFix.EntityToFix[]> { | ||
const results: snykFix.EntityToFix[] = []; | ||
const stdOutSpinner = ora({ | ||
isSilent: options.quiet, | ||
stream: process.stdout, | ||
}); | ||
const stdErrSpinner = ora({ | ||
isSilent: options.quiet, | ||
stream: process.stdout, | ||
}); | ||
stdErrSpinner.start(); | ||
stdOutSpinner.start(); | ||
|
||
for (const path of paths) { | ||
let displayPath = path; | ||
try { | ||
displayPath = getDisplayPath(path); | ||
stdOutSpinner.info(`Running \`snyk test\` for ${displayPath}`); | ||
// Create a copy of the options so a specific test can | ||
// modify them i.e. add `options.file` etc. We'll need | ||
// these options later. | ||
const snykTestOptions = { | ||
...options, | ||
path, | ||
projectName: options['project-name'], | ||
}; | ||
|
||
const testResults: TestResult[] = []; | ||
|
||
const testResultForPath: TestResult | TestResult[] = await snyk.test( | ||
path, | ||
{ ...snykTestOptions, quiet: true }, | ||
); | ||
testResults.push( | ||
...(Array.isArray(testResultForPath) | ||
? testResultForPath | ||
: [testResultForPath]), | ||
); | ||
const newRes = convertLegacyTestResultToFixEntities(testResults, path); | ||
results.push(...newRes); | ||
} catch (error) { | ||
const testError = formatTestError(error); | ||
const userMessage = `Test for ${displayPath} failed with error: ${testError.message}.\nRun \`snyk test ${displayPath} -d\` for more information.`; | ||
stdErrSpinner.fail(userMessage); | ||
debug(userMessage); | ||
} | ||
} | ||
stdOutSpinner.stop(); | ||
stdErrSpinner.stop(); | ||
return results; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
import * as Debug from 'debug'; | ||
|
||
import { getEcosystemForTest } from '../../../lib/ecosystems'; | ||
|
||
import { isFeatureFlagSupportedForOrg } from '../../../lib/feature-flags'; | ||
import { CommandNotSupportedError } from '../../../lib/errors/command-not-supported'; | ||
import { FeatureNotSupportedByEcosystemError } from '../../../lib/errors/not-supported-by-ecosystem'; | ||
import { Options, TestOptions } from '../../../lib/types'; | ||
|
||
const debug = Debug('snyk-fix'); | ||
const snykFixFeatureFlag = 'cliSnykFix'; | ||
|
||
export async function validateFixCommandIsSupported( | ||
options: Options & TestOptions, | ||
): Promise<boolean> { | ||
if (options.docker) { | ||
throw new FeatureNotSupportedByEcosystemError('snyk fix', 'docker'); | ||
} | ||
|
||
const ecosystem = getEcosystemForTest(options); | ||
if (ecosystem) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Am I missing something here? We will always throw FeatureNotSupportedByEcosystemError unless the ecosystem is null in which case we do support it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thats correct, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok. So a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thats the same as in |
||
throw new FeatureNotSupportedByEcosystemError('snyk fix', ecosystem); | ||
} | ||
|
||
const snykFixSupported = await isFeatureFlagSupportedForOrg( | ||
snykFixFeatureFlag, | ||
options.org, | ||
); | ||
|
||
if (!snykFixSupported.ok) { | ||
debug(snykFixSupported.userMessage); | ||
throw new CommandNotSupportedError('snyk fix', options.org || undefined); | ||
} | ||
|
||
return true; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
import { CustomError } from './custom-error'; | ||
|
||
export class CommandNotSupportedError extends CustomError { | ||
public readonly command: string; | ||
public readonly org?: string; | ||
|
||
constructor(command: string, org?: string) { | ||
super(`${command} is not supported for org ${org}.`); | ||
this.code = 422; | ||
this.command = command; | ||
this.org = org; | ||
|
||
this.userMessage = `\`${command}\` is not supported ${ | ||
org ? `for org '${org}'` : '' | ||
}`; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
import { CustomError } from './custom-error'; | ||
import { SupportedPackageManagers } from '../package-managers'; | ||
import { Ecosystem } from '../ecosystems/types'; | ||
|
||
export class FeatureNotSupportedByEcosystemError extends CustomError { | ||
public readonly feature: string; | ||
|
||
constructor( | ||
feature: string, | ||
ecosystem: SupportedPackageManagers | Ecosystem, | ||
) { | ||
super(`Unsupported ecosystem ${ecosystem} for ${feature}.`); | ||
this.code = 422; | ||
this.feature = feature; | ||
|
||
this.userMessage = `\`${feature}\` is not supported for ecosystem '${ecosystem}'`; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -85,7 +85,7 @@ export async function getDepsFromPlugin( | |
root, | ||
); | ||
|
||
if (!options.json && userWarningMessage) { | ||
if (!options.json && !options.quiet && userWarningMessage) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
console.warn(chalk.bold.red(userWarningMessage)); | ||
} | ||
return inspectRes; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,7 +31,9 @@ export async function assembleEcosystemPayloads( | |
path.relative('..', '.') + ' project dir'); | ||
|
||
spinner.clear<void>(spinnerLbl)(); | ||
await spinner(spinnerLbl); | ||
if (!options.quiet) { | ||
await spinner(spinnerLbl); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
||
try { | ||
const plugin = getPlugin(ecosystem); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
snyk fix
&@snyk/fix
uses Ora, intention is for this to be the future and only spinner.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm trying to figure out why is nothing failing, if the
@snyk/fix
is not defined in package.json 🤔 I guess we need a new CI check to make sure that nonpm link
ed dependencies are usedThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just added it now via
npm i --save @snyk/fix
is the version as expected?