From ac5705d1a052935ae60b48a026aa55fbe679acb8 Mon Sep 17 00:00:00 2001 From: Alvaro Aleman Date: Sun, 13 Oct 2024 23:01:25 -0400 Subject: [PATCH] :sparkles: Fakeclient: Add apply support This change is a POC for adding apply patch support to the fake client. This relies on the upstream support for this which is implemented in a new [FieldManagedObjectTracker][0]. There are two major problems with this for us though: 1. It requires a [type converter][1] which gets initialized with a [parser][2] that knows how the type look like. It doesn't look like it is possible to pass multiple parsers, which means the resulting client can only deals with the set of types the parser knows about, for example what is in client-go but it will not work with CRDs 2. We have some code that wants to look at the object after it was patched to check if its valid - Since this is implemented in the tracker, it doesn't look like its possible to dry run the patch [0]: https://github.com/kubernetes/kubernetes/blob/4dc7a48ac6fb631a84e1974772bf7b8fd0bb9c59/staging/src/k8s.io/client-go/testing/fixture.go#L643 [1]: https://github.com/kubernetes/kubernetes/blob/4dc7a48ac6fb631a84e1974772bf7b8fd0bb9c59/staging/src/k8s.io/client-go/applyconfigurations/utils.go#L1739 [2]: https://github.com/kubernetes/kubernetes/blob/4dc7a48ac6fb631a84e1974772bf7b8fd0bb9c59/staging/src/k8s.io/client-go/applyconfigurations/internal/internal.go#L28 --- pkg/client/fake/client.go | 40 +++++++++++++++++++++++++++++----- pkg/client/fake/client_test.go | 22 +++++++++++++++++++ 2 files changed, 57 insertions(+), 5 deletions(-) diff --git a/pkg/client/fake/client.go b/pkg/client/fake/client.go index 87a8a8380b..a5bbadf709 100644 --- a/pkg/client/fake/client.go +++ b/pkg/client/fake/client.go @@ -45,12 +45,14 @@ import ( "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/runtime/serializer" "k8s.io/apimachinery/pkg/types" utilrand "k8s.io/apimachinery/pkg/util/rand" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/strategicpatch" "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/apimachinery/pkg/watch" + clientgoapplyconfigurations "k8s.io/client-go/applyconfigurations" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/testing" "k8s.io/utils/ptr" @@ -230,7 +232,11 @@ func (f *ClientBuilder) Build() client.WithWatch { } if f.objectTracker == nil { - tracker = versionedTracker{ObjectTracker: testing.NewObjectTracker(f.scheme, scheme.Codecs.UniversalDecoder()), scheme: f.scheme, withStatusSubresource: withStatusSubResource} + tracker = versionedTracker{ObjectTracker: testing.NewFieldManagedObjectTracker( + f.scheme, + serializer.NewCodecFactory(f.scheme).UniversalDecoder(), + clientgoapplyconfigurations.NewTypeConverter(f.scheme), + )} } else { tracker = versionedTracker{ObjectTracker: f.objectTracker, scheme: f.scheme, withStatusSubresource: withStatusSubResource} } @@ -868,6 +874,12 @@ func (c *fakeClient) patch(obj client.Object, patch client.Patch, opts ...client if err != nil { return err } + + // otherwise the merge logic in the tracker complains + if patch.Type() == types.ApplyPatchType { + obj.SetManagedFields(nil) + } + data, err := patch.Data(obj) if err != nil { return err @@ -880,7 +892,11 @@ func (c *fakeClient) patch(obj client.Object, patch client.Patch, opts ...client oldObj, err := c.tracker.Get(gvr, accessor.GetNamespace(), accessor.GetName()) if err != nil { - return err + if patch.Type() == types.ApplyPatchType { + oldObj = &unstructured.Unstructured{} + } else { + return err + } } oldAccessor, err := meta.Accessor(oldObj) if err != nil { @@ -895,7 +911,7 @@ func (c *fakeClient) patch(obj client.Object, patch client.Patch, opts ...client // This ensures that the patch may be rejected if a deletionTimestamp is modified, prior // to updating the object. action := testing.NewPatchAction(gvr, accessor.GetNamespace(), accessor.GetName(), patch.Type(), data) - o, err := dryPatch(action, c.tracker) + o, err := dryPatch(action, c.tracker, obj) if err != nil { return err } @@ -954,12 +970,15 @@ func deletionTimestampEqual(newObj metav1.Object, obj metav1.Object) bool { // This results in some code duplication, but was found to be a cleaner alternative than unmarshalling and introspecting the patch data // and easier than refactoring the k8s client-go method upstream. // Duplicate of upstream: https://github.com/kubernetes/client-go/blob/783d0d33626e59d55d52bfd7696b775851f92107/testing/fixture.go#L146-L194 -func dryPatch(action testing.PatchActionImpl, tracker testing.ObjectTracker) (runtime.Object, error) { +func dryPatch(action testing.PatchActionImpl, tracker testing.ObjectTracker, newObj runtime.Object) (runtime.Object, error) { ns := action.GetNamespace() gvr := action.GetResource() obj, err := tracker.Get(gvr, ns, action.GetName()) if err != nil { + if action.GetPatchType() == types.ApplyPatchType { + return &unstructured.Unstructured{}, nil + } return nil, err } @@ -1005,7 +1024,18 @@ func dryPatch(action testing.PatchActionImpl, tracker testing.ObjectTracker) (ru return nil, err } case types.ApplyPatchType: - return nil, errors.New("apply patches are not supported in the fake client. Follow https://github.com/kubernetes/kubernetes/issues/115598 for the current status") + // There doesn't seem to be a way to test this without actually applying it as apply is implemented in the tracker. + // We have to make sure no reader sees this and we can not handle errors resetting the obj to the original state. + defer tracker.Add(obj) + defer func() { + if err := tracker.Add(obj); err != nil { + panic(err) + } + }() + if err := tracker.Apply(gvr, newObj, ns, action.PatchOptions); err != nil { + return nil, err + } + return tracker.Get(gvr, ns, action.GetName()) default: return nil, fmt.Errorf("%s PatchType is not supported", action.GetPatchType()) } diff --git a/pkg/client/fake/client_test.go b/pkg/client/fake/client_test.go index 0a7d17db47..dbf5d97706 100644 --- a/pkg/client/fake/client_test.go +++ b/pkg/client/fake/client_test.go @@ -2169,6 +2169,28 @@ var _ = Describe("Fake client", func() { Expect(cl.SubResource(subResourceScale).Update(context.Background(), obj, client.WithSubResourceBody(scale)).Error()).To(Equal(expectedErr)) }) + FIt("supports server-side apply", func() { + cl := NewClientBuilder().Build() + obj := &unstructured.Unstructured{} + obj.SetAPIVersion("v1") + obj.SetKind("ConfigMap") + obj.SetName("foo") + unstructured.SetNestedField(obj.Object, map[string]any{"some": "data"}, "data") + + Expect(cl.Patch(context.Background(), obj, client.Apply)).To(Succeed()) + + cm := &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "foo"}} + + Expect(cl.Get(context.Background(), client.ObjectKeyFromObject(cm), cm)).To(Succeed()) + Expect(cm.Data).To(Equal(map[string]string{"some": "data"})) + + unstructured.SetNestedField(obj.Object, map[string]any{"other": "data"}, "data") + Expect(cl.Patch(context.Background(), obj, client.Apply)).To(Succeed()) + + Expect(cl.Get(context.Background(), client.ObjectKeyFromObject(cm), cm)).To(Succeed()) + Expect(cm.Data).To(Equal(map[string]string{"other": "data"})) + }) + scalableObjs := []client.Object{ &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{