From 9fa2a3c7d0b4b7128beb13e53aed6be8d1135fd5 Mon Sep 17 00:00:00 2001 From: Jefftree Date: Tue, 18 May 2021 14:27:57 -0700 Subject: [PATCH] Feedback --- examples/applyconfig/main.go | 26 +++++++++----------------- pkg/client/options.go | 20 -------------------- pkg/client/patch.go | 32 ++++++++++++++++++++++++++++++++ pkg/client/typed_client.go | 30 +++--------------------------- 4 files changed, 44 insertions(+), 64 deletions(-) diff --git a/examples/applyconfig/main.go b/examples/applyconfig/main.go index 8c58fbddbc..018ca8c7e2 100644 --- a/examples/applyconfig/main.go +++ b/examples/applyconfig/main.go @@ -25,10 +25,6 @@ import ( var sch *runtime.Scheme -func makePtrForString(s string) *string { - return &s -} - func runMain() int { ctrl.SetLogger(zap.New(zap.UseDevMode(true))) @@ -112,27 +108,23 @@ func runMain() int { fooObj := &v1.Foo{} // To use the SSA typed client, use the ApplyConfiguration generated types instead of the types defined in v1 - applyConfig1 := &ac.FooApplyConfiguration{ - TypeMetaApplyConfiguration: metav1ac.TypeMetaApplyConfiguration{Kind: makePtrForString("Foo"), APIVersion: makePtrForString("applytest.kubebuilder.io/v1")}, + applyConfig1 := (&ac.FooApplyConfiguration{ + TypeMetaApplyConfiguration: *metav1ac.TypeMeta().WithKind("Foo").WithAPIVersion("applytest.kubebuilder.io/v1"), ObjectMetaApplyConfiguration: metav1ac.ObjectMeta().WithName("acdefault").WithNamespace("default"), - } - - applyConfig1 = applyConfig1.WithNonNullableField("value1") + }).WithNonNullableField("value1") // Without ApplyConfiguration, NonNullableField is not an optional field so it must be specified if Foo{} was used instead of the ApplyConfiguration - // The ApplyConfiguration uses pointers for all objects, so we can set a nil value to indicate that a field is not of interest in the Apply - applyConfig2 := &ac.FooApplyConfiguration{ - TypeMetaApplyConfiguration: metav1ac.TypeMetaApplyConfiguration{Kind: makePtrForString("Foo"), APIVersion: makePtrForString("applytest.kubebuilder.io/v1")}, + // The ApplyConfiguration uses pointers for all objects, so omitting a field is equivalent to saying that it is not of interest in this Apply + applyConfig2 := (&ac.FooApplyConfiguration{ + TypeMetaApplyConfiguration: *metav1ac.TypeMeta().WithKind("Foo").WithAPIVersion("applytest.kubebuilder.io/v1"), ObjectMetaApplyConfiguration: metav1ac.ObjectMeta().WithName("acdefault").WithNamespace("default"), - NonNullableField: nil, - NullableField: makePtrForString("value2"), - } + }).WithNullableField("value2") - if err := cl.Patch(context.TODO(), fooObj, client.Apply, owner, client.ApplyFrom(applyConfig1)); err != nil { + if err := cl.Patch(context.TODO(), fooObj, client.ApplyFrom(applyConfig1), owner); err != nil { panic(err) } - if err := cl.Patch(context.TODO(), fooObj, client.Apply, owner2, client.ApplyFrom(applyConfig2)); err != nil { + if err := cl.Patch(context.TODO(), fooObj, client.ApplyFrom(applyConfig2), owner2); err != nil { panic(err) } fooObj = &v1.Foo{} diff --git a/pkg/client/options.go b/pkg/client/options.go index aa53e1bab8..f253276466 100644 --- a/pkg/client/options.go +++ b/pkg/client/options.go @@ -607,9 +607,6 @@ type PatchOptions struct { // Raw represents raw PatchOptions, as passed to the API server. Raw *metav1.PatchOptions - - // ApplyConfiguration represents a typed object for use with Server Side Apply - ApplyConfiguration interface{} } // ApplyOptions applies the given patch options on these options, @@ -655,23 +652,6 @@ func (o *PatchOptions) ApplyToPatch(po *PatchOptions) { } } -// ApplyFrom provides the ApplyConfiguration for use with client Patch Apply -// and server side apply. -func ApplyFrom(ac interface{}) *ApplyConfiguration { - return &ApplyConfiguration{ac: ac} -} - -// ApplyConfiguration implements ApplyToPatch and contains the -// applyconfiguration for use with client Patch Apply. -type ApplyConfiguration struct { - ac interface{} -} - -// ApplyToPatch applies this configuration to the given patch options. -func (ac *ApplyConfiguration) ApplyToPatch(ops *PatchOptions) { - ops.ApplyConfiguration = ac.ac -} - // ForceOwnership indicates that in case of conflicts with server-side apply, // the client should acquire ownership of the conflicting field. Most // controllers should use this. diff --git a/pkg/client/patch.go b/pkg/client/patch.go index 10984c5342..7be3fd9f2d 100644 --- a/pkg/client/patch.go +++ b/pkg/client/patch.go @@ -18,6 +18,7 @@ package client import ( "fmt" + "reflect" jsonpatch "github.com/evanphx/json-patch" "k8s.io/apimachinery/pkg/types" @@ -211,3 +212,34 @@ func (p applyPatch) Data(obj Object) ([]byte, error) { // client-go does, more-or-less). return json.Marshal(obj) } + +//applyFromPatch uses server-side-apply with a ApplyConfiguration object. +type applyFromPatch struct { + patch interface{} +} + +// Type implements Patch. +func (p applyFromPatch) Type() types.PatchType { + return types.ApplyPatchType +} + +// Sets the name and namespace for the returned object if an ApplyConfiguration was supplied +func setNameNamespace(obj Object, ac interface{}) { + obj.SetName(reflect.ValueOf(ac).Elem().FieldByName("Name").Elem().String()) + obj.SetNamespace(reflect.ValueOf(ac).Elem().FieldByName("Namespace").Elem().String()) +} + +// Data implements Patch. +func (p applyFromPatch) Data(o Object) ([]byte, error) { + // If an ApplyConfiguration is provided, use that as the data to send + if p.patch == nil { + return nil, fmt.Errorf("ApplyConfiguration must be specified when using ApplyFrom Patch") + } + setNameNamespace(o, p.patch) + return json.Marshal(p.patch) +} + +// ApplyFrom creates an applyFromPatch object with the provided ApplyConfiguration +func ApplyFrom(ac interface{}) Patch { + return applyFromPatch{patch: ac} +} diff --git a/pkg/client/typed_client.go b/pkg/client/typed_client.go index 6462d4e807..a1b32653ca 100644 --- a/pkg/client/typed_client.go +++ b/pkg/client/typed_client.go @@ -18,9 +18,6 @@ package client import ( "context" - "encoding/json" - "fmt" - "reflect" "k8s.io/apimachinery/pkg/runtime" ) @@ -111,45 +108,24 @@ func (c *typedClient) DeleteAllOf(ctx context.Context, obj Object, opts ...Delet Error() } -// Sets the name and namespace for the returned object if an ApplyConfiguration was supplied -func setNameNamespace(obj Object, ac interface{}) { - obj.SetName(reflect.ValueOf(ac).Elem().FieldByName("Name").Elem().String()) - obj.SetNamespace(reflect.ValueOf(ac).Elem().FieldByName("Namespace").Elem().String()) -} - // Patch implements client.Client func (c *typedClient) Patch(ctx context.Context, obj Object, patch Patch, opts ...PatchOption) error { - patchOpts := &PatchOptions{} - patchOpts.ApplyOptions(opts) - - if patchOpts.ApplyConfiguration != nil && patch != Apply { - return fmt.Errorf("ApplyConfiguration may only be specified on Apply patch type") - } - - if patchOpts.ApplyConfiguration != nil { - setNameNamespace(obj, patchOpts.ApplyConfiguration) - } - o, err := c.cache.getObjMeta(obj) if err != nil { return err } - var data []byte - if patchOpts.ApplyConfiguration != nil { - data, err = json.Marshal(patchOpts.ApplyConfiguration) - } else { - data, err = patch.Data(obj) - } + data, err := patch.Data(obj) if err != nil { return err } + patchOpts := &PatchOptions{} return o.Patch(patch.Type()). NamespaceIfScoped(o.GetNamespace(), o.isNamespaced()). Resource(o.resource()). Name(o.GetName()). - VersionedParams(patchOpts.AsPatchOptions(), c.paramCodec). + VersionedParams(patchOpts.ApplyOptions(opts).AsPatchOptions(), c.paramCodec). Body(data). Do(ctx). Into(obj)