diff --git a/docs/basics.md b/docs/basics.md index 29cea3c44..c7b0f6043 100644 --- a/docs/basics.md +++ b/docs/basics.md @@ -191,6 +191,25 @@ backend = "backend2" rule = "Path:/test1,/test2" ``` +### Rules Order + +When combining `Modifier` rules with `Matcher` rules, it is important to remember that `Modifier` rules **ALWAYS** apply after the `Matcher` rules. +The following rules are both `Matchers` and `Modifiers`, so the `Matcher` portion of the rule will apply first, and the `Modifier` will apply later. + +- `PathStrip` +- `PathStripRegex` +- `PathPrefixStrip` +- `PathPrefixStripRegex` + +`Modifiers` will be applied in a pre-determined order regardless of their order in the `rule` configuration section. + +1. `PathStrip` +2. `PathPrefixStrip` +3. `PathStripRegex` +4. `PathPrefixStripRegex` +5. `AddPrefix` +6. `ReplacePath` + ### Priorities By default, routes will be sorted (in descending order) using rules length (to avoid path overlap): diff --git a/server/server.go b/server/server.go index 1547270f6..5630ffc2a 100644 --- a/server/server.go +++ b/server/server.go @@ -776,7 +776,17 @@ func (server *Server) loadConfig(configurations configs, globalConfiguration Glo } func (server *Server) wireFrontendBackend(serverRoute *serverRoute, handler http.Handler) { - // add prefix + // path replace - This needs to always be the very last on the handler chain (first in the order in this function) + // -- Replacing Path should happen at the very end of the Modifier chain, after all the Matcher+Modifiers ran + if len(serverRoute.replacePath) > 0 { + handler = &middlewares.ReplacePath{ + Path: serverRoute.replacePath, + Handler: handler, + } + } + + // add prefix - This needs to always be right before ReplacePath on the chain (second in order in this function) + // -- Adding Path Prefix should happen after all *Strip Matcher+Modifiers ran, but before Replace (in case it's configured) if len(serverRoute.addPrefix) > 0 { handler = &middlewares.AddPrefix{ Prefix: serverRoute.addPrefix, @@ -797,14 +807,6 @@ func (server *Server) wireFrontendBackend(serverRoute *serverRoute, handler http handler = middlewares.NewStripPrefixRegex(handler, serverRoute.stripPrefixesRegex) } - // path replace - if len(serverRoute.replacePath) > 0 { - handler = &middlewares.ReplacePath{ - Path: serverRoute.replacePath, - Handler: handler, - } - } - serverRoute.route.Handler(handler) } diff --git a/server/server_test.go b/server/server_test.go index bc41bdd30..96c4daae6 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -2,13 +2,16 @@ package server import ( "fmt" + "net/http" "net/url" "reflect" "testing" "time" "github.com/containous/flaeg" + "github.com/containous/mux" "github.com/containous/traefik/healthcheck" + "github.com/containous/traefik/testhelpers" "github.com/containous/traefik/types" "github.com/vulcand/oxy/roundrobin" ) @@ -27,6 +30,85 @@ func (lb *testLoadBalancer) Servers() []*url.URL { return []*url.URL{} } +func TestServerMultipleFrontendRules(t *testing.T) { + cases := []struct { + expression string + requestURL string + expectedURL string + }{ + { + expression: "Host:foo.bar", + requestURL: "http://foo.bar", + expectedURL: "http://foo.bar", + }, + { + expression: "PathPrefix:/management;ReplacePath:/health", + requestURL: "http://foo.bar/management", + expectedURL: "http://foo.bar/health", + }, + { + expression: "Host:foo.bar;AddPrefix:/blah", + requestURL: "http://foo.bar/baz", + expectedURL: "http://foo.bar/blah/baz", + }, + { + expression: "PathPrefixStripRegex:/one/{two}/{three:[0-9]+}", + requestURL: "http://foo.bar/one/some/12345/four", + expectedURL: "http://foo.bar/four", + }, + { + expression: "PathPrefixStripRegex:/one/{two}/{three:[0-9]+};AddPrefix:/zero", + requestURL: "http://foo.bar/one/some/12345/four", + expectedURL: "http://foo.bar/zero/four", + }, + { + expression: "AddPrefix:/blah;ReplacePath:/baz", + requestURL: "http://foo.bar/hello", + expectedURL: "http://foo.bar/baz", + }, + { + expression: "PathPrefixStrip:/management;ReplacePath:/health", + requestURL: "http://foo.bar/management", + expectedURL: "http://foo.bar/health", + }, + } + + for _, test := range cases { + test := test + t.Run(test.expression, func(t *testing.T) { + t.Parallel() + + router := mux.NewRouter() + route := router.NewRoute() + serverRoute := &serverRoute{route: route} + rules := &Rules{route: serverRoute} + + expression := test.expression + routeResult, err := rules.Parse(expression) + + if err != nil { + t.Fatalf("Error while building route for %s: %+v", expression, err) + } + + request := testhelpers.MustNewRequest(http.MethodGet, test.requestURL, nil) + routeMatch := routeResult.Match(request, &mux.RouteMatch{Route: routeResult}) + + if !routeMatch { + t.Fatalf("Rule %s doesn't match", expression) + } + + server := new(Server) + + server.wireFrontendBackend(serverRoute, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.String() != test.expectedURL { + t.Fatalf("got URL %s, expected %s", r.URL.String(), test.expectedURL) + } + })) + serverRoute.route.GetHandler().ServeHTTP(nil, request) + }) + } +} + func TestServerLoadConfigHealthCheckOptions(t *testing.T) { healthChecks := []*types.HealthCheck{ nil, diff --git a/testhelpers/helpers.go b/testhelpers/helpers.go index e75be547e..0d2bb9802 100644 --- a/testhelpers/helpers.go +++ b/testhelpers/helpers.go @@ -1,6 +1,21 @@ package testhelpers +import ( + "fmt" + "io" + "net/http" +) + // Intp returns a pointer to the given integer value. func Intp(i int) *int { return &i } + +// MustNewRequest creates a new http get request or panics if it can't +func MustNewRequest(method, urlStr string, body io.Reader) *http.Request { + request, err := http.NewRequest(method, urlStr, body) + if err != nil { + panic(fmt.Sprintf("failed to create HTTP %s Request for '%s': %s", method, urlStr, err)) + } + return request +}