Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -1108,7 +1108,7 @@ func (r *HostedControlPlaneReconciler) update(ctx context.Context, hostedControl
}

userReleaseImageProvider := imageprovider.New(userReleaseImage)
releaseImageProvider := imageprovider.New(releaseImage)
releaseImageProvider := imageprovider.NewWithRegistryOverrides(releaseImage, r.ReleaseProvider.GetRegistryOverrides())
Comment thread
raelga marked this conversation as resolved.

var errs []error
if err := r.reconcileCPOV2(ctx, hostedControlPlane, infraStatus, releaseImageProvider, userReleaseImageProvider); err != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -731,6 +731,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()

Expand Down Expand Up @@ -814,6 +816,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)
Expand Down
Original file line number Diff line number Diff line change
@@ -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

Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Comment thread
raelga marked this conversation as resolved.
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,
}
}
Original file line number Diff line number Diff line change
@@ -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) {
Expand Down Expand Up @@ -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},
})
Comment thread
raelga marked this conversation as resolved.
}
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")
})
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
7 changes: 3 additions & 4 deletions support/releaseinfo/registry_mirror_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ package releaseinfo

import (
"context"
"strings"
"sync"

"github.com/openshift/hypershift/support/util/registryoverride"
)

var _ ProviderWithRegistryOverrides = (*RegistryMirrorProviderDecorator)(nil)
Expand Down Expand Up @@ -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{
Expand Down
45 changes: 45 additions & 0 deletions support/util/registryoverride/registryoverride.go
Original file line number Diff line number Diff line change
@@ -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):]
}
Loading