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

client.Config describes how to make a client connection to a server
for HTTP traffic, but for connection upgrade scenarios cannot be
used because the underlying http.Transport object can't allow the
connection to be hijacked. Reorganize the TLS and connection wrapper
methods so that a sophisticated client can do:

    cfg := &client.Config{...} // from somewhere
    tlsConfig, _ := client.TLSConfigFor(cfg)
    _ := conn.Dial(...)
    rt := MyRoundTripper() // some func that implements grabbing requests
    wrapper, _ := client.HTTPWrappersFor(cfg)
    req := &http.Request{}
    req.Header.Set("Connection-Upgrade", ...)
    _, := wrapper.RoundTrip(req)
    // rt has been invoked with a fully formed Req with auth
    rt.Req.Write(conn)
    // read response for upgrade

It would be good to have utility function that does more of this,
but mostly enabling the HTTP2/SPDY client exec function right now.

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

@@ -147,9 +147,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)
@@ -165,11 +167,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")
 	}
 }