refactor: replace runtime.Object with metav1.Object in pkg/util/finalizers

pkg/controllers/federate/controller.go

@@ -417,17 +417,14 @@
 ) (*unstructured.Unstructured, error) {
 	logger := klog.FromContext(ctx).WithValues("finalizer", FinalizerFederateController)
 
-	isUpdated, err := finalizersutil.AddFinalizers(sourceObj, sets.NewString(FinalizerFederateController))
-	if err != nil {
-		return nil, fmt.Errorf("failed to add finalizer to source object: %w", err)
-	}
+	isUpdated := finalizersutil.AddFinalizers(sourceObj, sets.New(FinalizerFederateController))
 	if !isUpdated {
 		return sourceObj, nil
 	}
 
 	logger.V(1).Info("Adding finalizer to source object")
 
-	sourceObj, err = c.dynamicClient.Resource(sourceGVR).Namespace(sourceObj.GetNamespace()).Update(
+	sourceObj, err := c.dynamicClient.Resource(sourceGVR).Namespace(sourceObj.GetNamespace()).Update(
 		ctx,
 		sourceObj,
 		metav1.UpdateOptions{},
@@ -446,17 +443,14 @@
 ) (*unstructured.Unstructured, error) {
 	logger := klog.FromContext(ctx).WithValues("finalizer", FinalizerFederateController)
 
-	isUpdated, err := finalizersutil.RemoveFinalizers(sourceObj, sets.NewString(FinalizerFederateController))
-	if err != nil {
-		return nil, fmt.Errorf("failed to remove finalizer from source object: %w", err)
-	}
+	isUpdated := finalizersutil.RemoveFinalizers(sourceObj, sets.New(FinalizerFederateController))
 	if !isUpdated {
 		return sourceObj, nil
 	}
 
 	logger.V(1).Info("Removing finalizer from source object")
 
-	sourceObj, err = c.dynamicClient.Resource(sourceGVR).Namespace(sourceObj.GetNamespace()).Update(
+	sourceObj, err := c.dynamicClient.Resource(sourceGVR).Namespace(sourceObj.GetNamespace()).Update(
 		ctx,
 		sourceObj,
 		metav1.UpdateOptions{},

pkg/controllers/federatedcluster/controller.go

@@ -353,10 +353,7 @@
 	ctx context.Context,
 	cluster *fedcorev1a1.FederatedCluster,
 ) (*fedcorev1a1.FederatedCluster, error) {
-	updated, err := finalizers.AddFinalizers(cluster, sets.NewString(FinalizerFederatedClusterController))
-	if err != nil {
-		return nil, err
-	}
+	updated := finalizers.AddFinalizers(cluster, sets.New(FinalizerFederatedClusterController))
 
 	if updated {
 		return c.fedClient.CoreV1alpha1().FederatedClusters().Update(ctx, cluster, metav1.UpdateOptions{})

pkg/controllers/sync/controller.go

@@ -624,12 +624,7 @@
 			)
 			continue
 		}
-		hasFinalizer, err := finalizersutil.HasFinalizer(cluster, FinalizerCascadingDelete)
-		if err != nil {
-			shouldRecheckAfterDispatch = true
-			dispatcher.RecordClusterError(fedcorev1a1.FinalizerCheckFailed, clusterName, err)
-			continue
-		}
+		hasFinalizer := finalizersutil.HasFinalizer(cluster, FinalizerCascadingDelete)
 		if !hasFinalizer {
 			// we should not sync before finalizer is added
 			shouldRecheckAfterDispatch = true
@@ -971,9 +966,9 @@
 	ctx, keyedLogger := logging.InjectLoggerValues(ctx, "finalizer-name", FinalizerSyncController)
 
 	obj := fedResource.Object()
-	isUpdated, err := finalizersutil.AddFinalizers(obj, sets.NewString(FinalizerSyncController))
-	if err != nil || !isUpdated {
-		return err
+	isUpdated := finalizersutil.AddFinalizers(obj, sets.New(FinalizerSyncController))
+	if !isUpdated {
+		return nil
 	}
 
 	keyedLogger.V(1).Info("Adding finalizer to federated object")
@@ -995,9 +990,9 @@
 	ctx, keyedLogger := logging.InjectLoggerValues(ctx, "finalizer-name", FinalizerSyncController)
 
 	obj := fedResource.Object()
-	isUpdated, err := finalizersutil.RemoveFinalizers(obj, sets.NewString(FinalizerSyncController))
-	if err != nil || !isUpdated {
-		return err
+	isUpdated := finalizersutil.RemoveFinalizers(obj, sets.New(FinalizerSyncController))
+	if !isUpdated {
+		return nil
 	}
 
 	keyedLogger.V(1).Info("Removing finalizer from federated object")
@@ -1024,9 +1019,9 @@
 		if err != nil {
 			return err
 		}
-		isUpdated, err := finalizersutil.AddFinalizers(cluster, sets.NewString(FinalizerCascadingDelete))
-		if err != nil || !isUpdated {
-			return err
+		isUpdated := finalizersutil.AddFinalizers(cluster, sets.New(FinalizerCascadingDelete))
+		if !isUpdated {
+			return nil
 		}
 		cluster, err = s.fedClient.CoreV1alpha1().FederatedClusters().Update(ctx, cluster, metav1.UpdateOptions{})
 		return err
@@ -1046,9 +1041,9 @@
 		if err != nil {
 			return err
 		}
-		isUpdated, err := finalizersutil.RemoveFinalizers(cluster, sets.NewString(FinalizerCascadingDelete))
-		if err != nil || !isUpdated {
-			return err
+		isUpdated := finalizersutil.RemoveFinalizers(cluster, sets.New(FinalizerCascadingDelete))
+		if !isUpdated {
+			return nil
 		}
 		cluster, err = s.fedClient.CoreV1alpha1().FederatedClusters().Update(ctx, cluster, metav1.UpdateOptions{})
 		return err

pkg/controllers/sync/dispatch/unmanaged.go

@@ -132,18 +132,9 @@
 		// Avoid mutating the resource in the informer cache
 		clusterObj := clusterObj.DeepCopy()
 
-		needUpdate, err := removeRetainObjectFinalizer(clusterObj)
-		if err != nil {
-			if d.recorder == nil {
-				wrappedErr := d.wrapOperationError(err, clusterName, op)
-				keyedLogger.Error(wrappedErr, "Failed to delete target object in cluster")
-			} else {
-				d.recorder.recordOperationError(ctx, fedcorev1a1.DeletionFailed, clusterName, op, err)
-			}
-			result = false
-			return result
-		}
+		needUpdate := removeRetainObjectFinalizer(clusterObj)
 		if needUpdate {
+			var err error
 			clusterObj, err = client.Resource(d.targetGVR).Namespace(targetName.Namespace).Update(
 				ctx,
 				clusterObj,
@@ -177,7 +168,7 @@
 		// To avoid this, we explicitly set the PropagationPolicy to Background like `kubectl delete` does by default.
 		// Ref: https://github.com/kubernetes/kubernetes/pull/65908
 		deletionPropagation := metav1.DeletePropagationBackground
-		err = client.Resource(d.targetGVR).Namespace(targetName.Namespace).Delete(
+		err := client.Resource(d.targetGVR).Namespace(targetName.Namespace).Delete(
 			ctx,
 			targetName.Name,
 			metav1.DeleteOptions{PropagationPolicy: &deletionPropagation},
@@ -218,15 +209,7 @@
 		updateObj := clusterObj.DeepCopy()
 
 		managedlabel.RemoveManagedLabel(updateObj)
-		if _, err := removeRetainObjectFinalizer(updateObj); err != nil {
-			if d.recorder == nil {
-				wrappedErr := d.wrapOperationError(err, clusterName, op)
-				keyedLogger.Error(wrappedErr, "Failed to remove managed label from target object in cluster")
-			} else {
-				d.recorder.recordOperationError(ctx, fedcorev1a1.LabelRemovalFailed, clusterName, op, err)
-			}
-			return false
-		}
+		removeRetainObjectFinalizer(updateObj)
 
 		var err error
 		if _, err = client.Resource(d.targetGVR).Namespace(clusterObj.GetNamespace()).Update(
@@ -258,6 +241,6 @@
 	return errors.Wrapf(err, "Failed to "+eventTemplate, operation, targetGVR, targetName, clusterName)
 }
 
-func removeRetainObjectFinalizer(obj *unstructured.Unstructured) (bool, error) {
-	return finalizers.RemoveFinalizers(obj, sets.NewString(RetainTerminatingObjectFinalizer))
+func removeRetainObjectFinalizer(obj *unstructured.Unstructured) bool {
+	return finalizers.RemoveFinalizers(obj, sets.New(RetainTerminatingObjectFinalizer))
 }

pkg/controllers/sync/resource.go

@@ -177,9 +177,7 @@
 			return nil, err
 		}
 
-		if err := addRetainObjectFinalizer(obj); err != nil {
-			return nil, err
-		}
+		addRetainObjectFinalizer(obj)
 	case common.ServiceGVK:
 		if err := dropServiceFields(obj); err != nil {
 			return nil, err
@@ -189,19 +187,14 @@
 			return nil, err
 		}
 
-		if err := addRetainObjectFinalizer(obj); err != nil {
-			return nil, err
-		}
+		addRetainObjectFinalizer(obj)
 	}
 
 	return obj, nil
 }
 
-func addRetainObjectFinalizer(obj *unstructured.Unstructured) error {
-	if _, err := finalizers.AddFinalizers(obj, sets.NewString(dispatch.RetainTerminatingObjectFinalizer)); err != nil {
-		return err
-	}
-	return nil
+func addRetainObjectFinalizer(obj *unstructured.Unstructured) {
+	finalizers.AddFinalizers(obj, sets.New(dispatch.RetainTerminatingObjectFinalizer))
 }
 
 func dropJobFields(obj *unstructured.Unstructured) error {

pkg/util/finalizers/finalizers.go

@@ -22,49 +22,36 @@ are Copyright 2023 The KubeAdmiral Authors.
 package finalizers
 
 import (
-	meta "k8s.io/apimachinery/pkg/api/meta"
-	"k8s.io/apimachinery/pkg/runtime"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/sets"
 )
 
 // HasFinalizer returns true if the given object has the given finalizer in its ObjectMeta.
-func HasFinalizer(obj runtime.Object, finalizer string) (bool, error) {
-	accessor, err := meta.Accessor(obj)
-	if err != nil {
-		return false, err
-	}
-	finalizers := sets.NewString(accessor.GetFinalizers()...)
-	return finalizers.Has(finalizer), nil
+func HasFinalizer(obj metav1.Object, finalizer string) bool {
+	finalizers := sets.New(obj.GetFinalizers()...)
+	return finalizers.Has(finalizer)
 }
 
 // AddFinalizers adds the given finalizers to the given objects ObjectMeta.
 // Returns true if the object was updated.
-func AddFinalizers(obj runtime.Object, newFinalizers sets.String) (bool, error) {
-	accessor, err := meta.Accessor(obj)
-	if err != nil {
-		return false, err
-	}
-	oldFinalizers := sets.NewString(accessor.GetFinalizers()...)
+func AddFinalizers(obj metav1.Object, newFinalizers sets.Set[string]) bool {
+	oldFinalizers := sets.New(obj.GetFinalizers()...)
 	if oldFinalizers.IsSuperset(newFinalizers) {
-		return false, nil
+		return false
 	}
 	allFinalizers := oldFinalizers.Union(newFinalizers)
-	accessor.SetFinalizers(allFinalizers.List())
-	return true, nil
+	obj.SetFinalizers(sets.List(allFinalizers))
+	return true
 }
 
 // RemoveFinalizers removes the given finalizers from the given objects ObjectMeta.
 // Returns true if the object was updated.
-func RemoveFinalizers(obj runtime.Object, finalizers sets.String) (bool, error) {
-	accessor, err := meta.Accessor(obj)
-	if err != nil {
-		return false, err
-	}
-	oldFinalizers := sets.NewString(accessor.GetFinalizers()...)
+func RemoveFinalizers(obj metav1.Object, finalizers sets.Set[string]) bool {
+	oldFinalizers := sets.New(obj.GetFinalizers()...)
 	if oldFinalizers.Intersection(finalizers).Len() == 0 {
-		return false, nil
+		return false
 	}
 	newFinalizers := oldFinalizers.Difference(finalizers)
-	accessor.SetFinalizers(newFinalizers.List())
-	return true, nil
+	obj.SetFinalizers(sets.List(newFinalizers))
+	return true
 }