Merge pull request #572 from openshift-bot/synchronize-upstream

NO-ISSUE: Synchronize From Upstream Repositories

Author: openshift-merge-bot[bot]

cmd/catalogd/main.go

@@ -59,6 +59,7 @@ import (
 	"github.com/operator-framework/operator-controller/internal/catalogd/storage"
 	"github.com/operator-framework/operator-controller/internal/catalogd/webhook"
 	sharedcontrollers "github.com/operator-framework/operator-controller/internal/shared/controllers"
+	cacheutil "github.com/operator-framework/operator-controller/internal/shared/util/cache"
 	fsutil "github.com/operator-framework/operator-controller/internal/shared/util/fs"
 	httputil "github.com/operator-framework/operator-controller/internal/shared/util/http"
 	imageutil "github.com/operator-framework/operator-controller/internal/shared/util/image"
@@ -254,6 +255,8 @@ func run(ctx context.Context) error {
 
 	cacheOptions := crcache.Options{
 		ByObject: map[client.Object]crcache.ByObject{},
+		// Memory optimization: strip managed fields and large annotations from cached objects
+		DefaultTransform: cacheutil.StripManagedFieldsAndAnnotations(),
 	}
 
 	saKey, err := sautil.GetServiceAccount()

cmd/operator-controller/main.go

@@ -78,6 +78,7 @@ import (
 	"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render/registryv1"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/scheme"
 	sharedcontrollers "github.com/operator-framework/operator-controller/internal/shared/controllers"
+	cacheutil "github.com/operator-framework/operator-controller/internal/shared/util/cache"
 	fsutil "github.com/operator-framework/operator-controller/internal/shared/util/fs"
 	httputil "github.com/operator-framework/operator-controller/internal/shared/util/http"
 	imageutil "github.com/operator-framework/operator-controller/internal/shared/util/image"
@@ -257,6 +258,8 @@ func run() error {
 			cfg.systemNamespace: {LabelSelector: k8slabels.Everything()},
 		},
 		DefaultLabelSelector: k8slabels.Nothing(),
+		// Memory optimization: strip managed fields and large annotations from cached objects
+		DefaultTransform: cacheutil.StripAnnotations(),
 	}
 
 	if features.OperatorControllerFeatureGate.Enabled(features.BoxcutterRuntime) {

commitchecker.yaml

@@ -1,4 +1,4 @@
-expectedMergeBase: 34394ce0a7067e1f1622dc2c381a9a12439b10d2
+expectedMergeBase: 4e349e62c5314574f6194d64b1ff4508f2e9331f
 upstreamBranch: main
 upstreamOrg: operator-framework
 upstreamRepo: operator-controller

internal/catalogd/controllers/core/clustercatalog_controller.go

@@ -41,6 +41,7 @@ import (
 	ocv1 "github.com/operator-framework/operator-controller/api/v1"
 	"github.com/operator-framework/operator-controller/internal/catalogd/storage"
 	imageutil "github.com/operator-framework/operator-controller/internal/shared/util/image"
+	k8sutil "github.com/operator-framework/operator-controller/internal/shared/util/k8s"
 )
 
 const (
@@ -107,16 +108,16 @@ func (r *ClusterCatalogReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 	// Do checks before any Update()s, as Update() may modify the resource structure!
 	updateStatus := !equality.Semantic.DeepEqual(existingCatsrc.Status, reconciledCatsrc.Status)
 	updateFinalizers := !equality.Semantic.DeepEqual(existingCatsrc.Finalizers, reconciledCatsrc.Finalizers)
-	unexpectedFieldsChanged := checkForUnexpectedFieldChange(existingCatsrc, *reconciledCatsrc)
+	unexpectedFieldsChanged := k8sutil.CheckForUnexpectedFieldChange(&existingCatsrc, reconciledCatsrc)
 
 	if unexpectedFieldsChanged {
 		panic("spec or metadata changed by reconciler")
 	}
 
 	// Save the finalizers off to the side. If we update the status, the reconciledCatsrc will be updated
 	// to contain the new state of the ClusterCatalog, which contains the status update, but (critically)
-	// does not contain the finalizers. After the status update, we need to re-add the finalizers to the
-	// reconciledCatsrc before updating the object.
+	// does not contain the finalizers. After the status update, we will use the saved finalizers in the
+	// CreateOrPatch()
 	finalizers := reconciledCatsrc.Finalizers
 
 	if updateStatus {
@@ -125,10 +126,12 @@ func (r *ClusterCatalogReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 		}
 	}
 
-	reconciledCatsrc.Finalizers = finalizers
-
 	if updateFinalizers {
-		if err := r.Update(ctx, reconciledCatsrc); err != nil {
+		// Use CreateOrPatch to update finalizers on the server
+		if _, err := controllerutil.CreateOrPatch(ctx, r.Client, reconciledCatsrc, func() error {
+			reconciledCatsrc.Finalizers = finalizers
+			return nil
+		}); err != nil {
 			reconcileErr = errors.Join(reconcileErr, fmt.Errorf("error updating finalizers: %v", err))
 		}
 	}
@@ -415,13 +418,6 @@ func (r *ClusterCatalogReconciler) needsPoll(lastSuccessfulPoll time.Time, catal
 	return nextPoll.Before(time.Now())
 }
 
-// Compare resources - ignoring status & metadata.finalizers
-func checkForUnexpectedFieldChange(a, b ocv1.ClusterCatalog) bool {
-	a.Status, b.Status = ocv1.ClusterCatalogStatus{}, ocv1.ClusterCatalogStatus{}
-	a.Finalizers, b.Finalizers = []string{}, []string{}
-	return !equality.Semantic.DeepEqual(a, b)
-}
-
 type finalizerFunc func(ctx context.Context, obj client.Object) (crfinalizer.Result, error)
 
 func (f finalizerFunc) Finalize(ctx context.Context, obj client.Object) (crfinalizer.Result, error) {

internal/operator-controller/applier/boxcutter.go

@@ -27,6 +27,7 @@ import (
 
 	ocv1 "github.com/operator-framework/operator-controller/api/v1"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/labels"
+	"github.com/operator-framework/operator-controller/internal/shared/util/cache"
 )
 
 const (
@@ -66,6 +67,9 @@ func (r *SimpleRevisionGenerator) GenerateRevisionFromHelmRelease(
 		maps.Copy(labels, objectLabels)
 		obj.SetLabels(labels)
 
+		// Memory optimization: strip large annotations
+		// Note: ApplyStripTransform never returns an error in practice
+		_ = cache.ApplyStripAnnotationsTransform(&obj)
 		sanitizedUnstructured(ctx, &obj)
 
 		objs = append(objs, ocv1.ClusterExtensionRevisionObject{
@@ -117,6 +121,10 @@ func (r *SimpleRevisionGenerator) GenerateRevision(
 		unstr := unstructured.Unstructured{Object: unstrObj}
 		unstr.SetGroupVersionKind(gvk)
 
+		// Memory optimization: strip large annotations
+		if err := cache.ApplyStripAnnotationsTransform(&unstr); err != nil {
+			return nil, err
+		}
 		sanitizedUnstructured(ctx, &unstr)
 
 		objs = append(objs, ocv1.ClusterExtensionRevisionObject{

internal/operator-controller/controllers/clusterextension_controller.go

@@ -35,6 +35,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/builder"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	crcontroller "sigs.k8s.io/controller-runtime/pkg/controller"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/event"
 	crhandler "sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/log"
@@ -48,6 +49,7 @@ import (
 	ocv1 "github.com/operator-framework/operator-controller/api/v1"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/conditionsets"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/labels"
+	k8sutil "github.com/operator-framework/operator-controller/internal/shared/util/k8s"
 )
 
 const (
@@ -135,25 +137,28 @@ func (r *ClusterExtensionReconciler) Reconcile(ctx context.Context, req ctrl.Req
 	updateFinalizers := !equality.Semantic.DeepEqual(existingExt.Finalizers, reconciledExt.Finalizers)
 
 	// If any unexpected fields have changed, panic before updating the resource
-	unexpectedFieldsChanged := checkForUnexpectedClusterExtensionFieldChange(*existingExt, *reconciledExt)
+	unexpectedFieldsChanged := k8sutil.CheckForUnexpectedFieldChange(existingExt, reconciledExt)
 	if unexpectedFieldsChanged {
 		panic("spec or metadata changed by reconciler")
 	}
 
 	// Save the finalizers off to the side. If we update the status, the reconciledExt will be updated
 	// to contain the new state of the ClusterExtension, which contains the status update, but (critically)
-	// does not contain the finalizers. After the status update, we need to re-add the finalizers to the
-	// reconciledExt before updating the object.
+	// does not contain the finalizers. After the status update, we will use the saved finalizers in the
+	// CreateOrPatch()
 	finalizers := reconciledExt.Finalizers
 	if updateStatus {
 		if err := r.Client.Status().Update(ctx, reconciledExt); err != nil {
 			reconcileErr = errors.Join(reconcileErr, fmt.Errorf("error updating status: %v", err))
 		}
 	}
-	reconciledExt.Finalizers = finalizers
 
 	if updateFinalizers {
-		if err := r.Update(ctx, reconciledExt); err != nil {
+		// Use CreateOrPatch to update finalizers on the server
+		if _, err := controllerutil.CreateOrPatch(ctx, r.Client, reconciledExt, func() error {
+			reconciledExt.Finalizers = finalizers
+			return nil
+		}); err != nil {
 			reconcileErr = errors.Join(reconcileErr, fmt.Errorf("error updating finalizers: %v", err))
 		}
 	}
@@ -179,13 +184,6 @@ func ensureAllConditionsWithReason(ext *ocv1.ClusterExtension, reason v1alpha1.C
 	}
 }
 
-// Compare resources - ignoring status & metadata.finalizers
-func checkForUnexpectedClusterExtensionFieldChange(a, b ocv1.ClusterExtension) bool {
-	a.Status, b.Status = ocv1.ClusterExtensionStatus{}, ocv1.ClusterExtensionStatus{}
-	a.Finalizers, b.Finalizers = []string{}, []string{}
-	return !equality.Semantic.DeepEqual(a, b)
-}
-
 // SetDeprecationStatus will set the appropriate deprecation statuses for a ClusterExtension
 // based on the provided bundle
 func SetDeprecationStatus(ext *ocv1.ClusterExtension, bundleName string, deprecation *declcfg.Deprecation) {

internal/shared/util/cache/transform.go

@@ -0,0 +1,91 @@
+package cache
+
+import (
+	"maps"
+
+	toolscache "k8s.io/client-go/tools/cache"
+	crcache "sigs.k8s.io/controller-runtime/pkg/cache"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+// stripAnnotations removes memory-heavy annotations that aren't needed for controller operations.
+func stripAnnotations(obj interface{}) (interface{}, error) {
+	if metaObj, ok := obj.(client.Object); ok {
+		// Remove the last-applied-configuration annotation which can be very large
+		// Clone the annotations map to avoid modifying shared references
+		annotations := metaObj.GetAnnotations()
+		if annotations != nil {
+			annotations = maps.Clone(annotations)
+			delete(annotations, "kubectl.kubernetes.io/last-applied-configuration")
+			if len(annotations) == 0 {
+				metaObj.SetAnnotations(nil)
+			} else {
+				metaObj.SetAnnotations(annotations)
+			}
+		}
+	}
+	return obj, nil
+}
+
+// StripManagedFieldsAndAnnotations returns a cache transform function that removes
+// memory-heavy fields that aren't needed for controller operations.
+// This significantly reduces memory usage in informer caches by removing:
+// - Managed fields (can be several KB per object)
+// - kubectl.kubernetes.io/last-applied-configuration annotation (can be very large)
+//
+// Use this function as a DefaultTransform in controller-runtime cache.Options
+// to reduce memory overhead across all cached objects.
+//
+// Example:
+//
+//	cacheOptions := cache.Options{
+//	    DefaultTransform: cacheutil.StripManagedFieldsAndAnnotations(),
+//	}
+func StripManagedFieldsAndAnnotations() toolscache.TransformFunc {
+	// Use controller-runtime's built-in TransformStripManagedFields and compose it
+	// with our custom annotation stripping transform
+	managedFieldsTransform := crcache.TransformStripManagedFields()
+
+	return func(obj interface{}) (interface{}, error) {
+		// First strip managed fields using controller-runtime's transform
+		obj, err := managedFieldsTransform(obj)
+		if err != nil {
+			return obj, err
+		}
+
+		// Then strip the large annotations
+		return stripAnnotations(obj)
+	}
+}
+
+// StripAnnotations returns a cache transform function that removes
+// memory-heavy annotation fields that aren't needed for controller operations.
+// This significantly reduces memory usage in informer caches by removing:
+// - kubectl.kubernetes.io/last-applied-configuration annotation (can be very large)
+//
+// Use this function as a DefaultTransform in controller-runtime cache.Options
+// to reduce memory overhead across all cached objects.
+//
+// Example:
+//
+//	cacheOptions := cache.Options{
+//	    DefaultTransform: cacheutil.StripAnnotations(),
+//	}
+func StripAnnotations() toolscache.TransformFunc {
+	return func(obj interface{}) (interface{}, error) {
+		// Strip the large annotations
+		return stripAnnotations(obj)
+	}
+}
+
+// ApplyStripAnnotationsTransform applies the strip transform directly to an object.
+// This is a convenience function for cases where you need to strip fields
+// from an object outside of the cache transform context.
+//
+// Note: This function never returns an error in practice, but returns error
+// to satisfy the TransformFunc interface.
+func ApplyStripAnnotationsTransform(obj client.Object) error {
+	transform := StripAnnotations()
+	_, err := transform(obj)
+	return err
+}

internal/shared/util/k8s/k8s.go

@@ -0,0 +1,54 @@
+package k8s
+
+import (
+	"reflect"
+
+	"k8s.io/apimachinery/pkg/api/equality"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+// CheckForUnexpectedFieldChange compares two Kubernetes objects and returns true
+// if their annotations, labels, or spec have changed. This is useful for detecting
+// unexpected modifications during reconciliation.
+//
+// The function compares:
+//   - Annotations (via GetAnnotations)
+//   - Labels (via GetLabels)
+//   - Spec (using reflection to access the Spec field, with semantic equality)
+//
+// Status and finalizers are intentionally not compared, as these are expected
+// to change during reconciliation.
+//
+// This function uses reflection to access the Spec field, so no explicit GetSpec()
+// method is required. The objects must have a field named "Spec".
+func CheckForUnexpectedFieldChange(a, b metav1.Object) bool {
+	if !equality.Semantic.DeepEqual(a.GetAnnotations(), b.GetAnnotations()) {
+		return true
+	}
+	if !equality.Semantic.DeepEqual(a.GetLabels(), b.GetLabels()) {
+		return true
+	}
+
+	// Use reflection to access the Spec field
+	aVal := reflect.ValueOf(a)
+	bVal := reflect.ValueOf(b)
+
+	// Handle pointer types
+	if aVal.Kind() == reflect.Ptr {
+		aVal = aVal.Elem()
+	}
+	if bVal.Kind() == reflect.Ptr {
+		bVal = bVal.Elem()
+	}
+
+	// Get the Spec field from both objects
+	aSpec := aVal.FieldByName("Spec")
+	bSpec := bVal.FieldByName("Spec")
+
+	// If either Spec field is invalid, return false (no change detected)
+	if !aSpec.IsValid() || !bSpec.IsValid() {
+		return false
+	}
+
+	return !equality.Semantic.DeepEqual(aSpec.Interface(), bSpec.Interface())
+}