From 0b986bd0444725610e87f17e9c5dc2fde8cbdeb3 Mon Sep 17 00:00:00 2001 From: Marcin Niemira Date: Sat, 7 Sep 2019 18:36:18 +1000 Subject: [PATCH 1/4] don't pass http_proxy to dockerEnv if it's poiting to localhost --- cmd/minikube/cmd/start.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/cmd/minikube/cmd/start.go b/cmd/minikube/cmd/start.go index 03f3f628e7..6325693076 100644 --- a/cmd/minikube/cmd/start.go +++ b/cmd/minikube/cmd/start.go @@ -688,6 +688,12 @@ func generateCfgFromFlags(cmd *cobra.Command, k8sVersion string) (cfg.Config, er // convert https_proxy to HTTPS_PROXY for linux // TODO (@medyagh): if user has both http_proxy & HTTPS_PROXY set merge them. k = strings.ToUpper(k) + if k == "HTTP_PROXY" || k == "HTTPS_PROXY" { + if strings.HasPrefix(v, "localhost") || strings.HasPrefix(v, "127.0") { + out.WarningT("Not passing {{.name}}={{.value}} to docker env.", out.V{"name": k, "value": v}) + continue + } + } dockerEnv = append(dockerEnv, fmt.Sprintf("%s=%s", k, v)) } } From e9517ec69b693fd5bf81d33b7f28841b9e7e798e Mon Sep 17 00:00:00 2001 From: Marcin Niemira Date: Sat, 7 Sep 2019 20:26:57 +1000 Subject: [PATCH 2/4] add tests --- cmd/minikube/cmd/start_test.go | 56 ++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/cmd/minikube/cmd/start_test.go b/cmd/minikube/cmd/start_test.go index 446091c17f..f67d6f3278 100644 --- a/cmd/minikube/cmd/start_test.go +++ b/cmd/minikube/cmd/start_test.go @@ -17,7 +17,11 @@ limitations under the License. package cmd import ( + "os" "testing" + + "github.com/spf13/cobra" + "k8s.io/minikube/pkg/minikube/constants" ) func Test_extractVMDriverVersion(t *testing.T) { @@ -43,3 +47,55 @@ func Test_extractVMDriverVersion(t *testing.T) { t.Errorf("Expected version: %s, got: %s", expectedVersion, v) } } + +func TestGenerateCfgFromFlagsHTTPProxyHandling(t *testing.T) { + originalEnv := os.Getenv("HTTP_PROXY") + defer func() { + err := os.Setenv("HTTP_PROXY", originalEnv) + if err != nil { + t.Fatalf("Error reverting env HTTP_PROXY to it's original value. Got err (%s)", err) + } + }() + k8sVersion := constants.NewestKubernetesVersion + var tests = []struct { + description string + proxy string + proxyIgnored bool + }{ + { + description: "no http_proxy", + }, + { + description: "http_proxy=127.0.0.1:3128", + proxy: "127.0.0.1:3128", + proxyIgnored: true, + }, + { + description: "http_proxy=localhost:3128", + proxy: "localhost:3128", + proxyIgnored: true, + }, + { + description: "http_proxy=1.2.3.4:3128", + proxy: "1.2.3.4:3128", + }, + } + for _, test := range tests { + t.Run(test.description, func(t *testing.T) { + cmd := cobra.Command{} + if err := os.Setenv("HTTP_PROXY", test.proxy); err != nil { + t.Fatalf("Unexpected error setting HTTP_PROXY: %v", err) + } + config, err := generateCfgFromFlags(&cmd, k8sVersion) + if err != nil { + t.Fatalf("Got unexpected error %v during config generation", err) + } + // ignored proxy should not be in config + for _, v := range config.MachineConfig.DockerEnv { + if v == test.proxy && test.proxyIgnored { + t.Fatalf("Value %v not expected in dockerEnv but occured", v) + } + } + }) + } +} From e9f9df56ad09e066c4f2d6360dc08ff31a19bddb Mon Sep 17 00:00:00 2001 From: Marcin Niemira Date: Sat, 7 Sep 2019 20:38:40 +1000 Subject: [PATCH 3/4] fix misspeling --- cmd/minikube/cmd/start_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/minikube/cmd/start_test.go b/cmd/minikube/cmd/start_test.go index f67d6f3278..b4cb3213bf 100644 --- a/cmd/minikube/cmd/start_test.go +++ b/cmd/minikube/cmd/start_test.go @@ -93,7 +93,7 @@ func TestGenerateCfgFromFlagsHTTPProxyHandling(t *testing.T) { // ignored proxy should not be in config for _, v := range config.MachineConfig.DockerEnv { if v == test.proxy && test.proxyIgnored { - t.Fatalf("Value %v not expected in dockerEnv but occured", v) + t.Fatalf("Value %v not expected in dockerEnv but occurred", v) } } }) From ee8d4b757c1ba46f5a6b0727d57d404187a5f972 Mon Sep 17 00:00:00 2001 From: Marcin Niemira Date: Sun, 8 Sep 2019 15:53:03 +1000 Subject: [PATCH 4/4] fix unit tests --- cmd/minikube/cmd/start_test.go | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/cmd/minikube/cmd/start_test.go b/cmd/minikube/cmd/start_test.go index b4cb3213bf..97e203466f 100644 --- a/cmd/minikube/cmd/start_test.go +++ b/cmd/minikube/cmd/start_test.go @@ -21,6 +21,7 @@ import ( "testing" "github.com/spf13/cobra" + "github.com/spf13/viper" "k8s.io/minikube/pkg/minikube/constants" ) @@ -49,11 +50,13 @@ func Test_extractVMDriverVersion(t *testing.T) { } func TestGenerateCfgFromFlagsHTTPProxyHandling(t *testing.T) { + viper.SetDefault(memory, constants.DefaultMemorySize) + viper.SetDefault(humanReadableDiskSize, constants.DefaultDiskSize) originalEnv := os.Getenv("HTTP_PROXY") defer func() { err := os.Setenv("HTTP_PROXY", originalEnv) if err != nil { - t.Fatalf("Error reverting env HTTP_PROXY to it's original value. Got err (%s)", err) + t.Fatalf("Error reverting env HTTP_PROXY to it's original value. Got err: %s", err) } }() k8sVersion := constants.NewestKubernetesVersion @@ -62,9 +65,7 @@ func TestGenerateCfgFromFlagsHTTPProxyHandling(t *testing.T) { proxy string proxyIgnored bool }{ - { - description: "no http_proxy", - }, + { description: "http_proxy=127.0.0.1:3128", proxy: "127.0.0.1:3128", @@ -79,14 +80,17 @@ func TestGenerateCfgFromFlagsHTTPProxyHandling(t *testing.T) { description: "http_proxy=1.2.3.4:3128", proxy: "1.2.3.4:3128", }, + { + description: "no http_proxy", + }, } for _, test := range tests { t.Run(test.description, func(t *testing.T) { - cmd := cobra.Command{} + cmd := &cobra.Command{} if err := os.Setenv("HTTP_PROXY", test.proxy); err != nil { t.Fatalf("Unexpected error setting HTTP_PROXY: %v", err) } - config, err := generateCfgFromFlags(&cmd, k8sVersion) + config, err := generateCfgFromFlags(cmd, k8sVersion) if err != nil { t.Fatalf("Got unexpected error %v during config generation", err) }