feat: add SBOM-based dependency diff workflow #14

ThorbenLindhauer · 2023-09-28T14:25:12Z

related to camunda/camunda-bpm-platform#2781

- contains some changes of the following commits in accumulating files (e.g. dist/index.js, package.json) related to camunda/camunda-bpm-platform#2781

related to camunda/camunda-bpm-platform#2781

ThorbenLindhauer · 2023-09-29T09:22:07Z

I have restructured the PR into five meaningful increments. Note that the first commit contains some content in shared files (e.g. dist/index.js, package.json) that is only used by the subsequent commits. Also note that the individual commits may not represent and individually executable and working state. The primary purpose of this structure is to make code review easier.

danielkelemen

Looks really nice! I added some minor comments, mostly about obvious code smells and a few questions.

💬 Since it contains a lot of code it might be good to see some details in the job log in case we need to look into something. GitHub has the log grouping feature which makes reading big logs easier. Maybe we could use it here?

Some ideas (link):

🔧 Group the git and maven executions.
🔧 We could also print the head and base sbom in groups. Maybe print some intermediate infos during analysis.

What do you think?

danielkelemen · 2023-10-06T07:28:59Z

common/diff-sboms-standalone.js

+const partialPaths = [
+    'componentDetails:../java-dependency-check/component-details.hbs',
+    'componentDiff:../java-dependency-check/component-diff.hbs',
+    'componentTree:../java-dependency-check/component-tree.hbs',
+    'componentVersion:../java-dependency-check/component-version.hbs'
+];
+
+const partials = partialPaths.reduce(
+    (result, input) => {
+        [ partialId, partialPath ] = input.split(':');
+        result[partialId.trim()] = readFile(partialPath.trim());
+        return result;
+    }, 
+    {}
+);


💬 Interesting way of doing this :) But it does the job.
I guess it's because it uses the (almost) same format as the workflow parameter.

Let me know if there's a more elegant way to convert these parameters into a map. I have rather basic Javascript standard library knowledge (and google skills).

It's okay!
I later realized that it uses the same format as the workflow input parameter (which is just a string not a JSON object or map), so it makes sense. 👍

danielkelemen · 2023-10-06T07:34:14Z

common/dist/xhr-sync-worker.js

❓Why do we need this?

This is generated by ncc build index.js -o dist that bundles and minifies code along with the npm dependencies. I cannot tell exactly why it is like that, it appeared with adding one of the dependencies. My guess would be that this is a transitive dependency of one of the dependencies and it has some configuration that prevents bundling/minification.

Ah okay, thanks.
I really tried to understand why it's needed but couldn't figure it out. 😀

danielkelemen · 2023-10-06T07:44:31Z

common/diff-sboms-standalone.js

+
+var args = process.argv.slice(2); // first two arguments are the executable and the JS file
+
+if (args.length != 3) {


🔧

Suggested change

if (args.length != 3) {

if (args.length !== 3) {

danielkelemen · 2023-10-06T07:45:39Z

common/src/diff-sboms.js

+    core.info(`Dependency diff:`);
+    core.info(diff.fullDiff);
+
+    octokit.rest.issues.createComment({


❓Should we maybe wait for this?

Suggested change

octokit.rest.issues.createComment({

await octokit.rest.issues.createComment({

danielkelemen · 2023-10-06T07:49:21Z

common/src/sbom-diff/differ.js

+      dependency the traversal belongs
+  */
+
+  var componentDiff = sbomDiff.getComponentDiff(baseComponent, comparingComponent);


🔧

Suggested change

var componentDiff = sbomDiff.getComponentDiff(baseComponent, comparingComponent);

let componentDiff = sbomDiff.getComponentDiff(baseComponent, comparingComponent);

danielkelemen · 2023-10-06T08:41:30Z

common/src/test/sbom-matchers.js

+    var baseComponent = actual.baseComponent;
+    var comparingComponent = actual.comparingComponent;


🔧

Suggested change

var baseComponent = actual.baseComponent;

var comparingComponent = actual.comparingComponent;

const baseComponent = actual.baseComponent;

const comparingComponent = actual.comparingComponent;

danielkelemen · 2023-10-06T08:41:57Z

common/src/test/sbom-matchers.js

+
+    const actualChanges = Object.assign({}, actual.changedDependencies);
+    const unmatchedChanges = [];
+    var allChangesMatch = true;


🔧

Suggested change

var allChangesMatch = true;

let allChangesMatch = true;

danielkelemen · 2023-10-06T08:43:07Z

common/src/test/sbom-parse.test.js

+
+    expect(components.size).toEqual(19);
+
+    var numThirdPartyComponents = 0;


Suggested change

var numThirdPartyComponents = 0;

let numThirdPartyComponents = 0;

danielkelemen · 2023-10-06T08:45:19Z

common/src/test/sbom-diff.test.js

+
+expect.extend(sbomMatchers);
+
+describe("SBOM diff", () => {


🔧 These tests could also have the given-when-then comments.

danielkelemen · 2023-10-06T08:49:50Z

java-dependency-check/generate-sboms.sh

+    mkdir -p target/diff
+
+    mvn -s "$1" --no-transfer-progress org.cyclonedx:cyclonedx-maven-plugin:2.7.9:makeAggregateBom
+
+    echo "SBOM generated for github ref HEAD"
+
+    cp target/bom.json target/diff/head.json


💬 The job log output is pretty big, maybe we could take advantage of GitHub log grouping? (I haven't tried this)

Suggested change

mkdir -p target/diff

mvn -s "$1" --no-transfer-progress org.cyclonedx:cyclonedx-maven-plugin:2.7.9:makeAggregateBom

echo "SBOM generated for github ref HEAD"

cp target/bom.json target/diff/head.json

echo "::group::SBOM head"

mkdir -p target/diff

mvn -s "$1" --no-transfer-progress org.cyclonedx:cyclonedx-maven-plugin:2.7.9:makeAggregateBom

echo "SBOM generated for github ref HEAD"

cp target/bom.json target/diff/head.json

echo "::endgroup::"

*I mixed up the jobs, the output is actually not that big as I thought.

danielkelemen · 2023-10-06T09:50:21Z

java-dependency-check/diff.hbs

+
+{{#each diff.addedDependencies}}
+{{#if thirdParty}}
+- [ ] {{name}}:{{>componentVersion}}


🔧

Suggested change

- [ ] {{name}}:{{>componentVersion}}

- [ ] {{name}}: {{>componentVersion}}

ThorbenLindhauer · 2023-10-06T12:18:37Z

Re log grouping:

I can have a look and add it for the git and Maven output. I wouldn't log the SBOM files though. They are ~10,000 lines, which makes searching and browsing them very hard in the log file. github also adds the timestamp to each line so it's rather impossible to copy them into an editor from there. Instead I decided to add them as workflow artifacts (check the zip file in the build summary). I think that is easier to work with.

ThorbenLindhauer · 2023-10-06T15:36:31Z

@danielkelemen I have applied all the review hints that I didn't comment on and I have added log grouping to the step that generates SBOMs (see https://github.com/camunda/camunda-bpm-platform/actions/runs/6433372444/job/17470293552?pr=3581). Please re-review and let me know what you think about the items I commented on.

danielkelemen

Looks great! 👍

The zip download is better than the logging the SBOMs, I agree.

related to camunda/camunda-bpm-platform#2781

ThorbenLindhauer self-assigned this Sep 28, 2023

ThorbenLindhauer mentioned this pull request Sep 28, 2023

Make dependency updates more efficient camunda/camunda-bpm-platform#2781

Closed

ThorbenLindhauer added 2 commits September 29, 2023 11:07

feat(workflows): add SBOM parsing and diffing logic

a6e35d3

- contains some changes of the following commits in accumulating files (e.g. dist/index.js, package.json) related to camunda/camunda-bpm-platform#2781

feat(workflows): add rendering of SBOM diffs to Markdown/HTML

3a28741

related to camunda/camunda-bpm-platform#2781

ThorbenLindhauer force-pushed the 2781-dependency-updates branch from 8a52948 to 583276c Compare September 29, 2023 09:09

ThorbenLindhauer marked this pull request as ready for review September 29, 2023 09:23

ThorbenLindhauer requested a review from danielkelemen October 2, 2023 12:13

danielkelemen reviewed Oct 6, 2023

View reviewed changes

ThorbenLindhauer requested a review from danielkelemen October 6, 2023 15:36

danielkelemen approved these changes Oct 9, 2023

View reviewed changes

ThorbenLindhauer added 3 commits October 9, 2023 09:44

feat(workflows): add reusable github workflow for SBOM diffing

dd97e0a

related to camunda/camunda-bpm-platform#2781

chore(project): add github workflow as CI of Javascript sources

30c077d

related to camunda/camunda-bpm-platform#2781

chore(workflows): add standalone SBOM diffing script for local testing

3dfe8c2

related to camunda/camunda-bpm-platform#2781

ThorbenLindhauer force-pushed the 2781-dependency-updates branch from e8162e9 to 3dfe8c2 Compare October 9, 2023 07:44

ThorbenLindhauer merged commit 08cfc4f into main Oct 9, 2023
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add SBOM-based dependency diff workflow #14

feat: add SBOM-based dependency diff workflow #14

ThorbenLindhauer commented Sep 28, 2023

ThorbenLindhauer commented Sep 29, 2023

danielkelemen left a comment •

edited

Loading

danielkelemen Oct 6, 2023

ThorbenLindhauer Oct 6, 2023

danielkelemen Oct 9, 2023

danielkelemen Oct 6, 2023

ThorbenLindhauer Oct 6, 2023

danielkelemen Oct 6, 2023

danielkelemen Oct 6, 2023

danielkelemen Oct 6, 2023

danielkelemen Oct 6, 2023

danielkelemen Oct 6, 2023

danielkelemen Oct 6, 2023

danielkelemen Oct 6, 2023

danielkelemen Oct 6, 2023

danielkelemen Oct 6, 2023

danielkelemen Oct 6, 2023

danielkelemen Oct 6, 2023

ThorbenLindhauer commented Oct 6, 2023

ThorbenLindhauer commented Oct 6, 2023

danielkelemen left a comment


		var args = process.argv.slice(2); // first two arguments are the executable and the JS file

		if (args.length != 3) {

	octokit.rest.issues.createComment({
	await octokit.rest.issues.createComment({

	var componentDiff = sbomDiff.getComponentDiff(baseComponent, comparingComponent);
	let componentDiff = sbomDiff.getComponentDiff(baseComponent, comparingComponent);

		var baseComponent = actual.baseComponent;
		var comparingComponent = actual.comparingComponent;


		expect(components.size).toEqual(19);

		var numThirdPartyComponents = 0;

	var numThirdPartyComponents = 0;
	let numThirdPartyComponents = 0;

	- [ ] {{name}}:{{>componentVersion}}
	- [ ] {{name}}: {{>componentVersion}}


		expect.extend(sbomMatchers);

		describe("SBOM diff", () => {

feat: add SBOM-based dependency diff workflow #14

feat: add SBOM-based dependency diff workflow #14

Conversation

ThorbenLindhauer commented Sep 28, 2023

ThorbenLindhauer commented Sep 29, 2023

danielkelemen left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ThorbenLindhauer commented Oct 6, 2023

ThorbenLindhauer commented Oct 6, 2023

danielkelemen left a comment

Choose a reason for hiding this comment

danielkelemen left a comment •

edited

Loading