allow preserving the route cache in the JSON-gRPC transcoder filter

[ilackarms]

Description:
The JSON-gRPC transcoder currently clears the route cache (here), causing routes to be recalculated based on the outgoing request headers (e.g. the transformed :path header, etc.).

This is useful if the user has defined routes that match the outgoing request. If the user has not defined these outgoing routes, requests will be terminated with a 404 (for no cluster match for URL '/<service>/<method>').

It is currently not possible to use this filter to transcode requests that match the incoming request headers (i.e., without telling Envoy to recalculate the routes for transcoded requests).

What I'd like to request is an extra config option for this filter to allow the filter to transcode a request without recalculating the route. A simple bool would be enough, something like bool skip_recalculating_route.

[lizan]

What is the use case of not defining a route for outgoing request? Can you provide a concrete example, better with a sample envoy config?

I feel the configuration would make users confused, even you can skip the route cache being reset by the JSON-gRPC transcoder, other filters can trigger it easily. We might need a better approach to solve your issue.

[ilackarms]

What is the use case of not defining a route for outgoing request? Can you provide a concrete example, better with a sample envoy config?

Using the following gRPC method as an example:

  // Returns a list of all shelves in the bookstore.
  rpc ListShelves(google.protobuf.Empty) returns (ListShelvesResponse) {
    option (google.api.http) = {
      get: "/shelves"
    };
  }

The incoming request must already match GET /shelves in order to get routed to ListShelves. Why should the author of Envoy's config also need to be aware of the route on the upstream, /bookstore.Bookstore/ListShelves? Envoy already knows this because it has access to my descriptors.pb.

The workflow I'd like to achieve could conceivably be like this:

1. The incoming request matches == GET /shelves
2. The control plane for Envoy creates a route for GET /shelves indicating the upstream gRPC service the request maps to
3. Request comes in, is transcoded, and is properly routed to the upstream without any party having to know the corresponding route the gRPC service is expecting

This allows me to have one source of truth about what the upstream mapping is: the proto descriptor file. Neither the client, nor the control plane / user configuring Envoy need to introspect the proto descriptor to understand this.

Before the proposed change to Envoy, my route config looks like this:

static_resources:
  listeners:
  - name: listener_0
    address:
      socket_address: { address: 0.0.0.0, port_value: 10000 }
    filter_chains:
    - filters:
      - name: envoy.http_connection_manager
        config:
          codec_type: auto
          route_config:
            name: local_route
            virtual_hosts:
            - name: local_service
              domains: ["*"]
              routes:
              # coupled with the outgoing request
              - match: { prefix: "/bookstore.Bookstore/ListShelves" }
                route: { cluster: grpc_service }
          http_filters:
          - name: envoy.grpc_json_transcoder
            config:
              proto_descriptor: ./proto.pb
              services: [bookstore.Bookstore]

and my request needs to look like

curl localhost:10000/shelves

What I want instead is the following:

static_resources:
  listeners:
  - name: listener_0
    address:
      socket_address: { address: 0.0.0.0, port_value: 10000 }
    filter_chains:
    - filters:
      - name: envoy.http_connection_manager
        config:
          codec_type: auto
          route_config:
            name: local_route
            virtual_hosts:
            - name: local_service
              domains: ["*"]
              routes:
              # coupled with the incoming request
              - match: { prefix: "/shelves" }
                route: { cluster: grpc_service }
          http_filters:
          - name: envoy.grpc_json_transcoder
            config:
              proto_descriptor: ./proto.pb
              services: [bookstore.Bookstore]
              skip_recalculating_route: true

And my incoming request still points to /shelves

This is especially useful for a self-service configuration design.

Sorry if this example is too verbose or misses the point; the tldr is that i'd like a self-service client who is as decoupled as possible from the upstream gRPC service. Since the client already needs to know the incoming request path, I feel that should be enough to achieve the transcoding.

[lizan]

Thanks @ilackarms.

The incoming request must already match GET /shelves in order to get routed to ListShelves. Why should the author of Envoy's config also need to be aware of the route on the upstream, /bookstore.Bookstore/ListShelves? Envoy already knows this because it has access to my descriptors.pb.

The author of Envoy's config is alreay aware of the route on the upstream, since they needs to specify services, and they can let the descriptors.pb to decide what incoming request looks like. For example they can just route prefix /bookstore.Bookstore/ to route the whole gRPC service to a cluster.

In most cases, users use both native gRPC and JSON-gRPC transcoding at same time, in that case, route based on outgoing request are same for same service/method, i.e. /bookstore.Bookstore/ListShelves.

I partly agree this is a valid use case if the author of Envoy's config only cares about incoming request and don't use native-gRPC at all.

However I would still disagree with your change #2848, since current behavior that routes are pre-calculate before HTTP filter chain and transcoder clears the cache is not Envoy standard and not documented somewhere, it is just a implementation detail. This behavior might be changed in future Envoy version, this happened before.

Looks like you want the author of Envoy's config to be focused on incoming request, there are other filters that clears route caches as well (like Lua filter, it does many things). So rather than add a flag to change the implementation behavior of gRPC-JSON transcoder as a "fix", I think adding a flag like route_based_on_incoming_request to [HTTP route config] is more appropriate for this case.

WDYT? cc @mattklein123

[mattklein123]

Hmm. Would need to think about this more. I suppose we could add some type of option that locks the ingress route, but then filters would basically not work and I think that would be super confusing. i guess my gut feeling is to make this an option on a per-filter basis if it makes sense to do so.

[lizan]

I'm OK to doing per-filter basis. I thought it just confuses more in cases that some filter combinations would result random routing result. IMO having the option that locks the ingress route might confuses filter devs more, but confuses users less.

[htuch]

I would also vote for per-filter, since I think we are going to provide surprising behavior to users in any case if we break filters that can't handle route locking. This is the least surprising path IMHO.

[htuch]

I think this is resolved in #2848, please reopen if not.

[yangyangpig]

mark