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..9b5d6ebda 100644 --- a/pkg/resource/api.go +++ b/pkg/resource/api.go @@ -18,14 +18,19 @@ package resource import ( "context" + "encoding/json" + jsonpatch "github.com/evanphx/json-patch" kerrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/crossplane/crossplane-runtime/pkg/errors" "github.com/crossplane/crossplane-runtime/pkg/meta" + resourceunstructured "github.com/crossplane/crossplane-runtime/pkg/resource/unstructured" ) // Error strings. @@ -54,7 +59,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 +107,41 @@ func groupResource(c client.Client, o client.Object) (schema.GroupResource, erro return m.Resource.GroupResource(), nil } +// 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 { + u, ok := desired.(*unstructured.Unstructured) + if !ok { + uw, ok := desired.(resourceunstructured.Wrapper) + if !ok { + return errors.New("desired object is not an unstructured.Unstructured") + } + u = uw.GetUnstructured() + } + currentBytes, err := json.Marshal(current) + if err != nil { + return errors.Wrapf(err, "cannot marshal current %s", HumanReadableReference(nil, current)) + } + desiredBytes, err := json.Marshal(u) + if err != nil { + return errors.Wrapf(err, "cannot marshal desired %s", HumanReadableReference(nil, desired)) + } + mergedBytes, err := jsonpatch.MergePatch(currentBytes, desiredBytes) + if err != nil { + return errors.Wrapf(err, "cannot merge patch to %s", HumanReadableReference(nil, desired)) + } + u.Object = nil + if err = json.Unmarshal(mergedBytes, &u.Object); 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..90af4f1bc 100644 --- a/pkg/resource/api_test.go +++ b/pkg/resource/api_test.go @@ -18,15 +18,19 @@ package resource import ( "context" + "encoding/json" "testing" + jsonpatch "github.com/evanphx/json-patch" "github.com/google/go-cmp/cmp" 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 +58,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 +233,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 {