Use server-side apply patch to update the resolved cross-resource ref…

…erences

in managed.APISimpleReferenceResolver.

Signed-off-by: Alper Rifat Ulucinar <[email protected]>

go.mod

@@ -5,6 +5,7 @@ go 1.20
 require (
 	dario.cat/mergo v1.0.0
 	github.com/bufbuild/buf v1.28.1
+	github.com/evanphx/json-patch v5.6.0+incompatible
 	github.com/go-logr/logr v1.3.0
 	github.com/google/go-cmp v0.6.0
 	github.com/spf13/afero v1.11.0

go.sum

@@ -55,6 +55,7 @@ github.com/emicklei/go-restful/v3 v3.11.0 h1:rAQeMHw1c7zTmncogyy8VvRZwtkmkZ4FxER
 github.com/emicklei/go-restful/v3 v3.11.0/go.mod h1:6n3XBCmQQb25CM2LCACGz8ukIrRry+4bhvbpWn3mrbc=
 github.com/envoyproxy/protoc-gen-validate v1.0.2 h1:QkIBuU5k+x7/QXPvPPnWXWlCdaBFApVqftFV6k087DA=
 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/reconciler/managed/api.go

@@ -18,12 +18,15 @@ package managed
 
 import (
 	"context"
+	"encoding/json"
 
+	jsonpatch "github.com/evanphx/json-patch"
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
 	corev1 "k8s.io/api/core/v1"
 	kerrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/util/retry"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -33,10 +36,20 @@ import (
 	"github.com/crossplane/crossplane-runtime/pkg/resource"
 )
 
+const (
+	// fieldOwnerAPISimpleRefResolver owns the reference fields
+	// the managed reconciler resolves.
+	fieldOwnerAPISimpleRefResolver = "managed.crossplane.io/api-simple-reference-resolver"
+)
+
 // Error strings.
 const (
 	errCreateOrUpdateSecret      = "cannot create or update connection secret"
 	errUpdateManaged             = "cannot update managed resource"
+	errPatchManaged              = "cannot patch the managed resource via server-side apply"
+	errMarshalExisting           = "cannot marshal the existing object into JSON"
+	errMarshalResolved           = "cannot marshal the object with the resolved references into JSON"
+	errPreparePatch              = "cannot prepare the JSON merge patch for the resolved object"
 	errUpdateManagedStatus       = "cannot update managed resource status"
 	errResolveReferences         = "cannot resolve references"
 	errUpdateCriticalAnnotations = "cannot update critical annotations"
@@ -130,6 +143,27 @@ func NewAPISimpleReferenceResolver(c client.Client) *APISimpleReferenceResolver
 	return &APISimpleReferenceResolver{client: c}
 }
 
+func prepareJSONMerge(existing, resolved runtime.Object) ([]byte, error) {
+	// restore the to be replaced GVK so that the existing object is
+	// not modified by this function.
+	defer existing.GetObjectKind().SetGroupVersionKind(existing.GetObjectKind().GroupVersionKind())
+	// we need the apiVersion and kind in the patch document so we set them
+	// to their zero values and make them available in the calculated patch
+	// in the first place, instead of an unmarshal/marshal from the prepared
+	// patch []byte later.
+	existing.GetObjectKind().SetGroupVersionKind(schema.GroupVersionKind{})
+	eBuff, err := json.Marshal(existing)
+	if err != nil {
+		return nil, errors.Wrap(err, errMarshalExisting)
+	}
+	rBuff, err := json.Marshal(resolved)
+	if err != nil {
+		return nil, errors.Wrap(err, errMarshalResolved)
+	}
+	patch, err := jsonpatch.CreateMergePatch(eBuff, rBuff)
+	return patch, errors.Wrap(err, errPreparePatch)
+}
+
 // ResolveReferences of the supplied managed resource by calling its
 // ResolveReferences method, if any.
 func (a *APISimpleReferenceResolver) ResolveReferences(ctx context.Context, mg resource.Managed) error {
@@ -151,7 +185,11 @@ func (a *APISimpleReferenceResolver) ResolveReferences(ctx context.Context, mg r
 		return nil
 	}
 
-	return errors.Wrap(a.client.Update(ctx, mg), errUpdateManaged)
+	patch, err := prepareJSONMerge(existing, mg)
+	if err != nil {
+		return err
+	}
+	return errors.Wrap(a.client.Patch(ctx, mg, client.RawPatch(types.ApplyPatchType, patch), client.FieldOwner(fieldOwnerAPISimpleRefResolver), client.ForceOwnership), errPatchManaged)
 }
 
 // A RetryingCriticalAnnotationUpdater is a CriticalAnnotationUpdater that

pkg/reconciler/managed/api_test.go

@@ -239,7 +239,7 @@ func TestAPISecretPublisher(t *testing.T) {
 type mockSimpleReferencer struct {
 	resource.Managed
 
-	MockResolveReferences func(context.Context, client.Reader) error
+	MockResolveReferences func(context.Context, client.Reader) error `json:"-"`
 }
 
 func (r *mockSimpleReferencer) ResolveReferences(ctx context.Context, c client.Reader) error {
@@ -313,7 +313,7 @@ func TestResolveReferences(t *testing.T) {
 		"SuccessfulUpdate": {
 			reason: "Should return without error when a value is successfully resolved.",
 			c: &test.MockClient{
-				MockUpdate: test.NewMockUpdateFn(nil),
+				MockPatch: test.NewMockPatchFn(nil),
 			},
 			args: args{
 				ctx: context.Background(),
@@ -327,10 +327,10 @@ func TestResolveReferences(t *testing.T) {
 			},
 			want: nil,
 		},
-		"UpdateError": {
+		"PatchError": {
 			reason: "Should return an error when the managed resource cannot be updated.",
 			c: &test.MockClient{
-				MockUpdate: test.NewMockUpdateFn(errBoom),
+				MockPatch: test.NewMockPatchFn(errBoom),
 			},
 			args: args{
 				ctx: context.Background(),
@@ -342,7 +342,7 @@ func TestResolveReferences(t *testing.T) {
 					},
 				},
 			},
-			want: errors.Wrap(errBoom, errUpdateManaged),
+			want: errors.Wrap(errBoom, errPatchManaged),
 		},
 	}
 
@@ -357,6 +357,49 @@ func TestResolveReferences(t *testing.T) {
 	}
 }
 
+func TestPrepareJSONMerge(t *testing.T) {
+	type args struct {
+		existing runtime.Object
+		resolved runtime.Object
+	}
+	type want struct {
+		patch string
+		err   error
+	}
+
+	cases := map[string]struct {
+		reason string
+		args   args
+		want   want
+	}{
+		"SuccessfulPatch": {
+			reason: "Should successfully compute the JSON merge patch document.",
+			args: args{
+				existing: &fake.Managed{},
+				resolved: &fake.Managed{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "resolved",
+					}},
+			},
+			want: want{
+				patch: `{"name":"resolved"}`,
+			},
+		},
+	}
+
+	for name, tc := range cases {
+		t.Run(name, func(t *testing.T) {
+			patch, err := prepareJSONMerge(tc.args.existing, tc.args.resolved)
+			if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" {
+				t.Errorf("\n%s\nprepareJSONMerge(...): -wantErr, +gotErr:\n%s", tc.reason, diff)
+			}
+			if diff := cmp.Diff(tc.want.patch, string(patch)); diff != "" {
+				t.Errorf("\n%s\nprepareJSONMerge(...): -want, +got:\n%s", tc.reason, diff)
+			}
+		})
+	}
+}
+
 func TestRetryingCriticalAnnotationUpdater(t *testing.T) {
 	errBoom := errors.New("boom")