From cb49360d2263bd357ee83377fac50f1eef93aa9c Mon Sep 17 00:00:00 2001
From: "zhangxinjie.next" <zhangxinjie.next@bytedance.com>
Date: Wed, 18 Sep 2024 14:35:42 +0800
Subject: [PATCH] fix(override): failed to override finalizers

---
 pkg/controllers/override/util.go      | 15 ++++++++++-
 pkg/controllers/override/util_test.go | 39 ++++++++++++++++++++++++++-
 2 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/pkg/controllers/override/util.go b/pkg/controllers/override/util.go
index 90a75775..67cb49fe 100644
--- a/pkg/controllers/override/util.go
+++ b/pkg/controllers/override/util.go
@@ -182,7 +182,9 @@ func parseOverrides(
 				)
 			}
 
-			if err = ctrutil.ApplyJSONPatch(helper.sourceObj, currPatches); err != nil {
+			// Ignore finalizer patches because they depend on objects in the member cluster.
+			// The finalizer patches will be applied in sync controller.
+			if err = ctrutil.ApplyJSONPatch(helper.sourceObj, ignoreFinalizerPatches(currPatches)); err != nil {
 				return nil, err
 			}
 			helpers[cluster.Name] = helper
@@ -342,6 +344,17 @@ func parsePatchesFromOverriders(
 	return patches, nil
 }
 
+func ignoreFinalizerPatches(p fedcorev1a1.OverridePatches) fedcorev1a1.OverridePatches {
+	newPatches := make(fedcorev1a1.OverridePatches, 0, len(p))
+	for _, patch := range p {
+		if strings.HasPrefix(patch.Path, "/metadata/finalizers") {
+			continue
+		}
+		newPatches = append(newPatches, patch)
+	}
+	return newPatches
+}
+
 func getGVKFromFederatedObject(fedObject fedcorev1a1.GenericFederatedObject) (schema.GroupVersionKind, error) {
 	if fedObject == nil {
 		return schema.GroupVersionKind{}, fmt.Errorf("fedObject is nil")
diff --git a/pkg/controllers/override/util_test.go b/pkg/controllers/override/util_test.go
index 4db40ff8..c7899fef 100644
--- a/pkg/controllers/override/util_test.go
+++ b/pkg/controllers/override/util_test.go
@@ -829,7 +829,7 @@ func TestParseOverrides(t *testing.T) {
 			expectedOverridesMap: make(overridesMap),
 			isErrorExpected:      false,
 		},
-		"multiple clusters multiple Overrides(image, command, args, envs, annotations, labels, jsonPatch)" +
+		"multiple clusters multiple Overrides(image, command, args, envs, annotations, labels, jsonPatch with finalizers)" +
 			"- should return overrides for each cluster in order": {
 			fedObject: generateFedObjWithPodWithTwoNormalAndTwoInit(
 				"docker.io/ealen/echo-server:latest@sha256:bbbbf56b44807c64d294e6c8059b479f35350b454492398225034174808d1726"),
@@ -899,6 +899,13 @@ func TestParseOverrides(t *testing.T) {
 											Raw: []byte(`"all"`),
 										},
 									},
+									{
+										Operator: "add",
+										Path:     "/metadata/finalizers/-",
+										Value: apiextensionsv1.JSON{
+											Raw: []byte(`"foo.bar.com/test"`),
+										},
+									},
 								},
 							},
 						},
@@ -940,6 +947,13 @@ func TestParseOverrides(t *testing.T) {
 											Raw: []byte(`"cluster1"`),
 										},
 									},
+									{
+										Operator: "replace",
+										Path:     "/metadata/finalizers/0",
+										Value: apiextensionsv1.JSON{
+											Raw: []byte(`"foo.bar.com/cluster1"`),
+										},
+									},
 								},
 							},
 						},
@@ -978,6 +992,10 @@ func TestParseOverrides(t *testing.T) {
 										Operator: "remove",
 										Path:     "/metadata/labels/json2",
 									},
+									{
+										Operator: "remove",
+										Path:     "/metadata/finalizers/0",
+									},
 								},
 							},
 						},
@@ -1048,6 +1066,11 @@ func TestParseOverrides(t *testing.T) {
 						Path:  "/metadata/labels/json2",
 						Value: asJSON("all"),
 					},
+					{
+						Op:    "add",
+						Path:  "/metadata/finalizers/-",
+						Value: asJSON("foo.bar.com/test"),
+					},
 					// patches from overrideRules which apply to cluster-1
 					// image
 					generatePatch(
@@ -1079,6 +1102,11 @@ func TestParseOverrides(t *testing.T) {
 						Path:  "/metadata/labels/json1",
 						Value: asJSON("cluster1"),
 					},
+					{
+						Op:    "replace",
+						Path:  "/metadata/finalizers/0",
+						Value: asJSON("foo.bar.com/cluster1"),
+					},
 				},
 				"cluster2": fedcorev1a1.OverridePatches{
 					// patches from overrideRules which apply to all clusters
@@ -1131,6 +1159,11 @@ func TestParseOverrides(t *testing.T) {
 						Path:  "/metadata/labels/json2",
 						Value: asJSON("all"),
 					},
+					{
+						Op:    "add",
+						Path:  "/metadata/finalizers/-",
+						Value: asJSON("foo.bar.com/test"),
+					},
 					// patches from overrideRules which apply to cluster-2
 					generatePatch(
 						OperatorReplace,
@@ -1156,6 +1189,10 @@ func TestParseOverrides(t *testing.T) {
 						Op:   "remove",
 						Path: "/metadata/labels/json2",
 					},
+					{
+						Op:   "remove",
+						Path: "/metadata/finalizers/0",
+					},
 				},
 			},
 			isErrorExpected: false,