Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: the previous cop is completely overwritten when binding op #336

Merged
merged 1 commit into from
Sep 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 3 additions & 8 deletions pkg/controllers/override/annotationslabelspatch.go
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand Down
6 changes: 5 additions & 1 deletion pkg/controllers/override/annotationslabelspatch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
12 changes: 4 additions & 8 deletions pkg/controllers/override/commandargspatch.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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)
}
Expand All @@ -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
}
Expand Down
6 changes: 5 additions & 1 deletion pkg/controllers/override/commandargspatch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
11 changes: 3 additions & 8 deletions pkg/controllers/override/envspatch.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import (
)

func parseEnvOverriders(
fedObject fedcorev1a1.GenericFederatedObject,
helper *helpData,
overriders []fedcorev1a1.EnvOverrider,
) (fedcorev1a1.OverridePatches, error) {
if len(overriders) == 0 {
Expand All @@ -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)
}
Expand All @@ -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
}
Expand Down
6 changes: 5 additions & 1 deletion pkg/controllers/override/envspatch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
27 changes: 9 additions & 18 deletions pkg/controllers/override/imagepatch.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}
}
Expand Down Expand Up @@ -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)
Expand All @@ -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)
}
Expand All @@ -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)
}
Expand Down
12 changes: 9 additions & 3 deletions pkg/controllers/override/imagepatch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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{}{},
Expand Down
3 changes: 2 additions & 1 deletion pkg/controllers/override/overridepolicy_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -323,9 +323,10 @@
}

var overrides, currentOverrides overridesMap
helpers := map[string]*helpData{}

Check warning on line 326 in pkg/controllers/override/overridepolicy_controller.go

View check run for this annotation

Codecov / codecov/patch

pkg/controllers/override/overridepolicy_controller.go#L326

Added line #L326 was not covered by tests
// Apply overrides from each policy in order
for _, policy := range policies {
newOverrides, err := parseOverrides(policy, placedClusters, fedObject)
newOverrides, err := parseOverrides(policy, placedClusters, fedObject, helpers)

Check warning on line 329 in pkg/controllers/override/overridepolicy_controller.go

View check run for this annotation

Codecov / codecov/patch

pkg/controllers/override/overridepolicy_controller.go#L329

Added line #L329 was not covered by tests
if err != nil {
c.eventRecorder.Eventf(
fedObject,
Expand Down
55 changes: 47 additions & 8 deletions pkg/controllers/override/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,13 @@

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"
)
Expand Down Expand Up @@ -112,12 +114,36 @@
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)

Check warning on line 125 in pkg/controllers/override/util.go

View check run for this annotation

Codecov / codecov/patch

pkg/controllers/override/util.go#L125

Added line #L125 was not covered by tests
}
sourceObj, err := fedObj.GetSpec().GetTemplateAsUnstructured()
if err != nil {
return nil, fmt.Errorf("failed to get sourceObj from fedObj: %w", err)

Check warning on line 129 in pkg/controllers/override/util.go

View check run for this annotation

Codecov / codecov/patch

pkg/controllers/override/util.go#L129

Added line #L129 was not covered by tests
}
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)
Expand All @@ -139,7 +165,14 @@
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

Check warning on line 172 in pkg/controllers/override/util.go

View check run for this annotation

Codecov / codecov/patch

pkg/controllers/override/util.go#L172

Added line #L172 was not covered by tests
}

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",
Expand All @@ -148,6 +181,12 @@
err,
)
}

if err = ctrutil.ApplyJSONPatch(helper.sourceObj, currPatches); err != nil {
return nil, err

Check warning on line 186 in pkg/controllers/override/util.go

View check run for this annotation

Codecov / codecov/patch

pkg/controllers/override/util.go#L186

Added line #L186 was not covered by tests
}
helpers[cluster.Name] = helper

patches = append(patches, currPatches...)
}

Expand Down Expand Up @@ -241,7 +280,7 @@
}

func parsePatchesFromOverriders(
fedObject fedcorev1a1.GenericFederatedObject,
helper *helpData,
overriders *fedcorev1a1.Overriders,
) (fedcorev1a1.OverridePatches, error) {
patches := make(fedcorev1a1.OverridePatches, 0)
Expand All @@ -252,42 +291,42 @@
}

// 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...)
Expand Down
Loading
Loading