refactor splitDockerDomain to include more documentation

The splitDockerDomain attempts to determine whether the given name
contains a domain, or should be used as a "remote-name". The logic used
in doing so is based on (legacy) conventions, which have not always been
properly documented.

The logic used in this function was also optimized for "brevity", but
not easy to ready, due to the combination of multiple boolean conditions
combined on a single line, and some "double negatives".

More documentation may still be needed, but let's start with documenting
the logic used in this function;

- Use `strings.Cut()` instead of  `strings.IndexRune()`, which allows us to
  use descriptive variable names, and prevents use of the magic `-1` value.
- Split the conditions into a switch, so that each of them can be documented
  separately. While this makes the  code more verbose (and introduces some
  duplication), it should not impact performance, as only one condition
  would ever be met (performance may even be better, as the old code
  combined multiple conditions with `&&`).
- Introduce a fast-path for single-element ("familiar") names. These names
  can be canonicalized early, without doing further handling.

While working on the code, I also discovered an existing bug (or omission)
where the code would not handle bare _domain names_. Ironically, the
TestParseDockerRef test has a test-case name "hostname only", but which does
not cover that case. THat test-case was transferred from containerd/cri,
and does not describe this scenario (possibly was left as a "further exercise");
containerd/cri@25fdf72

Let keep it as a further exercise, but add a "TODO" to remind us doing so.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

normalize.go

@@ -123,20 +123,51 @@ func ParseDockerRef(ref string) (Named, error) {
 // splitDockerDomain splits a repository name to domain and remote-name.
 // If no valid domain is found, the default domain is used. Repository name
 // needs to be already validated before.
-func splitDockerDomain(name string) (domain, remainder string) {
-	i := strings.IndexRune(name, '/')
-	if i == -1 || (!strings.ContainsAny(name[:i], ".:") && name[:i] != localhost && strings.ToLower(name[:i]) == name[:i]) {
-		domain, remainder = defaultDomain, name
-	} else {
-		domain, remainder = name[:i], name[i+1:]
-	}
-	if domain == legacyDefaultDomain {
-		domain = defaultDomain
-	}
-	if domain == defaultDomain && !strings.ContainsRune(remainder, '/') {
-		remainder = officialRepoPrefix + remainder
-	}
-	return
+func splitDockerDomain(name string) (domain, remoteName string) {
+	maybeDomain, maybeRemoteName, ok := strings.Cut(name, "/")
+	if !ok {
+		// Fast-path for single element ("familiar" names), such as "ubuntu"
+		// or "ubuntu:latest". Familiar names must be handled separately, to
+		// prevent them from being handled as "hostname:port".
+		//
+		// Canonicalize them as "docker.io/library/name[:tag]"
+
+		// FIXME(thaJeztah): account for bare "localhost" or "example.com" names, which SHOULD be considered a domain.
+		return defaultDomain, officialRepoPrefix + name
+	}
+
+	switch {
+	case maybeDomain == localhost:
+		// localhost is a reserved namespace and always considered a domain.
+		domain, remoteName = maybeDomain, maybeRemoteName
+	case maybeDomain == legacyDefaultDomain:
+		// canonicalize the Docker Hub and legacy "Docker Index" domains.
+		domain, remoteName = defaultDomain, maybeRemoteName
+	case strings.ContainsAny(maybeDomain, ".:"):
+		// Likely a domain or IP-address:
+		//
+		// - contains a "." (e.g., "example.com" or "127.0.0.1")
+		// - contains a ":" (e.g., "example:5000", "::1", or "[::1]:5000")
+		domain, remoteName = maybeDomain, maybeRemoteName
+	case strings.ToLower(maybeDomain) != maybeDomain:
+		// Uppercase namespaces are not allowed, so if the first element
+		// is not lowercase, we assume it to be a domain-name.
+		domain, remoteName = maybeDomain, maybeRemoteName
+	default:
+		// None of the above: it's not a domain, so use the default, and
+		// use the name input the remote-name.
+		domain, remoteName = defaultDomain, name
+	}
+
+	if domain == defaultDomain && !strings.ContainsRune(remoteName, '/') {
+		// Canonicalize "familiar" names, but only on Docker Hub, not
+		// on other domains:
+		//
+		// "docker.io/ubuntu[:tag]" => "docker.io/library/ubuntu[:tag]"
+		remoteName = officialRepoPrefix + remoteName
+	}
+
+	return domain, remoteName
 }
 
 // familiarizeName returns a shortened version of the name familiar

normalize_test.go

@@ -13,6 +13,10 @@ func TestValidateReferenceName(t *testing.T) {
 		"docker/docker",
 		"library/debian",
 		"debian",
+		"localhost/library/debian",
+		"localhost/debian",
+		"LOCALDOMAIN/library/debian",
+		"LOCALDOMAIN/debian",
 		"docker.io/docker/docker",
 		"docker.io/library/debian",
 		"docker.io/debian",
@@ -147,6 +151,34 @@ func TestParseRepositoryInfo(t *testing.T) {
 	}
 
 	tests := []tcase{
+		{
+			RemoteName:    "fooo",
+			FamiliarName:  "localhost/fooo",
+			FullName:      "localhost/fooo",
+			AmbiguousName: "localhost/fooo",
+			Domain:        "localhost",
+		},
+		{
+			RemoteName:    "fooo/bar",
+			FamiliarName:  "localhost/fooo/bar",
+			FullName:      "localhost/fooo/bar",
+			AmbiguousName: "localhost/fooo/bar",
+			Domain:        "localhost",
+		},
+		{
+			RemoteName:    "fooo",
+			FamiliarName:  "LOCALDOMAIN/fooo",
+			FullName:      "LOCALDOMAIN/fooo",
+			AmbiguousName: "LOCALDOMAIN/fooo",
+			Domain:        "LOCALDOMAIN",
+		},
+		{
+			RemoteName:    "fooo/bar",
+			FamiliarName:  "LOCALDOMAIN/fooo/bar",
+			FullName:      "LOCALDOMAIN/fooo/bar",
+			AmbiguousName: "LOCALDOMAIN/fooo/bar",
+			Domain:        "LOCALDOMAIN",
+		},
 		{
 			RemoteName:    "fooo/bar",
 			FamiliarName:  "fooo/bar",