From 062a8ea6952a50dd3bf5cf734de37e669112bf6c Mon Sep 17 00:00:00 2001 From: JeremyWhite Date: Mon, 17 Sep 2018 13:29:16 -0500 Subject: [PATCH] Issue #548 added enable/disable glob matching --- config/config.go | 21 ++++++++++---------- config/default.go | 1 + config/load.go | 1 + fabio.properties | 11 +++++++++++ main.go | 2 +- proxy/http_integration_test.go | 29 ++++++++++++++++++++------- proxy/listen_test.go | 4 +++- proxy/ws_integration_test.go | 7 +++++-- route/issue57_test.go | 6 +++++- route/route_bench_test.go | 5 ++++- route/table.go | 36 ++++++++++++++++++++++++++-------- route/table_test.go | 9 +++++++-- 12 files changed, 99 insertions(+), 33 deletions(-) diff --git a/config/config.go b/config/config.go index f8ad7e4c2..8d0fee04c 100644 --- a/config/config.go +++ b/config/config.go @@ -7,16 +7,17 @@ import ( ) type Config struct { - Proxy Proxy - Registry Registry - Listen []Listen - Log Log - Metrics Metrics - UI UI - Runtime Runtime - ProfileMode string - ProfilePath string - Insecure bool + Proxy Proxy + Registry Registry + Listen []Listen + Log Log + Metrics Metrics + UI UI + Runtime Runtime + ProfileMode string + ProfilePath string + Insecure bool + GlobMatching bool } type CertSource struct { diff --git a/config/default.go b/config/default.go index 45f68424d..55fed35bb 100644 --- a/config/default.go +++ b/config/default.go @@ -77,4 +77,5 @@ var defaultConfig = &Config{ Color: "light-green", Access: "rw", }, + GlobMatching: true, } diff --git a/config/load.go b/config/load.go index d75188471..7992a446a 100644 --- a/config/load.go +++ b/config/load.go @@ -186,6 +186,7 @@ func load(cmdline, environ, envprefix []string, props *properties.Properties) (c f.StringVar(&cfg.UI.Title, "ui.title", defaultConfig.UI.Title, "optional title for the UI") f.StringVar(&cfg.ProfileMode, "profile.mode", defaultConfig.ProfileMode, "enable profiling mode, one of [cpu, mem, mutex, block]") f.StringVar(&cfg.ProfilePath, "profile.path", defaultConfig.ProfilePath, "path to profile dump file") + f.BoolVar(&cfg.GlobMatching, "glob.matching.enabled", defaultConfig.GlobMatching, "Enable/Disable Glob Matching on routes, one of [true, false]") // deprecated flags var proxyLogRoutes string diff --git a/fabio.properties b/fabio.properties index dae2caab0..2dc80dc85 100644 --- a/fabio.properties +++ b/fabio.properties @@ -743,6 +743,17 @@ # registry.consul.checksRequired = one +# glob.matching.enabled Enables 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 +# Fabio performs a static string compare for route lookups. +# +# The default is +# +# glob.matching.enabled = true + + # metrics.target configures the backend the metrics values are # sent to. # diff --git a/main.go b/main.go index 264c30978..769a68997 100644 --- a/main.go +++ b/main.go @@ -183,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) + t := route.GetTable().Lookup(r, r.Header.Get("trace"), pick, match, cfg) 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 ddc6c4c4c..e67280ba2 100644 --- a/proxy/http_integration_test.go +++ b/proxy/http_integration_test.go @@ -175,6 +175,8 @@ func TestProxyNoRouteStatus(t *testing.T) { } func TestProxyStripsPath(t *testing.T) { + //Glob Matching True + globMatching := config.Config{GlobMatching: true} server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch r.RequestURI { case "/bar": @@ -188,7 +190,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"]) + return tbl.Lookup(r, "", route.Picker["rr"], route.Matcher["prefix"], &globMatching) }, }) defer proxy.Close() @@ -215,6 +217,8 @@ 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) + //Glob Matching True + globMatching := config.Config{GlobMatching: true} proxy := httptest.NewServer(&HTTPProxy{ Transport: &http.Transport{ @@ -224,7 +228,7 @@ func TestProxyHost(t *testing.T) { }, }, Lookup: func(r *http.Request) *route.Target { - return tbl.Lookup(r, "", route.Picker["rr"], route.Matcher["prefix"]) + return tbl.Lookup(r, "", route.Picker["rr"], route.Matcher["prefix"], &globMatching) }, }) defer proxy.Close() @@ -266,12 +270,14 @@ func TestHostRedirect(t *testing.T) { routes := "route add https-redir *:80 https://$host$path opts \"redirect=301\"\n" tbl, _ := route.NewTable(routes) + //Glob Matching True + globMatching := config.Config{GlobMatching: true} 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"]) + return tbl.Lookup(r, "", route.Picker["rr"], route.Matcher["prefix"], &globMatching) }, }) defer proxy.Close() @@ -306,11 +312,13 @@ 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) + //Glob Matching True + globMatching := config.Config{GlobMatching: true} proxy := httptest.NewServer(&HTTPProxy{ Transport: http.DefaultTransport, Lookup: func(r *http.Request) *route.Target { - return tbl.Lookup(r, "", route.Picker["rr"], route.Matcher["prefix"]) + return tbl.Lookup(r, "", route.Picker["rr"], route.Matcher["prefix"], &globMatching) }, }) defer proxy.Close() @@ -472,13 +480,15 @@ func TestProxyHTTPSUpstream(t *testing.T) { server.TLS = tlsServerConfig() server.StartTLS() defer server.Close() + //Glob Matching True + globMatching := config.Config{GlobMatching: true} 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"]) + return tbl.Lookup(r, "", route.Picker["rr"], route.Matcher["prefix"], &globMatching) }, }) defer proxy.Close() @@ -497,6 +507,8 @@ func TestProxyHTTPSUpstreamSkipVerify(t *testing.T) { server.TLS = &tls.Config{} server.StartTLS() defer server.Close() + //Glob Matching True + globMatching := config.Config{GlobMatching: true} proxy := httptest.NewServer(&HTTPProxy{ Config: config.Proxy{}, @@ -506,7 +518,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"]) + return tbl.Lookup(r, "", route.Picker["rr"], route.Matcher["prefix"], &globMatching) }, }) defer proxy.Close() @@ -699,6 +711,9 @@ func BenchmarkProxyLogger(b *testing.B) { b.Fatal("logger.NewHTTPLogger:", err) } + //Glob Matching True + globMatching := config.Config{GlobMatching: true} + proxy := &HTTPProxy{ Config: config.Proxy{ LocalIP: "1.1.1.1", @@ -707,7 +722,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"]) + return tbl.Lookup(r, "", route.Picker["rr"], route.Matcher["prefix"], &globMatching) }, Logger: l, } diff --git a/proxy/listen_test.go b/proxy/listen_test.go index 607461641..ae307d27a 100644 --- a/proxy/listen_test.go +++ b/proxy/listen_test.go @@ -22,6 +22,8 @@ func TestGracefulShutdown(t *testing.T) { })) defer srv.Close() + //Glob Matching True + globMatching := config.Config{GlobMatching: true} // start proxy addr := "127.0.0.1:57777" var wg sync.WaitGroup @@ -32,7 +34,7 @@ func TestGracefulShutdown(t *testing.T) { Transport: http.DefaultTransport, Lookup: func(r *http.Request) *route.Target { tbl, _ := route.NewTable("route add svc / " + srv.URL) - return tbl.Lookup(r, "", route.Picker["rr"], route.Matcher["prefix"]) + return tbl.Lookup(r, "", route.Picker["rr"], route.Matcher["prefix"], &globMatching) }, } l := config.Listen{Addr: addr} diff --git a/proxy/ws_integration_test.go b/proxy/ws_integration_test.go index a03977481..f1a9d8035 100644 --- a/proxy/ws_integration_test.go +++ b/proxy/ws_integration_test.go @@ -33,6 +33,9 @@ func TestProxyWSUpstream(t *testing.T) { defer wssServer.Close() t.Log("Started WSS server: ", wssServer.URL) + //Glob Matching True + globMatching := config.Config{GlobMatching: true} + routes := "route add ws /ws " + wsServer.URL + "\n" routes += "route add ws /wss " + wssServer.URL + ` opts "proto=https"` + "\n" routes += "route add ws /insecure " + wssServer.URL + ` opts "proto=https tlsskipverify=true"` + "\n" @@ -44,7 +47,7 @@ func TestProxyWSUpstream(t *testing.T) { InsecureTransport: &http.Transport{TLSClientConfig: tlsInsecureConfig()}, Lookup: func(r *http.Request) *route.Target { tbl, _ := route.NewTable(routes) - return tbl.Lookup(r, "", route.Picker["rr"], route.Matcher["prefix"]) + return tbl.Lookup(r, "", route.Picker["rr"], route.Matcher["prefix"], &globMatching) }, }) defer httpProxy.Close() @@ -56,7 +59,7 @@ func TestProxyWSUpstream(t *testing.T) { InsecureTransport: &http.Transport{TLSClientConfig: tlsInsecureConfig()}, Lookup: func(r *http.Request) *route.Target { tbl, _ := route.NewTable(routes) - return tbl.Lookup(r, "", route.Picker["rr"], route.Matcher["prefix"]) + return tbl.Lookup(r, "", route.Picker["rr"], route.Matcher["prefix"], &globMatching) }, }) httpsProxy.TLS = tlsServerConfig() diff --git a/route/issue57_test.go b/route/issue57_test.go index a0f8fb2cc..c52835996 100644 --- a/route/issue57_test.go +++ b/route/issue57_test.go @@ -1,6 +1,7 @@ package route import ( + "github.com/fabiolb/fabio/config" "net/http" "testing" ) @@ -24,6 +25,9 @@ func TestIssue57(t *testing.T) { route del svcb`, } + //Glob Matching True + globMatching := config.Config{GlobMatching: true} + req := &http.Request{URL: mustParse("/foo")} want := "http://foo.com:800" @@ -32,7 +36,7 @@ func TestIssue57(t *testing.T) { if err != nil { t.Fatalf("%d: got %v want nil", i, err) } - target := tbl.Lookup(req, "", rrPicker, prefixMatcher) + target := tbl.Lookup(req, "", rrPicker, prefixMatcher, &globMatching) 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 2f9126710..e11dd32be 100644 --- a/route/route_bench_test.go +++ b/route/route_bench_test.go @@ -2,6 +2,7 @@ package route import ( "fmt" + "github.com/fabiolb/fabio/config" "net/http" "sync" "testing" @@ -123,8 +124,10 @@ func makeRequests(t Table) []*http.Request { func benchmarkGet(t Table, match matcher, pick picker, pb *testing.PB) { reqs := makeRequests(t) k, n := len(reqs), 0 + //Glob Matching True + globMatching := config.Config{GlobMatching: true} for pb.Next() { - t.Lookup(reqs[n%k], "", pick, match) + t.Lookup(reqs[n%k], "", pick, match, &globMatching) n++ } } diff --git a/route/table.go b/route/table.go index 117543e77..caaf6b18d 100644 --- a/route/table.go +++ b/route/table.go @@ -12,6 +12,7 @@ import ( "sync" "sync/atomic" + "github.com/fabiolb/fabio/config" "github.com/fabiolb/fabio/metrics" "github.com/gobwas/glob" ) @@ -295,13 +296,32 @@ func normalizeHost(host string, tls bool) string { // matchingHosts returns all keys (host name patterns) from the // routing table which match the normalized request hostname. -func (t Table) matchingHosts(req *http.Request) (hosts []string) { +func (t Table) matchingHosts(req *http.Request, cfg *config.Config) (hosts []string) { host := normalizeHost(req.Host, req.TLS != nil) - for pattern := range t { - normpat := normalizeHost(pattern, req.TLS != nil) - g := glob.MustCompile(normpat) - if g.Match(host) { - hosts = append(hosts, pattern) + + // Issue 548 Glob matching causes performance decrease. + // + // Updated config to allow for disabling of Glob Matches + // Standard string compare is used if disabled + // glob.matching.enabled is false + if !cfg.GlobMatching { + for pattern := range t { + normpat := normalizeHost(pattern, req.TLS != nil) + if normpat == host { + //log.Printf("DEBUG Matched %s and %s", normpat, host) + hosts = append(hosts, pattern) + return + } + } + } else { //glob.matching.enabled is true (default) Performance hit + for pattern := range t { + normpat := normalizeHost(pattern, req.TLS != nil) + // TODO setup compiled GLOBs in a separate MAP as routes are added/deleted + // TODO Issue #548 + g := glob.MustCompile(normpat) + if g.Match(host) { + hosts = append(hosts, pattern) + } } } @@ -342,7 +362,7 @@ func Reverse(s string) string { // or nil if there is none. It first checks the routes for the host // and if none matches then it falls back to generic routes without // a host. This is useful for a catch-all '/' rule. -func (t Table) Lookup(req *http.Request, trace string, pick picker, match matcher) (target *Target) { +func (t Table) Lookup(req *http.Request, trace string, pick picker, match matcher, cfg *config.Config) (target *Target) { if trace != "" { if len(trace) > 16 { trace = trace[:15] @@ -352,7 +372,7 @@ func (t Table) Lookup(req *http.Request, trace string, pick picker, match matche // find matching hosts for the request // and add "no host" as the fallback option - hosts := t.matchingHosts(req) + hosts := t.matchingHosts(req, cfg) if trace != "" { log.Printf("[TRACE] %s Matching hosts: %v", trace, hosts) } diff --git a/route/table_test.go b/route/table_test.go index 808c9168b..e0ac39407 100644 --- a/route/table_test.go +++ b/route/table_test.go @@ -3,6 +3,7 @@ package route import ( "crypto/tls" "fmt" + "github.com/fabiolb/fabio/config" "math" "net/http" "reflect" @@ -492,6 +493,8 @@ func TestTableLookupIssue448(t *testing.T) { route add mock ccc.com:443/bar https://ccc.com/baz opts "redirect=301" route add mock / http://foo.com/ ` + //Glob Matching True + globMatching := config.Config{GlobMatching: true} tbl, err := NewTable(s) if err != nil { @@ -551,7 +554,7 @@ func TestTableLookupIssue448(t *testing.T) { } for i, tt := range tests { - if got, want := tbl.Lookup(tt.req, "", rndPicker, prefixMatcher).URL.String(), tt.dst; got != want { + if got, want := tbl.Lookup(tt.req, "", rndPicker, prefixMatcher, &globMatching).URL.String(), tt.dst; got != want { t.Errorf("%d: got %v want %v", i, got, want) } } @@ -573,6 +576,8 @@ func TestTableLookup(t *testing.T) { route add svc *.bbb.abc.com/ http://foo.com:6100 route add svc xyz.com:80/ https://xyz.com ` + //Glob Matching True + globMatching := config.Config{GlobMatching: true} tbl, err := NewTable(s) if err != nil { @@ -626,7 +631,7 @@ func TestTableLookup(t *testing.T) { } for i, tt := range tests { - if got, want := tbl.Lookup(tt.req, "", rndPicker, prefixMatcher).URL.String(), tt.dst; got != want { + if got, want := tbl.Lookup(tt.req, "", rndPicker, prefixMatcher, &globMatching).URL.String(), tt.dst; got != want { t.Errorf("%d: got %v want %v", i, got, want) } }