Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

7.0: Docker Tag Parsing Issue with Registry Ports #3195

Closed
bbezak opened this issue Dec 12, 2023 · 4 comments · Fixed by #3196
Closed

7.0: Docker Tag Parsing Issue with Registry Ports #3195

bbezak opened this issue Dec 12, 2023 · 4 comments · Fixed by #3196

Comments

@bbezak
Copy link

bbezak commented Dec 12, 2023

Description:

We're facing a parsing issue in docker.py version 7.0, where registry server addresses with port numbers are incorrectly processed as part of the Docker tag, leading to an invalid tag format error.

Error:

it' happening in openstack kolla upstream CI:
docker.errors.DockerException: invalid tag 'primary:4000/lokolla/base:change_903165': invalid reference format

Possible issue:

The current regex introduced in commit: a9b5494
in docker.py does not accommodate port numbers in registry addresses:

_TAG = re.compile(
    r"^[a-z0-9]+((\.|_|__|-+)[a-z0-9]+)*(\/[a-z0-9]+((\.|_|__|-+)[a-z0-9]+)*)*" \
    + "(:[a-zA-Z0-9_][a-zA-Z0-9._-]{0,127})?$"
)
@6ax
Copy link

6ax commented Dec 12, 2023

We faced the same problem. Apparently the regular expression should be like this:
r"^[a-z0-9]+((\.|_|__|-+)[a-z0-9]+)*(:[0-9]{2,5})?" \ + "(\/[a-z0-9]+((\.|_|__|-+)[a-z0-9]+)*)*$"

@artificial-intelligence
Copy link
Contributor

well, looking at the OCI-Spec I think the new code is more correct/matches what OCI mandates.

a name must match [a-z0-9]+((\.|_|__|-+)[a-z0-9]+)*(\/[a-z0-9]+((\.|_|__|-+)[a-z0-9]+)*)*
and a tag must match [a-zA-Z0-9_][a-zA-Z0-9._-]{0,127}

@artificial-intelligence
Copy link
Contributor

artificial-intelligence commented Dec 12, 2023

:edit: this was wrong information, I misinterpreted the regex. still very weird.

@6ax
Copy link

6ax commented Dec 12, 2023

It looks like the resulting regex doesn't include this:

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

https://github.com/distribution/distribution/blob/97b1d649c4938d0f608d96454d6a8326b1f96acd/reference/regexp.go#L65

and the code should look like this? :

_TAG = re.compile(
    r"^[a-z0-9]+((\.|_|__|-+)[a-z0-9]+)*(?::[0-9]+)?(\/[a-z0-9]+((\.|_|__|-+)[a-z0-9]+)*)*" \
    + "(:[a-zA-Z0-9_][a-zA-Z0-9._-]{0,127})?$"
)

artificial-intelligence added a commit to artificial-intelligence/distribution-spec that referenced this issue Dec 12, 2023
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>
artificial-intelligence added a commit to artificial-intelligence/docker-py that referenced this issue Dec 12, 2023
Closes: docker#3195
Related: opencontainers/distribution-spec#498

Signed-off-by: Sven Kieske <kieske@osism.tech>
openstack-mirroring pushed a commit to openstack/kolla that referenced this issue Dec 14, 2023
Docker 7.0.0 introduced a pre build check for tag regex, which
fails where registry has port number defined - see [1] and [2].

[1]: docker/docker-py@a9b5494
[2]: docker/docker-py#3195

Also removing requirements check in CI as it is not allowing
such pinning, as kolla is not designed to be installed with
other openstack services in the same virtualenv.

Change-Id: Id64186bf87300f23acde4f90474abcd6944e5be0
openstack-mirroring pushed a commit to openstack/kolla that referenced this issue Dec 14, 2023
Docker 7.0.0 introduced a pre build check for tag regex, which
fails where registry has port number defined - see [1] and [2].

[1]: docker/docker-py@a9b5494
[2]: docker/docker-py#3195

Also removing requirements check in CI as it is not allowing
such pinning, as kolla is not designed to be installed with
other openstack services in the same virtualenv.

Change-Id: Id64186bf87300f23acde4f90474abcd6944e5be0
(cherry picked from commit acf23fa)
openstack-mirroring pushed a commit to openstack/kolla that referenced this issue Dec 20, 2023
Docker 7.0.0 introduced a pre build check for tag regex, which
fails where registry has port number defined - see [1] and [2].

[1]: docker/docker-py@a9b5494
[2]: docker/docker-py#3195

Also removing requirements check in CI as it is not allowing
such pinning, as kolla is not designed to be installed with
other openstack services in the same virtualenv.

Change-Id: Id64186bf87300f23acde4f90474abcd6944e5be0
(cherry picked from commit acf23fa)
openstack-mirroring pushed a commit to openstack/openstack that referenced this issue Dec 20, 2023
* Update kolla from branch 'master'
  to 8f863beb45ead74d5a7b4c956acad0c7e0f22978
  - CI: Pin docker installation to <7
    
    Docker 7.0.0 introduced a pre build check for tag regex, that is most
    probably the culprit - see [1] and [2].
    
    Depends-On: https://review.opendev.org/c/openstack/kolla-ansible/+/903364
    
    [1]: docker/docker-py@a9b5494
    [2]: docker/docker-py#3195
    
    Change-Id: I147c9715eb922abb6675a2786beaec85971718a8
openstack-mirroring pushed a commit to openstack/kolla that referenced this issue Dec 20, 2023
Docker 7.0.0 introduced a pre build check for tag regex, that is most
probably the culprit - see [1] and [2].

Depends-On: https://review.opendev.org/c/openstack/kolla-ansible/+/903364

[1]: docker/docker-py@a9b5494
[2]: docker/docker-py#3195

Change-Id: I147c9715eb922abb6675a2786beaec85971718a8
milas added a commit that referenced this issue Jan 3, 2024
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>
openstack-mirroring pushed a commit to openstack/kayobe that referenced this issue Mar 4, 2024
The tag regex is buggy and fails if the docker registry contains a port
number[1].

[1] docker/docker-py#3195

Change-Id: I5d85e751b490ab1e39e417ff8797ca8f8688590b
Closes-Bug: #2054715
openstack-mirroring pushed a commit to openstack/openstack that referenced this issue Mar 4, 2024
* Update kayobe from branch 'master'
  to e7e354989ea7768fe958ce105ab9a51017d46155
  - Merge "Skip buggy release of docker PyPI package"
  - Skip buggy release of docker PyPI package
    
    The tag regex is buggy and fails if the docker registry contains a port
    number[1].
    
    [1] docker/docker-py#3195
    
    Change-Id: I5d85e751b490ab1e39e417ff8797ca8f8688590b
    Closes-Bug: #2054715
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants