Skip to content

Commit

Permalink
ociregistry/ociclient: add knownDigest to Resolve* methods
Browse files Browse the repository at this point in the history
In the OCI spec, having `HEAD` requests return the digest header is
technically only a "SHOULD" (not a "MUST"), and it turns out that Docker
Hub is one of the registries on the sad side of that "SHOULD" -- the
code elsewhere was already accounting for this via the `knownDigest`
parameter to `descriptorFromResponse`, but it was (accidentally?) set to
`""` explicitly for all the `resolve` methods (even though 2 out of 3 of
them are by-digest):

https://github.com/opencontainers/distribution-spec/blob/v1.1.0/spec.md#checking-if-content-exists-in-the-registry

> A HEAD request to an existing blob or manifest URL MUST return `200
> OK`. A successful response SHOULD contain the digest of the uploaded
> blob in the header `Docker-Content-Digest`. A successful response
> SHOULD contain the size in bytes of the uploaded blob in the header
> `Content-Length`.

As a result, my attempts at using `ResolveBlob` to check if a specific
blob exists on Docker Hub were failing with "invalid descriptor in
response: no digest found in response" even though the proper digest was
in the request and the response was 200 OK:

```console
$ curl -H "$(crane auth token --header busybox)" --head -L https://registry-1.docker.io/v2/library/busybox/blobs/sha256:7b2699543f22d5b8dc8d66a5873eb246767bca37232dee1e7a3b8c9956bceb0c
HTTP/1.1 307 Temporary Redirect
content-type: application/octet-stream
docker-distribution-api-version: registry/2.0
location: https://production.cloudflare.docker.com/registry-v2/docker/registry/v2/blobs/sha256/7b/7b2699543f22d5b8dc8d66a5873eb246767bca37232dee1e7a3b8c9956bceb0c/data?verify=1709871346-z5HS81nN1Ov9OEB3RdN60eJ2MjA%3D
date: Fri, 08 Mar 2024 03:25:46 GMT
strict-transport-security: max-age=31536000

HTTP/2 200
date: Fri, 08 Mar 2024 03:25:46 GMT
content-type: application/octet-stream
content-length: 2152262
cf-ray: 860fb84bfd2f2b58-LAX
cf-cache-status: HIT
accept-ranges: bytes
age: 170784
cache-control: public, max-age=14400
etag: "2aeecf61106ed1c6e3a774e3db220ca7"
expires: Fri, 08 Mar 2024 07:25:46 GMT
last-modified: Wed, 06 Mar 2024 00:07:43 GMT
vary: Accept-Encoding
x-amz-id-2: GIEh93BaQl88NHOZYTVLgwDtBjOzY96VsTcyP2qql4lrK98VY+vcj1wOxHAT47ChDa+wRCKv7Pk=
x-amz-request-id: 8YB4XDY06KMSY2GN
x-amz-server-side-encryption: AES256
x-amz-version-id: oxJ9x0zrl0ICqBGgSAIDsycrCQ9qVxw5
server: cloudflare
```

I have verified that this fixes the issue and allows the `ResolveBlob`
call to complete successfully against Docker Hub.

Closes #29 as merged as of commit b652d61.

Signed-off-by: Tianon Gravi <admwiggin@gmail.com>
Change-Id: I47bec2e50528d7fb566eeaddfafdf9d44291b13a
Reviewed-on: https://review.gerrithub.io/c/cue-labs/oci/+/1191109
TryBot-Result: CUE porcuepine <cue.porcuepine@gmail.com>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
  • Loading branch information
tianon authored and rogpeppe committed Apr 3, 2024
1 parent 7eb5fc6 commit 9dc4175
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 3 deletions.
4 changes: 2 additions & 2 deletions ociregistry/ociclient/reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ func (c *client) resolve(ctx context.Context, rreq *ocirequest.Request) (ociregi
return ociregistry.Descriptor{}, err
}
resp.Body.Close()
desc, err := descriptorFromResponse(resp, "", true)
desc, err := descriptorFromResponse(resp, ociregistry.Digest(rreq.Digest), true)
if err != nil {
return ociregistry.Descriptor{}, fmt.Errorf("invalid descriptor in response: %v", err)
}
Expand Down Expand Up @@ -176,7 +176,7 @@ func (c *client) read(ctx context.Context, rreq *ocirequest.Request) (_ ociregis
return nil, err
}
resp1.Body.Close()
desc, err = descriptorFromResponse(resp1, "", true)
desc, err = descriptorFromResponse(resp1, ociregistry.Digest(rreq1.Digest), true)
if err != nil {
return nil, err
}
Expand Down
8 changes: 7 additions & 1 deletion ociregistry/ociserver/reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,13 @@ func (r *registry) handleManifestHead(ctx context.Context, resp http.ResponseWri
if err != nil {
return err
}
resp.Header().Set("Docker-Content-Digest", string(desc.Digest))
if !r.opts.OmitDigestFromTagGetResponse || rreq.Tag != "" {
// Note: when doing a HEAD of a tag, clients are entitled
// to expect that the digest header is set on the response
// even though the spec says it's only optional in this case.
// TODO raise an issue on the spec about this.
resp.Header().Set("Docker-Content-Digest", string(desc.Digest))
}
resp.Header().Set("Content-Type", desc.MediaType)
resp.Header().Set("Content-Length", fmt.Sprint(desc.Size))
resp.WriteHeader(http.StatusOK)
Expand Down

0 comments on commit 9dc4175

Please sign in to comment.