Implement more TODOs in oci-validate code#125
Conversation
|
I think I'm happy enough with this that it could become part of our builds, actually. 🤔 Longer-term I want to munge all our builds (even classic builder) to output an OCI layout, so this could become a standard part of the pipeline. For that we would want to get even more specific ( |
invalid .["manifests"][0]:
{"mediaType":"application/vnd.oci.image.index.v1+json","digest":"sha256:9DEE8c8868c2b379281f0956df25a86acb830c1f3eb88aa23a5a710547fe3165","size":1298}
invalid .["digest"]:
"sha256:9DEE8c8868c2b379281f0956df25a86acb830c1f3eb88aa23a5a710547fe3165"
invalid value:
{"algorithm":"sha256","encoded":"9DEE8c8868c2b379281f0956df25a86acb830c1f3eb88aa23a5a710547fe3165"}
invalid .["encoded"]:
"9DEE8c8868c2b379281f0956df25a86acb830c1f3eb88aa23a5a710547fe3165"
the encoded portion MUST match /[a-f0-9]{64}/"this brings joy" |
| local digest size dataDigest= dataSize= | ||
| digest="$("${algo}sum" "$file" | cut -d' ' -f1)" | ||
| digest="$algo:$digest" | ||
| size="$(stat --dereference --format '%s' "$file")" | ||
| if [ "$data" != ' ' ]; then | ||
| dataDigest="$(base64 <<<"$data" -d | "${algo}sum" | cut -d' ' -f1)" | ||
| dataDigest="$algo:$dataDigest" | ||
| dataSize="$(base64 <<<"$data" -d | wc --bytes)" | ||
| # TODO *technically* we could get clever here and pass `base64 -d` to something like `tee >(wc --bytes) >(dig="$(sha256sum | cut -d' ' -f1)" && echo "sha256:$dig" && false) > /dev/null` to avoid parsing the base64 twice, but then failure cases are less likely to be caught, so it's safer to simply redecode (and we can't decode into a variable because this might be binary data *and* bash will do newline munging in both directions) | ||
| fi |
There was a problem hiding this comment.
Should this be able to handle a descriptor that only has data and not also a file on disk? Or is that not a valid OCI layout?
There was a problem hiding this comment.
This is such an (unintentionally) brutal comment 😂 😭
You're absolutely right, and making me question whether I like this interface again.
Implementations worried about portability should absolutely have both data and the blob in the blobs/ folder, but it's not technically invalid not to, and I'd love to support that. 🤔
yosifkit
left a comment
yosifkit
left a comment
There was a problem hiding this comment.
LGTM, we can add more extensive data field using later since it isn't necessary right now.
I'm much happier with this interface now -- it can properly handle an image with attestations now, for example.
2 participants
I'm much happier with this interface now -- it can properly handle an image with attestations now, for example.