From ab03317d96504486b51785a7f4b7901d908d530d Mon Sep 17 00:00:00 2001 From: deads2k Date: Tue, 15 Mar 2016 15:29:08 -0400 Subject: [PATCH] support CIDRs in NO_PROXY --- pkg/apiserver/proxy_test.go | 7 +- pkg/client/transport/cache.go | 6 +- .../providers/mesos/client_test.go | 6 +- pkg/credentialprovider/gcp/metadata_test.go | 25 +++---- pkg/probe/http/http.go | 3 +- pkg/registry/generic/rest/proxy_test.go | 7 +- pkg/storage/etcd/etcd_helper.go | 5 +- pkg/util/net/http.go | 60 ++++++++++++++++- pkg/util/net/http_test.go | 66 +++++++++++++++++++ plugin/pkg/scheduler/extender.go | 7 +- test/e2e/ingress_utils.go | 5 +- test/e2e/kubectl.go | 7 +- test/e2e/kubeproxy.go | 3 +- test/e2e/service.go | 4 +- test/images/netexec/netexec.go | 4 +- 15 files changed, 176 insertions(+), 39 deletions(-) diff --git a/pkg/apiserver/proxy_test.go b/pkg/apiserver/proxy_test.go index 8a655797fc..01dae4f0ec 100644 --- a/pkg/apiserver/proxy_test.go +++ b/pkg/apiserver/proxy_test.go @@ -36,6 +36,7 @@ import ( "golang.org/x/net/websocket" "k8s.io/kubernetes/pkg/api/rest" + utilnet "k8s.io/kubernetes/pkg/util/net" ) func TestProxyRequestContentLengthAndTransferEncoding(t *testing.T) { @@ -381,7 +382,7 @@ func TestProxyUpgrade(t *testing.T) { ts.StartTLS() return ts }, - ProxyTransport: &http.Transport{TLSClientConfig: &tls.Config{InsecureSkipVerify: true}}, + ProxyTransport: utilnet.SetTransportDefaults(&http.Transport{TLSClientConfig: &tls.Config{InsecureSkipVerify: true}}), }, "https (valid hostname + RootCAs)": { ServerFunc: func(h http.Handler) *httptest.Server { @@ -396,7 +397,7 @@ func TestProxyUpgrade(t *testing.T) { ts.StartTLS() return ts }, - ProxyTransport: &http.Transport{TLSClientConfig: &tls.Config{RootCAs: localhostPool}}, + ProxyTransport: utilnet.SetTransportDefaults(&http.Transport{TLSClientConfig: &tls.Config{RootCAs: localhostPool}}), }, "https (valid hostname + RootCAs + custom dialer)": { ServerFunc: func(h http.Handler) *httptest.Server { @@ -411,7 +412,7 @@ func TestProxyUpgrade(t *testing.T) { ts.StartTLS() return ts }, - ProxyTransport: &http.Transport{Dial: net.Dial, TLSClientConfig: &tls.Config{RootCAs: localhostPool}}, + ProxyTransport: utilnet.SetTransportDefaults(&http.Transport{Dial: net.Dial, TLSClientConfig: &tls.Config{RootCAs: localhostPool}}), }, } diff --git a/pkg/client/transport/cache.go b/pkg/client/transport/cache.go index f1068930d8..90bd119029 100644 --- a/pkg/client/transport/cache.go +++ b/pkg/client/transport/cache.go @@ -22,6 +22,8 @@ import ( "net/http" "sync" "time" + + utilnet "k8s.io/kubernetes/pkg/util/net" ) // TlsTransportCache caches TLS http.RoundTrippers different configurations. The @@ -60,7 +62,7 @@ func (c *tlsTransportCache) get(config *Config) (http.RoundTripper, error) { } // Cache a single transport for these options - c.transports[key] = &http.Transport{ + c.transports[key] = utilnet.SetTransportDefaults(&http.Transport{ Proxy: http.ProxyFromEnvironment, TLSHandshakeTimeout: 10 * time.Second, TLSClientConfig: tlsConfig, @@ -68,7 +70,7 @@ func (c *tlsTransportCache) get(config *Config) (http.RoundTripper, error) { Timeout: 30 * time.Second, KeepAlive: 30 * time.Second, }).Dial, - } + }) return c.transports[key], nil } diff --git a/pkg/cloudprovider/providers/mesos/client_test.go b/pkg/cloudprovider/providers/mesos/client_test.go index 83209924be..b92fbaf9f8 100644 --- a/pkg/cloudprovider/providers/mesos/client_test.go +++ b/pkg/cloudprovider/providers/mesos/client_test.go @@ -29,6 +29,8 @@ import ( "github.com/mesos/mesos-go/detector" "github.com/mesos/mesos-go/mesosutil" "golang.org/x/net/context" + + utilnet "k8s.io/kubernetes/pkg/util/net" ) // Test data @@ -180,11 +182,11 @@ func makeHttpMocks() (*httptest.Server, *http.Client, *http.Transport) { })) // Intercept all client requests and feed them to the test server - transport := &http.Transport{ + transport := utilnet.SetTransportDefaults(&http.Transport{ Proxy: func(req *http.Request) (*url.URL, error) { return url.Parse(httpServer.URL) }, - } + }) httpClient := &http.Client{Transport: transport} diff --git a/pkg/credentialprovider/gcp/metadata_test.go b/pkg/credentialprovider/gcp/metadata_test.go index 5da1bcd3b9..c2e275f1f5 100644 --- a/pkg/credentialprovider/gcp/metadata_test.go +++ b/pkg/credentialprovider/gcp/metadata_test.go @@ -28,6 +28,7 @@ import ( "testing" "k8s.io/kubernetes/pkg/credentialprovider" + utilnet "k8s.io/kubernetes/pkg/util/net" ) func TestDockerKeyringFromGoogleDockerConfigMetadata(t *testing.T) { @@ -60,11 +61,11 @@ func TestDockerKeyringFromGoogleDockerConfigMetadata(t *testing.T) { // defer server.Close() // Make a transport that reroutes all traffic to the example server - transport := &http.Transport{ + transport := utilnet.SetTransportDefaults(&http.Transport{ Proxy: func(req *http.Request) (*url.URL, error) { return url.Parse(server.URL + req.URL.Path) }, - } + }) keyring := &credentialprovider.BasicDockerKeyring{} provider := &dockerConfigKeyProvider{ @@ -133,11 +134,11 @@ func TestDockerKeyringFromGoogleDockerConfigMetadataUrl(t *testing.T) { // defer server.Close() // Make a transport that reroutes all traffic to the example server - transport := &http.Transport{ + transport := utilnet.SetTransportDefaults(&http.Transport{ Proxy: func(req *http.Request) (*url.URL, error) { return url.Parse(server.URL + req.URL.Path) }, - } + }) keyring := &credentialprovider.BasicDockerKeyring{} provider := &dockerConfigUrlKeyProvider{ @@ -207,11 +208,11 @@ func TestContainerRegistryBasics(t *testing.T) { // defer server.Close() // Make a transport that reroutes all traffic to the example server - transport := &http.Transport{ + transport := utilnet.SetTransportDefaults(&http.Transport{ Proxy: func(req *http.Request) (*url.URL, error) { return url.Parse(server.URL + req.URL.Path) }, - } + }) keyring := &credentialprovider.BasicDockerKeyring{} provider := &containerRegistryProvider{ @@ -264,11 +265,11 @@ func TestContainerRegistryNoStorageScope(t *testing.T) { // defer server.Close() // Make a transport that reroutes all traffic to the example server - transport := &http.Transport{ + transport := utilnet.SetTransportDefaults(&http.Transport{ Proxy: func(req *http.Request) (*url.URL, error) { return url.Parse(server.URL + req.URL.Path) }, - } + }) provider := &containerRegistryProvider{ metadataProvider{Client: &http.Client{Transport: transport}}, @@ -298,11 +299,11 @@ func TestComputePlatformScopeSubstitutesStorageScope(t *testing.T) { // defer server.Close() // Make a transport that reroutes all traffic to the example server - transport := &http.Transport{ + transport := utilnet.SetTransportDefaults(&http.Transport{ Proxy: func(req *http.Request) (*url.URL, error) { return url.Parse(server.URL + req.URL.Path) }, - } + }) provider := &containerRegistryProvider{ metadataProvider{Client: &http.Client{Transport: transport}}, @@ -321,11 +322,11 @@ func TestAllProvidersNoMetadata(t *testing.T) { // defer server.Close() // Make a transport that reroutes all traffic to the example server - transport := &http.Transport{ + transport := utilnet.SetTransportDefaults(&http.Transport{ Proxy: func(req *http.Request) (*url.URL, error) { return url.Parse(server.URL + req.URL.Path) }, - } + }) providers := []credentialprovider.DockerConfigProvider{ &dockerConfigKeyProvider{ diff --git a/pkg/probe/http/http.go b/pkg/probe/http/http.go index e9673b4568..9e8b0e1168 100644 --- a/pkg/probe/http/http.go +++ b/pkg/probe/http/http.go @@ -25,13 +25,14 @@ import ( "time" "k8s.io/kubernetes/pkg/probe" + utilnet "k8s.io/kubernetes/pkg/util/net" "github.com/golang/glog" ) func New() HTTPProber { tlsConfig := &tls.Config{InsecureSkipVerify: true} - transport := &http.Transport{TLSClientConfig: tlsConfig, DisableKeepAlives: true} + transport := utilnet.SetTransportDefaults(&http.Transport{TLSClientConfig: tlsConfig, DisableKeepAlives: true}) return httpProber{transport} } diff --git a/pkg/registry/generic/rest/proxy_test.go b/pkg/registry/generic/rest/proxy_test.go index 5e01461797..1386c8826e 100644 --- a/pkg/registry/generic/rest/proxy_test.go +++ b/pkg/registry/generic/rest/proxy_test.go @@ -36,6 +36,7 @@ import ( "golang.org/x/net/websocket" + utilnet "k8s.io/kubernetes/pkg/util/net" "k8s.io/kubernetes/pkg/util/proxy" ) @@ -334,7 +335,7 @@ func TestProxyUpgrade(t *testing.T) { ts.StartTLS() return ts }, - ProxyTransport: &http.Transport{TLSClientConfig: &tls.Config{InsecureSkipVerify: true}}, + ProxyTransport: utilnet.SetTransportDefaults(&http.Transport{TLSClientConfig: &tls.Config{InsecureSkipVerify: true}}), }, "https (valid hostname + RootCAs)": { ServerFunc: func(h http.Handler) *httptest.Server { @@ -349,7 +350,7 @@ func TestProxyUpgrade(t *testing.T) { ts.StartTLS() return ts }, - ProxyTransport: &http.Transport{TLSClientConfig: &tls.Config{RootCAs: localhostPool}}, + ProxyTransport: utilnet.SetTransportDefaults(&http.Transport{TLSClientConfig: &tls.Config{RootCAs: localhostPool}}), }, "https (valid hostname + RootCAs + custom dialer)": { ServerFunc: func(h http.Handler) *httptest.Server { @@ -364,7 +365,7 @@ func TestProxyUpgrade(t *testing.T) { ts.StartTLS() return ts }, - ProxyTransport: &http.Transport{Dial: net.Dial, TLSClientConfig: &tls.Config{RootCAs: localhostPool}}, + ProxyTransport: utilnet.SetTransportDefaults(&http.Transport{Dial: net.Dial, TLSClientConfig: &tls.Config{RootCAs: localhostPool}}), }, } diff --git a/pkg/storage/etcd/etcd_helper.go b/pkg/storage/etcd/etcd_helper.go index 5edcb83c43..896baf63f7 100644 --- a/pkg/storage/etcd/etcd_helper.go +++ b/pkg/storage/etcd/etcd_helper.go @@ -34,6 +34,7 @@ import ( "k8s.io/kubernetes/pkg/storage/etcd/metrics" etcdutil "k8s.io/kubernetes/pkg/storage/etcd/util" "k8s.io/kubernetes/pkg/util" + utilnet "k8s.io/kubernetes/pkg/util/net" "k8s.io/kubernetes/pkg/watch" etcd "github.com/coreos/etcd/client" @@ -102,7 +103,7 @@ func (c *EtcdConfig) newHttpTransport() (*http.Transport, error) { // Copied from etcd.DefaultTransport declaration. // TODO: Determine if transport needs optimization - tr := &http.Transport{ + tr := utilnet.SetTransportDefaults(&http.Transport{ Proxy: http.ProxyFromEnvironment, Dial: (&net.Dialer{ Timeout: 30 * time.Second, @@ -111,7 +112,7 @@ func (c *EtcdConfig) newHttpTransport() (*http.Transport, error) { TLSHandshakeTimeout: 10 * time.Second, MaxIdleConnsPerHost: 500, TLSClientConfig: cfg, - } + }) return tr, nil } diff --git a/pkg/util/net/http.go b/pkg/util/net/http.go index f3d4473f0a..a5000a8b14 100644 --- a/pkg/util/net/http.go +++ b/pkg/util/net/http.go @@ -23,6 +23,7 @@ import ( "net" "net/http" "net/url" + "os" "strconv" "strings" ) @@ -55,8 +56,10 @@ var defaultTransport = http.DefaultTransport.(*http.Transport) // SetTransportDefaults applies the defaults from http.DefaultTransport // for the Proxy, Dial, and TLSHandshakeTimeout fields if unset func SetTransportDefaults(t *http.Transport) *http.Transport { - if t.Proxy == nil { - t.Proxy = defaultTransport.Proxy + if t.Proxy == nil || isDefault(t.Proxy) { + // http.ProxyFromEnvironment doesn't respect CIDRs and that makes it impossible to exclude things like pod and service IPs from proxy settings + // ProxierWithNoProxyCIDR allows CIDR rules in NO_PROXY + t.Proxy = NewProxierWithNoProxyCIDR(http.ProxyFromEnvironment) } if t.Dial == nil { t.Dial = defaultTransport.Dial @@ -153,3 +156,56 @@ func GetClientIP(req *http.Request) net.IP { ip := net.ParseIP(req.RemoteAddr) return ip } + +var defaultProxyFuncPointer = fmt.Sprintf("%p", http.ProxyFromEnvironment) + +// isDefault checks to see if the transportProxierFunc is pointing to the default one +func isDefault(transportProxier func(*http.Request) (*url.URL, error)) bool { + transportProxierPointer := fmt.Sprintf("%p", transportProxier) + return transportProxierPointer == defaultProxyFuncPointer +} + +// NewProxierWithNoProxyCIDR constructs a Proxier function that respects CIDRs in NO_PROXY and delegates if +// no matching CIDRs are found +func NewProxierWithNoProxyCIDR(delegate func(req *http.Request) (*url.URL, error)) func(req *http.Request) (*url.URL, error) { + // we wrap the default method, so we only need to perform our check if the NO_PROXY envvar has a CIDR in it + noProxyEnv := os.Getenv("NO_PROXY") + noProxyRules := strings.Split(noProxyEnv, ",") + + cidrs := []*net.IPNet{} + for _, noProxyRule := range noProxyRules { + _, cidr, _ := net.ParseCIDR(noProxyRule) + if cidr != nil { + cidrs = append(cidrs, cidr) + } + } + + if len(cidrs) == 0 { + return delegate + } + + return func(req *http.Request) (*url.URL, error) { + host := req.URL.Host + // for some urls, the Host is already the host, not the host:port + if net.ParseIP(host) == nil { + var err error + host, _, err = net.SplitHostPort(req.URL.Host) + if err != nil { + return delegate(req) + } + } + + ip := net.ParseIP(host) + if ip == nil { + return delegate(req) + } + + for _, cidr := range cidrs { + if cidr.Contains(ip) { + return nil, nil + } + } + + return delegate(req) + } +} diff --git a/pkg/util/net/http_test.go b/pkg/util/net/http_test.go index 7990a51d11..1de67cadeb 100644 --- a/pkg/util/net/http_test.go +++ b/pkg/util/net/http_test.go @@ -19,6 +19,8 @@ package net import ( "net" "net/http" + "net/url" + "os" "reflect" "testing" ) @@ -100,3 +102,67 @@ func TestGetClientIP(t *testing.T) { } } } + +func TestProxierWithNoProxyCIDR(t *testing.T) { + testCases := []struct { + name string + noProxy string + url string + + expectedDelegated bool + }{ + { + name: "no env", + url: "https://192.168.143.1/api", + expectedDelegated: true, + }, + { + name: "no cidr", + noProxy: "192.168.63.1", + url: "https://192.168.143.1/api", + expectedDelegated: true, + }, + { + name: "hostname", + noProxy: "192.168.63.0/24,192.168.143.0/24", + url: "https://my-hostname/api", + expectedDelegated: true, + }, + { + name: "match second cidr", + noProxy: "192.168.63.0/24,192.168.143.0/24", + url: "https://192.168.143.1/api", + expectedDelegated: false, + }, + { + name: "match second cidr with host:port", + noProxy: "192.168.63.0/24,192.168.143.0/24", + url: "https://192.168.143.1:8443/api", + expectedDelegated: false, + }, + } + + for _, test := range testCases { + os.Setenv("NO_PROXY", test.noProxy) + actualDelegated := false + proxyFunc := NewProxierWithNoProxyCIDR(func(req *http.Request) (*url.URL, error) { + actualDelegated = true + return nil, nil + }) + + req, err := http.NewRequest("GET", test.url, nil) + if err != nil { + t.Errorf("%s: unexpected err: %v", test.name, err) + continue + } + if _, err := proxyFunc(req); err != nil { + t.Errorf("%s: unexpected err: %v", test.name, err) + continue + } + + if test.expectedDelegated != actualDelegated { + t.Errorf("%s: expected %v, got %v", test.name, test.expectedDelegated, actualDelegated) + continue + } + } +} diff --git a/plugin/pkg/scheduler/extender.go b/plugin/pkg/scheduler/extender.go index 8a54aaf407..30b403f36e 100644 --- a/plugin/pkg/scheduler/extender.go +++ b/plugin/pkg/scheduler/extender.go @@ -26,6 +26,7 @@ import ( "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/client/restclient" + utilnet "k8s.io/kubernetes/pkg/util/net" "k8s.io/kubernetes/plugin/pkg/scheduler/algorithm" schedulerapi "k8s.io/kubernetes/plugin/pkg/scheduler/api" ) @@ -60,11 +61,11 @@ func makeTransport(config *schedulerapi.ExtenderConfig) (http.RoundTripper, erro return nil, err } if tlsConfig != nil { - return &http.Transport{ + return utilnet.SetTransportDefaults(&http.Transport{ TLSClientConfig: tlsConfig, - }, nil + }), nil } - return http.DefaultTransport, nil + return utilnet.SetTransportDefaults(&http.Transport{}), nil } func NewHTTPExtender(config *schedulerapi.ExtenderConfig, apiVersion string) (algorithm.SchedulerExtender, error) { diff --git a/test/e2e/ingress_utils.go b/test/e2e/ingress_utils.go index 41952dfbc8..49fb3af6ff 100644 --- a/test/e2e/ingress_utils.go +++ b/test/e2e/ingress_utils.go @@ -35,6 +35,7 @@ import ( "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/apis/extensions" client "k8s.io/kubernetes/pkg/client/unversioned" + utilnet "k8s.io/kubernetes/pkg/util/net" ) const ( @@ -110,13 +111,13 @@ func buildTransport(serverName string, rootCA []byte) (*http.Transport, error) { if !ok { return nil, fmt.Errorf("Unable to load serverCA.") } - return &http.Transport{ + return utilnet.SetTransportDefaults(&http.Transport{ TLSClientConfig: &tls.Config{ InsecureSkipVerify: false, ServerName: serverName, RootCAs: pool, }, - }, nil + }), nil } // createSecret creates a secret containing TLS certificates for the given Ingress. diff --git a/test/e2e/kubectl.go b/test/e2e/kubectl.go index 7f2a13d9b1..9fcf631bd4 100644 --- a/test/e2e/kubectl.go +++ b/test/e2e/kubectl.go @@ -45,6 +45,7 @@ import ( "k8s.io/kubernetes/pkg/kubectl" "k8s.io/kubernetes/pkg/kubectl/cmd/util" "k8s.io/kubernetes/pkg/labels" + utilnet "k8s.io/kubernetes/pkg/util/net" "k8s.io/kubernetes/pkg/util/wait" "k8s.io/kubernetes/pkg/version" @@ -1203,9 +1204,9 @@ func curlUnix(url string, path string) (string, error) { dial := func(proto, addr string) (net.Conn, error) { return net.Dial("unix", path) } - transport := &http.Transport{ + transport := utilnet.SetTransportDefaults(&http.Transport{ Dial: dial, - } + }) return curlTransport(url, transport) } @@ -1224,7 +1225,7 @@ func curlTransport(url string, transport *http.Transport) (string, error) { } func curl(url string) (string, error) { - return curlTransport(url, &http.Transport{}) + return curlTransport(url, utilnet.SetTransportDefaults(&http.Transport{})) } func validateGuestbookApp(c *client.Client, ns string) { diff --git a/test/e2e/kubeproxy.go b/test/e2e/kubeproxy.go index 2a107317c2..a5df91687a 100644 --- a/test/e2e/kubeproxy.go +++ b/test/e2e/kubeproxy.go @@ -34,6 +34,7 @@ import ( "k8s.io/kubernetes/pkg/labels" "k8s.io/kubernetes/pkg/util" "k8s.io/kubernetes/pkg/util/intstr" + utilnet "k8s.io/kubernetes/pkg/util/net" "k8s.io/kubernetes/pkg/util/wait" ) @@ -129,7 +130,7 @@ func (config *KubeProxyTestConfig) hitLoadBalancer(epCount int) { hostNames := make(map[string]bool) tries := epCount*epCount + 5 for i := 0; i < tries; i++ { - transport := &http.Transport{} + transport := utilnet.SetTransportDefaults(&http.Transport{}) httpClient := createHTTPClient(transport) resp, err := httpClient.Get(fmt.Sprintf("http://%s:%d/hostName", lbIP, loadBalancerHttpPort)) if err == nil { diff --git a/test/e2e/service.go b/test/e2e/service.go index 36876d505a..c2dc6e71f3 100644 --- a/test/e2e/service.go +++ b/test/e2e/service.go @@ -1442,9 +1442,9 @@ func verifyServeHostnameServiceDown(c *client.Client, host string, serviceIP str // This masks problems where the iptables rule has changed, but we don't see it // This is intended for relatively quick requests (status checks), so we set a short (5 seconds) timeout func httpGetNoConnectionPool(url string) (*http.Response, error) { - tr := &http.Transport{ + tr := utilnet.SetTransportDefaults(&http.Transport{ DisableKeepAlives: true, - } + }) client := &http.Client{ Transport: tr, Timeout: 5 * time.Second, diff --git a/test/images/netexec/netexec.go b/test/images/netexec/netexec.go index 4289e6da19..fc58ec12b2 100644 --- a/test/images/netexec/netexec.go +++ b/test/images/netexec/netexec.go @@ -32,6 +32,8 @@ import ( "strings" "sync/atomic" "time" + + utilnet "k8s.io/kubernetes/pkg/util/net" ) var ( @@ -212,7 +214,7 @@ func dialHandler(w http.ResponseWriter, r *http.Request) { } func dialHTTP(request, hostPort string) (string, error) { - transport := &http.Transport{} + transport := utilnet.SetTransportDefaults(&http.Transport{}) httpClient := createHTTPClient(transport) resp, err := httpClient.Get(fmt.Sprintf("http://%s/%s", hostPort, request)) defer transport.CloseIdleConnections()