Skip to content

Commit 0aa37e8

Browse files
stttssttts
committed
pkg/resource: add AdditiveMergePatchApplyOption
Signed-off-by: Dr. Stefan Schimanski <[email protected]>
1 parent 36c1fe2 commit 0aa37e8

File tree

4 files changed

+242
-1
lines changed

4 files changed

+242
-1
lines changed

‎go.mod

+1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ go 1.20
55
require (
66
dario.cat/mergo v1.0.0
77
github.com/bufbuild/buf v1.26.1
8+
github.com/evanphx/json-patch v5.6.0+incompatible
89
github.com/go-logr/logr v1.2.4
910
github.com/google/go-cmp v0.5.9
1011
github.com/spf13/afero v1.9.5

‎go.sum

+1
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ github.com/envoyproxy/go-control-plane v0.9.7/go.mod h1:cwu0lG7PUMfa9snN8LXBig5y
9696
github.com/envoyproxy/go-control-plane v0.9.9-0.20201210154907-fd9021fe5dad/go.mod h1:cXg6YxExXjJnVBQHBLXeUAgxn2UodCpnH306RInaBQk=
9797
github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c=
9898
github.com/evanphx/json-patch v5.6.0+incompatible h1:jBYDEEiFBPxA0v50tFdvOzQQTCvpL6mnFh5mB2/l16U=
99+
github.com/evanphx/json-patch v5.6.0+incompatible/go.mod h1:50XU6AFN0ol/bzJsmQLiYLvXMP4fmwYFNcr97nuDLSk=
99100
github.com/evanphx/json-patch/v5 v5.6.0 h1:b91NhWfaz02IuVxO9faSllyAtNXHMPkC5J8sJCLunww=
100101
github.com/evanphx/json-patch/v5 v5.6.0/go.mod h1:G79N1coSVB93tBe7j6PhzjmR3/2VvlbKOFpnXhI9Bw4=
101102
github.com/fatih/color v1.15.0 h1:kOqh6YHBtK8aywxGerMG2Eq3H6Qgoqeo13Bk2Mv/nBs=

‎pkg/resource/api.go

+36-1
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,14 @@ limitations under the License.
1717
package resource
1818

1919
import (
20+
"bytes"
2021
"context"
2122

23+
jsonpatch "github.com/evanphx/json-patch"
2224
kerrors "k8s.io/apimachinery/pkg/api/errors"
25+
"k8s.io/apimachinery/pkg/runtime"
2326
"k8s.io/apimachinery/pkg/runtime/schema"
27+
"k8s.io/apimachinery/pkg/runtime/serializer/json"
2428
"k8s.io/apimachinery/pkg/types"
2529
"sigs.k8s.io/controller-runtime/pkg/client"
2630

@@ -54,7 +58,7 @@ func NewAPIPatchingApplicator(c client.Client) *APIPatchingApplicator {
5458
// Apply changes to the supplied object. The object will be created if it does
5559
// not exist, or patched if it does. If the object does exist, it will only be
5660
// patched if the passed object has the same or an empty resource version.
57-
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
61+
func (a *APIPatchingApplicator) Apply(ctx context.Context, obj client.Object, ao ...ApplyOption) error {
5862
if obj.GetName() == "" && obj.GetGenerateName() != "" {
5963
return a.client.Create(ctx, obj)
6064
}
@@ -102,6 +106,37 @@ func groupResource(c client.Client, o client.Object) (schema.GroupResource, erro
102106
return m.Resource.GroupResource(), nil
103107
}
104108

109+
var emptyScheme = runtime.NewScheme() // no need to recognize any types
110+
var jsonSerializer = json.NewSerializerWithOptions(json.DefaultMetaFactory, emptyScheme, emptyScheme, json.SerializerOptions{Strict: true})
111+
112+
// AdditiveMergePatchApplyOption returns an ApplyOption that makes
113+
// the Apply additive in the sense of a merge patch without null values. This is
114+
// the old behavior of the APIPatchingApplicator.
115+
//
116+
// This only works with a desired object of type *unstructured.Unstructured.
117+
//
118+
// Deprecated: replace with Server Side Apply.
119+
func AdditiveMergePatchApplyOption(_ context.Context, current, desired runtime.Object) error {
120+
// merge `desired` additively with `current`
121+
var currentBytes, desiredBytes bytes.Buffer
122+
if err := jsonSerializer.Encode(current, &currentBytes); err != nil {
123+
return errors.Wrapf(err, "cannot marshal current %s", HumanReadableReference(nil, current))
124+
}
125+
if err := jsonSerializer.Encode(desired, &desiredBytes); err != nil {
126+
return errors.Wrapf(err, "cannot marshal desired %s", HumanReadableReference(nil, desired))
127+
}
128+
mergedBytes, err := jsonpatch.MergePatch(currentBytes.Bytes(), desiredBytes.Bytes())
129+
if err != nil {
130+
return errors.Wrapf(err, "cannot merge patch to %s", HumanReadableReference(nil, desired))
131+
}
132+
133+
// write merged object back to `desired`
134+
if _, _, err := jsonSerializer.Decode(mergedBytes, nil, desired); err != nil {
135+
return errors.Wrapf(err, "cannot unmarshal merged patch to %s", HumanReadableReference(nil, desired))
136+
}
137+
return nil
138+
}
139+
105140
// An APIUpdatingApplicator applies changes to an object by either creating or
106141
// updating it in a Kubernetes API server.
107142
type APIUpdatingApplicator struct {

‎pkg/resource/api_test.go

+204
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,20 @@ package resource
1818

1919
import (
2020
"context"
21+
"encoding/json"
2122
"testing"
2223

24+
jsonpatch "github.com/evanphx/json-patch"
2325
"github.com/google/go-cmp/cmp"
26+
corev1 "k8s.io/api/core/v1"
2427
kerrors "k8s.io/apimachinery/pkg/api/errors"
2528
"k8s.io/apimachinery/pkg/api/meta"
2629
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
30+
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
2731
"k8s.io/apimachinery/pkg/runtime"
2832
"k8s.io/apimachinery/pkg/runtime/schema"
2933
"sigs.k8s.io/controller-runtime/pkg/client"
34+
"sigs.k8s.io/yaml"
3035

3136
"github.com/crossplane/crossplane-runtime/pkg/errors"
3237
"github.com/crossplane/crossplane-runtime/pkg/resource/fake"
@@ -54,6 +59,14 @@ func TestAPIPatchingApplicator(t *testing.T) {
5459
fakeRESTMapper := meta.NewDefaultRESTMapper([]schema.GroupVersion{gvk.GroupVersion()})
5560
fakeRESTMapper.AddSpecific(gvk, gvr, singular, meta.RESTScopeRoot)
5661

62+
// for additive merge patch option test
63+
currentYAML := `
64+
metadata:
65+
resourceVersion: "42"
66+
a: old
67+
b: old
68+
`
69+
5770
type args struct {
5871
ctx context.Context
5972
o client.Object
@@ -221,6 +234,63 @@ func TestAPIPatchingApplicator(t *testing.T) {
221234
err: kerrors.NewConflict(schema.GroupResource{Group: "example.com", Resource: "things"}, current.GetName(), errors.New(errOptimisticLock)),
222235
},
223236
},
237+
"AdditiveMergePatch": {
238+
reason: "No error with the old additive behaviour if desired",
239+
c: &test.MockClient{
240+
MockGet: test.NewMockGetFn(nil, func(o client.Object) error {
241+
o.(*unstructured.Unstructured).Object = map[string]interface{}{}
242+
return yaml.Unmarshal([]byte(currentYAML), &o.(*unstructured.Unstructured).Object)
243+
}),
244+
MockPatch: func(_ context.Context, o client.Object, patch client.Patch, _ ...client.PatchOption) error {
245+
bs, err := patch.Data(o)
246+
if err != nil {
247+
return err
248+
}
249+
currentJSON, err := yaml.YAMLToJSON([]byte(currentYAML))
250+
if err != nil {
251+
return err
252+
}
253+
patched, err := jsonpatch.MergePatch(currentJSON, bs)
254+
if err != nil {
255+
return err
256+
}
257+
o.(*unstructured.Unstructured).Object = map[string]interface{}{}
258+
if err := json.Unmarshal(patched, &o.(*unstructured.Unstructured).Object); err != nil {
259+
return err
260+
}
261+
o.SetResourceVersion("43")
262+
return nil
263+
},
264+
MockGroupVersionKindFor: test.NewMockGroupVersionKindForFn(nil, gvk),
265+
MockRESTMapper: test.NewMockRESTMapperFn(fakeRESTMapper),
266+
},
267+
args: args{
268+
o: &unstructured.Unstructured{
269+
Object: map[string]interface{}{
270+
"kind": "Thing",
271+
"metadata": map[string]interface{}{
272+
"resourceVersion": "42",
273+
},
274+
"b": "changed",
275+
"c": "added",
276+
},
277+
},
278+
ao: []ApplyOption{AdditiveMergePatchApplyOption},
279+
},
280+
want: want{
281+
o: &unstructured.Unstructured{
282+
Object: map[string]interface{}{
283+
"kind": "Thing",
284+
"metadata": map[string]interface{}{
285+
"resourceVersion": "43",
286+
},
287+
"a": "old",
288+
"b": "changed",
289+
"c": "added",
290+
},
291+
},
292+
},
293+
},
224294
}
225295

226296
for name, tc := range cases {
@@ -518,3 +588,137 @@ func TestAPIFinalizerAdder(t *testing.T) {
518588
})
519589
}
520590
}
591+
592+
func TestAdditiveMergePatchApplyOption(t *testing.T) {
593+
type args struct {
594+
current runtime.Object
595+
desired runtime.Object
596+
}
597+
type want struct {
598+
err error
599+
current runtime.Object
600+
desired runtime.Object
601+
}
602+
tests := []struct {
603+
name string
604+
args args
605+
want want
606+
}{
607+
{
608+
name: "equal unstructed",
609+
args: args{
610+
current: &unstructured.Unstructured{Object: map[string]interface{}{
611+
"kind": "Thing",
612+
"a": "foo",
613+
}},
614+
desired: &unstructured.Unstructured{Object: map[string]interface{}{
615+
"kind": "Thing",
616+
"a": "foo",
617+
}},
618+
},
619+
want: want{
620+
current: &unstructured.Unstructured{Object: map[string]interface{}{
621+
"kind": "Thing",
622+
"a": "foo",
623+
}},
624+
desired: &unstructured.Unstructured{Object: map[string]interface{}{
625+
"kind": "Thing",
626+
"a": "foo",
627+
}},
628+
},
629+
},
630+
{
631+
name: "overlapping unstructed",
632+
args: args{
633+
current: &unstructured.Unstructured{Object: map[string]interface{}{
634+
"kind": "Thing",
635+
"a": "foo",
636+
"b": "foo",
637+
"c": "foo",
638+
}},
639+
desired: &unstructured.Unstructured{Object: map[string]interface{}{
640+
"kind": "Thing",
641+
"a": "foo",
642+
"b": "bar",
643+
"d": "bar",
644+
}},
645+
},
646+
want: want{
647+
current: &unstructured.Unstructured{Object: map[string]interface{}{
648+
"kind": "Thing",
649+
"a": "foo",
650+
"b": "foo",
651+
"c": "foo",
652+
}},
653+
desired: &unstructured.Unstructured{Object: map[string]interface{}{
654+
"kind": "Thing",
655+
"a": "foo",
656+
"b": "bar",
657+
"c": "foo",
658+
"d": "bar",
659+
}},
660+
},
661+
},
662+
{
663+
name: "equal typed",
664+
args: args{
665+
current: &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{
666+
"a": "foo",
667+
}}},
668+
desired: &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{
669+
"a": "foo",
670+
}}},
671+
},
672+
want: want{
673+
current: &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{
674+
"a": "foo",
675+
}}},
676+
desired: &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{
677+
"a": "foo",
678+
}}},
679+
},
680+
},
681+
{
682+
name: "overlapping typed",
683+
args: args{
684+
current: &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{
685+
"a": "foo",
686+
"b": "foo",
687+
"c": "foo",
688+
}}},
689+
desired: &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{
690+
"a": "foo",
691+
"b": "bar",
692+
"d": "bar",
693+
}}},
694+
},
695+
want: want{
696+
current: &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{
697+
"a": "foo",
698+
"b": "foo",
699+
"c": "foo",
700+
}}},
701+
desired: &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{
702+
"a": "foo",
703+
"b": "bar",
704+
"c": "foo",
705+
"d": "bar",
706+
}}},
707+
},
708+
},
709+
}
710+
for _, tt := range tests {
711+
t.Run(tt.name, func(t *testing.T) {
712+
err := AdditiveMergePatchApplyOption(context.Background(), tt.args.current, tt.args.desired)
713+
if diff := cmp.Diff(tt.want.err, err, test.EquateErrors()); diff != "" {
714+
t.Errorf("AdditiveMergePatchApplyOption() error = %v, wantErr %v", err, tt.want.err)
715+
}
716+
if diff := cmp.Diff(tt.want.current, tt.args.current); diff != "" {
717+
t.Errorf("AdditiveMergePatchApplyOption() current = %v, want %v", tt.args.current, tt.want.current)
718+
}
719+
if diff := cmp.Diff(tt.want.desired, tt.args.desired); diff != "" {
720+
t.Errorf("AdditiveMergePatchApplyOption() current = %v, want %v", tt.args.desired, tt.want.desired)
721+
}
722+
})
723+
}
724+
}

0 commit comments

Comments
 (0)