cli/config/credentials: ConvertToHostname: handle IP-addresses

[thaJeztah]

- What I did

• follow-up to https://github.com/docker/cli/pull/5195
• addresses another regression in https://github.com/docker/cli/pull/3599
• fixes https://github.com/docker/cli/issues/5194

- Description for the changelog

Fix handling of IPv6 addresses with custom ports on docker login

- A picture of a cute animal (not mandatory but encouraged)

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 61.43%. Comparing base (94e9aa6) to head (8b0a7b0).

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #5196 +/- ## ======================================= Coverage 61.43% 61.43% ======================================= Files 298 298 Lines 20799 20799 ======================================= Hits 12777 12777 Misses 7109 7109 Partials 913 913 ```

[vvoland]

Needs a rebase

[thaJeztah]

Rebased; I also removed the cherry-pick label for now, as this was not the previous handling of these, so perhaps OK for the next release.

[vvoland]

The implementation before https://github.com/docker/cli/pull/3599 only removed the http(s):// prefix and took everything until / - so it would still preserve the port in the IPv6 address (http://[::1]:1234/) and not add an extra : if no explicit port was given (http://[::1]/). https://github.com/docker/cli/blob/94e9aa68919428e861cd8659a2d0a13e4b50ba66/vendor/github.com/docker/docker/registry/auth.go#L130-L142

So IMO we should still cherrypick this.

[thaJeztah]

so it would still preserve the port in the IPv6 address (http://[::1]:1234/) and not add an extra : if no explicit port was given (http://[::1]/).

So this function is to normalise hosts, in order to lookup which credentials to use if they're present in ~/.docker/config.json. It's somewhat fine to pick whatever format for these, but we should make sure that we de-duplicate entries to prevent ::1:5000 being considered different than [::1]:5000.

There's some existing weird behavior where we treat index.docker.io separate, and for that we use the URL for lookup (https://index.docker.ioi/v1/) but for any other registry, we use hostname[:port].

AFAIK, the intent was always to store credentials per hostname, which is why URLs are trimmed (i.e. no separate credentials for hostname.example.com/v2/foo and hostname.example.com/v2/bar), but port is preserved, because different ports may be different registries / services, and credentials should not be sent to unrelated ones.

That, hmmm, also brings up the other topic; ambiguous port numbers, because https://example.com and http://example.com are implicitly different ports (443 vs 80), but because we're trimming the scheme, that information gets lost. This also means that example.com:80 and example.com:443 are now considered separate entries.

And to add to the fun, some credential helpers require a scheme in order to store credentials, and because we want to precent credentials that were stored for a TLS host to be sent to a non-TLS host (with the same name), we use https:// to store the credentials

🫠 🫠 🫠 🫠

TL;DR we need to better define how these must be normalised to prevent ambiguity.

[vvoland]

Without this PR, we're still not doing the same thing as before https://github.com/docker/cli/pull/3599.

Consider https://[::1]:5000 as an input:

Before we would correctly return the [::1]:5000, after https://github.com/docker/cli/pull/5195 the result would be ::1:5000.

If that host was stored on pre v27 versions, it would be stored as [::1]:5000. IMO, it's better to keep the stored data consistent with previous versions, just in case there is other code that's reading this state (like Docker Desktop?).

[thaJeztah]

OH!! Now I get it. Sorry, I'm slow. So yes, the url.Parse -> .HostName() would be stripping the square brackets because it's not part of the hostname (only used for host:port). Gotcha 😄

Yes, in that case we should take this one for 27.0.2 as well

[thaJeztah]

Prepared a backport; https://github.com/docker/cli/pull/5198