From 4960d4b1d3c69eb991caeafdab5871aac98ed82d Mon Sep 17 00:00:00 2001 From: Zvi Grinberg Date: Tue, 12 Mar 2024 18:46:13 +0200 Subject: [PATCH 1/5] feat: fix performance issue in python version >= 3.11.X Signed-off-by: Zvi Grinberg --- src/providers/python_controller.js | 119 ++++++++++++++++++++++++----- src/providers/python_pip.js | 2 +- 2 files changed, 99 insertions(+), 22 deletions(-) diff --git a/src/providers/python_controller.js b/src/providers/python_controller.js index 90c044f..8472bc5 100644 --- a/src/providers/python_controller.js +++ b/src/providers/python_controller.js @@ -161,24 +161,54 @@ export default class Python_controller { } #getDependenciesImpl(includeTransitive) { let dependencies = new Array() - let freezeOutput = getPipFreezeOutput.call(this); - //debug - // freezeOutput = "alternative pip freeze output goes here for debugging" - let lines = freezeOutput.split(EOL) - let depNames = lines.map( line => getDependencyName(line)).join(" ") - let pipShowOutput = getPipShowOutput.call(this, depNames); + let usePipDepTree = getCustom("EXHORT_PIP_USE_DEP_TREE","false",this.options); + let freezeOutput + let lines + let depNames + let pipShowOutput + let allPipShowDeps + let pipDepTreeJsonArrayOutput + if(usePipDepTree !== "true") { + freezeOutput = getPipFreezeOutput.call(this); + lines = freezeOutput.split(EOL) + depNames = lines.map( line => getDependencyName(line)).join(" ") + } + else { + pipDepTreeJsonArrayOutput = getDependencyTreeJsonFromPipDepTree(this.pathToPipBin,this.pathToPythonBin) + } + + + if(usePipDepTree !== "true") { + pipShowOutput = getPipShowOutput.call(this, depNames); + allPipShowDeps = pipShowOutput.split( EOL +"---" + EOL); + } //debug // pipShowOutput = "alternative pip show output goes here for debugging" - let allPipShowDeps = pipShowOutput.split( EOL +"---" + EOL); + let matchManifestVersions = getCustom("MATCH_MANIFEST_VERSIONS","true",this.options); let linesOfRequirements = fs.readFileSync(this.pathToRequirements).toString().split(EOL).filter( (line) => !line.startsWith("#")).map(line => line.trim()) let CachedEnvironmentDeps = {} - allPipShowDeps.forEach( (record) => { - let dependencyName = getDependencyNameShow(record).toLowerCase() - CachedEnvironmentDeps[dependencyName] = record - CachedEnvironmentDeps[dependencyName.replace("-","_")] = record - CachedEnvironmentDeps[dependencyName.replace("_","-")] = record - }) + if(usePipDepTree !== "true") { + allPipShowDeps.forEach((record) => { + let dependencyName = getDependencyNameShow(record).toLowerCase() + CachedEnvironmentDeps[dependencyName] = record + CachedEnvironmentDeps[dependencyName.replace("-", "_")] = record + CachedEnvironmentDeps[dependencyName.replace("_", "-")] = record + }) + } + else { + pipDepTreeJsonArrayOutput.forEach( depTreeEntry => { + let packageName = depTreeEntry["package"]["package_name"].toLowerCase() + let pipDepTreeEntryForCache = { + name: packageName, + version: depTreeEntry["package"]["installed_version"], + dependencies: depTreeEntry["dependencies"].map(dep => dep["package_name"]) + }; + CachedEnvironmentDeps[packageName] = pipDepTreeEntryForCache + CachedEnvironmentDeps[packageName.replace("-", "_")] = pipDepTreeEntryForCache + CachedEnvironmentDeps[packageName.replace("_", "-")] = pipDepTreeEntryForCache + }) + } linesOfRequirements.forEach( (dep) => { // if matchManifestVersions setting is turned on , then if(matchManifestVersions === "true") @@ -199,7 +229,12 @@ export default class Python_controller { dependencyName = getDependencyName(dep) // only compare between declared version in manifest to installed version , if the package is installed. if(CachedEnvironmentDeps[dependencyName.toLowerCase()] !== undefined) { - installedVersion = getDependencyVersion(CachedEnvironmentDeps[dependencyName.toLowerCase()]) + if(usePipDepTree !== "true") { + installedVersion = getDependencyVersion(CachedEnvironmentDeps[dependencyName.toLowerCase()]) + } + else { + installedVersion = CachedEnvironmentDeps[dependencyName.toLowerCase()].version + } } if(installedVersion) { if (manifestVersion.trim() !== installedVersion.trim()) { @@ -213,7 +248,7 @@ export default class Python_controller { let depName = getDependencyName(dep) //array to track a path for each branch in the dependency tree path.push(depName.toLowerCase()) - bringAllDependencies(dependencies,depName,CachedEnvironmentDeps,includeTransitive,path) + bringAllDependencies(dependencies,depName,CachedEnvironmentDeps,includeTransitive,path,usePipDepTree) }) dependencies.sort((dep1,dep2) =>{ const DEP1 = dep1.name.toLowerCase() @@ -285,9 +320,10 @@ function getDepsList(record) { * @param dependencyName * @param cachedEnvironmentDeps * @param includeTransitive + * @param usePipDepTree * @param {[string]}path array representing the path of the current branch in dependency tree, starting with a root dependency - that is - a given dependency in requirements.txt */ -function bringAllDependencies(dependencies, dependencyName, cachedEnvironmentDeps, includeTransitive,path) { +function bringAllDependencies(dependencies, dependencyName, cachedEnvironmentDeps, includeTransitive, path, usePipDepTree) { if(dependencyName === null || dependencyName === undefined || dependencyName.trim() === "" ) { return } @@ -298,12 +334,22 @@ function bringAllDependencies(dependencies, dependencyName, cachedEnvironmentDep environment variable EXHORT_PYTHON_VIRTUAL_ENV=true to automatically installs it on virtual environment ( will slow down the analysis) `) } - - let version = getDependencyVersion(record) - let directDeps = getDepsList(record) + let depName + let version; + let directDeps + if(usePipDepTree !== "true") { + depName = getDependencyNameShow(record) + version = getDependencyVersion(record); + directDeps = getDepsList(record) + } + else { + depName = record.name + version = record.version + directDeps = record.dependencies + } let targetDeps = new Array() - let entry = { "name" : getDependencyNameShow(record) , "version" : version, "dependencies" : [] } + let entry = { "name" : depName , "version" : version, "dependencies" : [] } dependencies.push(entry) directDeps.forEach( (dep) => { let depArray = new Array() @@ -313,7 +359,7 @@ function bringAllDependencies(dependencies, dependencyName, cachedEnvironmentDep depArray.push(dep.toLowerCase()) if (includeTransitive) { // send to recurrsion the array of all deps in path + the current dependency name which is not on the path. - bringAllDependencies(targetDeps, dep, cachedEnvironmentDeps, includeTransitive,path.concat(depArray)) + bringAllDependencies(targetDeps, dep, cachedEnvironmentDeps, includeTransitive, path.concat(depArray), usePipDepTree) } } // sort ra @@ -332,3 +378,34 @@ function bringAllDependencies(dependencies, dependencyName, cachedEnvironmentDep entry["dependencies"] = targetDeps }) } + +/** + * This function install tiny pipdeptree tool using pip ( if it's not already installed on python environment), and use it to fetch the dependency tree in json format. + * @param {string }pipPath - the filesystem path location of pip binary + * @param {string }pythonPath - the filesystem path location of python binary + * @return {Object[] } json array containing objects with the packages and their dependencies from pipdeptree utility + * @private + */ +function getDependencyTreeJsonFromPipDepTree(pipPath,pythonPath) { + let dependencyTree + try { + execSync(`${pipPath} install pipdeptree`) + } catch (e) { + throw new Error(`Couldn't install pipdeptree utility, reason: ${e.getMessage}`) + } + finally { + try { + if(pythonPath.startsWith("python")) { + dependencyTree = execSync(`pipdeptree --json`).toString() + } + else { + dependencyTree = execSync(`pipdeptree --json --python ${pythonPath} `).toString() + } + } catch (e) { + throw new Error(`couldn't produce dependency tree using pipdeptree tool, stop analysis, message -> ${e.getMessage}`) + } + + + } + return JSON.parse(dependencyTree) +} diff --git a/src/providers/python_pip.js b/src/providers/python_pip.js index b499a90..ca5d247 100644 --- a/src/providers/python_pip.js +++ b/src/providers/python_pip.js @@ -13,7 +13,7 @@ export default { isSupported, provideComponent, provideStack } const dummyVersionNotation = "dummy*=#?"; -/** @typedef {{name: string, , version: string, dependencies: DependencyEntry[]}} DependencyEntry */ +/** @typedef {{name: string, version: string, dependencies: DependencyEntry[]}} DependencyEntry */ /** * @type {string} ecosystem for python-pip is 'pip' From 60d98b31c3c464d49a7e278696da39c0c8c73c18 Mon Sep 17 00:00:00 2001 From: Zvi Grinberg Date: Wed, 13 Mar 2024 00:16:03 +0200 Subject: [PATCH 2/5] fix: lint problems Signed-off-by: Zvi Grinberg --- src/providers/python_controller.js | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/src/providers/python_controller.js b/src/providers/python_controller.js index 8472bc5..062732f 100644 --- a/src/providers/python_controller.js +++ b/src/providers/python_controller.js @@ -170,7 +170,7 @@ export default class Python_controller { let pipDepTreeJsonArrayOutput if(usePipDepTree !== "true") { freezeOutput = getPipFreezeOutput.call(this); - lines = freezeOutput.split(EOL) + lines = freezeOutput.split(EOL) depNames = lines.map( line => getDependencyName(line)).join(" ") } else { @@ -337,10 +337,10 @@ function bringAllDependencies(dependencies, dependencyName, cachedEnvironmentDep let depName let version; let directDeps - if(usePipDepTree !== "true") { + if(usePipDepTree !== "true") { depName = getDependencyNameShow(record) version = getDependencyVersion(record); - directDeps = getDepsList(record) + directDeps = getDepsList(record) } else { depName = record.name @@ -393,19 +393,17 @@ function getDependencyTreeJsonFromPipDepTree(pipPath,pythonPath) { } catch (e) { throw new Error(`Couldn't install pipdeptree utility, reason: ${e.getMessage}`) } - finally { - try { - if(pythonPath.startsWith("python")) { - dependencyTree = execSync(`pipdeptree --json`).toString() - } - else { - dependencyTree = execSync(`pipdeptree --json --python ${pythonPath} `).toString() - } - } catch (e) { - throw new Error(`couldn't produce dependency tree using pipdeptree tool, stop analysis, message -> ${e.getMessage}`) - } - + try { + if(pythonPath.startsWith("python")) { + dependencyTree = execSync(`pipdeptree --json`).toString() + } + else { + dependencyTree = execSync(`pipdeptree --json --python ${pythonPath} `).toString() + } + } catch (e) { + throw new Error(`couldn't produce dependency tree using pipdeptree tool, stop analysis, message -> ${e.getMessage}`) } + return JSON.parse(dependencyTree) } From 8d20370326ee0d2c99eebf0f6759c18965d9a87a Mon Sep 17 00:00:00 2001 From: Zvi Grinberg Date: Wed, 13 Mar 2024 11:14:50 +0200 Subject: [PATCH 3/5] fix: ex-request-id logging only when EXHORT_DEBUG=true Signed-off-by: Zvi Grinberg --- src/analysis.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/analysis.js b/src/analysis.js index c5eb42d..c7fb3e4 100644 --- a/src/analysis.js +++ b/src/analysis.js @@ -111,10 +111,11 @@ async function validateToken(url, opts = {}) { ...getTokenHeaders(opts), } }) - let exRequestId = resp.headers.get("ex-request-id"); - if(exRequestId) - { - console.log("Unique Identifier associated with this request - ex-request-id=" + exRequestId) + if (process.env["EXHORT_DEBUG"] === "true") { + let exRequestId = resp.headers.get("ex-request-id"); + if (exRequestId) { + console.log("Unique Identifier associated with this request - ex-request-id=" + exRequestId) + } } return resp.status } From 6e8a9ada544da383b4e6c6a1322c8cb90dd7db2c Mon Sep 17 00:00:00 2001 From: Zvi Grinberg Date: Wed, 13 Mar 2024 11:40:57 +0200 Subject: [PATCH 4/5] test: fix performance issue in python version >= 3.11.X Signed-off-by: Zvi Grinberg --- test/providers/python_pip.test.js | 93 ++++++++++++++++++++----------- 1 file changed, 59 insertions(+), 34 deletions(-) diff --git a/test/providers/python_pip.test.js b/test/providers/python_pip.test.js index 3faafad..ecba7f3 100644 --- a/test/providers/python_pip.test.js +++ b/test/providers/python_pip.test.js @@ -8,6 +8,48 @@ import {getCustomPath } from "../../src/tools.js" let clock + +async function sharedComponentAnalysisTestFlow(testCase,usePipDepTreeUtility) { + // load the expected list for the scenario + let expectedSbom = fs.readFileSync(`test/providers/tst_manifests/pip/${testCase}/expected_component_sbom.json`,).toString().trim() + expectedSbom = JSON.stringify(JSON.parse(expectedSbom)) + // read target manifest file + let manifestContent = fs.readFileSync(`test/providers/tst_manifests/pip/${testCase}/requirements.txt`).toString() + // invoke sut stack analysis for scenario manifest + let opts = { "EXHORT_PIP_USE_DEP_TREE" : usePipDepTreeUtility } + let providedDatForComponent = await pythonPip.provideComponent(manifestContent,opts) + // verify returned data matches expectation + expect(providedDatForComponent).to.deep.equal({ + ecosystem: 'pip', + contentType: 'application/vnd.cyclonedx+json', + content: expectedSbom + }) +} + +async function sharedStackAnalysisTestFlow(testCase,usePipDepTreeUtility) { + // load the expected graph for the scenario + let expectedSbom = fs.readFileSync(`test/providers/tst_manifests/pip/${testCase}/expected_stack_sbom.json`,).toString() + expectedSbom = JSON.stringify(JSON.parse(expectedSbom)) + // invoke sut stack analysis for scenario manifest + let pipPath = getCustomPath("pip3"); + execSync(`${pipPath} install -r test/providers/tst_manifests/pip/${testCase}/requirements.txt`, err => { + if (err) { + throw new Error('fail installing requirements.txt manifest in created virtual python environment --> ' + err.message) + } + }) + let opts = { "EXHORT_PIP_USE_DEP_TREE" : usePipDepTreeUtility } + let providedDataForStack = await pythonPip.provideStack(`test/providers/tst_manifests/pip/${testCase}/requirements.txt`,opts) + // new(year: number, month: number, date?: number, hours?: number, minutes?: number, seconds?: number, ms?: number): Date + + // providedDataForStack.content = providedDataForStack.content.replaceAll("\"timestamp\":\"[a-zA-Z0-9\\-\\:]+\"","") + // verify returned data matches expectation + expect(providedDataForStack).to.deep.equal({ + ecosystem: 'pip', + contentType: 'application/vnd.cyclonedx+json', + content: expectedSbom + }) +} + suite('testing the python-pip data provider', () => { [ {name: 'requirements.txt', expected: true}, @@ -24,45 +66,28 @@ suite('testing the python-pip data provider', () => { ].forEach(testCase => { let scenario = testCase.replace('pip_requirements_', '').replaceAll('_', ' ') test(`verify requirements.txt sbom provided for stack analysis with scenario ${scenario}`, async () => { - // load the expected graph for the scenario - let expectedSbom = fs.readFileSync(`test/providers/tst_manifests/pip/${testCase}/expected_stack_sbom.json`,).toString() - expectedSbom = JSON.stringify(JSON.parse(expectedSbom)) - // invoke sut stack analysis for scenario manifest - let pipPath = getCustomPath("pip3"); - execSync(`${pipPath} install -r test/providers/tst_manifests/pip/${testCase}/requirements.txt`, err => { - if (err) { - throw new Error('fail installing requirements.txt manifest in created virtual python environment --> ' + err.message) - } - }) - let providedDataForStack = await pythonPip.provideStack(`test/providers/tst_manifests/pip/${testCase}/requirements.txt`) - // new(year: number, month: number, date?: number, hours?: number, minutes?: number, seconds?: number, ms?: number): Date - - // providedDataForStack.content = providedDataForStack.content.replaceAll("\"timestamp\":\"[a-zA-Z0-9\\-\\:]+\"","") - // verify returned data matches expectation - expect(providedDataForStack).to.deep.equal({ - ecosystem: 'pip', - contentType: 'application/vnd.cyclonedx+json', - content: expectedSbom - }) - // these test cases takes ~2500-2700 ms each pr >10000 in CI (for the first test-case) + await sharedStackAnalysisTestFlow(testCase,false); + // these test cases takes ~2500-2700 ms each pr >10000 in CI (for the first test-case) }).timeout(process.env.GITHUB_ACTIONS ? 30000 : 10000) test(`verify requirements.txt sbom provided for component analysis with scenario ${scenario}`, async () => { - // load the expected list for the scenario - let expectedSbom = fs.readFileSync(`test/providers/tst_manifests/pip/${testCase}/expected_component_sbom.json`,).toString().trim() - expectedSbom = JSON.stringify(JSON.parse(expectedSbom)) - // read target manifest file - let manifestContent = fs.readFileSync(`test/providers/tst_manifests/pip/${testCase}/requirements.txt`).toString() - // invoke sut stack analysis for scenario manifest - let providedDatForComponent = await pythonPip.provideComponent(manifestContent) - // verify returned data matches expectation - expect(providedDatForComponent).to.deep.equal({ - ecosystem: 'pip', - contentType: 'application/vnd.cyclonedx+json', - content: expectedSbom - }) + await sharedComponentAnalysisTestFlow(testCase,false); // these test cases takes ~1400-2000 ms each pr >10000 in CI (for the first test-case) }).timeout(process.env.GITHUB_ACTIONS ? 15000 : 10000) + + test(`verify requirements.txt sbom provided for stack analysis using pipdeptree utility with scenario ${scenario}`, async () => { + await sharedStackAnalysisTestFlow(testCase,true); + // these test cases takes ~2500-2700 ms each pr >10000 in CI (for the first test-case) + }).timeout(process.env.GITHUB_ACTIONS ? 30000 : 10000) + + test(`verify requirements.txt sbom provided for component analysis using pipdeptree utility with scenario ${scenario}`, async () => { + await sharedComponentAnalysisTestFlow(testCase,true); + // these test cases takes ~1400-2000 ms each pr >10000 in CI (for the first test-case) + }).timeout(process.env.GITHUB_ACTIONS ? 15000 : 10000) + + + + }); }).beforeAll(() => clock = sinon.useFakeTimers(new Date('2023-10-01T00:00:00.000Z'))).afterAll(()=> clock.restore()); From 1fc349506f0fa5d75e661a3b55738dafc0a82b39 Mon Sep 17 00:00:00 2001 From: Zvi Grinberg Date: Wed, 13 Mar 2024 11:52:23 +0200 Subject: [PATCH 5/5] refactor: change wording of error message to be more generic Fixes: https://issues.redhat.com/browse/APPENG-2326 Signed-off-by: Zvi Grinberg --- src/providers/python_controller.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/providers/python_controller.js b/src/providers/python_controller.js index 062732f..ee525f9 100644 --- a/src/providers/python_controller.js +++ b/src/providers/python_controller.js @@ -330,8 +330,8 @@ function bringAllDependencies(dependencies, dependencyName, cachedEnvironmentDep let record = cachedEnvironmentDeps[dependencyName.toLowerCase()] if(record === null || record === undefined) { throw new Error(`Package name=>${dependencyName} is not installed on your python environment, - either install it ( better to install requirements.txt altogether) or turn on - environment variable EXHORT_PYTHON_VIRTUAL_ENV=true to automatically installs + either install it ( better to install requirements.txt altogether) or set + setting EXHORT_PYTHON_VIRTUAL_ENV=true to automatically installs it on virtual environment ( will slow down the analysis) `) } let depName