-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
fix: tag regex should allow ports #3196
Conversation
Closes: docker#3195 Related: opencontainers/distribution-spec#498 Signed-off-by: Sven Kieske <[email protected]>
further notice: I did fix the ruff warning about |
Make sure to add a simple test like: def test_build_with_ports_on_tag(self):
self.client.build(".", tag="registry.example.com:443/my-img:latest") on r"my-regex" instead of "my-regex" |
There are some xfails here for cases that the regex is not currently handling. It's too strict for IPv6 domains at the moment. Signed-off-by: Milas Bowman <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the bugfix!
I addressed the lint issue (it was valid but very subtle btw: needed to use r""
strings each time for the concatenation 😵) and added some test cases from the distribution/reference
codebase.
(There's some xfails on the IPv6 cases which can be addressed separately.)
When can we expect a release including this fix ? |
Thanks @milas for taking over (I was first on PTO, then this issue was drowned in my inbox due to other issues)! a new release would be highly appreciated for our downstream usage! |
Closes: #3195
Related: opencontainers/distribution-spec#498
notice: there is a false positive warning from ruff during
make test
:but it's a valid escape sequence, because it's a regex, had no idea yet how to fix this without introducing more complexity.