use json merge patch to update finalizers (#402)

Author: cbarbian-sap

internal/helm/util.go

@@ -10,6 +10,8 @@ import (
 	"strings"
 )
 
+// TODO: consolidate all the util files into an internal reuse package
+
 /*
 func getString(data map[string]any, key string) (string, bool, bool) {
 	if v, ok := data[key]; ok {

internal/util/finalizer.go

@@ -0,0 +1,30 @@
+/*
+SPDX-FileCopyrightText: 2026 SAP SE or an SAP affiliate company and component-operator-runtime contributors
+SPDX-License-Identifier: Apache-2.0
+*/
+
+package util
+
+import (
+	"context"
+	"encoding/json"
+
+	apitypes "k8s.io/apimachinery/pkg/types"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+// This is a workaround. Ideally Kubernetes would have a finalizers subresource, and controller-runtime
+// would offer the according subresource client.
+func UpdateFinalizers(ctx context.Context, clnt client.Client, obj client.Object, fieldOwner string) error {
+	// note: it's crucial to use string keyed maps to construct the patch; using metav1.ObjectMeta would lead to
+	// finalizers to be removed entirely from the patch when empty (because of omitempty usage there);
+	// but we need to preserve empty or null finalizers in the patch, in order to make JSON merge patch apply it correctly
+	finalizerPatch := map[string]any{
+		"metadata": map[string]any{
+			"resourceVersion": obj.GetResourceVersion(),
+			"finalizers":      obj.GetFinalizers(),
+		},
+	}
+	// note: this must() is ok because marshalling finalizers should always work
+	return clnt.Patch(ctx, obj, client.RawPatch(apitypes.MergePatchType, must(json.Marshal(finalizerPatch))), client.FieldOwner(fieldOwner))
+}

internal/util/util.go

@@ -0,0 +1,15 @@
+/*
+SPDX-FileCopyrightText: 2026 SAP SE or an SAP affiliate company and component-operator-runtime contributors
+SPDX-License-Identifier: Apache-2.0
+*/
+
+package util
+
+// TODO: consolidate all the util files into an internal reuse package
+
+func must[T any](x T, err error) T {
+	if err != nil {
+		panic(err)
+	}
+	return x
+}

pkg/component/reconciler.go

@@ -45,6 +45,7 @@ import (
 	"github.com/sap/component-operator-runtime/internal/clientfactory"
 	"github.com/sap/component-operator-runtime/internal/events"
 	"github.com/sap/component-operator-runtime/internal/metrics"
+	"github.com/sap/component-operator-runtime/internal/util"
 	"github.com/sap/component-operator-runtime/pkg/cluster"
 	"github.com/sap/component-operator-runtime/pkg/manifests"
 	"github.com/sap/component-operator-runtime/pkg/reconciler"
@@ -514,7 +515,7 @@ func (r *Reconciler[T]) Reconcile(ctx context.Context, req ctrl.Request) (result
 		// create/update case
 		// TODO: optionally (to be completely consistent) set finalizer through a mutating webhook
 		if added := controllerutil.AddFinalizer(component, *r.options.Finalizer); added {
-			if err := r.client.Update(ctx, component, client.FieldOwner(*r.options.FieldOwner)); err != nil {
+			if err := util.UpdateFinalizers(ctx, r.client, component, *r.options.FieldOwner); err != nil {
 				return ctrl.Result{}, legacyerrors.Wrap(err, "error adding finalizer")
 			}
 			// trigger another round trip
@@ -610,7 +611,7 @@ func (r *Reconciler[T]) Reconcile(ctx context.Context, req ctrl.Request) (result
 			// all dependent resources are already gone, so that's it
 			log.V(1).Info("all dependent resources are successfully deleted; removing finalizer")
 			if removed := controllerutil.RemoveFinalizer(component, *r.options.Finalizer); removed {
-				if err := r.client.Update(ctx, component, client.FieldOwner(*r.options.FieldOwner)); err != nil {
+				if err := util.UpdateFinalizers(ctx, r.client, component, *r.options.FieldOwner); err != nil {
 					return ctrl.Result{}, legacyerrors.Wrap(err, "error removing finalizer")
 				}
 			}

pkg/reconciler/reconciler.go

@@ -34,6 +34,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/log"
 
+	"github.com/sap/component-operator-runtime/internal/util"
 	"github.com/sap/component-operator-runtime/pkg/cluster"
 	"github.com/sap/component-operator-runtime/pkg/status"
 	"github.com/sap/component-operator-runtime/pkg/types"
@@ -1318,7 +1319,7 @@ func (r *Reconciler) deleteObject(ctx context.Context, key types.ObjectKey, exis
 			}
 			if ok := controllerutil.RemoveFinalizer(crd, r.finalizer); ok {
 				// note: 409 error is very likely here (because of concurrent updates happening through the api server); this is why we retry once
-				if err := r.client.Update(ctx, crd, client.FieldOwner(r.fieldOwner)); err != nil {
+				if err := util.UpdateFinalizers(ctx, r.client, crd, r.fieldOwner); err != nil {
 					if i == 1 && apierrors.IsConflict(err) {
 						log.V(1).Info("error while updating CustomResourcedefinition (409 conflict); doing one retry", "error", err.Error())
 						continue
@@ -1343,7 +1344,7 @@ func (r *Reconciler) deleteObject(ctx context.Context, key types.ObjectKey, exis
 			}
 			if ok := controllerutil.RemoveFinalizer(apiService, r.finalizer); ok {
 				// note: 409 error is very likely here (because of concurrent updates happening through the api server); this is why we retry once
-				if err := r.client.Update(ctx, apiService, client.FieldOwner(r.fieldOwner)); err != nil {
+				if err := util.UpdateFinalizers(ctx, r.client, apiService, r.fieldOwner); err != nil {
 					if i == 1 && apierrors.IsConflict(err) {
 						log.V(1).Info("error while updating APIService (409 conflict); doing one retry", "error", err.Error())
 						continue
@@ -1371,7 +1372,7 @@ func (r *Reconciler) orphanObject(ctx context.Context, existingObject *unstructu
 	if isCrd(existingObject) || isApiService(existingObject) {
 		object := existingObject.DeepCopy()
 		if controllerutil.RemoveFinalizer(object, r.finalizer) {
-			if err := r.client.Update(ctx, object, client.FieldOwner(r.fieldOwner)); err != nil {
+			if err := util.UpdateFinalizers(ctx, r.client, object, r.fieldOwner); err != nil {
 				return err
 			}
 		}