Apply fallback method for returning manifest digest when Docker-Content-Digest header is not set#3793
Apply fallback method for returning manifest digest when Docker-Content-Digest header is not set#3793rsafonseca wants to merge 1 commit into
Conversation
Signed-off-by: Rafael da Fonseca <rsafonseca@gmail.com>
| return nil, err | ||
| } | ||
| m, _, err := distribution.UnmarshalManifest(mt, body) | ||
| m, desc, err := distribution.UnmarshalManifest(mt, body) |
There was a problem hiding this comment.
I'd like to have some eyes on this, as the original implementation of this was to use the digest as known by the registry #1775. Maybe that was a poor choice of words in that PR, but;
- Is the unmarshalfunction guaranteed to return the same digest?
- Would this defeat the purpose of the
Docker-Content-Digestheader (IIRC, it's also used to verify if the downloaded digest matches the digest as specified in the header?)
(Maybe I'm wrong on either of them; just want to be sure this gets the right eyes) 😅
There was a problem hiding this comment.
I've tested a few different repositories other than AWS ECR, and they seem to be returning the exact same SHA in the HTTP header and in the Descriptor.Digest.
As I wasn't sure that this does exist in the descriptor for other older manifest/registry versions, I thought to keep the old behaviour as default to ensure it would not impact anything or anyone.
Indeed at some point maybe it makes sense to deprecate the old header based behaviour, especially since it's an explicit docker reference, and I assume that the lib is supposed to start becoming more agnostic of specific CREs, but I'll leave that for someone else to decide down the line.
More eyes welcome for sure :)
This PR addresses issue #3792
Implements a simple fallback mechanism for obtaining the digest from the Descriptor in the registry's API response body, while maintaining the original behavior.