From d0f58c3702386f392dba29318d72a753f50b7ac7 Mon Sep 17 00:00:00 2001 From: Liliana Kastilio Date: Thu, 21 May 2020 21:53:01 +0100 Subject: [PATCH] feat: skip previously imported targets --- src/common.ts | 2 + src/lib/get-concurrent-imports-number.ts | 8 +++ src/lib/get-logging-path.ts | 10 +++ src/lib/import/index.ts | 14 +++- src/load-file.ts | 4 +- src/log-imported-targets.ts | 18 +++++ src/scripts/import-projects.ts | 41 ++++++++++-- src/write-file.ts | 16 +++++ test/lib/index.test.ts | 15 +++-- .../import-projects.test.ts.snap | 10 +++ .../with-import-log/import-projects.json | 13 ++++ .../with-import-log/imported-targets.log | 1 + test/scripts/import-projects.test.ts | 66 +++++++++++++++---- 13 files changed, 191 insertions(+), 27 deletions(-) create mode 100644 src/common.ts create mode 100644 src/lib/get-concurrent-imports-number.ts create mode 100644 src/lib/get-logging-path.ts create mode 100644 src/log-imported-targets.ts create mode 100644 src/write-file.ts create mode 100644 test/scripts/__snapshots__/import-projects.test.ts.snap create mode 100644 test/scripts/fixtures/with-import-log/import-projects.json create mode 100644 test/scripts/fixtures/with-import-log/imported-targets.log diff --git a/src/common.ts b/src/common.ts new file mode 100644 index 00000000..76196768 --- /dev/null +++ b/src/common.ts @@ -0,0 +1,2 @@ +export const IMPORT_LOG_NAME = 'imported-targets.log'; +export const IMPORT_PROJECTS_FILE_NAME= 'import-projects.json'; diff --git a/src/lib/get-concurrent-imports-number.ts b/src/lib/get-concurrent-imports-number.ts new file mode 100644 index 00000000..bcfc786f --- /dev/null +++ b/src/lib/get-concurrent-imports-number.ts @@ -0,0 +1,8 @@ +export function getConcurrentImportsNumber(): number { + let importsNumber = parseInt(process.env.CONCURRENT_IMPORTS || '5'); + if (importsNumber > 20) { + // never more than 20! + importsNumber = 20; + } + return importsNumber; +} diff --git a/src/lib/get-logging-path.ts b/src/lib/get-logging-path.ts new file mode 100644 index 00000000..86d8bda7 --- /dev/null +++ b/src/lib/get-logging-path.ts @@ -0,0 +1,10 @@ +export function getLoggingPath(): string { + const snykLogPath = process.env.SNYK_LOG_PATH; + if (!snykLogPath) { + throw new Error( + `Please set the SNYK_LOG_PATH e.g. export SNYK_LOG_PATH='~/my/path'`, + ); + } + // TODO: what if path is not real? + return snykLogPath; +} diff --git a/src/lib/import/index.ts b/src/lib/import/index.ts index 2e9b7512..4ec2735b 100644 --- a/src/lib/import/index.ts +++ b/src/lib/import/index.ts @@ -6,6 +6,9 @@ import * as _ from 'lodash'; import { Target, FilePath, ImportTarget } from '../types'; import { getApiToken } from '../get-api-token'; import { getSnykHost } from '../get-snyk-host'; +import { logImportedTarget } from '../../log-imported-targets'; +import { getLoggingPath } from '../get-logging-path'; +import { getConcurrentImportsNumber } from '../get-concurrent-imports-number'; const debug = debugLib('snyk:api-import'); @@ -13,7 +16,8 @@ export async function importTarget( orgId: string, integrationId: string, target: Target, - files?: FilePath[], + files?: FilePath[] | undefined, + loggingPath?: string, ): Promise<{ pollingUrl: string }> { const apiToken = getApiToken(); debug('Importing:', JSON.stringify({ orgId, integrationId, target })); @@ -46,7 +50,8 @@ export async function importTarget( ); if (res.statusCode && res.statusCode !== 201) { throw new Error( - 'Expected a 201 response, instead received: ' + JSON.stringify(res.body), + 'Expected a 201 response, instead received: ' + + JSON.stringify(res.body), ); } const locationUrl = res.headers['location']; @@ -57,6 +62,7 @@ export async function importTarget( } // TODO: log success debug(`Received locationUrl for ${target.name}: ${locationUrl}`); + logImportedTarget(orgId, integrationId, target, locationUrl, loggingPath); return { pollingUrl: locationUrl }; } catch (error) { // TODO: log failure @@ -72,6 +78,7 @@ export async function importTarget( export async function importTargets( targets: ImportTarget[], + loggingPath = getLoggingPath(), ): Promise { const pollingUrls: string[] = []; // TODO: filter out previously processed @@ -86,6 +93,7 @@ export async function importTargets( integrationId, target, files, + loggingPath, ); // TODO: log all succeeded into a file pollingUrls.push(pollingUrl); @@ -94,7 +102,7 @@ export async function importTargets( debug('Failed to process:', JSON.stringify(t)); } }, - { concurrency: 5 }, + { concurrency: getConcurrentImportsNumber() }, ); return _.uniq(pollingUrls); } diff --git a/src/load-file.ts b/src/load-file.ts index 3619695e..bc25dc53 100644 --- a/src/load-file.ts +++ b/src/load-file.ts @@ -4,11 +4,11 @@ import * as debugLib from 'debug'; const debug = debugLib('snyk:load-file'); export async function loadFile(name: string): Promise { - const filename = path.resolve(__dirname, name); + const filename = path.resolve(name); try { return await fs.readFileSync(filename, 'utf8'); } catch (error) { - debug(error); + debug(error.message); throw new Error(`File can not be found at location: ${filename}`); } } diff --git a/src/log-imported-targets.ts b/src/log-imported-targets.ts new file mode 100644 index 00000000..605ab527 --- /dev/null +++ b/src/log-imported-targets.ts @@ -0,0 +1,18 @@ +import * as fs from 'fs'; +import { Target } from './lib/types'; +import { getLoggingPath } from './lib/get-logging-path'; + +export async function logImportedTarget( + orgId: string, + integrationId: string, + target: Target, + locationUrl: string, + loggingPath: string = getLoggingPath(), +): Promise { + try { + const log = `${orgId}:${integrationId}:${Object.values(target).join(':')},`; + fs.appendFileSync(`${loggingPath}/imported-targets.log`, log); + } catch (e) { + // do nothing + } +} diff --git a/src/scripts/import-projects.ts b/src/scripts/import-projects.ts index fb798ea0..342d1acd 100644 --- a/src/scripts/import-projects.ts +++ b/src/scripts/import-projects.ts @@ -1,16 +1,47 @@ import * as debugLib from 'debug'; - +import * as path from 'path'; import { loadFile } from '../load-file'; import { importTargets, pollImportUrls } from '../lib'; -import { Project } from '../lib/types'; +import { Project, ImportTarget } from '../lib/types'; +import { getLoggingPath } from '../lib/get-logging-path'; const debug = debugLib('snyk:import-projects-script'); +const regexForTarget = (target: string): RegExp => + new RegExp(`(,?)${target}.*,`, 'm'); + +async function filterOutImportedTargets( + targets: ImportTarget[], + loggingPath: string, +): Promise { + let logFile: string; + const filterOutImportedTargets: ImportTarget[] = []; + try { + logFile = await loadFile(path.resolve(loggingPath, 'imported-targets.log')); + } catch (e) { + return targets; + } + targets.forEach((targetItem) => { + const { orgId, integrationId, target } = targetItem; + const data = `${orgId}:${integrationId}:${Object.values(target).join(':')}`; + const targetRegExp = regexForTarget(data); + const match = logFile.match(targetRegExp); + if (!match) { + filterOutImportedTargets.push(targetItem); + } else { + debug('Dropped previously imported target: ', JSON.stringify(targetItem)); + } + }); + + return filterOutImportedTargets; +} + export async function ImportProjects( fileName: string, + loggingPath: string = getLoggingPath(), ): Promise { const content = await loadFile(fileName); - const targets = []; + const targets: ImportTarget[] = []; try { targets.push(...JSON.parse(content).targets); } catch (e) { @@ -18,7 +49,9 @@ export async function ImportProjects( } debug(`Loaded ${targets.length} targets to import`); //TODO: validation? - const pollingUrls = await importTargets(targets); + const filteredTargets = await filterOutImportedTargets(targets, loggingPath); + const pollingUrls = await importTargets(filteredTargets, loggingPath); const projects = await pollImportUrls(pollingUrls); + return projects; } diff --git a/src/write-file.ts b/src/write-file.ts new file mode 100644 index 00000000..afa4be28 --- /dev/null +++ b/src/write-file.ts @@ -0,0 +1,16 @@ +import * as fs from 'fs'; +import * as debugLib from 'debug'; +const debug = debugLib('snyk:write-file'); +import { getLoggingPath } from './lib/get-logging-path'; + +export async function writeFile(name: string, content: JSON): Promise { + const ROOT_DIR = getLoggingPath(); + const filename = `${ROOT_DIR}/${name}`; + + try { + return await fs.writeFileSync(filename, JSON.stringify(content)); + } catch (error) { + debug(error); + throw new Error(`Failed to write to file: ${filename}`); + } +} diff --git a/test/lib/index.test.ts b/test/lib/index.test.ts index c38c77ae..19627f02 100644 --- a/test/lib/index.test.ts +++ b/test/lib/index.test.ts @@ -25,11 +25,15 @@ async function deleteTestProjects( describe('Single target', () => { const discoveredProjects: Project[] = []; it('Import & poll a repo', async () => { - const { pollingUrl } = await importTarget(ORG_ID, GITHUB_INTEGRATION_ID, { - name: 'shallow-goof-policy', - owner: 'snyk-fixtures', - branch: 'master', - }); + const { pollingUrl } = await importTarget( + ORG_ID, + GITHUB_INTEGRATION_ID, + { + name: 'shallow-goof-policy', + owner: 'snyk-fixtures', + branch: 'master', + }, + ); expect(pollingUrl).not.toBeNull(); const projects = await pollImportUrl(pollingUrl); expect(projects[0]).toMatchObject({ @@ -95,4 +99,3 @@ describe('Multiple targets', () => { test.todo('Failed import 100%'); test.todo('Only 1 import fails out of a few + logs created'); -test.todo('If we stopped half way, restarted from where we left'); diff --git a/test/scripts/__snapshots__/import-projects.test.ts.snap b/test/scripts/__snapshots__/import-projects.test.ts.snap new file mode 100644 index 00000000..e036e740 --- /dev/null +++ b/test/scripts/__snapshots__/import-projects.test.ts.snap @@ -0,0 +1,10 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Import projects script Skips any badly formatted targets 1`] = `"f0125d9b-271a-4b50-ad23-80e12575a1bf:c4de291b-e083-4c43-a72c-113463e0d268:shallow-goof-policy:snyk-fixtures:master:https://dev.snyk.io/api/v1/org/f0125d9b-271a-4b50-ad23-80e12575a1bf/integrations/c4de291b-e083-4c43-a72c-113463e0d268/import/63f06690-1485-40cf-b81f-218104cfb277,f0125d9b-271a-4b50-ad23-80e12575a1bf:c4de291b-e083-4c43-a72c-113463e0d268:ruby-with-versions:snyk-fixtures:master:https://dev.snyk.io/api/v1/org/f0125d9b-271a-4b50-ad23-80e12575a1bf/integrations/c4de291b-e083-4c43-a72c-113463e0d268/import/63f06690-1485-40cf-b81f-218104cfb277,f0125d9b-271a-4b50-ad23-80e12575a1bf:c4de291b-e083-4c43-a72c-113463e0d268:composer-with-vulns:snyk-fixtures:master:https://dev.snyk.io/api/v1/org/f0125d9b-271a-4b50-ad23-80e12575a1bf/integrations/c4de291b-e083-4c43-a72c-113463e0d268/import/63f06690-1485-40cf-b81f-218104cfb277,f0125d9b-271a-4b50-ad23-80e12575a1bf:c4de291b-e083-4c43-a72c-113463e0d268:shallow-goof-policy:snyk-fixtures:master:https://dev.snyk.io/api/v1/org/f0125d9b-271a-4b50-ad23-80e12575a1bf/integrations/c4de291b-e083-4c43-a72c-113463e0d268/import/8ba73ea5-e1ff-4513-ad5a-8931ba3bb4d1,"`; + +exports[`Import projects script succeeds to import targets from file 1`] = `"f0125d9b-271a-4b50-ad23-80e12575a1bf:c4de291b-e083-4c43-a72c-113463e0d268:ruby-with-versions:snyk-fixtures:master,f0125d9b-271a-4b50-ad23-80e12575a1bf:c4de291b-e083-4c43-a72c-113463e0d268:composer-with-vulns:snyk-fixtures:master,f0125d9b-271a-4b50-ad23-80e12575a1bf:c4de291b-e083-4c43-a72c-113463e0d268:shallow-goof-policy:snyk-fixtures:master,"`; + +exports[`Import skips previously imported succeeds to import targets from file 1`] = ` +"f0125d9b-271a-4b50-ad23-80e12575a1bf:c4de291b-e083-4c43-a72c-113463e0d268:composer-with-vulns:snyk-fixtures:master, +" +`; diff --git a/test/scripts/fixtures/with-import-log/import-projects.json b/test/scripts/fixtures/with-import-log/import-projects.json new file mode 100644 index 00000000..3523bd64 --- /dev/null +++ b/test/scripts/fixtures/with-import-log/import-projects.json @@ -0,0 +1,13 @@ +{ + "targets": [ + { + "orgId": "f0125d9b-271a-4b50-ad23-80e12575a1bf", + "integrationId": "c4de291b-e083-4c43-a72c-113463e0d268", + "target": { + "name": "composer-with-vulns", + "owner": "snyk-fixtures", + "branch": "master" + } + } + ] +} diff --git a/test/scripts/fixtures/with-import-log/imported-targets.log b/test/scripts/fixtures/with-import-log/imported-targets.log new file mode 100644 index 00000000..989cb223 --- /dev/null +++ b/test/scripts/fixtures/with-import-log/imported-targets.log @@ -0,0 +1 @@ +f0125d9b-271a-4b50-ad23-80e12575a1bf:c4de291b-e083-4c43-a72c-113463e0d268:composer-with-vulns:snyk-fixtures:master, diff --git a/test/scripts/import-projects.test.ts b/test/scripts/import-projects.test.ts index 3bc17fa7..a28474b3 100644 --- a/test/scripts/import-projects.test.ts +++ b/test/scripts/import-projects.test.ts @@ -1,11 +1,17 @@ import * as path from 'path'; +import * as fs from 'fs'; import { ImportProjects } from '../../src/scripts/import-projects'; +import { IMPORT_PROJECTS_FILE_NAME, IMPORT_LOG_NAME } from '../../src/common'; -// TODO: after all delete the new projects describe('Import projects script', () => { + const logPath = path.resolve(__dirname, IMPORT_LOG_NAME); + afterEach(() => { + fs.unlinkSync(logPath); + }); it('succeeds to import targets from file', async () => { const projects = await ImportProjects( - path.resolve(__dirname + '/fixtures/import-projects.json'), + path.resolve(__dirname + `/fixtures/${IMPORT_PROJECTS_FILE_NAME}`), + __dirname, ); expect(projects).not.toBe([]); expect(projects[0]).toMatchObject({ @@ -13,11 +19,54 @@ describe('Import projects script', () => { success: true, targetFile: expect.any(String), }); + const logFile = fs.readFileSync(logPath, 'utf8'); + expect(logFile).toMatch('shallow-goof-policy'); + expect(logFile).toMatch('composer-with-vulns'); + expect(logFile).toMatch('ruby-with-versions:'); }, 30000000); - it('shows correct error when input can not be loaded', async () => { - expect(ImportProjects('do-not-exist/import-projects.json')).rejects.toThrow( - 'File can not be found at location', +}); + +describe('Import skips previously imported', () => { + const logPath = path.resolve( + __dirname + '/fixtures/with-import-log', + IMPORT_LOG_NAME, + ); + it('succeeds to import targets from file', async () => { + const projects = await ImportProjects( + path.resolve( + __dirname + `/fixtures/with-import-log/${IMPORT_PROJECTS_FILE_NAME}`, + ), + __dirname + '/fixtures/with-import-log', ); + expect(projects.length === 0).toBeTruthy(); + const logFile = fs.readFileSync(logPath, 'utf8'); + expect(logFile).toMatchSnapshot(); + }, 30000000); +}); + +describe('Skips issues', () => { + const logPath = path.resolve(__dirname, IMPORT_LOG_NAME); + it('Skips any badly formatted targets', async () => { + // TODO: ensure all failures are logged & assert it is present in the log + const projects = await ImportProjects( + path.resolve(__dirname + '/fixtures/import-projects-invalid-target.json'), + __dirname, + ); + expect(projects.length === 0).toBeTruthy(); + let logFile = null; + try { + logFile = fs.readFileSync(logPath, 'utf8'); + } catch (e) { + expect(logFile).toBeNull(); + } + }, 300); +}); + +describe('Error handling', () => { + it('shows correct error when input can not be loaded', async () => { + expect( + ImportProjects(`do-not-exist/${IMPORT_PROJECTS_FILE_NAME}`), + ).rejects.toThrow('File can not be found at location'); }, 300); it('shows correct error when input is invalid json', async () => { expect( @@ -26,11 +75,4 @@ describe('Import projects script', () => { ), ).rejects.toThrow('Failed to parse targets from'); }, 300); - it('Skips any badly formatted targets', async () => { - // TODO: ensure all failures are logged & assert it is present in the log - const projects = await ImportProjects( - path.resolve(__dirname + '/fixtures/import-projects-invalid-target.json'), - ); - expect(projects.length).toBe(0); - }, 300); });