envoy: http: gRPC-web filter does not clear the route cache

Title: gRPC-web filter does not clear the route cache

Description:

source/extensions/filters/http/grpc_web/grpc_web_filter.cc checks if the incoming request is a gRPC-web request and transforms it to gRPC proper. The transformation involves changing the content type to application/grpc. Unlike the JSON transcoding filter, gRPC-web one does not however clear the route cache making it harder to route the “upgraded” request the same way one would route a native gRPC request.

Envoy version: master @ 97d259d43a4f9b7c6d798804b4b2c9ffbd2d285d (02/11/2019)

Repro steps: Define a route explicitly matching gRPC traffic:

...
    "match": {
      "prefix": "/",
      "grpc": {}
    },
    "route": {
      "cluster": "lo_svc_grpc",
      "max_grpc_timeout": {
        "seconds": 0
      }
    },
  {
    "match": {
      "prefix": "/"
    },
    "route": {
      "cluster": "lo_svc_http",
      "timeout": {
        "seconds": 60
      }
    }
  }
...

Define the http filters in the following order:

...
    "http_filters": [
      {
        "name": "envoy.grpc_web"
      },
      {
        "name": "envoy.router"
      }]
...

Send a gRPC-web request through Envoy

Expected: the request is upgraded to gRPC proper and routed to lo_svc_grpc Actual: the request is upgraded to gRPC proper and routed to lo_svc_http (the cached route)

Why is this a bug?

  1. JSON transcoding filter clears the cache and supports such routing - consistency issue.
  2. The documentation states the filters are executed in order of their definition, since the router was defined as the last one, it should pick up the changes made by the previous filters.
  3. The routing supports matching on gRPC requests and when the gRPC-web request reaches the router, it is a gRPC request at that point and should be routed as such.

Workaround: Insert a route matching the initial request on gRPC-web explicitly:

  {
    "match": {
      "prefix": "/",
      "headers": [
        {
          "name": "content-type",
          "prefix_match": "application/grpc-web"
        }
      ]
    },
    "route": {
      "cluster": "lo_svc_grpc",
      "max_grpc_timeout": {
        "seconds": 0
      }
    }
  },

Proposed fix:

decoder_callbacks_->clearRouteCache();

after the line https://github.com/envoyproxy/envoy/blob/master/source/extensions/filters/http/grpc_web/grpc_web_filter.cc#L60

About this issue

  • Original URL
  • State: open
  • Created 5 years ago
  • Comments: 18 (18 by maintainers)

Most upvoted comments

For the transcoding one, I’d make a change to require a custom header requesting the transcoding (x-envoy-json-grpc) because currently it is very aggressive and will attempt to match any non-gRPC content type HTTP request with the proto descriptor and execute the transcoding, which may clash with the native HTTP endpoint of the service.

In both cases, I feel like the transcoding filter’s behavior is not ideal as it tries to transcode everything non application/grpc which may potentially clash with pure HTTP endpoints on the upstream (if the user happens to define a conflicting mapping).

Feel free to open an PR as long as requiring header is opt-in thus not breaking existing users. I don’t feel much value as it is a config problem in general, and if you have a proto descriptor defined transcoding clashes with the native HTTP endpoint, you will have your own problem anyway.

Transcoding filter try to match incoming request to proto descriptor but if it doesn’t, it is basically no-op.

For this particular case, IMO we can add a flag into GrpcRouteMatchOptions, such as include_grpc_web so the route will match grpc-web request too before the filter, WDYT?