Improve test coverage for proxy package

pkg/minikube/proxy/proxy.go

@@ -57,6 +57,11 @@ func isInBlock(ip string, block string) (bool, error) {
 
 // ExcludeIP will exclude the ip 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 as a parameter or CIDR", ip)
+		}
+	}
 	return updateEnv(ip, "NO_PROXY")
 }
 

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) {
@@ -55,6 +58,7 @@ 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) {
@@ -124,6 +128,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) {
@@ -149,3 +154,109 @@ func TestCheckEnv(t *testing.T) {
 	}
 
 }
+
+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 {
+		t.Run(fmt.Sprintf("exclude %s NO_PROXY(%v)", tc.ip, tc.env), func(t *testing.T) {
+			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)
+				}
+			}()
+			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},
+	}
+	for _, tc := range testCases {
+		t.Run(fmt.Sprintf("exclude %s NO_PROXY(%s)", tc.ip, tc.env), func(t *testing.T) {
+			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)
+				}
+			}()
+			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)")
+		}
+	})
+}