From da6726851704258a98d60bae7edc5608f249ae86 Mon Sep 17 00:00:00 2001 From: JeremyWhite Date: Thu, 20 Sep 2018 14:43:25 -0500 Subject: [PATCH] updated test with CONST, var clean up --- config/default.go | 1 - docs/content/ref/glob.matching.disabled.md | 2 +- fabio.properties | 2 +- main.go | 3 +- proxy/http_integration_test.go | 31 +++++----- route/issue57_test.go | 3 +- route/route_bench_test.go | 3 +- route/table_test.go | 68 ++++++++++++---------- 8 files changed, 57 insertions(+), 56 deletions(-) diff --git a/config/default.go b/config/default.go index d46ba9bc6..45f68424d 100644 --- a/config/default.go +++ b/config/default.go @@ -77,5 +77,4 @@ var defaultConfig = &Config{ Color: "light-green", Access: "rw", }, - DisableGlobMatching: false, } diff --git a/docs/content/ref/glob.matching.disabled.md b/docs/content/ref/glob.matching.disabled.md index 5ab2fd06d..ec1e41e00 100644 --- a/docs/content/ref/glob.matching.disabled.md +++ b/docs/content/ref/glob.matching.disabled.md @@ -2,7 +2,7 @@ title: "glob.matching.disabled" --- -`glob.matching.disabled` Disables glob matching on route lookups. +`glob.matching.disabled` disables glob matching on route lookups. Valid options are `true`, `false` diff --git a/fabio.properties b/fabio.properties index 28f7d994d..207c17da3 100644 --- a/fabio.properties +++ b/fabio.properties @@ -743,7 +743,7 @@ # registry.consul.checksRequired = one -# glob.matching.disabled Disables glob matching on route lookups +# glob.matching.disabled disables glob matching on route lookups # If glob matching is enabled there is a performance decrease # for every route lookup. At a large number of services (> 500) this # can have a significant impact on performance. If glob matching is disabled diff --git a/main.go b/main.go index 041102a27..3ccf43b97 100644 --- a/main.go +++ b/main.go @@ -137,7 +137,6 @@ func main() { func newHTTPProxy(cfg *config.Config) http.Handler { var w io.Writer - globDisabled := cfg.DisableGlobMatching switch cfg.Log.AccessTarget { case "": log.Printf("[INFO] Access logging disabled") @@ -184,7 +183,7 @@ func newHTTPProxy(cfg *config.Config) http.Handler { Transport: newTransport(nil), InsecureTransport: newTransport(&tls.Config{InsecureSkipVerify: true}), Lookup: func(r *http.Request) *route.Target { - t := route.GetTable().Lookup(r, r.Header.Get("trace"), pick, match, globDisabled) + t := route.GetTable().Lookup(r, r.Header.Get("trace"), pick, match, cfg.DisableGlobMatching) if t == nil { notFound.Inc(1) log.Print("[WARN] No route for ", r.Host, r.URL) diff --git a/proxy/http_integration_test.go b/proxy/http_integration_test.go index 92cbcce8b..4f223ffb2 100644 --- a/proxy/http_integration_test.go +++ b/proxy/http_integration_test.go @@ -27,6 +27,13 @@ import ( "github.com/pascaldekloe/goe/verify" ) + +const ( + // helper constants for the Lookup function + globEnabled = false + globDisabled = true +) + func TestProxyProducesCorrectXForwardedSomethingHeader(t *testing.T) { var hdr http.Header = make(http.Header) server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -175,8 +182,6 @@ func TestProxyNoRouteStatus(t *testing.T) { } func TestProxyStripsPath(t *testing.T) { - //Glob Matching True - globDisabled := false server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch r.RequestURI { case "/bar": @@ -190,7 +195,7 @@ func TestProxyStripsPath(t *testing.T) { Transport: http.DefaultTransport, Lookup: func(r *http.Request) *route.Target { tbl, _ := route.NewTable("route add mock /foo/bar " + server.URL + ` opts "strip=/foo"`) - return tbl.Lookup(r, "", route.Picker["rr"], route.Matcher["prefix"], globDisabled) + return tbl.Lookup(r, "", route.Picker["rr"], route.Matcher["prefix"], globEnabled) }, }) defer proxy.Close() @@ -217,7 +222,6 @@ func TestProxyHost(t *testing.T) { routes += "route add mock /hostcustom http://b.com/ opts \"host=bar.com\"\n" routes += "route add mock / http://a.com/" tbl, _ := route.NewTable(routes) - globDisabled := false proxy := httptest.NewServer(&HTTPProxy{ Transport: &http.Transport{ @@ -227,7 +231,7 @@ func TestProxyHost(t *testing.T) { }, }, Lookup: func(r *http.Request) *route.Target { - return tbl.Lookup(r, "", route.Picker["rr"], route.Matcher["prefix"], globDisabled) + return tbl.Lookup(r, "", route.Picker["rr"], route.Matcher["prefix"], globEnabled) }, }) defer proxy.Close() @@ -269,13 +273,12 @@ func TestHostRedirect(t *testing.T) { routes := "route add https-redir *:80 https://$host$path opts \"redirect=301\"\n" tbl, _ := route.NewTable(routes) - globDisabled := false proxy := httptest.NewServer(&HTTPProxy{ Transport: http.DefaultTransport, Lookup: func(r *http.Request) *route.Target { r.Host = "c.com" - return tbl.Lookup(r, "", route.Picker["rr"], route.Matcher["prefix"], globDisabled) + return tbl.Lookup(r, "", route.Picker["rr"], route.Matcher["prefix"], globEnabled) }, }) defer proxy.Close() @@ -310,12 +313,11 @@ func TestPathRedirect(t *testing.T) { routes += "route add mock /foo http://a.com/abc opts \"redirect=301\"\n" routes += "route add mock /bar http://b.com/$path opts \"redirect=302 strip=/bar\"\n" tbl, _ := route.NewTable(routes) - globDisabled := false proxy := httptest.NewServer(&HTTPProxy{ Transport: http.DefaultTransport, Lookup: func(r *http.Request) *route.Target { - return tbl.Lookup(r, "", route.Picker["rr"], route.Matcher["prefix"], globDisabled) + return tbl.Lookup(r, "", route.Picker["rr"], route.Matcher["prefix"], globEnabled) }, }) defer proxy.Close() @@ -477,14 +479,13 @@ func TestProxyHTTPSUpstream(t *testing.T) { server.TLS = tlsServerConfig() server.StartTLS() defer server.Close() - globDisabled := false proxy := httptest.NewServer(&HTTPProxy{ Config: config.Proxy{}, Transport: &http.Transport{TLSClientConfig: tlsClientConfig()}, Lookup: func(r *http.Request) *route.Target { tbl, _ := route.NewTable("route add srv / " + server.URL + ` opts "proto=https"`) - return tbl.Lookup(r, "", route.Picker["rr"], route.Matcher["prefix"], globDisabled) + return tbl.Lookup(r, "", route.Picker["rr"], route.Matcher["prefix"], globEnabled) }, }) defer proxy.Close() @@ -503,8 +504,6 @@ func TestProxyHTTPSUpstreamSkipVerify(t *testing.T) { server.TLS = &tls.Config{} server.StartTLS() defer server.Close() - globDisabled := false - proxy := httptest.NewServer(&HTTPProxy{ Config: config.Proxy{}, Transport: http.DefaultTransport, @@ -513,7 +512,7 @@ func TestProxyHTTPSUpstreamSkipVerify(t *testing.T) { }, Lookup: func(r *http.Request) *route.Target { tbl, _ := route.NewTable("route add srv / " + server.URL + ` opts "proto=https tlsskipverify=true"`) - return tbl.Lookup(r, "", route.Picker["rr"], route.Matcher["prefix"], globDisabled) + return tbl.Lookup(r, "", route.Picker["rr"], route.Matcher["prefix"], globEnabled) }, }) defer proxy.Close() @@ -706,7 +705,7 @@ func BenchmarkProxyLogger(b *testing.B) { b.Fatal("logger.NewHTTPLogger:", err) } - globDisabled := false + proxy := &HTTPProxy{ Config: config.Proxy{ @@ -716,7 +715,7 @@ func BenchmarkProxyLogger(b *testing.B) { Transport: http.DefaultTransport, Lookup: func(r *http.Request) *route.Target { tbl, _ := route.NewTable("route add mock / " + server.URL) - return tbl.Lookup(r, "", route.Picker["rr"], route.Matcher["prefix"], globDisabled) + return tbl.Lookup(r, "", route.Picker["rr"], route.Matcher["prefix"], globEnabled) }, Logger: l, } diff --git a/route/issue57_test.go b/route/issue57_test.go index e8d412462..17082b4d5 100644 --- a/route/issue57_test.go +++ b/route/issue57_test.go @@ -24,7 +24,6 @@ func TestIssue57(t *testing.T) { route del svcb`, } - globDisabled := false req := &http.Request{URL: mustParse("/foo")} want := "http://foo.com:800" @@ -34,7 +33,7 @@ func TestIssue57(t *testing.T) { if err != nil { t.Fatalf("%d: got %v want nil", i, err) } - target := tbl.Lookup(req, "", rrPicker, prefixMatcher, globDisabled) + target := tbl.Lookup(req, "", rrPicker, prefixMatcher, globEnabled) if target == nil { t.Fatalf("%d: got %v want %v", i, target, want) } diff --git a/route/route_bench_test.go b/route/route_bench_test.go index 2a5b511be..e5693c572 100644 --- a/route/route_bench_test.go +++ b/route/route_bench_test.go @@ -124,9 +124,8 @@ func benchmarkGet(t Table, match matcher, pick picker, pb *testing.PB) { reqs := makeRequests(t) k, n := len(reqs), 0 //Glob Matching True - globDisabled := false for pb.Next() { - t.Lookup(reqs[n%k], "", pick, match, globDisabled) + t.Lookup(reqs[n%k], "", pick, match, globEnabled) n++ } } diff --git a/route/table_test.go b/route/table_test.go index d9ef05429..7d20ecfc6 100644 --- a/route/table_test.go +++ b/route/table_test.go @@ -11,6 +11,12 @@ import ( "testing" ) +const ( + // helper constants for the Lookup function + globEnabled = false + globDisabled = true +) + func TestTableParse(t *testing.T) { genRoutes := func(n int, format string) (a []string) { for i := 0; i < n; i++ { @@ -501,7 +507,7 @@ func TestTableLookupIssue448(t *testing.T) { var tests = []struct { req *http.Request dst string - globDisabled bool + globEnabled bool }{ { req: &http.Request{ @@ -552,7 +558,7 @@ func TestTableLookupIssue448(t *testing.T) { } for i, tt := range tests { - if got, want := tbl.Lookup(tt.req, "", rndPicker, prefixMatcher, tt.globDisabled).URL.String(), tt.dst; got != want { + if got, want := tbl.Lookup(tt.req, "", rndPicker, prefixMatcher, globEnabled).URL.String(), tt.dst; got != want { t.Errorf("%d: got %v want %v", i, got, want) } } @@ -583,52 +589,52 @@ func TestTableLookup(t *testing.T) { var tests = []struct { req *http.Request dst string - globDisabled bool + globEnabled bool }{ // match on host and path with and without trailing slash - {&http.Request{Host: "abc.com", URL: mustParse("/")}, "http://foo.com:1000", false}, - {&http.Request{Host: "abc.com", URL: mustParse("/bar")}, "http://foo.com:1000", false}, - {&http.Request{Host: "abc.com", URL: mustParse("/foo")}, "http://foo.com:1500", false}, - {&http.Request{Host: "abc.com", URL: mustParse("/foo/")}, "http://foo.com:2000", false}, - {&http.Request{Host: "abc.com", URL: mustParse("/foo/bar")}, "http://foo.com:2500", false}, - {&http.Request{Host: "abc.com", URL: mustParse("/foo/bar/")}, "http://foo.com:3000", false}, + {&http.Request{Host: "abc.com", URL: mustParse("/")}, "http://foo.com:1000", globEnabled}, + {&http.Request{Host: "abc.com", URL: mustParse("/bar")}, "http://foo.com:1000", globEnabled}, + {&http.Request{Host: "abc.com", URL: mustParse("/foo")}, "http://foo.com:1500", globEnabled}, + {&http.Request{Host: "abc.com", URL: mustParse("/foo/")}, "http://foo.com:2000", globEnabled}, + {&http.Request{Host: "abc.com", URL: mustParse("/foo/bar")}, "http://foo.com:2500", globEnabled}, + {&http.Request{Host: "abc.com", URL: mustParse("/foo/bar/")}, "http://foo.com:3000", globEnabled}, // do not match on host but maybe on path - {&http.Request{Host: "def.com", URL: mustParse("/")}, "http://foo.com:800", false}, - {&http.Request{Host: "def.com", URL: mustParse("/bar")}, "http://foo.com:800", false}, - {&http.Request{Host: "def.com", URL: mustParse("/foo")}, "http://foo.com:900", false}, + {&http.Request{Host: "def.com", URL: mustParse("/")}, "http://foo.com:800", globEnabled}, + {&http.Request{Host: "def.com", URL: mustParse("/bar")}, "http://foo.com:800", globEnabled}, + {&http.Request{Host: "def.com", URL: mustParse("/foo")}, "http://foo.com:900", globEnabled}, // strip default port - {&http.Request{Host: "abc.com:80", URL: mustParse("/")}, "http://foo.com:1000", false}, - {&http.Request{Host: "abc.com:443", URL: mustParse("/"), TLS: &tls.ConnectionState{}}, "http://foo.com:1000", false}, + {&http.Request{Host: "abc.com:80", URL: mustParse("/")}, "http://foo.com:1000", globEnabled}, + {&http.Request{Host: "abc.com:443", URL: mustParse("/"), TLS: &tls.ConnectionState{}}, "http://foo.com:1000", globEnabled}, // not using default port - {&http.Request{Host: "abc.com:443", URL: mustParse("/")}, "http://foo.com:800", false}, - {&http.Request{Host: "abc.com:80", URL: mustParse("/"), TLS: &tls.ConnectionState{}}, "http://foo.com:800", false}, + {&http.Request{Host: "abc.com:443", URL: mustParse("/")}, "http://foo.com:800", globEnabled}, + {&http.Request{Host: "abc.com:80", URL: mustParse("/"), TLS: &tls.ConnectionState{}}, "http://foo.com:800", globEnabled}, // glob match the host - {&http.Request{Host: "x.abc.com", URL: mustParse("/")}, "http://foo.com:4000", false}, - {&http.Request{Host: "y.abc.com", URL: mustParse("/abc")}, "http://foo.com:4000", false}, - {&http.Request{Host: "x.abc.com", URL: mustParse("/foo/")}, "http://foo.com:5000", false}, - {&http.Request{Host: "y.abc.com", URL: mustParse("/foo/")}, "http://foo.com:5000", false}, - {&http.Request{Host: ".abc.com", URL: mustParse("/foo/")}, "http://foo.com:5000", false}, - {&http.Request{Host: "x.y.abc.com", URL: mustParse("/foo/")}, "http://foo.com:5000", false}, - {&http.Request{Host: "y.abc.com:80", URL: mustParse("/foo/")}, "http://foo.com:5000", false}, - {&http.Request{Host: "x.aaa.abc.com", URL: mustParse("/")}, "http://foo.com:6000", false}, - {&http.Request{Host: "x.aaa.abc.com", URL: mustParse("/foo")}, "http://foo.com:6000", false}, - {&http.Request{Host: "x.bbb.abc.com", URL: mustParse("/")}, "http://foo.com:6100", false}, - {&http.Request{Host: "x.bbb.abc.com", URL: mustParse("/foo")}, "http://foo.com:6100", false}, - {&http.Request{Host: "y.abc.com:443", URL: mustParse("/foo/"), TLS: &tls.ConnectionState{}}, "http://foo.com:5000", false}, + {&http.Request{Host: "x.abc.com", URL: mustParse("/")}, "http://foo.com:4000", globEnabled}, + {&http.Request{Host: "y.abc.com", URL: mustParse("/abc")}, "http://foo.com:4000", globEnabled}, + {&http.Request{Host: "x.abc.com", URL: mustParse("/foo/")}, "http://foo.com:5000", globEnabled}, + {&http.Request{Host: "y.abc.com", URL: mustParse("/foo/")}, "http://foo.com:5000", globEnabled}, + {&http.Request{Host: ".abc.com", URL: mustParse("/foo/")}, "http://foo.com:5000", globEnabled}, + {&http.Request{Host: "x.y.abc.com", URL: mustParse("/foo/")}, "http://foo.com:5000", globEnabled}, + {&http.Request{Host: "y.abc.com:80", URL: mustParse("/foo/")}, "http://foo.com:5000", globEnabled}, + {&http.Request{Host: "x.aaa.abc.com", URL: mustParse("/")}, "http://foo.com:6000", globEnabled}, + {&http.Request{Host: "x.aaa.abc.com", URL: mustParse("/foo")}, "http://foo.com:6000", globEnabled}, + {&http.Request{Host: "x.bbb.abc.com", URL: mustParse("/")}, "http://foo.com:6100", globEnabled}, + {&http.Request{Host: "x.bbb.abc.com", URL: mustParse("/foo")}, "http://foo.com:6100", globEnabled}, + {&http.Request{Host: "y.abc.com:443", URL: mustParse("/foo/"), TLS: &tls.ConnectionState{}}, "http://foo.com:5000", globEnabled}, // exact match has precedence over glob match - {&http.Request{Host: "z.abc.com", URL: mustParse("/foo/")}, "http://foo.com:3100", false}, + {&http.Request{Host: "z.abc.com", URL: mustParse("/foo/")}, "http://foo.com:3100", globEnabled}, // explicit port on route - {&http.Request{Host: "xyz.com", URL: mustParse("/")}, "https://xyz.com", false}, + {&http.Request{Host: "xyz.com", URL: mustParse("/")}, "https://xyz.com", globEnabled}, } for i, tt := range tests { - if got, want := tbl.Lookup(tt.req, "", rndPicker, prefixMatcher, tt.globDisabled).URL.String(), tt.dst; got != want { + if got, want := tbl.Lookup(tt.req, "", rndPicker, prefixMatcher, tt.globEnabled).URL.String(), tt.dst; got != want { t.Errorf("%d: got %v want %v", i, got, want) } }