From 6f542dbc0ede43d953a8354ca7e934238eecd51c Mon Sep 17 00:00:00 2001 From: Rael Garcia Date: Thu, 14 May 2026 13:41:06 +0200 Subject: [PATCH] fix: apply registry overrides to component images in CPO sub-resources When CPO creates sub-resources (e.g. capi-provider deployments), it injects init containers like availability-prober using image references from the release image. These references were not being remapped by --registry-overrides, causing the init containers to reference the original registry (e.g. quay.io) instead of the override target. Add NewWithRegistryOverrides to the imageprovider package that clones the component images map and applies registry overrides using safe prefix matching (source+"/") to prevent subdomain false matches. Use it when creating the releaseImageProvider in the HostedControlPlane reconciler. Bug: https://redhat.atlassian.net/browse/OCPBUGS-85585 --- .../hostedcontrolplane_controller.go | 2 +- .../hostedcontrolplane_controller_test.go | 4 + .../imageprovider/imageprovider.go | 32 ++++- .../imageprovider/imageprovider_test.go | 119 ++++++++++++++++++ 4 files changed, 155 insertions(+), 2 deletions(-) diff --git a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go index d21ee116ffd..6184a66b368 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go +++ b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go @@ -1103,7 +1103,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 3ddf48cd6be..8a3ad01d8b4 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller_test.go +++ b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller_test.go @@ -727,6 +727,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{}).AnyTimes() mockEC2 := awsapi.NewMockEC2API(mockCtrl) mockEC2.EXPECT().DescribeVpcEndpoints(gomock.Any(), gomock.Any()).Return(&ec2.DescribeVpcEndpointsOutput{}, fmt.Errorf("not ready")).AnyTimes() @@ -810,6 +812,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{}).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 9f3ee016281..d097a130ddd 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/imageprovider/imageprovider.go +++ b/control-plane-operator/controllers/hostedcontrolplane/imageprovider/imageprovider.go @@ -1,6 +1,10 @@ package imageprovider -import "github.com/openshift/hypershift/support/releaseinfo" +import ( + "strings" + + "github.com/openshift/hypershift/support/releaseinfo" +) //go:generate ../../../../hack/tools/bin/mockgen -source=imageprovider.go -package=imageprovider -destination=imageprovider_mock.go @@ -58,3 +62,29 @@ 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. +func NewWithRegistryOverrides(releaseImage *releaseinfo.ReleaseImage, registryOverrides map[string]string) *SimpleReleaseImageProvider { + originalImages := releaseImage.ComponentImages() + images := make(map[string]string, len(originalImages)) + for k, v := range originalImages { + images[k] = v + } + if len(registryOverrides) > 0 { + for key, image := range images { + for source, target := range registryOverrides { + if image == source || strings.HasPrefix(image, source+"/") { + images[key] = strings.Replace(image, source, target, 1) + break + } + } + } + } + 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 7a6b9b562ac..b5ed26f2c99 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/imageprovider/imageprovider_test.go +++ b/control-plane-operator/controllers/hostedcontrolplane/imageprovider/imageprovider_test.go @@ -4,6 +4,13 @@ import ( "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 +181,115 @@ 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")) + }) +}