diff --git a/go.mod b/go.mod index b76711d03..f0d99deb8 100644 --- a/go.mod +++ b/go.mod @@ -5,6 +5,7 @@ go 1.20 require ( dario.cat/mergo v1.0.0 github.com/bufbuild/buf v1.26.1 + github.com/evanphx/json-patch v5.6.0+incompatible github.com/go-logr/logr v1.2.4 github.com/google/go-cmp v0.5.9 github.com/spf13/afero v1.9.5 diff --git a/go.sum b/go.sum index 3b6ec0f8f..1f7918422 100644 --- a/go.sum +++ b/go.sum @@ -96,6 +96,7 @@ github.com/envoyproxy/go-control-plane v0.9.7/go.mod h1:cwu0lG7PUMfa9snN8LXBig5y github.com/envoyproxy/go-control-plane v0.9.9-0.20201210154907-fd9021fe5dad/go.mod h1:cXg6YxExXjJnVBQHBLXeUAgxn2UodCpnH306RInaBQk= github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c= github.com/evanphx/json-patch v5.6.0+incompatible h1:jBYDEEiFBPxA0v50tFdvOzQQTCvpL6mnFh5mB2/l16U= +github.com/evanphx/json-patch v5.6.0+incompatible/go.mod h1:50XU6AFN0ol/bzJsmQLiYLvXMP4fmwYFNcr97nuDLSk= github.com/evanphx/json-patch/v5 v5.6.0 h1:b91NhWfaz02IuVxO9faSllyAtNXHMPkC5J8sJCLunww= github.com/evanphx/json-patch/v5 v5.6.0/go.mod h1:G79N1coSVB93tBe7j6PhzjmR3/2VvlbKOFpnXhI9Bw4= github.com/fatih/color v1.15.0 h1:kOqh6YHBtK8aywxGerMG2Eq3H6Qgoqeo13Bk2Mv/nBs= diff --git a/pkg/resource/api.go b/pkg/resource/api.go index 10673d84c..6d13f170b 100644 --- a/pkg/resource/api.go +++ b/pkg/resource/api.go @@ -17,10 +17,14 @@ limitations under the License. package resource import ( + "bytes" "context" + jsonpatch "github.com/evanphx/json-patch" kerrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/runtime/serializer/json" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" @@ -54,7 +58,7 @@ func NewAPIPatchingApplicator(c client.Client) *APIPatchingApplicator { // Apply changes to the supplied object. The object will be created if it does // not exist, or patched if it does. If the object does exist, it will only be // patched if the passed object has the same or an empty resource version. -func (a *APIPatchingApplicator) Apply(ctx context.Context, obj client.Object, ao ...ApplyOption) error { //nolint:gocyclo // the logic here is crucial and deserves to stay in one method +func (a *APIPatchingApplicator) Apply(ctx context.Context, obj client.Object, ao ...ApplyOption) error { if obj.GetName() == "" && obj.GetGenerateName() != "" { return a.client.Create(ctx, obj) } @@ -102,6 +106,37 @@ func groupResource(c client.Client, o client.Object) (schema.GroupResource, erro return m.Resource.GroupResource(), nil } +var emptyScheme = runtime.NewScheme() // no need to recognize any types +var jsonSerializer = json.NewSerializerWithOptions(json.DefaultMetaFactory, emptyScheme, emptyScheme, json.SerializerOptions{Strict: true}) + +// AdditiveMergePatchApplyOption returns an ApplyOption that makes +// the Apply additive in the sense of a merge patch without null values. This is +// the old behavior of the APIPatchingApplicator. +// +// This only works with a desired object of type *unstructured.Unstructured. +// +// Deprecated: replace with Server Side Apply. +func AdditiveMergePatchApplyOption(_ context.Context, current, desired runtime.Object) error { + // merge `desired` additively with `current` + var currentBytes, desiredBytes bytes.Buffer + if err := jsonSerializer.Encode(current, ¤tBytes); err != nil { + return errors.Wrapf(err, "cannot marshal current %s", HumanReadableReference(nil, current)) + } + if err := jsonSerializer.Encode(desired, &desiredBytes); err != nil { + return errors.Wrapf(err, "cannot marshal desired %s", HumanReadableReference(nil, desired)) + } + mergedBytes, err := jsonpatch.MergePatch(currentBytes.Bytes(), desiredBytes.Bytes()) + if err != nil { + return errors.Wrapf(err, "cannot merge patch to %s", HumanReadableReference(nil, desired)) + } + + // write merged object back to `desired` + if _, _, err := jsonSerializer.Decode(mergedBytes, nil, desired); err != nil { + return errors.Wrapf(err, "cannot unmarshal merged patch to %s", HumanReadableReference(nil, desired)) + } + return nil +} + // An APIUpdatingApplicator applies changes to an object by either creating or // updating it in a Kubernetes API server. type APIUpdatingApplicator struct { diff --git a/pkg/resource/api_test.go b/pkg/resource/api_test.go index 0cf2dde34..d3aad6b97 100644 --- a/pkg/resource/api_test.go +++ b/pkg/resource/api_test.go @@ -18,15 +18,20 @@ package resource import ( "context" + "encoding/json" "testing" + jsonpatch "github.com/evanphx/json-patch" "github.com/google/go-cmp/cmp" + corev1 "k8s.io/api/core/v1" kerrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/yaml" "github.com/crossplane/crossplane-runtime/pkg/errors" "github.com/crossplane/crossplane-runtime/pkg/resource/fake" @@ -54,6 +59,14 @@ func TestAPIPatchingApplicator(t *testing.T) { fakeRESTMapper := meta.NewDefaultRESTMapper([]schema.GroupVersion{gvk.GroupVersion()}) fakeRESTMapper.AddSpecific(gvk, gvr, singular, meta.RESTScopeRoot) + // for additive merge patch option test + currentYAML := ` +metadata: + resourceVersion: "42" +a: old +b: old +` + type args struct { ctx context.Context o client.Object @@ -221,6 +234,63 @@ func TestAPIPatchingApplicator(t *testing.T) { err: kerrors.NewConflict(schema.GroupResource{Group: "example.com", Resource: "things"}, current.GetName(), errors.New(errOptimisticLock)), }, }, + "AdditiveMergePatch": { + reason: "No error with the old additive behaviour if desired", + c: &test.MockClient{ + MockGet: test.NewMockGetFn(nil, func(o client.Object) error { + o.(*unstructured.Unstructured).Object = map[string]interface{}{} + return yaml.Unmarshal([]byte(currentYAML), &o.(*unstructured.Unstructured).Object) + }), + MockPatch: func(_ context.Context, o client.Object, patch client.Patch, _ ...client.PatchOption) error { + bs, err := patch.Data(o) + if err != nil { + return err + } + currentJSON, err := yaml.YAMLToJSON([]byte(currentYAML)) + if err != nil { + return err + } + patched, err := jsonpatch.MergePatch(currentJSON, bs) + if err != nil { + return err + } + o.(*unstructured.Unstructured).Object = map[string]interface{}{} + if err := json.Unmarshal(patched, &o.(*unstructured.Unstructured).Object); err != nil { + return err + } + o.SetResourceVersion("43") + return nil + }, + MockGroupVersionKindFor: test.NewMockGroupVersionKindForFn(nil, gvk), + MockRESTMapper: test.NewMockRESTMapperFn(fakeRESTMapper), + }, + args: args{ + o: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "kind": "Thing", + "metadata": map[string]interface{}{ + "resourceVersion": "42", + }, + "b": "changed", + "c": "added", + }, + }, + ao: []ApplyOption{AdditiveMergePatchApplyOption}, + }, + want: want{ + o: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "kind": "Thing", + "metadata": map[string]interface{}{ + "resourceVersion": "43", + }, + "a": "old", + "b": "changed", + "c": "added", + }, + }, + }, + }, } for name, tc := range cases { @@ -518,3 +588,137 @@ func TestAPIFinalizerAdder(t *testing.T) { }) } } + +func TestAdditiveMergePatchApplyOption(t *testing.T) { + type args struct { + current runtime.Object + desired runtime.Object + } + type want struct { + err error + current runtime.Object + desired runtime.Object + } + tests := []struct { + name string + args args + want want + }{ + { + name: "equal unstructed", + args: args{ + current: &unstructured.Unstructured{Object: map[string]interface{}{ + "kind": "Thing", + "a": "foo", + }}, + desired: &unstructured.Unstructured{Object: map[string]interface{}{ + "kind": "Thing", + "a": "foo", + }}, + }, + want: want{ + current: &unstructured.Unstructured{Object: map[string]interface{}{ + "kind": "Thing", + "a": "foo", + }}, + desired: &unstructured.Unstructured{Object: map[string]interface{}{ + "kind": "Thing", + "a": "foo", + }}, + }, + }, + { + name: "overlapping unstructed", + args: args{ + current: &unstructured.Unstructured{Object: map[string]interface{}{ + "kind": "Thing", + "a": "foo", + "b": "foo", + "c": "foo", + }}, + desired: &unstructured.Unstructured{Object: map[string]interface{}{ + "kind": "Thing", + "a": "foo", + "b": "bar", + "d": "bar", + }}, + }, + want: want{ + current: &unstructured.Unstructured{Object: map[string]interface{}{ + "kind": "Thing", + "a": "foo", + "b": "foo", + "c": "foo", + }}, + desired: &unstructured.Unstructured{Object: map[string]interface{}{ + "kind": "Thing", + "a": "foo", + "b": "bar", + "c": "foo", + "d": "bar", + }}, + }, + }, + { + name: "equal typed", + args: args{ + current: &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{ + "a": "foo", + }}}, + desired: &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{ + "a": "foo", + }}}, + }, + want: want{ + current: &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{ + "a": "foo", + }}}, + desired: &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{ + "a": "foo", + }}}, + }, + }, + { + name: "overlapping typed", + args: args{ + current: &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{ + "a": "foo", + "b": "foo", + "c": "foo", + }}}, + desired: &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{ + "a": "foo", + "b": "bar", + "d": "bar", + }}}, + }, + want: want{ + current: &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{ + "a": "foo", + "b": "foo", + "c": "foo", + }}}, + desired: &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{ + "a": "foo", + "b": "bar", + "c": "foo", + "d": "bar", + }}}, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := AdditiveMergePatchApplyOption(context.Background(), tt.args.current, tt.args.desired) + if diff := cmp.Diff(tt.want.err, err, test.EquateErrors()); diff != "" { + t.Errorf("AdditiveMergePatchApplyOption() error = %v, wantErr %v", err, tt.want.err) + } + if diff := cmp.Diff(tt.want.current, tt.args.current); diff != "" { + t.Errorf("AdditiveMergePatchApplyOption() current = %v, want %v", tt.args.current, tt.want.current) + } + if diff := cmp.Diff(tt.want.desired, tt.args.desired); diff != "" { + t.Errorf("AdditiveMergePatchApplyOption() current = %v, want %v", tt.args.desired, tt.want.desired) + } + }) + } +}