Merge pull request #933 from k8s-infra-cherrypick-robot/cherry-pick-928-to-release-0.24

[release-0.24] 🐛 Allow non canonical image paths

Author: k8s-ci-robot

go.mod

@@ -5,6 +5,7 @@ go 1.24.4
 require (
 	github.com/MakeNowJust/heredoc v1.0.0
 	github.com/Masterminds/goutils v1.1.1
+	github.com/distribution/reference v0.6.0
 	github.com/evanphx/json-patch/v5 v5.9.11
 	github.com/go-errors/errors v1.5.1
 	github.com/go-logr/logr v1.4.3
@@ -40,7 +41,6 @@ require (
 	github.com/cespare/xxhash/v2 v2.3.0 // indirect
 	github.com/cloudflare/circl v1.6.1 // indirect
 	github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect
-	github.com/distribution/reference v0.6.0 // indirect
 	github.com/drone/envsubst/v2 v2.0.0-20210730161058-179042472c46 // indirect
 	github.com/emicklei/go-restful/v3 v3.12.2 // indirect
 	github.com/felixge/httpsnoop v1.0.4 // indirect

internal/controller/image_overrides.go

@@ -17,8 +17,10 @@ limitations under the License.
 package controller
 
 import (
+	"errors"
 	"fmt"
 
+	"github.com/distribution/reference"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/client-go/kubernetes/scheme"
 
@@ -39,13 +41,34 @@ 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 canonical image format.
+func isCanonicalError(err error) bool {
+	return errors.Is(err, reference.ErrNameNotCanonical)
+}
+
 // 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

@@ -20,6 +20,7 @@ import (
 	"fmt"
 	"testing"
 
+	"github.com/distribution/reference"
 	. "github.com/onsi/gomega"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
@@ -169,3 +170,117 @@ 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 "", reference.ErrNameNotCanonical
+			},
+			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: "ErrNameNotCanonical returns true",
+			err:  reference.ErrNameNotCanonical,
+			want: true,
+		},
+		{
+			name: "wrapped ErrNameNotCanonical returns true",
+			err:  fmt.Errorf("parse error: %w", reference.ErrNameNotCanonical),
+			want: true,
+		},
+		{
+			name: "other error returns false",
+			err:  fmt.Errorf("test"),
+			want: false,
+		},
+		{
+			name: "couldn't parse image name error returns false",
+			err:  fmt.Errorf("couldn't parse image name: invalid format"),
+			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))
+		})
+	}
+}

scripts/ci-install-mdbook.sh

@@ -30,5 +30,5 @@ curl https://sh.rustup.rs -sSf | sh -s -- -y
 
 # Install mdbook and dependencies
 cargo install mdbook --version "$VERSION" --root "$OUTPUT_PATH"
-cargo install mdbook-fs-summary --root "$OUTPUT_PATH"
-cargo install mdbook-toc --root "$OUTPUT_PATH"
+cargo install mdbook-fs-summary --version "=0.2.0" --root "$OUTPUT_PATH"
+cargo install mdbook-toc --version "=0.14.2" --root "$OUTPUT_PATH"