Merge pull request #328 from JackZxj/fix/override-finalizers

fix(override): support overwriting finalizers via OverridePolicy

pkg/controllers/sync/dispatch/managed.go

@@ -194,6 +194,9 @@ func (d *managedDispatcherImpl) Create(ctx context.Context, clusterName string)
 			return result
 		}
 
+		// Initialize finalizers to an empty list to avoid missing-path-error of JSONPatch.
+		obj.SetFinalizers([]string{})
+
 		err = d.fedResource.ApplyOverrides(obj, clusterName)
 		if err != nil {
 			result = d.recordOperationError(ctx, fedcorev1a1.ApplyOverridesFailed, clusterName, op, err)
@@ -316,12 +319,35 @@ func (d *managedDispatcherImpl) Update(ctx context.Context, clusterName string,
 			return result
 		}
 
+		// Retain finalizers since they will typically be set by
+		// controllers in a member cluster.  It is still possible to set the fields
+		// via overrides.
+		if fs := clusterObj.GetFinalizers(); len(fs) > 0 {
+			obj.SetFinalizers(fs)
+		} else {
+			// Initialize finalizers to an empty list to avoid missing-path-error of JSONPatch.
+			obj.SetFinalizers([]string{})
+		}
+
 		err = d.fedResource.ApplyOverrides(obj, clusterName)
 		if err != nil {
 			result = d.recordOperationError(ctx, fedcorev1a1.ApplyOverridesFailed, clusterName, op, err)
 			return result
 		}
 
+		// make finalizers unique
+		if fs := obj.GetFinalizers(); len(fs) > 0 {
+			i := 0
+			fSet := sets.NewString()
+			for _, v := range fs {
+				if fSet.Len() != fSet.Insert(v).Len() {
+					fs[i] = v
+					i++
+				}
+			}
+			obj.SetFinalizers(fs[:i])
+		}
+
 		recordPropagatedLabelsAndAnnotations(obj)
 
 		err = RetainOrMergeClusterFields(d.fedResource.TargetGVK(), obj, clusterObj)

pkg/controllers/sync/dispatch/retain.go

@@ -48,10 +48,9 @@ func RetainOrMergeClusterFields(
 	// Pass the same ResourceVersion as in the cluster object for update operation, otherwise operation will fail.
 	desiredObj.SetResourceVersion(clusterObj.GetResourceVersion())
 
-	// Retain finalizers and merge annotations since they will typically be set by
+	// Merge annotations since they will typically be set by
 	// controllers in a member cluster.  It is still possible to set the fields
 	// via overrides.
-	desiredObj.SetFinalizers(clusterObj.GetFinalizers())
 	mergeAnnotations(desiredObj, clusterObj)
 	mergeLabels(desiredObj, clusterObj)