Skip to content

Commit

Permalink
b/162888708: Support wildcards in envoy route matching with dynamic r…
Browse files Browse the repository at this point in the history 
…outing (#262)

In dynamic routing mode, config manager constructs envoy route config per HttpRule. Envoy route config doesn't understand Google's Http Template syntax, so some regex re-writes must be done to support the most common OpenAPI cases.

This aims to support paths in form:
- /foo/*/bar
- /foo/**

Without this change, ESPv2 crash loops on invalid envoy config:
```
[2020-08-06 11:27:17.851][2809084][warning][config] [external/envoy/source/common/config/grpc_subscription_impl.cc:100] gRPC config for type.googleapis.com/envoy.config.listener.v3.Listener rejected: Error adding/updating listener(s) ingress_listener: bad repetition operator: **
```

Signed-off-by: Teju Nareddy <[email protected]>
  • Loading branch information
@nareddyt
nareddyt authored Aug 10, 2020
1 parent 2da67a6 commit 38f0610
Show file tree
Hide file tree
Showing 11 changed files with 320 additions and 19 deletions.
37 changes: 36 additions & 1 deletion examples/testdata/path_matcher/envoy_config.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,41 @@
"httpMethod": "GET",
"uriTemplate": "/libraries/{library_id}/shelves/{shelf_id}"
}
},
{
"operation": "1.examples_path_matcher_endpoints_cloudesf_testing_cloud_goog.Google_Autogenerated_Unrecognized_Delete_Method_Call",
"pattern": {
"httpMethod": "DELETE",
"uriTemplate": "/**"
}
},
{
"operation": "1.examples_path_matcher_endpoints_cloudesf_testing_cloud_goog.Google_Autogenerated_Unrecognized_Get_Method_Call",
"pattern": {
"httpMethod": "GET",
"uriTemplate": "/**"
}
},
{
"operation": "1.examples_path_matcher_endpoints_cloudesf_testing_cloud_goog.Google_Autogenerated_Unrecognized_Patch_Method_Call",
"pattern": {
"httpMethod": "PATCH",
"uriTemplate": "/**"
}
},
{
"operation": "1.examples_path_matcher_endpoints_cloudesf_testing_cloud_goog.Google_Autogenerated_Unrecognized_Post_Method_Call",
"pattern": {
"httpMethod": "POST",
"uriTemplate": "/**"
}
},
{
"operation": "1.examples_path_matcher_endpoints_cloudesf_testing_cloud_goog.Google_Autogenerated_Unrecognized_Put_Method_Call",
"pattern": {
"httpMethod": "PUT",
"uriTemplate": "/**"
}
}
]
}
Expand Down Expand Up @@ -238,7 +273,7 @@
"googleRe2": {
"maxProgramSize": 1000
},
"regex": "/libraries/[^\\/]+/shelves/[^\\/]+$"
"regex": "^/libraries/[^\\/]+/shelves/[^\\/]+$"
}
},
"route": {
Expand Down
4 changes: 2 additions & 2 deletions examples/testdata/path_matcher/openapi_swagger.json
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
{
"basePath": "/",
"consumes": [
"application/json"
],
Expand Down Expand Up @@ -72,5 +71,6 @@
"schemes": [
"https"
],
"swagger": "2.0"
"swagger": "2.0",
"x-google-allow": "all"
}
93 changes: 91 additions & 2 deletions examples/testdata/path_matcher/service_config_generated.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,31 @@
"name": "GetShelf",
"requestTypeUrl": "type.googleapis.com/GetShelfRequest",
"responseTypeUrl": "type.googleapis.com/Shelf"
},
{
"name": "Google_Autogenerated_Unrecognized_Get_Method_Call",
"requestTypeUrl": "type.googleapis.com/google.protobuf.Empty",
"responseTypeUrl": "type.googleapis.com/google.protobuf.Empty"
},
{
"name": "Google_Autogenerated_Unrecognized_Delete_Method_Call",
"requestTypeUrl": "type.googleapis.com/google.protobuf.Empty",
"responseTypeUrl": "type.googleapis.com/google.protobuf.Empty"
},
{
"name": "Google_Autogenerated_Unrecognized_Patch_Method_Call",
"requestTypeUrl": "type.googleapis.com/google.protobuf.Empty",
"responseTypeUrl": "type.googleapis.com/google.protobuf.Empty"
},
{
"name": "Google_Autogenerated_Unrecognized_Post_Method_Call",
"requestTypeUrl": "type.googleapis.com/google.protobuf.Empty",
"responseTypeUrl": "type.googleapis.com/google.protobuf.Empty"
},
{
"name": "Google_Autogenerated_Unrecognized_Put_Method_Call",
"requestTypeUrl": "type.googleapis.com/google.protobuf.Empty",
"responseTypeUrl": "type.googleapis.com/google.protobuf.Empty"
}
],
"name": "1.examples_path_matcher_endpoints_cloudesf_testing_cloud_goog",
Expand All @@ -15,7 +40,25 @@
"version": "1.0.0"
}
],
"authentication": {},
"authentication": {
"rules": [
{
"selector": "1.examples_path_matcher_endpoints_cloudesf_testing_cloud_goog.Google_Autogenerated_Unrecognized_Get_Method_Call"
},
{
"selector": "1.examples_path_matcher_endpoints_cloudesf_testing_cloud_goog.Google_Autogenerated_Unrecognized_Delete_Method_Call"
},
{
"selector": "1.examples_path_matcher_endpoints_cloudesf_testing_cloud_goog.Google_Autogenerated_Unrecognized_Patch_Method_Call"
},
{
"selector": "1.examples_path_matcher_endpoints_cloudesf_testing_cloud_goog.Google_Autogenerated_Unrecognized_Post_Method_Call"
},
{
"selector": "1.examples_path_matcher_endpoints_cloudesf_testing_cloud_goog.Google_Autogenerated_Unrecognized_Put_Method_Call"
}
]
},
"backend": {
"rules": [
{
Expand Down Expand Up @@ -43,10 +86,30 @@
{
"get": "/libraries/{library_id}/shelves/{shelf_id}",
"selector": "1.examples_path_matcher_endpoints_cloudesf_testing_cloud_goog.GetShelf"
},
{
"get": "/**",
"selector": "1.examples_path_matcher_endpoints_cloudesf_testing_cloud_goog.Google_Autogenerated_Unrecognized_Get_Method_Call"
},
{
"delete": "/**",
"selector": "1.examples_path_matcher_endpoints_cloudesf_testing_cloud_goog.Google_Autogenerated_Unrecognized_Delete_Method_Call"
},
{
"patch": "/**",
"selector": "1.examples_path_matcher_endpoints_cloudesf_testing_cloud_goog.Google_Autogenerated_Unrecognized_Patch_Method_Call"
},
{
"post": "/**",
"selector": "1.examples_path_matcher_endpoints_cloudesf_testing_cloud_goog.Google_Autogenerated_Unrecognized_Post_Method_Call"
},
{
"put": "/**",
"selector": "1.examples_path_matcher_endpoints_cloudesf_testing_cloud_goog.Google_Autogenerated_Unrecognized_Put_Method_Call"
}
]
},
"id": "2020-07-14r0",
"id": "2020-08-05r0",
"logging": {
"producerDestinations": [
{
Expand Down Expand Up @@ -608,13 +671,39 @@
],
"name": "GetShelfRequest",
"sourceContext": {}
},
{
"name": "google.protobuf.Empty",
"sourceContext": {
"fileName": "struct.proto"
}
}
],
"usage": {
"rules": [
{
"allowUnregisteredCalls": true,
"selector": "1.examples_path_matcher_endpoints_cloudesf_testing_cloud_goog.GetShelf"
},
{
"allowUnregisteredCalls": true,
"selector": "1.examples_path_matcher_endpoints_cloudesf_testing_cloud_goog.Google_Autogenerated_Unrecognized_Get_Method_Call"
},
{
"allowUnregisteredCalls": true,
"selector": "1.examples_path_matcher_endpoints_cloudesf_testing_cloud_goog.Google_Autogenerated_Unrecognized_Delete_Method_Call"
},
{
"allowUnregisteredCalls": true,
"selector": "1.examples_path_matcher_endpoints_cloudesf_testing_cloud_goog.Google_Autogenerated_Unrecognized_Patch_Method_Call"
},
{
"allowUnregisteredCalls": true,
"selector": "1.examples_path_matcher_endpoints_cloudesf_testing_cloud_goog.Google_Autogenerated_Unrecognized_Post_Method_Call"
},
{
"allowUnregisteredCalls": true,
"selector": "1.examples_path_matcher_endpoints_cloudesf_testing_cloud_goog.Google_Autogenerated_Unrecognized_Put_Method_Call"
}
]
}
Expand Down
68 changes: 68 additions & 0 deletions src/go/configgenerator/route_generator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,74 @@ func TestMakeRouteConfig(t *testing.T) {
]
}
]
}`,
},
{
desc: "Wildcard paths for remote backend",
fakeServiceConfig: &confpb.Service{
Name: testProjectName,
Apis: []*apipb.Api{
{
Name: testApiName,
},
},
Backend: &confpb.Backend{
Rules: []*confpb.BackendRule{
{
Selector: "endpoints.examples.bookstore.Bookstore.Foo",
Address: "https://testapipb.com/foo",
PathTranslation: confpb.BackendRule_CONSTANT_ADDRESS,
Authentication: &confpb.BackendRule_JwtAudience{
JwtAudience: "bar.com",
},
},
},
},
Http: &annotationspb.Http{
Rules: []*annotationspb.HttpRule{
{
Selector: "endpoints.examples.bookstore.Bookstore.Foo",
Pattern: &annotationspb.HttpRule_Get{
Get: "/v1/{book_name=*}/test/**",
},
},
},
},
},
wantRouteConfig: `
{
"name": "local_route",
"virtualHosts": [
{
"domains": [
"*"
],
"name": "backend",
"routes": [
{
"match": {
"headers": [
{
"exactMatch": "GET",
"name": ":method"
}
],
"safeRegex": {
"googleRe2": {
"maxProgramSize": 1000
},
"regex": "^/v1/[^\\/]+/test/.*$"
}
},
"route": {
"cluster": "testapipb.com:443",
"hostRewriteLiteral": "testapipb.com",
"timeout": "15s"
}
}
]
}
]
}`,
},
}
Expand Down
2 changes: 1 addition & 1 deletion src/go/configinfo/service_info.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ func (s *ServiceInfo) processHttpRule() error {
for _, r := range s.ServiceConfig().GetHttp().GetRules() {
method := s.Methods[r.GetSelector()]
for _, httpRule := range method.HttpRule {
if httpRule.HttpMethod != "OPTIONS" {
if httpRule.HttpMethod != util.OPTIONS {
matcher := util.WildcardMatcherForPath(httpRule.UriTemplate)
if matcher == "" {
matcher = httpRule.UriTemplate
Expand Down
2 changes: 1 addition & 1 deletion src/go/configmanager/testdata/test_fixed_mode_dynamic_routing.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,7 @@ var (
"googleRe2":{
"maxProgramSize":1000
},
"regex":"/pet/[^\\/]+$"
"regex":"^/pet/[^\\/]+$"
}
},
"route":{
Expand Down
37 changes: 30 additions & 7 deletions src/go/util/url_util.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,21 @@ const (
)

var (
pathParamMatcherRegexp = regexp.MustCompile(`{[^{}]+}`)
// Various hacky regular expressions to match a subset of the http template syntax.
// Replace segments with single wildcards: /v1/books/*
singleWildcardMatcher = regexp.MustCompile(`/\*`)
// Replace segments with double wildcards: /v1/**
doubleWildcardMatcher = regexp.MustCompile(`/\*\*`)
// Replace any path templates: /v1/books/{book_id}
pathParamMatcher = regexp.MustCompile(`/{[^{}]+}`)
// Replace path templates with double wildcards: /v1/{name=**}
pathParamDoubleWildcardMatcher = regexp.MustCompile(`/{[^{}]+=\*\*}`)

// Common regex forms that emulate http template syntax.
// Matches 1 or more segments of any character except '/'.
singleWildcardReplacementRegex = `/[^\/]+`
// Matches any character or no characters at all.
doubleWildcardReplacementRegex = `/.*`
)

// ParseURI parses uri into scheme, hostname, port, path with err(if exist).
Expand Down Expand Up @@ -163,16 +177,25 @@ func ExtraAddressFromURI(jwksUri string) (string, error) {
return fmt.Sprintf("%s:%v", hostname, port), nil
}

// Returns a regex that will match requests to the uri with path parameters.
// If there are no path params, returns empty string.
// Returns a regex that will match requests to the uri with path parameters or wildcards.
// If there are no path params or wildcards, returns empty string.
//
// Essentially matches a subset of the http template syntax.
// FIXME(nareddyt): Remove this hack completely when envoy route config supports path matching with path templates.
func WildcardMatcherForPath(uri string) string {
if !pathParamMatcherRegexp.MatchString(uri) {

// Ordering matters, start with most specific and work upwards.
matcher := pathParamDoubleWildcardMatcher.ReplaceAllString(uri, doubleWildcardReplacementRegex)
matcher = pathParamMatcher.ReplaceAllString(matcher, singleWildcardReplacementRegex)
matcher = doubleWildcardMatcher.ReplaceAllString(matcher, doubleWildcardReplacementRegex)
matcher = singleWildcardMatcher.ReplaceAllString(matcher, singleWildcardReplacementRegex)

if matcher == uri {
return ""
}

// Replace path templates inside "{}" by regex "[^\/]+", which means
// any character except `/`, also adds `$` to match to the end of the string.
return pathParamMatcherRegexp.ReplaceAllString(uri, `[^\/]+`) + `$`
// Enforce strict prefix / suffix.
return "^" + matcher + "$"
}

var (
Expand Down
31 changes: 27 additions & 4 deletions src/go/util/url_util_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -446,14 +446,37 @@ func TestWildcardMatcherForPath(t *testing.T) {
wantMatcher: "",
},
{
desc: "Multiple path params",
desc: "Path params with fieldpath-only bindings",
uri: "/shelves/{shelf_id}/books/{book.id}",
wantMatcher: `/shelves/[^\/]+/books/[^\/]+$`,
wantMatcher: `^/shelves/[^\/]+/books/[^\/]+$`,
},
{
desc: "Path param with wildcard",
desc: "Path param with wildcard segments",
uri: "/test/*/test/**",
wantMatcher: `^/test/[^\/]+/test/.*$`,
},
{
desc: "Path param with wildcard in segment binding",
uri: "/test/{x=*}/test/{y=**}",
wantMatcher: `/test/[^\/]+/test/[^\/]+$`,
wantMatcher: `^/test/[^\/]+/test/.*$`,
},
{
desc: "Path param with mixed wildcards",
uri: "/test/{name=*}/test/**",
wantMatcher: `^/test/[^\/]+/test/.*$`,
},
{
desc: "Invalid http template, not preceded by '/' ",
uri: "**",
wantMatcher: "",
},
{
// TODO(nareddyt): This is incorrect. But it should be fine, it's only a problem
// when user configures gRPC transcoding manually. OpenAPI compiler will
// never generate such a segment binding.
desc: "Path params with full segment binding",
uri: "/v1/{name=books/*}",
wantMatcher: `^/v1/[^\/]+$`,
},
}

Expand Down
2 changes: 1 addition & 1 deletion tests/endpoints/echo/server/app.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ func websocketEchoHandler(w http.ResponseWriter, r *http.Request) {
}
}

// dynamicEoutingHandler reads URL from request header, and writes it back out.
// dynamicRoutingHandler reads URL from request header, and writes it back out.
func dynamicRoutingHandler(w http.ResponseWriter, r *http.Request) {
// Handle sleeps
if strings.Contains(r.URL.Path, "/sleep") {
Expand Down
Loading

0 comments on commit 38f0610

Please sign in to comment.