Merge pull request #10109 from lingsamuel/fix-docker-env

Ignore non-socks5 ALL_PROXY env var when checking docker status

cmd/minikube/cmd/docker-env.go

@@ -23,6 +23,7 @@ import (
 	"fmt"
 	"io"
 	"net"
+	"net/url"
 	"os"
 	"os/exec"
 	"strconv"
@@ -455,9 +456,41 @@ func dockerEnvVarsList(ec DockerEnvConfig) []string {
 	}
 }
 
+func isValidDockerProxy(env string) bool {
+	val := os.Getenv(env)
+	if val == "" {
+		return true
+	}
+
+	u, err := url.Parse(val)
+	if err != nil {
+		klog.Warningf("Parsing proxy env variable %s=%s error: %v", env, val, err)
+		return false
+	}
+	switch u.Scheme {
+	// See moby/moby#25740
+	case "socks5", "socks5h":
+		return true
+	default:
+		return false
+	}
+}
+
+func removeInvalidDockerProxy() {
+	for _, env := range []string{"ALL_PROXY", "all_proxy"} {
+		if !isValidDockerProxy(env) {
+			klog.Warningf("Ignoring non socks5 proxy env variable %s=%s", env, os.Getenv(env))
+			os.Unsetenv(env)
+		}
+	}
+}
+
 // tryDockerConnectivity will try to connect to docker env from user's POV to detect the problem if it needs reset or not
 func tryDockerConnectivity(bin string, ec DockerEnvConfig) ([]byte, error) {
 	c := exec.Command(bin, "version", "--format={{.Server}}")
+
+	// See #10098 for details
+	removeInvalidDockerProxy()
 	c.Env = append(os.Environ(), dockerEnvVarsList(ec)...)
 	klog.Infof("Testing Docker connectivity with: %v", c)
 	return c.CombinedOutput()

cmd/minikube/cmd/docker-env_test.go

@@ -18,6 +18,7 @@ package cmd
 
 import (
 	"bytes"
+	"os"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -306,3 +307,37 @@ MINIKUBE_ACTIVE_DOCKERD
 		})
 	}
 }
+
+func TestValidDockerProxy(t *testing.T) {
+	var tests = []struct {
+		proxy   string
+		isValid bool
+	}{
+		{
+			proxy:   "socks5://192.168.0.1:1080",
+			isValid: true,
+		},
+		{
+			proxy:   "",
+			isValid: true,
+		},
+		{
+			proxy:   "socks://192.168.0.1:1080",
+			isValid: false,
+		},
+		{
+			proxy:   "http://192.168.0.1:1080",
+			isValid: false,
+		},
+	}
+
+	for _, tc := range tests {
+		os.Setenv("ALL_PROXY", tc.proxy)
+		valid := isValidDockerProxy("ALL_PROXY")
+		if tc.isValid && valid != tc.isValid {
+			t.Errorf("Expect %#v to be valid docker proxy", tc.proxy)
+		} else if !tc.isValid && valid != tc.isValid {
+			t.Errorf("Expect %#v to be invalid docker proxy", tc.proxy)
+		}
+	}
+}