Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support CIDRs in NO_PROXY #23003

Merged
merged 1 commit into from
Mar 17, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 4 additions & 3 deletions pkg/apiserver/proxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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}}),
},
}

Expand Down
6 changes: 4 additions & 2 deletions pkg/client/transport/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import (
"net/http"
"sync"
"time"

utilnet "k8s.io/kubernetes/pkg/util/net"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm in favor of this, but it was apparently contentious when this package was created. Would like agreement from @krousey and @lavalamp

)

// TlsTransportCache caches TLS http.RoundTrippers different configurations. The
Expand Down Expand Up @@ -60,15 +62,15 @@ 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,
Dial: (&net.Dialer{
Timeout: 30 * time.Second,
KeepAlive: 30 * time.Second,
}).Dial,
}
})
return c.transports[key], nil
}

Expand Down
6 changes: 4 additions & 2 deletions pkg/cloudprovider/providers/mesos/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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}

Expand Down
25 changes: 13 additions & 12 deletions pkg/credentialprovider/gcp/metadata_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"testing"

"k8s.io/kubernetes/pkg/credentialprovider"
utilnet "k8s.io/kubernetes/pkg/util/net"
)

func TestDockerKeyringFromGoogleDockerConfigMetadata(t *testing.T) {
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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}},
Expand Down Expand Up @@ -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}},
Expand All @@ -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{
Expand Down
3 changes: 2 additions & 1 deletion pkg/probe/http/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -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}
}

Expand Down
7 changes: 4 additions & 3 deletions pkg/registry/generic/rest/proxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import (

"golang.org/x/net/websocket"

utilnet "k8s.io/kubernetes/pkg/util/net"
"k8s.io/kubernetes/pkg/util/proxy"
)

Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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}}),
},
}

Expand Down
5 changes: 3 additions & 2 deletions pkg/storage/etcd/etcd_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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,
Expand All @@ -111,7 +112,7 @@ func (c *EtcdConfig) newHttpTransport() (*http.Transport, error) {
TLSHandshakeTimeout: 10 * time.Second,
MaxIdleConnsPerHost: 500,
TLSClientConfig: cfg,
}
})

return tr, nil
}
Expand Down
60 changes: 58 additions & 2 deletions pkg/util/net/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"net"
"net/http"
"net/url"
"os"
"strconv"
"strings"
)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if it's just an IP and not a CIDR?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, I see your comment above.

}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: maybe, if len(cidrs) == 0 { return delegate } might be a tad cleaner?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: maybe, if len(cidrs) == 0 { return delegate } might be a tad cleaner?

Updated.


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)
}
}