kubelet/client: collapse transport wiring onto standard approach

Signed-off-by: Monis Khan <mok@microsoft.com>

cmd/kube-apiserver/app/options/options.go

@@ -210,13 +210,13 @@ func (s *ServerRunOptions) Flags() (fss cliflag.NamedFlagSets) {
 	fs.DurationVar(&s.KubeletConfig.HTTPTimeout, "kubelet-timeout", s.KubeletConfig.HTTPTimeout,
 		"Timeout for kubelet operations.")
 
-	fs.StringVar(&s.KubeletConfig.CertFile, "kubelet-client-certificate", s.KubeletConfig.CertFile,
+	fs.StringVar(&s.KubeletConfig.TLSClientConfig.CertFile, "kubelet-client-certificate", s.KubeletConfig.TLSClientConfig.CertFile,
 		"Path to a client cert file for TLS.")
 
-	fs.StringVar(&s.KubeletConfig.KeyFile, "kubelet-client-key", s.KubeletConfig.KeyFile,
+	fs.StringVar(&s.KubeletConfig.TLSClientConfig.KeyFile, "kubelet-client-key", s.KubeletConfig.TLSClientConfig.KeyFile,
 		"Path to a client key file for TLS.")
 
-	fs.StringVar(&s.KubeletConfig.CAFile, "kubelet-certificate-authority", s.KubeletConfig.CAFile,
+	fs.StringVar(&s.KubeletConfig.TLSClientConfig.CAFile, "kubelet-certificate-authority", s.KubeletConfig.TLSClientConfig.CAFile,
 		"Path to a cert file for the certificate authority.")
 
 	fs.StringVar(&s.ProxyClientCertFile, "proxy-client-cert-file", s.ProxyClientCertFile, ""+

cmd/kube-apiserver/app/options/options_test.go

@@ -26,13 +26,13 @@ import (
 	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/spf13/pflag"
 	oteltrace "go.opentelemetry.io/otel/trace"
+
 	"k8s.io/apiserver/pkg/admission"
 	apiserveroptions "k8s.io/apiserver/pkg/server/options"
 	"k8s.io/apiserver/pkg/storage/etcd3"
 	"k8s.io/apiserver/pkg/storage/storagebackend"
 	auditbuffered "k8s.io/apiserver/plugin/pkg/audit/buffered"
 	audittruncate "k8s.io/apiserver/plugin/pkg/audit/truncate"
-	restclient "k8s.io/client-go/rest"
 	cliflag "k8s.io/component-base/cli/flag"
 	"k8s.io/component-base/logs"
 	"k8s.io/component-base/metrics"
@@ -200,7 +200,7 @@ func TestAddFlags(t *testing.T) {
 				string(kapi.NodeExternalIP),
 			},
 			HTTPTimeout: time.Duration(5) * time.Second,
-			TLSClientConfig: restclient.TLSClientConfig{
+			TLSClientConfig: kubeletclient.KubeletTLSConfig{
 				CertFile: "/var/run/kubernetes/ceserver.crt",
 				KeyFile:  "/var/run/kubernetes/server.key",
 				CAFile:   "/var/run/kubernetes/caserver.crt",

pkg/kubelet/client/kubelet_client.go

@@ -26,9 +26,7 @@ import (
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
-	utilnet "k8s.io/apimachinery/pkg/util/net"
 	"k8s.io/apiserver/pkg/server/egressselector"
-	restclient "k8s.io/client-go/rest"
 	"k8s.io/client-go/transport"
 	nodeutil "k8s.io/kubernetes/pkg/util/node"
 )
@@ -45,21 +43,24 @@ type KubeletClientConfig struct {
 	PreferredAddressTypes []string
 
 	// TLSClientConfig contains settings to enable transport layer security
-	restclient.TLSClientConfig
-
-	// Server requires Bearer authentication
-	BearerToken string `datapolicy:"token"`
+	TLSClientConfig KubeletTLSConfig
 
 	// HTTPTimeout is used by the client to timeout http requests to Kubelet.
 	HTTPTimeout time.Duration
 
-	// Dial is a custom dialer used for the client
-	Dial utilnet.DialFunc
-
 	// Lookup will give us a dialer if the egress selector is configured for it
 	Lookup egressselector.Lookup
 }
 
+type KubeletTLSConfig struct {
+	// Server requires TLS client certificate authentication
+	CertFile string
+	// Server requires TLS client certificate authentication
+	KeyFile string
+	// Trusted root certificates for server
+	CAFile string
+}
+
 // ConnectionInfo provides the information needed to connect to a kubelet
 type ConnectionInfo struct {
 	Scheme                         string
@@ -88,53 +89,38 @@ func MakeInsecureTransport(config *KubeletClientConfig) (http.RoundTripper, erro
 func makeTransport(config *KubeletClientConfig, insecureSkipTLSVerify bool) (http.RoundTripper, error) {
 	// do the insecureSkipTLSVerify on the pre-transport *before* we go get a potentially cached connection.
 	// transportConfig always produces a new struct pointer.
-	preTLSConfig := config.transportConfig()
-	if insecureSkipTLSVerify && preTLSConfig != nil {
-		preTLSConfig.TLS.Insecure = true
-		preTLSConfig.TLS.CAData = nil
-		preTLSConfig.TLS.CAFile = ""
+	transportConfig := config.transportConfig()
+	if insecureSkipTLSVerify {
+		transportConfig.TLS.Insecure = true
+		transportConfig.TLS.CAFile = "" // we are only using files so we can ignore CAData
 	}
 
-	tlsConfig, err := transport.TLSConfigFor(preTLSConfig)
-	if err != nil {
-		return nil, err
-	}
-
-	rt := http.DefaultTransport
-	dialer := config.Dial
-	if dialer == nil && config.Lookup != nil {
+	if config.Lookup != nil {
 		// Assuming EgressSelector if SSHTunnel is not turned on.
 		// We will not get a dialer if egress selector is disabled.
 		networkContext := egressselector.Cluster.AsNetworkContext()
-		dialer, err = config.Lookup(networkContext)
+		dialer, err := config.Lookup(networkContext)
 		if err != nil {
 			return nil, fmt.Errorf("failed to get context dialer for 'cluster': got %v", err)
 		}
+		if dialer != nil {
+			transportConfig.DialHolder = &transport.DialHolder{Dial: dialer}
+		}
 	}
-	if dialer != nil || tlsConfig != nil {
-		// If SSH Tunnel is turned on
-		rt = utilnet.SetOldTransportDefaults(&http.Transport{
-			DialContext:     dialer,
-			TLSClientConfig: tlsConfig,
-		})
-	}
-
-	return transport.HTTPWrappersForConfig(config.transportConfig(), rt)
+	return transport.New(transportConfig)
 }
 
 // transportConfig converts a client config to an appropriate transport config.
 func (c *KubeletClientConfig) transportConfig() *transport.Config {
 	cfg := &transport.Config{
 		TLS: transport.TLSConfig{
-			CAFile:     c.CAFile,
-			CAData:     c.CAData,
-			CertFile:   c.CertFile,
-			CertData:   c.CertData,
-			KeyFile:    c.KeyFile,
-			KeyData:    c.KeyData,
-			NextProtos: c.NextProtos,
+			CAFile:   c.TLSClientConfig.CAFile,
+			CertFile: c.TLSClientConfig.CertFile,
+			KeyFile:  c.TLSClientConfig.KeyFile,
+			// transport.loadTLSFiles would set this to true because we are only using files
+			// it is clearer to set it explicitly here so we remember that this is happening
+			ReloadTLSFiles: true,
 		},
-		BearerToken: c.BearerToken,
 	}
 	if !cfg.HasCA() {
 		cfg.TLS.Insecure = true

pkg/kubelet/client/kubelet_client_test.go

@@ -24,14 +24,12 @@ import (
 	"net/url"
 	"strconv"
 	"testing"
-
-	restclient "k8s.io/client-go/rest"
 )
 
 func TestMakeTransportInvalid(t *testing.T) {
 	config := &KubeletClientConfig{
-		//Invalid certificate and key path
-		TLSClientConfig: restclient.TLSClientConfig{
+		// Invalid certificate and key path
+		TLSClientConfig: KubeletTLSConfig{
 			CertFile: "../../client/testdata/mycertinvalid.cer",
 			KeyFile:  "../../client/testdata/mycertinvalid.key",
 			CAFile:   "../../client/testdata/myCA.cer",
@@ -50,7 +48,7 @@ func TestMakeTransportInvalid(t *testing.T) {
 func TestMakeTransportValid(t *testing.T) {
 	config := &KubeletClientConfig{
 		Port: 1234,
-		TLSClientConfig: restclient.TLSClientConfig{
+		TLSClientConfig: KubeletTLSConfig{
 			CertFile: "../../client/testdata/mycertvalid.cer",
 			// TLS Configuration
 			KeyFile: "../../client/testdata/mycertvalid.key",
@@ -61,7 +59,7 @@ func TestMakeTransportValid(t *testing.T) {
 
 	rt, err := MakeTransport(config)
 	if err != nil {
-		t.Errorf("Not expecting an error #%v", err)
+		t.Errorf("Not expecting an error %#v", err)
 	}
 	if rt == nil {
 		t.Error("rt should not be nil")
@@ -89,7 +87,7 @@ func TestMakeInsecureTransport(t *testing.T) {
 
 	config := &KubeletClientConfig{
 		Port: uint(port),
-		TLSClientConfig: restclient.TLSClientConfig{
+		TLSClientConfig: KubeletTLSConfig{
 			CertFile: "../../client/testdata/mycertvalid.cer",
 			// TLS Configuration
 			KeyFile: "../../client/testdata/mycertvalid.key",

staging/src/k8s.io/client-go/transport/cache.go

@@ -109,7 +109,7 @@ func (c *tlsTransportCache) get(config *Config) (http.RoundTripper, error) {
 
 	// If we use are reloading files, we need to handle certificate rotation properly
 	// TODO(jackkleeman): We can also add rotation here when config.HasCertCallback() is true
-	if config.TLS.ReloadTLSFiles {
+	if config.TLS.ReloadTLSFiles && tlsConfig != nil && tlsConfig.GetClientCertificate != nil {
 		dynamicCertDialer := certRotatingDialer(tlsConfig.GetClientCertificate, dial)
 		tlsConfig.GetClientCertificate = dynamicCertDialer.GetClientCertificate
 		dial = dynamicCertDialer.connDialer.DialContext