Allow non canonical image paths

Signed-off-by: alexander-demicev <[email protected]>

Author: alexander-demicev

internal/controller/image_overrides.go

@@ -18,6 +18,7 @@ package controller
 
 import (
 	"fmt"
+	"strings"
 
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/client-go/kubernetes/scheme"
@@ -39,13 +40,41 @@ func imageOverrides(component string, overrides configclient.Client) func(objs [
 		}
 
 		return fixImages(objs, func(image string) (string, error) {
-			return overrides.ImageMeta().AlterImage(component, image)
+			return alterImage(component, image, overrides.ImageMeta())
 		})
 	}
 
 	return imageOverridesWrapper
 }
 
+// alterImage accepts images as is, including non canonical formats.
+// If image overrides fail due to non canonical format, the original image is returned unchanged.
+// Allowing non canonical formats is designed for advanced users who may want to use such formats intentionally.
+func alterImage(component, imageString string, imageMeta configclient.ImageMetaClient) (string, error) {
+	result, err := imageMeta.AlterImage(component, imageString)
+	if err != nil {
+		if isCanonicalError(err) {
+			return imageString, nil
+		}
+
+		return "", err
+	}
+
+	return result, nil
+}
+
+// isCanonicalError checks if error is about non nanonical image format.
+func isCanonicalError(err error) bool {
+	if err == nil {
+		return false
+	}
+
+	msg := err.Error()
+
+	return strings.Contains(msg, "repository name must be canonical") ||
+		strings.Contains(msg, "couldn't parse image name")
+}
+
 // fixImages alters images using the give alter func
 // NB. The implemented approach is specific for the provider components YAML & for the cert-manager manifest; it is not
 // intended to cover all the possible objects used to deploy containers existing in Kubernetes.

internal/controller/image_overrides_test.go

@@ -169,3 +169,112 @@ func TestFixImages(t *testing.T) {
 		})
 	}
 }
+
+// mockImageMetaClient is a test double for configclient.ImageMetaClient.
+type mockImageMetaClient struct {
+	alterFunc func(component, image string) (string, error)
+}
+
+func (m *mockImageMetaClient) AlterImage(component, image string) (string, error) {
+	return m.alterFunc(component, image)
+}
+
+func TestAlterImage(t *testing.T) {
+	tests := []struct {
+		name      string
+		component string
+		image     string
+		mockFunc  func(component, image string) (string, error)
+		want      string
+		wantErr   bool
+	}{
+		{
+			name:      "canonical image with override applies override",
+			component: "cluster-api",
+			image:     "example.com/controller:v1.0.0",
+			mockFunc: func(component, image string) (string, error) {
+				return "example.com/custom:v2.0.0", nil
+			},
+			want:    "example.com/custom:v2.0.0",
+			wantErr: false,
+		},
+		{
+			name:      "non-canonical image returns original on canonical error",
+			component: "cluster-api",
+			image:     "example.com/controller:v1.0.0",
+			mockFunc: func(component, image string) (string, error) {
+				return "", fmt.Errorf("couldn't parse image name: repository name must be canonical")
+			},
+			want:    "example.com/controller:v1.0.0",
+			wantErr: false,
+		},
+		{
+			name:      "other errors are propagated",
+			component: "cluster-api",
+			image:     "example.com/controller:v1.0.0",
+			mockFunc: func(component, image string) (string, error) {
+				return "", fmt.Errorf("test")
+			},
+			want:    "",
+			wantErr: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+
+			mock := &mockImageMetaClient{alterFunc: tt.mockFunc}
+			result, err := alterImage(tt.component, tt.image, mock)
+
+			if tt.wantErr {
+				g.Expect(err).To(HaveOccurred())
+				return
+			}
+
+			g.Expect(err).ToNot(HaveOccurred())
+			g.Expect(result).To(Equal(tt.want))
+		})
+	}
+}
+
+func TestIsCanonicalError(t *testing.T) {
+	tests := []struct {
+		name string
+		err  error
+		want bool
+	}{
+		{
+			name: "nil error returns false",
+			err:  nil,
+			want: false,
+		},
+		{
+			name: "canonical error with 'repository name must be canonical'",
+			err:  fmt.Errorf("repository name must be canonical"),
+			want: true,
+		},
+		{
+			name: "canonical error with 'couldn't parse image name'",
+			err:  fmt.Errorf("couldn't parse image name: invalid format"),
+			want: true,
+		},
+		{
+			name: "other error returns false",
+			err:  fmt.Errorf("test"),
+			want: false,
+		},
+		{
+			name: "empty error message returns false",
+			err:  fmt.Errorf(""),
+			want: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+			g.Expect(isCanonicalError(tt.err)).To(Equal(tt.want))
+		})
+	}
+}