diff --git a/pkg/minikube/proxy/proxy.go b/pkg/minikube/proxy/proxy.go index 9d911b5649..a65d29098c 100644 --- a/pkg/minikube/proxy/proxy.go +++ b/pkg/minikube/proxy/proxy.go @@ -55,8 +55,13 @@ func isInBlock(ip string, block string) (bool, error) { return false, errors.Wrapf(err, "Error ip not in block") } -// ExcludeIP will exclude the ip from the http(s)_proxy +// ExcludeIP takes ip or CIDR as string and excludes it from the http(s)_proxy func ExcludeIP(ip string) error { + if netIP := net.ParseIP(ip); netIP == nil { + if _, _, err := net.ParseCIDR(ip); err != nil { + return fmt.Errorf("ExcludeIP(%v) requires IP or CIDR as a parameter", ip) + } + } return updateEnv(ip, "NO_PROXY") } diff --git a/pkg/minikube/proxy/proxy_test.go b/pkg/minikube/proxy/proxy_test.go index 25dccce10d..17f62e2a86 100644 --- a/pkg/minikube/proxy/proxy_test.go +++ b/pkg/minikube/proxy/proxy_test.go @@ -18,8 +18,11 @@ package proxy import ( "fmt" + "net/http" "os" "testing" + + "k8s.io/client-go/rest" ) func TestIsValidEnv(t *testing.T) { @@ -33,11 +36,9 @@ func TestIsValidEnv(t *testing.T) { } for _, tc := range testCases { t.Run(tc.env, func(t *testing.T) { - got := isValidEnv(tc.env) - if got != tc.want { + if got := isValidEnv(tc.env); got != tc.want { t.Errorf("isValidEnv(\"%v\") got %v; want %v", tc.env, got, tc.want) } - }) } @@ -55,9 +56,10 @@ func TestIsInBlock(t *testing.T) { {"192.168.0.2", "192.168.0.1/32", false, false}, {"192.168.0.1", "192.168.0.1/18", true, false}, {"abcd", "192.168.0.1/18", false, true}, + {"192.168.0.1", "foo", false, true}, } for _, tc := range testCases { - t.Run(fmt.Sprintf("%s in %s", tc.ip, tc.block), func(t *testing.T) { + t.Run(fmt.Sprintf("%s in %s Want: %t WantAErr: %t", tc.ip, tc.block, tc.want, tc.wanntAErr), func(t *testing.T) { got, err := isInBlock(tc.ip, tc.block) gotErr := false if err != nil { @@ -88,7 +90,7 @@ func TestUpdateEnv(t *testing.T) { {"192.168.0.13", "NPROXY", true}, } for _, tc := range testCases { - t.Run(fmt.Sprintf("%s in %s", tc.ip, tc.env), func(t *testing.T) { + t.Run(fmt.Sprintf("%s in %s WantAErr: %t", tc.ip, tc.env, tc.wantErr), func(t *testing.T) { origVal := os.Getenv(tc.env) gotErr := false err := updateEnv(tc.ip, tc.env) @@ -124,6 +126,7 @@ func TestCheckEnv(t *testing.T) { {"192.168.0.13", "NO_PROXY", true, "192.168.0.13/22"}, {"192.168.0.13", "NO_PROXY", true, "10.10.0.13,192.168.0.13"}, {"192.168.0.13", "NO_PROXY", false, "10.10.0.13/22"}, + {"10.10.10.4", "NO_PROXY", true, "172.168.0.0/30,10.10.10.0/24"}, } for _, tc := range testCases { t.Run(fmt.Sprintf("%s in %s", tc.ip, tc.envName), func(t *testing.T) { @@ -135,17 +138,119 @@ func TestCheckEnv(t *testing.T) { } }() - // defer os.Setenv(tc.envName, originalEnv) - err := os.Setenv(tc.envName, tc.mockEnvValue) // setting up the test case - if err != nil { + if err := os.Setenv(tc.envName, tc.mockEnvValue); err != nil { t.Error("Error setting env var for taste case") } - got := checkEnv(tc.ip, tc.envName) - if got != tc.want { + if got := checkEnv(tc.ip, tc.envName); got != tc.want { t.Errorf("CheckEnv(%v,%v) got %v ; want is %v", tc.ip, tc.envName, got, tc.want) } - }) } } + +func TestIsIPExcluded(t *testing.T) { + var testCases = []struct { + ip, env string + excluded bool + }{ + {"1.2.3.4", "7.7.7.7", false}, + {"1.2.3.4", "1.2.3.4", true}, + {"1.2.3.4", "", false}, + {"foo", "", false}, + {"foo", "bar", false}, + {"foo", "1.2.3.4", false}, + } + for _, tc := range testCases { + originalEnv := os.Getenv("NO_PROXY") + defer func() { // revert to pre-test env var + err := os.Setenv("NO_PROXY", originalEnv) + if err != nil { + t.Fatalf("Error reverting env NO_PROXY to its original value (%s) var after test ", originalEnv) + } + }() + t.Run(fmt.Sprintf("exclude %s NO_PROXY(%v)", tc.ip, tc.env), func(t *testing.T) { + if err := os.Setenv("NO_PROXY", tc.env); err != nil { + t.Errorf("Error during setting env: %v", err) + } + if excluded := IsIPExcluded(tc.ip); excluded != tc.excluded { + t.Fatalf("IsIPExcluded(%v) should return %v. NO_PROXY=%v", tc.ip, tc.excluded, tc.env) + } + }) + } +} + +func TestExcludeIP(t *testing.T) { + var testCases = []struct { + ip, env string + wantAErr bool + }{ + {"1.2.3.4", "", false}, + {"", "", true}, + {"7.7.7.7", "7.7.7.7", false}, + {"7.7.7.7", "1.2.3.4", false}, + {"foo", "", true}, + {"foo", "1.2.3.4", true}, + } + originalEnv := os.Getenv("NO_PROXY") + defer func() { // revert to pre-test env var + err := os.Setenv("NO_PROXY", originalEnv) + if err != nil { + t.Fatalf("Error reverting env NO_PROXY to its original value (%s) var after test ", originalEnv) + } + }() + for _, tc := range testCases { + t.Run(fmt.Sprintf("exclude %s NO_PROXY(%s)", tc.ip, tc.env), func(t *testing.T) { + if err := os.Setenv("NO_PROXY", tc.env); err != nil { + t.Errorf("Error during setting env: %v", err) + } + err := ExcludeIP(tc.ip) + if err != nil && !tc.wantAErr { + t.Errorf("ExcludeIP(%v) returned unexpected error %v", tc.ip, err) + } + if err == nil && tc.wantAErr { + t.Errorf("ExcludeIP(%v) should return error but error is %v", tc.ip, err) + } + }) + } +} + +func TestUpdateTransport(t *testing.T) { + t.Run("new", func(t *testing.T) { + rc := rest.Config{} + c := UpdateTransport(&rc) + tr := &http.Transport{} + tr.RegisterProtocol("file", http.NewFileTransport(http.Dir("/tmp"))) + rt := c.WrapTransport(tr) + if _, ok := rt.(http.RoundTripper); !ok { + t.Fatalf("Cannot cast rt(%v) to http.RoundTripper", rt) + } + }) + t.Run("existing", func(t *testing.T) { + // rest config initialized with WrapTransport function + rc := rest.Config{WrapTransport: func(http.RoundTripper) http.RoundTripper { + rt := &http.Transport{} + rt.RegisterProtocol("file", http.NewFileTransport(http.Dir("/tmp"))) + return rt + }} + + transport := &http.Transport{} + transport.RegisterProtocol("file", http.NewFileTransport(http.Dir("/"))) + + c := UpdateTransport(&rc) + rt := c.WrapTransport(nil) + + if rt == rc.WrapTransport(transport) { + t.Fatalf("Excpected to reuse existing RoundTripper(%v) but found %v", rt, transport) + } + + }) + t.Run("nil", func(t *testing.T) { + rc := rest.Config{} + c := UpdateTransport(&rc) + rt := c.WrapTransport(nil) + if rt != nil { + t.Fatalf("Expected RoundTripper nil for invocation WrapTransport(nil)") + } + }) +}