diff --git a/pkg/client/helper.go b/pkg/client/helper.go index 45800c50e27..4d93ef7f245 100644 --- a/pkg/client/helper.go +++ b/pkg/client/helper.go @@ -27,6 +27,7 @@ import ( "reflect" gruntime "runtime" "strings" + "sync" "time" "github.com/golang/glog" @@ -329,6 +330,56 @@ func RESTClientFor(config *Config) (*RESTClient, error) { return client, nil } +var ( + // tlsTransports stores reusable round trippers with custom TLSClientConfig options + tlsTransports = map[string]*http.Transport{} + + // tlsTransportLock protects retrieval and storage of round trippers into the tlsTransports map + tlsTransportLock sync.Mutex +) + +// tlsTransportFor returns a http.RoundTripper for the given config, or an error +// The same RoundTripper will be returned for configs with identical TLS options +// If the config has no custom TLS options, http.DefaultTransport is returned +func tlsTransportFor(config *Config) (http.RoundTripper, error) { + // Get a unique key for the TLS options in the config + key, err := tlsConfigKey(config) + if err != nil { + return nil, err + } + + // Ensure we only create a single transport for the given TLS options + tlsTransportLock.Lock() + defer tlsTransportLock.Unlock() + + // See if we already have a custom transport for this config + if cachedTransport, ok := tlsTransports[key]; ok { + return cachedTransport, nil + } + + // Get the TLS options for this client config + tlsConfig, err := TLSConfigFor(config) + if err != nil { + return nil, err + } + // The options didn't require a custom TLS config + if tlsConfig == nil { + return http.DefaultTransport, nil + } + + // Cache a single transport for these options + tlsTransports[key] = &http.Transport{ + TLSClientConfig: tlsConfig, + Proxy: http.ProxyFromEnvironment, + Dial: (&net.Dialer{ + Timeout: 30 * time.Second, + KeepAlive: 30 * time.Second, + }).Dial, + TLSHandshakeTimeout: 10 * time.Second, + } + return tlsTransports[key], nil +} + // TransportFor returns an http.RoundTripper that will provide the authentication // or transport level security defined by the provided Config. Will return the // default http.DefaultTransport if no special case behavior is needed. @@ -341,30 +392,25 @@ func TransportFor(config *Config) (http.RoundTripper, error) { return nil, fmt.Errorf("using a custom transport with TLS certificate options or the insecure flag is not allowed") } - tlsConfig, err := TLSConfigFor(config) - if err != nil { - return nil, err - } + var ( + transport http.RoundTripper + err error + ) - var transport http.RoundTripper if config.Transport != nil { transport = config.Transport } else { - if tlsConfig != nil { - transport = &http.Transport{ - TLSClientConfig: tlsConfig, - Proxy: http.ProxyFromEnvironment, - Dial: (&net.Dialer{ - Timeout: 30 * time.Second, - KeepAlive: 30 * time.Second, - }).Dial, - TLSHandshakeTimeout: 10 * time.Second, - } - } else { - transport = http.DefaultTransport + transport, err = tlsTransportFor(config) + if err != nil { + return nil, err } } + // Call wrap prior to adding debugging wrappers + if config.WrapTransport != nil { + transport = config.WrapTransport(transport) + } + switch { case bool(glog.V(9)): transport = NewDebuggingRoundTripper(transport, CurlCommand, URLTiming, ResponseHeaders) @@ -376,10 +422,6 @@ func TransportFor(config *Config) (http.RoundTripper, error) { transport = NewDebuggingRoundTripper(transport, URLTiming) } - if config.WrapTransport != nil { - transport = config.WrapTransport(transport) - } - transport, err = HTTPWrappersForConfig(config, transport) if err != nil { return nil, err diff --git a/pkg/client/helper_test.go b/pkg/client/helper_test.go index 6556fb2e340..88690bbab9b 100644 --- a/pkg/client/helper_test.go +++ b/pkg/client/helper_test.go @@ -213,6 +213,66 @@ func TestTransportFor(t *testing.T) { } } +func TestTLSTransportCache(t *testing.T) { + // Empty the cache + tlsTransports = map[string]*http.Transport{} + // Construct several transports (Insecure=true to force a transport with custom tls settings) + identicalConfigurations := map[string]*Config{ + "empty": {Insecure: true}, + "host": {Insecure: true, Host: "foo"}, + "prefix": {Insecure: true, Prefix: "foo"}, + "version": {Insecure: true, Version: "foo"}, + "codec": {Insecure: true, Codec: latest.Codec}, + "basic": {Insecure: true, Username: "bob", Password: "password"}, + "bearer": {Insecure: true, BearerToken: "token"}, + "user agent": {Insecure: true, UserAgent: "useragent"}, + "wrap transport": {Insecure: true, WrapTransport: func(http.RoundTripper) http.RoundTripper { return nil }}, + "qps/burst": {Insecure: true, QPS: 1.0, Burst: 10}, + } + for k, v := range identicalConfigurations { + if _, err := TransportFor(v); err != nil { + t.Errorf("Unexpected error for %q: %v", k, err) + } + } + if len(tlsTransports) != 1 { + t.Errorf("Expected 1 cached transport, got %d", len(tlsTransports)) + } + + // Empty the cache + tlsTransports = map[string]*http.Transport{} + // Construct several transports with custom TLS settings + // (no normalization is performed on ca/cert/key data, so appending a newline lets us test "different" content) + uniqueConfigurations := map[string]*Config{ + "insecure": {Insecure: true}, + "cadata 1": {TLSClientConfig: TLSClientConfig{CAData: []byte(rootCACert)}}, + "cadata 2": {TLSClientConfig: TLSClientConfig{CAData: []byte(rootCACert + "\n")}}, + "cert 1, key 1": {TLSClientConfig: TLSClientConfig{CertData: []byte(certData), KeyData: []byte(keyData)}}, + "cert 1, key 2": {TLSClientConfig: TLSClientConfig{CertData: []byte(certData), KeyData: []byte(keyData + "\n")}}, + "cert 2, key 1": {TLSClientConfig: TLSClientConfig{CertData: []byte(certData + "\n"), KeyData: []byte(keyData)}}, + "cert 2, key 2": {TLSClientConfig: TLSClientConfig{CertData: []byte(certData + "\n"), KeyData: []byte(keyData + "\n")}}, + "cadata 1, cert 1, key 1": {TLSClientConfig: TLSClientConfig{CAData: []byte(rootCACert), CertData: []byte(certData), KeyData: []byte(keyData)}}, + } + for k, v := range uniqueConfigurations { + if _, err := TransportFor(v); err != nil { + t.Errorf("Unexpected error for %q: %v", k, err) + } + } + // All custom configs should result in a cache entry + if len(tlsTransports) != len(uniqueConfigurations) { + t.Errorf("Expected %d cached transports, got %d", len(uniqueConfigurations), len(tlsTransports)) + } + + // Empty the cache + tlsTransports = map[string]*http.Transport{} + if _, err := TransportFor(&Config{}); err != nil { + t.Errorf("Unexpected error: %v", err) + } + // A client config with no TLS options should use http.DefaultTransport, not a cached custom transport + if len(tlsTransports) != 0 { + t.Errorf("Expected no cached transports, got %d", len(tlsTransports)) + } +} + func TestIsConfigTransportTLS(t *testing.T) { testCases := []struct { Config *Config diff --git a/pkg/client/transport.go b/pkg/client/transport.go index b0da4ee14a1..fd9c942d900 100644 --- a/pkg/client/transport.go +++ b/pkg/client/transport.go @@ -117,6 +117,16 @@ func TLSConfigFor(config *Config) (*tls.Config, error) { return tlsConfig, nil } +// tlsConfigKey returns a unique key for tls.Config objects returned from TLSConfigFor +func tlsConfigKey(config *Config) (string, error) { + // Make sure ca/key/cert content is loaded + if err := LoadTLSFiles(config); err != nil { + return "", err + } + // Only include the things that actually affect the tls.Config + return fmt.Sprintf("%v/%x/%x/%x", config.Insecure, config.CAData, config.CertData, config.KeyData), nil +} + // LoadTLSFiles copies the data from the CertFile, KeyFile, and CAFile fields into the CertData, // KeyData, and CAFile fields, or returns an error. If no error is returned, all three fields are // either populated or were empty to start. diff --git a/pkg/client/transport_test.go b/pkg/client/transport_test.go index b4de569279e..f7bdd740e09 100644 --- a/pkg/client/transport_test.go +++ b/pkg/client/transport_test.go @@ -20,6 +20,8 @@ import ( "encoding/base64" "net/http" "testing" + + "k8s.io/kubernetes/pkg/api/latest" ) func TestUnsecuredTLSTransport(t *testing.T) { @@ -99,3 +101,74 @@ func TestUserAgentRoundTripper(t *testing.T) { t.Errorf("unexpected user agent header: %#v", rt.Request) } } + +func TestTLSConfigKey(t *testing.T) { + // Make sure config fields that don't affect the tls config don't affect the cache key + identicalConfigurations := map[string]*Config{ + "empty": {}, + "host": {Host: "foo"}, + "prefix": {Prefix: "foo"}, + "version": {Version: "foo"}, + "codec": {Codec: latest.Codec}, + "basic": {Username: "bob", Password: "password"}, + "bearer": {BearerToken: "token"}, + "user agent": {UserAgent: "useragent"}, + "transport": {Transport: http.DefaultTransport}, + "wrap transport": {WrapTransport: func(http.RoundTripper) http.RoundTripper { return nil }}, + "qps/burst": {QPS: 1.0, Burst: 10}, + } + for nameA, valueA := range identicalConfigurations { + for nameB, valueB := range identicalConfigurations { + keyA, err := tlsConfigKey(valueA) + if err != nil { + t.Errorf("Unexpected error for %q: %v", nameA, err) + continue + } + keyB, err := tlsConfigKey(valueB) + if err != nil { + t.Errorf("Unexpected error for %q: %v", nameB, err) + continue + } + if keyA != keyB { + t.Errorf("Expected identical cache keys for %q and %q, got:\n\t%s\n\t%s", nameA, nameB, keyA, keyB) + continue + } + } + } + + // Make sure config fields that affect the tls config affect the cache key + uniqueConfigurations := map[string]*Config{ + "no tls": {}, + "insecure": {Insecure: true}, + "cadata 1": {TLSClientConfig: TLSClientConfig{CAData: []byte{1}}}, + "cadata 2": {TLSClientConfig: TLSClientConfig{CAData: []byte{2}}}, + "cert 1, key 1": {TLSClientConfig: TLSClientConfig{CertData: []byte{1}, KeyData: []byte{1}}}, + "cert 1, key 2": {TLSClientConfig: TLSClientConfig{CertData: []byte{1}, KeyData: []byte{2}}}, + "cert 2, key 1": {TLSClientConfig: TLSClientConfig{CertData: []byte{2}, KeyData: []byte{1}}}, + "cert 2, key 2": {TLSClientConfig: TLSClientConfig{CertData: []byte{2}, KeyData: []byte{2}}}, + "cadata 1, cert 1, key 1": {TLSClientConfig: TLSClientConfig{CAData: []byte{1}, CertData: []byte{1}, KeyData: []byte{1}}}, + } + for nameA, valueA := range uniqueConfigurations { + for nameB, valueB := range uniqueConfigurations { + // Don't compare to ourselves + if nameA == nameB { + continue + } + + keyA, err := tlsConfigKey(valueA) + if err != nil { + t.Errorf("Unexpected error for %q: %v", nameA, err) + continue + } + keyB, err := tlsConfigKey(valueB) + if err != nil { + t.Errorf("Unexpected error for %q: %v", nameB, err) + continue + } + if keyA == keyB { + t.Errorf("Expected unique cache keys for %q and %q, got:\n\t%s\n\t%s", nameA, nameB, keyA, keyB) + continue + } + } + } +}