From f16c725329f2f90d304cd91aa2a33f5e2be9b3fc Mon Sep 17 00:00:00 2001 From: "chentao.cht" Date: Tue, 3 Sep 2024 14:45:29 +0800 Subject: [PATCH] fix: the previous cop is completely overwritten when binding op --- .../override/annotationslabelspatch.go | 11 +- .../override/annotationslabelspatch_test.go | 6 +- pkg/controllers/override/commandargspatch.go | 12 +- .../override/commandargspatch_test.go | 6 +- pkg/controllers/override/envspatch.go | 11 +- pkg/controllers/override/envspatch_test.go | 6 +- pkg/controllers/override/imagepatch.go | 27 +- pkg/controllers/override/imagepatch_test.go | 12 +- .../override/overridepolicy_controller.go | 3 +- pkg/controllers/override/util.go | 55 +++- pkg/controllers/override/util_test.go | 268 +++++++++++------- pkg/controllers/sync/overrides.go | 38 --- pkg/controllers/sync/resource.go | 3 +- pkg/controllers/util/overridepolicy.go | 62 ++++ pkg/util/pod/pod.go | 32 ++- 15 files changed, 339 insertions(+), 213 deletions(-) create mode 100644 pkg/controllers/util/overridepolicy.go diff --git a/pkg/controllers/override/annotationslabelspatch.go b/pkg/controllers/override/annotationslabelspatch.go index 31006328..23045bcd 100644 --- a/pkg/controllers/override/annotationslabelspatch.go +++ b/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]) diff --git a/pkg/controllers/override/annotationslabelspatch_test.go b/pkg/controllers/override/annotationslabelspatch_test.go index 40dac253..9442a15e 100644 --- a/pkg/controllers/override/annotationslabelspatch_test.go +++ b/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) } diff --git a/pkg/controllers/override/commandargspatch.go b/pkg/controllers/override/commandargspatch.go index 40cf2e68..15fa96b8 100644 --- a/pkg/controllers/override/commandargspatch.go +++ b/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() 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 } diff --git a/pkg/controllers/override/commandargspatch_test.go b/pkg/controllers/override/commandargspatch_test.go index f026fe89..98bbb717 100644 --- a/pkg/controllers/override/commandargspatch_test.go +++ b/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) } diff --git a/pkg/controllers/override/envspatch.go b/pkg/controllers/override/envspatch.go index dc2208b4..629ce370 100644 --- a/pkg/controllers/override/envspatch.go +++ b/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() 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 } diff --git a/pkg/controllers/override/envspatch_test.go b/pkg/controllers/override/envspatch_test.go index ddb1432f..d1c3d10a 100644 --- a/pkg/controllers/override/envspatch_test.go +++ b/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) } diff --git a/pkg/controllers/override/imagepatch.go b/pkg/controllers/override/imagepatch.go index 274799c1..c0ded09d 100644 --- a/pkg/controllers/override/imagepatch.go +++ b/pkg/controllers/override/imagepatch.go @@ -39,7 +39,7 @@ const ( ) func parseImageOverriders( - fedObject fedcorev1a1.GenericFederatedObject, + helper *helpData, imageOverriders []fedcorev1a1.ImageOverrider, ) (fedcorev1a1.OverridePatches, error) { // patchMap() 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) } diff --git a/pkg/controllers/override/imagepatch_test.go b/pkg/controllers/override/imagepatch_test.go index 013205d4..dc25046b 100644 --- a/pkg/controllers/override/imagepatch_test.go +++ b/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{}{}, diff --git a/pkg/controllers/override/overridepolicy_controller.go b/pkg/controllers/override/overridepolicy_controller.go index 203e7b11..73cb4574 100644 --- a/pkg/controllers/override/overridepolicy_controller.go +++ b/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, diff --git a/pkg/controllers/override/util.go b/pkg/controllers/override/util.go index 44b03d8a..90a75775 100644 --- a/pkg/controllers/override/util.go +++ b/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...) diff --git a/pkg/controllers/override/util_test.go b/pkg/controllers/override/util_test.go index 1c756480..4db40ff8 100644 --- a/pkg/controllers/override/util_test.go +++ b/pkg/controllers/override/util_test.go @@ -22,8 +22,10 @@ import ( "testing" "github.com/stretchr/testify/assert" + corev1 "k8s.io/api/core/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" jsonutil "k8s.io/apimachinery/pkg/util/json" "k8s.io/client-go/tools/cache" @@ -393,6 +395,7 @@ func TestParseOverrides(t *testing.T) { isErrorExpected: false, }, "single cluster multiple OverrideRules - should return overrides from matched rules in order": { + fedObject: generateCustomObjectForJSONTest(), policy: &fedcorev1a1.OverridePolicy{ Spec: fedcorev1a1.GenericOverridePolicySpec{ OverrideRules: []fedcorev1a1.OverrideRule{ @@ -496,6 +499,7 @@ func TestParseOverrides(t *testing.T) { isErrorExpected: false, }, "multiple clusters multiple Overrides - should return overrides for each cluster in order": { + fedObject: generateCustomObjectForJSONTest(), policy: &fedcorev1a1.OverridePolicy{ Spec: fedcorev1a1.GenericOverridePolicySpec{ OverrideRules: []fedcorev1a1.OverrideRule{ @@ -649,16 +653,16 @@ func TestParseOverrides(t *testing.T) { JsonPatch: []fedcorev1a1.JsonPatchOverrider{ { Operator: "add", - Path: "/a/b", + Path: "/metadata/labels/json1", Value: apiextensionsv1.JSON{ - Raw: []byte(`1`), + Raw: []byte(`"all"`), }, }, { - Operator: "replace", - Path: "/aa/bb", + Operator: "add", + Path: "/metadata/labels/json2", Value: apiextensionsv1.JSON{ - Raw: []byte(`["banana","mango"]`), + Raw: []byte(`"all"`), }, }, }, @@ -684,16 +688,9 @@ func TestParseOverrides(t *testing.T) { JsonPatch: []fedcorev1a1.JsonPatchOverrider{ { Operator: "replace", - Path: "/c/d", - Value: apiextensionsv1.JSON{ - Raw: []byte(`1`), - }, - }, - { - Operator: "replace", - Path: "/cc/dd", + Path: "/metadata/labels/json1", Value: apiextensionsv1.JSON{ - Raw: []byte(`{"key":"value"}`), + Raw: []byte(`"cluster1"`), }, }, }, @@ -719,14 +716,7 @@ func TestParseOverrides(t *testing.T) { JsonPatch: []fedcorev1a1.JsonPatchOverrider{ { Operator: "remove", - Path: "/e/f", - }, - { - Operator: "add", - Path: "/ee/ff", - Value: apiextensionsv1.JSON{ - Raw: []byte(`"some string"`), - }, + Path: "/metadata/labels/json2", }, }, Image: []fedcorev1a1.ImageOverrider{ @@ -765,13 +755,13 @@ func TestParseOverrides(t *testing.T) { ), { Op: "add", - Path: "/a/b", - Value: asJSON(float64(1)), + Path: "/metadata/labels/json1", + Value: asJSON("all"), }, { - Op: "replace", - Path: "/aa/bb", - Value: asJSON([]interface{}{"banana", "mango"}), + Op: "add", + Path: "/metadata/labels/json2", + Value: asJSON("all"), }, generatePatch( OperatorReplace, @@ -780,15 +770,8 @@ func TestParseOverrides(t *testing.T) { ), { Op: "replace", - Path: "/c/d", - Value: asJSON(float64(1)), - }, - { - Op: "replace", - Path: "/cc/dd", - Value: asJSON(map[string]interface{}{ - "key": "value", - }), + Path: "/metadata/labels/json1", + Value: asJSON("cluster1"), }, }, "cluster2": fedcorev1a1.OverridePatches{ @@ -799,13 +782,13 @@ func TestParseOverrides(t *testing.T) { ), { Op: "add", - Path: "/a/b", - Value: asJSON(float64(1)), + Path: "/metadata/labels/json1", + Value: asJSON("all"), }, { - Op: "replace", - Path: "/aa/bb", - Value: asJSON([]interface{}{"banana", "mango"}), + Op: "add", + Path: "/metadata/labels/json2", + Value: asJSON("all"), }, generatePatch( OperatorReplace, @@ -814,18 +797,15 @@ func TestParseOverrides(t *testing.T) { ), { Op: "remove", - Path: "/e/f", - }, - { - Op: "add", - Path: "/ee/ff", - Value: asJSON("some string"), + Path: "/metadata/labels/json2", }, }, }, isErrorExpected: false, }, "OverrideRule.Overriders is nil - should return no overrides": { + fedObject: generateFedDeploymentWithImage( + "docker.io/ealen/echo-server:latest@sha256:bbbbf56b44807c64d294e6c8059b479f35350b454492398225034174808d1726"), policy: &fedcorev1a1.OverridePolicy{ Spec: fedcorev1a1.GenericOverridePolicySpec{ OverrideRules: []fedcorev1a1.OverrideRule{ @@ -849,7 +829,7 @@ func TestParseOverrides(t *testing.T) { expectedOverridesMap: make(overridesMap), isErrorExpected: false, }, - "multiple clusters multiple Overrides(image, command, args, annotations, labels, jsonPatch)" + + "multiple clusters multiple Overrides(image, command, args, envs, annotations, labels, jsonPatch)" + "- should return overrides for each cluster in order": { fedObject: generateFedObjWithPodWithTwoNormalAndTwoInit( "docker.io/ealen/echo-server:latest@sha256:bbbbf56b44807c64d294e6c8059b479f35350b454492398225034174808d1726"), @@ -883,6 +863,9 @@ func TestParseOverrides(t *testing.T) { generateEntrypointOverrider("init-server-1", OperatorAppend, "-a", "-b"), generateEntrypointOverrider("server-1", OperatorOverwrite, "-c", "-d"), }, + Envs: []fedcorev1a1.EnvOverrider{ + generateEnvOverrider("server-1", OperatorAddIfAbsent, corev1.EnvVar{Name: "all", Value: "xxx"}), + }, Annotations: []fedcorev1a1.StringMapOverrider{ generateStringMapOverrider(OperatorAddIfAbsent, map[string]string{ "abc": "xxx", @@ -904,16 +887,16 @@ func TestParseOverrides(t *testing.T) { JsonPatch: []fedcorev1a1.JsonPatchOverrider{ { Operator: "add", - Path: "/a/b", + Path: "/metadata/labels/json1", Value: apiextensionsv1.JSON{ - Raw: []byte(`1`), + Raw: []byte(`"all"`), }, }, { - Operator: "replace", - Path: "/aa/bb", + Operator: "add", + Path: "/metadata/labels/json2", Value: apiextensionsv1.JSON{ - Raw: []byte(`["banana","mango"]`), + Raw: []byte(`"all"`), }, }, }, @@ -926,22 +909,6 @@ func TestParseOverrides(t *testing.T) { }, }, Overriders: &fedcorev1a1.Overriders{ - JsonPatch: []fedcorev1a1.JsonPatchOverrider{ - { - Operator: "replace", - Path: "/c/d", - Value: apiextensionsv1.JSON{ - Raw: []byte(`1`), - }, - }, - { - Operator: "replace", - Path: "/cc/dd", - Value: apiextensionsv1.JSON{ - Raw: []byte(`{"key":"value"}`), - }, - }, - }, Image: []fedcorev1a1.ImageOverrider{ { ContainerNames: []string{"server-1"}, @@ -953,6 +920,27 @@ func TestParseOverrides(t *testing.T) { "sha256:aaaaf56b44807c64d294e6c8059b479f35350b454492398225034174808d1726"), }, }, + Args: []fedcorev1a1.EntrypointOverrider{ + generateEntrypointOverrider("server-1", OperatorDelete, "-d"), + generateEntrypointOverrider("init-server-1", OperatorAppend, "cluster1"), + }, + Envs: []fedcorev1a1.EnvOverrider{ + generateEnvOverrider("server-1", OperatorAddIfAbsent, corev1.EnvVar{Name: "cluster1", Value: "xxx"}), + }, + Annotations: []fedcorev1a1.StringMapOverrider{ + generateStringMapOverrider(OperatorAddIfAbsent, map[string]string{ + "cluster1": "xxx", + }), + }, + JsonPatch: []fedcorev1a1.JsonPatchOverrider{ + { + Operator: "replace", + Path: "/metadata/labels/json1", + Value: apiextensionsv1.JSON{ + Raw: []byte(`"cluster1"`), + }, + }, + }, }, }, { @@ -962,19 +950,6 @@ func TestParseOverrides(t *testing.T) { }, }, Overriders: &fedcorev1a1.Overriders{ - JsonPatch: []fedcorev1a1.JsonPatchOverrider{ - { - Operator: "remove", - Path: "/e/f", - }, - { - Operator: "add", - Path: "/ee/ff", - Value: apiextensionsv1.JSON{ - Raw: []byte(`"some string"`), - }, - }, - }, Image: []fedcorev1a1.ImageOverrider{ { ContainerNames: []string{"server-1"}, @@ -986,6 +961,24 @@ func TestParseOverrides(t *testing.T) { "sha256:aaaaf56b44807c64d294e6c8059b479f35350b454492398225034174808d1726"), }, }, + Command: []fedcorev1a1.EntrypointOverrider{ + generateEntrypointOverrider("server-1", OperatorAppend, "cluster2"), + generateEntrypointOverrider("init-server-1", OperatorOverwrite, "cluster2"), + }, + Labels: []fedcorev1a1.StringMapOverrider{ + generateStringMapOverrider(OperatorAddIfAbsent, map[string]string{ + "cluster2": "xxx", + }), + generateStringMapOverrider(OperatorOverwrite, map[string]string{ + "def": "cluster2", + }), + }, + JsonPatch: []fedcorev1a1.JsonPatchOverrider{ + { + Operator: "remove", + Path: "/metadata/labels/json2", + }, + }, }, }, }, @@ -1030,6 +1023,10 @@ func TestParseOverrides(t *testing.T) { "/spec/initContainers/0/args", []string{"-a", "-b"}, ), + // envs + generatePatch("replace", + "/spec/containers/0/env", + []corev1.EnvVar{{Name: "all", Value: "xxx"}}), // annotations generatePatch("replace", "/metadata/annotations", @@ -1043,13 +1040,13 @@ func TestParseOverrides(t *testing.T) { // jsonPatch { Op: "add", - Path: "/a/b", - Value: asJSON(float64(1)), + Path: "/metadata/labels/json1", + Value: asJSON("all"), }, { - Op: "replace", - Path: "/aa/bb", - Value: asJSON([]interface{}{"banana", "mango"}), + Op: "add", + Path: "/metadata/labels/json2", + Value: asJSON("all"), }, // patches from overrideRules which apply to cluster-1 // image @@ -1058,18 +1055,29 @@ func TestParseOverrides(t *testing.T) { "/spec/containers/0/image", "cluster.one.io/cluster-one/echo-server:one@sha256:aaaaf56b44807c64d294e6c8059b479f35350b454492398225034174808d1726", ), + // args + generatePatch("replace", + "/spec/containers/0/args", + []string{"-c"}, + ), + generatePatch("replace", + "/spec/initContainers/0/args", + []string{"-a", "-b", "cluster1"}, + ), + // envs + generatePatch("replace", + "/spec/containers/0/env", + []corev1.EnvVar{{Name: "all", Value: "xxx"}, {Name: "cluster1", Value: "xxx"}}), + // annotations + generatePatch("replace", + "/metadata/annotations", + map[string]string{"abc": "xxx", "cluster1": "xxx"}, + ), // jsonPatch { Op: "replace", - Path: "/c/d", - Value: asJSON(float64(1)), - }, - { - Op: "replace", - Path: "/cc/dd", - Value: asJSON(map[string]interface{}{ - "key": "value", - }), + Path: "/metadata/labels/json1", + Value: asJSON("cluster1"), }, }, "cluster2": fedcorev1a1.OverridePatches{ @@ -1098,6 +1106,10 @@ func TestParseOverrides(t *testing.T) { "/spec/initContainers/0/args", []string{"-a", "-b"}, ), + // envs + generatePatch("replace", + "/spec/containers/0/env", + []corev1.EnvVar{{Name: "all", Value: "xxx"}}), // annotations generatePatch("replace", "/metadata/annotations", @@ -1111,13 +1123,13 @@ func TestParseOverrides(t *testing.T) { // jsonPatch { Op: "add", - Path: "/a/b", - Value: asJSON(float64(1)), + Path: "/metadata/labels/json1", + Value: asJSON("all"), }, { - Op: "replace", - Path: "/aa/bb", - Value: asJSON([]interface{}{"banana", "mango"}), + Op: "add", + Path: "/metadata/labels/json2", + Value: asJSON("all"), }, // patches from overrideRules which apply to cluster-2 generatePatch( @@ -1125,14 +1137,24 @@ func TestParseOverrides(t *testing.T) { "/spec/containers/0/image", "cluster.two.io/cluster-two/echo-server:two@sha256:aaaaf56b44807c64d294e6c8059b479f35350b454492398225034174808d1726", ), + // command + generatePatch("replace", + "/spec/containers/0/command", + []string{"/bin/bash", "cluster2"}, + ), + generatePatch("replace", + "/spec/initContainers/0/command", + []string{"cluster2"}, + ), + // labels + generatePatch("replace", + "/metadata/labels", + map[string]string{"abc": "xxx", "def": "cluster2", "json1": "all", "json2": "all", "cluster2": "xxx"}, + ), + // jsonPatch { Op: "remove", - Path: "/e/f", - }, - { - Op: "add", - Path: "/ee/ff", - Value: asJSON("some string"), + Path: "/metadata/labels/json2", }, }, }, @@ -1142,7 +1164,7 @@ func TestParseOverrides(t *testing.T) { for testName, testCase := range testCases { t.Run(testName, func(t *testing.T) { - overrides, err := parseOverrides(testCase.policy, testCase.clusters, testCase.fedObject) + overrides, err := parseOverrides(testCase.policy, testCase.clusters, testCase.fedObject, nil) if (err != nil) != testCase.isErrorExpected { t.Fatalf("err = %v, but testCase.isErrorExpected = %v", err, testCase.isErrorExpected) } @@ -1699,3 +1721,33 @@ func TestConvertOverridesListToMap(t *testing.T) { } } } + +func generateCustomObjectForJSONTest() *fedcorev1a1.FederatedObject { + basicTemplate := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "example.com/v1", + "kind": "Test", + "metadata": map[string]interface{}{ + "name": "job-test", + }, + "spec": map[string]interface{}{}, + "a": map[string]interface{}{}, + "b": map[string]interface{}{}, + "c": map[string]interface{}{}, + "d": map[string]interface{}{}, + "e": map[string]interface{}{ + "f": "f", + }, + "f": map[string]interface{}{}, + "aa": map[string]interface{}{}, + "cc": map[string]interface{}{}, + "ee": map[string]interface{}{}, + }, + } + rawTargetTemplate, _ := basicTemplate.MarshalJSON() + return &fedcorev1a1.FederatedObject{ + Spec: fedcorev1a1.GenericFederatedObjectSpec{ + Template: apiextensionsv1.JSON{Raw: rawTargetTemplate}, + }, + } +} diff --git a/pkg/controllers/sync/overrides.go b/pkg/controllers/sync/overrides.go index f1d4dc6b..d4fd060e 100644 --- a/pkg/controllers/sync/overrides.go +++ b/pkg/controllers/sync/overrides.go @@ -21,13 +21,7 @@ are Copyright 2023 The KubeAdmiral Authors. package sync import ( - "encoding/json" - - jsonpatch "github.com/evanphx/json-patch" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/util/sets" - - fedcorev1a1 "github.com/kubewharf/kubeadmiral/pkg/apis/core/v1alpha1" ) // Namespace and name may not be overridden since these fields are the primary @@ -46,35 +40,3 @@ var invalidOverridePaths = sets.New( "/metadata/generateName", "/kind", ) - -// ApplyJSONPatch applies the override on to the given unstructured object. -func ApplyJSONPatch(obj *unstructured.Unstructured, overrides fedcorev1a1.OverridePatches) error { - // TODO: Do the defaulting of "op" field to "replace" in API defaulting - for i, overrideItem := range overrides { - if overrideItem.Op == "" { - overrides[i].Op = "replace" - } - } - jsonPatchBytes, err := json.Marshal(overrides) - if err != nil { - return err - } - - patch, err := jsonpatch.DecodePatch(jsonPatchBytes) - if err != nil { - return err - } - - objectJSONBytes, err := obj.MarshalJSON() - if err != nil { - return err - } - - patchedObjectJSONBytes, err := patch.Apply(objectJSONBytes) - if err != nil { - return err - } - - err = obj.UnmarshalJSON(patchedObjectJSONBytes) - return err -} diff --git a/pkg/controllers/sync/resource.go b/pkg/controllers/sync/resource.go index 07c1f0d1..f1aaf1a0 100644 --- a/pkg/controllers/sync/resource.go +++ b/pkg/controllers/sync/resource.go @@ -39,6 +39,7 @@ import ( "github.com/kubewharf/kubeadmiral/pkg/controllers/common" "github.com/kubewharf/kubeadmiral/pkg/controllers/sync/dispatch" "github.com/kubewharf/kubeadmiral/pkg/controllers/sync/version" + "github.com/kubewharf/kubeadmiral/pkg/controllers/util" annotationutil "github.com/kubewharf/kubeadmiral/pkg/util/annotation" "github.com/kubewharf/kubeadmiral/pkg/util/finalizers" "github.com/kubewharf/kubeadmiral/pkg/util/managedlabel" @@ -250,7 +251,7 @@ func (r *federatedResource) ApplyOverrides( return fmt.Errorf("invalid override path(s): %v", invalidPathsFound.UnsortedList()) } if overrides != nil { - if err := ApplyJSONPatch(obj, overrides); err != nil { + if err := util.ApplyJSONPatch(obj, overrides); err != nil { return err } } diff --git a/pkg/controllers/util/overridepolicy.go b/pkg/controllers/util/overridepolicy.go new file mode 100644 index 00000000..79ef1c95 --- /dev/null +++ b/pkg/controllers/util/overridepolicy.go @@ -0,0 +1,62 @@ +/* +Copyright 2018 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. + +This file may have been modified by The KubeAdmiral Authors +("KubeAdmiral Modifications"). All KubeAdmiral Modifications +are Copyright 2024 The KubeAdmiral Authors. +*/ + +package util + +import ( + "encoding/json" + + jsonpatch "github.com/evanphx/json-patch" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + + fedcorev1a1 "github.com/kubewharf/kubeadmiral/pkg/apis/core/v1alpha1" +) + +// ApplyJSONPatch applies the override on to the given unstructured object. +func ApplyJSONPatch(obj *unstructured.Unstructured, overrides fedcorev1a1.OverridePatches) error { + // TODO: Do the defaulting of "op" field to "replace" in API defaulting + for i, overrideItem := range overrides { + if overrideItem.Op == "" { + overrides[i].Op = "replace" + } + } + jsonPatchBytes, err := json.Marshal(overrides) + if err != nil { + return err + } + + patch, err := jsonpatch.DecodePatch(jsonPatchBytes) + if err != nil { + return err + } + + objectJSONBytes, err := obj.MarshalJSON() + if err != nil { + return err + } + + patchedObjectJSONBytes, err := patch.Apply(objectJSONBytes) + if err != nil { + return err + } + + err = obj.UnmarshalJSON(patchedObjectJSONBytes) + return err +} diff --git a/pkg/util/pod/pod.go b/pkg/util/pod/pod.go index 4bc9abee..84758b39 100644 --- a/pkg/util/pod/pod.go +++ b/pkg/util/pod/pod.go @@ -47,15 +47,8 @@ var PodSpecPaths = map[schema.GroupKind]string{ {Group: "", Kind: common.PodKind}: "spec", } -func GetPodSpec(fedObject fedcorev1a1.GenericFederatedObject, podSpecPath string) (*corev1.PodSpec, error) { - if fedObject == nil { - return nil, fmt.Errorf("fedObject is nil") - } - unsFedObject, err := fedObject.GetSpec().GetTemplateAsUnstructured() - if err != nil { - return nil, err - } - podSpecMap, found, err := unstructured.NestedMap(unsFedObject.Object, strings.Split(podSpecPath, ".")...) +func getPodSpecFromUnstructuredObj(unstructuredObj *unstructured.Unstructured, podSpecPath string) (*corev1.PodSpec, error) { + podSpecMap, found, err := unstructured.NestedMap(unstructuredObj.Object, strings.Split(podSpecPath, ".")...) if err != nil { return nil, err } @@ -69,6 +62,17 @@ func GetPodSpec(fedObject fedcorev1a1.GenericFederatedObject, podSpecPath string return podSpec, nil } +func GetPodSpec(fedObject fedcorev1a1.GenericFederatedObject, podSpecPath string) (*corev1.PodSpec, error) { + if fedObject == nil { + return nil, fmt.Errorf("fedObject is nil") + } + unsFedObject, err := fedObject.GetSpec().GetTemplateAsUnstructured() + if err != nil { + return nil, err + } + return getPodSpecFromUnstructuredObj(unsFedObject, podSpecPath) +} + func GetResourcePodSpec(fedObject fedcorev1a1.GenericFederatedObject, gvk schema.GroupVersionKind) (*corev1.PodSpec, error) { path, ok := PodSpecPaths[gvk.GroupKind()] if !ok { @@ -76,3 +80,13 @@ func GetResourcePodSpec(fedObject fedcorev1a1.GenericFederatedObject, gvk schema } return GetPodSpec(fedObject, path) } + +func GetResourcePodSpecFromUnstructuredObj( + unstructuredObj *unstructured.Unstructured, gvk schema.GroupVersionKind, +) (*corev1.PodSpec, error) { + podSpecPath, ok := PodSpecPaths[gvk.GroupKind()] + if !ok { + return nil, fmt.Errorf("%w: %s", ErrUnknownTypeToGetPodSpec, gvk.String()) + } + return getPodSpecFromUnstructuredObj(unstructuredObj, podSpecPath) +}