From 49d5e3b8b280d5c758825e1161f79d572486b378 Mon Sep 17 00:00:00 2001 From: Deep Mistry Date: Thu, 3 Sep 2026 07:42:35 -0400 Subject: [PATCH] fix: use integrated stream spec digests in release snapshot --- pkg/api/configresolver/configresolver.go | 15 +++- pkg/api/configresolver/configresolver_test.go | 69 +++++++++++++++++++ pkg/api/promotion.go | 27 ++++++++ pkg/api/promotion_test.go | 27 ++++++++ pkg/steps/release/snapshot.go | 16 ++++- pkg/steps/release/snapshot_test.go | 58 +++++++++++++--- 6 files changed, 197 insertions(+), 15 deletions(-) diff --git a/pkg/api/configresolver/configresolver.go b/pkg/api/configresolver/configresolver.go index 686960d16f..f99c01cbd0 100644 --- a/pkg/api/configresolver/configresolver.go +++ b/pkg/api/configresolver/configresolver.go @@ -7,6 +7,7 @@ import ( "github.com/sirupsen/logrus" + coreapi "k8s.io/api/core/v1" ctrlruntimeclient "sigs.k8s.io/controller-runtime/pkg/client" imagev1 "github.com/openshift/api/image/v1" @@ -15,8 +16,9 @@ import ( ) type IntegratedStream struct { - Tags []string `json:"tags,omitempty"` - ReleaseControllerConfigName string `json:"releaseControllerConfigName"` + Tags []string `json:"tags,omitempty"` + TagSources map[string]coreapi.ObjectReference `json:"tagSources,omitempty"` + ReleaseControllerConfigName string `json:"releaseControllerConfigName"` } type releaseConfig struct { @@ -50,8 +52,15 @@ func LocalIntegratedStream(ctx context.Context, client ctrlruntimeclient.Client, return nil, fmt.Errorf("failed to get image stream %s/%s: %w", ns, name, err) } var tags []string + var tagSources map[string]coreapi.ObjectReference for _, tag := range is.Spec.Tags { tags = append(tags, tag.Name) + if tag.From != nil && api.UsableImageStreamTagImportSource(tag.From) { + if tagSources == nil { + tagSources = map[string]coreapi.ObjectReference{} + } + tagSources[tag.Name] = *tag.From + } } var releaseControllerConfigName string if raw, ok := is.ObjectMeta.Annotations[api.ReleaseConfigAnnotation]; ok { @@ -61,5 +70,5 @@ func LocalIntegratedStream(ctx context.Context, client ctrlruntimeclient.Client, } releaseControllerConfigName = configName } - return &IntegratedStream{Tags: tags, ReleaseControllerConfigName: releaseControllerConfigName}, nil + return &IntegratedStream{Tags: tags, TagSources: tagSources, ReleaseControllerConfigName: releaseControllerConfigName}, nil } diff --git a/pkg/api/configresolver/configresolver_test.go b/pkg/api/configresolver/configresolver_test.go index 37a3aaf6df..62537c4db8 100644 --- a/pkg/api/configresolver/configresolver_test.go +++ b/pkg/api/configresolver/configresolver_test.go @@ -8,6 +8,7 @@ import ( "github.com/google/go-cmp/cmp" + coreapi "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes/scheme" ctrlruntimeclient "sigs.k8s.io/controller-runtime/pkg/client" @@ -119,6 +120,74 @@ func TestIntegratedStream(t *testing.T) { }).Build(), expected: &IntegratedStream{Tags: []string{"spec-only", "common"}, ReleaseControllerConfigName: ""}, }, + { + name: "tag sources from spec", + isNS: "ocp", + isName: "5.1", + client: fakeclient.NewClientBuilder().WithRuntimeObjects(&imagev1.ImageStream{ + ObjectMeta: metav1.ObjectMeta{ + Name: "5.1", + Namespace: "ocp", + }, + Spec: imagev1.ImageStreamSpec{ + Tags: []imagev1.TagReference{{ + Name: "karpenter-operator", + From: &coreapi.ObjectReference{ + Kind: "DockerImage", + Name: "quay-proxy.ci.openshift.org/openshift/ci@sha256:abc", + }, + }}, + }, + }).Build(), + expected: &IntegratedStream{ + Tags: []string{"karpenter-operator"}, + TagSources: map[string]coreapi.ObjectReference{ + "karpenter-operator": {Kind: "DockerImage", Name: "quay-proxy.ci.openshift.org/openshift/ci@sha256:abc"}, + }, + }, + }, + { + name: "internal registry tag sources excluded", + isNS: "ocp-priv", + isName: "5.1", + client: fakeclient.NewClientBuilder().WithRuntimeObjects(&imagev1.ImageStream{ + ObjectMeta: metav1.ObjectMeta{ + Name: "5.1", + Namespace: "ocp-priv", + }, + Spec: imagev1.ImageStreamSpec{ + Tags: []imagev1.TagReference{ + { + Name: "cli", + From: &coreapi.ObjectReference{ + Kind: "DockerImage", + Name: "registry.ci.openshift.org/ocp-priv/5.1:cli", + }, + }, + { + Name: "installer", + From: &coreapi.ObjectReference{ + Kind: "DockerImage", + Name: "registry.ci.openshift.org/origin/4.23:installer", + }, + }, + { + Name: "good", + From: &coreapi.ObjectReference{ + Kind: "DockerImage", + Name: "quay-proxy.ci.openshift.org/openshift/ci@sha256:good", + }, + }, + }, + }, + }).Build(), + expected: &IntegratedStream{ + Tags: []string{"cli", "installer", "good"}, + TagSources: map[string]coreapi.ObjectReference{ + "good": {Kind: "DockerImage", Name: "quay-proxy.ci.openshift.org/openshift/ci@sha256:good"}, + }, + }, + }, } for _, tc := range testCases { diff --git a/pkg/api/promotion.go b/pkg/api/promotion.go index 210cf942dc..0369a633d5 100644 --- a/pkg/api/promotion.go +++ b/pkg/api/promotion.go @@ -6,6 +6,7 @@ import ( "github.com/sirupsen/logrus" + coreapi "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/sets" ) @@ -111,6 +112,32 @@ func RefersToQuayReferenceImage(namespace string, includeOKD OKDInclusion) bool return RefersToOfficialImage(namespace, includeOKD) || namespace == ocpPrivPromotionNamespace } +// UsableImageStreamTagImportSource reports whether ref may be copied into a release snapshot tag From. +func UsableImageStreamTagImportSource(from *coreapi.ObjectReference) bool { + if from == nil || from.Name == "" { + return false + } + switch from.Kind { + case "DockerImage": + return !isInternalAPPCIRegistryReference(from.Name) + case "ImageStreamTag", "ImageStreamImage": + return !officialPayloadNamespaces.Has(from.Namespace) + default: + return false + } +} + +var officialPayloadNamespaces = sets.New[string](ocpPromotionNamespace, ocpPrivPromotionNamespace, okdPromotionNamespace) + +func isInternalAPPCIRegistryReference(name string) bool { + for _, ns := range []string{ocpPromotionNamespace, ocpPrivPromotionNamespace, okdPromotionNamespace} { + if strings.HasPrefix(name, ServiceDomainAPPCIRegistry+"/"+ns+"/") { + return true + } + } + return false +} + // QuayImage returns the image in quay.io for an image stream tag which is used to push the image func QuayImage(tag ImageStreamTagReference) string { return fmt.Sprintf("%s:%s_%s_%s", QuayOpenShiftCIRepo, tag.Namespace, tag.Name, tag.Tag) diff --git a/pkg/api/promotion_test.go b/pkg/api/promotion_test.go index 1b8975136d..709837dd2d 100644 --- a/pkg/api/promotion_test.go +++ b/pkg/api/promotion_test.go @@ -4,6 +4,8 @@ import ( "testing" "github.com/google/go-cmp/cmp" + + coreapi "k8s.io/api/core/v1" ) func TestPromotesOfficialImages(t *testing.T) { @@ -96,6 +98,31 @@ func TestRefersToOfficialImage(t *testing.T) { } } +func TestUsableImageStreamTagImportSource(t *testing.T) { + testCases := []struct { + name string + from *coreapi.ObjectReference + expected bool + }{ + {name: "external docker digest", from: &coreapi.ObjectReference{Kind: "DockerImage", Name: "quay-proxy.ci.openshift.org/openshift/ci@sha256:abc"}, expected: true}, + {name: "internal ocp registry", from: &coreapi.ObjectReference{Kind: "DockerImage", Name: ServiceDomainAPPCIRegistry + "/ocp/4.23:cli"}, expected: false}, + {name: "internal ocp-priv registry", from: &coreapi.ObjectReference{Kind: "DockerImage", Name: ServiceDomainAPPCIRegistry + "/ocp-priv/5.1:cli"}, expected: false}, + {name: "internal origin registry", from: &coreapi.ObjectReference{Kind: "DockerImage", Name: ServiceDomainAPPCIRegistry + "/origin/4.23:cli"}, expected: false}, + {name: "imagestreamtag in ci", from: &coreapi.ObjectReference{Kind: "ImageStreamTag", Namespace: "ci", Name: "pipeline:cli"}, expected: true}, + {name: "imagestreamtag in ocp-priv", from: &coreapi.ObjectReference{Kind: "ImageStreamTag", Namespace: "ocp-priv", Name: "5.1:cli"}, expected: false}, + {name: "imagestreamimage in origin", from: &coreapi.ObjectReference{Kind: "ImageStreamImage", Namespace: "origin", Name: "4.23@sha256:abc"}, expected: false}, + {name: "configmap rejected", from: &coreapi.ObjectReference{Kind: "ConfigMap", Name: "pull-secret"}, expected: false}, + {name: "empty name rejected", from: &coreapi.ObjectReference{Kind: "DockerImage", Name: ""}, expected: false}, + } + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + if diff := cmp.Diff(testCase.expected, UsableImageStreamTagImportSource(testCase.from)); diff != "" { + t.Errorf("%s: mismatch (-want +got):\n%s", testCase.name, diff) + } + }) + } +} + func TestRefersToQuayReferenceImage(t *testing.T) { testCases := []struct { name string diff --git a/pkg/steps/release/snapshot.go b/pkg/steps/release/snapshot.go index c7d66c0b52..2132d408b1 100644 --- a/pkg/steps/release/snapshot.go +++ b/pkg/steps/release/snapshot.go @@ -100,7 +100,7 @@ func snapshotStream(ctx context.Context, client loggingclient.LoggingClient, sou return nil, fmt.Errorf("could not resolve source imagestream %s/%s for release %s: %w", sourceNamespace, sourceName, targetRelease, err) } } - from, ok := snapshotImportSource(sourceNamespace, sourceName, tag, source) + from, ok := snapshotImportSource(sourceNamespace, sourceName, tag, source, integratedStream.TagSources) if !ok { continue } @@ -129,7 +129,7 @@ func snapshotStream(ctx context.Context, client loggingclient.LoggingClient, sou return snapshot, nil } -func snapshotImportSource(sourceNamespace, sourceName, tag string, source *imagev1.ImageStream) (*coreapi.ObjectReference, bool) { +func snapshotImportSource(sourceNamespace, sourceName, tag string, source *imagev1.ImageStream, tagSources map[string]coreapi.ObjectReference) (*coreapi.ObjectReference, bool) { base := api.ImageStreamTagReference{Namespace: sourceNamespace, Name: sourceName, Tag: tag} from := &coreapi.ObjectReference{ Kind: "DockerImage", @@ -142,7 +142,17 @@ func snapshotImportSource(sourceNamespace, sourceName, tag string, source *image return nil, false } if api.RefersToOfficialImage(sourceNamespace, api.WithoutOKD) { - return utils.OfficialImageTagFrom(source, base), true + if source != nil { + return utils.OfficialImageTagFrom(source, base), true + } + if ref, ok := tagSources[tag]; ok && api.UsableImageStreamTagImportSource(&ref) { + refCopy := ref + return &refCopy, true + } + if len(tagSources) == 0 { + return utils.OfficialImageTagFrom(nil, base), true + } + return nil, false } return from, true } diff --git a/pkg/steps/release/snapshot_test.go b/pkg/steps/release/snapshot_test.go index 451ab7996c..ca2ddba14f 100644 --- a/pkg/steps/release/snapshot_test.go +++ b/pkg/steps/release/snapshot_test.go @@ -17,13 +17,14 @@ func TestSnapshotImportSource(t *testing.T) { specPull := "quay-proxy.ci.openshift.org/openshift/ci@sha256:abc" base := api.ImageStreamTagReference{Namespace: "ocp", Name: "4.18", Tag: "cluster-version-operator"} tests := []struct { - name string - namespace string - stream string - tag string - source *imagev1.ImageStream - wantOK bool - wantFrom *coreapi.ObjectReference + name string + namespace string + stream string + tag string + source *imagev1.ImageStream + tagSources map[string]coreapi.ObjectReference + wantOK bool + wantFrom *coreapi.ObjectReference }{ { name: "ocp spec first", @@ -48,12 +49,51 @@ func TestSnapshotImportSource(t *testing.T) { wantFrom: &coreapi.ObjectReference{Kind: "DockerImage", Name: api.QuayImageReference(base)}, }, { - name: "ocp missing source imagestream", + name: "ocp missing source imagestream without tag sources", stream: "4.22", tag: base.Tag, wantOK: true, wantFrom: &coreapi.ObjectReference{Kind: "DockerImage", Name: api.QuayImageReference(api.ImageStreamTagReference{Namespace: "ocp", Name: "4.22", Tag: base.Tag})}, }, + { + name: "uses tag sources when resolver provides them", + stream: "5.1", + tag: "cli", + tagSources: map[string]coreapi.ObjectReference{ + "cli": {Kind: "DockerImage", Name: specPull}, + "karpenter-operator": {Kind: "DockerImage", Name: api.ServiceDomainAPPCIRegistry + "/origin/4.23:cli"}, + }, + wantOK: true, + wantFrom: &coreapi.ObjectReference{Kind: "DockerImage", Name: specPull}, + }, + { + name: "ocp uses integrated stream spec when source missing", + stream: "5.1", + tag: "karpenter-operator", + tagSources: map[string]coreapi.ObjectReference{ + "karpenter-operator": {Kind: "DockerImage", Name: specPull}, + }, + wantOK: true, + wantFrom: &coreapi.ObjectReference{Kind: "DockerImage", Name: specPull}, + }, + { + name: "rejects internal registry tag source", + stream: "5.1", + tag: "cli", + tagSources: map[string]coreapi.ObjectReference{ + "cli": {Kind: "DockerImage", Name: api.ServiceDomainAPPCIRegistry + "/origin/4.23:cli"}, + }, + wantOK: false, + }, + { + name: "rejects unsupported tag source kind", + stream: "5.1", + tag: "cli", + tagSources: map[string]coreapi.ObjectReference{ + "cli": {Kind: "ConfigMap", Name: "pull-secret"}, + }, + wantOK: false, + }, { name: "ocp spec docker 4.23", stream: "4.23", @@ -83,7 +123,7 @@ func TestSnapshotImportSource(t *testing.T) { if namespace == "" { namespace = "ocp" } - from, ok := snapshotImportSource(namespace, tt.stream, tt.tag, tt.source) + from, ok := snapshotImportSource(namespace, tt.stream, tt.tag, tt.source, tt.tagSources) if ok != tt.wantOK { t.Fatalf("ok = %v, want %v", ok, tt.wantOK) }