opencontainers / distribution-spec

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

spec: include optional port separator #498

Closed artificial-intelligence closed 6 months ago

artificial-intelligence commented 9 months ago

This change makes the separators consistent with the implementation in the docker distribution here

Relevant part from the code:

// optionalPort matches an optional port-number including the port separator
// (e.g. ":80").
optionalPort = `(?::[0-9]+)?`

Fix found by: 6ax here Related: https://github.com/docker/docker-py/issues/3195

artificial-intelligence commented 9 months ago

This regexp is for the name component of the requests received by the registry and does not include the hostname or post. It's a path on the registry.

So I'm really no expert on the spec, so it's totally possible I'm wrong but, according to:

https://github.com/distribution/distribution/commit/bbd41f40bba09ef34f532089aae8578f66130518

optionalPort is a part of the remote-name component which in turn is used interchangeably with path according to the linked commit message. That made me think this could also be a part of a tag, is that wrong? It looks like it, according to your comment?

But before docker-py 7.0.0 it was possible to use ":" in a tag at least, see the linked bug report.

I'm not sure if this is or is not allowed by the spec, because I honestly find the wording in the Pulling manifests paragraph quite confusing.

My main complaint would be the sentence:

Throughout this document, `<reference>` as a tag MUST be at most 128 characters in length and MUST match the following regular expression:

does this mean, that every tag must match this regex, or only a <reference> must match this regex when used as a tag? Can there be something else as a tag, that is not a reference? That's not 100% clear to me.

I think the following changes could it make more clear to the reader, but this is just my reading as a drive-by-contributor, so ymmv :slightly_smiling_face:

Maybe I just missed this information somewhere in the docs, or in some of the linked docs.

Sorry if this maybe got a little bit off topic.

Thanks for your work and help on this!

sudo-bmitch commented 9 months ago

Distribution and distribution-spec are separate projects. There's a lot of overlap in the terms, but also inconsistent usage between the two. The concept of a reference from the distribution project doesn't exist in the distribution-spec. At least not yet.

The distribution-spec only deals with the http API, and the name field referenced here is within the Path of that request. It does not cover the Host field in the http API, that isn't specified in the spec.

sudo-bmitch commented 6 months ago

Closing this one for now because we don't define a reference in OCI, yet.