Merge pull request kubernetes-sigs#1409 from alvaroaleman/allow-new-client

⚠️ Allow setting NewClientFunc w/o implementing an interface

Author: k8s-ci-robot

pkg/cluster/client_builder.go

@@ -109,10 +109,10 @@ type Options struct {
 	// by the manager. If not set this will use the default new cache function.
 	NewCache cache.NewCacheFunc
 
-	// ClientBuilder is the builder that creates the client to be used by the manager.
+	// NewClient is the func that creates the client to be used by the manager.
 	// If not set this will create the default DelegatingClient that will
 	// use the cache for reads and the client for writes.
-	ClientBuilder ClientBuilder
+	NewClient NewClientFunc
 
 	// ClientDisableCacheFor tells the client that, if any cache is used, to bypass it
 	// for the given objects.
@@ -174,9 +174,7 @@ func New(config *rest.Config, opts ...Option) (Cluster, error) {
 		return nil, err
 	}
 
-	writeObj, err := options.ClientBuilder.
-		WithUncached(options.ClientDisableCacheFor...).
-		Build(cache, config, clientOptions)
+	writeObj, err := options.NewClient(cache, config, clientOptions, options.ClientDisableCacheFor...)
 	if err != nil {
 		return nil, err
 	}
@@ -219,9 +217,9 @@ func setOptionsDefaults(options Options) Options {
 		}
 	}
 
-	// Allow the client builder to be mocked
-	if options.ClientBuilder == nil {
-		options.ClientBuilder = NewClientBuilder()
+	// Allow users to define how to create a new client
+	if options.NewClient == nil {
+		options.NewClient = DefaultNewClient
 	}
 
 	// Allow newCache to be mocked
@@ -253,3 +251,20 @@ func setOptionsDefaults(options Options) Options {
 
 	return options
 }
+
+// NewClientFunc allows a user to define how to create a client
+type NewClientFunc func(cache cache.Cache, config *rest.Config, options client.Options, uncachedObjects ...client.Object) (client.Client, error)
+
+// DefaultNewClient creates the default caching client
+func DefaultNewClient(cache cache.Cache, config *rest.Config, options client.Options, uncachedObjects ...client.Object) (client.Client, error) {
+	c, err := client.New(config, options)
+	if err != nil {
+		return nil, err
+	}
+
+	return client.NewDelegatingClient(client.NewDelegatingClientInput{
+		CacheReader:     cache,
+		Client:          c,
+		UncachedObjects: uncachedObjects,
+	})
+}

pkg/cluster/cluster.go

@@ -18,6 +18,7 @@ package cluster
 
 import (
 	"context"
+	"errors"
 	"fmt"
 
 	"github.com/go-logr/logr"
@@ -35,18 +36,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/runtime/inject"
 )
 
-type fakeClientBuilder struct {
-	err error
-}
-
-func (e *fakeClientBuilder) WithUncached(objs ...client.Object) ClientBuilder {
-	return e
-}
-
-func (e *fakeClientBuilder) Build(cache cache.Cache, config *rest.Config, options client.Options) (client.Client, error) {
-	return nil, e.err
-}
-
 var _ = Describe("cluster.Cluster", func() {
 	Describe("New", func() {
 		It("should return an error if there is no Config", func() {
@@ -68,7 +57,9 @@ var _ = Describe("cluster.Cluster", func() {
 
 		It("should return an error it can't create a client.Client", func(done Done) {
 			c, err := New(cfg, func(o *Options) {
-				o.ClientBuilder = &fakeClientBuilder{err: fmt.Errorf("expected error")}
+				o.NewClient = func(cache cache.Cache, config *rest.Config, options client.Options, uncachedObjects ...client.Object) (client.Client, error) {
+					return nil, errors.New("expected error")
+				}
 			})
 			Expect(c).To(BeNil())
 			Expect(err).To(HaveOccurred())
@@ -92,7 +83,9 @@ var _ = Describe("cluster.Cluster", func() {
 
 		It("should create a client defined in by the new client function", func(done Done) {
 			c, err := New(cfg, func(o *Options) {
-				o.ClientBuilder = &fakeClientBuilder{}
+				o.NewClient = func(cache cache.Cache, config *rest.Config, options client.Options, uncachedObjects ...client.Object) (client.Client, error) {
+					return nil, nil
+				}
 			})
 			Expect(c).ToNot(BeNil())
 			Expect(err).ToNot(HaveOccurred())

pkg/cluster/cluster_test.go

@@ -226,10 +226,10 @@ type Options struct {
 	// by the manager. If not set this will use the default new cache function.
 	NewCache cache.NewCacheFunc
 
-	// ClientBuilder is the builder that creates the client to be used by the manager.
+	// NewClient is the func that creates the client to be used by the manager.
 	// If not set this will create the default DelegatingClient that will
 	// use the cache for reads and the client for writes.
-	ClientBuilder ClientBuilder
+	NewClient cluster.NewClientFunc
 
 	// ClientDisableCacheFor tells the client that, if any cache is used, to bypass it
 	// for the given objects.
@@ -309,7 +309,7 @@ func New(config *rest.Config, options Options) (Manager, error) {
 		clusterOptions.SyncPeriod = options.SyncPeriod
 		clusterOptions.Namespace = options.Namespace
 		clusterOptions.NewCache = options.NewCache
-		clusterOptions.ClientBuilder = options.ClientBuilder
+		clusterOptions.NewClient = options.NewClient
 		clusterOptions.ClientDisableCacheFor = options.ClientDisableCacheFor
 		clusterOptions.DryRunClient = options.DryRunClient
 		clusterOptions.EventBroadcaster = options.EventBroadcaster

pkg/manager/client_builder.go

@@ -55,18 +55,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/runtime/inject"
 )
 
-type fakeClientBuilder struct {
-	err error
-}
-
-func (e *fakeClientBuilder) WithUncached(objs ...client.Object) ClientBuilder {
-	return e
-}
-
-func (e *fakeClientBuilder) Build(cache cache.Cache, config *rest.Config, options client.Options) (client.Client, error) {
-	return nil, e.err
-}
-
 var _ = Describe("manger.Manager", func() {
 	Describe("New", func() {
 		It("should return an error if there is no Config", func() {
@@ -88,7 +76,9 @@ var _ = Describe("manger.Manager", func() {
 
 		It("should return an error it can't create a client.Client", func(done Done) {
 			m, err := New(cfg, Options{
-				ClientBuilder: &fakeClientBuilder{err: fmt.Errorf("expected error")},
+				NewClient: func(cache cache.Cache, config *rest.Config, options client.Options, uncachedObjects ...client.Object) (client.Client, error) {
+					return nil, errors.New("expected error")
+				},
 			})
 			Expect(m).To(BeNil())
 			Expect(err).To(HaveOccurred())
@@ -112,7 +102,9 @@ var _ = Describe("manger.Manager", func() {
 
 		It("should create a client defined in by the new client function", func(done Done) {
 			m, err := New(cfg, Options{
-				ClientBuilder: &fakeClientBuilder{},
+				NewClient: func(cache cache.Cache, config *rest.Config, options client.Options, uncachedObjects ...client.Object) (client.Client, error) {
+					return nil, nil
+				},
 			})
 			Expect(m).ToNot(BeNil())
 			Expect(err).ToNot(HaveOccurred())