Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue #548 added enable/disable glob matching #550

Merged
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 11 additions & 10 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
DisableGlobMatching bool
}

type CertSource struct {
Expand Down
1 change: 1 addition & 0 deletions config/default.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,4 +77,5 @@ var defaultConfig = &Config{
Color: "light-green",
Access: "rw",
},
DisableGlobMatching: false,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's the default so you can remove it.

}
1 change: 1 addition & 0 deletions config/load.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.DisableGlobMatching, "glob.matching.disabled", defaultConfig.DisableGlobMatching, "Disable Glob Matching on routes, one of [true, false]")

// deprecated flags
var proxyLogRoutes string
Expand Down
11 changes: 11 additions & 0 deletions docs/content/ref/glob.matching.disabled.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
title: "glob.matching.disabled"
---

`glob.matching.disabled` Disables glob matching on route lookups.

Valid options are `true`, `false`

The default is

glob.matching.disabled = false
11 changes: 11 additions & 0 deletions fabio.properties
Original file line number Diff line number Diff line change
Expand Up @@ -743,6 +743,17 @@
# registry.consul.checksRequired = one


# glob.matching.disabled Disables glob matching on route lookups
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/Disables/disables/

# 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.disabled = false


# metrics.target configures the backend the metrics values are
# sent to.
#
Expand Down
3 changes: 2 additions & 1 deletion main.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ func main() {

func newHTTPProxy(cfg *config.Config) http.Handler {
var w io.Writer
globDisabled := cfg.DisableGlobMatching
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the purpose of the var?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not needed. I'll update.

switch cfg.Log.AccessTarget {
case "":
log.Printf("[INFO] Access logging disabled")
Expand Down Expand Up @@ -183,7 +184,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, globDisabled)
if t == nil {
notFound.Inc(1)
log.Print("[WARN] No route for ", r.Host, r.URL)
Expand Down
23 changes: 16 additions & 7 deletions proxy/http_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,8 @@ func TestProxyNoRouteStatus(t *testing.T) {
}

func TestProxyStripsPath(t *testing.T) {
//Glob Matching True
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you add this var to all tests you can also add some constants, e.g.

const (
    // helper constants for the Lookup function
    globEnabled = false
    globDisabled = true
)

globDisabled := false
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
switch r.RequestURI {
case "/bar":
Expand All @@ -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"], globDisabled)
},
})
defer proxy.Close()
Expand All @@ -215,6 +217,7 @@ 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{
Expand All @@ -224,7 +227,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"], globDisabled)
},
})
defer proxy.Close()
Expand Down Expand Up @@ -266,12 +269,13 @@ 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"])
return tbl.Lookup(r, "", route.Picker["rr"], route.Matcher["prefix"], globDisabled)
},
})
defer proxy.Close()
Expand Down Expand Up @@ -306,11 +310,12 @@ 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"])
return tbl.Lookup(r, "", route.Picker["rr"], route.Matcher["prefix"], globDisabled)
},
})
defer proxy.Close()
Expand Down Expand Up @@ -472,13 +477,14 @@ 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"])
return tbl.Lookup(r, "", route.Picker["rr"], route.Matcher["prefix"], globDisabled)
},
})
defer proxy.Close()
Expand All @@ -497,6 +503,7 @@ func TestProxyHTTPSUpstreamSkipVerify(t *testing.T) {
server.TLS = &tls.Config{}
server.StartTLS()
defer server.Close()
globDisabled := false

proxy := httptest.NewServer(&HTTPProxy{
Config: config.Proxy{},
Expand All @@ -506,7 +513,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"], globDisabled)
},
})
defer proxy.Close()
Expand Down Expand Up @@ -699,6 +706,8 @@ func BenchmarkProxyLogger(b *testing.B) {
b.Fatal("logger.NewHTTPLogger:", err)
}

globDisabled := false

proxy := &HTTPProxy{
Config: config.Proxy{
LocalIP: "1.1.1.1",
Expand All @@ -707,7 +716,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"], globDisabled)
},
Logger: l,
}
Expand Down
4 changes: 3 additions & 1 deletion proxy/listen_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ func TestGracefulShutdown(t *testing.T) {
}))
defer srv.Close()

globDisabled := false

// start proxy
addr := "127.0.0.1:57777"
var wg sync.WaitGroup
Expand All @@ -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"], globDisabled)
},
}
l := config.Listen{Addr: addr}
Expand Down
6 changes: 4 additions & 2 deletions proxy/ws_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ func TestProxyWSUpstream(t *testing.T) {
defer wssServer.Close()
t.Log("Started WSS server: ", wssServer.URL)

globDisabled := false

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"
Expand All @@ -44,7 +46,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"], globDisabled)
},
})
defer httpProxy.Close()
Expand All @@ -56,7 +58,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"], globDisabled)
},
})
httpsProxy.TLS = tlsServerConfig()
Expand Down
4 changes: 3 additions & 1 deletion route/issue57_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ func TestIssue57(t *testing.T) {
route del svcb`,
}

globDisabled := false

req := &http.Request{URL: mustParse("/foo")}
want := "http://foo.com:800"

Expand All @@ -32,7 +34,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, globDisabled)
if target == nil {
t.Fatalf("%d: got %v want %v", i, target, want)
}
Expand Down
4 changes: 3 additions & 1 deletion route/route_bench_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,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
globDisabled := false
for pb.Next() {
t.Lookup(reqs[n%k], "", pick, match)
t.Lookup(reqs[n%k], "", pick, match, globDisabled)
n++
}
}
36 changes: 33 additions & 3 deletions route/table.go
Original file line number Diff line number Diff line change
Expand Up @@ -295,10 +295,12 @@ 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,) (hosts []string) {
host := normalizeHost(req.Host, req.TLS != nil)
for pattern := range t {
normpat := normalizeHost(pattern, req.TLS != nil)
// TODO setup compiled GLOBs in a separate MAP
// TODO Issue #548
g := glob.MustCompile(normpat)
if g.Match(host) {
hosts = append(hosts, pattern)
Expand Down Expand Up @@ -327,6 +329,25 @@ func (t Table) matchingHosts(req *http.Request) (hosts []string) {
return
}


// Issue 548 - Added separate func
//
// matchingHostNoGlob returns the route from the
// routing table which matches the normalized request hostname.
func (t Table) matchingHostNoGlob(req *http.Request) (hosts []string) {
host := normalizeHost(req.Host, req.TLS != nil)

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
}
}
return
}

// Reverse returns its argument string reversed rune-wise left to right.
//
// taken from https://github.com/golang/example/blob/master/stringutil/reverse.go
Expand All @@ -342,7 +363,9 @@ 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, globDisabled bool) (target *Target) {

var hosts []string
if trace != "" {
if len(trace) > 16 {
trace = trace[:15]
Expand All @@ -352,7 +375,14 @@ 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)
// if globDisabled then match without Glob
// Issue 548
if globDisabled {
hosts = t.matchingHostNoGlob(req)
} else {
hosts = t.matchingHosts(req)
}

if trace != "" {
log.Printf("[TRACE] %s Matching hosts: %v", trace, hosts)
}
Expand Down
Loading