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
14 changes: 14 additions & 0 deletions pkg/controller/render/render_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
88 changes: 88 additions & 0 deletions pkg/controller/render/render_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}
}
})
}
}