Merge pull request #52792 from kad/warn-cidrs

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Warn user if Pod/Service networks will be accessed via proxy.

**What this PR does / why we need it**:
In environments where HTTP proxies are used, it is important
to whitelist Pod and Services network ranges in the NO_PROXY
variable, so cluster will be properly operational.

**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #

**Special notes for your reviewer**:

**Release note**:
```release-note
- kubeadm  will warn users if access to IP ranges for Pods or Services will be done via HTTP proxy.
```

cmd/kubeadm/app/preflight/BUILD

@@ -20,6 +20,7 @@ go_library(
         "//cmd/kubeadm/app/constants:go_default_library",
         "//pkg/api/validation:go_default_library",
         "//pkg/kubeapiserver/authorizer/modes:go_default_library",
+        "//pkg/registry/core/service/ipallocator:go_default_library",
         "//pkg/util/initsystem:go_default_library",
         "//pkg/util/version:go_default_library",
         "//pkg/version:go_default_library",
@@ -28,6 +29,7 @@ go_library(
         "//vendor/github.com/PuerkitoBio/purell:go_default_library",
         "//vendor/github.com/blang/semver:go_default_library",
         "//vendor/github.com/spf13/pflag:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/util/net:go_default_library",
     ],
 )
 

cmd/kubeadm/app/preflight/checks.go

@@ -40,12 +40,14 @@ import (
 
 	"net/url"
 
+	netutil "k8s.io/apimachinery/pkg/util/net"
 	apiservoptions "k8s.io/kubernetes/cmd/kube-apiserver/app/options"
 	cmoptions "k8s.io/kubernetes/cmd/kube-controller-manager/app/options"
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
 	kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants"
 	"k8s.io/kubernetes/pkg/api/validation"
 	authzmodes "k8s.io/kubernetes/pkg/kubeapiserver/authorizer/modes"
+	"k8s.io/kubernetes/pkg/registry/core/service/ipallocator"
 	"k8s.io/kubernetes/pkg/util/initsystem"
 	versionutil "k8s.io/kubernetes/pkg/util/version"
 	kubeadmversion "k8s.io/kubernetes/pkg/version"
@@ -333,6 +335,56 @@ func (hst HTTPProxyCheck) Check() (warnings, errors []error) {
 	return nil, nil
 }
 
+// HTTPProxyCIDRCheck checks if https connection to specific subnet is going
+// to be done directly or over proxy. If proxy detected, it will return warning.
+// Similar to HTTPProxyCheck above, but operates with subnets and uses API
+// machinery transport defaults to simulate kube-apiserver accessing cluster
+// services and pods.
+type HTTPProxyCIDRCheck struct {
+	Proto string
+	CIDR  string
+}
+
+// Check validates http connectivity to first IP address in the CIDR.
+// If it is not directly connected and goes via proxy it will produce warning.
+func (subnet HTTPProxyCIDRCheck) Check() (warnings, errors []error) {
+
+	if len(subnet.CIDR) == 0 {
+		return nil, nil
+	}
+
+	_, cidr, err := net.ParseCIDR(subnet.CIDR)
+	if err != nil {
+		return nil, []error{fmt.Errorf("error parsing CIDR %q: %v", subnet.CIDR, err)}
+	}
+
+	testIP, err := ipallocator.GetIndexedIP(cidr, 1)
+	if err != nil {
+		return nil, []error{fmt.Errorf("unable to get first IP address from the given CIDR (%s): %v", cidr.String(), err)}
+	}
+
+	testIPstring := testIP.String()
+	if len(testIP) == net.IPv6len {
+		testIPstring = fmt.Sprintf("[%s]:1234", testIP)
+	}
+	url := fmt.Sprintf("%s://%s/", subnet.Proto, testIPstring)
+
+	req, err := http.NewRequest("GET", url, nil)
+	if err != nil {
+		return nil, []error{err}
+	}
+
+	// Utilize same transport defaults as it will be used by API server
+	proxy, err := netutil.SetOldTransportDefaults(&http.Transport{}).Proxy(req)
+	if err != nil {
+		return nil, []error{err}
+	}
+	if proxy != nil {
+		return []error{fmt.Errorf("connection to %q uses proxy %q. This may lead to malfunctional cluster setup. Make sure that Pod and Services IP ranges specified correctly as exceptions in proxy configuration", subnet.CIDR, proxy)}, nil
+	}
+	return nil, nil
+}
+
 // ExtraArgsCheck checks if arguments are valid.
 type ExtraArgsCheck struct {
 	APIServerExtraArgs         map[string]string
@@ -649,7 +701,6 @@ func RunInitMasterChecks(cfg *kubeadmapi.MasterConfiguration) error {
 		PortOpenCheck{port: 10250},
 		PortOpenCheck{port: 10251},
 		PortOpenCheck{port: 10252},
-		HTTPProxyCheck{Proto: "https", Host: cfg.API.AdvertiseAddress, Port: int(cfg.API.BindPort)},
 		DirAvailableCheck{Path: filepath.Join(kubeadmconstants.KubernetesDir, kubeadmconstants.ManifestsSubDirName)},
 		FileContentCheck{Path: bridgenf, Content: []byte{'1'}},
 		SwapCheck{},
@@ -667,6 +718,9 @@ func RunInitMasterChecks(cfg *kubeadmapi.MasterConfiguration) error {
 			ControllerManagerExtraArgs: cfg.ControllerManagerExtraArgs,
 			SchedulerExtraArgs:         cfg.SchedulerExtraArgs,
 		},
+		HTTPProxyCheck{Proto: "https", Host: cfg.API.AdvertiseAddress, Port: int(cfg.API.BindPort)},
+		HTTPProxyCIDRCheck{Proto: "https", CIDR: cfg.Networking.ServiceSubnet},
+		HTTPProxyCIDRCheck{Proto: "https", CIDR: cfg.Networking.PodSubnet},
 	}
 
 	if len(cfg.Etcd.Endpoints) == 0 {

cmd/kubeadm/app/preflight/checks_test.go

@@ -20,10 +20,12 @@ import (
 	"bytes"
 	"fmt"
 	"io/ioutil"
+	"strings"
 	"testing"
 
 	"github.com/renstrom/dedent"
 
+	"net/http"
 	"os"
 
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
@@ -450,3 +452,107 @@ func TestKubernetesVersionCheck(t *testing.T) {
 		}
 	}
 }
+
+func TestHTTPProxyCIDRCheck(t *testing.T) {
+	var tests = []struct {
+		check          HTTPProxyCIDRCheck
+		expectWarnings bool
+	}{
+		{
+			check: HTTPProxyCIDRCheck{
+				Proto: "https",
+				CIDR:  "127.0.0.0/8",
+			}, // Loopback addresses never should produce proxy warnings
+			expectWarnings: false,
+		},
+		{
+			check: HTTPProxyCIDRCheck{
+				Proto: "https",
+				CIDR:  "10.96.0.0/12",
+			}, // Expected to be accessed directly, we set NO_PROXY to 10.0.0.0/8
+			expectWarnings: false,
+		},
+		{
+			check: HTTPProxyCIDRCheck{
+				Proto: "https",
+				CIDR:  "192.168.0.0/16",
+			}, // Expected to go via proxy as this range is not listed in NO_PROXY
+			expectWarnings: true,
+		},
+		{
+			check: HTTPProxyCIDRCheck{
+				Proto: "https",
+				CIDR:  "2001:db8::/56",
+			}, // Expected to be accessed directly, part of 2001:db8::/48 in NO_PROXY
+			expectWarnings: false,
+		},
+		{
+			check: HTTPProxyCIDRCheck{
+				Proto: "https",
+				CIDR:  "2001:db8:1::/56",
+			}, // Expected to go via proxy, range is not in 2001:db8::/48
+			expectWarnings: true,
+		},
+	}
+
+	// Save current content of *_proxy and *_PROXY variables.
+	savedEnv := resetProxyEnv()
+	defer restoreEnv(savedEnv)
+	t.Log("Saved environment: ", savedEnv)
+
+	os.Setenv("HTTP_PROXY", "http://proxy.example.com:3128")
+	os.Setenv("NO_PROXY", "example.com,10.0.0.0/8,2001:db8::/48")
+
+	// Check if we can reliably execute tests:
+	// ProxyFromEnvironment caches the *_proxy environment variables and
+	// if ProxyFromEnvironment already executed before our test with empty
+	// HTTP_PROXY it will make these tests return false positive failures
+	req, err := http.NewRequest("GET", "http://host.fake.tld/", nil)
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	proxy, err := http.ProxyFromEnvironment(req)
+	if err != nil {
+		t.Fatalf("unexpected err: %v", err)
+	}
+	if proxy == nil {
+		t.Skip("test skipped as ProxyFromEnvironment already initialized in environment without defined HTTP proxy")
+	}
+	t.Log("http.ProxyFromEnvironment is usable, continue executing test")
+
+	for _, rt := range tests {
+		warning, _ := rt.check.Check()
+		if (warning != nil) != rt.expectWarnings {
+			t.Errorf(
+				"failed HTTPProxyCIDRCheck:\n\texpected: %t\n\t  actual: %t (CIDR:%s). Warnings: %v",
+				rt.expectWarnings,
+				(warning != nil),
+				rt.check.CIDR,
+				warning,
+			)
+		}
+	}
+}
+
+// resetProxyEnv is helper function that unsets all *_proxy variables
+// and return previously set values as map. This can be used to restore
+// original state of the environment.
+func resetProxyEnv() map[string]string {
+	savedEnv := make(map[string]string)
+	for _, e := range os.Environ() {
+		pair := strings.Split(e, "=")
+		if strings.HasSuffix(strings.ToLower(pair[0]), "_proxy") {
+			savedEnv[pair[0]] = pair[1]
+			os.Unsetenv(pair[0])
+		}
+	}
+	return savedEnv
+}
+
+// restoreEnv is helper function to restores values
+// of environment variables from saved state in the map
+func restoreEnv(e map[string]string) {
+	for k, v := range e {
+		os.Setenv(k, v)
+	}
+}