Merge pull request #336 from qclc/fix-op

fix: the previous cop is completely overwritten when binding op

pkg/controllers/override/annotationslabelspatch.go

@@ -27,22 +27,17 @@ import (
 )
 
 func parseStringMapOverriders(
-	fedObject fedcorev1a1.GenericFederatedObject,
+	helper *helpData,
 	overriders []fedcorev1a1.StringMapOverrider,
 	target string,
 ) (fedcorev1a1.OverridePatches, error) {
 	if len(overriders) == 0 {
 		return fedcorev1a1.OverridePatches{}, nil
 	}
-
-	sourceObj, err := fedObject.GetSpec().GetTemplateAsUnstructured()
-	if err != nil {
-		return nil, fmt.Errorf("failed to get sourceObj from fedObj: %w", err)
-	}
-
 	// get labels or annotations of sourceObj
-	mapValue := getLabelsOrAnnotationsFromObject(sourceObj, target)
+	mapValue := getLabelsOrAnnotationsFromObject(helper.sourceObj, target)
 
+	var err error
 	// apply StringMapOverriders to mapValue
 	for index := range overriders {
 		mapValue, err = applyStringMapOverrider(mapValue, &overriders[index])

pkg/controllers/override/annotationslabelspatch_test.go

@@ -252,7 +252,11 @@ func Test_parseAnnotationsOrLabelsOverriders(t *testing.T) {
 		testCases := generateTestCases(testTarget)
 		for testName, testCase := range testCases {
 			t.Run(testName, func(t *testing.T) {
-				overridePatches, err := parseStringMapOverriders(testCase.fedObject, testCase.stringMapOverriders, testTarget)
+				helper, err := getHelpDataFromFedObj(testCase.fedObject)
+				if err != nil {
+					t.Fatalf("failed to construct helper: %v", err)
+				}
+				overridePatches, err := parseStringMapOverriders(helper, testCase.stringMapOverriders, testTarget)
 				if (err != nil) != testCase.isErrorExpected {
 					t.Fatalf("err = %v, but testCase.isErrorExpected = %v", err, testCase.isErrorExpected)
 				}

pkg/controllers/override/commandargspatch.go

@@ -31,7 +31,7 @@ import (
 
 // parseEntrypointOverriders parse OverridePatches from command/args overriders
 func parseEntrypointOverriders(
-	fedObject fedcorev1a1.GenericFederatedObject,
+	helper *helpData,
 	overriders []fedcorev1a1.EntrypointOverrider,
 	target string,
 ) (fedcorev1a1.OverridePatches, error) {
@@ -42,12 +42,8 @@ func parseEntrypointOverriders(
 	// patchMap(<targetPath, []target]>) is used to store the newest command/args values of each command path
 	patchMap := make(map[string][]string)
 
-	// get gvk from fedObj
-	gvk, err := getGVKFromFederatedObject(fedObject)
-	if err != nil {
-		return nil, err
-	}
-	podSpec, err := podutil.GetResourcePodSpec(fedObject, gvk)
+	// get podSpec from sourceObj
+	podSpec, err := podutil.GetResourcePodSpecFromUnstructuredObj(helper.sourceObj, helper.gvk)
 	if err != nil {
 		return nil, fmt.Errorf("failed to get podSpec from sourceObj: %w", err)
 	}
@@ -61,7 +57,7 @@ func parseEntrypointOverriders(
 			continue
 		}
 
-		targetPath, err := generateTargetPathForPodSpec(gvk, containerKind, target, containerIndex)
+		targetPath, err := generateTargetPathForPodSpec(helper.gvk, containerKind, target, containerIndex)
 		if err != nil {
 			return nil, err
 		}

pkg/controllers/override/commandargspatch_test.go

@@ -646,7 +646,11 @@ func Test_parseCommandOrArgsOverriders(t *testing.T) {
 	for _, testTarget := range testTargets {
 		for testName, testCase := range generateTestCases(testTarget) {
 			t.Run(testName, func(t *testing.T) {
-				overridePatches, err := parseEntrypointOverriders(testCase.fedObject, testCase.entrypointOverriders, testTarget)
+				helper, err := getHelpDataFromFedObj(testCase.fedObject)
+				if err != nil {
+					t.Fatalf("failed to construct helper: %v", err)
+				}
+				overridePatches, err := parseEntrypointOverriders(helper, testCase.entrypointOverriders, testTarget)
 				if (err != nil) != testCase.isErrorExpected {
 					t.Fatalf("err = %v, but testCase.isErrorExpected = %v", err, testCase.isErrorExpected)
 				}

pkg/controllers/override/envspatch.go

@@ -29,7 +29,7 @@ import (
 )
 
 func parseEnvOverriders(
-	fedObject fedcorev1a1.GenericFederatedObject,
+	helper *helpData,
 	overriders []fedcorev1a1.EnvOverrider,
 ) (fedcorev1a1.OverridePatches, error) {
 	if len(overriders) == 0 {
@@ -39,12 +39,7 @@ func parseEnvOverriders(
 	// patchMap(<targetPath, []target]>) is used to store the newest env values of each command path
 	patchMap := make(map[string][]corev1.EnvVar)
 
-	// get gvk from fedObj
-	gvk, err := getGVKFromFederatedObject(fedObject)
-	if err != nil {
-		return nil, err
-	}
-	podSpec, err := podutil.GetResourcePodSpec(fedObject, gvk)
+	podSpec, err := podutil.GetResourcePodSpecFromUnstructuredObj(helper.sourceObj, helper.gvk)
 	if err != nil {
 		return nil, fmt.Errorf("failed to get podSpec from sourceObj: %w", err)
 	}
@@ -58,7 +53,7 @@ func parseEnvOverriders(
 			continue
 		}
 
-		targetPath, err := generateTargetPathForPodSpec(gvk, containerKind, EnvTarget, containerIndex)
+		targetPath, err := generateTargetPathForPodSpec(helper.gvk, containerKind, EnvTarget, containerIndex)
 		if err != nil {
 			return nil, err
 		}

pkg/controllers/override/envspatch_test.go

@@ -1089,7 +1089,11 @@ func Test_parseEnvOverriders(t *testing.T) {
 
 	for testName, testCase := range generateTestCases() {
 		t.Run(testName, func(t *testing.T) {
-			overridePatches, err := parseEnvOverriders(testCase.fedObject, testCase.envOverriders)
+			helper, err := getHelpDataFromFedObj(testCase.fedObject)
+			if err != nil {
+				t.Fatalf("failed to construct helper: %v", err)
+			}
+			overridePatches, err := parseEnvOverriders(helper, testCase.envOverriders)
 			if (err != nil) != testCase.isErrorExpected {
 				t.Fatalf("err = %v, but testCase.isErrorExpected = %v", err, testCase.isErrorExpected)
 			}

pkg/controllers/override/imagepatch.go

@@ -39,7 +39,7 @@ const (
 )
 
 func parseImageOverriders(
-	fedObject fedcorev1a1.GenericFederatedObject,
+	helper *helpData,
 	imageOverriders []fedcorev1a1.ImageOverrider,
 ) (fedcorev1a1.OverridePatches, error) {
 	// patchMap(<imagePath, imageValue>) is used to store the newest image value of each image path
@@ -49,11 +49,11 @@ func parseImageOverriders(
 	for i := range imageOverriders {
 		imageOverrider := &imageOverriders[i]
 		if imageOverrider.ImagePath != "" {
-			if err := parsePatchesFromImagePath(fedObject, imageOverrider, patchMap); err != nil {
+			if err := parsePatchesFromImagePath(helper, imageOverrider, patchMap); err != nil {
 				return nil, err
 			}
 		} else {
-			if err := parsePatchesFromWorkload(fedObject, imageOverrider, patchMap); err != nil {
+			if err := parsePatchesFromWorkload(helper, imageOverrider, patchMap); err != nil {
 				return nil, err
 			}
 		}
@@ -83,10 +83,10 @@ func parseImageOverriders(
 // parsePatchesFromImagePath applies overrider to image value on the specified path,
 // and generates a patch which is stored in patchMap
 func parsePatchesFromImagePath(
-	fedObject fedcorev1a1.GenericFederatedObject,
+	helper *helpData,
 	imageOverrider *fedcorev1a1.ImageOverrider,
 	patchMap map[string]string,
-) error {
+) (err error) {
 	imagePath := imageOverrider.ImagePath
 	if !strings.HasPrefix(imagePath, pathSeparator) {
 		return fmt.Errorf("image path should be start with %s", pathSeparator)
@@ -95,13 +95,7 @@ func parsePatchesFromImagePath(
 
 	// get image value
 	if imageValue == "" {
-		// get source obj
-		sourceObj, err := fedObject.GetSpec().GetTemplateAsUnstructured()
-		if err != nil {
-			return fmt.Errorf("failed to get sourceObj from fedObj: %w", err)
-		}
-
-		imageValue, err = GetStringFromUnstructuredObj(sourceObj, imageOverrider.ImagePath)
+		imageValue, err = GetStringFromUnstructuredObj(helper.sourceObj, imageOverrider.ImagePath)
 		if err != nil {
 			return fmt.Errorf("failed to parse image value from unstructured obj: %w", err)
 		}
@@ -120,16 +114,13 @@ func parsePatchesFromImagePath(
 // parsePatchesFromWorkload applies overrider to image value on the default path of workload,
 // and generates a patch which is stored in patchMap
 func parsePatchesFromWorkload(
-	fedObject fedcorev1a1.GenericFederatedObject,
+	helper *helpData,
 	imageOverrider *fedcorev1a1.ImageOverrider,
 	patchMap map[string]string,
 ) error {
 	// get pod spec from fedObj
-	gvk, err := getGVKFromFederatedObject(fedObject)
-	if err != nil {
-		return err
-	}
-	podSpec, err := podutil.GetResourcePodSpec(fedObject, gvk)
+	gvk := helper.gvk
+	podSpec, err := podutil.GetResourcePodSpecFromUnstructuredObj(helper.sourceObj, gvk)
 	if err != nil {
 		return fmt.Errorf("failed to get podSpec from sourceObj: %w", err)
 	}

pkg/controllers/override/imagepatch_test.go

@@ -912,7 +912,11 @@ func Test_parseImageOverriders(t *testing.T) {
 
 	for testName, testCase := range testCases {
 		t.Run(testName, func(t *testing.T) {
-			overridePatches, err := parseImageOverriders(testCase.fedObject, testCase.imageOverriders)
+			helper, err := getHelpDataFromFedObj(testCase.fedObject)
+			if err != nil {
+				t.Fatalf("failed to construct helper: %v", err)
+			}
+			overridePatches, err := parseImageOverriders(helper, testCase.imageOverriders)
 			if (err != nil) != testCase.isErrorExpected {
 				t.Fatalf("err = %v, but testCase.isErrorExpected = %v", err, testCase.isErrorExpected)
 			}
@@ -948,7 +952,8 @@ var (
 			"apiVersion": "apps/v1",
 			"kind":       "Deployment",
 			"metadata": map[string]interface{}{
-				"name": "deployment-test",
+				"name":   "deployment-test",
+				"labels": map[string]interface{}{},
 			},
 			"spec": map[string]interface{}{
 				"replicas": int64(1),
@@ -1036,7 +1041,8 @@ var (
 			"apiVersion": "v1",
 			"kind":       "Pod",
 			"metadata": map[string]interface{}{
-				"name": "pod-test",
+				"name":   "pod-test",
+				"labels": map[string]interface{}{},
 			},
 			"spec": map[string]interface{}{
 				"containers": []interface{}{},

pkg/controllers/override/overridepolicy_controller.go

@@ -323,9 +323,10 @@ func (c *Controller) reconcile(ctx context.Context, qualifiedName common.Qualifi
 	}
 
 	var overrides, currentOverrides overridesMap
+	helpers := map[string]*helpData{}
 	// Apply overrides from each policy in order
 	for _, policy := range policies {
-		newOverrides, err := parseOverrides(policy, placedClusters, fedObject)
+		newOverrides, err := parseOverrides(policy, placedClusters, fedObject, helpers)
 		if err != nil {
 			c.eventRecorder.Eventf(
 				fedObject,

pkg/controllers/override/util.go

@@ -23,11 +23,13 @@ import (
 
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 
 	fedcorev1a1 "github.com/kubewharf/kubeadmiral/pkg/apis/core/v1alpha1"
 	fedcorev1a1listers "github.com/kubewharf/kubeadmiral/pkg/client/listers/core/v1alpha1"
+	ctrutil "github.com/kubewharf/kubeadmiral/pkg/controllers/util"
 	"github.com/kubewharf/kubeadmiral/pkg/util/clusterselector"
 	podutil "github.com/kubewharf/kubeadmiral/pkg/util/pod"
 )
@@ -112,12 +114,36 @@ func lookForMatchedPolicies(
 	return policies, false, nil
 }
 
+type helpData struct {
+	gvk       schema.GroupVersionKind
+	sourceObj *unstructured.Unstructured
+}
+
+func getHelpDataFromFedObj(fedObj fedcorev1a1.GenericFederatedObject) (*helpData, error) {
+	gvk, err := getGVKFromFederatedObject(fedObj)
+	if err != nil {
+		return nil, fmt.Errorf("failed to get gvk from fedObj: %w", err)
+	}
+	sourceObj, err := fedObj.GetSpec().GetTemplateAsUnstructured()
+	if err != nil {
+		return nil, fmt.Errorf("failed to get sourceObj from fedObj: %w", err)
+	}
+	return &helpData{
+		gvk:       gvk,
+		sourceObj: sourceObj,
+	}, nil
+}
+
 func parseOverrides(
 	policy fedcorev1a1.GenericOverridePolicy,
 	clusters []*fedcorev1a1.FederatedCluster,
 	fedObject fedcorev1a1.GenericFederatedObject,
+	helpers map[string]*helpData,
 ) (overridesMap, error) {
 	overridesMap := make(overridesMap)
+	if helpers == nil {
+		helpers = make(map[string]*helpData)
+	}
 
 	for _, cluster := range clusters {
 		patches := make(fedcorev1a1.OverridePatches, 0)
@@ -139,7 +165,14 @@ func parseOverrides(
 				continue
 			}
 
-			currPatches, err := parsePatchesFromOverriders(fedObject, rule.Overriders)
+			var helper *helpData
+			if v, ok := helpers[cluster.Name]; ok {
+				helper = v
+			} else if helper, err = getHelpDataFromFedObj(fedObject); err != nil {
+				return nil, err
+			}
+
+			currPatches, err := parsePatchesFromOverriders(helper, rule.Overriders)
 			if err != nil {
 				return nil, fmt.Errorf(
 					"failed to parse patches from policy %q's overrideRules[%v], error: %w",
@@ -148,6 +181,12 @@ func parseOverrides(
 					err,
 				)
 			}
+
+			if err = ctrutil.ApplyJSONPatch(helper.sourceObj, currPatches); err != nil {
+				return nil, err
+			}
+			helpers[cluster.Name] = helper
+
 			patches = append(patches, currPatches...)
 		}
 
@@ -241,7 +280,7 @@ func isClusterMatchedByClusterAffinity(
 }
 
 func parsePatchesFromOverriders(
-	fedObject fedcorev1a1.GenericFederatedObject,
+	helper *helpData,
 	overriders *fedcorev1a1.Overriders,
 ) (fedcorev1a1.OverridePatches, error) {
 	patches := make(fedcorev1a1.OverridePatches, 0)
@@ -252,42 +291,42 @@ func parsePatchesFromOverriders(
 	}
 
 	// parse patches from image overriders
-	if imagePatches, err := parseImageOverriders(fedObject, overriders.Image); err != nil {
+	if imagePatches, err := parseImageOverriders(helper, overriders.Image); err != nil {
 		return nil, fmt.Errorf("failed to parse image overriders: %w", err)
 	} else {
 		patches = append(patches, imagePatches...)
 	}
 
 	// parse patches from command overriders
-	if commandPatches, err := parseEntrypointOverriders(fedObject, overriders.Command, CommandTarget); err != nil {
+	if commandPatches, err := parseEntrypointOverriders(helper, overriders.Command, CommandTarget); err != nil {
 		return nil, fmt.Errorf("failed to parse command overriders: %w", err)
 	} else {
 		patches = append(patches, commandPatches...)
 	}
 
 	// parse patches from args overriders
-	if argsPatches, err := parseEntrypointOverriders(fedObject, overriders.Args, ArgsTarget); err != nil {
+	if argsPatches, err := parseEntrypointOverriders(helper, overriders.Args, ArgsTarget); err != nil {
 		return nil, fmt.Errorf("failed to parse args overriders: %w", err)
 	} else {
 		patches = append(patches, argsPatches...)
 	}
 
 	// parse patches from env overriders
-	if envsPatches, err := parseEnvOverriders(fedObject, overriders.Envs); err != nil {
+	if envsPatches, err := parseEnvOverriders(helper, overriders.Envs); err != nil {
 		return nil, fmt.Errorf("failed to parse env overriders: %w", err)
 	} else {
 		patches = append(patches, envsPatches...)
 	}
 
 	// parse patches from annotation overriders
-	if annotationsPatches, err := parseStringMapOverriders(fedObject, overriders.Annotations, AnnotationsTarget); err != nil {
+	if annotationsPatches, err := parseStringMapOverriders(helper, overriders.Annotations, AnnotationsTarget); err != nil {
 		return nil, fmt.Errorf("failed to parse annotations overriders: %w", err)
 	} else {
 		patches = append(patches, annotationsPatches...)
 	}
 
 	// parse patches from labels overriders
-	if labelsPatches, err := parseStringMapOverriders(fedObject, overriders.Labels, LabelsTarget); err != nil {
+	if labelsPatches, err := parseStringMapOverriders(helper, overriders.Labels, LabelsTarget); err != nil {
 		return nil, fmt.Errorf("failed to parse labels overriders: %w", err)
 	} else {
 		patches = append(patches, labelsPatches...)