Keep manifest list digest in meta.json and use in cosa sign
#4306
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a good improvement by storing the manifest list digest in meta.json and using it in cosa sign. This simplifies the signing process by removing the need for an explicit command-line argument. The changes to the Go schema and Python scripts are logical and follow the stated goal.
I've found a few issues in src/cosalib/container_manifest.py related to temporary file handling and type hinting. Specifically, there's a resource leak where temporary files are not being cleaned up, and an invalid type hint is used. I've provided suggestions to fix these issues for improved robustness and correctness.
| # For the manifest list digest, reuse the tags from the x86_64 build. As | ||
| # mentioned above, it's the same tags on all arches. | ||
| if args.manifest_list_digest: | ||
| if manifest_list_digest: |
There was a problem hiding this comment.
I'm trying to come up with a situation where manifest_list_digest would ever be a false value here. Is the if condition needed or should this block just be unconditional?
There was a problem hiding this comment.
This is to be nice to older metadata that one wants to cosa sign.
Add a new `manifest-list-digest` to the OCI image objects we publish in `meta.json` for our pushed images containing a backreference to the digest of the manifest list. Otherwise, that digest is not really captured anywhere in our metadata. This could be used down the line to also add the manifest list digest to release metadata, which would be more appropriate as the aggregation point of metadata across all the arches. But the more immediate want for it is for use in `cosa sign`.
Now that we put the manifest list digest in `meta.json`, we can just use that and not have to pass it through awkwardly via another switch.
jlebon
force-pushed
the
pr/manifest-list-digest
branch
from
0157ba9 to
b88d821
Compare
|
Updated for comments! |
wow. the new code looks much easier to follow! |
schema: also keep the manifest list digest in meta.json
Add a new
manifest-list-digestto the OCI image objects we publishin
meta.jsonfor our pushed images containing a backreference tothe digest of the manifest list. Otherwise, that digest is not really
captured anywhere in our metadata.
This could be used down the line to also add the manifest list digest
to release metadata, which would be more appropriate as the aggregation
point of metadata across all the arches. But the more immediate want for
it is for use in
cosa sign.cmd-sign: use
manifest-list-digestin meta.json if availableNow that we put the manifest list digest in
meta.json, we can just usethat and not have to pass it through awkwardly via another switch.