Merge pull request #5545 from bfournie/AGENT-1412-iri-deletion-guard

AGENT-1412: Prevent deletion of InternalReleaseImage when in use

Author: openshift-merge-bot[bot]

cmd/machine-config-controller/start.go

@@ -143,6 +143,7 @@ func runStartCmd(_ *cobra.Command, _ []string) {
 				ctrlctx.InformerFactory.Machineconfiguration().V1alpha1().InternalReleaseImages(),
 				ctrlctx.InformerFactory.Machineconfiguration().V1().ControllerConfigs(),
 				ctrlctx.InformerFactory.Machineconfiguration().V1().MachineConfigs(),
+				ctrlctx.ConfigInformerFactory.Config().V1().ClusterVersions(),
 				ctrlctx.ClientBuilder.KubeClientOrDie("internalreleaseimage-controller"),
 				ctrlctx.ClientBuilder.MachineConfigClientOrDie("internalreleaseimage-controller"))
 

cmd/machine-config-operator/start.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"os"
 
+	features "github.com/openshift/api/features"
 	"github.com/openshift/machine-config-operator/cmd/common"
 	"github.com/openshift/machine-config-operator/internal/clients"
 	ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common"
@@ -66,6 +67,13 @@ func runStartCmd(_ *cobra.Command, _ []string) {
 			klog.Fatal(fmt.Errorf("failed to connect to feature gates %w", fgErr))
 		}
 
+		// Only pass IRI informer when the feature gate is enabled to avoid
+		// watching for a CRD that may not exist
+		var iriInformer = ctrlctx.InformerFactory.Machineconfiguration().V1alpha1().InternalReleaseImages()
+		if !ctrlctx.FeatureGatesHandler.Enabled(features.FeatureGateNoRegistryClusterInstall) {
+			iriInformer = nil
+		}
+
 		controller := operator.New(
 			ctrlcommon.MCONamespace, componentName,
 			startOpts.imagesFile,
@@ -111,6 +119,7 @@ func runStartCmd(_ *cobra.Command, _ []string) {
 			ctrlctx.NamespacedInformerFactory.Machineconfiguration().V1().MachineOSConfigs(),
 			ctrlctx.ConfigInformerFactory.Config().V1().ClusterVersions(),
 			ctrlctx.InformerFactory.Machineconfiguration().V1alpha1().OSImageStreams(),
+			iriInformer,
 			ctrlctx,
 		)
 

manifests/machineconfigcontroller/internalreleaseimage-deletion-guard-validatingadmissionpolicy.yaml

@@ -0,0 +1,22 @@
+apiVersion: admissionregistration.k8s.io/v1
+kind: ValidatingAdmissionPolicy
+metadata:
+  name: "internalreleaseimage-deletion-guard"
+spec:
+  failurePolicy: Fail
+  paramKind:
+    apiVersion: config.openshift.io/v1
+    kind: ClusterVersion
+  matchConstraints:
+    matchPolicy: Equivalent
+    namespaceSelector: {}
+    objectSelector: {}
+    resourceRules:
+    - apiGroups:   ["machineconfiguration.openshift.io"]
+      apiVersions: ["v1alpha1"]
+      operations:  ["DELETE"]
+      resources:   ["internalreleaseimages"]
+      scope: "*"
+  validations:
+    - expression: "!oldObject.status.releases.exists(r, has(r.image) && r.image == params.status.desired.image)"
+      message: "Cannot delete InternalReleaseImage while the cluster is using a release bundle from this resource. The current cluster release image matches a release stored in this InternalReleaseImage. Please upgrade or downgrade to a different release before deletion."

manifests/machineconfigcontroller/internalreleaseimage-deletion-guard-validatingadmissionpolicybinding.yaml

@@ -0,0 +1,10 @@
+apiVersion: admissionregistration.k8s.io/v1
+kind: ValidatingAdmissionPolicyBinding
+metadata:
+  name: "internalreleaseimage-deletion-guard-binding"
+spec:
+  policyName: "internalreleaseimage-deletion-guard"
+  validationActions: [Deny]
+  paramRef:
+    name: "version"
+    parameterNotFoundAction: "Deny"

pkg/controller/internalreleaseimage/internalreleaseimage_controller.go

@@ -20,6 +20,8 @@ import (
 
 	mcfgv1 "github.com/openshift/api/machineconfiguration/v1"
 	mcfgv1alpha1 "github.com/openshift/api/machineconfiguration/v1alpha1"
+	configinformersv1 "github.com/openshift/client-go/config/informers/externalversions/config/v1"
+	configlistersv1 "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"
@@ -28,6 +30,7 @@ import (
 	mcfglistersv1alpha1 "github.com/openshift/client-go/machineconfiguration/listers/machineconfiguration/v1alpha1"
 	ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common"
 	templatectrl "github.com/openshift/machine-config-operator/pkg/controller/template"
+	"github.com/openshift/machine-config-operator/pkg/osimagestream"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
@@ -64,6 +67,9 @@ type Controller struct {
 	mcLister       mcfglistersv1.MachineConfigLister
 	mcListerSynced cache.InformerSynced
 
+	clusterVersionLister       configlistersv1.ClusterVersionLister
+	clusterVersionListerSynced cache.InformerSynced
+
 	queue workqueue.TypedRateLimitingInterface[string]
 }
 
@@ -72,6 +78,7 @@ func New(
 	iriInformer mcfginformersv1alpha1.InternalReleaseImageInformer,
 	ccInformer mcfginformersv1.ControllerConfigInformer,
 	mcInformer mcfginformersv1.MachineConfigInformer,
+	clusterVersionInformer configinformersv1.ClusterVersionInformer,
 	kubeClient clientset.Interface,
 	mcfgClient mcfgclientset.Interface,
 ) *Controller {
@@ -115,6 +122,9 @@ func New(
 	ctrl.mcLister = mcInformer.Lister()
 	ctrl.mcListerSynced = mcInformer.Informer().HasSynced
 
+	ctrl.clusterVersionLister = clusterVersionInformer.Lister()
+	ctrl.clusterVersionListerSynced = clusterVersionInformer.Informer().HasSynced
+
 	return ctrl
 }
 
@@ -123,7 +133,7 @@ func (ctrl *Controller) Run(workers int, stopCh <-chan struct{}) {
 	defer utilruntime.HandleCrash()
 	defer ctrl.queue.ShutDown()
 
-	if !cache.WaitForCacheSync(stopCh, ctrl.iriListerSynced) {
+	if !cache.WaitForCacheSync(stopCh, ctrl.iriListerSynced, ctrl.ccListerSynced, ctrl.mcListerSynced, ctrl.clusterVersionListerSynced) {
 		return
 	}
 
@@ -336,6 +346,61 @@ func (ctrl *Controller) syncInternalReleaseImage(key string) error {
 		}
 	}
 
+	// Initialize status if empty
+	if err := ctrl.initializeInternalReleaseImageStatus(iri); err != nil {
+		return err
+	}
+
+	return nil
+}
+
+// initializeInternalReleaseImageStatus initializes the status of an InternalReleaseImage
+// if it is empty. It populates the status with release bundle entries from the spec,
+// setting the Image field from the current ClusterVersion and adding initial conditions.
+func (ctrl *Controller) initializeInternalReleaseImageStatus(iri *mcfgv1alpha1.InternalReleaseImage) error {
+	// Only initialize if status is empty and spec has releases
+	if len(iri.Status.Releases) != 0 || len(iri.Spec.Releases) == 0 {
+		return nil
+	}
+
+	klog.V(4).Infof("Initializing status for InternalReleaseImage %s", iri.Name)
+
+	// Get the release payload image from ClusterVersion
+	releaseImage, err := osimagestream.GetReleasePayloadImage(ctrl.clusterVersionLister)
+	if err != nil {
+		return fmt.Errorf("error getting Release Image from ClusterVersion for InternalReleaseImage status initialization: %w", err)
+	}
+
+	// Build status releases from spec releases
+	statusReleases := make([]mcfgv1alpha1.InternalReleaseImageBundleStatus, 0, len(iri.Spec.Releases))
+	for _, specRelease := range iri.Spec.Releases {
+		statusRelease := mcfgv1alpha1.InternalReleaseImageBundleStatus{
+			Name:  specRelease.Name,
+			Image: releaseImage,
+			Conditions: []metav1.Condition{
+				{
+					Type:               string(mcfgv1alpha1.InternalReleaseImageConditionTypeAvailable),
+					Status:             metav1.ConditionTrue,
+					LastTransitionTime: metav1.Now(),
+					Reason:             "Installed",
+					Message:            "Release bundle is available",
+				},
+			},
+		}
+		statusReleases = append(statusReleases, statusRelease)
+	}
+
+	iri.Status.Releases = statusReleases
+
+	// Update the status subresource
+	if err := retry.RetryOnConflict(updateBackoff, func() error {
+		_, err := ctrl.client.MachineconfigurationV1alpha1().InternalReleaseImages().UpdateStatus(context.TODO(), iri, metav1.UpdateOptions{})
+		return err
+	}); err != nil {
+		return fmt.Errorf("failed to update InternalReleaseImage status: %w", err)
+	}
+
+	klog.V(2).Infof("Initialized status for InternalReleaseImage %s with %d releases", iri.Name, len(statusReleases))
 	return nil
 }
 

pkg/controller/internalreleaseimage/internalreleaseimage_controller_test.go

@@ -7,6 +7,7 @@ import (
 
 	mcfgv1 "github.com/openshift/api/machineconfiguration/v1"
 	mcfgv1alpha1 "github.com/openshift/api/machineconfiguration/v1alpha1"
+	configinformers "github.com/openshift/client-go/config/informers/externalversions"
 	"github.com/openshift/client-go/machineconfiguration/clientset/versioned/fake"
 	informers "github.com/openshift/client-go/machineconfiguration/informers/externalversions"
 	ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common"
@@ -18,6 +19,8 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	k8sfake "k8s.io/client-go/kubernetes/fake"
 	"k8s.io/client-go/tools/record"
+
+	fakeconfigv1client "github.com/openshift/client-go/config/clientset/versioned/fake"
 )
 
 func TestInternalReleaseImageCreate(t *testing.T) {
@@ -153,15 +156,18 @@ func TestInternalReleaseImageCreate(t *testing.T) {
 type fixture struct {
 	t *testing.T
 
-	client    *fake.Clientset
-	k8sClient *k8sfake.Clientset
-	iriLister []*mcfgv1alpha1.InternalReleaseImage
-	ccLister  []*mcfgv1.ControllerConfig
-	mcLister  []*mcfgv1.MachineConfig
-
-	controller *Controller
-	objects    []runtime.Object
-	k8sObjects []runtime.Object
+	client       *fake.Clientset
+	k8sClient    *k8sfake.Clientset
+	configClient *fakeconfigv1client.Clientset
+	iriLister    []*mcfgv1alpha1.InternalReleaseImage
+	ccLister     []*mcfgv1.ControllerConfig
+	mcLister     []*mcfgv1.MachineConfig
+
+	controller      *Controller
+	objects         []runtime.Object
+	k8sObjects      []runtime.Object
+	configObjects   []runtime.Object
+	configInformer  configinformers.SharedInformerFactory
 }
 
 func newFixture(t *testing.T, objects []runtime.Object) *fixture {
@@ -185,12 +191,15 @@ func (f *fixture) setupObjects(objs []runtime.Object) {
 func (f *fixture) newController() *Controller {
 	f.client = fake.NewSimpleClientset(f.objects...)
 	f.k8sClient = k8sfake.NewSimpleClientset(f.k8sObjects...)
+	f.configClient = fakeconfigv1client.NewSimpleClientset(f.configObjects...)
 	i := informers.NewSharedInformerFactory(f.client, func() time.Duration { return 0 }())
+	f.configInformer = configinformers.NewSharedInformerFactory(f.configClient, func() time.Duration { return 0 }())
 
 	c := New(
 		i.Machineconfiguration().V1alpha1().InternalReleaseImages(),
 		i.Machineconfiguration().V1().ControllerConfigs(),
 		i.Machineconfiguration().V1().MachineConfigs(),
+		f.configInformer.Config().V1().ClusterVersions(),
 		f.k8sClient,
 		f.client,
 	)
@@ -199,12 +208,15 @@ func (f *fixture) newController() *Controller {
 	c.iriListerSynced = alwaysReady
 	c.ccListerSynced = alwaysReady
 	c.mcListerSynced = alwaysReady
+	c.clusterVersionListerSynced = alwaysReady
 	c.eventRecorder = &record.FakeRecorder{}
 
 	stopCh := make(chan struct{})
 	defer close(stopCh)
 	i.Start(stopCh)
 	i.WaitForCacheSync(stopCh)
+	f.configInformer.Start(stopCh)
+	f.configInformer.WaitForCacheSync(stopCh)
 
 	for _, c := range f.iriLister {
 		i.Machineconfiguration().V1alpha1().InternalReleaseImages().Informer().GetIndexer().Add(c)

pkg/operator/operator.go

@@ -122,6 +122,7 @@ type Operator struct {
 	apiserverLister       configlistersv1.APIServerLister
 	clusterVersionLister  configlistersv1.ClusterVersionLister
 	osImageStreamLister   mcfglistersv1alpha1.OSImageStreamLister
+	iriLister             mcfglistersv1alpha1.InternalReleaseImageLister
 
 	crdListerSynced                  cache.InformerSynced
 	deployListerSynced               cache.InformerSynced
@@ -158,6 +159,7 @@ type Operator struct {
 	moscListerSynced                 cache.InformerSynced
 	apiserverListerSynced            cache.InformerSynced
 	osImageStreamListerSynced        cache.InformerSynced
+	iriListerSynced                  cache.InformerSynced
 
 	// queue only ever has one item, but it has nice error handling backoff/retry semantics
 	queue workqueue.TypedRateLimitingInterface[string]
@@ -216,6 +218,7 @@ func New(
 	moscInformer mcfginformersv1.MachineOSConfigInformer,
 	clusterVersionInformer configinformersv1.ClusterVersionInformer,
 	osImageStreamInformer mcfginformersv1alpha1.OSImageStreamInformer,
+	iriInformer mcfginformersv1alpha1.InternalReleaseImageInformer,
 	ctrlctx *ctrlcommon.ControllerContext,
 ) *Operator {
 	eventBroadcaster := record.NewBroadcaster()
@@ -365,6 +368,10 @@ func New(
 		optr.osImageStreamLister = osImageStreamInformer.Lister()
 		optr.osImageStreamListerSynced = osImageStreamInformer.Informer().HasSynced
 	}
+	if iriInformer != nil {
+		optr.iriLister = iriInformer.Lister()
+		optr.iriListerSynced = iriInformer.Informer().HasSynced
+	}
 
 	optr.vStore.Set("operator", version.ReleaseVersion)
 	optr.vStore.Set("operator-image", version.OperatorImage)
@@ -425,6 +432,9 @@ func (optr *Operator) Run(workers int, stopCh <-chan struct{}) {
 	if optr.osImageStreamListerSynced != nil && osimagestream.IsFeatureEnabled(optr.fgHandler) {
 		cacheSynced = append(cacheSynced, optr.osImageStreamListerSynced)
 	}
+	if optr.iriListerSynced != nil {
+		cacheSynced = append(cacheSynced, optr.iriListerSynced)
+	}
 	if !cache.WaitForCacheSync(stopCh,
 		cacheSynced...) {
 		klog.Error("failed to sync caches")

pkg/operator/sync.go

@@ -102,6 +102,8 @@ const (
 	mccUpdateBootImagesValidatingAdmissionPolicyBindingPath               = "manifests/machineconfigcontroller/update-bootimages-validatingadmissionpolicybinding.yaml"
 	mccMachineConfigPoolOSImageStreamValidatingAdmissionPolicyPath        = "manifests/machineconfigcontroller/machineconfigpool-osimagestream-reference-validatingadmissionpolicy.yaml"
 	mccMachineConfigPoolOSImageStreamValidatingAdmissionPolicyBindingPath = "manifests/machineconfigcontroller/machineconfigpool-osimagestream-reference-validatingadmissionpolicybinding.yaml"
+	mccIRIDeletionGuardValidatingAdmissionPolicyPath                      = "manifests/machineconfigcontroller/internalreleaseimage-deletion-guard-validatingadmissionpolicy.yaml"
+	mccIRIDeletionGuardValidatingAdmissionPolicyBindingPath               = "manifests/machineconfigcontroller/internalreleaseimage-deletion-guard-validatingadmissionpolicybinding.yaml"
 
 	// Machine OS Builder manifest paths
 	mobClusterRoleManifestPath                      = "manifests/machineosbuilder/clusterrole.yaml"
@@ -1187,6 +1189,16 @@ func (optr *Operator) syncMachineConfigController(config *renderConfig, _ *confi
 		paths.validatingAdmissionPolicyBindings = append(paths.validatingAdmissionPolicyBindings, mccMachineConfigPoolOSImageStreamValidatingAdmissionPolicyBindingPath)
 	}
 
+	if optr.fgHandler.Enabled(features.FeatureGateNoRegistryClusterInstall) {
+		// Only deploy the IRI deletion guard policy if the IRI resource actually exists
+		if optr.iriLister != nil {
+			if _, err := optr.iriLister.Get(ctrlcommon.InternalReleaseImageInstanceName); err == nil {
+				paths.validatingAdmissionPolicies = append(paths.validatingAdmissionPolicies, mccIRIDeletionGuardValidatingAdmissionPolicyPath)
+				paths.validatingAdmissionPolicyBindings = append(paths.validatingAdmissionPolicyBindings, mccIRIDeletionGuardValidatingAdmissionPolicyBindingPath)
+			}
+		}
+	}
+
 	if err := optr.applyManifests(config, paths); err != nil {
 		return fmt.Errorf("failed to apply machine config controller manifests: %w", err)
 	}