Merge pull request #3959 from smarterclayton/expose_tls_config_and_wrappers

Allow client.Config to be used for HTTP2 and WebSocket connections

pkg/client/helper.go

@@ -61,22 +61,8 @@ type Config struct {
 	// TODO: demonstrate an OAuth2 compatible client.
 	BearerToken string
 
-	// Server requires TLS client certificate authentication
-	CertFile string
-	// Server requires TLS client certificate authentication
-	KeyFile string
-	// Trusted root certificates for server
-	CAFile string
-
-	// CertData holds PEM-encoded bytes (typically read from a client certificate file).
-	// CertData takes precedence over CertFile
-	CertData []byte
-	// KeyData holds PEM-encoded bytes (typically read from a client certificate key file).
-	// KeyData takes precedence over KeyFile
-	KeyData []byte
-	// CAData holds PEM-encoded bytes (typically read from a root certificates bundle).
-	// CAData takes precedence over CAFile
-	CAData []byte
+	// TLSClientConfig contains settings to enable transport layer security
+	TLSClientConfig
 
 	// Server should be accessed without verifying the TLS
 	// certificate. For testing only.
@@ -92,11 +78,17 @@ type KubeletConfig struct {
 	Port        uint
 	EnableHttps bool
 
-	// TLS Configuration, only applies if EnableHttps is true.
+	// TLSClientConfig contains settings to enable transport layer security
+	TLSClientConfig
+}
+
+// TLSClientConfig contains settings to enable transport layer security
+type TLSClientConfig struct {
+	// Server requires TLS client certificate authentication
 	CertFile string
-	// TLS Configuration, only applies if EnableHttps is true.
+	// Server requires TLS client certificate authentication
 	KeyFile string
-	// TLS Configuration, only applies if EnableHttps is true.
+	// Trusted root certificates for server
 	CAFile string
 
 	// CertData holds PEM-encoded bytes (typically read from a client certificate file).
@@ -215,47 +207,40 @@ func TransportFor(config *Config) (http.RoundTripper, error) {
 	if config.Transport != nil && (hasCA || hasCert || config.Insecure) {
 		return nil, fmt.Errorf("using a custom transport with TLS certificate options or the insecure flag is not allowed")
 	}
-	if hasCA && config.Insecure {
-		return nil, fmt.Errorf("specifying a root certificates file with the insecure flag is not allowed")
-	}
-	var transport http.RoundTripper
-	switch {
-	case config.Transport != nil:
-		transport = config.Transport
-	case hasCert:
-		var (
-			certData, keyData, caData []byte
-			err                       error
-		)
-		if certData, err = dataFromSliceOrFile(config.CertData, config.CertFile); err != nil {
-			return nil, err
-		}
-		if keyData, err = dataFromSliceOrFile(config.KeyData, config.KeyFile); err != nil {
-			return nil, err
-		}
-		if caData, err = dataFromSliceOrFile(config.CAData, config.CAFile); err != nil {
-			return nil, err
-		}
-		if transport, err = NewClientCertTLSTransport(certData, keyData, caData); err != nil {
-			return nil, err
-		}
-	case hasCA:
-		var (
-			caData []byte
-			err    error
-		)
-		if caData, err = dataFromSliceOrFile(config.CAData, config.CAFile); err != nil {
-			return nil, err
-		}
-		if transport, err = NewTLSTransport(caData); err != nil {
-			return nil, err
-		}
-	case config.Insecure:
-		transport = NewUnsafeTLSTransport()
-	default:
-		transport = http.DefaultTransport
+
+	tlsConfig, err := TLSConfigFor(config)
+	if err != nil {
+		return nil, err
 	}
 
+	var transport http.RoundTripper
+	if config.Transport != nil {
+		transport = config.Transport
+	} else {
+		if tlsConfig != nil {
+			transport = &http.Transport{
+				TLSClientConfig: tlsConfig,
+			}
+		} else {
+			transport = http.DefaultTransport
+		}
+	}
+
+	transport, err = HTTPWrappersForConfig(config, transport)
+	if err != nil {
+		return nil, err
+	}
+
+	// TODO: use the config context to wrap a transport
+
+	return transport, nil
+}
+
+// HTTPWrappersForConfig wraps a round tripper with any relevant layered behavior from the
+// config. Exposed to allow more clients that need HTTP-like behavior but then must hijack
+// the underlying connection (like WebSocket or HTTP2 clients). Pure HTTP clients should use
+// the higher level TransportFor or RESTClientFor methods.
+func HTTPWrappersForConfig(config *Config, rt http.RoundTripper) (http.RoundTripper, error) {
 	// Set authentication wrappers
 	hasBasicAuth := config.Username != "" || config.Password != ""
 	if hasBasicAuth && config.BearerToken != "" {
@@ -263,14 +248,11 @@ func TransportFor(config *Config) (http.RoundTripper, error) {
 	}
 	switch {
 	case config.BearerToken != "":
-		transport = NewBearerAuthRoundTripper(config.BearerToken, transport)
+		rt = NewBearerAuthRoundTripper(config.BearerToken, rt)
 	case hasBasicAuth:
-		transport = NewBasicAuthRoundTripper(config.Username, config.Password, transport)
+		rt = NewBasicAuthRoundTripper(config.Username, config.Password, rt)
 	}
-
-	// TODO: use the config context to wrap a transport
-
-	return transport, nil
+	return rt, nil
 }
 
 // dataFromSliceOrFile returns data from the slice (if non-empty), or from the file,

pkg/client/helper_test.go

@@ -104,54 +104,68 @@ func TestTransportFor(t *testing.T) {
 		"ca transport": {
 			TLS: true,
 			Config: &Config{
-				CAData: []byte(rootCACert),
+				TLSClientConfig: TLSClientConfig{
+					CAData: []byte(rootCACert),
+				},
 			},
 		},
 		"bad ca file transport": {
 			Err: true,
 			Config: &Config{
-				CAFile: "invalid file",
+				TLSClientConfig: TLSClientConfig{
+					CAFile: "invalid file",
+				},
 			},
 		},
 		"ca data overriding bad ca file transport": {
 			TLS: true,
 			Config: &Config{
-				CAData: []byte(rootCACert),
-				CAFile: "invalid file",
+				TLSClientConfig: TLSClientConfig{
+					CAData: []byte(rootCACert),
+					CAFile: "invalid file",
+				},
 			},
 		},
 
 		"cert transport": {
 			TLS: true,
 			Config: &Config{
-				CertData: []byte(certData),
-				KeyData:  []byte(keyData),
-				CAData:   []byte(rootCACert),
+				TLSClientConfig: TLSClientConfig{
+					CertData: []byte(certData),
+					KeyData:  []byte(keyData),
+					CAData:   []byte(rootCACert),
+				},
 			},
 		},
 		"bad cert data transport": {
 			Err: true,
 			Config: &Config{
-				CertData: []byte(certData),
-				KeyData:  []byte("bad key data"),
-				CAData:   []byte(rootCACert),
+				TLSClientConfig: TLSClientConfig{
+					CertData: []byte(certData),
+					KeyData:  []byte("bad key data"),
+					CAData:   []byte(rootCACert),
+				},
 			},
 		},
 		"bad file cert transport": {
 			Err: true,
 			Config: &Config{
-				CertData: []byte(certData),
-				KeyFile:  "invalid file",
-				CAData:   []byte(rootCACert),
+				TLSClientConfig: TLSClientConfig{
+					CertData: []byte(certData),
+					KeyFile:  "invalid file",
+					CAData:   []byte(rootCACert),
+				},
 			},
 		},
 		"key data overriding bad file cert transport": {
 			TLS: true,
 			Config: &Config{
-				CertData: []byte(certData),
-				KeyData:  []byte(keyData),
-				KeyFile:  "invalid file",
-				CAData:   []byte(rootCACert),
+				TLSClientConfig: TLSClientConfig{
+					CertData: []byte(certData),
+					KeyData:  []byte(keyData),
+					KeyFile:  "invalid file",
+					CAData:   []byte(rootCACert),
+				},
 			},
 		},
 	}
@@ -206,15 +220,19 @@ func TestIsConfigTransportTLS(t *testing.T) {
 		},
 		{
 			Config: &Config{
-				Host:     "localhost",
-				CertFile: "foo",
+				Host: "localhost",
+				TLSClientConfig: TLSClientConfig{
+					CertFile: "foo",
+				},
 			},
 			TransportTLS: true,
 		},
 		{
 			Config: &Config{
-				Host:     "///:://localhost",
-				CertFile: "foo",
+				Host: "///:://localhost",
+				TLSClientConfig: TLSClientConfig{
+					CertFile: "foo",
+				},
 			},
 			TransportTLS: false,
 		},

pkg/client/kubelet.go

@@ -59,41 +59,25 @@ type HTTPKubeletClient struct {
 	EnableHttps bool
 }
 
+// TODO: this structure is questionable, it should be using client.Config and overriding defaults.
 func NewKubeletClient(config *KubeletConfig) (KubeletClient, error) {
 	transport := http.DefaultTransport
-	hasCA := len(config.CAFile) > 0 || len(config.CAData) > 0
-	hasCert := len(config.CertFile) > 0 || len(config.CertData) > 0
-	if hasCert {
-		var (
-			certData, keyData, caData []byte
-			err                       error
-		)
-		if certData, err = dataFromSliceOrFile(config.CertData, config.CertFile); err != nil {
-			return nil, err
-		}
-		if keyData, err = dataFromSliceOrFile(config.KeyData, config.KeyFile); err != nil {
-			return nil, err
-		}
-		if caData, err = dataFromSliceOrFile(config.CAData, config.CAFile); err != nil {
-			return nil, err
-		}
-		if transport, err = NewClientCertTLSTransport(certData, keyData, caData); err != nil {
-			return nil, err
-		}
-	} else if hasCA {
-		var (
-			caData []byte
-			err    error
-		)
-		if caData, err = dataFromSliceOrFile(config.CAData, config.CAFile); err != nil {
-			return nil, err
-		}
-		if transport, err = NewTLSTransport(caData); err != nil {
-			return nil, err
+
+	tlsConfig, err := TLSConfigFor(&Config{
+		TLSClientConfig: config.TLSClientConfig,
+	})
+	if err != nil {
+		return nil, err
+	}
+	if tlsConfig != nil {
+		transport = &http.Transport{
+			TLSClientConfig: tlsConfig,
 		}
 	}
 
-	c := &http.Client{Transport: transport}
+	c := &http.Client{
+		Transport: transport,
+	}
 	return &HTTPKubeletClient{
 		Client:      c,
 		Port:        config.Port,

pkg/client/kubelet_test.go

@@ -187,9 +187,11 @@ func TestNewKubeletClientTLSInvalid(t *testing.T) {
 		Port:        9000,
 		EnableHttps: true,
 		//Invalid certificate and key path
-		CertFile: "./testdata/mycertinvalid.cer",
-		KeyFile:  "./testdata/mycertinvalid.key",
-		CAFile:   "./testdata/myCA.cer",
+		TLSClientConfig: TLSClientConfig{
+			CertFile: "./testdata/mycertinvalid.cer",
+			KeyFile:  "./testdata/mycertinvalid.key",
+			CAFile:   "./testdata/myCA.cer",
+		},
 	}
 
 	client, err := NewKubeletClient(config)
@@ -205,11 +207,13 @@ func TestNewKubeletClientTLSValid(t *testing.T) {
 	config := &KubeletConfig{
 		Port:        9000,
 		EnableHttps: true,
-		CertFile:    "./testdata/mycertvalid.cer",
-		// TLS Configuration, only applies if EnableHttps is true.
-		KeyFile: "./testdata/mycertvalid.key",
-		// TLS Configuration, only applies if EnableHttps is true.
-		CAFile: "./testdata/myCA.cer",
+		TLSClientConfig: TLSClientConfig{
+			CertFile: "./testdata/mycertvalid.cer",
+			// TLS Configuration, only applies if EnableHttps is true.
+			KeyFile: "./testdata/mycertvalid.key",
+			// TLS Configuration, only applies if EnableHttps is true.
+			CAFile: "./testdata/myCA.cer",
+		},
 	}
 
 	client, err := NewKubeletClient(config)

pkg/client/transport.go

@@ -54,44 +54,84 @@ func (rt *bearerAuthRoundTripper) RoundTrip(req *http.Request) (*http.Response,
 	return rt.rt.RoundTrip(req)
 }
 
-func NewClientCertTLSTransport(certData, keyData, caData []byte) (*http.Transport, error) {
+// TLSConfigFor returns a tls.Config that will provide the transport level security defined
+// by the provided Config. Will return nil if no transport level security is requested.
+func TLSConfigFor(config *Config) (*tls.Config, error) {
+	hasCA := len(config.CAFile) > 0 || len(config.CAData) > 0
+	hasCert := len(config.CertFile) > 0 || len(config.CertData) > 0
+
+	if hasCA && config.Insecure {
+		return nil, fmt.Errorf("specifying a root certificates file with the insecure flag is not allowed")
+	}
+	var tlsConfig *tls.Config
+	switch {
+	case hasCert:
+		certData, err := dataFromSliceOrFile(config.CertData, config.CertFile)
+		if err != nil {
+			return nil, err
+		}
+		keyData, err := dataFromSliceOrFile(config.KeyData, config.KeyFile)
+		if err != nil {
+			return nil, err
+		}
+		caData, err := dataFromSliceOrFile(config.CAData, config.CAFile)
+		if err != nil {
+			return nil, err
+		}
+		cfg, err := NewClientCertTLSConfig(certData, keyData, caData)
+		if err != nil {
+			return nil, err
+		}
+		tlsConfig = cfg
+	case hasCA:
+		caData, err := dataFromSliceOrFile(config.CAData, config.CAFile)
+		if err != nil {
+			return nil, err
+		}
+		cfg, err := NewTLSConfig(caData)
+		if err != nil {
+			return nil, err
+		}
+		tlsConfig = cfg
+	case config.Insecure:
+		tlsConfig = NewUnsafeTLSConfig()
+	}
+
+	return tlsConfig, nil
+}
+
+func NewClientCertTLSConfig(certData, keyData, caData []byte) (*tls.Config, error) {
 	cert, err := tls.X509KeyPair(certData, keyData)
 	if err != nil {
 		return nil, err
 	}
 	certPool := x509.NewCertPool()
 	certPool.AppendCertsFromPEM(caData)
-	return &http.Transport{
-		TLSClientConfig: &tls.Config{
-			// Change default from SSLv3 to TLSv1.0 (because of POODLE vulnerability)
-			MinVersion: tls.VersionTLS10,
-			Certificates: []tls.Certificate{
-				cert,
-			},
-			RootCAs:    certPool,
-			ClientCAs:  certPool,
-			ClientAuth: tls.RequireAndVerifyClientCert,
+	return &tls.Config{
+		// Change default from SSLv3 to TLSv1.0 (because of POODLE vulnerability)
+		MinVersion: tls.VersionTLS10,
+		Certificates: []tls.Certificate{
+			cert,
 		},
+		RootCAs:    certPool,
+		ClientCAs:  certPool,
+		ClientAuth: tls.RequireAndVerifyClientCert,
 	}, nil
 }
 
-func NewTLSTransport(caData []byte) (*http.Transport, error) {
+func NewTLSConfig(caData []byte) (*tls.Config, error) {
 	certPool := x509.NewCertPool()
 	certPool.AppendCertsFromPEM(caData)
-	return &http.Transport{
-		TLSClientConfig: &tls.Config{
-			// Change default from SSLv3 to TLSv1.0 (because of POODLE vulnerability)
-			MinVersion: tls.VersionTLS10,
-			RootCAs:    certPool,
-		},
+	return &tls.Config{
+		// Change default from SSLv3 to TLSv1.0 (because of POODLE vulnerability)
+		MinVersion: tls.VersionTLS10,
+		RootCAs:    certPool,
 	}, nil
 }
 
-func NewUnsafeTLSTransport() *http.Transport {
-	return &http.Transport{
-		TLSClientConfig: &tls.Config{
-			InsecureSkipVerify: true,
-		},
+func NewUnsafeTLSConfig() *tls.Config {
+	return &tls.Config{
+		InsecureSkipVerify: true,
 	}
 }
 

pkg/client/transport_test.go

@@ -23,9 +23,9 @@ import (
 )
 
 func TestUnsecuredTLSTransport(t *testing.T) {
-	transport := NewUnsafeTLSTransport()
-	if !transport.TLSClientConfig.InsecureSkipVerify {
-		t.Errorf("expected transport to be insecure")
+	cfg := NewUnsafeTLSConfig()
+	if !cfg.InsecureSkipVerify {
+		t.Errorf("expected config to be insecure")
 	}
 }