From a148900dec87cc2c0129fb8ccbe0afb3815da5f8 Mon Sep 17 00:00:00 2001 From: Alvaro Aleman Date: Sat, 7 Jun 2025 18:05:38 -0400 Subject: [PATCH 1/4] :sparkles: Add native SSA support This change adds native server-side apply support to the client by extending it with an `Apply` method that takes an `runtime.ApplyConfiguration`. --- pkg/cache/internal/informers.go | 2 +- pkg/client/apiutil/apimachinery.go | 20 ++++-- pkg/client/applyconfigurations.go | 56 +++++++++++++++ pkg/client/client.go | 10 +++ pkg/client/client_rest_resources.go | 82 +++++++++++++++++---- pkg/client/client_test.go | 92 ++++++++++++++++++++++++ pkg/client/dryrun.go | 4 ++ pkg/client/fake/client.go | 43 ++++++++++- pkg/client/fake/client_test.go | 85 +++++++++++++++++++--- pkg/client/fieldowner.go | 4 ++ pkg/client/fieldvalidation.go | 5 ++ pkg/client/interceptor/intercept.go | 9 +++ pkg/client/interceptor/intercept_test.go | 4 ++ pkg/client/interfaces.go | 3 + pkg/client/namespaced_client.go | 5 ++ pkg/client/options.go | 85 ++++++++++++++++++++-- pkg/client/patch.go | 1 + pkg/client/typed_client.go | 57 +++++++++++---- pkg/client/unstructured_client.go | 57 +++++++++++---- 19 files changed, 558 insertions(+), 66 deletions(-) create mode 100644 pkg/client/applyconfigurations.go diff --git a/pkg/cache/internal/informers.go b/pkg/cache/internal/informers.go index 4bf832b2d9..f216be0d9e 100644 --- a/pkg/cache/internal/informers.go +++ b/pkg/cache/internal/informers.go @@ -518,7 +518,7 @@ func (ip *Informers) makeListWatcher(gvk schema.GroupVersionKind, obj runtime.Ob // Structured. // default: - client, err := apiutil.RESTClientForGVK(gvk, false, ip.config, ip.codecs, ip.httpClient) + client, err := apiutil.RESTClientForGVK(gvk, false, false, ip.config, ip.codecs, ip.httpClient) if err != nil { return nil, err } diff --git a/pkg/client/apiutil/apimachinery.go b/pkg/client/apiutil/apimachinery.go index 1d4ce264c9..b132cb2d4d 100644 --- a/pkg/client/apiutil/apimachinery.go +++ b/pkg/client/apiutil/apimachinery.go @@ -161,15 +161,27 @@ func GVKForObject(obj runtime.Object, scheme *runtime.Scheme) (schema.GroupVersi // RESTClientForGVK constructs a new rest.Interface capable of accessing the resource associated // with the given GroupVersionKind. The REST client will be configured to use the negotiated serializer from // baseConfig, if set, otherwise a default serializer will be set. -func RESTClientForGVK(gvk schema.GroupVersionKind, isUnstructured bool, baseConfig *rest.Config, codecs serializer.CodecFactory, httpClient *http.Client) (rest.Interface, error) { +func RESTClientForGVK( + gvk schema.GroupVersionKind, + forceDisableProtoBuf bool, + isUnstructured bool, + baseConfig *rest.Config, + codecs serializer.CodecFactory, + httpClient *http.Client, +) (rest.Interface, error) { if httpClient == nil { return nil, fmt.Errorf("httpClient must not be nil, consider using rest.HTTPClientFor(c) to create a client") } - return rest.RESTClientForConfigAndClient(createRestConfig(gvk, isUnstructured, baseConfig, codecs), httpClient) + return rest.RESTClientForConfigAndClient(createRestConfig(gvk, forceDisableProtoBuf, isUnstructured, baseConfig, codecs), httpClient) } // createRestConfig copies the base config and updates needed fields for a new rest config. -func createRestConfig(gvk schema.GroupVersionKind, isUnstructured bool, baseConfig *rest.Config, codecs serializer.CodecFactory) *rest.Config { +func createRestConfig(gvk schema.GroupVersionKind, + forceDisableProtoBuf bool, + isUnstructured bool, + baseConfig *rest.Config, + codecs serializer.CodecFactory, +) *rest.Config { gv := gvk.GroupVersion() cfg := rest.CopyConfig(baseConfig) @@ -183,7 +195,7 @@ func createRestConfig(gvk schema.GroupVersionKind, isUnstructured bool, baseConf cfg.UserAgent = rest.DefaultKubernetesUserAgent() } // TODO(FillZpp): In the long run, we want to check discovery or something to make sure that this is actually true. - if cfg.ContentType == "" && !isUnstructured { + if cfg.ContentType == "" && !forceDisableProtoBuf { protobufSchemeLock.RLock() if protobufScheme.Recognizes(gvk) { cfg.ContentType = runtime.ContentTypeProtobuf diff --git a/pkg/client/applyconfigurations.go b/pkg/client/applyconfigurations.go new file mode 100644 index 0000000000..15d29acd95 --- /dev/null +++ b/pkg/client/applyconfigurations.go @@ -0,0 +1,56 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package client + +import ( + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" +) + +type unstructuredApplyConfiguration struct { + *unstructured.Unstructured +} + +func (u *unstructuredApplyConfiguration) IsApplyConfiguration() {} + +// ApplyConfigurationFromUnstructured creates a runtime.ApplyConfiguration from an *unstructured.Unstructured object. +func ApplyConfigurationFromUnstructured(u *unstructured.Unstructured) runtime.ApplyConfiguration { + return &unstructuredApplyConfiguration{Unstructured: u} +} + +type applyconfigurationRuntimeObject struct { + runtime.ApplyConfiguration +} + +func (a *applyconfigurationRuntimeObject) GetObjectKind() schema.ObjectKind { + return a +} + +func (a *applyconfigurationRuntimeObject) GroupVersionKind() schema.GroupVersionKind { + return schema.GroupVersionKind{} +} + +func (a *applyconfigurationRuntimeObject) SetGroupVersionKind(gvk schema.GroupVersionKind) {} + +func (a *applyconfigurationRuntimeObject) DeepCopyObject() runtime.Object { + panic("applyconfigurationRuntimeObject does not support DeepCopyObject") +} + +func runtimeObjectFromApplyConfiguration(ac runtime.ApplyConfiguration) runtime.Object { + return &applyconfigurationRuntimeObject{ApplyConfiguration: ac} +} diff --git a/pkg/client/client.go b/pkg/client/client.go index 50b0ebf338..092deb43d4 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -329,6 +329,16 @@ func (c *client) Patch(ctx context.Context, obj Object, patch Patch, opts ...Pat } } +func (c *client) Apply(ctx context.Context, obj runtime.ApplyConfiguration, opts ...ApplyOption) error { + switch obj := obj.(type) { + case *unstructuredApplyConfiguration: + defer c.resetGroupVersionKind(obj, obj.GetObjectKind().GroupVersionKind()) + return c.unstructuredClient.Apply(ctx, obj, opts...) + default: + return c.typedClient.Apply(ctx, obj, opts...) + } +} + // Get implements client.Client. func (c *client) Get(ctx context.Context, key ObjectKey, obj Object, opts ...GetOption) error { if isUncached, err := c.shouldBypassCache(obj); err != nil { diff --git a/pkg/client/client_rest_resources.go b/pkg/client/client_rest_resources.go index 2d07879520..80e269bdbf 100644 --- a/pkg/client/client_rest_resources.go +++ b/pkg/client/client_rest_resources.go @@ -17,16 +17,17 @@ limitations under the License. package client import ( + "fmt" "net/http" "strings" "sync" "k8s.io/apimachinery/pkg/api/meta" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/runtime/serializer" "k8s.io/client-go/rest" + "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client/apiutil" ) @@ -56,13 +57,17 @@ type clientRestResources struct { // newResource maps obj to a Kubernetes Resource and constructs a client for that Resource. // If the object is a list, the resource represents the item's type instead. -func (c *clientRestResources) newResource(gvk schema.GroupVersionKind, isList, isUnstructured bool) (*resourceMeta, error) { +func (c *clientRestResources) newResource(gvk schema.GroupVersionKind, + isList bool, + forceDisableProtoBuf bool, + isUnstructured bool, +) (*resourceMeta, error) { if strings.HasSuffix(gvk.Kind, "List") && isList { // if this was a list, treat it as a request for the item's resource gvk.Kind = gvk.Kind[:len(gvk.Kind)-4] } - client, err := apiutil.RESTClientForGVK(gvk, isUnstructured, c.config, c.codecs, c.httpClient) + client, err := apiutil.RESTClientForGVK(gvk, forceDisableProtoBuf, isUnstructured, c.config, c.codecs, c.httpClient) if err != nil { return nil, err } @@ -73,15 +78,44 @@ func (c *clientRestResources) newResource(gvk schema.GroupVersionKind, isList, i return &resourceMeta{Interface: client, mapping: mapping, gvk: gvk}, nil } +type applyConfiguration interface { + GetName() *string + GetNamespace() *string + GetKind() *string + GetAPIVersion() *string +} + // getResource returns the resource meta information for the given type of object. // If the object is a list, the resource represents the item's type instead. -func (c *clientRestResources) getResource(obj runtime.Object) (*resourceMeta, error) { - gvk, err := apiutil.GVKForObject(obj, c.scheme) - if err != nil { - return nil, err +func (c *clientRestResources) getResource(obj any) (*resourceMeta, error) { + var gvk schema.GroupVersionKind + var err error + var isApplyConfiguration bool + switch o := obj.(type) { + case runtime.Object: + gvk, err = apiutil.GVKForObject(o, c.scheme) + if err != nil { + return nil, err + } + case runtime.ApplyConfiguration: + ac, ok := o.(applyConfiguration) + if !ok { + return nil, fmt.Errorf("%T is a runtime.ApplyConfiguration but not an applyConfiguration", o) + } + gv, err := schema.ParseGroupVersion(ptr.Deref(ac.GetAPIVersion(), "")) + if err != nil { + return nil, fmt.Errorf("failed to parse %q as GroupVersion: %w", ptr.Deref(ac.GetAPIVersion(), ""), err) + } + gvk.Group = gv.Group + gvk.Version = gv.Version + gvk.Kind = ptr.Deref(ac.GetKind(), "") + isApplyConfiguration = true + default: + return nil, fmt.Errorf("bug: %T is neither a runtime.Object nor a runtime.ApplyConfiguration", o) } _, isUnstructured := obj.(runtime.Unstructured) + disableProtoBuf := isUnstructured || isApplyConfiguration // It's better to do creation work twice than to not let multiple // people make requests at once @@ -97,10 +131,15 @@ func (c *clientRestResources) getResource(obj runtime.Object) (*resourceMeta, er return r, nil } + var isList bool + if runtimeObject, ok := obj.(runtime.Object); ok && meta.IsListType(runtimeObject) { + isList = true + } + // Initialize a new Client c.mu.Lock() defer c.mu.Unlock() - r, err = c.newResource(gvk, meta.IsListType(obj), isUnstructured) + r, err = c.newResource(gvk, isList, disableProtoBuf, isUnstructured) if err != nil { return nil, err } @@ -109,16 +148,29 @@ func (c *clientRestResources) getResource(obj runtime.Object) (*resourceMeta, er } // getObjMeta returns objMeta containing both type and object metadata and state. -func (c *clientRestResources) getObjMeta(obj runtime.Object) (*objMeta, error) { +func (c *clientRestResources) getObjMeta(obj any) (*objMeta, error) { r, err := c.getResource(obj) if err != nil { return nil, err } - m, err := meta.Accessor(obj) - if err != nil { - return nil, err + objMeta := &objMeta{resourceMeta: r} + + switch o := obj.(type) { + case runtime.Object: + m, err := meta.Accessor(obj) + if err != nil { + return nil, err + } + objMeta.namespace = m.GetNamespace() + objMeta.name = m.GetName() + case applyConfiguration: + objMeta.namespace = ptr.Deref(o.GetNamespace(), "") + objMeta.name = ptr.Deref(o.GetName(), "") + default: + return nil, fmt.Errorf("object %T is neither a runtime.Object nor a runtime.ApplyConfiguration", obj) } - return &objMeta{resourceMeta: r, Object: m}, err + + return objMeta, nil } // resourceMeta stores state for a Kubernetes type. @@ -146,6 +198,6 @@ type objMeta struct { // resourceMeta contains type information for the object *resourceMeta - // Object contains meta data for the object instance - metav1.Object + namespace string + name string } diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index 01075f1255..d7748f0fb6 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -43,6 +43,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" + corev1applyconfigurations "k8s.io/client-go/applyconfigurations/core/v1" kscheme "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" "k8s.io/utils/ptr" @@ -859,6 +860,97 @@ U5wwSivyi7vmegHKmblOzNVKA5qPO8zWzqBC }) }) + Describe("Apply", func() { + Context("Unstructured Client", func() { + It("should create and update a configMap using SSA", func() { + cl, err := client.New(cfg, client.Options{}) + Expect(err).NotTo(HaveOccurred()) + Expect(cl).NotTo(BeNil()) + + data := map[string]any{ + "some-key": "some-value", + } + obj := &unstructured.Unstructured{Object: map[string]any{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]any{ + "name": "test-configmap", + "namespace": "default", + }, + "data": data, + }} + + err = cl.Apply(context.Background(), client.ApplyConfigurationFromUnstructured(obj), &client.ApplyOptions{FieldManager: "test-manager"}) + Expect(err).NotTo(HaveOccurred()) + + cm, err := clientset.CoreV1().ConfigMaps(obj.GetNamespace()).Get(context.Background(), obj.GetName(), metav1.GetOptions{}) + Expect(err).NotTo(HaveOccurred()) + + actualData := map[string]any{} + for k, v := range cm.Data { + actualData[k] = v + } + + Expect(actualData).To(BeComparableTo(data)) + Expect(actualData).To(BeComparableTo(obj.Object["data"])) + + data["a-new-key"] = "a-new-value" + obj.Object["data"] = data + unstructured.RemoveNestedField(obj.Object, "metadata", "managedFields") + + err = cl.Apply(context.Background(), client.ApplyConfigurationFromUnstructured(obj), &client.ApplyOptions{FieldManager: "test-manager"}) + Expect(err).NotTo(HaveOccurred()) + + cm, err = clientset.CoreV1().ConfigMaps(obj.GetNamespace()).Get(context.Background(), obj.GetName(), metav1.GetOptions{}) + Expect(err).NotTo(HaveOccurred()) + + actualData = map[string]any{} + for k, v := range cm.Data { + actualData[k] = v + } + + Expect(actualData).To(BeComparableTo(data)) + Expect(actualData).To(BeComparableTo(obj.Object["data"])) + }) + }) + + Context("Structured Client", func() { + It("should create and update a configMap using SSA", func() { + cl, err := client.New(cfg, client.Options{}) + Expect(err).NotTo(HaveOccurred()) + Expect(cl).NotTo(BeNil()) + + data := map[string]string{ + "some-key": "some-value", + } + obj := corev1applyconfigurations. + ConfigMap("test-configmap", "default"). + WithData(data) + + err = cl.Apply(context.Background(), obj, &client.ApplyOptions{FieldManager: "test-manager"}) + Expect(err).NotTo(HaveOccurred()) + + cm, err := clientset.CoreV1().ConfigMaps(ptr.Deref(obj.GetNamespace(), "")).Get(context.Background(), ptr.Deref(obj.GetName(), ""), metav1.GetOptions{}) + Expect(err).NotTo(HaveOccurred()) + + Expect(cm.Data).To(BeComparableTo(data)) + Expect(cm.Data).To(BeComparableTo(obj.Data)) + + data["a-new-key"] = "a-new-value" + obj.Data = data + + err = cl.Apply(context.Background(), obj, &client.ApplyOptions{FieldManager: "test-manager"}) + Expect(err).NotTo(HaveOccurred()) + + cm, err = clientset.CoreV1().ConfigMaps(ptr.Deref(obj.GetNamespace(), "")).Get(context.Background(), ptr.Deref(obj.GetName(), ""), metav1.GetOptions{}) + Expect(err).NotTo(HaveOccurred()) + + Expect(cm.Data).To(BeComparableTo(data)) + Expect(cm.Data).To(BeComparableTo(obj.Data)) + }) + }) + }) + Describe("SubResourceClient", func() { Context("with structured objects", func() { It("should be able to read the Scale subresource", func() { diff --git a/pkg/client/dryrun.go b/pkg/client/dryrun.go index bbcdd38321..a185860d33 100644 --- a/pkg/client/dryrun.go +++ b/pkg/client/dryrun.go @@ -82,6 +82,10 @@ func (c *dryRunClient) Patch(ctx context.Context, obj Object, patch Patch, opts return c.client.Patch(ctx, obj, patch, append(opts, DryRunAll)...) } +func (c *dryRunClient) Apply(ctx context.Context, obj runtime.ApplyConfiguration, opts ...ApplyOption) error { + return c.client.Apply(ctx, obj, append(opts, DryRunAll)...) +} + // Get implements client.Client. func (c *dryRunClient) Get(ctx context.Context, key ObjectKey, obj Object, opts ...GetOption) error { return c.client.Get(ctx, key, obj, opts...) diff --git a/pkg/client/fake/client.go b/pkg/client/fake/client.go index 2ee891e26b..380c728b0d 100644 --- a/pkg/client/fake/client.go +++ b/pkg/client/fake/client.go @@ -52,6 +52,7 @@ import ( "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/apis/meta/v1/validation" "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" @@ -1046,17 +1047,53 @@ func (c *fakeClient) Patch(ctx context.Context, obj client.Object, patch client. return c.patch(obj, patch, opts...) } +func (c *fakeClient) Apply(ctx context.Context, obj runtime.ApplyConfiguration, opts ...client.ApplyOption) error { + applyOpts := &client.ApplyOptions{} + applyOpts.ApplyOptions(opts) + + data, err := json.Marshal(obj) + if err != nil { + return fmt.Errorf("failed to marshal apply configuration: %w", err) + } + + u := &unstructured.Unstructured{} + if err := json.Unmarshal(data, u); err != nil { + return fmt.Errorf("failed to unmarshal apply configuration: %w", err) + } + + applyPatch := &fakeApplyPatch{} + + patchOpts := &client.PatchOptions{} + patchOpts.Raw = applyOpts.AsPatchOptions() + + return c.patch(u, applyPatch, patchOpts) +} + +type fakeApplyPatch struct{} + +func (p *fakeApplyPatch) Type() types.PatchType { + return types.ApplyPatchType +} + +func (p *fakeApplyPatch) Data(obj client.Object) ([]byte, error) { + return json.Marshal(obj) +} + func (c *fakeClient) patch(obj client.Object, patch client.Patch, opts ...client.PatchOption) error { if err := c.addToSchemeIfUnknownAndUnstructuredOrPartial(obj); err != nil { return err } - c.schemeLock.RLock() - defer c.schemeLock.RUnlock() - patchOptions := &client.PatchOptions{} patchOptions.ApplyOptions(opts) + if errs := validation.ValidatePatchOptions(patchOptions.AsPatchOptions(), patch.Type()); len(errs) > 0 { + return apierrors.NewInvalid(schema.GroupKind{Group: "meta.k8s.io", Kind: "PatchOptions"}, "", errs) + } + + c.schemeLock.RLock() + defer c.schemeLock.RUnlock() + for _, dryRunOpt := range patchOptions.DryRun { if dryRunOpt == metav1.DryRunAll { return nil diff --git a/pkg/client/fake/client_test.go b/pkg/client/fake/client_test.go index 707d74adc9..1ee1b71bd3 100644 --- a/pkg/client/fake/client_test.go +++ b/pkg/client/fake/client_test.go @@ -45,6 +45,7 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/watch" clientgoapplyconfigurations "k8s.io/client-go/applyconfigurations" + corev1applyconfigurations "k8s.io/client-go/applyconfigurations/core/v1" "k8s.io/client-go/kubernetes/fake" clientgoscheme "k8s.io/client-go/kubernetes/scheme" "k8s.io/utils/ptr" @@ -2548,7 +2549,7 @@ var _ = Describe("Fake client", func() { obj.SetName("foo") Expect(unstructured.SetNestedField(obj.Object, map[string]any{"some": "data"}, "data")).To(Succeed()) - Expect(cl.Patch(context.Background(), obj, client.Apply)).To(Succeed()) + Expect(cl.Patch(context.Background(), obj, client.Apply, client.FieldOwner("foo"))).To(Succeed()) cm := &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "foo"}} @@ -2556,7 +2557,7 @@ var _ = Describe("Fake client", func() { Expect(cm.Data).To(Equal(map[string]string{"some": "data"})) Expect(unstructured.SetNestedField(obj.Object, map[string]any{"other": "data"}, "data")).To(Succeed()) - Expect(cl.Patch(context.Background(), obj, client.Apply)).To(Succeed()) + Expect(cl.Patch(context.Background(), obj, client.Apply, client.FieldOwner("foo"))).To(Succeed()) Expect(cl.Get(context.Background(), client.ObjectKeyFromObject(cm), cm)).To(Succeed()) Expect(cm.Data).To(Equal(map[string]string{"other": "data"})) @@ -2572,13 +2573,13 @@ var _ = Describe("Fake client", func() { Expect(unstructured.SetNestedField(obj.Object, map[string]any{"some": "data"}, "spec")).To(Succeed()) - Expect(cl.Patch(context.Background(), obj, client.Apply)).To(Succeed()) + Expect(cl.Patch(context.Background(), obj, client.Apply, client.FieldOwner("foo"))).To(Succeed()) Expect(cl.Get(context.Background(), client.ObjectKeyFromObject(result), result)).To(Succeed()) Expect(result.Object["spec"]).To(Equal(map[string]any{"some": "data"})) Expect(unstructured.SetNestedField(obj.Object, map[string]any{"other": "data"}, "spec")).To(Succeed()) - Expect(cl.Patch(context.Background(), obj, client.Apply)).To(Succeed()) + Expect(cl.Patch(context.Background(), obj, client.Apply, client.FieldOwner("foo"))).To(Succeed()) Expect(cl.Get(context.Background(), client.ObjectKeyFromObject(result), result)).To(Succeed()) Expect(result.Object["spec"]).To(Equal(map[string]any{"other": "data"})) @@ -2592,9 +2593,9 @@ var _ = Describe("Fake client", func() { obj.SetName("foo") Expect(unstructured.SetNestedField(obj.Object, map[string]any{"some": "data"}, "data")).To(Succeed()) - Expect(cl.Patch(context.Background(), obj, client.Apply)).To(Succeed()) + Expect(cl.Patch(context.Background(), obj, client.Apply, client.FieldOwner("foo"))).To(Succeed()) - err := cl.Patch(context.Background(), obj, client.Apply) + err := cl.Patch(context.Background(), obj, client.Apply, client.FieldOwner("foo")) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("metadata.managedFields must be nil")) }) @@ -2610,7 +2611,7 @@ var _ = Describe("Fake client", func() { Expect(unstructured.SetNestedField(obj.Object, map[string]any{"some": "data"}, "data")).To(Succeed()) - Expect(cl.Patch(context.Background(), obj, client.Apply)).To(Succeed()) + Expect(cl.Patch(context.Background(), obj, client.Apply, client.FieldOwner("foo"))).To(Succeed()) cm := &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "foo"}} @@ -2618,7 +2619,7 @@ var _ = Describe("Fake client", func() { Expect(cm.Data).To(Equal(map[string]string{"some": "data"})) Expect(unstructured.SetNestedField(obj.Object, map[string]any{"other": "data"}, "data")).To(Succeed()) - Expect(cl.Patch(context.Background(), obj, client.Apply)).To(Succeed()) + Expect(cl.Patch(context.Background(), obj, client.Apply, client.FieldOwner("foo"))).To(Succeed()) Expect(cl.Get(context.Background(), client.ObjectKeyFromObject(cm), cm)).To(Succeed()) Expect(cm.Data).To(Equal(map[string]string{"other": "data"})) @@ -2664,12 +2665,78 @@ var _ = Describe("Fake client", func() { "ssa": "value", }, }} - Expect(cl.Patch(context.Background(), u, client.Apply)).NotTo(HaveOccurred()) + Expect(cl.Patch(context.Background(), u, client.Apply, client.FieldOwner("foo"))).NotTo(HaveOccurred()) _, exists, err := unstructured.NestedFieldNoCopy(u.Object, "metadata", "managedFields") Expect(err).NotTo(HaveOccurred()) Expect(exists).To(BeTrue()) }) + It("supports server-side apply of a client-go resource via Apply method", func() { + cl := NewClientBuilder().Build() + obj := corev1applyconfigurations. + ConfigMap("foo", "default"). + WithData(map[string]string{"some": "data"}) + + Expect(cl.Apply(context.Background(), obj, &client.ApplyOptions{FieldManager: "test-manager"})).To(Succeed()) + + cm := &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"}} + + Expect(cl.Get(context.Background(), client.ObjectKeyFromObject(cm), cm)).To(Succeed()) + Expect(cm.Data).To(BeComparableTo(map[string]string{"some": "data"})) + + obj.Data = map[string]string{"other": "data"} + Expect(cl.Apply(context.Background(), obj, &client.ApplyOptions{FieldManager: "test-manager"})).To(Succeed()) + + Expect(cl.Get(context.Background(), client.ObjectKeyFromObject(cm), cm)).To(Succeed()) + Expect(cm.Data).To(BeComparableTo(map[string]string{"other": "data"})) + }) + + It("errors when trying to server-side apply an object without configuring a FieldManager", func() { + cl := NewClientBuilder().Build() + obj := corev1applyconfigurations. + ConfigMap("foo", "default"). + WithData(map[string]string{"some": "data"}) + + err := cl.Apply(context.Background(), obj) + Expect(err).To(HaveOccurred()) + Expect(apierrors.IsInvalid(err)).To(BeTrue()) + }) + + It("errors when trying to server-side apply an object with an invalid FieldManager", func() { + cl := NewClientBuilder().Build() + obj := corev1applyconfigurations. + ConfigMap("foo", "default"). + WithData(map[string]string{"some": "data"}) + + err := cl.Apply(context.Background(), obj, client.FieldOwner("\x00")) + Expect(err).To(HaveOccurred()) + Expect(apierrors.IsInvalid(err)).To(BeTrue()) + }) + + It("supports server-side apply of a custom resource via Apply method", func() { + cl := NewClientBuilder().Build() + obj := &unstructured.Unstructured{} + obj.SetAPIVersion("custom/v1") + obj.SetKind("FakeResource") + obj.SetName("foo") + result := obj.DeepCopy() + + Expect(unstructured.SetNestedField(obj.Object, map[string]any{"some": "data"}, "spec")).To(Succeed()) + + applyConfig := client.ApplyConfigurationFromUnstructured(obj) + Expect(cl.Apply(context.Background(), applyConfig, &client.ApplyOptions{FieldManager: "test-manager"})).To(Succeed()) + + Expect(cl.Get(context.Background(), client.ObjectKeyFromObject(result), result)).To(Succeed()) + Expect(result.Object["spec"]).To(Equal(map[string]any{"some": "data"})) + + Expect(unstructured.SetNestedField(obj.Object, map[string]any{"other": "data"}, "spec")).To(Succeed()) + applyConfig2 := client.ApplyConfigurationFromUnstructured(obj) + Expect(cl.Apply(context.Background(), applyConfig2, &client.ApplyOptions{FieldManager: "test-manager"})).To(Succeed()) + + Expect(cl.Get(context.Background(), client.ObjectKeyFromObject(result), result)).To(Succeed()) + Expect(result.Object["spec"]).To(Equal(map[string]any{"other": "data"})) + }) + scalableObjs := []client.Object{ &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ diff --git a/pkg/client/fieldowner.go b/pkg/client/fieldowner.go index 07183cd192..93274f9500 100644 --- a/pkg/client/fieldowner.go +++ b/pkg/client/fieldowner.go @@ -54,6 +54,10 @@ func (f *clientWithFieldManager) Patch(ctx context.Context, obj Object, patch Pa return f.c.Patch(ctx, obj, patch, append([]PatchOption{FieldOwner(f.owner)}, opts...)...) } +func (f *clientWithFieldManager) Apply(ctx context.Context, obj runtime.ApplyConfiguration, opts ...ApplyOption) error { + return f.c.Apply(ctx, obj, append([]ApplyOption{FieldOwner(f.owner)}, opts...)...) +} + func (f *clientWithFieldManager) Delete(ctx context.Context, obj Object, opts ...DeleteOption) error { return f.c.Delete(ctx, obj, opts...) } diff --git a/pkg/client/fieldvalidation.go b/pkg/client/fieldvalidation.go index 659b3d44c9..fe5a7c8911 100644 --- a/pkg/client/fieldvalidation.go +++ b/pkg/client/fieldvalidation.go @@ -18,6 +18,7 @@ package client import ( "context" + "errors" "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/runtime" @@ -53,6 +54,10 @@ func (c *clientWithFieldValidation) Patch(ctx context.Context, obj Object, patch return c.client.Patch(ctx, obj, patch, append([]PatchOption{c.validation}, opts...)...) } +func (c *clientWithFieldValidation) Apply(ctx context.Context, obj runtime.ApplyConfiguration, opts ...ApplyOption) error { + return errors.New("Apply is not supported with field validation") +} + func (c *clientWithFieldValidation) Delete(ctx context.Context, obj Object, opts ...DeleteOption) error { return c.client.Delete(ctx, obj, opts...) } diff --git a/pkg/client/interceptor/intercept.go b/pkg/client/interceptor/intercept.go index 3d3f3cb011..7ff73bd8da 100644 --- a/pkg/client/interceptor/intercept.go +++ b/pkg/client/interceptor/intercept.go @@ -19,6 +19,7 @@ type Funcs struct { DeleteAllOf func(ctx context.Context, client client.WithWatch, obj client.Object, opts ...client.DeleteAllOfOption) error Update func(ctx context.Context, client client.WithWatch, obj client.Object, opts ...client.UpdateOption) error Patch func(ctx context.Context, client client.WithWatch, obj client.Object, patch client.Patch, opts ...client.PatchOption) error + Apply func(ctx context.Context, client client.WithWatch, obj runtime.ApplyConfiguration, opts ...client.ApplyOption) error Watch func(ctx context.Context, client client.WithWatch, obj client.ObjectList, opts ...client.ListOption) (watch.Interface, error) SubResource func(client client.WithWatch, subResource string) client.SubResourceClient SubResourceGet func(ctx context.Context, client client.Client, subResourceName string, obj client.Object, subResource client.Object, opts ...client.SubResourceGetOption) error @@ -92,6 +93,14 @@ func (c interceptor) Patch(ctx context.Context, obj client.Object, patch client. return c.client.Patch(ctx, obj, patch, opts...) } +func (c interceptor) Apply(ctx context.Context, obj runtime.ApplyConfiguration, opts ...client.ApplyOption) error { + if c.funcs.Apply != nil { + return c.funcs.Apply(ctx, c.client, obj, opts...) + } + + return c.client.Apply(ctx, obj, opts...) +} + func (c interceptor) DeleteAllOf(ctx context.Context, obj client.Object, opts ...client.DeleteAllOfOption) error { if c.funcs.DeleteAllOf != nil { return c.funcs.DeleteAllOf(ctx, c.client, obj, opts...) diff --git a/pkg/client/interceptor/intercept_test.go b/pkg/client/interceptor/intercept_test.go index a0536789b1..1ff88ea9b5 100644 --- a/pkg/client/interceptor/intercept_test.go +++ b/pkg/client/interceptor/intercept_test.go @@ -360,6 +360,10 @@ func (d dummyClient) Patch(ctx context.Context, obj client.Object, patch client. return nil } +func (d dummyClient) Apply(ctx context.Context, obj runtime.ApplyConfiguration, opts ...client.ApplyOption) error { + return nil +} + func (d dummyClient) DeleteAllOf(ctx context.Context, obj client.Object, opts ...client.DeleteAllOfOption) error { return nil } diff --git a/pkg/client/interfaces.go b/pkg/client/interfaces.go index 3b282fc2c5..20a16a97b8 100644 --- a/pkg/client/interfaces.go +++ b/pkg/client/interfaces.go @@ -78,6 +78,9 @@ type Writer interface { // DeleteAllOf deletes all objects of the given type matching the given options. DeleteAllOf(ctx context.Context, obj Object, opts ...DeleteAllOfOption) error + + // Apply applies the given apply configuration to the Kubernetes cluster. + Apply(ctx context.Context, obj runtime.ApplyConfiguration, opts ...ApplyOption) error } // StatusClient knows how to create a client which can update status subresource diff --git a/pkg/client/namespaced_client.go b/pkg/client/namespaced_client.go index 222dc79579..a8a60c31f7 100644 --- a/pkg/client/namespaced_client.go +++ b/pkg/client/namespaced_client.go @@ -18,6 +18,7 @@ package client import ( "context" + "errors" "fmt" "k8s.io/apimachinery/pkg/api/meta" @@ -147,6 +148,10 @@ func (n *namespacedClient) Patch(ctx context.Context, obj Object, patch Patch, o return n.client.Patch(ctx, obj, patch, opts...) } +func (n *namespacedClient) Apply(ctx context.Context, obj runtime.ApplyConfiguration, opts ...ApplyOption) error { + return errors.New("Apply is not supported on namespaced client") +} + // Get implements client.Client. func (n *namespacedClient) Get(ctx context.Context, key ObjectKey, obj Object, opts ...GetOption) error { isNamespaceScoped, err := n.IsObjectNamespaced(obj) diff --git a/pkg/client/options.go b/pkg/client/options.go index ad4b8182ac..d9f321febe 100644 --- a/pkg/client/options.go +++ b/pkg/client/options.go @@ -62,6 +62,12 @@ type PatchOption interface { ApplyToPatch(*PatchOptions) } +// ApplyOption is some configuration that modifies options for a apply request. +type ApplyOption interface { + // A ApplyToApply applies this configuration to the given apply options. + ApplyToApply(*ApplyOptions) +} + // DeleteAllOfOption is some configuration that modifies options for a delete request. type DeleteAllOfOption interface { // ApplyToDeleteAllOf applies this configuration to the given deletecollection options. @@ -116,6 +122,10 @@ func (dryRunAll) ApplyToPatch(opts *PatchOptions) { opts.DryRun = []string{metav1.DryRunAll} } +func (drun dryRunAll) ApplyToApply(opts *ApplyOptions) { + opts.DryRun = []string{metav1.DryRunAll} +} + // ApplyToPatch applies this configuration to the given delete options. func (dryRunAll) ApplyToDelete(opts *DeleteOptions) { opts.DryRun = []string{metav1.DryRunAll} @@ -155,6 +165,11 @@ func (f FieldOwner) ApplyToUpdate(opts *UpdateOptions) { opts.FieldManager = string(f) } +// ApplyToApply applies this configuration to the given apply options. +func (f FieldOwner) ApplyToApply(opts *ApplyOptions) { + opts.FieldManager = string(f) +} + // ApplyToSubResourcePatch applies this configuration to the given patch options. func (f FieldOwner) ApplyToSubResourcePatch(opts *SubResourcePatchOptions) { opts.FieldManager = string(f) @@ -872,10 +887,18 @@ func (o *PatchOptions) AsPatchOptions() *metav1.PatchOptions { o.Raw = &metav1.PatchOptions{} } - o.Raw.DryRun = o.DryRun - o.Raw.Force = o.Force - o.Raw.FieldManager = o.FieldManager - o.Raw.FieldValidation = o.FieldValidation + if o.DryRun != nil { + o.Raw.DryRun = o.DryRun + } + if o.Force != nil { + o.Raw.Force = o.Force + } + if o.FieldManager != "" { + o.Raw.FieldManager = o.FieldManager + } + if o.FieldValidation != "" { + o.Raw.FieldValidation = o.FieldValidation + } return o.Raw } @@ -948,3 +971,57 @@ func (o *DeleteAllOfOptions) ApplyToDeleteAllOf(do *DeleteAllOfOptions) { } // }}} + +// ApplyOptions are the options for an apply request. +type ApplyOptions struct { + // When present, indicates that modifications should not be + // persisted. An invalid or unrecognized dryRun directive will + // result in an error response and no further processing of the + // request. Valid values are: + // - All: all dry run stages will be processed + // +optional + // +listType=atomic + DryRun []string `json:"dryRun,omitempty" protobuf:"bytes,1,rep,name=dryRun"` + + // Force is going to "force" Apply requests. It means user will + // re-acquire conflicting fields owned by other people. + Force *bool `json:"force" protobuf:"varint,2,opt,name=force"` + + // fieldManager is a name associated with the actor or entity + // that is making these changes. The value must be less than or + // 128 characters long, and only contain printable characters, + // as defined by https://golang.org/pkg/unicode/#IsPrint. This + // field is required. + FieldManager string `json:"fieldManager" protobuf:"bytes,3,name=fieldManager"` +} + +// ApplyOptions applies the given opts onto the ApplyOptions +func (o *ApplyOptions) ApplyOptions(opts []ApplyOption) *ApplyOptions { + for _, opt := range opts { + opt.ApplyToApply(o) + } + return o +} + +// ApplyToApply applies the given opts onto the ApplyOptions +func (o *ApplyOptions) ApplyToApply(opts *ApplyOptions) { + if o.DryRun != nil { + opts.DryRun = o.DryRun + } + if o.Force != nil { + opts.Force = o.Force + } + + if o.FieldManager != "" { + opts.FieldManager = o.FieldManager + } +} + +// AsPatchOptions constructs patch options from the given ApplyOptions +func (o *ApplyOptions) AsPatchOptions() *metav1.PatchOptions { + return &metav1.PatchOptions{ + DryRun: o.DryRun, + Force: o.Force, + FieldManager: o.FieldManager, + } +} diff --git a/pkg/client/patch.go b/pkg/client/patch.go index 11d6083885..95cb331e7d 100644 --- a/pkg/client/patch.go +++ b/pkg/client/patch.go @@ -27,6 +27,7 @@ import ( var ( // Apply uses server-side apply to patch the given object. + // Deprecated: Use client.Apply instead. Apply Patch = applyPatch{} // Merge uses the raw object as a merge patch, without modifications. diff --git a/pkg/client/typed_client.go b/pkg/client/typed_client.go index 92afd9a9c2..3bd762a638 100644 --- a/pkg/client/typed_client.go +++ b/pkg/client/typed_client.go @@ -18,8 +18,10 @@ package client import ( "context" + "fmt" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/util/apply" ) var _ Reader = &typedClient{} @@ -41,7 +43,7 @@ func (c *typedClient) Create(ctx context.Context, obj Object, opts ...CreateOpti createOpts.ApplyOptions(opts) return o.Post(). - NamespaceIfScoped(o.GetNamespace(), o.isNamespaced()). + NamespaceIfScoped(o.namespace, o.isNamespaced()). Resource(o.resource()). Body(obj). VersionedParams(createOpts.AsCreateOptions(), c.paramCodec). @@ -60,9 +62,9 @@ func (c *typedClient) Update(ctx context.Context, obj Object, opts ...UpdateOpti updateOpts.ApplyOptions(opts) return o.Put(). - NamespaceIfScoped(o.GetNamespace(), o.isNamespaced()). + NamespaceIfScoped(o.namespace, o.isNamespaced()). Resource(o.resource()). - Name(o.GetName()). + Name(o.name). Body(obj). VersionedParams(updateOpts.AsUpdateOptions(), c.paramCodec). Do(ctx). @@ -80,9 +82,9 @@ func (c *typedClient) Delete(ctx context.Context, obj Object, opts ...DeleteOpti deleteOpts.ApplyOptions(opts) return o.Delete(). - NamespaceIfScoped(o.GetNamespace(), o.isNamespaced()). + NamespaceIfScoped(o.namespace, o.isNamespaced()). Resource(o.resource()). - Name(o.GetName()). + Name(o.name). Body(deleteOpts.AsDeleteOptions()). Do(ctx). Error() @@ -123,15 +125,40 @@ func (c *typedClient) Patch(ctx context.Context, obj Object, patch Patch, opts . patchOpts.ApplyOptions(opts) return o.Patch(patch.Type()). - NamespaceIfScoped(o.GetNamespace(), o.isNamespaced()). + NamespaceIfScoped(o.namespace, o.isNamespaced()). Resource(o.resource()). - Name(o.GetName()). + Name(o.name). VersionedParams(patchOpts.AsPatchOptions(), c.paramCodec). Body(data). Do(ctx). Into(obj) } +func (c *typedClient) Apply(ctx context.Context, obj runtime.ApplyConfiguration, opts ...ApplyOption) error { + o, err := c.resources.getObjMeta(obj) + if err != nil { + return err + } + req, err := apply.NewRequest(o, obj) + if err != nil { + return fmt.Errorf("failed to create apply request: %w", err) + } + applyOpts := &ApplyOptions{} + applyOpts.ApplyOptions(opts) + + return req. + NamespaceIfScoped(o.namespace, o.isNamespaced()). + Resource(o.resource()). + Name(o.name). + VersionedParams(applyOpts.AsPatchOptions(), c.paramCodec). + Do(ctx). + // This is hacky, it is required because `Into` takes a `runtime.Object` and + // that is not implemented by the ApplyConfigurations. The generated clients + // don't have this problem because they deserialize into the api type, not the + // apply configuration: https://github.com/kubernetes/kubernetes/blob/22f5e01a37c0bc6a5f494dec14dd4e3688ee1d55/staging/src/k8s.io/client-go/gentype/type.go#L296-L317 + Into(runtimeObjectFromApplyConfiguration(obj)) +} + // Get implements client.Client. func (c *typedClient) Get(ctx context.Context, key ObjectKey, obj Object, opts ...GetOption) error { r, err := c.resources.getResource(obj) @@ -179,9 +206,9 @@ func (c *typedClient) GetSubResource(ctx context.Context, obj, subResourceObj Ob getOpts.ApplyOptions(opts) return o.Get(). - NamespaceIfScoped(o.GetNamespace(), o.isNamespaced()). + NamespaceIfScoped(o.namespace, o.isNamespaced()). Resource(o.resource()). - Name(o.GetName()). + Name(o.name). SubResource(subResource). VersionedParams(getOpts.AsGetOptions(), c.paramCodec). Do(ctx). @@ -202,9 +229,9 @@ func (c *typedClient) CreateSubResource(ctx context.Context, obj Object, subReso createOpts.ApplyOptions(opts) return o.Post(). - NamespaceIfScoped(o.GetNamespace(), o.isNamespaced()). + NamespaceIfScoped(o.namespace, o.isNamespaced()). Resource(o.resource()). - Name(o.GetName()). + Name(o.name). SubResource(subResource). Body(subResourceObj). VersionedParams(createOpts.AsCreateOptions(), c.paramCodec). @@ -237,9 +264,9 @@ func (c *typedClient) UpdateSubResource(ctx context.Context, obj Object, subReso } return o.Put(). - NamespaceIfScoped(o.GetNamespace(), o.isNamespaced()). + NamespaceIfScoped(o.namespace, o.isNamespaced()). Resource(o.resource()). - Name(o.GetName()). + Name(o.name). SubResource(subResource). Body(body). VersionedParams(updateOpts.AsUpdateOptions(), c.paramCodec). @@ -268,9 +295,9 @@ func (c *typedClient) PatchSubResource(ctx context.Context, obj Object, subResou } return o.Patch(patch.Type()). - NamespaceIfScoped(o.GetNamespace(), o.isNamespaced()). + NamespaceIfScoped(o.namespace, o.isNamespaced()). Resource(o.resource()). - Name(o.GetName()). + Name(o.name). SubResource(subResource). Body(data). VersionedParams(patchOpts.AsPatchOptions(), c.paramCodec). diff --git a/pkg/client/unstructured_client.go b/pkg/client/unstructured_client.go index 0d96951780..e636c3beef 100644 --- a/pkg/client/unstructured_client.go +++ b/pkg/client/unstructured_client.go @@ -22,6 +22,7 @@ import ( "strings" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/util/apply" ) var _ Reader = &unstructuredClient{} @@ -50,7 +51,7 @@ func (uc *unstructuredClient) Create(ctx context.Context, obj Object, opts ...Cr createOpts.ApplyOptions(opts) result := o.Post(). - NamespaceIfScoped(o.GetNamespace(), o.isNamespaced()). + NamespaceIfScoped(o.namespace, o.isNamespaced()). Resource(o.resource()). Body(obj). VersionedParams(createOpts.AsCreateOptions(), uc.paramCodec). @@ -79,9 +80,9 @@ func (uc *unstructuredClient) Update(ctx context.Context, obj Object, opts ...Up updateOpts.ApplyOptions(opts) result := o.Put(). - NamespaceIfScoped(o.GetNamespace(), o.isNamespaced()). + NamespaceIfScoped(o.namespace, o.isNamespaced()). Resource(o.resource()). - Name(o.GetName()). + Name(o.name). Body(obj). VersionedParams(updateOpts.AsUpdateOptions(), uc.paramCodec). Do(ctx). @@ -106,9 +107,9 @@ func (uc *unstructuredClient) Delete(ctx context.Context, obj Object, opts ...De deleteOpts.ApplyOptions(opts) return o.Delete(). - NamespaceIfScoped(o.GetNamespace(), o.isNamespaced()). + NamespaceIfScoped(o.namespace, o.isNamespaced()). Resource(o.resource()). - Name(o.GetName()). + Name(o.name). Body(deleteOpts.AsDeleteOptions()). Do(ctx). Error() @@ -157,15 +158,41 @@ func (uc *unstructuredClient) Patch(ctx context.Context, obj Object, patch Patch patchOpts.ApplyOptions(opts) return o.Patch(patch.Type()). - NamespaceIfScoped(o.GetNamespace(), o.isNamespaced()). + NamespaceIfScoped(o.namespace, o.isNamespaced()). Resource(o.resource()). - Name(o.GetName()). + Name(o.name). VersionedParams(patchOpts.AsPatchOptions(), uc.paramCodec). Body(data). Do(ctx). Into(obj) } +func (uc *unstructuredClient) Apply(ctx context.Context, obj runtime.ApplyConfiguration, opts ...ApplyOption) error { + unstructuredApplyConfig, ok := obj.(*unstructuredApplyConfiguration) + if !ok { + return fmt.Errorf("bug: unstructured client got an applyconfiguration that was not %T but %T", &unstructuredApplyConfiguration{}, obj) + } + o, err := uc.resources.getObjMeta(unstructuredApplyConfig.Unstructured) + if err != nil { + return err + } + + req, err := apply.NewRequest(o, obj) + if err != nil { + return fmt.Errorf("failed to create apply request: %w", err) + } + applyOpts := &ApplyOptions{} + applyOpts.ApplyOptions(opts) + + return req. + NamespaceIfScoped(o.namespace, o.isNamespaced()). + Resource(o.resource()). + Name(o.name). + VersionedParams(applyOpts.AsPatchOptions(), uc.paramCodec). + Do(ctx). + Into(unstructuredApplyConfig.Unstructured) +} + // Get implements client.Client. func (uc *unstructuredClient) Get(ctx context.Context, key ObjectKey, obj Object, opts ...GetOption) error { u, ok := obj.(runtime.Unstructured) @@ -244,9 +271,9 @@ func (uc *unstructuredClient) GetSubResource(ctx context.Context, obj, subResour getOpts.ApplyOptions(opts) return o.Get(). - NamespaceIfScoped(o.GetNamespace(), o.isNamespaced()). + NamespaceIfScoped(o.namespace, o.isNamespaced()). Resource(o.resource()). - Name(o.GetName()). + Name(o.name). SubResource(subResource). VersionedParams(getOpts.AsGetOptions(), uc.paramCodec). Do(ctx). @@ -275,9 +302,9 @@ func (uc *unstructuredClient) CreateSubResource(ctx context.Context, obj, subRes createOpts.ApplyOptions(opts) return o.Post(). - NamespaceIfScoped(o.GetNamespace(), o.isNamespaced()). + NamespaceIfScoped(o.namespace, o.isNamespaced()). Resource(o.resource()). - Name(o.GetName()). + Name(o.name). SubResource(subResource). Body(subResourceObj). VersionedParams(createOpts.AsCreateOptions(), uc.paramCodec). @@ -310,9 +337,9 @@ func (uc *unstructuredClient) UpdateSubResource(ctx context.Context, obj Object, } return o.Put(). - NamespaceIfScoped(o.GetNamespace(), o.isNamespaced()). + NamespaceIfScoped(o.namespace, o.isNamespaced()). Resource(o.resource()). - Name(o.GetName()). + Name(o.name). SubResource(subResource). Body(body). VersionedParams(updateOpts.AsUpdateOptions(), uc.paramCodec). @@ -347,9 +374,9 @@ func (uc *unstructuredClient) PatchSubResource(ctx context.Context, obj Object, } result := o.Patch(patch.Type()). - NamespaceIfScoped(o.GetNamespace(), o.isNamespaced()). + NamespaceIfScoped(o.namespace, o.isNamespaced()). Resource(o.resource()). - Name(o.GetName()). + Name(o.name). SubResource(subResource). Body(data). VersionedParams(patchOpts.AsPatchOptions(), uc.paramCodec). From ad1966a958635dc7f658e6904b676088a0946701 Mon Sep 17 00:00:00 2001 From: Alvaro Aleman Date: Sun, 6 Jul 2025 14:40:41 +0200 Subject: [PATCH 2/4] Field validation --- pkg/client/client_test.go | 56 ++++++++++++++++++++++++++++++ pkg/client/fieldvalidation.go | 3 +- pkg/client/fieldvalidation_test.go | 28 +++++++++++++-- pkg/client/options.go | 33 ++++++++++++++++-- 4 files changed, 113 insertions(+), 7 deletions(-) diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index d7748f0fb6..2b704ab742 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -949,6 +949,62 @@ U5wwSivyi7vmegHKmblOzNVKA5qPO8zWzqBC Expect(cm.Data).To(BeComparableTo(obj.Data)) }) }) + + Context("with FieldValidation", func() { + It("should handle invalid fields based on FieldValidation setting", func() { + cl, err := client.New(cfg, client.Options{}) + Expect(err).NotTo(HaveOccurred()) + Expect(cl).NotTo(BeNil()) + + data := map[string]any{ + "some-key": "some-value", + } + obj := &unstructured.Unstructured{Object: map[string]any{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]any{ + "name": "test-configmap-fieldvalidation", + "namespace": "default", + }, + "data": data, + "invalidField": "invalid-value", + }} + + // Apply with disabled field validation should succeed + err = cl.Apply(context.Background(), client.ApplyConfigurationFromUnstructured(obj), &client.ApplyOptions{ + FieldManager: "test-manager", + FieldValidation: metav1.FieldValidationIgnore, + }) + Expect(err).NotTo(HaveOccurred()) + + cm, err := clientset.CoreV1().ConfigMaps(obj.GetNamespace()).Get(context.Background(), obj.GetName(), metav1.GetOptions{}) + Expect(err).NotTo(HaveOccurred()) + + actualData := map[string]any{} + for k, v := range cm.Data { + actualData[k] = v + } + Expect(actualData).To(BeComparableTo(data)) + + // Apply with default (strict) field validation should fail + obj2 := &unstructured.Unstructured{Object: map[string]any{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]any{ + "name": "test-configmap-fieldvalidation-strict", + "namespace": "default", + }, + "data": data, + "invalidField": "invalid-value", + }} + + err = cl.Apply(context.Background(), client.ApplyConfigurationFromUnstructured(obj2), &client.ApplyOptions{ + FieldManager: "test-manager", + }) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("unknown field")) + }) + }) }) Describe("SubResourceClient", func() { diff --git a/pkg/client/fieldvalidation.go b/pkg/client/fieldvalidation.go index fe5a7c8911..1c4d61eecb 100644 --- a/pkg/client/fieldvalidation.go +++ b/pkg/client/fieldvalidation.go @@ -18,7 +18,6 @@ package client import ( "context" - "errors" "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/runtime" @@ -55,7 +54,7 @@ func (c *clientWithFieldValidation) Patch(ctx context.Context, obj Object, patch } func (c *clientWithFieldValidation) Apply(ctx context.Context, obj runtime.ApplyConfiguration, opts ...ApplyOption) error { - return errors.New("Apply is not supported with field validation") + return c.client.Apply(ctx, obj, append([]ApplyOption{c.validation}, opts...)...) } func (c *clientWithFieldValidation) Delete(ctx context.Context, obj Object, opts ...DeleteOption) error { diff --git a/pkg/client/fieldvalidation_test.go b/pkg/client/fieldvalidation_test.go index 4d06e5d96f..6839bc7788 100644 --- a/pkg/client/fieldvalidation_test.go +++ b/pkg/client/fieldvalidation_test.go @@ -25,6 +25,7 @@ import ( corev1 "k8s.io/api/core/v1" 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/controller-runtime/pkg/client/fake" @@ -105,6 +106,7 @@ func TestWithStrictFieldValidation(t *testing.T) { _ = wrappedClient.Create(ctx, dummyObj) _ = wrappedClient.Update(ctx, dummyObj) _ = wrappedClient.Patch(ctx, dummyObj, nil) + _ = wrappedClient.Apply(ctx, nil) _ = wrappedClient.Status().Create(ctx, dummyObj, dummyObj) _ = wrappedClient.Status().Update(ctx, dummyObj) _ = wrappedClient.Status().Patch(ctx, dummyObj, nil) @@ -112,7 +114,7 @@ func TestWithStrictFieldValidation(t *testing.T) { _ = wrappedClient.SubResource("some-subresource").Update(ctx, dummyObj) _ = wrappedClient.SubResource("some-subresource").Patch(ctx, dummyObj, nil) - if expectedCalls := 9; calls != expectedCalls { + if expectedCalls := 10; calls != expectedCalls { t.Fatalf("wrong number of calls to assertions: expected=%d; got=%d", expectedCalls, calls) } } @@ -129,6 +131,7 @@ func TestWithStrictFieldValidationOverridden(t *testing.T) { _ = wrappedClient.Create(ctx, dummyObj, client.FieldValidation(metav1.FieldValidationWarn)) _ = wrappedClient.Update(ctx, dummyObj, client.FieldValidation(metav1.FieldValidationWarn)) _ = wrappedClient.Patch(ctx, dummyObj, nil, client.FieldValidation(metav1.FieldValidationWarn)) + _ = wrappedClient.Apply(ctx, nil, client.FieldValidation(metav1.FieldValidationWarn)) _ = wrappedClient.Status().Create(ctx, dummyObj, dummyObj, client.FieldValidation(metav1.FieldValidationWarn)) _ = wrappedClient.Status().Update(ctx, dummyObj, client.FieldValidation(metav1.FieldValidationWarn)) _ = wrappedClient.Status().Patch(ctx, dummyObj, nil, client.FieldValidation(metav1.FieldValidationWarn)) @@ -136,7 +139,7 @@ func TestWithStrictFieldValidationOverridden(t *testing.T) { _ = wrappedClient.SubResource("some-subresource").Update(ctx, dummyObj, client.FieldValidation(metav1.FieldValidationWarn)) _ = wrappedClient.SubResource("some-subresource").Patch(ctx, dummyObj, nil, client.FieldValidation(metav1.FieldValidationWarn)) - if expectedCalls := 9; calls != expectedCalls { + if expectedCalls := 10; calls != expectedCalls { t.Fatalf("wrong number of calls to assertions: expected=%d; got=%d", expectedCalls, calls) } } @@ -272,5 +275,26 @@ func testFieldValidationClient(t *testing.T, expectedFieldValidation string, cal } return nil }, + Apply: func(ctx context.Context, c client.WithWatch, obj runtime.ApplyConfiguration, opts ...client.ApplyOption) error { + callback() + out := &client.ApplyOptions{} + for _, f := range opts { + f.ApplyToApply(out) + } + if got := out.FieldValidation; expectedFieldValidation != got { + t.Fatalf("wrong field validation: expected=%q; got=%q", expectedFieldValidation, got) + } + + if got := out.AsPatchOptions().FieldValidation; expectedFieldValidation != got { + t.Fatalf("wrong field validation: expected=%q; got=%q", expectedFieldValidation, got) + } + + co := &client.ApplyOptions{} + out.ApplyToApply(co) + if got := co.FieldValidation; expectedFieldValidation != got { + t.Fatalf("wrong field validation: expected=%q; got=%q", expectedFieldValidation, got) + } + return nil + }, }).Build() } diff --git a/pkg/client/options.go b/pkg/client/options.go index d9f321febe..47695249fc 100644 --- a/pkg/client/options.go +++ b/pkg/client/options.go @@ -218,6 +218,11 @@ func (f FieldValidation) ApplyToSubResourceUpdate(opts *SubResourceUpdateOptions opts.FieldValidation = string(f) } +// ApplyToApply applies this configuration to the given apply options. +func (f FieldValidation) ApplyToApply(opts *ApplyOptions) { + opts.FieldValidation = string(f) +} + // }}} // {{{ Create Options @@ -993,6 +998,24 @@ type ApplyOptions struct { // as defined by https://golang.org/pkg/unicode/#IsPrint. This // field is required. FieldManager string `json:"fieldManager" protobuf:"bytes,3,name=fieldManager"` + + // fieldValidation instructs the server on how to handle + // objects in the apply request containing unknown + // or duplicate fields. Valid values are: + // - Ignore: This will ignore any unknown fields that are silently + // dropped from the object, and will ignore all but the last duplicate + // field that the decoder encounters. + // - Warn: This will send a warning via the standard warning response + // header for each unknown field that is dropped from the object, and + // for each duplicate field that is encountered. The request will + // still succeed if there are no other errors, and will only persist + // the last of any duplicate fields. + // - Strict: This will fail the request with a BadRequest error if + // any unknown fields would be dropped from the object, or if any + // duplicate fields are present. The error returned from the server + // will contain all unknown and duplicate fields encountered. + // This is the default for apply requests. + FieldValidation string `json:"fieldValidation,omitempty" protobuf:"bytes,4,opt,name=fieldValidation"` } // ApplyOptions applies the given opts onto the ApplyOptions @@ -1015,13 +1038,17 @@ func (o *ApplyOptions) ApplyToApply(opts *ApplyOptions) { if o.FieldManager != "" { opts.FieldManager = o.FieldManager } + if o.FieldValidation != "" { + opts.FieldValidation = o.FieldValidation + } } // AsPatchOptions constructs patch options from the given ApplyOptions func (o *ApplyOptions) AsPatchOptions() *metav1.PatchOptions { return &metav1.PatchOptions{ - DryRun: o.DryRun, - Force: o.Force, - FieldManager: o.FieldManager, + DryRun: o.DryRun, + Force: o.Force, + FieldManager: o.FieldManager, + FieldValidation: o.FieldValidation, } } From 892382c641350ab2ac6a0ee70c24b7539475bbf7 Mon Sep 17 00:00:00 2001 From: Alvaro Aleman Date: Sun, 6 Jul 2025 14:40:45 +0200 Subject: [PATCH 3/4] Revert "Field validation" This reverts commit ad1966a958635dc7f658e6904b676088a0946701. This doesn't work, the server will always error on additional fields when using SSA, even when fieldValiation=None is configured. --- pkg/client/client_test.go | 56 ------------------------------ pkg/client/fieldvalidation.go | 3 +- pkg/client/fieldvalidation_test.go | 28 ++------------- pkg/client/options.go | 33 ++---------------- 4 files changed, 7 insertions(+), 113 deletions(-) diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index 2b704ab742..d7748f0fb6 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -949,62 +949,6 @@ U5wwSivyi7vmegHKmblOzNVKA5qPO8zWzqBC Expect(cm.Data).To(BeComparableTo(obj.Data)) }) }) - - Context("with FieldValidation", func() { - It("should handle invalid fields based on FieldValidation setting", func() { - cl, err := client.New(cfg, client.Options{}) - Expect(err).NotTo(HaveOccurred()) - Expect(cl).NotTo(BeNil()) - - data := map[string]any{ - "some-key": "some-value", - } - obj := &unstructured.Unstructured{Object: map[string]any{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]any{ - "name": "test-configmap-fieldvalidation", - "namespace": "default", - }, - "data": data, - "invalidField": "invalid-value", - }} - - // Apply with disabled field validation should succeed - err = cl.Apply(context.Background(), client.ApplyConfigurationFromUnstructured(obj), &client.ApplyOptions{ - FieldManager: "test-manager", - FieldValidation: metav1.FieldValidationIgnore, - }) - Expect(err).NotTo(HaveOccurred()) - - cm, err := clientset.CoreV1().ConfigMaps(obj.GetNamespace()).Get(context.Background(), obj.GetName(), metav1.GetOptions{}) - Expect(err).NotTo(HaveOccurred()) - - actualData := map[string]any{} - for k, v := range cm.Data { - actualData[k] = v - } - Expect(actualData).To(BeComparableTo(data)) - - // Apply with default (strict) field validation should fail - obj2 := &unstructured.Unstructured{Object: map[string]any{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]any{ - "name": "test-configmap-fieldvalidation-strict", - "namespace": "default", - }, - "data": data, - "invalidField": "invalid-value", - }} - - err = cl.Apply(context.Background(), client.ApplyConfigurationFromUnstructured(obj2), &client.ApplyOptions{ - FieldManager: "test-manager", - }) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("unknown field")) - }) - }) }) Describe("SubResourceClient", func() { diff --git a/pkg/client/fieldvalidation.go b/pkg/client/fieldvalidation.go index 1c4d61eecb..fe5a7c8911 100644 --- a/pkg/client/fieldvalidation.go +++ b/pkg/client/fieldvalidation.go @@ -18,6 +18,7 @@ package client import ( "context" + "errors" "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/runtime" @@ -54,7 +55,7 @@ func (c *clientWithFieldValidation) Patch(ctx context.Context, obj Object, patch } func (c *clientWithFieldValidation) Apply(ctx context.Context, obj runtime.ApplyConfiguration, opts ...ApplyOption) error { - return c.client.Apply(ctx, obj, append([]ApplyOption{c.validation}, opts...)...) + return errors.New("Apply is not supported with field validation") } func (c *clientWithFieldValidation) Delete(ctx context.Context, obj Object, opts ...DeleteOption) error { diff --git a/pkg/client/fieldvalidation_test.go b/pkg/client/fieldvalidation_test.go index 6839bc7788..4d06e5d96f 100644 --- a/pkg/client/fieldvalidation_test.go +++ b/pkg/client/fieldvalidation_test.go @@ -25,7 +25,6 @@ import ( corev1 "k8s.io/api/core/v1" 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/controller-runtime/pkg/client/fake" @@ -106,7 +105,6 @@ func TestWithStrictFieldValidation(t *testing.T) { _ = wrappedClient.Create(ctx, dummyObj) _ = wrappedClient.Update(ctx, dummyObj) _ = wrappedClient.Patch(ctx, dummyObj, nil) - _ = wrappedClient.Apply(ctx, nil) _ = wrappedClient.Status().Create(ctx, dummyObj, dummyObj) _ = wrappedClient.Status().Update(ctx, dummyObj) _ = wrappedClient.Status().Patch(ctx, dummyObj, nil) @@ -114,7 +112,7 @@ func TestWithStrictFieldValidation(t *testing.T) { _ = wrappedClient.SubResource("some-subresource").Update(ctx, dummyObj) _ = wrappedClient.SubResource("some-subresource").Patch(ctx, dummyObj, nil) - if expectedCalls := 10; calls != expectedCalls { + if expectedCalls := 9; calls != expectedCalls { t.Fatalf("wrong number of calls to assertions: expected=%d; got=%d", expectedCalls, calls) } } @@ -131,7 +129,6 @@ func TestWithStrictFieldValidationOverridden(t *testing.T) { _ = wrappedClient.Create(ctx, dummyObj, client.FieldValidation(metav1.FieldValidationWarn)) _ = wrappedClient.Update(ctx, dummyObj, client.FieldValidation(metav1.FieldValidationWarn)) _ = wrappedClient.Patch(ctx, dummyObj, nil, client.FieldValidation(metav1.FieldValidationWarn)) - _ = wrappedClient.Apply(ctx, nil, client.FieldValidation(metav1.FieldValidationWarn)) _ = wrappedClient.Status().Create(ctx, dummyObj, dummyObj, client.FieldValidation(metav1.FieldValidationWarn)) _ = wrappedClient.Status().Update(ctx, dummyObj, client.FieldValidation(metav1.FieldValidationWarn)) _ = wrappedClient.Status().Patch(ctx, dummyObj, nil, client.FieldValidation(metav1.FieldValidationWarn)) @@ -139,7 +136,7 @@ func TestWithStrictFieldValidationOverridden(t *testing.T) { _ = wrappedClient.SubResource("some-subresource").Update(ctx, dummyObj, client.FieldValidation(metav1.FieldValidationWarn)) _ = wrappedClient.SubResource("some-subresource").Patch(ctx, dummyObj, nil, client.FieldValidation(metav1.FieldValidationWarn)) - if expectedCalls := 10; calls != expectedCalls { + if expectedCalls := 9; calls != expectedCalls { t.Fatalf("wrong number of calls to assertions: expected=%d; got=%d", expectedCalls, calls) } } @@ -275,26 +272,5 @@ func testFieldValidationClient(t *testing.T, expectedFieldValidation string, cal } return nil }, - Apply: func(ctx context.Context, c client.WithWatch, obj runtime.ApplyConfiguration, opts ...client.ApplyOption) error { - callback() - out := &client.ApplyOptions{} - for _, f := range opts { - f.ApplyToApply(out) - } - if got := out.FieldValidation; expectedFieldValidation != got { - t.Fatalf("wrong field validation: expected=%q; got=%q", expectedFieldValidation, got) - } - - if got := out.AsPatchOptions().FieldValidation; expectedFieldValidation != got { - t.Fatalf("wrong field validation: expected=%q; got=%q", expectedFieldValidation, got) - } - - co := &client.ApplyOptions{} - out.ApplyToApply(co) - if got := co.FieldValidation; expectedFieldValidation != got { - t.Fatalf("wrong field validation: expected=%q; got=%q", expectedFieldValidation, got) - } - return nil - }, }).Build() } diff --git a/pkg/client/options.go b/pkg/client/options.go index 47695249fc..d9f321febe 100644 --- a/pkg/client/options.go +++ b/pkg/client/options.go @@ -218,11 +218,6 @@ func (f FieldValidation) ApplyToSubResourceUpdate(opts *SubResourceUpdateOptions opts.FieldValidation = string(f) } -// ApplyToApply applies this configuration to the given apply options. -func (f FieldValidation) ApplyToApply(opts *ApplyOptions) { - opts.FieldValidation = string(f) -} - // }}} // {{{ Create Options @@ -998,24 +993,6 @@ type ApplyOptions struct { // as defined by https://golang.org/pkg/unicode/#IsPrint. This // field is required. FieldManager string `json:"fieldManager" protobuf:"bytes,3,name=fieldManager"` - - // fieldValidation instructs the server on how to handle - // objects in the apply request containing unknown - // or duplicate fields. Valid values are: - // - Ignore: This will ignore any unknown fields that are silently - // dropped from the object, and will ignore all but the last duplicate - // field that the decoder encounters. - // - Warn: This will send a warning via the standard warning response - // header for each unknown field that is dropped from the object, and - // for each duplicate field that is encountered. The request will - // still succeed if there are no other errors, and will only persist - // the last of any duplicate fields. - // - Strict: This will fail the request with a BadRequest error if - // any unknown fields would be dropped from the object, or if any - // duplicate fields are present. The error returned from the server - // will contain all unknown and duplicate fields encountered. - // This is the default for apply requests. - FieldValidation string `json:"fieldValidation,omitempty" protobuf:"bytes,4,opt,name=fieldValidation"` } // ApplyOptions applies the given opts onto the ApplyOptions @@ -1038,17 +1015,13 @@ func (o *ApplyOptions) ApplyToApply(opts *ApplyOptions) { if o.FieldManager != "" { opts.FieldManager = o.FieldManager } - if o.FieldValidation != "" { - opts.FieldValidation = o.FieldValidation - } } // AsPatchOptions constructs patch options from the given ApplyOptions func (o *ApplyOptions) AsPatchOptions() *metav1.PatchOptions { return &metav1.PatchOptions{ - DryRun: o.DryRun, - Force: o.Force, - FieldManager: o.FieldManager, - FieldValidation: o.FieldValidation, + DryRun: o.DryRun, + Force: o.Force, + FieldManager: o.FieldManager, } } From f92f5875dd60f37b1b301625bf228ebbe9e5ac9c Mon Sep 17 00:00:00 2001 From: Alvaro Aleman Date: Sun, 6 Jul 2025 15:21:54 +0200 Subject: [PATCH 4/4] Explain why we don't update the namespaced client --- pkg/client/namespaced_client.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/client/namespaced_client.go b/pkg/client/namespaced_client.go index a8a60c31f7..d1129c7f3b 100644 --- a/pkg/client/namespaced_client.go +++ b/pkg/client/namespaced_client.go @@ -149,6 +149,10 @@ func (n *namespacedClient) Patch(ctx context.Context, obj Object, patch Patch, o } func (n *namespacedClient) Apply(ctx context.Context, obj runtime.ApplyConfiguration, opts ...ApplyOption) error { + // It is non-trivial to make this work as the current check if an object is namespaces takes a runtime.Object, + // we would need to update that to be able to deal with a runtime.ApplyConfiguration. Additionally, ACs + // do allow setting the namespace through a `WithNamespace(string) T` method, but we would have to use reflect + // due to the `T` return. return errors.New("Apply is not supported on namespaced client") }