Skip to content

Commit

Permalink
Merge pull request #5228 from n0npax/improve-proxy-test-coverage
Browse files Browse the repository at this point in the history 
Improve test coverage for proxy package
  • Loading branch information
@tstromberg
tstromberg authored Sep 4, 2019
2 parents ec27e3a + fa9ddcd commit c74a59e
Show file tree
Hide file tree
Showing 2 changed files with 121 additions and 11 deletions.
7 changes: 6 additions & 1 deletion pkg/minikube/proxy/proxy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}

Expand Down
125 changes: 115 additions & 10 deletions pkg/minikube/proxy/proxy_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,11 @@ package proxy

import (
"fmt"
"net/http"
"os"
"testing"

"k8s.io/client-go/rest"
)

func TestIsValidEnv(t *testing.T) {
Expand All @@ -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)
}

})
}

Expand All @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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) {
Expand All @@ -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)")
}
})
}

0 comments on commit c74a59e

Please sign in to comment.