-
Notifications
You must be signed in to change notification settings - Fork 663
Protobuf bundle support for subcommands save and load
#4530
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
Conversation
Signed-off-by: Zach Steindler <[email protected]>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4530 +/- ##
==========================================
- Coverage 40.10% 36.34% -3.76%
==========================================
Files 155 220 +65
Lines 10044 12285 +2241
==========================================
+ Hits 4028 4465 +437
- Misses 5530 7127 +1597
- Partials 486 693 +207 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| if err != nil { | ||
| return err | ||
| } | ||
| err = WriteSignaturesExperimentalOCI(digest, se, opts...) |
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.
Definitely not at all an expert in this part of the codebase, so I'm trying to wrap my head around how WriteSignaturesExperimentalOCI works here. Is this now needed because we were writing referring artifacts to OCI using a different API?
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.
Yeah, I also had to do a lot of digging in this code base to understand what was going on.
But that's exactly right - we don't want to "just" write the protobuf bundle as an OCI blob, we also need to write an OCI manifest that (1) points at the protobuf bundle blob and (2) has a subject referring to the signed image - which is how the registry knows that this is a "referring artifact".
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.
That makes sense, thanks. So how were things working previously? Was it that we were looking up the referring artifact directly rather than going through the OCI manifest?
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 guess to add to that, is there a change we need to make to the verification code?
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.
Yeah, so previously signatures were pushed to a tag made from the digest of the image manifest.
So if your container was example.com/myapp@sha256:abcd, then you would ask for the signature at example.com/myapp:sha256-abcd.sig (note that this is a tag, even though it looks like a digest).
The verification code shouldn't need to change. New protobuf bundle signatures should always use the OCI referrers API. But I still need to update tests 😅
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.
Sorry, by previously I meant with bundles and OCI. If we were only publishing the referring artifact and not the manifest that points to the referring artifact, how was verification working? How did the OCI library know how to look up the referring artifact without a manifest?
Signed-off-by: Zach Steindler <[email protected]>
|
I don't think this should be part of v3.0.3 anymore 😅 Saving is pretty straightforward. For loading you basically need to implement something that behaves like the OCI referring API (look at the files for manifests that are the subject of the container image). Such a thing is possible, but I'm really not familiar with |
|
I'm wondering if not writing the manifest is a bug that needs to be fixed before a new release? |
Summary
Continuing to make progress on #4470.
To test:
Run local registries on port
1338(default) and port1337(to test loading)Build and sign an image:
orasto confirm they are in new registry:Release Note
cosign saveandcosign loadDocumentation
N/A