fix: erase the docker hub credential on registry:logout

Docker stores Docker Hub under the index server address rather than the hostname the user logged in with, and `docker logout` only erases the exact key it is given, so `registry:logout docker.io` reported success while leaving the credential in place. The server is now resolved to the key docker actually stores before the logout runs.

plugins/registry/functions.go

@@ -15,6 +15,9 @@ import (
 
 const registryConfigDir = "/var/lib/dokku/config/registry"
 
+// dockerIndexServer is the auths key docker stores Docker Hub credentials under
+const dockerIndexServer = "https://index.docker.io/v1/"
+
 // GetAppRegistryConfigDir returns the per-app registry config directory
 func GetAppRegistryConfigDir(appName string) string {
 	return filepath.Join(registryConfigDir, appName)
@@ -30,9 +33,19 @@ func GetComputedAppRegistryConfigDir(appName string) string {
 		return GetAppRegistryConfigDir(appName)
 	}
 
+	return GetGlobalRegistryConfigDir()
+}
+
+// GetGlobalRegistryConfigDir returns the docker config directory global logins write to
+func GetGlobalRegistryConfigDir() string {
 	return filepath.Join(os.Getenv("DOKKU_ROOT"), ".docker")
 }
 
+// GetGlobalRegistryConfigPath returns the path to the global docker config.json
+func GetGlobalRegistryConfigPath() string {
+	return filepath.Join(GetGlobalRegistryConfigDir(), "config.json")
+}
+
 // HasAppRegistryAuth checks if an app has registry credentials configured
 func HasAppRegistryAuth(appName string) bool {
 	configPath := GetAppRegistryConfigPath(appName)
@@ -245,3 +258,38 @@ func imageCleanup(appName string, imageRepo string, imageTag string, tag int) {
 	imagesToRemove = append(imagesToRemove, imageIDs...)
 	common.RemoveImages(imagesToRemove)
 }
+
+// normalizeRegistryServer applies the dokku-level aliases for a registry server
+func normalizeRegistryServer(server string) string {
+	if server == "hub.docker.com" || server == "docker.com" {
+		return "docker.io"
+	}
+
+	return server
+}
+
+// convertToHostname strips any scheme and path from a registry server, matching
+// the rule docker applies before storing or looking up a credential
+func convertToHostname(server string) string {
+	stripped := server
+	if strings.HasPrefix(stripped, "http://") {
+		stripped = strings.TrimPrefix(stripped, "http://")
+	} else if strings.HasPrefix(stripped, "https://") {
+		stripped = strings.TrimPrefix(stripped, "https://")
+	}
+
+	return strings.SplitN(stripped, "/", 2)[0]
+}
+
+// dockerAuthKey returns the key docker uses for a registry server in the auths
+// map of a config.json. Docker Hub is stored under the index server address
+// rather than the hostname the user logged in with, and is also the only server
+// docker treats that way: registry-1.docker.io gets a key of its own.
+func dockerAuthKey(server string) string {
+	hostname := convertToHostname(normalizeRegistryServer(server))
+	if hostname == "" || hostname == "docker.io" || hostname == "index.docker.io" {
+		return dockerIndexServer
+	}
+
+	return hostname
+}

plugins/registry/functions_test.go

@@ -0,0 +1,79 @@
+package registry
+
+import "testing"
+
+func TestNormalizeRegistryServer(t *testing.T) {
+	cases := []struct {
+		name string
+		in   string
+		want string
+	}{
+		{"hub.docker.com alias", "hub.docker.com", "docker.io"},
+		{"docker.com alias", "docker.com", "docker.io"},
+		{"docker.io untouched", "docker.io", "docker.io"},
+		{"other registry untouched", "ghcr.io", "ghcr.io"},
+		{"empty untouched", "", ""},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			if got := normalizeRegistryServer(tc.in); got != tc.want {
+				t.Errorf("normalizeRegistryServer(%q) = %q, want %q", tc.in, got, tc.want)
+			}
+		})
+	}
+}
+
+func TestConvertToHostname(t *testing.T) {
+	cases := []struct {
+		name string
+		in   string
+		want string
+	}{
+		{"bare hostname", "ghcr.io", "ghcr.io"},
+		{"https scheme", "https://ghcr.io", "ghcr.io"},
+		{"http scheme", "http://ghcr.io", "ghcr.io"},
+		{"scheme and path", "https://index.docker.io/v1/", "index.docker.io"},
+		{"trailing slash", "ghcr.io/", "ghcr.io"},
+		{"port preserved", "localhost:5000", "localhost:5000"},
+		{"port with scheme and path", "https://localhost:5000/v2/", "localhost:5000"},
+		{"empty", "", ""},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			if got := convertToHostname(tc.in); got != tc.want {
+				t.Errorf("convertToHostname(%q) = %q, want %q", tc.in, got, tc.want)
+			}
+		})
+	}
+}
+
+func TestDockerAuthKey(t *testing.T) {
+	cases := []struct {
+		name string
+		in   string
+		want string
+	}{
+		{"docker.io", "docker.io", dockerIndexServer},
+		{"hub.docker.com", "hub.docker.com", dockerIndexServer},
+		{"docker.com", "docker.com", dockerIndexServer},
+		{"index.docker.io", "index.docker.io", dockerIndexServer},
+		{"index server address", dockerIndexServer, dockerIndexServer},
+		{"empty server", "", dockerIndexServer},
+		// docker does not treat registry-1.docker.io as the official index and
+		// stores it under a key of its own
+		{"registry-1.docker.io is not the index", "registry-1.docker.io", "registry-1.docker.io"},
+		{"other registry", "ghcr.io", "ghcr.io"},
+		{"other registry with scheme", "https://ghcr.io/v2/", "ghcr.io"},
+		{"registry with port", "localhost:5000", "localhost:5000"},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			if got := dockerAuthKey(tc.in); got != tc.want {
+				t.Errorf("dockerAuthKey(%q) = %q, want %q", tc.in, got, tc.want)
+			}
+		})
+	}
+}

plugins/registry/subcommands.go

@@ -32,9 +32,7 @@ func CommandLogin(appName string, server string, username string, password strin
 		return errors.New("Missing password argument")
 	}
 
-	if server == "hub.docker.com" || server == "docker.com" {
-		server = "docker.io"
-	}
+	server = normalizeRegistryServer(server)
 
 	buffer := bytes.Buffer{}
 	buffer.Write([]byte(password + "\n"))
@@ -92,9 +90,10 @@ func CommandLogout(appName string, server string) error {
 		return errors.New("Missing server argument")
 	}
 
-	if server == "hub.docker.com" || server == "docker.com" {
-		server = "docker.io"
-	}
+	// docker only erases the exact auths key it is given, and stores Docker Hub
+	// under the index server address rather than the hostname logged in with, so
+	// a logout for docker.io has to name that address to remove anything
+	server = dockerAuthKey(server)
 
 	env := map[string]string{}
 	if appName != "" {

tests.mk

@@ -186,6 +186,7 @@ go-tests:
 	@$(MAKE) go-test-plugin PLUGIN_NAME=logs
 	@$(MAKE) go-test-plugin PLUGIN_NAME=network
 	@$(MAKE) go-test-plugin PLUGIN_NAME=ps
+	@$(MAKE) go-test-plugin PLUGIN_NAME=registry
 	@$(MAKE) go-test-plugin PLUGIN_NAME=buildpacks
 	@$(MAKE) go-test-plugin PLUGIN_NAME=scheduler-k3s
 	@$(MAKE) go-test-plugin PLUGIN_NAME=storage

tests/unit/registry.bats

@@ -90,6 +90,12 @@ teardown() {
   echo "output: $output"
   echo "status: $status"
   assert_success
+
+  run /bin/bash -c "jq -r '.auths | keys | join(\",\")' /var/lib/dokku/config/registry/$TEST_APP/config.json"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+  assert_output_not_contains "index.docker.io"
 }
 
 @test "(registry:logout) global logout" {
@@ -106,6 +112,12 @@ teardown() {
   echo "output: $output"
   echo "status: $status"
   assert_success
+
+  run /bin/bash -c "jq -r '.auths | keys | join(\",\")' $DOKKU_ROOT/.docker/config.json"
+  echo "output: $output"
+  echo "status: $status"
+  assert_success
+  assert_output_not_contains "index.docker.io"
 }
 
 @test "(registry) per-app credentials deleted on app destroy" {