diff --git a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go index 2223c878193b..3d4c4f30099b 100644 --- a/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go +++ b/control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go @@ -1080,7 +1080,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 a5936e9c0454..07a2752a94a3 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{}).AnyTimes() mockEC2 := awsapi.NewMockEC2API(mockCtrl) mockEC2.EXPECT().DescribeVpcEndpoints(gomock.Any(), gomock.Any()).Return(&ec2.DescribeVpcEndpointsOutput{}, fmt.Errorf("not ready")).AnyTimes() 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/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..e7eff74bad49 --- /dev/null +++ b/support/util/registryoverride/registryoverride.go @@ -0,0 +1,68 @@ +// 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 +// valid image-reference separator ("/" for path components, "@" for digests, +// or ":" for tags). This prevents accidental substring matches (e.g. an +// override for "quay.io" must not match "quay.io.example.com/foo") while +// correctly handling repository-level overrides against digest references +// (e.g. "quay.io/org/repo" must match "quay.io/org/repo@sha256:abc"). +// +// 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 !matchesPrefix(image, source) { + continue + } + if len(source) > len(bestSource) { + bestSource, bestTarget = source, target + } + } + if bestSource == "" { + return image + } + return bestTarget + image[len(bestSource):] +} + +// matchesPrefix reports whether image is exactly source, or source is a prefix +// of image followed by a valid separator. Valid separators are "/" (path) and +// "@" (digest), always. ":" is accepted only when source contains a "/" (i.e. +// includes a path component), where it denotes a tag boundary. Without a "/" +// the source is a bare hostname and ":" would be a port separator, which must +// not match (e.g. override "quay.io" must not match "quay.io:5000/foo"). +func matchesPrefix(image, source string) bool { + if image == source { + return true + } + if !strings.HasPrefix(image, source) { + return false + } + sep := image[len(source)] + if sep == '/' || sep == '@' { + return true + } + return sep == ':' && strings.ContainsRune(source, '/') +} diff --git a/support/util/registryoverride/registryoverride_test.go b/support/util/registryoverride/registryoverride_test.go new file mode 100644 index 000000000000..1212c8ae9b74 --- /dev/null +++ b/support/util/registryoverride/registryoverride_test.go @@ -0,0 +1,170 @@ +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: "When source matches full repository with digest separator it should replace prefix", + image: "quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:abc123", + overrides: map[string]string{"quay.io/openshift-release-dev/ocp-v4.0-art-dev": "mirror.example.com/art-dev"}, + want: "mirror.example.com/art-dev@sha256:abc123", + }, + { + name: "When source matches full repository with tag separator it should replace prefix", + image: "quay.io/openshift-release-dev/ocp-v4.0-art-dev:latest", + overrides: map[string]string{"quay.io/openshift-release-dev/ocp-v4.0-art-dev": "mirror.example.com/art-dev"}, + want: "mirror.example.com/art-dev:latest", + }, + { + name: "When multiple overrides match with digest it should pick longest prefix", + image: "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/ocp-v4.0-art-dev": "narrow.example.com/art-dev", + }, + want: "narrow.example.com/art-dev@sha256:abc123", + }, + { + name: "When source has trailing dash it should not match similar prefix (no false positive)", + image: "quay.io/openshift-release-dev/ocp-v4.0-art-dev-extra@sha256:abc", + overrides: map[string]string{"quay.io/openshift-release-dev/ocp-v4.0-art-dev": "mirror/art-dev"}, + want: "quay.io/openshift-release-dev/ocp-v4.0-art-dev-extra@sha256:abc", + }, + { + name: "When host-only source matches host:port image it should not match (port is not a tag)", + image: "quay.io:5000/org/repo@sha256:abc", + overrides: map[string]string{"quay.io": "mirror.example.com"}, + want: "quay.io:5000/org/repo@sha256:abc", + }, + { + name: "When host:port source matches host:port image it should match via slash", + image: "myregistry:5000/org/repo@sha256:abc", + overrides: map[string]string{"myregistry:5000": "mirror.example.com"}, + want: "mirror.example.com/org/repo@sha256:abc", + }, + { + 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) + } +}