-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Attestations docs #3260
Attestations docs #3260
Conversation
docs/attestations.md
Outdated
--opt build-arg:SBOM_SCAN_CONTEXT=true \ | ||
--opt attest:sbom=generator=jedevc/buildkit-syft-scanner | ||
``` | ||
|
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.
We could show a snippet of how the SPDX content actually looks like.
How to add custom packages to SBOM using the nested SBOM technique.
How to read the layer info from the SPDX record.
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.
- Added an SPDX snippet ✔️
- This is scanner-specific behaviour - we should document this in the scanner itself, since a scanner may not include this information. Also, this behaviour isn't fully implemented in syft, and is partially blocked on SBOM cataloger anchore/syft#1029. ✖️
- Added info on the layer data ✔️
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.
CC @wagoodman @kzantow 🎉
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 the default scanner support the nested files now that the PR looks to be merged?
I think this should be documented in the regular sbom docs like the Dockerfile ARGs. This is something that we expect the users to know and modify their Dockerfiles to get better sboms. We can mention that this works in default scanner and if user uses custom scanner then it is up to that implementation to decide if it is supported.
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.
Ahh, so yes syft has merged it, but it's not made into a release.
I think we should aim to follow syft releases for buildkit-syft-scanner, instead of building off of master?
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.
We already vendor a syft release right? https://github.com/docker/buildkit-syft-scanner/blob/6a7324fe9deac77f3a6f22770b7a8a09b4ce1723/go.mod#L8
|
||
> **Note** | ||
> | ||
> Currently, only SBOMs in the [SPDX](https://spdx.dev) JSON format are |
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.
Follow-up: could we use some labels to track what is supported so we have a upgrade path?
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 assume you mean in the image labels? Yup, I'll do a follow-up 😄
Added do-not-merge label - this PR documents functionality that is added in:
Additionally, we need to move jedevc/buildkit-syft-scanner to another location 😄 |
7261129
to
7ead879
Compare
ed2e9c4
to
ea4a476
Compare
Signed-off-by: Justin Chadwell <me@jedevc.com>
Signed-off-by: Justin Chadwell <me@jedevc.com>
ea4a476
to
db5489f
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.
LGTM
This PR adds two pieces of documentation: