-
Notifications
You must be signed in to change notification settings - Fork 401
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 writing SBOMs when the build.Result
is oci.Signed*
.
#506
Conversation
5c6331e
to
ed9005f
Compare
if (targetRepoOverride != name.Repository{}) { | ||
ociOpts = append(ociOpts, ociremote.WithTargetRepository(targetRepoOverride)) | ||
} | ||
h, err := se.(interface{ Digest() (v1.Hash, error) }).Digest() |
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.
Wut
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 don't have a Digestable
interface, and this is what I've been using as a surrogate. This has to be either SignedImage
or SignedImageIndex
and both implement this.
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.
Or maybe I'll implement WriteSBOM
and just hide it from you 😂
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'll only have SBOMs for images, so can we just cast that here?
This is a bit complicated since we'll want to sign the whole index, but presumably sign each platform-specific SBOM.
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 is called on both indices and images regardless of which we're dealing with. For indices below this is handled by the walk.SignedEntity
. Ultimately it'll write everything it needs to (sigs for index, SBOMs for images), and we'll have flexibility to get smarter about placement without worrying too much. I don't feel super strongly, but I'm not sure what exactly you are proposing as an alternative?
ed9005f
to
8f935f8
Compare
// annotations indicating the digest (and possibly tag) of the | ||
// base image. This will be inherited by the image produced. | ||
if mt != types.DockerManifestList { |
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 moved this up because it gets the job done and it avoids me needing to create an ocimutate.Annotations
so this preserves the SBOM attachment.
TODO list:
I'll probably knock these off tomorrow, but since it's mostly testing I'd appreciate feedback on the main bits sooner than later. |
Non blocking questions:
|
I think the way I’d bias the implementation would be to have publishers publish everything that exists, down to just the v1.Image, and toggle what’s “attached” to the oci.Signed* (then we save on computing SBOMs and signatures). I think my inclination would be:
I’ll think about how we guard SBOM after I write some tests. 👍 |
b07c14e
to
c49bb36
Compare
e69bf48
to
6334bc2
Compare
6334bc2
to
28d16f2
Compare
build.Result
is signed.build.Result
is oci.Signed*
.
Ok the latest changes should have all of the pieces including |
28d16f2
to
ce3af5f
Compare
I added one final change: normalizing the binary path printed by Basically prior to this the SBOMs weren't deterministic, so nop builds would churn the SBOM. With this I get deterministic SBOMs and nothing is written on reruns. |
pkg/publish/default.go
Outdated
@@ -120,18 +124,73 @@ func pushResult(tag name.Tag, br build.Result, opt []remote.Option) error { | |||
return err | |||
} | |||
|
|||
// writePeripherals implements walk.Fn | |||
writePeripherals := func(_ context.Context, se oci.SignedEntity) 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.
Can we use the context passed in here when remote.Write
ing at least? We might also want to pipe the ctx through to SBOM generation so that interrupting ko
cancels the go version -m
call.
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 looks like the context we would thread through is already part of the opts
being passed in.
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.
LMK if you want me to do it again for good measure!
Codecov Report
@@ Coverage Diff @@
## main #506 +/- ##
==========================================
+ Coverage 51.12% 51.27% +0.14%
==========================================
Files 41 41
Lines 1950 2007 +57
==========================================
+ Hits 997 1029 +32
- Misses 794 807 +13
- Partials 159 171 +12
Continue to review full report at Codecov.
|
This adds functionality that enables the default publisher to publish SBOMs (and later signatures and attestations) when the `build.Result` is an `oci.SignedEntity`. This also changes the `gobuild` logic to start producing `oci.Signed*` as its `build.Result`s, so when executed we get an SBOM for each architecture image. For example, see the "Published SBOM" lines below: ```shell 2021/11/19 19:24:50 Using base gcr.io/distroless/static:nonroot for github.com/google/ko 2021/11/19 19:24:51 Building github.com/google/ko for linux/amd64 2021/11/19 19:24:52 Building github.com/google/ko for linux/arm64 2021/11/19 19:24:57 Publishing ghcr.io/mattmoor/ko:latest 2021/11/19 19:24:58 existing blob: sha256:c78c74e7bb4a511f7d31061fbf140d55d5549a62d33cdbdf0c57ffe43603bbeb 2021/11/19 19:24:58 existing blob: sha256:4aa59d0bf53d4190174fbbfa3e9b15fdab72e5a95077025abfa8435ccafa2920 2021/11/19 19:24:58 ghcr.io/mattmoor/ko:sha256-d2bc030f5ed083d5e6a30a7969c9a8e599511b8d7a6e20695bf5ea029b6e2c3f.sbom: digest: sha256:c67ec671aaa82902e619883a7ac7486e6f9af36653449e2eb030ba273fe5a022 size: 348 2021/11/19 19:24:58 Published SBOM ghcr.io/mattmoor/ko:sha256-d2bc030f5ed083d5e6a30a7969c9a8e599511b8d7a6e20695bf5ea029b6e2c3f.sbom 2021/11/19 19:24:58 existing blob: sha256:c78c74e7bb4a511f7d31061fbf140d55d5549a62d33cdbdf0c57ffe43603bbeb 2021/11/19 19:24:58 existing blob: sha256:4aa59d0bf53d4190174fbbfa3e9b15fdab72e5a95077025abfa8435ccafa2920 2021/11/19 19:24:59 ghcr.io/mattmoor/ko:sha256-b74c230f20efd94981e5fd823bacc23cbd71055a1b3b6a0893152b398c67743b.sbom: digest: sha256:c67ec671aaa82902e619883a7ac7486e6f9af36653449e2eb030ba273fe5a022 size: 348 2021/11/19 19:24:59 Published SBOM ghcr.io/mattmoor/ko:sha256-b74c230f20efd94981e5fd823bacc23cbd71055a1b3b6a0893152b398c67743b.sbom 2021/11/19 19:24:59 existing blob: sha256:3f7e3c6765a6abc682cd40ea256fbea5c1d4debbc07659efbc0dedc13eee0da6 2021/11/19 19:24:59 existing blob: sha256:250c06f7c38e52dc77e5c7586c3e40280dc7ff9bb9007c396e06d96736cf8542 2021/11/19 19:24:59 existing blob: sha256:e8614d09b7bebabd9d8a450f44e88a8807c98a438a2ddd63146865286b132d1b 2021/11/19 19:24:59 existing blob: sha256:7067b1bc6f9ce59f3a4ed2216946ebbb27a4f7a102f55d96c6af1dc90e77b510 2021/11/19 19:25:00 ghcr.io/mattmoor/ko@sha256:d2bc030f5ed083d5e6a30a7969c9a8e599511b8d7a6e20695bf5ea029b6e2c3f: digest: sha256:d2bc030f5ed083d5e6a30a7969c9a8e599511b8d7a6e20695bf5ea029b6e2c3f size: 751 2021/11/19 19:25:01 existing blob: sha256:250c06f7c38e52dc77e5c7586c3e40280dc7ff9bb9007c396e06d96736cf8542 2021/11/19 19:25:02 pushed blob: sha256:121c637d5c84562b51404a6f71c1f995ad059740293a3911a0dc33eb223e41a4 2021/11/19 19:25:02 pushed blob: sha256:859e03b7461b2a512159493ef1504d2859ed37c05ed1ef781ff98394ea4799b5 2021/11/19 19:25:02 pushed blob: sha256:d1b55c3db0f16b5056776c6d2c279efd16d28dbf1aae3eef1f3f9b7551d1f490 2021/11/19 19:25:03 ghcr.io/mattmoor/ko@sha256:b74c230f20efd94981e5fd823bacc23cbd71055a1b3b6a0893152b398c67743b: digest: sha256:b74c230f20efd94981e5fd823bacc23cbd71055a1b3b6a0893152b398c67743b size: 751 2021/11/19 19:25:03 ghcr.io/mattmoor/ko:latest: digest: sha256:e4466a7dd9be66c7c1b43a8ecc19247041ece232407a14e3d6ea3c51d2561a71 size: 529 2021/11/19 19:25:03 Published ghcr.io/mattmoor/ko@sha256:e4466a7dd9be66c7c1b43a8ecc19247041ece232407a14e3d6ea3c51d2561a71 ghcr.io/mattmoor/ko@sha256:e4466a7dd9be66c7c1b43a8ecc19247041ece232407a14e3d6ea3c51d2561a71 ``` The "SBOM" being attached in this change is the raw output of `go version -m`, which we will convert to one of the standard formats in a subsequent change.
a817410
to
93164d9
Compare
93164d9
to
84c2ffe
Compare
run: | | ||
set -o pipefail | ||
|
||
IMAGE=$(ko publish ./test) |
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.
Do we want to add some dummy underscore-import to ./test
that makes its SBOM a bit more interesting? I wouldn't want to add any deps that aren't already included in the main tool's go.mod, but at least having ./test
's SBOM include ggcr would help exercise some code paths.
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 can add ./pkg/registry
😉
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.
# cosign download sbom $(ko build -B ./test/)
2021/11/22 10:28:51 Using base gcr.io/distroless/static:nonroot for github.com/google/ko/test
2021/11/22 10:28:52 Building github.com/google/ko/test for linux/amd64
2021/11/22 10:28:53 Publishing gcr.io/mattmoor-chainguard/test:latest
2021/11/22 10:28:55 pushed blob: sha256:f0f9ae85d92dbd28f183a89b76294c6080bb496140dea890cef03a489639d1fe
2021/11/22 10:28:55 pushed blob: sha256:11f55e089f0062ef4a4d70bff133d915aeccf878528f765482e87cfbbc738743
2021/11/22 10:28:56 gcr.io/mattmoor-chainguard/test:sha256-16a451d99de28ffc98f053a918fe797766896bbe7c9b17d1146fbac0d4d62476.sbom: digest: sha256:c31b1e02ab1d549d3257c540f6f44e40e06a7a48e8d0d9e1df33dab80efac1a3 size: 329
2021/11/22 10:28:56 Published SBOM gcr.io/mattmoor-chainguard/test:sha256-16a451d99de28ffc98f053a918fe797766896bbe7c9b17d1146fbac0d4d62476.sbom
2021/11/22 10:28:56 existing blob: sha256:e8614d09b7bebabd9d8a450f44e88a8807c98a438a2ddd63146865286b132d1b
2021/11/22 10:28:57 pushed blob: sha256:4260a810acc66202754e7bf4f5db2d4df5589780c0c41716580d7de02eaa4e8d
2021/11/22 10:28:57 pushed blob: sha256:bfb7f3f84494f5e13f177efee4ca2e587619dbec98f68971c26428bc693efeba
2021/11/22 10:28:57 pushed blob: sha256:7d0e2ce4cf408744cfdbb1fc4fc9809408bedf55a12d92c43911739d645eeed5
2021/11/22 10:28:58 gcr.io/mattmoor-chainguard/test:latest: digest: sha256:16a451d99de28ffc98f053a918fe797766896bbe7c9b17d1146fbac0d4d62476 size: 952
2021/11/22 10:28:58 Published gcr.io/mattmoor-chainguard/test@sha256:16a451d99de28ffc98f053a918fe797766896bbe7c9b17d1146fbac0d4d62476
Found SBOM of media type: application/vnd.go.version-m
/ko-app/test: go1.17.1
path github.com/google/ko/test
mod github.com/google/ko (devel)
dep github.com/google/go-containerregistry v0.7.0 h1:u0onUUOcyoCDHEiJoyR1R1gx5er1+r06V5DBhUU5ndk=
im, err := baseIndex.IndexManifest() | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
// Build an image for each child from the base and append it to a new index to produce the result. | ||
adds := []mutate.IndexAddendum{} | ||
adds := []ocimutate.IndexAddendum{} |
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 feels a bit like a smell, that we have to use cosign's mutate methods instead of ggcr's, throughout this code. And that buildOne
only ever returns a (misnomered) SignedImage
.
AIUI the Signed*
constructs exist so that we can attach things to it, can't cosign's packages just take a regular v1.Image
or v1.ImageIndex
in its Attach*
methods?
(I'm fine with forging ahead despite this smell, but I'd love to have a better way to compose these things than having ko
-- and any other tool that wants to integrate attachment -- adopt Signed*
variants everywhere)
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'd love to have a better way to compose these things than having ko -- and any other tool that wants to integrate attachment -- adopt Signed* variants everywhere
I'm certainly open to alternatives, but I would expect some degree of this unless/until OCI itself has constructs for attachments, signatures, attestations. Once that happens, GGCR gets API for it, and the Cosign-specific aspects die off.
Until then, the whole point of the "cosign as an SDK" efforts has been to make this as small a lift for GGCR users as we can make it (in terms of api familiarity and compatibility).
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.
Synced up quickly on meet. I think we're pretty well aligned that if/as Attachment-like functionality lands in GGCR that a lot of the need for cosign to define it's own interfaces for images/indices will subside, and it will devolve into providing higher-level access patterns for building / accessing things within attachments (if attachments continue to have a multiple parts, e.g. signature-per-layer).
This adds functionality that enables the default publisher to
publish SBOMs (and later signatures and attestations) when the
build.Result
is anoci.SignedEntity
.This also changes the
gobuild
logic to start producingoci.Signed*
as itsbuild.Result
s, so when executed we get anSBOM for each architecture image.
For example, see the "Published SBOM" lines below:
The "SBOM" being attached in this change is the raw output of
go version -m
,which we will convert to one of the standard formats in a subsequent change.