Skip to content

Commit

Permalink
Merge pull request #52792 from kad/warn-cidrs
Browse files Browse the repository at this point in the history 
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.
```
  • Loading branch information
Kubernetes Submit Queue committed Oct 21, 2017
2 parents 668bedd + 1ed7692 commit abfaada
Show file tree
Hide file tree
Showing 3 changed files with 163 additions and 1 deletion.
2 changes: 2 additions & 0 deletions cmd/kubeadm/app/preflight/BUILD
View file
Expand Up @@ -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",
Expand All @@ -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",
],
)

Expand Down
56 changes: 55 additions & 1 deletion cmd/kubeadm/app/preflight/checks.go
View file
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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{},
Expand All @@ -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 {
Expand Down
106 changes: 106 additions & 0 deletions cmd/kubeadm/app/preflight/checks_test.go
View file
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}
}

0 comments on commit abfaada

Please sign in to comment.