Migrate to spdx-tools 0.8.0a2 (a pre-release) #114

jspeed-meyers · 2023-04-21T13:14:26Z

This branch introduces v0.8.0a2 of spdx-tools. This migration required substantial changes to the codebase (for the better, honestly) and the test documents and test cases.

For those following along at home:

There is new version of spdx-tools (thanks to the great efforts of @meretp and others) and it creates breaking changes. This migration addresses those breaking changes. Importantly, some of those breaking changes also led to new test documents (the new spdx-tools parsing rules are stricter) and so that's why there are so many changes lines of code. For reviewers, the most important changes are in sbom_checker.py, IMO.

This is a larger change than I would prefer to make in a single PR. But I don't see any way around it short of having a broken build. This way we avoid a broken build and complete the migration.

@goneall, @meretp, @linynjosh -- Feel free to take a look over the next week. Assuming positive reviews and no objections, we can merge this into main in a week. :)

Again, big kudos to @meretp, without whom this migration would not have happened or would have happened at a glacially slow rate. THANK YOU!

spdx-tools implemented a significant refactor. This commit bumps the version and is expected to introduce breaking changes to ntia-conformance-checker. Signed-off-by: John Speed Meyers <jsmeyers@chainguard.dev>

Because spdx-tools has been refactored and is available now only in a pre-release versions, this change enables the build test pipeline to use prerelease versions of packages, which is not allowed by pip/pipenv with default settings. Signed-off-by: John Speed Meyers <jsmeyers@chainguard.dev>

Signed-off-by: John Speed Meyers <jsmeyers@chainguard.dev>

yong-aan

Thanks @jspeed-meyers!

jspeed-meyers · 2023-04-22T00:20:11Z

Thanks, @linynjosh! I'll work to figure out the migration before merging :)

Signed-off-by: John Speed Meyers <jsmeyers@chainguard.dev>

yong-aan

Thanks @jspeed-meyers!

The spdx-tools refactoring made the spdx spec validation stricter and this document no longer matches the spdx spec. Adding this created timestamp makes the document compliant. Signed-off-by: John Speed Meyers <jsmeyers@chainguard.dev>

Since the new spdx-tools version has stricter requirements that the SBOM comply with spdx spec, the SBOM used for this test needed to be ammended and so now too does the expected test output need to be ammended since there is a timestamp in the underlying SBOM. Signed-off-by: John Speed Meyers <jsmeyers@chainguard.dev>

Signed-off-by: John Speed Meyers <jsmeyers@chainguard.dev>

Because the underlying test SBOM now does have a timestamp, because the refactored version of spdx-tools requires ingested SBOMs to conform strictly to the spdx spec, this test now ought to expect that an SBOM does have a timestamp. Signed-off-by: John Speed Meyers <jsmeyers@chainguard.dev>

Signed-off-by: John Speed Meyers <jsmeyers@chainguard.dev>

The migration to the 0.8.0 spdx-tools library version means dealing with stricter adherence to the spdx spec. This commit drops a version field that is empty since it was throwing an error with the new library. Signed-off-by: John Speed Meyers <jsmeyers@chainguard.dev>

Signed-off-by: John Speed Meyers <jsmeyers@chainguard.dev>

Drop RDF SBOM since there seem to be so many specification-related issues. Signed-off-by: John Speed Meyers <jsmeyers@chainguard.dev>

Signed-off-by: John Speed Meyers <jsmeyers@chainguard.dev>

meretp

Thanks for opening this PR, the changes made look good to me and fit what I would have expected.
I have two remarks on the current changes which are associated to some basic changes coming with the refacotred version. So to provide a bit more background on the refactored version of tools-python:
On the one hand we introduced a new separate layer for the validation of a parsed SPDX document. This validation takes care of the more "implicit rules for SPDX documents". So for example each SPDXID used in the document need to be unique. It might make sense to also use this validation here and expect a valid SPDX document before checking for the minimal ntia elements.
On the other hand we introduced a more strict underlying model, which enforces correct types and cardinalities for each property. So for example an SPDX document must have at least one creator that is a person, a tool or an organization. These types of "validation" are handled by the parser and the tools will raise a SPDXParsingError if the parsed document doesn't fit the model.

I think this is also the reason for most of the failing tests. I had a look at some of the test files and noticed that they aren't valid according to the spec. For example the SPDXJsonExample.json contains a surrounding Document object which is not valid according to the spec. I think a first step to fix the tests is to make all used documents valid SPDX documents. Apart from that I will take a closer look at the tests and try to provide more feedback on why they fail and how to fix them.

ntia_conformance_checker/sbom_checker.py

jspeed-meyers · 2023-04-24T12:40:34Z

It might make sense to also use this validation here and expect a valid SPDX document before checking for the minimal ntia elements.

Perfect. I like this.

I think a first step to fix the tests is to make all used documents valid SPDX documents.

Good call. Will do.

Apart from that I will take a closer look at the tests and try to provide more feedback on why they fail and how to fix them.

Thank you. You've really gone above and beyond in helping this project and others do the migration. THANK YOU!

meretp · 2023-04-24T13:20:28Z

Your welcome! This also helps us a lot to verify that the refactored version is stable. I already found a small bug thanks to this PR, so a big thank you too for being willing to serve as an early adopter! 🙂

Because the new spdx-tools version won't accept does the work of the old check, that old code can be replaced by a much simpler version. Signed-off-by: John Speed Meyers <jsmeyers@chainguard.dev>

If the document is parsed successfully, the new spdx-tools logic guarantees that there is an author and there is a document timestamp. So this commit sets those values to always true. Signed-off-by: John Speed Meyers <jsmeyers@chainguard.dev>

Signed-off-by: John Speed Meyers <jsmeyers@chainguard.dev>

tests/data/missing_dependency_relationships/SPDXJsonExample.json

meretp · 2023-04-25T13:07:34Z

FYI: We have a weekly call to discuss topics related to the spdx-tools. It's open to everyone, and meant as an informal way to sync on current topics and where development should focus next. Maybe you are also interested in joining. It's every Thursday, 3.30pm - 4.00pm GMT (except the first Thursday of the month, when it is shifted by half an hour to 4.00pm - 4.30pm) and takes place per zoom: https://zoom.us/j/98741582779.

meretp · 2023-05-12T12:13:24Z

@jspeed-meyers I did some work on this and pushed the changes to the branch on my fork as I don't have the permission to push to this branch directly. I can also open a PR against this branch, might be easier to see the changes...

4 tests are still failing due to bugs in the spdx-tools. We work on fixes for these bugs and plan to do another alpha release including the fixes, so this PR should probably wait until the release and then include the second alpha release as dependency.

I wasn't sure about the generated output, especially the formatting. As the parser in the refactored version of the spdx-tools is stricter I decided to except the possibly thrown SPDXParsingError and generate an indiviudal output as all other checks can't be performed. I also added the output of validation messages if the document is parseable but not compliant with the spdx specification. Please tell me what you think about these changes.

jspeed-meyers · 2023-05-12T13:10:44Z

@meretp, first, thank you! This is tremendous.

Please do open a PR against this branch. After that, I'll do a review and point out any thoughts I have. We can then wait for the next alpha release of spdx-tools.

Again, thank you. I hope this migration work, from myself and now you, has benefitted spdx-tools!

…cation Signed-off-by: Meret Behrens <meret.behrens@tngtech.com>

Signed-off-by: Meret Behrens <meret.behrens@tngtech.com>

Migrate2spdx tools 0.8.0a1

jspeed-meyers · 2023-05-15T12:17:31Z

@meretp, amazing. Thank you! What great work. I'll review this new branch now and ask any questions. I have no doubt this code is excellent.

jspeed-meyers · 2023-05-15T12:25:10Z

No questions! This looks really great. I'm also of the opinion that the best way way to find bugs is through users, so even though this is a lot of change, I'm not convinced it's worth it going over it too closely in code review :) Again, great work.

You mentioned that a new future pre-release will fix the remaining bugs. I'll watch out for that. AND THANK YOU!

meretp · 2023-05-17T08:38:35Z

With pleasure, thank you for the feedback and the great cooperation! This helps us a lot in the development of spdx-tools!

The new release is now on Pypi. This should fix the tests. It seems that I've missed to format one file to also fix the black-formatting action. So if you could change the dependency to 0.8.0a2 and fix the formatting the CI should be green and this PR can be merged.

github-actions · 2023-05-17T13:52:34Z

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines	Covered	Coverage	Threshold	Status
271	224	83%	0%	🟢

New Files

No new covered files...

Modified Files

File	Coverage	Status
ntia_conformance_checker/sbom_checker.py	68%	🟢
tests/test_checker.py	100%	🟢
TOTAL	84%	🟢

updated for commit: d0da352 by action🐍

Signed-off-by: John Speed Meyers <jsmeyers@chainguard.dev>

Update pipfile with 0.8.0a2 spdx-tools bump

Signed-off-by: John Speed Meyers <jsmeyers@chainguard.dev>

Perform black formatting sbom_checker.py

goneall

LGTM

jspeed-meyers · 2023-05-20T17:41:14Z

Thank you, @goneall!

meretp

Sorry for the late response, looks good to me! Only one small remark from my side to make the dependency consistent in the pyoproject.toml.

pyproject.toml

Co-authored-by: Meret B. <50019307+meretp@users.noreply.github.com>

jspeed-meyers · 2023-05-22T12:39:08Z

Thank you, @meretp!

jspeed-meyers · 2023-05-24T14:09:14Z

Woohoo, merging! 🥳

jspeed-meyers added 2 commits April 21, 2023 08:56

Update spdx-tools version to alpha release 0.8.0a1

260760d

spdx-tools implemented a significant refactor. This commit bumps the version and is expected to introduce breaking changes to ntia-conformance-checker. Signed-off-by: John Speed Meyers <jsmeyers@chainguard.dev>

jspeed-meyers added the enhancement New feature or request label Apr 21, 2023

jspeed-meyers self-assigned this Apr 21, 2023

jspeed-meyers mentioned this pull request Apr 21, 2023

Use refactored version of tools-python #113

Closed

Fix creation info and SpdxNoAssertion for migration spdx-tools 0.8.0a1

32a7ab1

Signed-off-by: John Speed Meyers <jsmeyers@chainguard.dev>

yong-aan approved these changes Apr 22, 2023

View reviewed changes

Change import path to parse_anything function

85a83c0

Signed-off-by: John Speed Meyers <jsmeyers@chainguard.dev>

yong-aan approved these changes Apr 23, 2023

View reviewed changes

jspeed-meyers added 12 commits April 23, 2023 13:39

Add missing date to a YAML SBOM

9e6270d

The spdx-tools refactoring made the spdx spec validation stricter and this document no longer matches the spdx spec. Adding this created timestamp makes the document compliant. Signed-off-by: John Speed Meyers <jsmeyers@chainguard.dev>

Implement various migration changes

302ab9b

Signed-off-by: John Speed Meyers <jsmeyers@chainguard.dev>

Fix spdxnoassertion logic related to package supplier for migration

6f943b0

Signed-off-by: John Speed Meyers <jsmeyers@chainguard.dev>

Merge branch 'main' into migrate2spdx-tools-0.8.0a1

3412c74

Enable pre-release for testing this branch

0af0262

Signed-off-by: John Speed Meyers <jsmeyers@chainguard.dev>

Implement pytest exception pattern for missing unique identifiers

dc34429

Signed-off-by: John Speed Meyers <jsmeyers@chainguard.dev>

Fix issues with missing supplier test SBOMs

4d273e4

Signed-off-by: John Speed Meyers <jsmeyers@chainguard.dev>

Fix errors related to missing component version test SBOMs

622ef28

Drop RDF SBOM since there seem to be so many specification-related issues. Signed-off-by: John Speed Meyers <jsmeyers@chainguard.dev>

Fix pylint and black erros with sbom_checker.py

19b498b

Signed-off-by: John Speed Meyers <jsmeyers@chainguard.dev>

meretp reviewed Apr 24, 2023

View reviewed changes

ntia_conformance_checker/sbom_checker.py Outdated Show resolved Hide resolved

ntia_conformance_checker/sbom_checker.py Outdated Show resolved Hide resolved

jspeed-meyers added 3 commits April 24, 2023 20:41

Simplify checking doc author by simply checking for non-zero number

107c8b7

Because the new spdx-tools version won't accept does the work of the old check, that old code can be replaced by a much simpler version. Signed-off-by: John Speed Meyers <jsmeyers@chainguard.dev>

Fix test documents related to missing relationships

6848cce

Signed-off-by: John Speed Meyers <jsmeyers@chainguard.dev>

meretp reviewed Apr 25, 2023

View reviewed changes

tests/data/missing_dependency_relationships/SPDXJsonExample.json Outdated Show resolved Hide resolved

meretp and others added 5 commits May 15, 2023 08:13

add output if the document is not valid according to the spdx specifi…

7512034

…cation Signed-off-by: Meret Behrens <meret.behrens@tngtech.com>

make example file for generated html output valid

c1e44b1

Signed-off-by: Meret Behrens <meret.behrens@tngtech.com>

use string instead of docstring to shorten code for html output

832f75e

Signed-off-by: Meret Behrens <meret.behrens@tngtech.com>

fix linting

bebdf48

Signed-off-by: Meret Behrens <meret.behrens@tngtech.com>

Merge pull request #122 from meretp/migrate2spdx-tools-0.8.0a1

b8b5712

Migrate2spdx tools 0.8.0a1

jspeed-meyers added the blocked-on-upstream-work Fix depends on an upstream project label May 15, 2023

Update spdx-tools to 0.8.0a2

2da007c

jspeed-meyers added 2 commits May 17, 2023 09:52

Update pipfile with 0.8.0a2 spdx-tools bump

6524e89

Signed-off-by: John Speed Meyers <jsmeyers@chainguard.dev>

Merge pull request #123 from jspeed-meyers/migrate2spdx-tools-0.8.0a1

34b393e

Update pipfile with 0.8.0a2 spdx-tools bump

jspeed-meyers removed the blocked-on-upstream-work Fix depends on an upstream project label May 17, 2023

jspeed-meyers added 2 commits May 17, 2023 10:04

Perform black formatting sbom_checker.py

51746ca

Signed-off-by: John Speed Meyers <jsmeyers@chainguard.dev>

Merge pull request #124 from jspeed-meyers/migrate2spdx-tools-0.8.0a1

f5f5ba0

Perform black formatting sbom_checker.py

jspeed-meyers requested review from goneall, meretp and yong-aan May 17, 2023 14:05

jspeed-meyers changed the title ~~Migrate to spdx-tools 0.8.0a1 (a pre-release)~~ Migrate to spdx-tools 0.8.0a2 (a pre-release) May 17, 2023

goneall approved these changes May 20, 2023

View reviewed changes

meretp approved these changes May 22, 2023

View reviewed changes

pyproject.toml Outdated Show resolved Hide resolved

Bump pyproject.toml spdx-tools version

d0da352

Co-authored-by: Meret B. <50019307+meretp@users.noreply.github.com>

jspeed-meyers merged commit cc85a18 into main May 24, 2023
6 checks passed

jspeed-meyers deleted the migrate2spdx-tools-0.8.0a1 branch September 4, 2023 13:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate to spdx-tools 0.8.0a2 (a pre-release) #114

Migrate to spdx-tools 0.8.0a2 (a pre-release) #114

jspeed-meyers commented Apr 21, 2023 •

edited

yong-aan left a comment

jspeed-meyers commented Apr 22, 2023

yong-aan left a comment

meretp left a comment

jspeed-meyers commented Apr 24, 2023

meretp commented Apr 24, 2023

meretp commented Apr 25, 2023

meretp commented May 12, 2023

jspeed-meyers commented May 12, 2023

jspeed-meyers commented May 15, 2023

jspeed-meyers commented May 15, 2023 •

edited

meretp commented May 17, 2023

github-actions bot commented May 17, 2023 •

edited

goneall left a comment

jspeed-meyers commented May 20, 2023

meretp left a comment

jspeed-meyers commented May 22, 2023

jspeed-meyers commented May 24, 2023

Migrate to spdx-tools 0.8.0a2 (a pre-release) #114

Migrate to spdx-tools 0.8.0a2 (a pre-release) #114

Conversation

jspeed-meyers commented Apr 21, 2023 • edited

yong-aan left a comment

Choose a reason for hiding this comment

jspeed-meyers commented Apr 22, 2023

yong-aan left a comment

Choose a reason for hiding this comment

meretp left a comment

Choose a reason for hiding this comment

jspeed-meyers commented Apr 24, 2023

meretp commented Apr 24, 2023

meretp commented Apr 25, 2023

meretp commented May 12, 2023

jspeed-meyers commented May 12, 2023

jspeed-meyers commented May 15, 2023

jspeed-meyers commented May 15, 2023 • edited

meretp commented May 17, 2023

github-actions bot commented May 17, 2023 • edited

☂️ Python Coverage

Overall Coverage

New Files

Modified Files

goneall left a comment

Choose a reason for hiding this comment

jspeed-meyers commented May 20, 2023

meretp left a comment

Choose a reason for hiding this comment

jspeed-meyers commented May 22, 2023

jspeed-meyers commented May 24, 2023

jspeed-meyers commented Apr 21, 2023 •

edited

jspeed-meyers commented May 15, 2023 •

edited

github-actions bot commented May 17, 2023 •

edited