Merge pull request #2122 from inteon/configurable_http_client

⚠️ make *http.Client configurable and use/ share the same http.Client in the default configuration

Author: k8s-ci-robot

pkg/cache/cache.go

@@ -19,6 +19,7 @@ package cache
 import (
 	"context"
 	"fmt"
+	"net/http"
 	"reflect"
 	"time"
 
@@ -108,6 +109,9 @@ type SelectorsByObject map[client.Object]ObjectSelector
 
 // Options are the optional arguments for creating a new InformersMap object.
 type Options struct {
+	// HTTPClient is the http client to use for the REST client
+	HTTPClient *http.Client
+
 	// Scheme is the scheme to use for mapping objects to GroupVersionKinds
 	Scheme *runtime.Scheme
 
@@ -184,6 +188,7 @@ func New(config *rest.Config, opts Options) (Cache, error) {
 	return &informerCache{
 		scheme: opts.Scheme,
 		Informers: internal.NewInformers(config, &internal.InformersOpts{
+			HTTPClient:   opts.HTTPClient,
 			Scheme:       opts.Scheme,
 			Mapper:       opts.Mapper,
 			ResyncPeriod: *opts.Resync,
@@ -414,6 +419,18 @@ func combineTransform(inherited, current toolscache.TransformFunc) toolscache.Tr
 }
 
 func defaultOpts(config *rest.Config, opts Options) (Options, error) {
+	logger := log.WithName("setup")
+
+	// Use the rest HTTP client for the provided config if unset
+	if opts.HTTPClient == nil {
+		var err error
+		opts.HTTPClient, err = rest.HTTPClientFor(config)
+		if err != nil {
+			logger.Error(err, "Failed to get HTTP client")
+			return opts, fmt.Errorf("could not create HTTP client from config")
+		}
+	}
+
 	// Use the default Kubernetes Scheme if unset
 	if opts.Scheme == nil {
 		opts.Scheme = scheme.Scheme
@@ -422,9 +439,9 @@ func defaultOpts(config *rest.Config, opts Options) (Options, error) {
 	// Construct a new Mapper if unset
 	if opts.Mapper == nil {
 		var err error
-		opts.Mapper, err = apiutil.NewDiscoveryRESTMapper(config)
+		opts.Mapper, err = apiutil.NewDiscoveryRESTMapper(config, opts.HTTPClient)
 		if err != nil {
-			log.WithName("setup").Error(err, "Failed to get API Group-Resources")
+			logger.Error(err, "Failed to get API Group-Resources")
 			return opts, fmt.Errorf("could not create RESTMapper from config")
 		}
 	}

pkg/cache/informer_cache_test.go

@@ -31,7 +31,9 @@ var _ = Describe("informerCache", func() {
 	It("should not require LeaderElection", func() {
 		cfg := &rest.Config{}
 
-		mapper, err := apiutil.NewDynamicRESTMapper(cfg, apiutil.WithLazyDiscovery)
+		httpClient, err := rest.HTTPClientFor(cfg)
+		Expect(err).ToNot(HaveOccurred())
+		mapper, err := apiutil.NewDynamicRESTMapper(cfg, httpClient, apiutil.WithLazyDiscovery)
 		Expect(err).ToNot(HaveOccurred())
 
 		c, err := cache.New(cfg, cache.Options{Mapper: mapper})

pkg/cache/internal/informers.go

@@ -20,6 +20,7 @@ import (
 	"context"
 	"fmt"
 	"math/rand"
+	"net/http"
 	"sync"
 	"time"
 
@@ -44,6 +45,7 @@ func init() {
 
 // InformersOpts configures an InformerMap.
 type InformersOpts struct {
+	HTTPClient   *http.Client
 	Scheme       *runtime.Scheme
 	Mapper       meta.RESTMapper
 	ResyncPeriod time.Duration
@@ -62,9 +64,10 @@ type InformersOptsByGVK struct {
 // NewInformers creates a new InformersMap that can create informers under the hood.
 func NewInformers(config *rest.Config, options *InformersOpts) *Informers {
 	return &Informers{
-		config: config,
-		scheme: options.Scheme,
-		mapper: options.Mapper,
+		config:     config,
+		httpClient: options.HTTPClient,
+		scheme:     options.Scheme,
+		mapper:     options.Mapper,
 		tracker: tracker{
 			Structured:   make(map[schema.GroupVersionKind]*Cache),
 			Unstructured: make(map[schema.GroupVersionKind]*Cache),
@@ -99,6 +102,9 @@ type tracker struct {
 // Informers create and caches Informers for (runtime.Object, schema.GroupVersionKind) pairs.
 // It uses a standard parameter codec constructed based on the given generated Scheme.
 type Informers struct {
+	// httpClient is used to create a new REST client
+	httpClient *http.Client
+
 	// scheme maps runtime.Objects to GroupVersionKinds
 	scheme *runtime.Scheme
 
@@ -364,7 +370,7 @@ func (ip *Informers) makeListWatcher(gvk schema.GroupVersionKind, obj runtime.Ob
 		// we should remove it and use the one that the dynamic client sets for us.
 		cfg := rest.CopyConfig(ip.config)
 		cfg.NegotiatedSerializer = nil
-		dynamicClient, err := dynamic.NewForConfig(cfg)
+		dynamicClient, err := dynamic.NewForConfigAndClient(cfg, ip.httpClient)
 		if err != nil {
 			return nil, err
 		}
@@ -394,7 +400,7 @@ func (ip *Informers) makeListWatcher(gvk schema.GroupVersionKind, obj runtime.Ob
 		cfg.NegotiatedSerializer = nil
 
 		// Grab the metadata metadataClient.
-		metadataClient, err := metadata.NewForConfig(cfg)
+		metadataClient, err := metadata.NewForConfigAndClient(cfg, ip.httpClient)
 		if err != nil {
 			return nil, err
 		}
@@ -435,7 +441,7 @@ func (ip *Informers) makeListWatcher(gvk schema.GroupVersionKind, obj runtime.Ob
 	// Structured.
 	//
 	default:
-		client, err := apiutil.RESTClientForGVK(gvk, false, ip.config, ip.codecs)
+		client, err := apiutil.RESTClientForGVK(gvk, false, ip.config, ip.codecs, ip.httpClient)
 		if err != nil {
 			return nil, err
 		}

pkg/client/apiutil/apimachinery.go

@@ -22,6 +22,7 @@ package apiutil
 import (
 	"errors"
 	"fmt"
+	"net/http"
 	"reflect"
 	"sync"
 
@@ -61,9 +62,13 @@ func AddToProtobufScheme(addToScheme func(*runtime.Scheme) error) error {
 
 // NewDiscoveryRESTMapper constructs a new RESTMapper based on discovery
 // information fetched by a new client with the given config.
-func NewDiscoveryRESTMapper(c *rest.Config) (meta.RESTMapper, error) {
+func NewDiscoveryRESTMapper(c *rest.Config, httpClient *http.Client) (meta.RESTMapper, error) {
+	if httpClient == nil {
+		panic("httpClient must not be nil")
+	}
+
 	// Get a mapper
-	dc, err := discovery.NewDiscoveryClientForConfig(c)
+	dc, err := discovery.NewDiscoveryClientForConfigAndClient(c, httpClient)
 	if err != nil {
 		return nil, err
 	}
@@ -150,8 +155,11 @@ 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) (rest.Interface, error) {
-	return rest.RESTClientFor(createRestConfig(gvk, isUnstructured, baseConfig, codecs))
+func RESTClientForGVK(gvk schema.GroupVersionKind, isUnstructured bool, baseConfig *rest.Config, codecs serializer.CodecFactory, httpClient *http.Client) (rest.Interface, error) {
+	if httpClient == nil {
+		panic("httpClient must not be nil")
+	}
+	return rest.RESTClientForConfigAndClient(createRestConfig(gvk, isUnstructured, baseConfig, codecs), httpClient)
 }
 
 // createRestConfig copies the base config and updates needed fields for a new rest config.

pkg/client/apiutil/dynamicrestmapper.go

@@ -17,6 +17,7 @@ limitations under the License.
 package apiutil
 
 import (
+	"net/http"
 	"sync"
 	"sync/atomic"
 
@@ -75,8 +76,12 @@ func WithCustomMapper(newMapper func() (meta.RESTMapper, error)) DynamicRESTMapp
 // NewDynamicRESTMapper returns a dynamic RESTMapper for cfg. The dynamic
 // RESTMapper dynamically discovers resource types at runtime. opts
 // configure the RESTMapper.
-func NewDynamicRESTMapper(cfg *rest.Config, opts ...DynamicRESTMapperOption) (meta.RESTMapper, error) {
-	client, err := discovery.NewDiscoveryClientForConfig(cfg)
+func NewDynamicRESTMapper(cfg *rest.Config, httpClient *http.Client, opts ...DynamicRESTMapperOption) (meta.RESTMapper, error) {
+	if httpClient == nil {
+		panic("httpClient must not be nil")
+	}
+
+	client, err := discovery.NewDiscoveryClientForConfigAndClient(cfg, httpClient)
 	if err != nil {
 		return nil, err
 	}

pkg/client/apiutil/dynamicrestmapper_test.go

@@ -27,6 +27,7 @@ import (
 	"golang.org/x/time/rate"
 	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/client-go/rest"
 )
 
 var (
@@ -50,8 +51,10 @@ var _ = Describe("Dynamic REST Mapper", func() {
 			baseMapper.Add(targetGVK, meta.RESTScopeNamespace)
 		}
 
+		httpClient, err := rest.HTTPClientFor(cfg)
+		Expect(err).ToNot(HaveOccurred())
 		lim = rate.NewLimiter(rate.Limit(5), 5)
-		mapper, err = NewDynamicRESTMapper(cfg, WithLimiter(lim), WithCustomMapper(func() (meta.RESTMapper, error) {
+		mapper, err = NewDynamicRESTMapper(cfg, httpClient, WithLimiter(lim), WithCustomMapper(func() (meta.RESTMapper, error) {
 			baseMapper := meta.NewDefaultRESTMapper(nil)
 			addToMapper(baseMapper)
 
@@ -150,7 +153,9 @@ var _ = Describe("Dynamic REST Mapper", func() {
 			var err error
 			var failedOnce bool
 			mockErr := fmt.Errorf("mock failed once")
-			mapper, err = NewDynamicRESTMapper(cfg, WithLazyDiscovery, WithCustomMapper(func() (meta.RESTMapper, error) {
+			httpClient, err := rest.HTTPClientFor(cfg)
+			Expect(err).ToNot(HaveOccurred())
+			mapper, err = NewDynamicRESTMapper(cfg, httpClient, WithLazyDiscovery, WithCustomMapper(func() (meta.RESTMapper, error) {
 				// Make newMapper fail once
 				if !failedOnce {
 					failedOnce = true

pkg/client/client.go

@@ -20,6 +20,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"net/http"
 	"strings"
 
 	"k8s.io/apimachinery/pkg/api/meta"
@@ -38,6 +39,9 @@ import (
 
 // Options are creation options for a Client.
 type Options struct {
+	// HTTPClient is the HTTP client to use for requests.
+	HTTPClient *http.Client
+
 	// Scheme, if provided, will be used to map go structs to GroupVersionKinds
 	Scheme *runtime.Scheme
 
@@ -116,6 +120,15 @@ func newClient(config *rest.Config, options Options) (*client, error) {
 		)
 	}
 
+	// Use the rest HTTP client for the provided config if unset
+	if options.HTTPClient == nil {
+		var err error
+		options.HTTPClient, err = rest.HTTPClientFor(config)
+		if err != nil {
+			return nil, err
+		}
+	}
+
 	// Init a scheme if none provided
 	if options.Scheme == nil {
 		options.Scheme = scheme.Scheme
@@ -124,23 +137,24 @@ func newClient(config *rest.Config, options Options) (*client, error) {
 	// Init a Mapper if none provided
 	if options.Mapper == nil {
 		var err error
-		options.Mapper, err = apiutil.NewDynamicRESTMapper(config)
+		options.Mapper, err = apiutil.NewDynamicRESTMapper(config, options.HTTPClient)
 		if err != nil {
 			return nil, err
 		}
 	}
 
 	resources := &clientRestResources{
-		config: config,
-		scheme: options.Scheme,
-		mapper: options.Mapper,
-		codecs: serializer.NewCodecFactory(options.Scheme),
+		httpClient: options.HTTPClient,
+		config:     config,
+		scheme:     options.Scheme,
+		mapper:     options.Mapper,
+		codecs:     serializer.NewCodecFactory(options.Scheme),
 
 		structuredResourceByType:   make(map[schema.GroupVersionKind]*resourceMeta),
 		unstructuredResourceByType: make(map[schema.GroupVersionKind]*resourceMeta),
 	}
 
-	rawMetaClient, err := metadata.NewForConfig(config)
+	rawMetaClient, err := metadata.NewForConfigAndClient(config, options.HTTPClient)
 	if err != nil {
 		return nil, fmt.Errorf("unable to construct metadata-only client for use as part of client: %w", err)
 	}

pkg/client/client_rest_resources.go

@@ -17,6 +17,7 @@ limitations under the License.
 package client
 
 import (
+	"net/http"
 	"strings"
 	"sync"
 
@@ -32,6 +33,9 @@ import (
 
 // clientRestResources creates and stores rest clients and metadata for Kubernetes types.
 type clientRestResources struct {
+	// httpClient is the http client to use for requests
+	httpClient *http.Client
+
 	// config is the rest.Config to talk to an apiserver
 	config *rest.Config
 
@@ -59,7 +63,7 @@ func (c *clientRestResources) newResource(gvk schema.GroupVersionKind, isList, i
 		gvk.Kind = gvk.Kind[:len(gvk.Kind)-4]
 	}
 
-	client, err := apiutil.RESTClientForGVK(gvk, isUnstructured, c.config, c.codecs)
+	client, err := apiutil.RESTClientForGVK(gvk, isUnstructured, c.config, c.codecs, c.httpClient)
 	if err != nil {
 		return nil, err
 	}