-
Notifications
You must be signed in to change notification settings - Fork 506
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
Full set of bom READMEs and documentation #2109
Conversation
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.
Great documentation, I added a few nits and clarifications. 👍
cmd/bom/README.md
Outdated
Bill of Materials project. It enables software authors to generate an | ||
SBOM for their projects in a simple, yet powerful way. | ||
|
||
[![asciicast](https://asciinema.org/a/418528.svg)](https://asciinema.org/a/418528) |
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.
Does it make sense to put the svg into this repository?
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.
OK, I've found a way to rewrite the asciinema cast to an animated one. I've uploaded it
@@ -0,0 +1,76 @@ | |||
# bom (Bill of Materials) |
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.
For consistency, we should clarify how we want to spell the abbreviation, like:
bom
when referring to the binary- BOM when referring to "Bill of Materials"
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've seen that as of late SBOM has been gaining more traction and BOM is getting left behind. I used SBOM to abbreviate a generic bill of materials. I will set all instances of bom
in backticks to refer to the binary
|
||
Packages are a non-specific element in SPDX representing anything that can | ||
group other elements. An `.rpm` or `.deb` package can be an SPDX package, but | ||
so can be a docker image or a tarball. Packages contain files, but can also |
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.
so can be a docker image or a tarball. Packages contain files, but can also | |
so can be a container image or a tarball. Packages contain files, but can also |
I'll not highlight more of those since it will occur in multiple places in the following paragraphs. I assume it works with OCI images in the same way as Docker v2 manifests, right? Then we should always refer the OCI format aka the term "container image".
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.
Absolutely right. I have to change that automatic behavior of always writing "docker image". I changed all of them in the dock.
0c2bc36
to
1e7377a
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.
here's my first pass, mostly nitpicky. well done on the overall write up!
packing the data along with licensing information. | ||
|
||
bom is still in its early stages and it is an effort to open | ||
the libraries developed for the Kubernetes SBOM for other |
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 made suggestions on BOM / SBOM references but generally I have mixed feelings on the usage of them. On one hand it increases search hits to use both, but on the other I think we should be consistent on which term to use. I'm leaning on saying SBOM for sake of consistency, since we already mention SBOM in the document for "What is Bill of Materials".
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.
+1 I've noticed SBOM gaining more acceptance in discussions happening everywhere. Following up on @saschagrunert comment someplace else, I think we should settle on:
bom
for the binary
SBOM to refer to a software bill of materials in general.
This commit adds an in-depth howto with instructions to generate a Bill of Materials. Signed-off-by: Adolfo García Veytia (Puerco) <[email protected]>
Signed-off-by: Adolfo García Veytia (Puerco) <[email protected]>
This commit adds the bom utility readme. It features an asciinema recording show how it is used.
This commit adds the raw documentation for bom and its only subcommand: generate. The format mimics previous docs for our tools (like krel). Signed-off-by: Adolfo García Veytia (Puerco) <[email protected]>
I think I've addressed all nits, PTAL @saschagrunert , @wilsonehusin |
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
📦
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: puerco, wilsonehusin The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/kind documentation
What this PR does / why we need it:
This PR adds the documentation for the first version of the
bom
utility. Still missing is the YAML compositiondocumentation, but the PR has not yet merged.
Specifically, this commit does the following:
Another commit adds bom to the
compile-release-tools
script so that end users can compile it.Which issue(s) this PR fixes:
Part of: #1837
Special notes for your reviewer:
Does this PR introduce a user-facing change?