spec: include optional port separator#498
spec: include optional port separator#498artificial-intelligence wants to merge 1 commit intoopencontainers:mainfrom
Conversation
This change makes the separators consistent with the implementation in the docker distribution [here](https://github.com/distribution/distribution/blob/97b1d649c4938d0f608d96454d6a8326b1f96acd/reference/regexp.go#L65) 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](https://github.com/6ax) [here](docker/docker-py#3195 (comment)) Related: docker/docker-py#3195 Signed-off-by: Sven Kieske <kieske@osism.tech>
Closes: docker#3195 Related: opencontainers/distribution-spec#498 Signed-off-by: Sven Kieske <kieske@osism.tech>
sudo-bmitch
left a comment
There was a problem hiding this comment.
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: distribution/distribution@bbd41f4
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 My main complaint would be the sentence: does this mean, that every tag must match this regex, or only a 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 🙂
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! |
|
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. |
Update the regex and add test cases. (There are some xfails here for cases that the regex is not currently handling. It's too strict for IPv6 domains at the moment.) Closes: #3195 Related: opencontainers/distribution-spec#498 Signed-off-by: Sven Kieske <kieske@osism.tech> Signed-off-by: Milas Bowman <milas.bowman@docker.com> Co-authored-by: Milas Bowman <milas.bowman@docker.com>
|
Closing this one for now because we don't define a reference in OCI, yet. |
This change makes the separators consistent
with the implementation in the docker distribution here
Relevant part from the code:
Fix found by: 6ax here Related: docker/docker-py#3195