Use patches for adding and removing finalizers

Using update can have unintended consequences such as setting empty
values of fields that were nil on the server. This happens for
the evaluationInterval field where if it wasn't set, the update adds an
empty map, which triggers the spec-sync to remove the nil map. This
unnecessary spec-sync update causes the status update to fail later on
in the ConfigurationPolicy processing.

Additionally, when a finalizer can't be added or removed, the evaluation
stops and would be retried on the next run. This is a safer approach.

Signed-off-by: mprahl <mprahl@users.noreply.github.com>

controllers/configurationpolicy_controller.go

@@ -704,11 +704,13 @@ func (r *ConfigurationPolicyReconciler) handleObjectTemplates(plc policyv1.Confi
 
  if cleanupNow {
  if objHasFinalizer(&plc, pruneObjectFinalizer) {
- plc.SetFinalizers(removeObjFinalizer(&plc, pruneObjectFinalizer))
+ patch := removeObjFinalizerPatch(&plc, pruneObjectFinalizer)
 
- err := r.Update(context.TODO(), &plc)
+ err = r.Patch(context.TODO(), &plc, client.RawPatch(types.JSONPatchType, patch))
  if err != nil {
- log.V(1).Error(err, "Error removing finalizer for configuration policy", plc)
+ log.V(1).Error(err, "Error removing finalizer for configuration policy")
+
+ return
  }
  }
 
@@ -717,11 +719,13 @@ func (r *ConfigurationPolicyReconciler) handleObjectTemplates(plc policyv1.Confi
 
  // set finalizer if it hasn't been set
  if !objHasFinalizer(&plc, pruneObjectFinalizer) {
- plc.SetFinalizers(addObjFinalizer(&plc, pruneObjectFinalizer))
+ mergePatch := []byte(`{"metadata": {"finalizers": ["` + pruneObjectFinalizer + `"]}}`)
 
- err := r.Update(context.TODO(), &plc)
+ err := r.Patch(context.TODO(), &plc, client.RawPatch(types.MergePatchType, mergePatch))
  if err != nil {
- log.V(1).Error(err, "Error setting finalizer for configuration policy", plc)
+ log.V(1).Error(err, "Error setting finalizer for configuration policy")
+
+ return
  }
  }
 
@@ -733,11 +737,14 @@ func (r *ConfigurationPolicyReconciler) handleObjectTemplates(plc policyv1.Confi
 
  if len(failures) == 0 {
  log.V(1).Info("Objects have been successfully cleaned up, removing finalizer")
- plc.SetFinalizers(removeObjFinalizer(&plc, pruneObjectFinalizer))
 
- err := r.Update(context.TODO(), &plc)
+ patch := removeObjFinalizerPatch(&plc, pruneObjectFinalizer)
+
+ err = r.Patch(context.TODO(), &plc, client.RawPatch(types.JSONPatchType, patch))
  if err != nil {
- log.V(1).Error(err, "Error unsetting finalizer for configuration policy", plc)
+ log.V(1).Error(err, "Error removing finalizer for configuration policy")
+
+ return
  }
  } else {
  log.V(1).Info("Object cleanup failed, some objects have not been deleted from the cluster")
@@ -767,10 +774,13 @@ func (r *ConfigurationPolicyReconciler) handleObjectTemplates(plc policyv1.Confi
  }
  } else if objHasFinalizer(&plc, pruneObjectFinalizer) {
  // if pruneObjectBehavior is none, no finalizer is needed
- plc.SetFinalizers(removeObjFinalizer(&plc, pruneObjectFinalizer))
- err := r.Update(context.TODO(), &plc)
+ patch := removeObjFinalizerPatch(&plc, pruneObjectFinalizer)
+
+ err := r.Patch(context.TODO(), &plc, client.RawPatch(types.JSONPatchType, patch))
  if err != nil {
- log.V(1).Error(err, "Error unsetting finalizer for configuration policy", plc)
+ log.V(1).Error(err, "Error removing finalizer for configuration policy")
+
+ return
  }
  }
 

controllers/configurationpolicy_utils.go

@@ -7,6 +7,7 @@ import (
  "fmt"
  "reflect"
  "sort"
+ "strconv"
  "strings"
 
  gocmp "github.com/google/go-cmp/cmp"
@@ -562,24 +563,14 @@ func objHasFinalizer(obj metav1.Object, finalizer string) bool {
  return false
 }
 
-func addObjFinalizer(obj metav1.Object, finalizer string) []string {
- if objHasFinalizer(obj, finalizer) {
- return obj.GetFinalizers()
- }
-
- return append(obj.GetFinalizers(), finalizer)
-}
-
-func removeObjFinalizer(obj metav1.Object, finalizer string) []string {
- result := []string{}
-
- for _, existingFinalizer := range obj.GetFinalizers() {
- if existingFinalizer != finalizer {
- result = append(result, existingFinalizer)
+func removeObjFinalizerPatch(obj metav1.Object, finalizer string) []byte {
+ for i, existingFinalizer := range obj.GetFinalizers() {
+ if existingFinalizer == finalizer {
+ return []byte(`[{"op":"remove","path":"/metadata/finalizers/` + strconv.FormatInt(int64(i), 10) + `"}]`)
  }
  }
 
- return result
+ return nil
 }
 
 func containRelated(arr []policyv1.RelatedObject, input policyv1.RelatedObject) bool {