From f3f666d5f1f6f74a8c948a5c64af993696178244 Mon Sep 17 00:00:00 2001 From: Mike Danese Date: Fri, 3 May 2019 13:50:17 -0700 Subject: [PATCH 1/2] rest.Config: support configuring an explict proxy URL With support of http, https, and socks5 proxy support. We already support configuring this via environmnet variables, but this approach becomes inconvenient dealing with multiple clusters on different networks, that require different proxies to connect to. Most solutions require wrapping clients (like kubectl) in bash scripts. Part of: https://github.com/kubernetes/client-go/issues/351 --- staging/src/k8s.io/client-go/rest/BUILD | 1 + .../src/k8s.io/client-go/rest/client_test.go | 59 ++++++++++++- staging/src/k8s.io/client-go/rest/config.go | 10 +++ .../src/k8s.io/client-go/rest/config_test.go | 64 +++++++++++---- .../src/k8s.io/client-go/rest/transport.go | 3 +- .../client-go/tools/clientcmd/api/types.go | 11 +++ .../client-go/tools/clientcmd/api/v1/types.go | 11 +++ .../api/v1/zz_generated.conversion.go | 2 + .../tools/clientcmd/client_config.go | 14 +++- .../tools/clientcmd/client_config_test.go | 82 ++++++++++++++++++- .../client-go/tools/clientcmd/helpers.go | 15 ++++ .../client-go/tools/clientcmd/validation.go | 5 ++ .../src/k8s.io/client-go/transport/cache.go | 13 ++- .../k8s.io/client-go/transport/cache_test.go | 21 +++++ .../src/k8s.io/client-go/transport/config.go | 8 ++ 15 files changed, 292 insertions(+), 27 deletions(-) diff --git a/staging/src/k8s.io/client-go/rest/BUILD b/staging/src/k8s.io/client-go/rest/BUILD index b6ef3f55ed1..c966078fdd9 100644 --- a/staging/src/k8s.io/client-go/rest/BUILD +++ b/staging/src/k8s.io/client-go/rest/BUILD @@ -38,6 +38,7 @@ go_test( "//staging/src/k8s.io/client-go/transport:go_default_library", "//staging/src/k8s.io/client-go/util/flowcontrol:go_default_library", "//staging/src/k8s.io/client-go/util/testing:go_default_library", + "//vendor/github.com/google/go-cmp/cmp:go_default_library", "//vendor/github.com/google/gofuzz:go_default_library", "//vendor/github.com/stretchr/testify/assert:go_default_library", "//vendor/k8s.io/klog:go_default_library", diff --git a/staging/src/k8s.io/client-go/rest/client_test.go b/staging/src/k8s.io/client-go/rest/client_test.go index a30e79e18ca..5e12152d403 100644 --- a/staging/src/k8s.io/client-go/rest/client_test.go +++ b/staging/src/k8s.io/client-go/rest/client_test.go @@ -18,16 +18,16 @@ package rest import ( "context" + "fmt" "net/http" "net/http/httptest" + "net/http/httputil" "net/url" "os" "reflect" "testing" "time" - "fmt" - v1 "k8s.io/api/core/v1" v1beta1 "k8s.io/api/extensions/v1beta1" "k8s.io/apimachinery/pkg/api/errors" @@ -37,6 +37,8 @@ import ( "k8s.io/apimachinery/pkg/util/diff" "k8s.io/client-go/kubernetes/scheme" utiltesting "k8s.io/client-go/util/testing" + + "github.com/google/go-cmp/cmp" ) type TestParam struct { @@ -252,7 +254,7 @@ func validate(testParam TestParam, t *testing.T, body []byte, fakeHandler *utilt } -func TestHttpMethods(t *testing.T) { +func TestHTTPMethods(t *testing.T) { testServer, _, _ := testServerEnv(t, 200) defer testServer.Close() c, _ := restClient(testServer) @@ -283,6 +285,57 @@ func TestHttpMethods(t *testing.T) { } } +func TestHTTPProxy(t *testing.T) { + ctx := context.Background() + testServer, fh, _ := testServerEnv(t, 200) + fh.ResponseBody = "backend data" + defer testServer.Close() + + testProxyServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + to, err := url.Parse(req.RequestURI) + if err != nil { + t.Fatalf("err: %v", err) + } + w.Write([]byte("proxied: ")) + httputil.NewSingleHostReverseProxy(to).ServeHTTP(w, req) + })) + defer testProxyServer.Close() + + t.Logf(testProxyServer.URL) + + u, err := url.Parse(testProxyServer.URL) + if err != nil { + t.Fatalf("Failed to parse test proxy server url: %v", err) + } + + c, err := RESTClientFor(&Config{ + Host: testServer.URL, + ContentConfig: ContentConfig{ + GroupVersion: &v1.SchemeGroupVersion, + NegotiatedSerializer: scheme.Codecs.WithoutConversion(), + }, + Proxy: http.ProxyURL(u), + Username: "user", + Password: "pass", + }) + if err != nil { + t.Fatalf("Failed to create client: %v", err) + } + + request := c.Get() + if request == nil { + t.Fatalf("Get: Object returned should not be nil") + } + + b, err := request.DoRaw(ctx) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if got, want := string(b), "proxied: backend data"; !cmp.Equal(got, want) { + t.Errorf("unexpected body: %v", cmp.Diff(want, got)) + } +} + func TestCreateBackoffManager(t *testing.T) { theUrl, _ := url.Parse("http://localhost") diff --git a/staging/src/k8s.io/client-go/rest/config.go b/staging/src/k8s.io/client-go/rest/config.go index f58f518303b..f371565a7bc 100644 --- a/staging/src/k8s.io/client-go/rest/config.go +++ b/staging/src/k8s.io/client-go/rest/config.go @@ -23,6 +23,7 @@ import ( "io/ioutil" "net" "net/http" + "net/url" "os" "path/filepath" gruntime "runtime" @@ -128,6 +129,13 @@ type Config struct { // Dial specifies the dial function for creating unencrypted TCP connections. Dial func(ctx context.Context, network, address string) (net.Conn, error) + // Proxy is the the proxy func to be used for all requests made by this + // transport. If Proxy is nil, http.ProxyFromEnvironment is used. If Proxy + // returns a nil *URL, no proxy is used. + // + // socks5 proxying does not currently support spdy streaming endpoints. + Proxy func(*http.Request) (*url.URL, error) + // Version forces a specific version to be used (if registered) // Do we need this? // Version string @@ -560,6 +568,7 @@ func AnonymousClientConfig(config *Config) *Config { Burst: config.Burst, Timeout: config.Timeout, Dial: config.Dial, + Proxy: config.Proxy, } } @@ -601,5 +610,6 @@ func CopyConfig(config *Config) *Config { RateLimiter: config.RateLimiter, Timeout: config.Timeout, Dial: config.Dial, + Proxy: config.Proxy, } } diff --git a/staging/src/k8s.io/client-go/rest/config_test.go b/staging/src/k8s.io/client-go/rest/config_test.go index f8e6b2d64b6..72c7f5b026f 100644 --- a/staging/src/k8s.io/client-go/rest/config_test.go +++ b/staging/src/k8s.io/client-go/rest/config_test.go @@ -23,6 +23,7 @@ import ( "io" "net" "net/http" + "net/url" "path/filepath" "reflect" "strings" @@ -32,12 +33,12 @@ import ( v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/util/diff" "k8s.io/client-go/kubernetes/scheme" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" "k8s.io/client-go/transport" "k8s.io/client-go/util/flowcontrol" + "github.com/google/go-cmp/cmp" fuzz "github.com/google/gofuzz" "github.com/stretchr/testify/assert" ) @@ -274,8 +275,13 @@ func (n *fakeNegotiatedSerializer) DecoderToVersion(serializer runtime.Decoder, var fakeDialFunc = func(ctx context.Context, network, addr string) (net.Conn, error) { return nil, fakeDialerError } + var fakeDialerError = errors.New("fakedialer") +func fakeProxyFunc(*http.Request) (*url.URL, error) { + return nil, errors.New("fakeproxy") +} + type fakeAuthProviderConfigPersister struct{} func (fakeAuthProviderConfigPersister) Persist(map[string]string) error { @@ -318,8 +324,12 @@ func TestAnonymousConfig(t *testing.T) { func(r *clientcmdapi.AuthProviderConfig, f fuzz.Continue) { r.Config = map[string]string{} }, - // Dial does not require fuzzer - func(r *func(ctx context.Context, network, addr string) (net.Conn, error), f fuzz.Continue) {}, + func(r *func(ctx context.Context, network, addr string) (net.Conn, error), f fuzz.Continue) { + *r = fakeDialFunc + }, + func(r *func(*http.Request) (*url.URL, error), f fuzz.Continue) { + *r = fakeProxyFunc + }, ) for i := 0; i < 20; i++ { original := &Config{} @@ -350,13 +360,22 @@ func TestAnonymousConfig(t *testing.T) { if !reflect.DeepEqual(expectedError, actualError) { t.Fatalf("AnonymousClientConfig dropped the Dial field") } - } else { - actual.Dial = nil - expected.Dial = nil } + actual.Dial = nil + expected.Dial = nil - if !reflect.DeepEqual(*actual, expected) { - t.Fatalf("AnonymousClientConfig dropped unexpected fields, identify whether they are security related or not: %s", diff.ObjectGoPrintDiff(expected, actual)) + if actual.Proxy != nil { + _, actualError := actual.Proxy(nil) + _, expectedError := expected.Proxy(nil) + if !reflect.DeepEqual(expectedError, actualError) { + t.Fatalf("AnonymousClientConfig dropped the Proxy field") + } + } + actual.Proxy = nil + expected.Proxy = nil + + if diff := cmp.Diff(*actual, expected); diff != "" { + t.Fatalf("AnonymousClientConfig dropped unexpected fields, identify whether they are security related or not (-got, +want): %s", diff) } } } @@ -396,6 +415,9 @@ func TestCopyConfig(t *testing.T) { func(r *func(ctx context.Context, network, addr string) (net.Conn, error), f fuzz.Continue) { *r = fakeDialFunc }, + func(r *func(*http.Request) (*url.URL, error), f fuzz.Continue) { + *r = fakeProxyFunc + }, ) for i := 0; i < 20; i++ { original := &Config{} @@ -410,10 +432,10 @@ func TestCopyConfig(t *testing.T) { // function return the expected object. if actual.WrapTransport == nil || !reflect.DeepEqual(expected.WrapTransport(nil), &fakeRoundTripper{}) { t.Fatalf("CopyConfig dropped the WrapTransport field") - } else { - actual.WrapTransport = nil - expected.WrapTransport = nil } + actual.WrapTransport = nil + expected.WrapTransport = nil + if actual.Dial != nil { _, actualError := actual.Dial(context.Background(), "", "") _, expectedError := expected.Dial(context.Background(), "", "") @@ -423,6 +445,7 @@ func TestCopyConfig(t *testing.T) { } actual.Dial = nil expected.Dial = nil + if actual.AuthConfigPersister != nil { actualError := actual.AuthConfigPersister.Persist(nil) expectedError := expected.AuthConfigPersister.Persist(nil) @@ -433,8 +456,18 @@ func TestCopyConfig(t *testing.T) { actual.AuthConfigPersister = nil expected.AuthConfigPersister = nil - if !reflect.DeepEqual(*actual, expected) { - t.Fatalf("CopyConfig dropped unexpected fields, identify whether they are security related or not: %s", diff.ObjectReflectDiff(expected, *actual)) + if actual.Proxy != nil { + _, actualError := actual.Proxy(nil) + _, expectedError := expected.Proxy(nil) + if !reflect.DeepEqual(expectedError, actualError) { + t.Fatalf("CopyConfig dropped the Proxy field") + } + } + actual.Proxy = nil + expected.Proxy = nil + + if diff := cmp.Diff(*actual, expected); diff != "" { + t.Fatalf("CopyConfig dropped unexpected fields, identify whether they are security related or not (-got, +want): %s", diff) } } } @@ -564,10 +597,11 @@ func TestConfigSprint(t *testing.T) { RateLimiter: &fakeLimiter{}, Timeout: 3 * time.Second, Dial: fakeDialFunc, + Proxy: fakeProxyFunc, } want := fmt.Sprintf( - `&rest.Config{Host:"localhost:8080", APIPath:"v1", ContentConfig:rest.ContentConfig{AcceptContentTypes:"application/json", ContentType:"application/json", GroupVersion:(*schema.GroupVersion)(nil), NegotiatedSerializer:runtime.NegotiatedSerializer(nil)}, Username:"gopher", Password:"--- REDACTED ---", BearerToken:"--- REDACTED ---", BearerTokenFile:"", Impersonate:rest.ImpersonationConfig{UserName:"gopher2", Groups:[]string(nil), Extra:map[string][]string(nil)}, AuthProvider:api.AuthProviderConfig{Name: "gopher", Config: map[string]string{--- REDACTED ---}}, AuthConfigPersister:rest.AuthProviderConfigPersister(--- REDACTED ---), ExecProvider:api.AuthProviderConfig{Command: "sudo", Args: []string{"--- REDACTED ---"}, Env: []ExecEnvVar{--- REDACTED ---}, APIVersion: ""}, TLSClientConfig:rest.sanitizedTLSClientConfig{Insecure:false, ServerName:"", CertFile:"a.crt", KeyFile:"a.key", CAFile:"", CertData:[]uint8{0x2d, 0x2d, 0x2d, 0x20, 0x54, 0x52, 0x55, 0x4e, 0x43, 0x41, 0x54, 0x45, 0x44, 0x20, 0x2d, 0x2d, 0x2d}, KeyData:[]uint8{0x2d, 0x2d, 0x2d, 0x20, 0x52, 0x45, 0x44, 0x41, 0x43, 0x54, 0x45, 0x44, 0x20, 0x2d, 0x2d, 0x2d}, CAData:[]uint8(nil), NextProtos:[]string{"h2", "http/1.1"}}, UserAgent:"gobot", DisableCompression:false, Transport:(*rest.fakeRoundTripper)(%p), WrapTransport:(transport.WrapperFunc)(%p), QPS:1, Burst:2, RateLimiter:(*rest.fakeLimiter)(%p), Timeout:3000000000, Dial:(func(context.Context, string, string) (net.Conn, error))(%p)}`, - c.Transport, fakeWrapperFunc, c.RateLimiter, fakeDialFunc, + `&rest.Config{Host:"localhost:8080", APIPath:"v1", ContentConfig:rest.ContentConfig{AcceptContentTypes:"application/json", ContentType:"application/json", GroupVersion:(*schema.GroupVersion)(nil), NegotiatedSerializer:runtime.NegotiatedSerializer(nil)}, Username:"gopher", Password:"--- REDACTED ---", BearerToken:"--- REDACTED ---", BearerTokenFile:"", Impersonate:rest.ImpersonationConfig{UserName:"gopher2", Groups:[]string(nil), Extra:map[string][]string(nil)}, AuthProvider:api.AuthProviderConfig{Name: "gopher", Config: map[string]string{--- REDACTED ---}}, AuthConfigPersister:rest.AuthProviderConfigPersister(--- REDACTED ---), ExecProvider:api.AuthProviderConfig{Command: "sudo", Args: []string{"--- REDACTED ---"}, Env: []ExecEnvVar{--- REDACTED ---}, APIVersion: ""}, TLSClientConfig:rest.sanitizedTLSClientConfig{Insecure:false, ServerName:"", CertFile:"a.crt", KeyFile:"a.key", CAFile:"", CertData:[]uint8{0x2d, 0x2d, 0x2d, 0x20, 0x54, 0x52, 0x55, 0x4e, 0x43, 0x41, 0x54, 0x45, 0x44, 0x20, 0x2d, 0x2d, 0x2d}, KeyData:[]uint8{0x2d, 0x2d, 0x2d, 0x20, 0x52, 0x45, 0x44, 0x41, 0x43, 0x54, 0x45, 0x44, 0x20, 0x2d, 0x2d, 0x2d}, CAData:[]uint8(nil), NextProtos:[]string{"h2", "http/1.1"}}, UserAgent:"gobot", DisableCompression:false, Transport:(*rest.fakeRoundTripper)(%p), WrapTransport:(transport.WrapperFunc)(%p), QPS:1, Burst:2, RateLimiter:(*rest.fakeLimiter)(%p), Timeout:3000000000, Dial:(func(context.Context, string, string) (net.Conn, error))(%p), Proxy:(func(*http.Request) (*url.URL, error))(%p)}`, + c.Transport, fakeWrapperFunc, c.RateLimiter, fakeDialFunc, fakeProxyFunc, ) for _, f := range []string{"%s", "%v", "%+v", "%#v"} { diff --git a/staging/src/k8s.io/client-go/rest/transport.go b/staging/src/k8s.io/client-go/rest/transport.go index 0800e4ec747..450edc6edde 100644 --- a/staging/src/k8s.io/client-go/rest/transport.go +++ b/staging/src/k8s.io/client-go/rest/transport.go @@ -85,7 +85,8 @@ func (c *Config) TransportConfig() (*transport.Config, error) { Groups: c.Impersonate.Groups, Extra: c.Impersonate.Extra, }, - Dial: c.Dial, + Dial: c.Dial, + Proxy: c.Proxy, } if c.ExecProvider != nil && c.AuthProvider != nil { diff --git a/staging/src/k8s.io/client-go/tools/clientcmd/api/types.go b/staging/src/k8s.io/client-go/tools/clientcmd/api/types.go index 44317dd019a..57acb3dbe8a 100644 --- a/staging/src/k8s.io/client-go/tools/clientcmd/api/types.go +++ b/staging/src/k8s.io/client-go/tools/clientcmd/api/types.go @@ -82,6 +82,17 @@ type Cluster struct { // CertificateAuthorityData contains PEM-encoded certificate authority certificates. Overrides CertificateAuthority // +optional CertificateAuthorityData []byte `json:"certificate-authority-data,omitempty"` + // ProxyURL is the URL to the proxy to be used for all requests made by this + // client. URLs with "http", "https", and "socks5" schemes are supported. If + // this configuration is not provided or the empty string, the client + // attempts to construct a proxy configuration from http_proxy and + // https_proxy environment variables. If these environment variables are not + // set, the client does not attempt to proxy requests. + // + // socks5 proxying does not currently support spdy streaming endpoints (exec, + // attach, port forward). + // +optional + ProxyURL string `json:"proxy-url,omitempty"` // Extensions holds additional information. This is useful for extenders so that reads and writes don't clobber unknown fields // +optional Extensions map[string]runtime.Object `json:"extensions,omitempty"` diff --git a/staging/src/k8s.io/client-go/tools/clientcmd/api/v1/types.go b/staging/src/k8s.io/client-go/tools/clientcmd/api/v1/types.go index 8ccacd3f879..c6880f43b7d 100644 --- a/staging/src/k8s.io/client-go/tools/clientcmd/api/v1/types.go +++ b/staging/src/k8s.io/client-go/tools/clientcmd/api/v1/types.go @@ -75,6 +75,17 @@ type Cluster struct { // CertificateAuthorityData contains PEM-encoded certificate authority certificates. Overrides CertificateAuthority // +optional CertificateAuthorityData []byte `json:"certificate-authority-data,omitempty"` + // ProxyURL is the URL to the proxy to be used for all requests made by this + // client. URLs with "http", "https", and "socks5" schemes are supported. If + // this configuration is not provided or the empty string, the client + // attempts to construct a proxy configuration from http_proxy and + // https_proxy environment variables. If these environment variables are not + // set, the client does not attempt to proxy requests. + // + // socks5 proxying does not currently support spdy streaming endpoints (exec, + // attach, port forward). + // +optional + ProxyURL string `json:"proxy-url,omitempty"` // Extensions holds additional information. This is useful for extenders so that reads and writes don't clobber unknown fields // +optional Extensions []NamedExtension `json:"extensions,omitempty"` diff --git a/staging/src/k8s.io/client-go/tools/clientcmd/api/v1/zz_generated.conversion.go b/staging/src/k8s.io/client-go/tools/clientcmd/api/v1/zz_generated.conversion.go index 8f3631e151b..5c12551ee75 100644 --- a/staging/src/k8s.io/client-go/tools/clientcmd/api/v1/zz_generated.conversion.go +++ b/staging/src/k8s.io/client-go/tools/clientcmd/api/v1/zz_generated.conversion.go @@ -237,6 +237,7 @@ func autoConvert_v1_Cluster_To_api_Cluster(in *Cluster, out *api.Cluster, s conv out.InsecureSkipTLSVerify = in.InsecureSkipTLSVerify out.CertificateAuthority = in.CertificateAuthority out.CertificateAuthorityData = *(*[]byte)(unsafe.Pointer(&in.CertificateAuthorityData)) + out.ProxyURL = in.ProxyURL if err := Convert_Slice_v1_NamedExtension_To_Map_string_To_runtime_Object(&in.Extensions, &out.Extensions, s); err != nil { return err } @@ -255,6 +256,7 @@ func autoConvert_api_Cluster_To_v1_Cluster(in *api.Cluster, out *Cluster, s conv out.InsecureSkipTLSVerify = in.InsecureSkipTLSVerify out.CertificateAuthority = in.CertificateAuthority out.CertificateAuthorityData = *(*[]byte)(unsafe.Pointer(&in.CertificateAuthorityData)) + out.ProxyURL = in.ProxyURL if err := Convert_Map_string_To_runtime_Object_To_Slice_v1_NamedExtension(&in.Extensions, &out.Extensions, s); err != nil { return err } diff --git a/staging/src/k8s.io/client-go/tools/clientcmd/client_config.go b/staging/src/k8s.io/client-go/tools/clientcmd/client_config.go index a9806384aab..c010f70697a 100644 --- a/staging/src/k8s.io/client-go/tools/clientcmd/client_config.go +++ b/staging/src/k8s.io/client-go/tools/clientcmd/client_config.go @@ -20,16 +20,17 @@ import ( "fmt" "io" "io/ioutil" + "net/http" "net/url" "os" "strings" - "github.com/imdario/mergo" - "k8s.io/klog" - restclient "k8s.io/client-go/rest" clientauth "k8s.io/client-go/tools/auth" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" + "k8s.io/klog" + + "github.com/imdario/mergo" ) var ( @@ -150,6 +151,13 @@ func (config *DirectClientConfig) ClientConfig() (*restclient.Config, error) { clientConfig := &restclient.Config{} clientConfig.Host = configClusterInfo.Server + if configClusterInfo.ProxyURL != "" { + u, err := parseProxyURL(configClusterInfo.ProxyURL) + if err != nil { + return nil, err + } + clientConfig.Proxy = http.ProxyURL(u) + } if len(config.overrides.Timeout) > 0 { timeout, err := ParseTimeout(config.overrides.Timeout) diff --git a/staging/src/k8s.io/client-go/tools/clientcmd/client_config_test.go b/staging/src/k8s.io/client-go/tools/clientcmd/client_config_test.go index 3232d8b04ec..550fa91d728 100644 --- a/staging/src/k8s.io/client-go/tools/clientcmd/client_config_test.go +++ b/staging/src/k8s.io/client-go/tools/clientcmd/client_config_test.go @@ -23,10 +23,10 @@ import ( "strings" "testing" - "github.com/imdario/mergo" - restclient "k8s.io/client-go/rest" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" + + "github.com/imdario/mergo" ) func TestMergoSemantics(t *testing.T) { @@ -330,6 +330,84 @@ func TestCertificateData(t *testing.T) { matchByteArg(keyData, clientConfig.TLSClientConfig.KeyData, t) } +func TestProxyURL(t *testing.T) { + tests := []struct { + desc string + proxyURL string + expectErr bool + }{ + { + desc: "no proxy-url", + }, + { + desc: "socks5 proxy-url", + proxyURL: "socks5://example.com", + }, + { + desc: "https proxy-url", + proxyURL: "https://example.com", + }, + { + desc: "http proxy-url", + proxyURL: "http://example.com", + }, + { + desc: "bad scheme proxy-url", + proxyURL: "socks6://example.com", + expectErr: true, + }, + { + desc: "no scheme proxy-url", + proxyURL: "example.com", + expectErr: true, + }, + { + desc: "not a url proxy-url", + proxyURL: "chewbacca@example.com", + expectErr: true, + }, + } + + for _, test := range tests { + t.Run(test.proxyURL, func(t *testing.T) { + + config := clientcmdapi.NewConfig() + config.Clusters["clean"] = &clientcmdapi.Cluster{ + Server: "https://localhost:8443", + ProxyURL: test.proxyURL, + } + config.AuthInfos["clean"] = &clientcmdapi.AuthInfo{} + config.Contexts["clean"] = &clientcmdapi.Context{ + Cluster: "clean", + AuthInfo: "clean", + } + config.CurrentContext = "clean" + + clientBuilder := NewNonInteractiveClientConfig(*config, "clean", &ConfigOverrides{}, nil) + + clientConfig, err := clientBuilder.ClientConfig() + if test.expectErr { + if err == nil { + t.Fatal("Expected error constructing config") + } + return + } + if err != nil { + t.Fatalf("Unexpected error constructing config: %v", err) + } + + if test.proxyURL == "" { + return + } + gotURL, err := clientConfig.Proxy(nil) + if err != nil { + t.Fatalf("Unexpected error from proxier: %v", err) + } + matchStringArg(test.proxyURL, gotURL.String(), t) + }) + } +} + func TestBasicAuthData(t *testing.T) { username := "myuser" password := "mypass" // Fake value for testing. diff --git a/staging/src/k8s.io/client-go/tools/clientcmd/helpers.go b/staging/src/k8s.io/client-go/tools/clientcmd/helpers.go index b609d1a766c..d7572232aae 100644 --- a/staging/src/k8s.io/client-go/tools/clientcmd/helpers.go +++ b/staging/src/k8s.io/client-go/tools/clientcmd/helpers.go @@ -18,6 +18,7 @@ package clientcmd import ( "fmt" + "net/url" "strconv" "time" ) @@ -33,3 +34,17 @@ func ParseTimeout(duration string) (time.Duration, error) { } return 0, fmt.Errorf("Invalid timeout value. Timeout must be a single integer in seconds, or an integer followed by a corresponding time unit (e.g. 1s | 2m | 3h)") } + +func parseProxyURL(proxyURL string) (*url.URL, error) { + u, err := url.Parse(proxyURL) + if err != nil { + return nil, fmt.Errorf("could not parse: %v", proxyURL) + } + + switch u.Scheme { + case "http", "https", "socks5": + default: + return nil, fmt.Errorf("unsupported scheme %q, must be http, https, or socks5", u.Scheme) + } + return u, nil +} diff --git a/staging/src/k8s.io/client-go/tools/clientcmd/validation.go b/staging/src/k8s.io/client-go/tools/clientcmd/validation.go index a480bdf0d6b..f77ef04fe54 100644 --- a/staging/src/k8s.io/client-go/tools/clientcmd/validation.go +++ b/staging/src/k8s.io/client-go/tools/clientcmd/validation.go @@ -227,6 +227,11 @@ func validateClusterInfo(clusterName string, clusterInfo clientcmdapi.Cluster) [ validationErrors = append(validationErrors, fmt.Errorf("no server found for cluster %q", clusterName)) } } + if proxyURL := clusterInfo.ProxyURL; proxyURL != "" { + if _, err := parseProxyURL(proxyURL); err != nil { + validationErrors = append(validationErrors, fmt.Errorf("invalid 'proxy-url' %q for cluster %q: %v", proxyURL, clusterName, err)) + } + } // Make sure CA data and CA file aren't both specified if len(clusterInfo.CertificateAuthority) != 0 && len(clusterInfo.CertificateAuthorityData) != 0 { validationErrors = append(validationErrors, fmt.Errorf("certificate-authority-data and certificate-authority are both specified for %v. certificate-authority-data will override.", clusterName)) diff --git a/staging/src/k8s.io/client-go/transport/cache.go b/staging/src/k8s.io/client-go/transport/cache.go index 36d6500f581..3ec4e19357d 100644 --- a/staging/src/k8s.io/client-go/transport/cache.go +++ b/staging/src/k8s.io/client-go/transport/cache.go @@ -52,6 +52,7 @@ type tlsCacheKey struct { nextProtos string dial string disableCompression bool + proxy string } func (t tlsCacheKey) String() string { @@ -59,7 +60,7 @@ func (t tlsCacheKey) String() string { if len(t.keyData) > 0 { keyText = "" } - return fmt.Sprintf("insecure:%v, caData:%#v, certData:%#v, keyData:%s, getCert: %s, serverName:%s, dial:%s disableCompression:%t", t.insecure, t.caData, t.certData, keyText, t.getCert, t.serverName, t.dial, t.disableCompression) + return fmt.Sprintf("insecure:%v, caData:%#v, certData:%#v, keyData:%s, getCert: %s, serverName:%s, dial:%s disableCompression:%t, proxy: %s", t.insecure, t.caData, t.certData, keyText, t.getCert, t.serverName, t.dial, t.disableCompression, t.proxy) } func (c *tlsTransportCache) get(config *Config) (http.RoundTripper, error) { @@ -83,7 +84,7 @@ func (c *tlsTransportCache) get(config *Config) (http.RoundTripper, error) { return nil, err } // The options didn't require a custom TLS config - if tlsConfig == nil && config.Dial == nil { + if tlsConfig == nil && config.Dial == nil && config.Proxy == nil { return http.DefaultTransport, nil } @@ -104,9 +105,14 @@ func (c *tlsTransportCache) get(config *Config) (http.RoundTripper, error) { go dynamicCertDialer.Run(wait.NeverStop) } + proxy := http.ProxyFromEnvironment + if config.Proxy != nil { + proxy = config.Proxy + } + // Cache a single transport for these options c.transports[key] = utilnet.SetTransportDefaults(&http.Transport{ - Proxy: http.ProxyFromEnvironment, + Proxy: proxy, TLSHandshakeTimeout: 10 * time.Second, TLSClientConfig: tlsConfig, MaxIdleConnsPerHost: idleConnsPerHost, @@ -130,6 +136,7 @@ func tlsConfigKey(c *Config) (tlsCacheKey, error) { nextProtos: strings.Join(c.TLS.NextProtos, ","), dial: fmt.Sprintf("%p", c.Dial), disableCompression: c.DisableCompression, + proxy: fmt.Sprintf("%p", c.Proxy), } if c.TLS.ReloadTLSFiles { diff --git a/staging/src/k8s.io/client-go/transport/cache_test.go b/staging/src/k8s.io/client-go/transport/cache_test.go index 8b9779e68d8..11ee6253575 100644 --- a/staging/src/k8s.io/client-go/transport/cache_test.go +++ b/staging/src/k8s.io/client-go/transport/cache_test.go @@ -21,6 +21,7 @@ import ( "crypto/tls" "net" "net/http" + "net/url" "testing" ) @@ -157,3 +158,23 @@ func TestTLSConfigKey(t *testing.T) { } } } + +func TestTLSConfigKeyFuncPtr(t *testing.T) { + keys := make(map[tlsCacheKey]struct{}) + makeKey := func(p func(*http.Request) (*url.URL, error)) tlsCacheKey { + key, err := tlsConfigKey(&Config{Proxy: p}) + if err != nil { + t.Fatalf("Unexpected error creating cache key: %v", err) + } + return key + } + + keys[makeKey(http.ProxyFromEnvironment)] = struct{}{} + keys[makeKey(http.ProxyFromEnvironment)] = struct{}{} + keys[makeKey(http.ProxyURL(nil))] = struct{}{} + keys[makeKey(nil)] = struct{}{} + + if got, want := len(keys), 3; got != want { + t.Fatalf("Unexpected number of keys: got=%d want=%d", got, want) + } +} diff --git a/staging/src/k8s.io/client-go/transport/config.go b/staging/src/k8s.io/client-go/transport/config.go index c20a4a8fcb9..45db2486471 100644 --- a/staging/src/k8s.io/client-go/transport/config.go +++ b/staging/src/k8s.io/client-go/transport/config.go @@ -21,6 +21,7 @@ import ( "crypto/tls" "net" "net/http" + "net/url" ) // Config holds various options for establishing a transport. @@ -68,6 +69,13 @@ type Config struct { // Dial specifies the dial function for creating unencrypted TCP connections. Dial func(ctx context.Context, network, address string) (net.Conn, error) + + // Proxy is the the proxy func to be used for all requests made by this + // transport. If Proxy is nil, http.ProxyFromEnvironment is used. If Proxy + // returns a nil *URL, no proxy is used. + // + // socks5 proxying does not currently support spdy streaming endpoints. + Proxy func(*http.Request) (*url.URL, error) } // ImpersonationConfig has all the available impersonation options From 652a48d2e7af76783cf08b5b1922da925e3d2a46 Mon Sep 17 00:00:00 2001 From: Mike Danese Date: Wed, 6 May 2020 10:51:52 -0700 Subject: [PATCH 2/2] wire up proxier in spdy transport and delete useless function --- pkg/kubelet/server/server_test.go | 2 +- .../pkg/util/httpstream/spdy/roundtripper.go | 21 ++++++++----------- .../util/httpstream/spdy/roundtripper_test.go | 4 ++-- .../k8s.io/client-go/transport/spdy/spdy.go | 6 +++++- 4 files changed, 17 insertions(+), 16 deletions(-) diff --git a/pkg/kubelet/server/server_test.go b/pkg/kubelet/server/server_test.go index 96dce2c86a9..b5aecbc685a 100644 --- a/pkg/kubelet/server/server_test.go +++ b/pkg/kubelet/server/server_test.go @@ -1034,7 +1034,7 @@ func TestServeExecInContainerIdleTimeout(t *testing.T) { url := fw.testHTTPServer.URL + "/exec/" + podNamespace + "/" + podName + "/" + expectedContainerName + "?c=ls&c=-a&" + api.ExecStdinParam + "=1" - upgradeRoundTripper := spdy.NewSpdyRoundTripper(nil, true, true) + upgradeRoundTripper := spdy.NewRoundTripper(nil, true, true) c := &http.Client{Transport: upgradeRoundTripper} resp, err := c.Do(makeReq(t, "POST", url, "v4.channel.k8s.io")) diff --git a/staging/src/k8s.io/apimachinery/pkg/util/httpstream/spdy/roundtripper.go b/staging/src/k8s.io/apimachinery/pkg/util/httpstream/spdy/roundtripper.go index 2699597e7a5..6309fbc26bb 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/httpstream/spdy/roundtripper.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/httpstream/spdy/roundtripper.go @@ -76,19 +76,20 @@ var _ utilnet.TLSClientConfigHolder = &SpdyRoundTripper{} var _ httpstream.UpgradeRoundTripper = &SpdyRoundTripper{} var _ utilnet.Dialer = &SpdyRoundTripper{} -// NewRoundTripper creates a new SpdyRoundTripper that will use -// the specified tlsConfig. -func NewRoundTripper(tlsConfig *tls.Config, followRedirects, requireSameHostRedirects bool) httpstream.UpgradeRoundTripper { - return NewSpdyRoundTripper(tlsConfig, followRedirects, requireSameHostRedirects) +// NewRoundTripper creates a new SpdyRoundTripper that will use the specified +// tlsConfig. +func NewRoundTripper(tlsConfig *tls.Config, followRedirects, requireSameHostRedirects bool) *SpdyRoundTripper { + return NewRoundTripperWithProxy(tlsConfig, followRedirects, requireSameHostRedirects, utilnet.NewProxierWithNoProxyCIDR(http.ProxyFromEnvironment)) } -// NewSpdyRoundTripper creates a new SpdyRoundTripper that will use -// the specified tlsConfig. This function is mostly meant for unit tests. -func NewSpdyRoundTripper(tlsConfig *tls.Config, followRedirects, requireSameHostRedirects bool) *SpdyRoundTripper { +// NewRoundTripperWithProxy creates a new SpdyRoundTripper that will use the +// specified tlsConfig and proxy func. +func NewRoundTripperWithProxy(tlsConfig *tls.Config, followRedirects, requireSameHostRedirects bool, proxier func(*http.Request) (*url.URL, error)) *SpdyRoundTripper { return &SpdyRoundTripper{ tlsConfig: tlsConfig, followRedirects: followRedirects, requireSameHostRedirects: requireSameHostRedirects, + proxier: proxier, } } @@ -116,11 +117,7 @@ func (s *SpdyRoundTripper) Dial(req *http.Request) (net.Conn, error) { // dial dials the host specified by req, using TLS if appropriate, optionally // using a proxy server if one is configured via environment variables. func (s *SpdyRoundTripper) dial(req *http.Request) (net.Conn, error) { - proxier := s.proxier - if proxier == nil { - proxier = utilnet.NewProxierWithNoProxyCIDR(http.ProxyFromEnvironment) - } - proxyURL, err := proxier(req) + proxyURL, err := s.proxier(req) if err != nil { return nil, err } diff --git a/staging/src/k8s.io/apimachinery/pkg/util/httpstream/spdy/roundtripper_test.go b/staging/src/k8s.io/apimachinery/pkg/util/httpstream/spdy/roundtripper_test.go index 11c6966d683..caca6670d96 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/httpstream/spdy/roundtripper_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/httpstream/spdy/roundtripper_test.go @@ -282,7 +282,7 @@ func TestRoundTripAndNewConnection(t *testing.T) { t.Fatalf("%s: Error creating request: %s", k, err) } - spdyTransport := NewSpdyRoundTripper(testCase.clientTLS, redirect, redirect) + spdyTransport := NewRoundTripper(testCase.clientTLS, redirect, redirect) var proxierCalled bool var proxyCalledWithHost string @@ -425,7 +425,7 @@ func TestRoundTripRedirects(t *testing.T) { t.Fatalf("Error creating request: %s", err) } - spdyTransport := NewSpdyRoundTripper(nil, true, true) + spdyTransport := NewRoundTripper(nil, true, true) client := &http.Client{Transport: spdyTransport} resp, err := client.Do(req) diff --git a/staging/src/k8s.io/client-go/transport/spdy/spdy.go b/staging/src/k8s.io/client-go/transport/spdy/spdy.go index 53cc7ee18c5..682f964f6f4 100644 --- a/staging/src/k8s.io/client-go/transport/spdy/spdy.go +++ b/staging/src/k8s.io/client-go/transport/spdy/spdy.go @@ -38,7 +38,11 @@ func RoundTripperFor(config *restclient.Config) (http.RoundTripper, Upgrader, er if err != nil { return nil, nil, err } - upgradeRoundTripper := spdy.NewRoundTripper(tlsConfig, true, false) + proxy := http.ProxyFromEnvironment + if config.Proxy != nil { + proxy = config.Proxy + } + upgradeRoundTripper := spdy.NewRoundTripperWithProxy(tlsConfig, true, false, proxy) wrapper, err := restclient.HTTPWrappersForConfig(config, upgradeRoundTripper) if err != nil { return nil, nil, err