diff --git a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go index b1838187d3ff..34a78b8e06b4 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go +++ b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go @@ -1082,7 +1082,7 @@ func (r *HostedControlPlaneReconciler) update(ctx context.Context, hostedControl } userReleaseImageProvider := imageprovider.New(userReleaseImage) - releaseImageProvider := imageprovider.New(releaseImage) + releaseImageProvider := imageprovider.NewWithRegistryOverrides(releaseImage, r.ReleaseProvider.GetRegistryOverrides()) var errs []error if err := r.reconcileCPOV2(ctx, hostedControlPlane, infraStatus, releaseImageProvider, userReleaseImageProvider); err != nil { diff --git a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller_test.go b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller_test.go index 9aab37fdc81f..c5db5d9b1931 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller_test.go +++ b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller_test.go @@ -715,6 +715,8 @@ func TestEventHandling(t *testing.T) { mockedProviderWithOpenshiftImageRegistryOverrides.EXPECT(). Lookup(gomock.Any(), gomock.Any(), gomock.Any()). Return(testutils.InitReleaseImageOrDie("4.15.0"), nil).AnyTimes() + mockedProviderWithOpenshiftImageRegistryOverrides.EXPECT(). + GetRegistryOverrides().Return(map[string]string{"registry": "override"}).AnyTimes() mockEC2 := awsapi.NewMockEC2API(mockCtrl) mockEC2.EXPECT().DescribeVpcEndpoints(gomock.Any(), gomock.Any()).Return(&ec2.DescribeVpcEndpointsOutput{}, fmt.Errorf("not ready")).AnyTimes() @@ -797,6 +799,8 @@ func TestNonReadyInfraTriggersRequeueAfter(t *testing.T) { mockedProviderWithOpenshiftImageRegistryOverrides.EXPECT(). Lookup(gomock.Any(), gomock.Any(), gomock.Any()). Return(testutils.InitReleaseImageOrDie("4.15.0"), nil).AnyTimes() + mockedProviderWithOpenshiftImageRegistryOverrides.EXPECT(). + GetRegistryOverrides().Return(map[string]string{"registry": "override"}).AnyTimes() mockEC2 := awsapi.NewMockEC2API(mockCtrl) mockEC2.EXPECT().DescribeVpcEndpoints(gomock.Any(), gomock.Any()).Return(&ec2.DescribeVpcEndpointsOutput{}, fmt.Errorf("not ready")).AnyTimes() hcp := sampleHCP(t) diff --git a/control-plane-operator/controllers/hostedcontrolplane/imageprovider/imageprovider.go b/control-plane-operator/controllers/hostedcontrolplane/imageprovider/imageprovider.go index 9f3ee016281a..aefb8cac3dc5 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/imageprovider/imageprovider.go +++ b/control-plane-operator/controllers/hostedcontrolplane/imageprovider/imageprovider.go @@ -1,6 +1,11 @@ package imageprovider -import "github.com/openshift/hypershift/support/releaseinfo" +import ( + "maps" + + "github.com/openshift/hypershift/support/releaseinfo" + "github.com/openshift/hypershift/support/util/registryoverride" +) //go:generate ../../../../hack/tools/bin/mockgen -source=imageprovider.go -package=imageprovider -destination=imageprovider_mock.go @@ -23,11 +28,7 @@ type SimpleReleaseImageProvider struct { } func New(releaseImage *releaseinfo.ReleaseImage) *SimpleReleaseImageProvider { - return &SimpleReleaseImageProvider{ - componentsImages: releaseImage.ComponentImages(), - missingImages: make([]string, 0), - ReleaseImage: releaseImage, - } + return NewWithRegistryOverrides(releaseImage, nil) } func NewFromImages(componentsImages map[string]string) *SimpleReleaseImageProvider { @@ -58,3 +59,21 @@ func (p *SimpleReleaseImageProvider) ImageExist(key string) (string, bool) { func (p *SimpleReleaseImageProvider) ComponentImages() map[string]string { return p.componentsImages } + +// NewWithRegistryOverrides creates a SimpleReleaseImageProvider that applies +// registry overrides to all component images. This ensures init containers +// and other sub-resources created by CPO use the overridden image references. +// +// The returned provider owns a private copy of releaseImage.ComponentImages() +// so callers can safely mutate it. +func NewWithRegistryOverrides(releaseImage *releaseinfo.ReleaseImage, registryOverrides map[string]string) *SimpleReleaseImageProvider { + images := maps.Clone(releaseImage.ComponentImages()) + for key, image := range images { + images[key] = registryoverride.Replace(image, registryOverrides) + } + return &SimpleReleaseImageProvider{ + componentsImages: images, + missingImages: make([]string, 0), + ReleaseImage: releaseImage, + } +} diff --git a/control-plane-operator/controllers/hostedcontrolplane/imageprovider/imageprovider_test.go b/control-plane-operator/controllers/hostedcontrolplane/imageprovider/imageprovider_test.go index 7a6b9b562ac3..22bd25e8e91b 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/imageprovider/imageprovider_test.go +++ b/control-plane-operator/controllers/hostedcontrolplane/imageprovider/imageprovider_test.go @@ -1,9 +1,17 @@ package imageprovider import ( + "maps" "testing" . "github.com/onsi/gomega" + + "github.com/openshift/hypershift/support/releaseinfo" + + imageapi "github.com/openshift/api/image/v1" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) func TestNewFromImages(t *testing.T) { @@ -174,3 +182,178 @@ func TestComponentImages(t *testing.T) { g.Expect(result).To(Equal(images)) }) } + +func newTestReleaseImage(images map[string]string) *releaseinfo.ReleaseImage { + tags := make([]imageapi.TagReference, 0, len(images)) + for name, image := range images { + tags = append(tags, imageapi.TagReference{ + Name: name, + From: &corev1.ObjectReference{Name: image}, + }) + } + return &releaseinfo.ReleaseImage{ + ImageStream: &imageapi.ImageStream{ + ObjectMeta: metav1.ObjectMeta{Name: "4.20.0"}, + Spec: imageapi.ImageStreamSpec{Tags: tags}, + }, + } +} + +func TestNewWithRegistryOverrides(t *testing.T) { + t.Parallel() + + t.Run("When overrides match, component images should be remapped", func(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + releaseImage := newTestReleaseImage(map[string]string{ + "availability-prober": "quay.io/redhat-user-workloads/crt-redhat-acm-tenant/control-plane-operator-4-20:latest", + "kube-apiserver": "quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:abc123", + "etcd": "registry.access.redhat.com/rhel8/etcd:latest", + }) + overrides := map[string]string{ + "quay.io": "mirror.example.com/quay-cache", + } + + provider := NewWithRegistryOverrides(releaseImage, overrides) + + g.Expect(provider.GetImage("availability-prober")).To(Equal( + "mirror.example.com/quay-cache/redhat-user-workloads/crt-redhat-acm-tenant/control-plane-operator-4-20:latest")) + g.Expect(provider.GetImage("kube-apiserver")).To(Equal( + "mirror.example.com/quay-cache/openshift-release-dev/ocp-v4.0-art-dev@sha256:abc123")) + g.Expect(provider.GetImage("etcd")).To(Equal( + "registry.access.redhat.com/rhel8/etcd:latest")) + }) + + t.Run("When no overrides provided, images should be unchanged", func(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + releaseImage := newTestReleaseImage(map[string]string{ + "availability-prober": "quay.io/redhat-user-workloads/crt-redhat-acm-tenant/cpo:latest", + }) + + provider := NewWithRegistryOverrides(releaseImage, nil) + + g.Expect(provider.GetImage("availability-prober")).To(Equal( + "quay.io/redhat-user-workloads/crt-redhat-acm-tenant/cpo:latest")) + }) + + t.Run("When overrides don't match any image, images should be unchanged", func(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + releaseImage := newTestReleaseImage(map[string]string{ + "etcd": "registry.access.redhat.com/rhel8/etcd:latest", + }) + overrides := map[string]string{ + "quay.io": "mirror.example.com", + } + + provider := NewWithRegistryOverrides(releaseImage, overrides) + + g.Expect(provider.GetImage("etcd")).To(Equal( + "registry.access.redhat.com/rhel8/etcd:latest")) + }) + + t.Run("When override prefix matches subdomain, it should not apply", func(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + releaseImage := newTestReleaseImage(map[string]string{ + "component": "quay.io.example.com/namespace/image:tag", + }) + overrides := map[string]string{ + "quay.io": "mirror.example.com", + } + + provider := NewWithRegistryOverrides(releaseImage, overrides) + + g.Expect(provider.GetImage("component")).To(Equal( + "quay.io.example.com/namespace/image:tag")) + }) + + t.Run("When multiple overrides exist, only the matching one should apply", func(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + releaseImage := newTestReleaseImage(map[string]string{ + "availability-prober": "quay.io/redhat-user-workloads/crt-redhat-acm-tenant/cpo:latest", + "etcd": "gcr.io/etcd-development/etcd:v3.5", + }) + overrides := map[string]string{ + "quay.io": "acr.example.com/quay-cache", + "gcr.io": "acr.example.com/gcr-cache", + } + + provider := NewWithRegistryOverrides(releaseImage, overrides) + + g.Expect(provider.GetImage("availability-prober")).To(Equal( + "acr.example.com/quay-cache/redhat-user-workloads/crt-redhat-acm-tenant/cpo:latest")) + g.Expect(provider.GetImage("etcd")).To(Equal( + "acr.example.com/gcr-cache/etcd-development/etcd:v3.5")) + }) + t.Run("When applied, longest-prefix override wins over a broader one", func(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + releaseImage := newTestReleaseImage(map[string]string{ + "kube-apiserver": "quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:abc123", + }) + overrides := map[string]string{ + "quay.io": "broad.example.com", + "quay.io/openshift-release-dev": "narrow.example.com/mirror", + } + + provider := NewWithRegistryOverrides(releaseImage, overrides) + + g.Expect(provider.GetImage("kube-apiserver")).To(Equal( + "narrow.example.com/mirror/ocp-v4.0-art-dev@sha256:abc123")) + }) + + t.Run("When applied, overrides and release-image maps are not mutated", func(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + sourceImages := map[string]string{ + "availability-prober": "quay.io/redhat-user-workloads/crt-redhat-acm-tenant/cpo:latest", + "kube-apiserver": "quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:abc123", + } + sourceImagesSnapshot := maps.Clone(sourceImages) + releaseImage := newTestReleaseImage(sourceImages) + + overrides := map[string]string{ + "quay.io": "mirror.example.com/quay-cache", + } + overridesSnapshot := maps.Clone(overrides) + + _ = NewWithRegistryOverrides(releaseImage, overrides) + + g.Expect(overrides).To(Equal(overridesSnapshot), + "NewWithRegistryOverrides must not mutate its overrides argument") + g.Expect(releaseImage.ComponentImages()).To(Equal(sourceImagesSnapshot), + "NewWithRegistryOverrides must not mutate the source release image's ComponentImages map") + }) + + t.Run("When applied twice, the second application is a no-op (idempotent)", func(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + overrides := map[string]string{ + "quay.io": "mirror.example.com/quay-cache", + } + firstReleaseImage := newTestReleaseImage(map[string]string{ + "availability-prober": "quay.io/redhat-user-workloads/crt-redhat-acm-tenant/cpo:latest", + "etcd": "registry.access.redhat.com/rhel8/etcd:latest", + }) + + firstProvider := NewWithRegistryOverrides(firstReleaseImage, overrides) + firstImages := maps.Clone(firstProvider.ComponentImages()) + + secondReleaseImage := newTestReleaseImage(firstImages) + secondProvider := NewWithRegistryOverrides(secondReleaseImage, overrides) + + g.Expect(secondProvider.ComponentImages()).To(Equal(firstImages), + "applying the same overrides a second time must not change images already rewritten") + }) +} diff --git a/support/releaseinfo/registry_mirror_provider.go b/support/releaseinfo/registry_mirror_provider.go index e1b8803ae9fb..61efd1f2f081 100644 --- a/support/releaseinfo/registry_mirror_provider.go +++ b/support/releaseinfo/registry_mirror_provider.go @@ -2,8 +2,9 @@ package releaseinfo import ( "context" - "strings" "sync" + + "github.com/openshift/hypershift/support/util/registryoverride" ) var _ ProviderWithRegistryOverrides = (*RegistryMirrorProviderDecorator)(nil) @@ -34,9 +35,7 @@ func (p *RegistryMirrorProviderDecorator) Lookup(ctx context.Context, image stri imageStream := releaseImage.ImageStream.DeepCopy() // deepCopy so the cache is not overridden. for i := range imageStream.Spec.Tags { - for registrySource, registryDest := range p.RegistryOverrides { - imageStream.Spec.Tags[i].From.Name = strings.Replace(imageStream.Spec.Tags[i].From.Name, registrySource, registryDest, 1) - } + imageStream.Spec.Tags[i].From.Name = registryoverride.Replace(imageStream.Spec.Tags[i].From.Name, p.RegistryOverrides) } return &ReleaseImage{ diff --git a/support/util/registryoverride/registryoverride.go b/support/util/registryoverride/registryoverride.go new file mode 100644 index 000000000000..009c4d43da1a --- /dev/null +++ b/support/util/registryoverride/registryoverride.go @@ -0,0 +1,45 @@ +// Package registryoverride applies registry-prefix overrides to image +// references with strict, deterministic matching semantics. It is intentionally +// a leaf package with no project-internal imports so it can be used both from +// support/releaseinfo and from sub-packages that depend on it. +package registryoverride + +import "strings" + +// Replace remaps an image reference using a set of registry-prefix overrides. +// The overrides map keys are source registry prefixes and values are +// replacement prefixes. +// +// Matching is strict: an override applies only when the image reference is +// exactly equal to the source key or starts with the source key followed by a +// "/" separator. This prevents accidental substring matches (e.g. an override +// for "quay.io" must not match "quay.io.example.com/foo"). +// +// When several override keys match the same image, the longest key wins. This +// makes the result deterministic regardless of map iteration order and lets +// callers express both broad ("quay.io") and narrow +// ("quay.io/openshift-release-dev") overrides simultaneously. +// +// If no override matches, image is returned unchanged. +func Replace(image string, overrides map[string]string) string { + if image == "" || len(overrides) == 0 { + return image + } + + var bestSource, bestTarget string + for source, target := range overrides { + if source == "" { + continue + } + if image != source && !strings.HasPrefix(image, source+"/") { + continue + } + if len(source) > len(bestSource) { + bestSource, bestTarget = source, target + } + } + if bestSource == "" { + return image + } + return bestTarget + image[len(bestSource):] +} diff --git a/support/util/registryoverride/registryoverride_test.go b/support/util/registryoverride/registryoverride_test.go new file mode 100644 index 000000000000..487dbfd4c12f --- /dev/null +++ b/support/util/registryoverride/registryoverride_test.go @@ -0,0 +1,131 @@ +package registryoverride + +import ( + "reflect" + "testing" +) + +func TestReplace(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + image string + overrides map[string]string + want string + }{ + { + name: "nil overrides returns input unchanged", + image: "quay.io/openshift-release-dev/ocp-release@sha256:abc", + overrides: nil, + want: "quay.io/openshift-release-dev/ocp-release@sha256:abc", + }, + { + name: "empty overrides returns input unchanged", + image: "quay.io/openshift-release-dev/ocp-release@sha256:abc", + overrides: map[string]string{}, + want: "quay.io/openshift-release-dev/ocp-release@sha256:abc", + }, + { + name: "no matching override returns input unchanged", + image: "quay.io/openshift-release-dev/ocp-release@sha256:abc", + overrides: map[string]string{"registry.redhat.io": "mirror.example.com"}, + want: "quay.io/openshift-release-dev/ocp-release@sha256:abc", + }, + { + name: "exact-match key replaces image", + image: "quay.io", + overrides: map[string]string{"quay.io": "mirror.example.com"}, + want: "mirror.example.com", + }, + { + name: "slash-boundary prefix match preserves path and digest", + image: "quay.io/openshift-release-dev/ocp-release@sha256:abc", + overrides: map[string]string{"quay.io": "mirror.example.com"}, + want: "mirror.example.com/openshift-release-dev/ocp-release@sha256:abc", + }, + { + name: "subdomain does not match (no false positive)", + image: "quay.io.example.com/foo/bar:latest", + overrides: map[string]string{"quay.io": "mirror.example.com"}, + want: "quay.io.example.com/foo/bar:latest", + }, + { + name: "trailing path component does not match (no false positive)", + image: "quay.io-evil/foo:latest", + overrides: map[string]string{"quay.io": "mirror.example.com"}, + want: "quay.io-evil/foo:latest", + }, + { + name: "longest matching prefix wins", + image: "quay.io/openshift-release-dev/ocp-release@sha256:abc", + overrides: map[string]string{ + "quay.io": "broad.example.com", + "quay.io/openshift-release-dev": "narrow.example.com/mirror", + }, + want: "narrow.example.com/mirror/ocp-release@sha256:abc", + }, + { + name: "shorter prefix used when longer prefix does not match", + image: "quay.io/some-other-org/image:tag", + overrides: map[string]string{ + "quay.io": "broad.example.com", + "quay.io/openshift-release-dev": "narrow.example.com/mirror", + }, + want: "broad.example.com/some-other-org/image:tag", + }, + { + name: "empty source key is skipped", + image: "quay.io/foo/bar:latest", + overrides: map[string]string{ + "": "should-never-be-used", + "quay.io": "mirror.example.com", + }, + want: "mirror.example.com/foo/bar:latest", + }, + { + name: "tag is preserved", + image: "quay.io/foo/bar:v1.2.3", + overrides: map[string]string{"quay.io": "mirror.example.com"}, + want: "mirror.example.com/foo/bar:v1.2.3", + }, + { + name: "empty image returns empty", + image: "", + overrides: map[string]string{"quay.io": "mirror.example.com"}, + want: "", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + got := Replace(tc.image, tc.overrides) + if got != tc.want { + t.Errorf("Replace(%q, %v) = %q, want %q", tc.image, tc.overrides, got, tc.want) + } + }) + } +} + +func TestReplace_DoesNotMutateOverrides(t *testing.T) { + t.Parallel() + + overrides := map[string]string{ + "quay.io": "broad.example.com", + "quay.io/openshift-release-dev": "narrow.example.com/mirror", + "registry.redhat.io": "rh.mirror.example.com", + } + snapshot := make(map[string]string, len(overrides)) + for k, v := range overrides { + snapshot[k] = v + } + + _ = Replace("quay.io/openshift-release-dev/ocp-release@sha256:abc", overrides) + _ = Replace("registry.redhat.io/some/image:latest", overrides) + _ = Replace("does.not.match/anything:tag", overrides) + + if !reflect.DeepEqual(overrides, snapshot) { + t.Errorf("overrides map was mutated by Replace; got %v, want %v", overrides, snapshot) + } +}