WIP 🐛 Preserve user changes to deployment pod templates

internal/operator-controller/applier/boxcutter.go

@@ -12,6 +12,7 @@ import (
 	"slices"
 	"strings"
 
+	"github.com/go-logr/logr"
 	"helm.sh/helm/v3/pkg/release"
 	"helm.sh/helm/v3/pkg/storage/driver"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -153,8 +154,61 @@ func (r *SimpleRevisionGenerator) GenerateRevision(
 	return r.buildClusterExtensionRevision(objs, ext, revisionAnnotations), nil
 }
 
+// removePodTemplateMetadata conditionally removes empty pod template annotations from Deployments.
+// Empty annotations are removed to prevent OLM from claiming ownership via Server-Side Apply,
+// allowing users to add custom annotations. Non-empty (bundle-provided) annotations are preserved.
+// Labels are always preserved as they are required for selector matching and chart functionality.
+func removePodTemplateMetadata(unstr *unstructured.Unstructured, l logr.Logger) {
+	if unstr.GetKind() != "Deployment" || unstr.GroupVersionKind().Group != "apps" {
+		return
+	}
+
+	obj := unstr.Object
+	spec, ok := obj["spec"].(map[string]any)
+	if !ok {
+		return
+	}
+
+	template, ok := spec["template"].(map[string]any)
+	if !ok {
+		return
+	}
+
+	templateMeta, ok := template["metadata"].(map[string]any)
+	if !ok {
+		return
+	}
+
+	// Remove pod template annotations only when the rendered manifest has no annotations.
+	// This allows users to add custom annotations (like kubectl rollout restart) without
+	// OLM overwriting them, while still preserving any chart-provided annotations when present.
+	if annotationsVal, hasAnnotations := templateMeta["annotations"]; hasAnnotations {
+		if annotationsMap, ok := annotationsVal.(map[string]any); ok {
+			if len(annotationsMap) == 0 {
+				delete(templateMeta, "annotations")
+				l.V(1).Info("removed empty pod template annotations from Deployment to preserve user changes",
+					"deployment", unstr.GetName())
+			} else {
+				// Non-empty annotations are preserved so bundle/chart-provided annotations
+				// continue to be applied and maintained by OLM.
+				l.V(2).Info("preserving non-empty pod template annotations on Deployment",
+					"deployment", unstr.GetName(), "count", len(annotationsMap))
+			}
+		}
+	}
+}
+
 // sanitizedUnstructured takes an unstructured obj, removes status if present, and returns a sanitized copy containing only the allowed metadata entries set below.
 // If any unallowed entries are removed, a warning will be logged.
+//
+// For Deployment objects, this function conditionally removes empty pod template annotations.
+// Bundle-provided annotations are preserved to maintain operator functionality.
+// Empty annotations are removed to allow users to add custom annotations without OLM reverting them.
+// Examples of user annotations: "kubectl rollout restart", "kubectl annotate", custom monitoring annotations.
+// Labels are kept because: (1) deployment selector must match template labels, and
+// (2) chart-provided labels may be referenced by other resources.
+// This fixes the issue where user changes to pod template annotations would be undone by OLM.
+// See: https://github.com/operator-framework/operator-lifecycle-manager/issues/3392
 func sanitizedUnstructured(ctx context.Context, unstr *unstructured.Unstructured) {
 	l := log.FromContext(ctx)
 	obj := unstr.Object
@@ -193,6 +247,15 @@ func sanitizedUnstructured(ctx context.Context, unstr *unstructured.Unstructured
 		l.Info("warning: extraneous values removed from manifest metadata", "allowed metadata", allowedMetadata)
 	}
 	obj["metadata"] = metadataSanitized
+
+	// For Deployment objects, conditionally remove empty pod template annotations (always keep labels).
+	// Empty annotations are removed to prevent OLM from claiming ownership.
+	// Non-empty (bundle-provided) annotations are preserved.
+	// This allows users to add custom annotations (kubectl rollout restart, kubectl annotate, etc.)
+	// without OLM overwriting them.
+	// Labels are always preserved because the deployment selector must match template labels,
+	// and chart-provided labels may be needed by other resources.
+	removePodTemplateMetadata(unstr, l)
 }
 
 func (r *SimpleRevisionGenerator) buildClusterExtensionRevision(

internal/operator-controller/applier/boxcutter_test.go

@@ -1884,3 +1884,144 @@ func (s *statusWriterMock) Apply(ctx context.Context, obj runtime.ApplyConfigura
 	// helps ensure tests fail if this assumption changes
 	return fmt.Errorf("unexpected call to StatusWriter.Apply() - this method is not expected to be used in these tests")
 }
+
+func Test_SimpleRevisionGenerator_PodTemplateAnnotationSanitization(t *testing.T) {
+	tests := []struct {
+		name                        string
+		podTemplateAnnotations      map[string]string
+		expectAnnotationsInRevision bool
+	}{
+		{
+			name: "deployment with non-empty pod template annotations preserves them",
+			podTemplateAnnotations: map[string]string{
+				"kubectl.kubernetes.io/default-container": "main",
+				"prometheus.io/scrape":                    "true",
+			},
+			expectAnnotationsInRevision: true,
+		},
+		{
+			name:                        "deployment with empty pod template annotations removes them",
+			podTemplateAnnotations:      map[string]string{},
+			expectAnnotationsInRevision: false,
+		},
+		{
+			name:                        "deployment with nil pod template annotations has none in revision",
+			podTemplateAnnotations:      nil,
+			expectAnnotationsInRevision: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Create a deployment with specified pod template annotations
+			deployment := &appsv1.Deployment{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-deployment",
+				},
+				Spec: appsv1.DeploymentSpec{
+					Replicas: ptr.To(int32(1)),
+					Selector: &metav1.LabelSelector{
+						MatchLabels: map[string]string{"app": "test"},
+					},
+					Template: corev1.PodTemplateSpec{
+						ObjectMeta: metav1.ObjectMeta{
+							Labels:      map[string]string{"app": "test"},
+							Annotations: tt.podTemplateAnnotations,
+						},
+						Spec: corev1.PodSpec{
+							Containers: []corev1.Container{
+								{
+									Name:  "main",
+									Image: "nginx:latest",
+								},
+							},
+						},
+					},
+				},
+			}
+
+			// Create revision generator with fake manifest provider
+			scheme := runtime.NewScheme()
+			require.NoError(t, k8scheme.AddToScheme(scheme))
+			require.NoError(t, ocv1.AddToScheme(scheme))
+
+			ext := &ocv1.ClusterExtension{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-extension",
+				},
+			}
+
+			manifestProvider := &FakeManifestProvider{
+				GetFn: func(_ fs.FS, _ *ocv1.ClusterExtension) ([]client.Object, error) {
+					return []client.Object{deployment}, nil
+				},
+			}
+
+			rg := &applier.SimpleRevisionGenerator{
+				Scheme:           scheme,
+				ManifestProvider: manifestProvider,
+			}
+
+			// Create a valid bundle FS
+			bundleFS := bundlefs.Builder().
+				WithPackageName("test-package").
+				WithCSV(clusterserviceversion.Builder().WithName("test-csv").Build()).
+				Build()
+
+			// Generate revision
+			revision, err := rg.GenerateRevision(
+				context.Background(),
+				bundleFS,
+				ext,
+				map[string]string{"test": "label"},
+				nil,
+			)
+			require.NoError(t, err)
+			require.NotNil(t, revision)
+
+			// Find the deployment in the revision
+			var deploymentFound bool
+			for _, obj := range revision.Spec.Phases[0].Objects {
+				if obj.Object.GetKind() == "Deployment" {
+					deploymentFound = true
+
+					// Check pod template annotations
+					spec, ok := obj.Object.Object["spec"].(map[string]any)
+					require.True(t, ok, "deployment should have spec")
+
+					template, ok := spec["template"].(map[string]any)
+					require.True(t, ok, "deployment spec should have template")
+
+					templateMeta, ok := template["metadata"].(map[string]any)
+					require.True(t, ok, "pod template should have metadata")
+
+					annotations, hasAnnotations := templateMeta["annotations"]
+					if tt.expectAnnotationsInRevision {
+						assert.True(t, hasAnnotations, "expected annotations to be present in revision")
+						annotationsMap, ok := annotations.(map[string]any)
+						require.True(t, ok, "annotations should be a map")
+
+						for key, expectedValue := range tt.podTemplateAnnotations {
+							actualValue, exists := annotationsMap[key]
+							assert.True(t, exists, "expected annotation key %s to exist", key)
+							assert.Equal(t, expectedValue, actualValue, "annotation value mismatch for key %s", key)
+						}
+						assert.Len(t, annotationsMap, len(tt.podTemplateAnnotations), "annotation count mismatch")
+					} else {
+						assert.False(t, hasAnnotations, "expected annotations to be removed from revision")
+					}
+
+					// Labels should always be preserved
+					labels, hasLabels := templateMeta["labels"]
+					assert.True(t, hasLabels, "labels should always be preserved")
+					labelsMap, ok := labels.(map[string]any)
+					require.True(t, ok, "labels should be a map")
+					assert.Equal(t, "test", labelsMap["app"], "label app should be preserved")
+
+					break
+				}
+			}
+			assert.True(t, deploymentFound, "deployment should be found in revision")
+		})
+	}
+}

test/e2e/features/rollout-restart.feature

@@ -0,0 +1,42 @@
+Feature: Rollout Restart User Changes
+  # Verifies that user-added pod template annotations persist after OLM reconciliation.
+  # Fixes: https://github.com/operator-framework/operator-lifecycle-manager/issues/3392
+
+  Background:
+    Given OLM is available
+    And ClusterCatalog "test" serves bundles
+    And ServiceAccount "olm-sa" with needed permissions is available in ${TEST_NAMESPACE}
+
+  @BoxcutterRuntime
+  Scenario: User-initiated deployment changes persist after OLM reconciliation
+    When ClusterExtension is applied
+      """
+      apiVersion: olm.operatorframework.io/v1
+      kind: ClusterExtension
+      metadata:
+        name: ${NAME}
+      spec:
+        namespace: ${TEST_NAMESPACE}
+        serviceAccount:
+          name: olm-sa
+        source:
+          sourceType: Catalog
+          catalog:
+            packageName: test
+            selector:
+              matchLabels:
+                "olm.operatorframework.io/metadata.name": test-catalog
+      """
+    Then ClusterExtension is rolled out
+    And ClusterExtension is available
+    And resource "deployment/test-operator" is installed
+    And deployment "test-operator" is ready
+    When user performs rollout restart on "deployment/test-operator"
+    Then deployment "test-operator" rollout completes successfully
+    And resource "deployment/test-operator" has restart annotation
+    # Wait for OLM reconciliation cycle (runs every 10 seconds)
+    And I wait for "30" seconds
+    # Verify user changes persisted after OLM reconciliation
+    Then deployment "test-operator" rollout is still successful
+    And resource "deployment/test-operator" has restart annotation
+    And deployment "test-operator" has expected number of ready replicas