cosign verify-manifest now supports Jobs and CRD Types that fit within PodSpec, PodSpecTemplate, or JobSpecTemplate #697
Conversation
|
We've got |
|
See: cosign/cmd/cosign/webhook/main.go Lines 76 to 82 in fb04df8 |
|
Ok awesome, let me switch to those. |
|
Na, |
|
honestly parsing it twice feels less gross than your crazy struct beast 🤣 |
|
It would be parsing three times, or using the scheme. This is very simple: type imageContainer struct {
Spec struct {
Containers []struct {
Image string
}
InitContainers []struct {
Image string
}
Template struct {
Spec struct {
Containers []struct {
Image string
}
InitContainers []struct {
Image string
}
}
}
JobTemplate struct {
Spec struct {
Template struct {
Spec struct {
Containers []struct {
Image string
}
InitContainers []struct {
Image string
}
}
}
}
}
}
}I could break it apart if you like, I like the idea of the decoder getting to throw away the majority of the keys that come in. |
| } | ||
|
|
||
| func getImagesFromYamlManifest(manifest []byte) ([]string, error) { | ||
| dec := yaml.NewYAMLOrJSONDecoder(bytes.NewReader(manifest), 4096) |
There was a problem hiding this comment.
I'm sort of puzzled by how this works without the annotations 🤔
There was a problem hiding this comment.
the marshaler assumes camelCase for the keys, the json keys are only needed when you want to control a behavior or key name (like you want init_containers vs the default initContainers).
| // imageContainer is the union type that match PodSpec, PodSpecTemplate, and | ||
| // JobSpecTemplate; but filtering all keys except for `Image`. | ||
| type imageContainer struct { | ||
| Spec struct { |
There was a problem hiding this comment.
I'd be curious what this did if you just inlined both of duckv1.Pod and duckv1.WithPod 🤔 😅 🤣
There was a problem hiding this comment.
I did try, and you can't because both duckv1.Pod and duckv1.WithPod define the common metadata and only one wins. The decoder gets real confused 🤣 .
There was a problem hiding this comment.
I could create my own union type and break the duckv1.Pod components up and make a new parent wrapper like I do here, but this usage just is not complicated enough to bother
There was a problem hiding this comment.
Yeah, I wouldn't worry about it.
|
I don't really care, it just seems super verbose, and I was hoping for better. We can always iterate later. Do you want to add a ksvc test, since it should work? Is there one already maybe? |
|
Also, I'd be curious if this worked with CronJob, which isn't PodSpecable 🙄 |
Very poor first showing from the new Scott. Really makes you miss the old Scott |
and I test for cron jobs: https://github.com/sigstore/cosign/pull/697/files#diff-4756f111371f1063a1bc2510070a8b51ebe6d2dd2c45b28cc0c785ff1eeacd34R205-R230
I test with a made up type: https://github.com/sigstore/cosign/pull/697/files#diff-4756f111371f1063a1bc2510070a8b51ebe6d2dd2c45b28cc0c785ff1eeacd34R121-R131 😎 |
ok I can do better. One sec. |
|
/hold |
…n PodSpec, PodSpecTemplate, or JobSpecTemplate Signed-off-by: Scott Nichols <[email protected]>
|
/unhold |
|
tell me you have used prow without telling me you used prow... 🤣 |
Summary
Rather than using the known Kubernetes types, to extract images from inbound manifests using
cosign verify-manifest, we can use a simple duck type to union the types into just the fields we need and extract the image fields directly.As a result of this change, we can now verify the manifests of things like Knative Serving.
Ticket Link
Fixes #679
Release Note