-
Notifications
You must be signed in to change notification settings - Fork 141
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 Package Purpose field #622
Conversation
@goneall, @iamwillbar This is a first-pass PR. A review is appreciated :) |
cc @puerco |
Thanks for this @nishakm! I haven't been closely involved with the discussions around adding a Package Purpose field, but here are a few random thoughts anyway :)
Hope this helps -- and apologies if any of this is contrary to things the tech team has discussed separately! |
Should I be turning this into a Draft PR? |
Suggest adding one more enumeration:
This would allow mapping to the CycloneDX file type and also solve some of the issues @rjb4standards has raised. |
I'm leaning toward 0..1 and changing the semantics slightly to be the "primary" purpose of the package. This would making the mappings to CycloneDX easier. That being said, there is value to having the multiple purposes represented in that it may help identify expected properties to be present. However, this scenario may be better addressed through Profiles rather than having multiple purposes. |
Thanks for filing this @nishakm! I agree with @swinslow and @goneall , I would not make it mandatory as it would break compatibility with older docs. I'm wondering if purpose really captures the notion we want. Talking about purpose sets the origin of the subject matter within the SBOM. In other words, it sounds as if the SBOM were dictating what the software should be used for while we are really trying to indicate what kind of bundle the package is describing. The said bundle has its inherent properties which the SBOM can only describe but not set. I'm sorry, I know it is ontological pedantry, I just wanted to note the misalignment. I think I would add an enumerated |
I believe the 2.3 effort is considering the addition of a Classification element; I'm wondering if PackageType would be more appropriate and in harmony with 3.0, if @puerco suggestion passes muster. The 2.3 effort is intended to address DocFest issues involving NTIA minimum elements and is considering additional elements under the Package object: |
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 for getting this started @nishakm! Nice work.
Agree with Gary - this should be optional, so Cardinality 0..1
Of the Purposes (or Classifications) we've got listed, some are beyond software, so we should discuss further.
We should probably discuss if the Examples should include JSON as well at this point? Topic for the next tech call I guess.
@rjb4standards - FileTypes is referencing MIME types today, so as this has a different semantics, we should use a different term to describe it. Classification, Purpose, PrimaryPurpose, ??? yeah let's see if we can come to agreement either in this thread or at one of the meetings. |
I defer to the group consensus on the name to use. I'll be happy if I can report a version on a file that is part of an installation package. This fills the NTIA minimum check in SAG-PM that is kicking out SPDX SBOM for non-conformance with NTIA minimum standards. That solves a problem for me. Also, we are currently coding in support for CycloneDX VEX for vulnerability reporting and look forward to doing the same for whatever the SPDX defects team produces to answer the question: Thanks to all for their technical contributions in helping to solve these problems. |
@goneall, wouldn't a |
+1 for FILE as a catchall for objects that have a version identifier that don't fit comfortably into another calss. |
@seabass-labrax I agree a FILE will likely have a specific purpose, however, it may not be easy for the document creator to know the purpose of the file and it may not always be important to the use case. I would suggest having a generic FILE purpose and if there are specific use cases where there is value in knowing the purpose having those as additional purposes. |
Quick question, is PURPOSE the current element name to represent CLASSIFICATION info, i.e. File, Library, installPackage, etc. in V 2.3 or are these two different concepts? |
I am thinking we only have one new property to represent CLASSIFICATION info. |
Thanks, Gary. I can live with a minimum of 2 classifications: |
To summarize everyone's feedback thus far:
Do I understand correctly that we are unclear on whether this field should be "PackagePurpose" or "PackageType"? |
@iamwillbar - Did we already discuss and agree on the name for this field? If so, we should go with the previously chosen field name. |
@goneall For SPDX 3.0 there was agreement on PackagePurpose. Type was deemed to be ambiguous. |
On Cardinality: |
From Kate: |
@nishakm, I would also agree with expanding the cardinality to I also agree with Kate's point that |
@kestewart @goneall Will a 2.3 branch be created soon? :) |
@kestewart @tsteenbe were you going to set this up? |
@nishakm - please update fixes, and submit to 2.3 branch. |
613227d
to
91a9133
Compare
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 have a few specific comments / suggested edits for particular parts of this, which I'm including inline.
I'd also asked earlier in the conversation thread about whether some of the definitions can be clarified, in particular FRAMEWORK vs. LIBRARY. But I know I haven't been on tech team calls for a long time so you may have discussed or resolved any of these comments already. Just sharing in case they're helpful. Thanks!
| Attribute | Value | | ||
| --------- | ----- | | ||
| Required | Yes | | ||
| Cardinality | 0..* | |
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.
If Cardinality will be 0..*
, then I think Required should be "No" rather than "Yes"?
`APPLICATION` if the package is a software application; | ||
`FRAMEWORK` if the package is a software framework; | ||
`LIBRARY` if the package is a software library; | ||
`CONTAINER` if the package refers to a container image which can be used by a container runtime application; |
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.
Here and below, maybe consider whether "refers to" is the intended language to use here?
In the first few entries, it says "if the package is a...". Is it meant to mean something different where it says "refers to" instead of "is"?
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 one is hard - containers are usually declared in the form of "name:tag" and this refers to a remote tag which points to a merkle tree. Eventually, the leaf artifacts are downloaded by the client but container users have no visibility into this. So yes, I intentionally used "refers to", but I am willing to change it to something more appropriate.
`LIBRARY` if the package is a software library; | ||
`CONTAINER` if the package refers to a container image which can be used by a container runtime application; | ||
`OPERATING-SYSTEM` if the package refers to an operating system; | ||
`DEVICE` if the package refers to a chipset, processor, or electronic board; |
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'm not sure I understand the meaning of DEVICE
as defined here. Is the intention that this would be used for:
- files which are the actual embodiment of a chipset, etc. (such as PCB images, or VHDL files, etc.); or
- files which are specific to a particular device (such as architecture-specific files in the Linux kernel sources)
I'm guessing the intention is 1 but not 2, but the current definition seems to potentially apply to either one... maybe consider clarifying whether that's the intended meaning.
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.
IIRC "DEVICE" is one of the CycloneDX enumerations.
`FIRMWARE` if the package provides low level control over a device's hardware; | ||
`SOURCE` if the package is a collection of source files; | ||
`ARCHIVE` if the package refers to an archived collection of files (.tar, .zip, etc); | ||
`FILE` if the package is a single file which can be independently distributed (configuration file, statically linked binary, Kubernetes deployment, etc); |
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.
"Kubernetes deployment" seems a bit odd to call out here. Is Kubernetes always or typically deployed as a single file? Apologies if it is, I genuinely don't know :)
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.
It's a YAML file that directs kubernetes to pull and deploy 1 or more containers. I would say it's similar to a config file.
chapters/package-information.md
Outdated
|
||
The metadata for the Package Purpose field is shown in Table 36. | ||
|
||
**Table 36 — Metadata for the package name field** |
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.
The caption here says "the package name field", should that be "the package purpose field"?
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.
@nishakm does this proposal require the use of "CAPITAL" letters for the enumerated set of 'PackageProposal" content model members? Thanks.
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.
Typically, enumerations are written with CAPITAL letters, at least in code. To be honest, I was just following the pattern in the model document :).
https://github.com/spdx/meetings/blob/main/tech/Model.png
91a9133
to
555cc32
Compare
Some questions have come up that I would like clarification on:
|
555cc32
to
1026215
Compare
Suggestion: |
From discussion on call. Agreement to add: Next steps: Nisha to revise PR, then submit for re-review. |
0235fae
to
957d16e
Compare
ba088fe
to
722bd20
Compare
@rjb4standards Not sure if I added the information you requested. Please review. |
722bd20
to
d5a0c46
Compare
chapters/package-information.md
Outdated
a LICENSE file | ||
a CHANGE log | ||
a properties file | ||
``` |
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.
@nishakm this looks good. Will need to sync this material with section 5.2.2 which needs to be updated accordingly
https://github.com/spdx/spdx-spec/blob/development/v2.3/chapters/composition-of-an-SPDX-document.md
:
5.2.2 Package information section
If SPDX information is being used to describe packages, then one instance of the Package Information per package being described shall exist. It provides important meta information about the package as a whole. Packages are an abstract concept that can be used to refer to any distribution of software, typically consisting of one or more files and capable of containing sub-packages. Starting with SPDX 2.0, it is not necessary to have a package wrapping a set of files.
A Package refers to any unit of content that can be associated with a distribution of software. Typically, a Package is composed of one or more files. An SPDX document may, but is not required to, provide details about the individual files comprising a Package (see Clause 8).
Any of the following non-limiting examples may be (but are not required to be) represented in SPDX as a Package:
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.
Oh I see! I will update accordingly.
Nisha,
I added a comment regarding the need to sync up section 5.2.2 with section 7 concepts.
Thanks,
Dick Brooks
Active Member of the CISA Critical Manufacturing Sector,
Sector Coordinating Council – A Public-Private Partnership
<https://reliableenergyanalytics.com/products> Never trust software, always verify and report! ™
<http://www.reliableenergyanalytics.com/> http://www.reliableenergyanalytics.com
Email: ***@***.***> ***@***.***
Tel: +1 978-696-1788
From: nisha ***@***.***>
Sent: Thursday, April 28, 2022 1:25 PM
To: spdx/spdx-spec ***@***.***>
Cc: Dick Brooks ***@***.***>; Mention ***@***.***>
Subject: Re: [spdx/spdx-spec] Add Package Purpose field (PR #622)
@rjb4standards <https://github.com/rjb4standards> Not sure if I added the information you requested. Please review.
—
Reply to this email directly, view it on GitHub <#622 (comment)> , or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABFI3NHX2MAOWHZGXXW6CJDVHLCX5ANCNFSM5NY5JTWA> .
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
d5a0c46
to
0531f15
Compare
@rjb4standards I just realized what you were asking as a request to sync the package information section. There is a lot to synchronize, so if you don't mind, I would like to submit a separate PR for that bit once this PR is merged. I think you filed an issue for it so I will refer to that. |
No problem at all Nisha - I defer to your better judgement as to how to accomplish this matter. Thanks for the update. |
- Add information about "package" in the information section - Add metadata about Package Purpose Fixes spdx#621 Signed-off-by: nisha <[email protected]>
0531f15
to
d66bab6
Compare
Fixes #621
Signed-off-by: Nisha K [email protected]