add more cases to handle the auth config | refactor error handling

Signed-off-by: mabulgu <mabulgu@gmail.com>

Author: mabulgu

pkg/secretutils/dockerconfig.go

@@ -3,6 +3,7 @@ package secretutils
 import (
 	"encoding/base64"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"net/url"
 	"strings"
@@ -32,7 +33,7 @@ type DockerConfig map[string]DockerAuthConfig
 // Returns the credentials in base64-encoded "username:password" format as expected by Ironic.
 func ExtractRegistryCredentials(secret *corev1.Secret, imageURL string) (string, error) {
 	if secret == nil {
-		return "", fmt.Errorf("secret is nil")
+		return "", errors.New("secret is nil")
 	}
 
 	registryHost, err := extractRegistryHost(imageURL)
@@ -74,7 +75,7 @@ func ExtractRegistryCredentials(secret *corev1.Secret, imageURL string) (string,
 }
 
 // extractRegistryHost extracts the registry hostname from an OCI image URL.
-// For example, "oci://registry.example.com/repo/image:tag" returns "registry.example.com"
+// For example, "oci://registry.example.com/repo/image:tag" returns "registry.example.com".
 func extractRegistryHost(imageURL string) (string, error) {
 	if !strings.HasPrefix(imageURL, "oci://") {
 		return "", fmt.Errorf("image URL does not have oci:// scheme: %s", imageURL)
@@ -127,8 +128,9 @@ func parseDockerConfig(data []byte, registryHost string) (*DockerAuthConfig, err
 
 // findAuthConfig searches for the auth config matching the registry host.
 // It tries several variations of the registry host to handle different formats.
+// Returns a clear RegistryEntryMissing error when no entry matches.
 func findAuthConfig(auths map[string]DockerAuthConfig, registryHost string) (*DockerAuthConfig, error) {
-	// Try exact match first
+	// Try exact match first (handles both "host" and "host:port")
 	if authConfig, ok := auths[registryHost]; ok {
 		return &authConfig, nil
 	}
@@ -153,20 +155,36 @@ func findAuthConfig(auths map[string]DockerAuthConfig, registryHost string) (*Do
 		return &authConfig, nil
 	}
 
-	// For Docker Hub, try common variations
-	if strings.Contains(registryHost, "docker.io") || registryHost == "docker.io" {
-		for _, key := range []string{
+	// Try with https:// prefix and /v1/ suffix
+	if authConfig, ok := auths["https://"+registryHost+"/v1/"]; ok {
+		return &authConfig, nil
+	}
+
+	// Try with https:// prefix and /v2/ suffix
+	if authConfig, ok := auths["https://"+registryHost+"/v2/"]; ok {
+		return &authConfig, nil
+	}
+
+	// Special handling for Docker Hub
+	// Docker Hub can appear as: docker.io, index.docker.io, https://index.docker.io/v1/, etc.
+	if registryHost == "docker.io" || registryHost == "index.docker.io" ||
+		strings.HasPrefix(registryHost, "docker.io:") || strings.HasPrefix(registryHost, "index.docker.io:") {
+		dockerHubKeys := []string{
 			"https://index.docker.io/v1/",
 			"index.docker.io",
 			"docker.io",
 			"https://docker.io",
-		} {
+			"https://index.docker.io",
+			registryHost, // Already tried but keep for clarity
+		}
+		for _, key := range dockerHubKeys {
 			if authConfig, ok := auths[key]; ok {
 				return &authConfig, nil
 			}
 		}
 	}
 
+	// Return clear error when no entry matches
 	return nil, fmt.Errorf("registry %s not found in auth config", registryHost)
 }
 
@@ -185,14 +203,14 @@ func extractCredentials(authConfig *DockerAuthConfig) (username, password string
 			return "", "", fmt.Errorf("failed to decode auth field: %w", err)
 		}
 
-		parts := strings.SplitN(string(decoded), ":", 2)
-		if len(parts) != 2 {
-			return "", "", fmt.Errorf("invalid auth format: expected username:password")
+		const credentialParts = 2
+		parts := strings.SplitN(string(decoded), ":", credentialParts)
+		if len(parts) != credentialParts {
+			return "", "", errors.New("invalid auth format: expected username:password")
 		}
 
 		return parts[0], parts[1], nil
 	}
 
-	return "", "", fmt.Errorf("no credentials found in auth config")
+	return "", "", errors.New("no credentials found in auth config")
 }
-

pkg/secretutils/dockerconfig_test.go

@@ -75,11 +75,11 @@ func TestExtractRegistryHost(t *testing.T) {
 
 func TestExtractCredentials(t *testing.T) {
 	tests := []struct {
-		name           string
-		authConfig     *DockerAuthConfig
-		expectedUser   string
-		expectedPass   string
-		expectError    bool
+		name         string
+		authConfig   *DockerAuthConfig
+		expectedUser string
+		expectedPass string
+		expectError  bool
 	}{
 		{
 			name: "explicit username and password",
@@ -327,7 +327,10 @@ func TestExtractRegistryCredentials(t *testing.T) {
 				},
 			},
 		}
-		data, _ := json.Marshal(config)
+		data, err := json.Marshal(config)
+		if err != nil {
+			t.Fatalf("failed to marshal config: %v", err)
+		}
 		return data
 	}
 
@@ -339,16 +342,19 @@ func TestExtractRegistryCredentials(t *testing.T) {
 				Password: password,
 			},
 		}
-		data, _ := json.Marshal(config)
+		data, err := json.Marshal(config)
+		if err != nil {
+			t.Fatalf("failed to marshal config: %v", err)
+		}
 		return data
 	}
 
 	tests := []struct {
-		name             string
-		secret           *corev1.Secret
-		imageURL         string
-		expectError      bool
-		validateResult   bool
+		name           string
+		secret         *corev1.Secret
+		imageURL       string
+		expectError    bool
+		validateResult bool
 	}{
 		{
 			name: "dockerconfigjson secret with exact match",
@@ -508,6 +514,10 @@ func TestFindAuthConfig(t *testing.T) {
 			Username: "user4",
 			Password: "pass4",
 		},
+		"registry.local:5000": {
+			Username: "user5",
+			Password: "pass5",
+		},
 	}
 
 	tests := []struct {
@@ -535,16 +545,33 @@ func TestFindAuthConfig(t *testing.T) {
 			expectedUser: "user3",
 		},
 		{
-			name:         "Docker Hub special case",
+			name:         "Docker Hub special case - docker.io",
 			registryHost: "docker.io",
 			expectFound:  true,
 			expectedUser: "user4",
 		},
 		{
-			name:         "not found",
+			name:         "Docker Hub special case - index.docker.io",
+			registryHost: "index.docker.io",
+			expectFound:  true,
+			expectedUser: "user4",
+		},
+		{
+			name:         "registry with port - exact match",
+			registryHost: "registry.local:5000",
+			expectFound:  true,
+			expectedUser: "user5",
+		},
+		{
+			name:         "missing registry entry",
 			registryHost: "notfound.registry.com",
 			expectFound:  false,
 		},
+		{
+			name:         "missing registry with port",
+			registryHost: "notfound.registry.com:8080",
+			expectFound:  false,
+		},
 	}
 
 	for _, tt := range tests {
@@ -554,6 +581,9 @@ func TestFindAuthConfig(t *testing.T) {
 				if err == nil {
 					t.Errorf("expected error but got none")
 				}
+				if !strings.Contains(err.Error(), "not found in auth config") {
+					t.Errorf("expected 'not found in auth config' error, got: %v", err)
+				}
 				return
 			}
 			if err != nil {
@@ -571,3 +601,129 @@ func TestFindAuthConfig(t *testing.T) {
 	}
 }
 
+// TestFindAuthConfigCornerCases tests specific corner cases for registry matching.
+func TestFindAuthConfigCornerCases(t *testing.T) {
+	tests := []struct {
+		name         string
+		auths        map[string]DockerAuthConfig
+		registryHost string
+		expectFound  bool
+		expectedUser string
+	}{
+		{
+			name: "registry with port in both",
+			auths: map[string]DockerAuthConfig{
+				"registry.example.com:5000": {Username: "portuser", Password: "portpass"},
+			},
+			registryHost: "registry.example.com:5000",
+			expectFound:  true,
+			expectedUser: "portuser",
+		},
+		{
+			name: "registry with http prefix in config",
+			auths: map[string]DockerAuthConfig{
+				"http://registry.local": {Username: "httpuser", Password: "httppass"},
+			},
+			registryHost: "registry.local",
+			expectFound:  true,
+			expectedUser: "httpuser",
+		},
+		{
+			name: "registry with https and /v1/ suffix",
+			auths: map[string]DockerAuthConfig{
+				"https://registry.example.com/v1/": {Username: "v1user", Password: "v1pass"},
+			},
+			registryHost: "registry.example.com",
+			expectFound:  true,
+			expectedUser: "v1user",
+		},
+		{
+			name: "quay.io exact match",
+			auths: map[string]DockerAuthConfig{
+				"quay.io": {Username: "quayuser", Password: "quaypass"},
+			},
+			registryHost: "quay.io",
+			expectFound:  true,
+			expectedUser: "quayuser",
+		},
+		{
+			name: "quay.io with https",
+			auths: map[string]DockerAuthConfig{
+				"https://quay.io": {Username: "quayuser", Password: "quaypass"},
+			},
+			registryHost: "quay.io",
+			expectFound:  true,
+			expectedUser: "quayuser",
+		},
+		{
+			name: "gcr.io exact match",
+			auths: map[string]DockerAuthConfig{
+				"gcr.io": {Username: "gcruser", Password: "gcrpass"},
+			},
+			registryHost: "gcr.io",
+			expectFound:  true,
+			expectedUser: "gcruser",
+		},
+		{
+			name: "Docker Hub - index.docker.io legacy",
+			auths: map[string]DockerAuthConfig{
+				"https://index.docker.io/v1/": {Username: "dockeruser", Password: "dockerpass"},
+			},
+			registryHost: "docker.io",
+			expectFound:  true,
+			expectedUser: "dockeruser",
+		},
+		{
+			name: "Docker Hub - docker.io in config",
+			auths: map[string]DockerAuthConfig{
+				"docker.io": {Username: "dockeruser", Password: "dockerpass"},
+			},
+			registryHost: "index.docker.io",
+			expectFound:  true,
+			expectedUser: "dockeruser",
+		},
+		{
+			name: "custom registry with port",
+			auths: map[string]DockerAuthConfig{
+				"https://registry.local:8443": {Username: "customuser", Password: "custompass"},
+			},
+			registryHost: "registry.local:8443",
+			expectFound:  true,
+			expectedUser: "customuser",
+		},
+		{
+			name: "registry not in config - clear error",
+			auths: map[string]DockerAuthConfig{
+				"other.registry.com": {Username: "otheruser", Password: "otherpass"},
+			},
+			registryHost: "missing.registry.com",
+			expectFound:  false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			authConfig, err := findAuthConfig(tt.auths, tt.registryHost)
+			if !tt.expectFound {
+				if err == nil {
+					t.Errorf("expected RegistryEntryMissing error but got none")
+				}
+				if !strings.Contains(err.Error(), "not found in auth config") {
+					t.Errorf("expected clear 'not found in auth config' error, got: %v", err)
+				}
+				return
+			}
+			if err != nil {
+				t.Errorf("unexpected error: %v", err)
+				return
+			}
+			if authConfig == nil {
+				t.Errorf("expected auth config but got nil")
+				return
+			}
+			if authConfig.Username != tt.expectedUser {
+				t.Errorf("expected username %q, got %q", tt.expectedUser, authConfig.Username)
+			}
+		})
+	}
+}