opencontainers / distribution-spec

OCI Distribution Specification
https://opencontainers.org
Apache License 2.0
792 stars 199 forks source link

conformance: make blob mount and upload checks more strict #399

Closed imjasonh closed 1 year ago

imjasonh commented 1 year ago
  1. when mounting is successful, require the Location response header to be exactly /v2/<repo>/blobs/<digest>, and not just contain the string <repo>
  2. when mounting is unsuccessful and Location points to an upload session URL, require the Location response header to start with /v2/<repo>/blobs/uploads/, and not just contain /blobs/uploads/.

In the case of (2), this corresponds to common behavior, but based on my reading of the spec it might not actually be exactly specified as such:

Alternatively, if a registry does not support cross-repository mounting or is unable to mount the requested blob, it SHOULD return a 202. This indicates that the upload session has begun and that the client MAY proceed with the upload.

(source)

We may want to clarify this failed-mounting behavior more to specify that the Location response header MUST be /v2/<repo>/blobs/uploads/, as it normally is.

imjasonh commented 1 year ago

Also, I'm not sure what automated checks we have on conformance test changes to check that this doesn't regress on any currently-passing registries.

mikebrow commented 1 year ago
  1. when mounting is successful, require the Location response header to be exactly /v2/<repo>/blobs/<digest>, and not just contain the string <repo>
  2. when mounting is unsuccessful and Location points to an upload session URL, require the Location response header to start with /v2/<repo>/blobs/uploads/, and not just contain /blobs/uploads/.

In the case of (2), this corresponds to common behavior, but based on my reading of the spec it might not actually be exactly specified as such:

Alternatively, if a registry does not support cross-repository mounting or is unable to mount the requested blob, it SHOULD return a 202. This indicates that the upload session has begun and that the client MAY proceed with the upload.

(source)

We may want to clarify this failed-mounting behavior more to specify that the Location response header MUST be /v2/<repo>/blobs/uploads/, as it normally is.

we used the term name instead of repo to avoid namespace and repository format requirements

1hr approve and merge? That must be a record :-O

If memory serves at one point v1 docker api was considered acceptable for certain scenarios.. probably why this wasn't necessarily restricted?

jdolitsky commented 1 year ago

RE:

Also, I'm not sure what automated checks we have on conformance test changes to check that this doesn't regress on any currently-passing registries.

See https://github.com/opencontainers/distribution-spec/pull/401

cc @imjasonh