Add finalizer only for cluster scope objects (#4574)

- Add finalizers to CR only when cluster roles and bindings
  for SA are created by Operator
- Fixes #4367

Author: smanojj

.chloggen/fix-finalizer-addition.yaml

@@ -0,0 +1,20 @@
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: bug_fix
+
+# The name of the component, or a single word describing the area of concern, (e.g. collector, target allocator, auto-instrumentation, opamp, github action)
+component: collector
+
+# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Add finalizers to OpenTelemetryCollector CR only when cluster roles and bindings for SA are created by Operator.
+
+# One or more tracking issues related to the change
+issues: [4367]
+
+# (Optional) One or more lines of additional information to render under the primary note.
+# These lines will be padded with 2 spaces and then inserted directly into the document.
+# Use pipe (|) for multiline entries.
+subtext: |
+  Finalizer usage was restricted to cluster scoped resources only. Namespaced resources no longer receive finalizers,
+  preventing blocked namespace deletion if the operator is removed first. The change aligns finalizer behavior with
+  cluster-level RBAC availability, ensuring finalizers are applied only when the operator has the required
+  cluster scoped permissions.

internal/controllers/opentelemetrycollector_controller.go

@@ -285,12 +285,10 @@ func (r *OpenTelemetryCollectorReconciler) Reconcile(ctx context.Context, req ct
 	}
 
 	// Add finalizer for this CR
-	if !controllerutil.ContainsFinalizer(&instance, collectorFinalizer) {
-		if controllerutil.AddFinalizer(&instance, collectorFinalizer) {
-			err = r.Update(ctx, &instance)
-			if err != nil {
-				return ctrl.Result{}, err
-			}
+	if maybeAddFinalizer(params, &instance) {
+		err = r.Update(ctx, &instance)
+		if err != nil {
+			return ctrl.Result{}, err
 		}
 	}
 
@@ -395,3 +393,10 @@ func (r *OpenTelemetryCollectorReconciler) finalizeCollector(ctx context.Context
 	}
 	return nil
 }
+
+func maybeAddFinalizer(params manifests.Params, instance *v1beta1.OpenTelemetryCollector) bool {
+	if params.Config.CreateRBACPermissions == rbac.Available && !controllerutil.ContainsFinalizer(instance, collectorFinalizer) {
+		return controllerutil.AddFinalizer(instance, collectorFinalizer)
+	}
+	return false
+}

internal/controllers/opentelemetrycollector_reconciler_test.go

@@ -10,6 +10,12 @@ import (
 	"github.com/stretchr/testify/assert"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
+
+	"github.com/open-telemetry/opentelemetry-operator/apis/v1beta1"
+	"github.com/open-telemetry/opentelemetry-operator/internal/autodetect/rbac"
+	"github.com/open-telemetry/opentelemetry-operator/internal/config"
+	"github.com/open-telemetry/opentelemetry-operator/internal/manifests"
 )
 
 func TestGetCollectorConfigMapsToKeep(t *testing.T) {
@@ -65,3 +71,55 @@ func TestGetCollectorConfigMapsToKeep(t *testing.T) {
 		})
 	}
 }
+
+func TestMaybeAddFinalizer(t *testing.T) {
+	testCases := []struct {
+		name          string
+		rbacAvailable rbac.Availability
+		hasFinalizer  bool
+		expectAdded   bool
+	}{
+		{
+			name:          "adds finalizer when RBAC available and no finalizer exists",
+			rbacAvailable: rbac.Available,
+			hasFinalizer:  false,
+			expectAdded:   true,
+		},
+		{
+			name:          "does not add finalizer when RBAC not available",
+			rbacAvailable: rbac.NotAvailable,
+			hasFinalizer:  false,
+			expectAdded:   false,
+		},
+		{
+			name:          "does not add finalizer when already exists",
+			rbacAvailable: rbac.Available,
+			hasFinalizer:  true,
+			expectAdded:   false,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			instance := &v1beta1.OpenTelemetryCollector{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-collector",
+					Namespace: "default",
+				},
+			}
+
+			if tc.hasFinalizer {
+				controllerutil.AddFinalizer(instance, collectorFinalizer)
+			}
+
+			params := manifests.Params{
+				Config: config.Config{
+					CreateRBACPermissions: tc.rbacAvailable,
+				},
+			}
+
+			result := maybeAddFinalizer(params, instance)
+			assert.Equal(t, tc.expectAdded, result)
+		})
+	}
+}