diff --git a/cmd/machine-config-controller/start.go b/cmd/machine-config-controller/start.go index 4f0ad29019..e009c87dea 100644 --- a/cmd/machine-config-controller/start.go +++ b/cmd/machine-config-controller/start.go @@ -273,6 +273,11 @@ func createControllers(ctx *ctrlcommon.ControllerContext) []ctrlcommon.Controlle ctx.InformerFactory.Machineconfiguration().V1().KubeletConfigs(), ctx.OperatorInformerFactory.Operator().V1().MachineConfigurations(), ctx.InformerFactory.Machineconfiguration().V1().OSImageStreams(), + // TODO(OCP 5.3): Remove these four informers when runc is removed. + ctx.ConfigInformerFactory.Config().V1().Images(), + ctx.ConfigInformerFactory.Config().V1().ImageDigestMirrorSets(), + ctx.ConfigInformerFactory.Config().V1().ImageTagMirrorSets(), + ctx.OperatorInformerFactory.Operator().V1alpha1().ImageContentSourcePolicies(), ctx.ClientBuilder.KubeClientOrDie("render-controller"), ctx.ClientBuilder.MachineConfigClientOrDie("render-controller"), ctx.FeatureGatesHandler, diff --git a/pkg/controller/bootstrap/bootstrap.go b/pkg/controller/bootstrap/bootstrap.go index 83fbaec174..d2b878fbef 100644 --- a/pkg/controller/bootstrap/bootstrap.go +++ b/pkg/controller/bootstrap/bootstrap.go @@ -357,7 +357,21 @@ func (b *Bootstrap) Run(destDir string) error { klog.Infof("Successfully created %d pre-built image component MachineConfigs for hybrid OCL.", len(preBuiltImageMCs)) } - fpools, gconfigs, err := render.RunBootstrap(pools, configs, cconfig, osImageStream) + // When no OSImageStream is available, fall back to inspecting the + // BaseOSContainerImage (derived from the release payload) to determine the + // OS stream class (e.g. "rhel-9", "rhel-10"). This is used downstream to + // decide whether runc should be blocked on RHEL 10. + // TODO(OCP 5.3): remove this once runc is removed. + var baseStreamClass string + if osImageStream == nil && cconfig.Spec.BaseOSContainerImage != "" { + sc, err := b.getBaseStreamClass(cconfig, pullSecretBytes, icspRules, idmsRules, itmsRules, imgCfg) + if err != nil { + return fmt.Errorf("failed to determine base OS stream class: %w", err) + } + baseStreamClass = sc + } + + fpools, gconfigs, err := render.RunBootstrap(pools, configs, cconfig, osImageStream, baseStreamClass) if err != nil { return err } @@ -536,6 +550,29 @@ func (b *Bootstrap) fetchOSImageStream( return osImageStream, nil } +// getBaseStreamClass inspects the base OS container image to determine the OS +// stream class (e.g. "rhel-9", "rhel-10") when OSImageStream is not available. +// TODO(OCP 5.3): Remove when runc is removed. +func (b *Bootstrap) getBaseStreamClass( + cconfig *mcfgv1.ControllerConfig, + pullSecretBytes []byte, + icspRules []*apioperatorsv1alpha1.ImageContentSourcePolicy, + idmsRules []*apicfgv1.ImageDigestMirrorSet, + itmsRules []*apicfgv1.ImageTagMirrorSet, + imgCfg *apicfgv1.Image, +) (string, error) { + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + + secret := &corev1.Secret{ + Data: map[string][]byte{ + corev1.DockerConfigJsonKey: pullSecretBytes, + }, + Type: corev1.SecretTypeDockerConfigJson, + } + return osimagestream.InspectStreamClassWithMirrors(ctx, secret, cconfig, imgCfg, icspRules, idmsRules, itmsRules, cconfig.Spec.BaseOSContainerImage) +} + // Returns the embedded ImageStreamFactory or constructs a new default one. Used primarily for testing. func (b *Bootstrap) getImageStreamFactory() osimagestream.ImageStreamFactory { if b.imageStreamFactory != nil { diff --git a/pkg/controller/common/constants.go b/pkg/controller/common/constants.go index 80b5540f30..2a34d25d97 100644 --- a/pkg/controller/common/constants.go +++ b/pkg/controller/common/constants.go @@ -30,6 +30,16 @@ const ( // OSImageURLOverriddenKey is used to tag a rendered machineconfig when OSImageURL has been overridden from default using machineconfig OSImageURLOverriddenKey = "machineconfiguration.openshift.io/os-image-url-overridden" + // OSImageStreamClassAnnotationKey caches the stream class (e.g. "rhel-9", "rhel-10") + // determined by inspecting the OS container image's io.openshift.os.streamclass label. + // TODO(OCP 5.3): Remove when runc is removed. + OSImageStreamClassAnnotationKey = "machineconfiguration.openshift.io/os-image-stream-class" + + // OSImageURLInspectedAnnotationKey tracks which OSImageURL was inspected to produce + // the cached stream class. When the OSImageURL changes, the cache is invalidated. + // TODO(OCP 5.3): Remove when runc is removed. + OSImageURLInspectedAnnotationKey = "machineconfiguration.openshift.io/os-image-url-inspected" + // RenderedMachineConfigPrefix is the name prefix for rendered MachineConfigs RenderedMachineConfigPrefix = "rendered-" diff --git a/pkg/controller/render/render_controller.go b/pkg/controller/render/render_controller.go index 719a88ef33..4c0a0f3c06 100644 --- a/pkg/controller/render/render_controller.go +++ b/pkg/controller/render/render_controller.go @@ -11,12 +11,16 @@ import ( "github.com/openshift/api/features" mcfgv1 "github.com/openshift/api/machineconfiguration/v1" opv1 "github.com/openshift/api/operator/v1" + cligoinformersv1 "github.com/openshift/client-go/config/informers/externalversions/config/v1" + cligolistersv1 "github.com/openshift/client-go/config/listers/config/v1" mcfgclientset "github.com/openshift/client-go/machineconfiguration/clientset/versioned" "github.com/openshift/client-go/machineconfiguration/clientset/versioned/scheme" mcfginformersv1 "github.com/openshift/client-go/machineconfiguration/informers/externalversions/machineconfiguration/v1" mcfglistersv1 "github.com/openshift/client-go/machineconfiguration/listers/machineconfiguration/v1" mcopinformersv1 "github.com/openshift/client-go/operator/informers/externalversions/operator/v1" + operatorinformersv1alpha1 "github.com/openshift/client-go/operator/informers/externalversions/operator/v1alpha1" mcoplistersv1 "github.com/openshift/client-go/operator/listers/operator/v1" + operatorlistersv1alpha1 "github.com/openshift/client-go/operator/listers/operator/v1alpha1" mcoResourceApply "github.com/openshift/machine-config-operator/lib/resourceapply" "github.com/openshift/machine-config-operator/pkg/apihelpers" ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common" @@ -58,6 +62,14 @@ var ( machineconfigKind = mcfgv1.SchemeGroupVersion.WithKind("MachineConfig") ) +// osImageStreamClassInspector abstracts OS image stream class inspection to +// enable test mocking. The default implementation inspects the container +// image's io.openshift.os.streamclass label via the cluster's pull secret. +// TODO(OCP 5.3): Remove when runc is removed. +type osImageStreamClassInspector interface { + GetStreamClass(ctx context.Context, imageURL string, cc *mcfgv1.ControllerConfig) (string, error) +} + // Controller defines the render controller. type Controller struct { client mcfgclientset.Interface @@ -86,8 +98,15 @@ type Controller struct { mcopLister mcoplistersv1.MachineConfigurationLister mcopListerSynced cache.InformerSynced + imgListerSynced cache.InformerSynced + icspListerSynced cache.InformerSynced + idmsListerSynced cache.InformerSynced + itmsListerSynced cache.InformerSynced + fgHandler ctrlcommon.FeatureGatesHandler + imageInspector osImageStreamClassInspector + queue workqueue.TypedRateLimitingInterface[string] } @@ -100,6 +119,10 @@ func New( mckInformer mcfginformersv1.KubeletConfigInformer, mcopInformer mcopinformersv1.MachineConfigurationInformer, osImageStreamInformer mcfginformersv1.OSImageStreamInformer, + imgInformer cligoinformersv1.ImageInformer, + idmsInformer cligoinformersv1.ImageDigestMirrorSetInformer, + itmsInformer cligoinformersv1.ImageTagMirrorSetInformer, + icspInformer operatorinformersv1alpha1.ImageContentSourcePolicyInformer, kubeClient clientset.Interface, mcfgClient mcfgclientset.Interface, featureGatesHandler ctrlcommon.FeatureGatesHandler, @@ -115,6 +138,13 @@ func New( workqueue.DefaultTypedControllerRateLimiter[string](), workqueue.TypedRateLimitingQueueConfig[string]{Name: "machineconfigcontroller-rendercontroller"}), fgHandler: featureGatesHandler, + imageInspector: &defaultOSImageStreamClassInspector{ + kubeClient: kubeClient, + imgLister: imgInformer.Lister(), + icspLister: icspInformer.Lister(), + idmsLister: idmsInformer.Lister(), + itmsLister: itmsInformer.Lister(), + }, } mcpInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ @@ -143,6 +173,10 @@ func New( ctrl.mckListerSynced = mckInformer.Informer().HasSynced ctrl.mcopLister = mcopInformer.Lister() ctrl.mcopListerSynced = mcopInformer.Informer().HasSynced + ctrl.imgListerSynced = imgInformer.Informer().HasSynced + ctrl.icspListerSynced = icspInformer.Informer().HasSynced + ctrl.idmsListerSynced = idmsInformer.Informer().HasSynced + ctrl.itmsListerSynced = itmsInformer.Informer().HasSynced if osImageStreamInformer != nil && osimagestream.IsFeatureEnabled(ctrl.fgHandler) { ctrl.osImageStreamLister = osImageStreamInformer.Lister() @@ -159,7 +193,10 @@ func (ctrl *Controller) Run(workers int, stopCh <-chan struct{}) { defer utilruntime.HandleCrash() defer ctrl.queue.ShutDown() - listerCaches := []cache.InformerSynced{ctrl.mcpListerSynced, ctrl.mcListerSynced, ctrl.ccListerSynced} + listerCaches := []cache.InformerSynced{ + ctrl.mcpListerSynced, ctrl.mcListerSynced, ctrl.ccListerSynced, + ctrl.imgListerSynced, ctrl.icspListerSynced, ctrl.idmsListerSynced, ctrl.itmsListerSynced, + } // OSImageStreams and MCPs fetched only if FeatureGateOSStreams active if ctrl.osImageStreamListerSynced != nil { @@ -702,6 +739,16 @@ func (ctrl *Controller) syncGeneratedMachineConfig(pool *mcfgv1.MachineConfigPoo return fmt.Errorf("size validation failed: %w", err) } + // When OSImageStream is not in use, check for runc on RHEL 10 via OSImageURL + // inspection. When OSImageStream IS in use, the check is already performed + // inside generateRenderedMachineConfig() via validateNoRuncOnRHEL10(). + // TODO(OCP 5.3): Remove when runc is removed. + if osImageStreamSet == nil { + if err := ctrl.validateNoRuncFromOSImageURL(pool, generated, cc); err != nil { + return err + } + } + // Collect metric when OSImageURL was overridden var isOSImageURLOverridden bool if generated.Spec.OSImageURL != ctrlcommon.GetBaseImageContainer(&cc.Spec, osImageStreamSet) { @@ -812,7 +859,7 @@ func generateRenderedMachineConfig(pool *mcfgv1.MachineConfigPool, configs []*mc merged.Annotations[ctrlcommon.OSImageURLOverriddenKey] = "true" // Log a warning if the osImageURL is set using a tag instead of a digest if !strings.Contains(merged.Spec.OSImageURL, "sha256:") { - klog.Warningf("OSImageURL %q for MachineConfig %s is set using a tag instead of a digest. It is highly recommended to use a digest", merged.Spec.OSImageURL, merged.Name) + klog.Warningf("OSImageURL for MachineConfig %s is set using a tag instead of a digest. It is highly recommended to use a digest", merged.Name) } // If the cluster admin overrides the OSImageURL field, it means they want @@ -922,7 +969,7 @@ func getOSImageStreamNameForPoolBootstrap(pool *mcfgv1.MachineConfigPool, pools // RunBootstrap runs the render controller in bootstrap mode. // For each pool, it matches the machineconfigs based on label selector and // returns the generated machineconfigs and pool with CurrentMachineConfig status field set. -func RunBootstrap(pools []*mcfgv1.MachineConfigPool, configs []*mcfgv1.MachineConfig, cconfig *mcfgv1.ControllerConfig, osImageStream *mcfgv1.OSImageStream) ([]*mcfgv1.MachineConfigPool, []*mcfgv1.MachineConfig, error) { +func RunBootstrap(pools []*mcfgv1.MachineConfigPool, configs []*mcfgv1.MachineConfig, cconfig *mcfgv1.ControllerConfig, osImageStream *mcfgv1.OSImageStream, baseStreamClass string) ([]*mcfgv1.MachineConfigPool, []*mcfgv1.MachineConfig, error) { var ( opools []*mcfgv1.MachineConfigPool oconfigs []*mcfgv1.MachineConfig @@ -947,6 +994,16 @@ func RunBootstrap(pools []*mcfgv1.MachineConfigPool, configs []*mcfgv1.MachineCo return nil, nil, err } + // When OSImageStream is not in use, check for runc on RHEL 10 using the + // base stream class determined by the caller from OSImageURL inspection. + // TODO(OCP 5.3): Remove when runc is removed. + if osImageStreamSet == nil && osimagestream.IsRHEL10Stream(baseStreamClass) { + if err := checkRuncBlockedOnStream(pool.Name, generated, + fmt.Sprintf("OS image with stream class %q", baseStreamClass)); err != nil { + return nil, nil, err + } + } + source := getMachineConfigRefs(configs) pool.Spec.Configuration.Name = generated.Name @@ -959,6 +1016,31 @@ func RunBootstrap(pools []*mcfgv1.MachineConfigPool, configs []*mcfgv1.MachineCo return opools, oconfigs, nil } +// runcBlockedError returns an actionable error message explaining that runc is +// not available on the target stream and how to migrate to crun. +// TODO(OCP 5.3): Remove when runc is removed. +func runcBlockedError(poolName, streamIdentifier string) error { + return fmt.Errorf( + "MachineConfigPool %s targets %s where runc is not available. "+ + "To unblock, migrate to crun by removing any ContainerRuntimeConfig that sets defaultRuntime to runc, "+ + "and removing any MachineConfig that sets default_runtime = \"runc\" in CRI-O configuration under /etc/crio/crio.conf.d/", + poolName, streamIdentifier) +} + +// checkRuncBlockedOnStream returns an error if the MachineConfig uses runc as +// the default container runtime on a stream where runc is not available. +// TODO(OCP 5.3): Remove when runc is removed. +func checkRuncBlockedOnStream(poolName string, mc *mcfgv1.MachineConfig, streamIdentifier string) error { + runcMCName, err := ctrlcommon.DetectRuncInMachineConfig(mc) + if err != nil { + return fmt.Errorf("failed to check runc in generated MachineConfig for pool %s: %w", poolName, err) + } + if runcMCName != "" { + return runcBlockedError(poolName, streamIdentifier) + } + return nil +} + // validateNoRuncOnRHEL10 returns an error if the generated MachineConfig uses runc // as the default container runtime and the pool targets a RHEL 10 / CentOS 10 OS // image stream. runc is not available on RHCOS 10; clusters must migrate to crun @@ -971,19 +1053,120 @@ func validateNoRuncOnRHEL10(poolName string, mc *mcfgv1.MachineConfig, osImageSt if !osimagestream.IsRHEL10Stream(osImageStreamSet.Name) { return nil } + return checkRuncBlockedOnStream(poolName, mc, fmt.Sprintf("OS image stream %q", osImageStreamSet.Name)) +} - runcMCName, err := ctrlcommon.DetectRuncInMachineConfig(mc) +// getCachedStreamClass returns the cached stream class from the pool's current +// rendered MachineConfig, if the OSImageURL has not changed since it was inspected. +// TODO(OCP 5.3): Remove when runc is removed. +func (ctrl *Controller) getCachedStreamClass(pool *mcfgv1.MachineConfigPool, osImageURL string) string { + if pool.Spec.Configuration.Name == "" { + return "" + } + + currentMC, err := ctrl.mcLister.Get(pool.Spec.Configuration.Name) if err != nil { - return fmt.Errorf("failed to check runc in generated MachineConfig for pool %s: %w", poolName, err) + klog.V(4).Infof("Pool %s: could not get current rendered MC %s for stream class cache: %v", + pool.Name, pool.Spec.Configuration.Name, err) + return "" } - if runcMCName != "" { - return fmt.Errorf( - "MachineConfigPool %s targets OS image stream %q where runc is not available. "+ - "To unblock, migrate to crun by removing any ContainerRuntimeConfig that sets defaultRuntime to runc, "+ - "and removing any MachineConfig that sets default_runtime = \"runc\" in CRI-O configuration under /etc/crio/crio.conf.d/", - poolName, osImageStreamSet.Name) + + cachedURL := currentMC.Annotations[ctrlcommon.OSImageURLInspectedAnnotationKey] + if cachedURL == osImageURL { + return currentMC.Annotations[ctrlcommon.OSImageStreamClassAnnotationKey] } - return nil + return "" +} + +// validateNoRuncFromOSImageURL checks whether the generated MachineConfig uses +// runc on a RHEL 10 image when OSImageStream is not available. It determines the +// OS version by inspecting the container image's io.openshift.os.streamclass label, +// caching the result as annotations on the rendered MachineConfig. +// TODO(OCP 5.3): Remove when runc is removed. +func (ctrl *Controller) validateNoRuncFromOSImageURL(pool *mcfgv1.MachineConfigPool, generated *mcfgv1.MachineConfig, cc *mcfgv1.ControllerConfig) error { + osImageURL := generated.Spec.OSImageURL + if osImageURL == "" { + return nil + } + + // Short-circuit: only inspect the image if runc is actually in use. + runcMCName, err := ctrlcommon.DetectRuncInMachineConfig(generated) + if err != nil { + return fmt.Errorf("pool %s: failed to check runc in generated MachineConfig: %w", pool.Name, err) + } + if runcMCName == "" { + return nil + } + + // Try the annotation cache first to avoid a registry round-trip on every sync. + streamClass := ctrl.getCachedStreamClass(pool, osImageURL) + + if streamClass == "" { + // Cache miss or OSImageURL changed — inspect the container image. + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + + streamClass, err = ctrl.imageInspector.GetStreamClass(ctx, osImageURL, cc) + if err != nil { + return fmt.Errorf("pool %s: failed to inspect OS image for stream class: %w", pool.Name, err) + } + if streamClass == "" { + klog.V(4).Infof("Pool %s: OS image has no stream class label; skipping runc check", pool.Name) + return nil + } + } + + // Cache the result on the rendered MC so subsequent syncs skip the registry call. + generated.Annotations[ctrlcommon.OSImageStreamClassAnnotationKey] = streamClass + generated.Annotations[ctrlcommon.OSImageURLInspectedAnnotationKey] = osImageURL + + if !osimagestream.IsRHEL10Stream(streamClass) { + return nil + } + + return runcBlockedError(pool.Name, + fmt.Sprintf("OS image (stream class %q)", streamClass)) +} + +// defaultOSImageStreamClassInspector is the production implementation of +// osImageStreamClassInspector. It fetches the cluster pull secret and registry +// mirror rules (ICSP/IDMS/ITMS), builds an authenticated image system context, +// and inspects the container image's io.openshift.os.streamclass label. +// TODO(OCP 5.3): Remove when runc is removed. +type defaultOSImageStreamClassInspector struct { + kubeClient clientset.Interface + imgLister cligolistersv1.ImageLister + icspLister operatorlistersv1alpha1.ImageContentSourcePolicyLister + idmsLister cligolistersv1.ImageDigestMirrorSetLister + itmsLister cligolistersv1.ImageTagMirrorSetLister +} + +// GetStreamClass fetches the cluster pull secret and registry mirror rules, +// then inspects the container image to return its OS stream class label. +func (d *defaultOSImageStreamClassInspector) GetStreamClass(ctx context.Context, imageURL string, cc *mcfgv1.ControllerConfig) (string, error) { + secret, err := d.kubeClient.CoreV1().Secrets(ctrlcommon.OpenshiftConfigNamespace).Get(ctx, ctrlcommon.GlobalPullSecretName, metav1.GetOptions{}) + if err != nil { + return "", fmt.Errorf("failed to get pull secret: %w", err) + } + + imgCfg, err := d.imgLister.Get("cluster") + if err != nil && !apierrors.IsNotFound(err) { + return "", fmt.Errorf("failed to get cluster image config: %w", err) + } + icspRules, err := d.icspLister.List(labels.Everything()) + if err != nil { + return "", fmt.Errorf("failed to list ICSP rules: %w", err) + } + idmsRules, err := d.idmsLister.List(labels.Everything()) + if err != nil { + return "", fmt.Errorf("failed to list IDMS rules: %w", err) + } + itmsRules, err := d.itmsLister.List(labels.Everything()) + if err != nil { + return "", fmt.Errorf("failed to list ITMS rules: %w", err) + } + + return osimagestream.InspectStreamClassWithMirrors(ctx, secret, cc, imgCfg, icspRules, idmsRules, itmsRules, imageURL) } // getMachineConfigsForPool is called by RunBootstrap and returns configs that match label from configs for a pool. diff --git a/pkg/controller/render/render_controller_test.go b/pkg/controller/render/render_controller_test.go index d8ed8f155a..18fa1257b2 100644 --- a/pkg/controller/render/render_controller_test.go +++ b/pkg/controller/render/render_controller_test.go @@ -1,6 +1,7 @@ package render import ( + "context" "fmt" "reflect" "testing" @@ -10,6 +11,8 @@ import ( ign3types "github.com/coreos/ignition/v2/config/v3_5/types" apicfgv1 "github.com/openshift/api/config/v1" configv1 "github.com/openshift/api/config/v1" + fakeconfigv1client "github.com/openshift/client-go/config/clientset/versioned/fake" + configv1informer "github.com/openshift/client-go/config/informers/externalversions" mcopfake "github.com/openshift/client-go/operator/clientset/versioned/fake" operatorinformer "github.com/openshift/client-go/operator/informers/externalversions" "github.com/stretchr/testify/assert" @@ -79,23 +82,37 @@ func (f *fixture) newController() *Controller { f.oclient = mcopfake.NewSimpleClientset(f.oObjects...) i := informers.NewSharedInformerFactory(f.client, noResyncPeriodFunc()) oi := operatorinformer.NewSharedInformerFactory(f.oclient, noResyncPeriodFunc()) + ci := configv1informer.NewSharedInformerFactory(fakeconfigv1client.NewSimpleClientset(), noResyncPeriodFunc()) c := New(i.Machineconfiguration().V1().MachineConfigPools(), i.Machineconfiguration().V1().MachineConfigs(), i.Machineconfiguration().V1().ControllerConfigs(), i.Machineconfiguration().V1().ContainerRuntimeConfigs(), i.Machineconfiguration().V1().KubeletConfigs(), oi.Operator().V1().MachineConfigurations(), - i.Machineconfiguration().V1().OSImageStreams(), k8sfake.NewSimpleClientset(), f.client, f.fgHandler) + i.Machineconfiguration().V1().OSImageStreams(), + ci.Config().V1().Images(), + ci.Config().V1().ImageDigestMirrorSets(), + ci.Config().V1().ImageTagMirrorSets(), + oi.Operator().V1alpha1().ImageContentSourcePolicies(), + k8sfake.NewSimpleClientset(), f.client, f.fgHandler) c.mcpListerSynced = alwaysReady c.mcListerSynced = alwaysReady c.ccListerSynced = alwaysReady c.crcListerSynced = alwaysReady c.mckListerSynced = alwaysReady + c.imgListerSynced = alwaysReady + c.icspListerSynced = alwaysReady + c.idmsListerSynced = alwaysReady + c.itmsListerSynced = alwaysReady c.eventRecorder = ctrlcommon.NamespacedEventRecorder(&record.FakeRecorder{}) stopCh := make(chan struct{}) defer close(stopCh) i.Start(stopCh) i.WaitForCacheSync(stopCh) + oi.Start(stopCh) + oi.WaitForCacheSync(stopCh) + ci.Start(stopCh) + ci.WaitForCacheSync(stopCh) for _, c := range f.ccLister { i.Machineconfiguration().V1().ControllerConfigs().Informer().GetIndexer().Add(c) @@ -281,11 +298,13 @@ func TestCreatesGeneratedMachineConfig(t *testing.T) { mcp := helpers.NewMachineConfigPool("test-cluster-master", helpers.MasterSelector, nil, "") files := []ign3types.File{{ Node: ign3types.Node{ - Path: "/dummy/0", + Path: "/dummy/0", + Overwrite: helpers.BoolToPtr(false), }, }, { Node: ign3types.Node{ - Path: "/dummy/1", + Path: "/dummy/1", + Overwrite: helpers.BoolToPtr(false), }, }} mcs := []*mcfgv1.MachineConfig{ @@ -1015,6 +1034,240 @@ func makeCRIODropIn(runtime string) string { return "[crio.runtime]\ndefault_runtime = \"" + runtime + "\"\n" } +type mockOSImageStreamClassInspector struct { + streamClass string + err error +} + +// GetStreamClass returns the preconfigured stream class and error for testing. +func (m *mockOSImageStreamClassInspector) GetStreamClass(_ context.Context, _ string, _ *mcfgv1.ControllerConfig) (string, error) { + return m.streamClass, m.err +} + +// TestValidateNoRuncFromOSImageURL verifies that runc is blocked on RHEL 10 streams detected via OSImageURL inspection. +func TestValidateNoRuncFromOSImageURL(t *testing.T) { + tests := []struct { + name string + mc *mcfgv1.MachineConfig + streamClass string + inspectErr error + expectError bool + expectErrMsg string + expectAnnotation bool + }{ + { + name: "empty OSImageURL skips check", + mc: helpers.NewMachineConfig("rendered-worker", nil, "", nil), + streamClass: "rhel-10", + expectError: false, + }, + { + name: "runc on RHEL 10 should error", + mc: func() *mcfgv1.MachineConfig { + mc := helpers.NewMachineConfig("rendered-worker", nil, "", []ign3types.File{ + helpers.CreateEncodedIgn3File("/etc/crio/crio.conf.d/00-default", makeCRIODropIn("runc"), 0644), + }) + mc.Spec.OSImageURL = "quay.io/openshift/rhcos@sha256:abc123" + return mc + }(), + streamClass: "rhel-10", + expectError: true, + expectAnnotation: true, + }, + { + name: "crun on RHEL 10 should succeed", + mc: func() *mcfgv1.MachineConfig { + mc := helpers.NewMachineConfig("rendered-worker", nil, "", []ign3types.File{ + helpers.CreateEncodedIgn3File("/etc/crio/crio.conf.d/00-default", makeCRIODropIn("crun"), 0644), + }) + mc.Spec.OSImageURL = "quay.io/openshift/rhcos@sha256:abc123" + return mc + }(), + streamClass: "rhel-10", + expectError: false, + }, + { + name: "runc on RHEL 9 should succeed", + mc: func() *mcfgv1.MachineConfig { + mc := helpers.NewMachineConfig("rendered-worker", nil, "", []ign3types.File{ + helpers.CreateEncodedIgn3File("/etc/crio/crio.conf.d/00-default", makeCRIODropIn("runc"), 0644), + }) + mc.Spec.OSImageURL = "quay.io/openshift/rhcos@sha256:abc123" + return mc + }(), + streamClass: "rhel-9", + expectError: false, + expectAnnotation: true, + }, + { + name: "runc on CentOS 10 should error", + mc: func() *mcfgv1.MachineConfig { + mc := helpers.NewMachineConfig("rendered-worker", nil, "", []ign3types.File{ + helpers.CreateEncodedIgn3File("/etc/crio/crio.conf.d/00-default", makeCRIODropIn("runc"), 0644), + }) + mc.Spec.OSImageURL = "quay.io/openshift/scos@sha256:abc123" + return mc + }(), + streamClass: "centos-10", + expectError: true, + expectAnnotation: true, + }, + { + name: "inspection failure should fail closed", + mc: func() *mcfgv1.MachineConfig { + mc := helpers.NewMachineConfig("rendered-worker", nil, "", []ign3types.File{ + helpers.CreateEncodedIgn3File("/etc/crio/crio.conf.d/00-default", makeCRIODropIn("runc"), 0644), + }) + mc.Spec.OSImageURL = "quay.io/openshift/rhcos@sha256:abc123" + return mc + }(), + inspectErr: fmt.Errorf("network error"), + expectError: true, + expectErrMsg: "network error", + }, + { + name: "no stream class label should succeed", + mc: func() *mcfgv1.MachineConfig { + mc := helpers.NewMachineConfig("rendered-worker", nil, "", []ign3types.File{ + helpers.CreateEncodedIgn3File("/etc/crio/crio.conf.d/00-default", makeCRIODropIn("runc"), 0644), + }) + mc.Spec.OSImageURL = "quay.io/openshift/rhcos@sha256:abc123" + return mc + }(), + streamClass: "", + expectError: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + pool := &mcfgv1.MachineConfigPool{ + ObjectMeta: metav1.ObjectMeta{Name: "worker"}, + } + cc := newControllerConfig(ctrlcommon.ControllerConfigName) + + ctrl := &Controller{ + imageInspector: &mockOSImageStreamClassInspector{ + streamClass: tt.streamClass, + err: tt.inspectErr, + }, + mcLister: newFakeMCLister(t, nil), + } + + err := ctrl.validateNoRuncFromOSImageURL(pool, tt.mc, cc) + if tt.expectError { + require.Error(t, err) + if tt.expectErrMsg != "" { + assert.Contains(t, err.Error(), tt.expectErrMsg) + } else { + assert.Contains(t, err.Error(), "runc") + assert.Contains(t, err.Error(), "not available") + } + } else { + require.NoError(t, err) + } + + if tt.expectAnnotation { + assert.Equal(t, tt.streamClass, tt.mc.Annotations[ctrlcommon.OSImageStreamClassAnnotationKey]) + assert.Equal(t, tt.mc.Spec.OSImageURL, tt.mc.Annotations[ctrlcommon.OSImageURLInspectedAnnotationKey]) + } + }) + } +} + +// TestGetCachedStreamClass verifies that stream class annotation caching avoids redundant image inspections. +func TestGetCachedStreamClass(t *testing.T) { + tests := []struct { + name string + pool *mcfgv1.MachineConfigPool + osImageURL string + currentMC *mcfgv1.MachineConfig + wantStream string + }{ + { + name: "no current config name", + pool: &mcfgv1.MachineConfigPool{ + ObjectMeta: metav1.ObjectMeta{Name: "worker"}, + }, + osImageURL: "quay.io/openshift/rhcos@sha256:abc123", + wantStream: "", + }, + { + name: "matching OSImageURL returns cached stream class", + pool: &mcfgv1.MachineConfigPool{ + ObjectMeta: metav1.ObjectMeta{Name: "worker"}, + Spec: mcfgv1.MachineConfigPoolSpec{ + Configuration: mcfgv1.MachineConfigPoolStatusConfiguration{ + ObjectReference: corev1.ObjectReference{Name: "rendered-worker-abc"}, + }, + }, + }, + osImageURL: "quay.io/openshift/rhcos@sha256:abc123", + currentMC: &mcfgv1.MachineConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "rendered-worker-abc", + Annotations: map[string]string{ + ctrlcommon.OSImageURLInspectedAnnotationKey: "quay.io/openshift/rhcos@sha256:abc123", + ctrlcommon.OSImageStreamClassAnnotationKey: "rhel-10", + }, + }, + }, + wantStream: "rhel-10", + }, + { + name: "different OSImageURL invalidates cache", + pool: &mcfgv1.MachineConfigPool{ + ObjectMeta: metav1.ObjectMeta{Name: "worker"}, + Spec: mcfgv1.MachineConfigPoolSpec{ + Configuration: mcfgv1.MachineConfigPoolStatusConfiguration{ + ObjectReference: corev1.ObjectReference{Name: "rendered-worker-abc"}, + }, + }, + }, + osImageURL: "quay.io/openshift/rhcos@sha256:def456", + currentMC: &mcfgv1.MachineConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "rendered-worker-abc", + Annotations: map[string]string{ + ctrlcommon.OSImageURLInspectedAnnotationKey: "quay.io/openshift/rhcos@sha256:abc123", + ctrlcommon.OSImageStreamClassAnnotationKey: "rhel-10", + }, + }, + }, + wantStream: "", + }, + { + name: "current MC not found", + pool: &mcfgv1.MachineConfigPool{ + ObjectMeta: metav1.ObjectMeta{Name: "worker"}, + Spec: mcfgv1.MachineConfigPoolSpec{ + Configuration: mcfgv1.MachineConfigPoolStatusConfiguration{ + ObjectReference: corev1.ObjectReference{Name: "rendered-worker-missing"}, + }, + }, + }, + osImageURL: "quay.io/openshift/rhcos@sha256:abc123", + wantStream: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var mcs []*mcfgv1.MachineConfig + if tt.currentMC != nil { + mcs = append(mcs, tt.currentMC) + } + + ctrl := &Controller{ + mcLister: newFakeMCLister(t, mcs), + } + + got := ctrl.getCachedStreamClass(tt.pool, tt.osImageURL) + assert.Equal(t, tt.wantStream, got) + }) + } +} + func TestValidateNoRuncOnRHEL10(t *testing.T) { tests := []struct { name string @@ -1130,8 +1383,106 @@ func TestRunBootstrapBlocksRuncOnRHEL10(t *testing.T) { []*mcfgv1.MachineConfig{runcMC}, cc, osImageStream, + "", ) require.Error(t, err) assert.Contains(t, err.Error(), "runc") assert.Contains(t, err.Error(), "not available") } + +// TestRunBootstrapBlocksRuncOnRHEL10ViaStreamClass verifies that bootstrap rendering rejects runc on RHEL 10 streams. +func TestRunBootstrapBlocksRuncOnRHEL10ViaStreamClass(t *testing.T) { + pool := &mcfgv1.MachineConfigPool{ + ObjectMeta: metav1.ObjectMeta{Name: "worker"}, + Spec: mcfgv1.MachineConfigPoolSpec{ + MachineConfigSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"machineconfiguration.openshift.io/role": "worker"}, + }, + }, + } + + cc := newControllerConfig(ctrlcommon.ControllerConfigName) + + runcMC := helpers.NewMachineConfig("99-worker-runc", + map[string]string{"machineconfiguration.openshift.io/role": "worker"}, + "", + []ign3types.File{ + helpers.CreateEncodedIgn3File("/etc/crio/crio.conf.d/99-runc", + makeCRIODropIn("runc"), 0644), + }) + + crunMC := helpers.NewMachineConfig("99-worker-crun", + map[string]string{"machineconfiguration.openshift.io/role": "worker"}, + "", + []ign3types.File{ + helpers.CreateEncodedIgn3File("/etc/crio/crio.conf.d/99-crun", + makeCRIODropIn("crun"), 0644), + }) + + tests := []struct { + name string + mc *mcfgv1.MachineConfig + baseStreamClass string + expectError bool + }{ + { + name: "runc on RHEL 10 via stream class should error", + mc: runcMC, + baseStreamClass: "rhel-10", + expectError: true, + }, + { + name: "crun on RHEL 10 via stream class should succeed", + mc: crunMC, + baseStreamClass: "rhel-10", + expectError: false, + }, + { + name: "runc on RHEL 9 via stream class should succeed", + mc: runcMC, + baseStreamClass: "rhel-9", + expectError: false, + }, + { + name: "runc with empty stream class should succeed", + mc: runcMC, + baseStreamClass: "", + expectError: false, + }, + { + name: "runc on CentOS 10 via stream class should error", + mc: runcMC, + baseStreamClass: "centos-10", + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, _, err := RunBootstrap( + []*mcfgv1.MachineConfigPool{pool}, + []*mcfgv1.MachineConfig{tt.mc}, + cc, + nil, + tt.baseStreamClass, + ) + if tt.expectError { + require.Error(t, err) + assert.Contains(t, err.Error(), "runc") + assert.Contains(t, err.Error(), "not available") + } else { + require.NoError(t, err) + } + }) + } +} + +// newFakeMCLister creates a fake MachineConfig lister from a slice of MachineConfigs. +func newFakeMCLister(t *testing.T, mcs []*mcfgv1.MachineConfig) mcfglistersv1.MachineConfigLister { + t.Helper() + indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{}) + for _, mc := range mcs { + require.NoError(t, indexer.Add(mc)) + } + return mcfglistersv1.NewMachineConfigLister(indexer) +} diff --git a/pkg/osimagestream/inspector.go b/pkg/osimagestream/inspector.go index 27ea843b21..f910f14083 100644 --- a/pkg/osimagestream/inspector.go +++ b/pkg/osimagestream/inspector.go @@ -4,12 +4,17 @@ import ( "archive/tar" "context" "errors" + "fmt" "net" "strings" "github.com/containers/common/pkg/retry" "github.com/containers/image/v5/types" + apicfgv1 "github.com/openshift/api/config/v1" + mcfgv1 "github.com/openshift/api/machineconfiguration/v1" + apioperatorsv1alpha1 "github.com/openshift/api/operator/v1alpha1" "github.com/openshift/machine-config-operator/pkg/imageutils" + corev1 "k8s.io/api/core/v1" ) // isNetworkErrorRetryable checks if an error is a network-related error that should be retried. @@ -89,6 +94,68 @@ func (i *ImagesInspectorImpl) FetchImageFile(ctx context.Context, image, path st }, newImageInspectionRetryOptions()) } +// InspectStreamClass inspects a container image and returns its OS stream class +// (e.g. "rhel-9", "rhel-10") from the image's labels. Returns ("", nil) when +// the image has no stream class label. +// TODO(OCP 5.3): Remove when runc is removed. +func InspectStreamClass(ctx context.Context, sysCtx *types.SystemContext, imageURL string) (string, error) { + inspector := NewImagesInspector(sysCtx) + results, err := inspector.Inspect(ctx, imageURL) + if err != nil { + return "", fmt.Errorf("failed to inspect OS image: %w", err) + } + if len(results) == 0 { + return "", fmt.Errorf("no inspection result for OS image") + } + if results[0].Error != nil { + return "", fmt.Errorf("failed to inspect OS image: %w", results[0].Error) + } + if results[0].InspectInfo == nil { + return "", nil + } + + extractor := NewImageStreamExtractor() + imageData := extractor.GetImageData(imageURL, results[0].InspectInfo.Labels) + if imageData == nil { + return "", nil + } + return imageData.Stream, nil +} + +// InspectStreamClassWithMirrors builds an image system context with registry mirror +// rules applied, then inspects the container image to determine its OS stream class. +// TODO(OCP 5.3): Remove when runc is removed. +func InspectStreamClassWithMirrors( + ctx context.Context, + secret *corev1.Secret, + cc *mcfgv1.ControllerConfig, + imgCfg *apicfgv1.Image, + icspRules []*apioperatorsv1alpha1.ImageContentSourcePolicy, + idmsRules []*apicfgv1.ImageDigestMirrorSet, + itmsRules []*apicfgv1.ImageTagMirrorSet, + imageURL string, +) (string, error) { + builder := imageutils.NewSysContextBuilder(). + WithSecret(secret). + WithControllerConfig(cc) + + registriesConfig, err := imageutils.GenerateRegistriesConfig(imgCfg, icspRules, idmsRules, itmsRules) + if err != nil { + return "", fmt.Errorf("failed to generate registries config: %w", err) + } + if registriesConfig != nil { + builder.WithRegistriesConfig(registriesConfig) + } + + sysCtx, err := builder.Build() + if err != nil { + return "", fmt.Errorf("failed to build image system context: %w", err) + } + defer sysCtx.Cleanup() + + return InspectStreamClass(ctx, sysCtx.SysContext, imageURL) +} + // ImagesInspectorFactory creates ImagesInspector instances for different system contexts. type ImagesInspectorFactory interface { // ForContext creates an ImagesInspector for the given system context. diff --git a/test/e2e-bootstrap/bootstrap_test.go b/test/e2e-bootstrap/bootstrap_test.go index ac0c85d200..ad3c1e0d0f 100644 --- a/test/e2e-bootstrap/bootstrap_test.go +++ b/test/e2e-bootstrap/bootstrap_test.go @@ -613,6 +613,10 @@ func createControllers(ctx *ctrlcommon.ControllerContext) []ctrlcommon.Controlle ctx.InformerFactory.Machineconfiguration().V1().KubeletConfigs(), ctx.OperatorInformerFactory.Operator().V1().MachineConfigurations(), ctx.InformerFactory.Machineconfiguration().V1().OSImageStreams(), + ctx.ConfigInformerFactory.Config().V1().Images(), + ctx.ConfigInformerFactory.Config().V1().ImageDigestMirrorSets(), + ctx.ConfigInformerFactory.Config().V1().ImageTagMirrorSets(), + ctx.OperatorInformerFactory.Operator().V1alpha1().ImageContentSourcePolicies(), ctx.ClientBuilder.KubeClientOrDie("render-controller"), ctx.ClientBuilder.MachineConfigClientOrDie("render-controller"), ctx.FeatureGatesHandler,