You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by as...@apache.org on 2022/01/18 15:23:16 UTC
[camel-k] 01/03: fix(patch): Do not compute positive patch for unstructured object
This is an automated email from the ASF dual-hosted git repository.
astefanutti pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/camel-k.git
commit 436a34ff835dcc8a1e31d116475f1ff7c0849f9f
Author: Antonin Stefanutti <an...@stefanutti.fr>
AuthorDate: Mon Jan 17 19:10:26 2022 +0100
fix(patch): Do not compute positive patch for unstructured object
---
e2e/support/test_support.go | 4 +-
pkg/client/apply.go | 4 +-
pkg/cmd/builder/builder.go | 2 +-
pkg/cmd/install.go | 2 +-
pkg/controller/build/monitor_routine.go | 2 +-
pkg/controller/kameletbinding/initialize.go | 2 +-
pkg/install/kamelets.go | 4 +-
pkg/install/operator.go | 2 +-
pkg/trait/deployer.go | 6 +--
pkg/util/patch/patch.go | 75 +++++++++++++++--------------
10 files changed, 53 insertions(+), 50 deletions(-)
diff --git a/e2e/support/test_support.go b/e2e/support/test_support.go
index 319ab49..1a69db0 100644
--- a/e2e/support/test_support.go
+++ b/e2e/support/test_support.go
@@ -701,7 +701,7 @@ func PatchIntegration(ns string, name string, mutate func(it *v1.Integration)) e
}
target := it.DeepCopy()
mutate(target)
- p, err := patch.PositiveMergePatch(it, target)
+ p, err := patch.MergePatch(it, target)
if err != nil {
return err
} else if len(p) == 0 {
@@ -774,7 +774,7 @@ func UpdateKameletBinding(ns string, name string, upd func(it *v1alpha1.KameletB
target := klb.DeepCopy()
upd(target)
// For some reasons, full patch fails on some clusters
- p, err := patch.PositiveMergePatch(klb, target)
+ p, err := patch.MergePatch(klb, target)
if err != nil {
return err
} else if len(p) == 0 {
diff --git a/pkg/client/apply.go b/pkg/client/apply.go
index cfcc2c6..2a63bc2 100644
--- a/pkg/client/apply.go
+++ b/pkg/client/apply.go
@@ -77,7 +77,7 @@ func (a *ServerOrClientSideApplier) Apply(ctx context.Context, object ctrl.Objec
}
func (a *ServerOrClientSideApplier) serverSideApply(ctx context.Context, resource runtime.Object) error {
- target, err := patch.PositiveApplyPatch(resource)
+ target, err := patch.ApplyPatch(resource)
if err != nil {
return err
}
@@ -99,7 +99,7 @@ func (a *ServerOrClientSideApplier) clientSideApply(ctx context.Context, resourc
if err != nil {
return err
}
- p, err := patch.PositiveMergePatch(object, resource)
+ p, err := patch.MergePatch(object, resource)
if err != nil {
return err
} else if len(p) == 0 {
diff --git a/pkg/cmd/builder/builder.go b/pkg/cmd/builder/builder.go
index bd22ddf..390a535 100644
--- a/pkg/cmd/builder/builder.go
+++ b/pkg/cmd/builder/builder.go
@@ -78,7 +78,7 @@ func Run(namespace string, buildName string, taskName string) {
// is made on the build containers.
target.Status.Phase = v1.BuildPhaseNone
// Patch the build status with the result
- p, err := patch.PositiveMergePatch(build, target)
+ p, err := patch.MergePatch(build, target)
exitOnError(err, "cannot create merge patch")
if len(p) > 0 {
diff --git a/pkg/cmd/install.go b/pkg/cmd/install.go
index f3869dd..878c15d 100644
--- a/pkg/cmd/install.go
+++ b/pkg/cmd/install.go
@@ -715,7 +715,7 @@ func createDefaultMavenSettingsConfigMap(ctx context.Context, client client.Clie
return err
}
- p, err := patch.PositiveMergePatch(existing, cm)
+ p, err := patch.MergePatch(existing, cm)
if err != nil {
return err
} else if len(p) != 0 {
diff --git a/pkg/controller/build/monitor_routine.go b/pkg/controller/build/monitor_routine.go
index f9d0083..92e55af 100644
--- a/pkg/controller/build/monitor_routine.go
+++ b/pkg/controller/build/monitor_routine.go
@@ -186,7 +186,7 @@ func (action *monitorRoutineAction) updateBuildStatus(ctx context.Context, build
// Copy the failure field from the build to persist recovery state
target.Status.Failure = build.Status.Failure
// Patch the build status with the result
- p, err := patch.PositiveMergePatch(build, target)
+ p, err := patch.MergePatch(build, target)
if err != nil {
action.L.Errorf(err, "Cannot patch build status: %s", build.Name)
event.NotifyBuildError(ctx, action.client, action.recorder, build, target, err)
diff --git a/pkg/controller/kameletbinding/initialize.go b/pkg/controller/kameletbinding/initialize.go
index d65656a..3f85198 100644
--- a/pkg/controller/kameletbinding/initialize.go
+++ b/pkg/controller/kameletbinding/initialize.go
@@ -85,7 +85,7 @@ func (action *initializeAction) propagateIcon(ctx context.Context, binding *v1al
if _, ok := clone.Annotations[v1alpha1.AnnotationIcon]; !ok {
clone.Annotations[v1alpha1.AnnotationIcon] = icon
}
- p, err := patch.PositiveMergePatch(binding, clone)
+ p, err := patch.MergePatch(binding, clone)
if err != nil {
action.L.Errorf(err, "cannot compute patch to update icon for kamelet binding %q", binding.Name)
return
diff --git a/pkg/install/kamelets.go b/pkg/install/kamelets.go
index 82a818b..f910436 100644
--- a/pkg/install/kamelets.go
+++ b/pkg/install/kamelets.go
@@ -131,7 +131,7 @@ func KameletCatalog(ctx context.Context, c client.Client, namespace string) erro
}
func serverSideApply(ctx context.Context, c client.Client, resource runtime.Object) error {
- target, err := patch.PositiveApplyPatch(resource)
+ target, err := patch.ApplyPatch(resource)
if err != nil {
return err
}
@@ -153,7 +153,7 @@ func clientSideApply(ctx context.Context, c client.Client, resource ctrl.Object)
if err != nil {
return err
}
- p, err := patch.PositiveMergePatch(object, resource)
+ p, err := patch.MergePatch(object, resource)
if err != nil {
return err
} else if len(p) == 0 {
diff --git a/pkg/install/operator.go b/pkg/install/operator.go
index 33b8371..d602bb5 100644
--- a/pkg/install/operator.go
+++ b/pkg/install/operator.go
@@ -368,7 +368,7 @@ func installClusterRoleBinding(ctx context.Context, c client.Client, collection
// The ClusterRoleBinding.Subjects field does not have a patchStrategy key in its field tag,
// so a strategic merge patch would use the default patch strategy, which is replace.
// Let's compute a simple JSON merge patch from the existing resource, and patch it.
- p, err := patch.PositiveMergePatch(existing, target)
+ p, err := patch.MergePatch(existing, target)
if err != nil {
return err
} else if len(p) == 0 {
diff --git a/pkg/trait/deployer.go b/pkg/trait/deployer.go
index 67cdb79..8bc3b23 100644
--- a/pkg/trait/deployer.go
+++ b/pkg/trait/deployer.go
@@ -62,7 +62,7 @@ func (t *deployerTrait) Apply(e *Environment) error {
e.PostActions = append(e.PostActions, func(env *Environment) error {
for _, resource := range env.Resources.Items() {
// We assume that server-side apply is enabled by default.
- // It is currently convoluted to check pro-actively whether server-side apply
+ // It is currently convoluted to check proactively whether server-side apply
// is enabled. This is possible to fetch the OpenAPI endpoint, which returns
// the entire server API document, then lookup the resource PATCH endpoint, and
// check its list of accepted MIME types.
@@ -92,7 +92,7 @@ func (t *deployerTrait) Apply(e *Environment) error {
}
func (t *deployerTrait) serverSideApply(env *Environment, resource ctrl.Object) error {
- target, err := patch.PositiveApplyPatch(resource)
+ target, err := patch.ApplyPatch(resource)
if err != nil {
return err
}
@@ -119,7 +119,7 @@ func (t *deployerTrait) clientSideApply(env *Environment, resource ctrl.Object)
if err != nil {
return err
}
- p, err := patch.PositiveMergePatch(object, resource)
+ p, err := patch.MergePatch(object, resource)
if err != nil {
return err
} else if len(p) == 0 {
diff --git a/pkg/util/patch/patch.go b/pkg/util/patch/patch.go
index b672459..b7ad290 100644
--- a/pkg/util/patch/patch.go
+++ b/pkg/util/patch/patch.go
@@ -27,60 +27,63 @@ import (
"k8s.io/apimachinery/pkg/util/json"
)
-func PositiveMergePatch(source interface{}, target interface{}) ([]byte, error) {
+func MergePatch(source interface{}, target interface{}) ([]byte, error) {
sourceJSON, err := json.Marshal(source)
if err != nil {
return nil, err
}
-
targetJSON, err := json.Marshal(target)
if err != nil {
return nil, err
}
-
mergePatch, err := jsonpatch.CreateMergePatch(sourceJSON, targetJSON)
if err != nil {
return nil, err
}
- var positivePatch map[string]interface{}
- err = json.Unmarshal(mergePatch, &positivePatch)
- if err != nil {
- return nil, err
- }
-
- // The following is a work-around to remove null fields from the JSON merge patch,
- // so that values defaulted by controllers server-side are not deleted.
- // It's generally acceptable as these values are orthogonal to the values managed
- // by the traits.
- removeNilValues(reflect.ValueOf(positivePatch), reflect.Value{})
-
- // Return an empty patch if no keys remain
- if len(positivePatch) == 0 {
- return make([]byte, 0), nil
+ switch target.(type) {
+ case *unstructured.Unstructured:
+ // Take the JSON merge patch as is for unstructured objects
+ return mergePatch, nil
+ default:
+ // Otherwise, for typed objects, remove null fields from the JSON merge patch,
+ // so that values managed by controllers server-side are not deleted.
+ var positivePatch map[string]interface{}
+ err = json.Unmarshal(mergePatch, &positivePatch)
+ if err != nil {
+ return nil, err
+ }
+ removeNilValues(reflect.ValueOf(positivePatch), reflect.Value{})
+ // Return an empty patch if no keys remain
+ if len(positivePatch) == 0 {
+ return make([]byte, 0), nil
+ }
+ return json.Marshal(positivePatch)
}
-
- return json.Marshal(positivePatch)
}
-func PositiveApplyPatch(source runtime.Object) (*unstructured.Unstructured, error) {
- sourceJSON, err := json.Marshal(source)
- if err != nil {
- return nil, err
- }
+func ApplyPatch(source runtime.Object) (*unstructured.Unstructured, error) {
+ switch s := source.(type) {
+ case *unstructured.Unstructured:
+ // Directly take the unstructured object as apply patch
+ return s, nil
+ default:
+ // Otherwise, for typed objects, remove null fields from the apply patch,
+ // so that ownership is not taken for non-managed fields.
+ // See https://github.com/kubernetes/enhancements/tree/master/keps/sig-api-machinery/2155-clientgo-apply
+ sourceJSON, err := json.Marshal(source)
+ if err != nil {
+ return nil, err
+ }
+ var positivePatch map[string]interface{}
+ err = json.Unmarshal(sourceJSON, &positivePatch)
+ if err != nil {
+ return nil, err
+ }
+ removeNilValues(reflect.ValueOf(positivePatch), reflect.Value{})
- var positivePatch map[string]interface{}
- err = json.Unmarshal(sourceJSON, &positivePatch)
- if err != nil {
- return nil, err
+ return &unstructured.Unstructured{Object: positivePatch}, nil
}
-
- // The following is a work-around to remove null fields from the apply patch,
- // so that ownership is not taken for non-managed fields.
- // See https://github.com/kubernetes/enhancements/tree/master/keps/sig-api-machinery/2155-clientgo-apply
- removeNilValues(reflect.ValueOf(positivePatch), reflect.Value{})
-
- return &unstructured.Unstructured{Object: positivePatch}, nil
}
func removeNilValues(v reflect.Value, parent reflect.Value) {