-
Notifications
You must be signed in to change notification settings - Fork 15
Implement more TODOs in oci-validate
code
#125
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
base: main
Are you sure you want to change the base?
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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. 🤔
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, 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.
I'm much happier with this interface now -- it can properly handle an image with attestations now, for example.