feat: cherry-pick: fix for private registries in test workflows (#5400)

* fix issue with image pull secrets for workflows due to invalid registry append (#5397)

* fix issue with image pull secrets for workflows (#5378)

pkg/imageinspector/skopeofetcher.go

@@ -6,6 +6,8 @@ import (
 	"strings"
 	"time"
 
+	"github.com/kubeshop/testkube/pkg/utils"
+
 	corev1 "k8s.io/api/core/v1"
 
 	"github.com/kubeshop/testkube/pkg/skopeo"
@@ -19,6 +21,10 @@ func NewSkopeoFetcher() InfoFetcher {
 }
 
 func (s *skopeoFetcher) Fetch(ctx context.Context, registry, image string, pullSecrets []corev1.Secret) (*Info, error) {
+	// If registry is not provided, extract it from the image name
+	if registry == "" {
+		registry = extractRegistry(image)
+	}
 	client, err := skopeo.NewClientFromSecrets(pullSecrets, registry)
 	if err != nil {
 		return nil, err
@@ -39,6 +45,21 @@ func (s *skopeoFetcher) Fetch(ctx context.Context, registry, image string, pullS
 	}, nil
 }
 
+// extractRegistry takes a container image string and returns the registry part.
+// It defaults to "docker.io" if no registry is specified.
+func extractRegistry(image string) string {
+	parts := strings.Split(image, "/")
+	// If the image is just a name, return the default registry.
+	if len(parts) == 1 {
+		return utils.DefaultDockerRegistry
+	}
+	// If the first part contains '.' or ':', it's likely a registry.
+	if strings.Contains(parts[0], ".") || strings.Contains(parts[0], ":") {
+		return parts[0]
+	}
+	return utils.DefaultDockerRegistry
+}
+
 func determineUserGroupPair(userGroupStr string) (int64, int64) {
 	if userGroupStr == "" {
 		userGroupStr = "0"

pkg/imageinspector/skopeofetcher_test.go

@@ -0,0 +1,35 @@
+package imageinspector
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+// TestExtractRegistry uses table-driven tests to validate the extractRegistry function.
+func TestExtractRegistry(t *testing.T) {
+	tests := []struct {
+		name     string
+		image    string
+		expected string
+	}{
+		{"DockerHub short", "nginx:latest", "https://index.docker.io/v1/"},
+		{"DockerHub long", "library/nginx:latest", "https://index.docker.io/v1/"},
+		{"GCR", "gcr.io/google-containers/busybox:latest", "gcr.io"},
+		{"ECR", "123456789012.dkr.ecr.us-east-1.amazonaws.com/my-application:latest", "123456789012.dkr.ecr.us-east-1.amazonaws.com"},
+		{"MCR", "mcr.microsoft.com/dotnet/core/sdk:3.1", "mcr.microsoft.com"},
+		{"Quay", "quay.io/bitnami/nginx:latest", "quay.io"},
+		{"Custom port", "localhost:5000/myimage:latest", "localhost:5000"},
+		{"No tag", "myregistry.com/myimage", "myregistry.com"},
+		{"Only image", "myimage", "https://index.docker.io/v1/"},
+		{"Custom GitLab", "registry.gitlab.com/company/base-docker-images/ubuntu-python-base-image:3.12.0-jammy", "registry.gitlab.com"},
+	}
+
+	for _, tc := range tests {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			got := extractRegistry(tc.image)
+			assert.Equal(t, tc.expected, got)
+		})
+	}
+}

pkg/skopeo/client.go

@@ -10,6 +10,8 @@ import (
 	"strings"
 	"time"
 
+	"github.com/kubeshop/testkube/pkg/utils"
+
 	corev1 "k8s.io/api/core/v1"
 
 	"github.com/kubeshop/testkube/pkg/process"
@@ -86,14 +88,16 @@ func (c *client) Inspect(registry, image string) (*DockerImage, error) {
 	}
 
 	if len(c.dockerAuthConfigs) != 0 {
+		// TODO: Is it a good idea to randomly select a secret?
 		i := rand.Intn(len(c.dockerAuthConfigs))
 		args = append(args, "--creds", c.dockerAuthConfigs[i].Username+":"+c.dockerAuthConfigs[i].Password)
 	}
 
-	config := "docker://" + image
-	if registry != "" {
-		config = registry + "/" + image
+	// If registry is provided via config and the image does not start with the registry, prepend it
+	if registry != "" && registry != utils.DefaultDockerRegistry && !strings.HasPrefix(image, registry) {
+		image = registry + "/" + image
 	}
+	config := "docker://" + image
 
 	args = append(args, "--config", config)
 	result, err := process.Execute("skopeo", args...)
@@ -157,10 +161,6 @@ func ParseSecretData(imageSecrets []corev1.Secret, registry string) ([]DockerAut
 			return nil, fmt.Errorf("imagePullSecret %s contains neither .dockercfg nor .dockerconfigjson", imageSecret.Name)
 		}
 
-		// If registry is not provided, extract it from the image name
-		if registry == "" {
-			registry = extractRegistry(imageSecret.Name)
-		}
 		// Determine if there is a secret for the specified registry
 		if creds, ok := auths.Auths[registry]; ok {
 			username, password, err := extractRegistryCredentials(creds)
@@ -169,30 +169,12 @@ func ParseSecretData(imageSecrets []corev1.Secret, registry string) ([]DockerAut
 			}
 
 			results = append(results, DockerAuthConfig{Username: username, Password: password})
-		} else {
-			return nil, fmt.Errorf("secret %s is not defined for registry: %s", imageSecret.Name, registry)
 		}
 	}
 
 	return results, nil
 }
 
-// extractRegistry takes a container image string and returns the registry part.
-// It defaults to "docker.io" if no registry is specified.
-func extractRegistry(image string) string {
-	defaultRegistry := "https://index.docker.io/v1/"
-	parts := strings.Split(image, "/")
-	// If the image is just a name, return the default registry.
-	if len(parts) == 1 {
-		return defaultRegistry
-	}
-	// If the first part contains '.' or ':', it's likely a registry.
-	if strings.Contains(parts[0], ".") || strings.Contains(parts[0], ":") {
-		return parts[0]
-	}
-	return defaultRegistry
-}
-
 func extractRegistryCredentials(creds DockerAuthConfig) (username, password string, err error) {
 	if creds.Auth == "" {
 		return creds.Username, creds.Password, nil

pkg/skopeo/client_test.go

@@ -56,23 +56,6 @@ func TestParseSecretData(t *testing.T) {
 		assert.NoError(t, err)
 	})
 
-	t.Run("parse docker config missed registry", func(t *testing.T) {
-		t.Parallel()
-
-		secret := corev1.Secret{
-			ObjectMeta: metav1.ObjectMeta{
-				Name: "dockercfg",
-			},
-
-			Data: map[string][]byte{".dockerconfigjson": []byte("{\"auths\": {\"https://index.docker.io/v1/\": {\"username\": \"plainuser\", \"password\": \"plainpass\"}}}")},
-		}
-
-		out, err := ParseSecretData([]corev1.Secret{secret}, "fake")
-
-		assert.Nil(t, out)
-		assert.EqualError(t, err, "secret dockercfg is not defined for registry: fake")
-	})
-
 	t.Run("parse docker config missed data", func(t *testing.T) {
 		t.Parallel()
 
@@ -103,33 +86,6 @@ func TestParseSecretData(t *testing.T) {
 
 }
 
-// TestExtractRegistry uses table-driven tests to validate the extractRegistry function.
-func TestExtractRegistry(t *testing.T) {
-	tests := []struct {
-		name     string
-		image    string
-		expected string
-	}{
-		{"DockerHub short", "nginx:latest", "https://index.docker.io/v1/"},
-		{"DockerHub long", "library/nginx:latest", "https://index.docker.io/v1/"},
-		{"GCR", "gcr.io/google-containers/busybox:latest", "gcr.io"},
-		{"ECR", "123456789012.dkr.ecr.us-east-1.amazonaws.com/my-application:latest", "123456789012.dkr.ecr.us-east-1.amazonaws.com"},
-		{"MCR", "mcr.microsoft.com/dotnet/core/sdk:3.1", "mcr.microsoft.com"},
-		{"Quay", "quay.io/bitnami/nginx:latest", "quay.io"},
-		{"Custom port", "localhost:5000/myimage:latest", "localhost:5000"},
-		{"No tag", "myregistry.com/myimage", "myregistry.com"},
-		{"Only image", "myimage", "https://index.docker.io/v1/"},
-	}
-
-	for _, tc := range tests {
-		tc := tc
-		t.Run(tc.name, func(t *testing.T) {
-			got := extractRegistry(tc.image)
-			assert.Equal(t, tc.expected, got)
-		})
-	}
-}
-
 // TestTrimTopNonJSON tests the trimNonJSON function with various inputs to ensure it correctly trims non-JSON leading characters.
 func TestTrimTopNonJSON(t *testing.T) {
 	tests := []struct {

pkg/utils/consts.go

@@ -0,0 +1,6 @@
+package utils
+
+const (
+	// DefaultDockerRegistry is the default registry used when no registry is specified in the image name.
+	DefaultDockerRegistry = "https://index.docker.io/v1/"
+)