-
Notifications
You must be signed in to change notification settings - Fork 61
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
Add support for SPDX 2.3 #164
Conversation
Signed-off-by: Keith Zantow <[email protected]>
Signed-off-by: Keith Zantow <[email protected]>
Signed-off-by: Keith Zantow <[email protected]>
Signed-off-by: Keith Zantow <[email protected]>
a445943
to
11771ee
Compare
Signed-off-by: Keith Zantow <[email protected]>
Signed-off-by: Keith Zantow <[email protected]>
Signed-off-by: Keith Zantow <[email protected]>
Signed-off-by: Keith Zantow <[email protected]>
Signed-off-by: Keith Zantow <[email protected]>
Signed-off-by: Keith Zantow <[email protected]>
Signed-off-by: Keith Zantow <[email protected]>
Signed-off-by: Keith Zantow <[email protected]>
NOTE: I would squash all of these commits into a |
Signed-off-by: Keith Zantow <[email protected]>
Sorry i missed this (kubecon prep crunch). Let me review this this weekend and hope we can merge it in the next few weeks! |
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.
Thanks so much @kzantow for this effort! It was a lot of code wrangling!! Have a couple minor nits on adding ENUMs and should be good to merge, this will provide a good basis of using the struct, and then additional validation we can add on later!
@@ -0,0 +1,406 @@ | |||
{ | |||
"spdxVersion": "SPDX-2.3", |
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.
TODO: open a separate follow-up issue to have another test case which contains more 2.3 specific fields.
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.
NOTE: there are all the additional fields added to at least one package: https://github.com/spdx/tools-golang/pull/164/files/d326e1063b8265703fae4677e7fd63eb97e2df28#diff-f21e5fa3e6e7c061fc6553ed162414f5a0394e27bae30f647627d0e1a7199a11R151-R154
|
||
// 8.4: File Checksum: may have keys for SHA1, SHA256, MD5, SHA3-256, SHA3-384, SHA3-512, BLAKE2b-256, BLAKE2b-384, BLAKE2b-512, BLAKE3, ADLER32 | ||
// Cardinality: mandatory, one SHA1, others may be optionally provided | ||
Checksums []common.Checksum `json:"checksums"` |
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 think we should add the new algorithms to the common
folder as well as consts.
Added hash algorithms (SHA3-256, SHA3-384, SHA3-512, BLAKE2b-256, BLAKE2b-384, BLAKE2b-512, BLAKE3, ADLER32 ) to the set recognized by 7.10 (Package Checksum field) and 8.4 (File checksum field)
// Relationship is type from 11.1.1 | ||
RefA common.DocElementID `json:"spdxElementId"` | ||
RefB common.DocElementID `json:"relatedSpdxElement"` | ||
Relationship string `json:"relationshipType"` |
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.
Add 2 new relationship consts to spdx/common/external.go
Update Clause 11 to add the new relationship types: REQUIREMENT_DESCRIPTION_FOR and SPECIFICATION_FOR.
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.
In addition, since its touching the same files, adding the following consts:
Update Annex F ( External Repository Identifiers ) to expand security references to include advisory, fix, URL, SWID. Expand persistent identifiers to include gitoid.
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.
spdx/common/external.go
already has those constants: https://github.com/spdx/tools-golang/blob/main/spdx/common/external.go#L64-L65
The other fields are here: https://github.com/spdx/tools-golang/blob/main/spdx/common/external.go#L10-L13
spdxlib/described_elements_test.go
Outdated
// set up document and one package, but no relationships | ||
// b/c only one package | ||
doc := &v2_3.Document{ | ||
SPDXVersion: "SPDX-2.2", |
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.
SPDX-2.2
-> SPDX-2.3
Couple of occurrences
spdxlib/documents_test.go
Outdated
func Test2_3InvalidDocumentFailsValidation(t *testing.T) { | ||
// set up document and some packages and relationships | ||
doc := &v2_3.Document{ | ||
SPDXVersion: "SPDX-2.1", |
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.
is the old version intended for failure here?
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.
This was copied from the 2.2 version, but doesn't actually do much to test the failure condition; so I've updated it.
switch common.ChecksumAlgorithm(subkey) { | ||
case common.SHA1, common.SHA256, common.MD5: | ||
algorithm := common.ChecksumAlgorithm(subkey) | ||
parser.file.Checksums = append(parser.file.Checksums, common.Checksum{Algorithm: algorithm, Value: subvalue}) |
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.
add case for newly defined unhandled checksums and create an issue as a TODO to add support for new checksums
"github.com/spdx/tools-golang/spdx/v2_3" | ||
) | ||
|
||
func (parser *tvParser2_3) parsePairFromOtherLicense2_3(tag string, value string) error { |
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.
Signed-off-by: Keith Zantow <[email protected]>
Signed-off-by: Keith Zantow <[email protected]>
Signed-off-by: Keith Zantow <[email protected]>
Signed-off-by: Keith Zantow <[email protected]>
Thank you so much for the review @lumjjb! I think I've made all the changes requested -- please let me know if I've missed anything! |
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.
LGTM!
This PR adds support for SPDX 2.3 with the following changes:
Fixes: #156
NOTE: this PR also includes a change to fix the references to the SPDX spec in the
v2_2
model. This can be removed, but I've done this so it's possible to run a diff between the models and see a more meaningful set of changes, e.g. run:diff v2_2 v2_3 --ignore-matching-lines='package v2_.'
Results