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

Allow minikube to function with misconfigured NO_PROXY value #4229

Merged
merged 28 commits into from
May 20, 2019
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
581b63a
Fixed API connectivity with http(s) proxy
medyagh May 7, 2019
0529f0b
Added more comment
medyagh May 8, 2019
8bc0010
Fix dashboard behind proxy and refactor
medyagh May 10, 2019
6a433d0
Fixing K8S Client Config with TranportWrap
medyagh May 15, 2019
ddb95e1
return error if nil
medyagh May 15, 2019
8120cdd
No Proxy for Dashboard and moving logic to proxy package
medyagh May 15, 2019
830e18d
Updated warn message and remove duplicate warning
medyagh May 15, 2019
6384a22
Fix naming and improve public functions singatures
medyagh May 15, 2019
6fe614e
Fix stutter between the package name and the function
medyagh May 15, 2019
b9149b1
Better name
medyagh May 15, 2019
e878f9b
Merge remote-tracking branch 'upstream/master' into no_proxy_for_self
medyagh May 15, 2019
1079933
Added integration tests for proxy
medyagh May 15, 2019
a0c6013
Fixed Error handling for http serve
medyagh May 15, 2019
b8501ea
Fix formatting
medyagh May 15, 2019
08d19bf
Improve formatting and messages
medyagh May 15, 2019
53a4625
Return host IP when using vmware as vm driver.
ne0h May 14, 2019
88a1489
Make derivation of Host IP address more generic when using vmware.
ne0h May 15, 2019
a2590ec
Adding more unit test
medyagh May 16, 2019
f38efb9
Improved Style
medyagh May 16, 2019
3ff806a
Added unittest and rebase
medyagh May 16, 2019
a1e34b0
Improve http server shutdown handling
medyagh May 16, 2019
6484548
fix formatting
medyagh May 16, 2019
0d58773
Clean up after unit test
medyagh May 17, 2019
d4cf3b0
Merge remote-tracking branch 'upstream/master' into no_proxy_for_self
medyagh May 17, 2019
42e40e3
Debug
medyagh May 19, 2019
871c2e6
Better clean up after tesdt proxy setup
medyagh May 19, 2019
c428ce5
Fix typo
medyagh May 19, 2019
b6bf1df
Improve test clean up
medyagh May 19, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion cmd/minikube/cmd/dashboard.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,11 @@ import (
configcmd "k8s.io/minikube/cmd/minikube/cmd/config"
"k8s.io/minikube/pkg/minikube/cluster"
"k8s.io/minikube/pkg/minikube/config"
pkg_config "k8s.io/minikube/pkg/minikube/config"
"k8s.io/minikube/pkg/minikube/console"
"k8s.io/minikube/pkg/minikube/exit"
"k8s.io/minikube/pkg/minikube/machine"
"k8s.io/minikube/pkg/minikube/proxy"
"k8s.io/minikube/pkg/minikube/service"
"k8s.io/minikube/pkg/util"
)
Expand All @@ -52,11 +54,13 @@ var dashboardCmd = &cobra.Command{
Short: "Access the kubernetes dashboard running within the minikube cluster",
Long: `Access the kubernetes dashboard running within the minikube cluster`,
Run: func(cmd *cobra.Command, args []string) {
cc, err := pkg_config.Load()
medyagh marked this conversation as resolved.
Show resolved Hide resolved
proxy.UpdateNoProxy(cc.KubernetesConfig.NodeIP)

kubectl, err := exec.LookPath("kubectl")
if err != nil {
exit.WithCode(exit.NoInput, "kubectl not found in PATH, but is required for the dashboard. Installation guide: https://kubernetes.io/docs/tasks/tools/install-kubectl/")
}

api, err := machine.NewAPIClient()
defer func() {
err := api.Close()
Expand Down Expand Up @@ -117,7 +121,9 @@ var dashboardCmd = &cobra.Command{
func kubectlProxy(path string) (*exec.Cmd, string, error) {
// port=0 picks a random system port
// config.GetMachineName() respects the -p (profile) flag

cmd := exec.Command(path, "--context", config.GetMachineName(), "proxy", "--port=0")

stdoutPipe, err := cmd.StdoutPipe()
if err != nil {
return nil, "", errors.Wrap(err, "cmd stdout")
Expand Down
14 changes: 9 additions & 5 deletions cmd/minikube/cmd/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ import (
"k8s.io/minikube/pkg/minikube/exit"
"k8s.io/minikube/pkg/minikube/logs"
"k8s.io/minikube/pkg/minikube/machine"
"k8s.io/minikube/pkg/minikube/proxy"
pkgutil "k8s.io/minikube/pkg/util"
"k8s.io/minikube/pkg/version"
)
Expand Down Expand Up @@ -102,9 +103,6 @@ var (
apiServerNames []string
apiServerIPs []net.IP
extraOptions pkgutil.ExtraOptionSlice

// proxyVars are variables we plumb through to the underlying container runtime
proxyVars = []string{"HTTP_PROXY", "HTTPS_PROXY", "NO_PROXY"}
)

func init() {
Expand Down Expand Up @@ -222,6 +220,8 @@ func runStart(cmd *cobra.Command, args []string) {
host, preexisting := startHost(m, config.MachineConfig)

ip := validateNetwork(host)
// Makes minikube node ip to bypass http(s) proxy. since it is local traffic.
proxy.UpdateNoProxy(ip)
// Save IP to configuration file for subsequent use
config.KubernetesConfig.NodeIP = ip
if err := saveConfig(config); err != nil {
Expand Down Expand Up @@ -383,7 +383,7 @@ func generateConfig(cmd *cobra.Command, k8sVersion string) (cfg.Config, error) {
// Feed Docker our host proxy environment by default, so that it can pull images
if _, ok := r.(*cruntime.Docker); ok {
if !cmd.Flags().Changed("docker-env") {
for _, k := range proxyVars {
for _, k := range proxy.EnvVars {
if v := os.Getenv(k); v != "" {
dockerEnv = append(dockerEnv, fmt.Sprintf("%s=%s", k, v))
}
Expand Down Expand Up @@ -522,13 +522,17 @@ func validateNetwork(h *host.Host) string {
}

optSeen := false
for _, k := range proxyVars {
for _, k := range proxy.EnvVars {
if v := os.Getenv(k); v != "" {
if !optSeen {
console.OutStyle("internet", "Found network options:")
optSeen = true
}
console.OutStyle("option", "%s=%s", k, v)
isNp, _ := proxy.IsInNoProxyEnv(ip) // Skip warning if minikube ip is already in NO_PROXY
if (k == "HTTP_PROXY" || k == "HTTPS_PROXY") && !isNp {
console.Warning("You are using a proxy, You need to add minikube IP to the NO_PROXY. Use `export NO_PROXY=$NO_PROXY,%s`", ip)
medyagh marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

"export" will only work for bourne-shell compatible users, and might confuse Windows users. How about something that points users to https://github.com/kubernetes/minikube/blob/master/docs/http_proxy.md

For example:

"You appear to be using a proxy, but your NO_PROXY environment does not include the minikube IP (X). Please see https://github.com/kubernetes/minikube/blob/master/docs/http_proxy.md for more details"

Also, the minikube IP will change every run for some users -- perhaps we should suggest that they set the CIDR form instead?

}
}
}

Expand Down
1 change: 1 addition & 0 deletions pkg/minikube/bootstrapper/kubeadm/kubeadm.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ func (k *Bootstrapper) GetAPIServerStatus(ip net.IP, apiserverPort int) (string,
url := fmt.Sprintf("https://%s:%d/healthz", ip, apiserverPort)
// To avoid: x509: certificate signed by unknown authority
tr := &http.Transport{
Proxy: nil, // To avoid connectiv issue if http(s)_proxy is set.
TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
}
client := &http.Client{Transport: tr}
Expand Down
32 changes: 32 additions & 0 deletions pkg/minikube/proxy/cidir.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/*
Copyright 2019 The Kubernetes Authors All rights reserved.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package proxy

import "net"

// isInBlock checks if ip is a CIDIR block
medyagh marked this conversation as resolved.
Show resolved Hide resolved
func isInBlock(ip string, block string) (bool, error) {
_, b, err := net.ParseCIDR(block)
if err != nil {
return false, err
}
i := net.ParseIP(ip)
if b.Contains(i) {
medyagh marked this conversation as resolved.
Show resolved Hide resolved
return true, nil
}
return false, nil
}
48 changes: 48 additions & 0 deletions pkg/minikube/proxy/cidir_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/*
Copyright 2019 The Kubernetes Authors All rights reserved.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package proxy

import (
"testing"
)

var bTests = []struct {
ip string
block string
expectedResult bool
expectedErr error
}{
{"", "192.168.0.1/32", false, nil},
{"192.168.0.1", "192.168.0.1/32", true, nil},
{"192.168.0.2", "192.168.0.1/32", false, nil},
{"192.168.0.1", "192.168.0.1/18", true, nil},
{"abcd", "192.168.0.1/18", false, nil},
}

func TestIsInBlock(t *testing.T) {

medyagh marked this conversation as resolved.
Show resolved Hide resolved
for _, tt := range bTests {
actualR, actualErr := isInBlock(tt.ip, tt.block)
medyagh marked this conversation as resolved.
Show resolved Hide resolved
if actualR != tt.expectedResult {
t.Errorf("isInBlock(%s,%s): expected %t, actual %t", tt.ip, tt.block, tt.expectedResult, actualR)
medyagh marked this conversation as resolved.
Show resolved Hide resolved
}
if actualErr != tt.expectedErr {
t.Errorf("isInBlock(%s,%s): expected error %s, err %s", tt.ip, tt.block, tt.expectedErr, actualErr)
}
}

}
20 changes: 20 additions & 0 deletions pkg/minikube/proxy/env.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/*
Copyright 2019 The Kubernetes Authors All rights reserved.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package proxy

// EnvVars are variables we plumb through to the underlying container runtime
var EnvVars = []string{"HTTP_PROXY", "HTTPS_PROXY", "NO_PROXY"}
medyagh marked this conversation as resolved.
Show resolved Hide resolved
58 changes: 58 additions & 0 deletions pkg/minikube/proxy/noproxy.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/*
Copyright 2019 The Kubernetes Authors All rights reserved.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package proxy

import (
"fmt"
"os"
"strings"
)

// UpdateNoProxy is used to whitelist minikube's VM ip from going through proxy
// It updates NO_PROXY environment variable, for the current run.
func UpdateNoProxy(ip string) error {
medyagh marked this conversation as resolved.
Show resolved Hide resolved
yes, v := IsInNoProxyEnv(ip)
if yes { // skip if already whitelisted
return nil
}
return os.Setenv("NO_PROXY", fmt.Sprintf("%s,%s", v, ip))
}

// IsInNoProxyEnv checks if ip is set in NO_PROXY env variable
// Checks for both IP and IP ranges
medyagh marked this conversation as resolved.
Show resolved Hide resolved
func IsInNoProxyEnv(ip string) (bool, string) {
medyagh marked this conversation as resolved.
Show resolved Hide resolved
v := os.Getenv("NO_PROXY")

if v == "" {
return false, ""
}

// Checking for IP explicitly, i.e., 192.168.39.224
if strings.Contains(v, ip) {
return true, v
}

// Checks if ip included in IP ranges, i.e., 192.168.39.13/24
noProxyBlocks := strings.Split(v, ",")
for _, b := range noProxyBlocks {
if yes, _ := isInBlock(ip, b); yes {
return true, v
}
}

return false, v
}