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

[thaJeztah]

- What I did

• follow-up to re-introduced support for port numbers in docker registry URL #5195
• addresses another regression in use local ConvertToHostname() implementation #3599
• fixesv26.1.4 -> 27.0.1: "docker login https://myregistry:444/" loses port #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 ✅

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

Additional details and impacted files

@@ 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 #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]/).

cli/vendor/github.com/docker/docker/registry/auth.go

Lines 130 to 142 in 94e9aa6

	// ConvertToHostname normalizes a registry URL which has http|https prepended
	// to just its hostname. It is used to match credentials, which may be either
	// stored as hostname or as hostname including scheme (in legacy configuration
	// files).
	funcConvertToHostname(urlstring) string {
	stripped:=url
	ifstrings.HasPrefix(url, "http://") {
	stripped=strings.TrimPrefix(url, "http://")
	} elseifstrings.HasPrefix(url, "https://") {
	stripped=strings.TrimPrefix(url, "https://")
	}
	returnstrings.SplitN(stripped, "/", 2)[0]
	}

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 #3599.

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

Before we would correctly return the [::1]:5000, after #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; #5198