Merge pull request #23003 from deads2k/no-proxy-cidr

Auto commit by PR queue bot

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

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