From ad9e7b6ae98c993ff5178ef368d7d1ef5f9e4b5c Mon Sep 17 00:00:00 2001 From: "R.B. Boyer" Date: Tue, 23 Jul 2019 20:56:39 -0500 Subject: [PATCH] connect: allow L7 routers to match on http methods (#6164) Fixes #6158 --- agent/config/runtime_test.go | 3 ++ agent/structs/config_entry_discoverychain.go | 33 ++++++++++++++++--- .../config_entry_discoverychain_test.go | 22 +++++++++++++ agent/structs/config_entry_test.go | 3 ++ agent/xds/routes.go | 13 ++++++++ agent/xds/routes_test.go | 18 ++++++++++ ...connect-proxy-with-chain-and-router.golden | 32 ++++++++++++++++++ api/config_entry_discoverychain.go | 4 +-- command/config/write/config_write_test.go | 3 ++ .../config-entries/service-router.html.md | 3 ++ 10 files changed, 126 insertions(+), 8 deletions(-) diff --git a/agent/config/runtime_test.go b/agent/config/runtime_test.go index 8fc8b7a85f..d545e790af 100644 --- a/agent/config/runtime_test.go +++ b/agent/config/runtime_test.go @@ -3119,6 +3119,7 @@ func TestConfigFlagsAndEdgecases(t *testing.T) { "match": { "http": { "path_prefix": "/foo", + "methods": [ "GET", "DELETE" ], "query_param": [ { "name": "hack1", @@ -3202,6 +3203,7 @@ func TestConfigFlagsAndEdgecases(t *testing.T) { match { http { path_prefix = "/foo" + methods = [ "GET", "DELETE" ] query_param = [ { name = "hack1" @@ -3284,6 +3286,7 @@ func TestConfigFlagsAndEdgecases(t *testing.T) { Match: &structs.ServiceRouteMatch{ HTTP: &structs.ServiceRouteHTTPMatch{ PathPrefix: "/foo", + Methods: []string{"GET", "DELETE"}, QueryParam: []structs.ServiceRouteHTTPMatchQueryParam{ { Name: "hack1", diff --git a/agent/structs/config_entry_discoverychain.go b/agent/structs/config_entry_discoverychain.go index fdfe9c527d..935e7c8738 100644 --- a/agent/structs/config_entry_discoverychain.go +++ b/agent/structs/config_entry_discoverychain.go @@ -58,6 +58,21 @@ func (e *ServiceRouterConfigEntry) Normalize() error { e.Kind = ServiceRouter + for _, route := range e.Routes { + if route.Match == nil || route.Match.HTTP == nil { + continue + } + + httpMatch := route.Match.HTTP + if len(httpMatch.Methods) == 0 { + continue + } + + for j := 0; j < len(httpMatch.Methods); j++ { + httpMatch.Methods[j] = strings.ToUpper(httpMatch.Methods[j]) + } + } + return nil } @@ -139,6 +154,16 @@ func (e *ServiceRouterConfigEntry) Validate() error { return fmt.Errorf("Route[%d] QueryParam[%d] should only contain one of Present, Exact, or Regex", i, j) } } + + if len(route.Match.HTTP.Methods) > 0 { + found := make(map[string]struct{}) + for _, m := range route.Match.HTTP.Methods { + if _, ok := found[m]; ok { + return fmt.Errorf("Route[%d] Methods contains %q more than once", i, m) + } + found[m] = struct{}{} + } + } } if route.Destination != nil { @@ -218,9 +243,7 @@ type ServiceRouteHTTPMatch struct { Header []ServiceRouteHTTPMatchHeader `json:",omitempty"` QueryParam []ServiceRouteHTTPMatchQueryParam `json:",omitempty"` - - // TODO(rb): reenable Methods - // Methods []string `json:",omitempty"` + Methods []string `json:",omitempty"` } func (m *ServiceRouteHTTPMatch) IsEmpty() bool { @@ -228,8 +251,8 @@ func (m *ServiceRouteHTTPMatch) IsEmpty() bool { m.PathPrefix == "" && m.PathRegex == "" && len(m.Header) == 0 && - len(m.QueryParam) == 0 - // && len(m.Methods) == 0 + len(m.QueryParam) == 0 && + len(m.Methods) == 0 } type ServiceRouteHTTPMatchHeader struct { diff --git a/agent/structs/config_entry_discoverychain_test.go b/agent/structs/config_entry_discoverychain_test.go index d4982f91dc..09f7c2c2f9 100644 --- a/agent/structs/config_entry_discoverychain_test.go +++ b/agent/structs/config_entry_discoverychain_test.go @@ -1099,6 +1099,28 @@ func TestServiceRouterConfigEntry(t *testing.T) { }), validateErr: "cannot make use of PrefixRewrite without configuring either PathExact or PathPrefix", }, + //////////////// + { + name: "route with method matches", + entry: makerouter(routeMatch(httpMatch(&ServiceRouteHTTPMatch{ + Methods: []string{ + "get", "POST", "dElEtE", + }, + }))), + check: func(t *testing.T, entry *ServiceRouterConfigEntry) { + m := entry.Routes[0].Match.HTTP.Methods + require.Equal(t, []string{"GET", "POST", "DELETE"}, m) + }, + }, + { + name: "route with method matches repeated", + entry: makerouter(routeMatch(httpMatch(&ServiceRouteHTTPMatch{ + Methods: []string{ + "GET", "DELETE", "get", + }, + }))), + validateErr: "Methods contains \"GET\" more than once", + }, } for _, tc := range cases { diff --git a/agent/structs/config_entry_test.go b/agent/structs/config_entry_test.go index 0e8f7fce95..7576dce8a6 100644 --- a/agent/structs/config_entry_test.go +++ b/agent/structs/config_entry_test.go @@ -168,6 +168,7 @@ func TestDecodeConfigEntry(t *testing.T) { match { http { path_prefix = "/foo" + methods = [ "GET", "DELETE" ] query_param = [ { name = "hack1" @@ -246,6 +247,7 @@ func TestDecodeConfigEntry(t *testing.T) { Match { HTTP { PathPrefix = "/foo" + Methods = [ "GET", "DELETE" ] QueryParam = [ { Name = "hack1" @@ -324,6 +326,7 @@ func TestDecodeConfigEntry(t *testing.T) { Match: &ServiceRouteMatch{ HTTP: &ServiceRouteHTTPMatch{ PathPrefix: "/foo", + Methods: []string{"GET", "DELETE"}, QueryParam: []ServiceRouteHTTPMatchQueryParam{ { Name: "hack1", diff --git a/agent/xds/routes.go b/agent/xds/routes.go index b5f566c99f..ee836d5b39 100644 --- a/agent/xds/routes.go +++ b/agent/xds/routes.go @@ -3,6 +3,7 @@ package xds import ( "errors" "fmt" + "strings" "github.com/gogo/protobuf/proto" @@ -252,6 +253,18 @@ func makeRouteMatchForDiscoveryRoute(discoveryRoute *structs.DiscoveryRoute, pro } } + if len(match.HTTP.Methods) > 0 { + methodHeaderRegex := strings.Join(match.HTTP.Methods, "|") + + eh := &envoyroute.HeaderMatcher{ + Name: ":method", + HeaderMatchSpecifier: &envoyroute.HeaderMatcher_RegexMatch{ + RegexMatch: methodHeaderRegex, + }, + } + em.Headers = append(em.Headers, eh) + } + if len(match.HTTP.QueryParam) > 0 { em.QueryParameters = make([]*envoyroute.QueryParameterMatcher, 0, len(match.HTTP.QueryParam)) for _, qm := range match.HTTP.QueryParam { diff --git a/agent/xds/routes_test.go b/agent/xds/routes_test.go index 2c102e72f2..436d0135d1 100644 --- a/agent/xds/routes_test.go +++ b/agent/xds/routes_test.go @@ -190,6 +190,24 @@ func TestRoutesFromSnapshot(t *testing.T) { }), Destination: toService("hdr-regex"), }, + { + Match: httpMatch(&structs.ServiceRouteHTTPMatch{ + Methods: []string{"GET", "PUT"}, + }), + Destination: toService("just-methods"), + }, + { + Match: httpMatch(&structs.ServiceRouteHTTPMatch{ + Header: []structs.ServiceRouteHTTPMatchHeader{ + { + Name: "x-debug", + Exact: "exact", + }, + }, + Methods: []string{"GET", "PUT"}, + }), + Destination: toService("hdr-exact-with-method"), + }, { Match: httpMatchParam(structs.ServiceRouteHTTPMatchQueryParam{ Name: "secretparam1", diff --git a/agent/xds/testdata/routes/connect-proxy-with-chain-and-router.golden b/agent/xds/testdata/routes/connect-proxy-with-chain-and-router.golden index c61146ac5b..8380ea490e 100644 --- a/agent/xds/testdata/routes/connect-proxy-with-chain-and-router.golden +++ b/agent/xds/testdata/routes/connect-proxy-with-chain-and-router.golden @@ -120,6 +120,38 @@ "cluster": "hdr-regex.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul" } }, + { + "match": { + "prefix": "/", + "headers": [ + { + "name": ":method", + "regexMatch": "GET|PUT" + } + ] + }, + "route": { + "cluster": "just-methods.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul" + } + }, + { + "match": { + "prefix": "/", + "headers": [ + { + "name": "x-debug", + "exactMatch": "exact" + }, + { + "name": ":method", + "regexMatch": "GET|PUT" + } + ] + }, + "route": { + "cluster": "hdr-exact-with-method.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul" + } + }, { "match": { "prefix": "/", diff --git a/api/config_entry_discoverychain.go b/api/config_entry_discoverychain.go index 61c0bbb32a..8623618afc 100644 --- a/api/config_entry_discoverychain.go +++ b/api/config_entry_discoverychain.go @@ -33,9 +33,7 @@ type ServiceRouteHTTPMatch struct { Header []ServiceRouteHTTPMatchHeader `json:",omitempty"` QueryParam []ServiceRouteHTTPMatchQueryParam `json:",omitempty"` - - // TODO(rb): reenable Methods - // Methods []string `json:",omitempty"` + Methods []string `json:",omitempty"` } type ServiceRouteHTTPMatchHeader struct { diff --git a/command/config/write/config_write_test.go b/command/config/write/config_write_test.go index 45f6f564f0..4b15ea864e 100644 --- a/command/config/write/config_write_test.go +++ b/command/config/write/config_write_test.go @@ -262,6 +262,7 @@ func TestParseConfigEntry(t *testing.T) { match { http { path_prefix = "/foo" + methods = [ "GET", "DELETE" ] query_param = [ { name = "hack1" @@ -340,6 +341,7 @@ func TestParseConfigEntry(t *testing.T) { Match { HTTP { PathPrefix = "/foo" + Methods = [ "GET", "DELETE" ] QueryParam = [ { Name = "hack1" @@ -418,6 +420,7 @@ func TestParseConfigEntry(t *testing.T) { Match: &api.ServiceRouteMatch{ HTTP: &api.ServiceRouteHTTPMatch{ PathPrefix: "/foo", + Methods: []string{"GET", "DELETE"}, QueryParam: []api.ServiceRouteHTTPMatchQueryParam{ { Name: "hack1", diff --git a/website/source/docs/agent/config-entries/service-router.html.md b/website/source/docs/agent/config-entries/service-router.html.md index 3184deb936..5fd84e9c46 100644 --- a/website/source/docs/agent/config-entries/service-router.html.md +++ b/website/source/docs/agent/config-entries/service-router.html.md @@ -193,6 +193,9 @@ routes = [ At most only one of `Exact`, `Regex`, or `Present` may be configured. + - `Methods` `(array)` - A list of HTTP methods for which this match + applies. If unspecified all http methods are matched. + - `Destination` `(ServiceRouteDestination: )` - Controls how to proxy the actual matching request to a service.