pkg/resource: add AdditiveMergePatchApplyOption

Signed-off-by: Dr. Stefan Schimanski <stefan.schimanski@upbound.io>

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

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=

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,43 @@ 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{})
+
+// 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, &currentBytes); 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))
+	}
+
+	// wipe APIVersion and Kind from the merged object in case of mixed types
+	if _, isUnstructured := desired.(runtime.Unstructured); !isUnstructured {
+		desired.GetObjectKind().SetGroupVersionKind(schema.GroupVersionKind{})
+	}
+
+	return nil
+}
+
 // An APIUpdatingApplicator applies changes to an object by either creating or
 // updating it in a Kubernetes API server.
 type APIUpdatingApplicator struct {

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,204 @@ 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",
+				}}},
+			},
+		},
+		{
+			name: "equal mixed",
+			args: args{
+				current: &unstructured.Unstructured{Object: map[string]interface{}{
+					"kind": "Thing",
+					"metadata": map[string]interface{}{
+						"labels": map[string]interface{}{
+							"a": "foo",
+						},
+					},
+				}},
+				desired: &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{
+					"a": "foo",
+				}}},
+			},
+			want: want{
+				current: &unstructured.Unstructured{Object: map[string]interface{}{
+					"kind": "Thing",
+					"metadata": map[string]interface{}{
+						"labels": map[string]interface{}{
+							"a": "foo",
+						},
+					},
+				}},
+				desired: &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{
+					"a": "foo",
+				}}},
+			},
+		},
+		{
+			name: "overlapping mixed",
+			args: args{
+				current: &unstructured.Unstructured{Object: map[string]interface{}{
+					"kind": "Thing",
+					"metadata": map[string]interface{}{
+						"labels": map[string]interface{}{
+							"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: &unstructured.Unstructured{Object: map[string]interface{}{
+					"kind": "Thing",
+					"metadata": map[string]interface{}{
+						"labels": map[string]interface{}{
+							"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()\ncurrent = %v\nwant    = %v\ndiff    = %s", tt.args.current, tt.want.current, diff)
+			}
+			if diff := cmp.Diff(tt.want.desired, tt.args.desired); diff != "" {
+				t.Errorf("AdditiveMergePatchApplyOption()\ncurrent = %v\nwant    = %v\ndiff    = %s", tt.args.desired, tt.want.desired, diff)
+			}
+		})
+	}
+}