From 60ff74a44569454ad04cbbb41f7445c46b9adc5d Mon Sep 17 00:00:00 2001
From: David Joshy <djoshy@redhat.com>
Date: Mon, 1 Jun 2026 10:19:18 -0400
Subject: [PATCH] render: guard OSImageStream ver before generation

---
 pkg/controller/render/render_controller.go    | 14 +++
 .../render/render_controller_test.go          | 88 +++++++++++++++++++
 2 files changed, 102 insertions(+)

diff --git a/pkg/controller/render/render_controller.go b/pkg/controller/render/render_controller.go
index 0c5b68730a..b20b81ebc4 100644
--- a/pkg/controller/render/render_controller.go
+++ b/pkg/controller/render/render_controller.go
@@ -659,6 +659,20 @@ func (ctrl *Controller) getOSImageStreamForPool(pool *mcfgv1.MachineConfigPool)
 		return nil, fmt.Errorf("could not get OSImageStream for pool %s: %w", pool.Name, err)
 	}
 
+	// Guard against using an OSImageStream written by a different operator version.
+	// Unlike ControllerConfig (which has its own generated-by-version guard), the
+	// OSImageStream is fetched independently and directly supplies the OS image URL,
+	// bypassing the ControllerConfig path entirely. Without this check, an old MCC
+	// could render from a new OSImageStream mid-upgrade, producing configs with a new
+	// OS image URL but old config data from the ControllerConfig.
+	streamVersion, ok := imageStream.Annotations[ctrlcommon.ReleaseImageVersionAnnotationKey]
+	if !ok {
+		return nil, fmt.Errorf("ignoring OSImageStream without %s annotation (my version: %s)", ctrlcommon.ReleaseImageVersionAnnotationKey, version.Hash)
+	}
+	if streamVersion != version.Hash {
+		return nil, fmt.Errorf("ignoring OSImageStream with version %s (my version: %s)", streamVersion, version.Hash)
+	}
+
 	imageStreamSet, err := osimagestream.GetOSImageStreamSetByName(imageStream, streamName)
 	if err != nil {
 		return nil, fmt.Errorf("could not get OSImageStreamSet for pool %s: %w", pool.Name, err)
diff --git a/pkg/controller/render/render_controller_test.go b/pkg/controller/render/render_controller_test.go
index 0f8259a770..2c0e94802f 100644
--- a/pkg/controller/render/render_controller_test.go
+++ b/pkg/controller/render/render_controller_test.go
@@ -27,9 +27,12 @@ import (
 	"k8s.io/client-go/tools/cache"
 	"k8s.io/client-go/tools/record"
 
+	features "github.com/openshift/api/features"
 	mcfgv1 "github.com/openshift/api/machineconfiguration/v1"
+	"github.com/openshift/api/machineconfiguration/v1alpha1"
 	"github.com/openshift/client-go/machineconfiguration/clientset/versioned/fake"
 	informers "github.com/openshift/client-go/machineconfiguration/informers/externalversions"
+	mcfglistersv1alpha1 "github.com/openshift/client-go/machineconfiguration/listers/machineconfiguration/v1alpha1"
 	ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common"
 	daemonconsts "github.com/openshift/machine-config-operator/pkg/daemon/constants"
 	"github.com/openshift/machine-config-operator/pkg/version"
@@ -923,3 +926,88 @@ func TestWorkerPoolOtherChangeDoesNotEnqueueCustomPools(t *testing.T) {
 	assert.NotContains(t, enqueuedPools, "infra", "Infra pool should NOT be enqueued when osImageStream unchanged")
 	assert.Len(t, enqueuedPools, 1, "Only worker pool should be enqueued")
 }
+
+// TestGetOSImageStreamVersionGuard verifies that getOSImageStreamForPool refuses to
+// use an OSImageStream whose version annotation does not match the running MCC version.
+// This guards against an old MCC rendering from a new OSImageStream written by the new
+// operator during an upgrade, which would produce configs with mixed provenance.
+func TestGetOSImageStreamVersionGuard(t *testing.T) {
+	pool := helpers.NewMachineConfigPool("worker", helpers.WorkerSelector, nil, "")
+
+	makeStream := func(annotationValue *string) *v1alpha1.OSImageStream {
+		s := &v1alpha1.OSImageStream{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:        ctrlcommon.ClusterInstanceNameOSImageStream,
+				Annotations: map[string]string{},
+			},
+			Status: v1alpha1.OSImageStreamStatus{
+				AvailableStreams: []v1alpha1.OSImageStreamSet{
+					{
+						Name:    "rhel-9",
+						OSImage: v1alpha1.ImageDigestFormat("registry.example.com/os@sha256:abc"),
+					},
+				},
+				DefaultStream: "rhel-9",
+			},
+		}
+		if annotationValue != nil {
+			s.Annotations[ctrlcommon.ReleaseImageVersionAnnotationKey] = *annotationValue
+		}
+		return s
+	}
+
+	myVersion := version.Hash
+	otherVersion := myVersion + "-other"
+
+	tests := []struct {
+		name        string
+		stream      *v1alpha1.OSImageStream
+		wantErr     bool
+		wantNilSet  bool
+	}{
+		{
+			name:       "matching version returns stream set",
+			stream:     makeStream(&myVersion),
+			wantErr:    false,
+			wantNilSet: false,
+		},
+		{
+			name:    "mismatched version returns error",
+			stream:  makeStream(&otherVersion),
+			wantErr: true,
+		},
+		{
+			name:    "missing annotation returns error",
+			stream:  makeStream(nil),
+			wantErr: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{})
+			require.NoError(t, indexer.Add(tt.stream))
+
+			ctrl := &Controller{
+				osImageStreamLister: mcfglistersv1alpha1.NewOSImageStreamLister(indexer),
+				fgHandler: ctrlcommon.NewFeatureGatesHardcodedHandler(
+					[]apicfgv1.FeatureGateName{features.FeatureGateOSStreams},
+					[]apicfgv1.FeatureGateName{},
+				),
+			}
+
+			set, err := ctrl.getOSImageStreamForPool(pool)
+			if tt.wantErr {
+				require.Error(t, err)
+				assert.Nil(t, set)
+			} else {
+				require.NoError(t, err)
+				if tt.wantNilSet {
+					assert.Nil(t, set)
+				} else {
+					assert.NotNil(t, set)
+				}
+			}
+		})
+	}
+}