Add ability to optionally disable gRPC-Web filter

sunjayBhatia commented 2 years ago

Currently Contour configures all HTTP Connection Manager filter chains with the gRPC-Web filter which means that Envoy attempts to proxy/translate any gRPC traffic over HTTP/1.1 as actual gRPC (converted to HTTP/2 etc.).

However, as we can see in this issue some applications explicitly do not want a proxy to translate gRPC-Web traffic and would rather it is left as-is.

The general feature request from the linked issue is to optionally disable the gRPC-Web filter.

However, given how Contour constructs filter chains this is a little tricky. There is only one filter chain for all HTTP virtualhosts, whereas for HTTPS there is a HTTP Connection Manager/filter chain for each HTTPS virtualhost (and a catchall for requests with no TLS SNI).

It may be possible to per-TLS enabled Ingress/HTTPProxy disable the gRPC-Web filter, but for plain HTTP it is not. We would likely rather have to have a global toggle. This may or may not be desirable in a multi-tenant environment where Contour provides ingress capabilities for many applications. If it is a global configuration the operator of Contour opts into this may be a feasible solution. Also adding a per-TLS vhost toggle may be overkill if we implement that, but that is up for debate. Only implementing a per-vhost toggle doesn't sound like a full solution, but that is also up for debate.

The still-alpha Envoy matching APIs may offer a way out of this. These would enable us to skip the gRPC-Web filter if for example requests contained a certain header, or some other attribute. However, of course this feature is still alpha (as of this issue creation).

Links:

Contour default filters: https://github.com/projectcontour/contour/blob/dbaa2a6f89acf714ed42e0904d455d2696abba38/internal/envoy/v3/listener.go#L245-L252
Envoy gPRC protocol docs: https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/other_protocols/grpc
Envoy matching API: https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/advanced/matching/matching_api

vrabbi commented 2 years ago

This would be extremely beneficial for us as we want to use contour but cant without this functionality. Is this being planned for a relatively near release or is there any other alternative?

youngnick commented 2 years ago

This isn't prioritized yet, but it doesn't seem like it would be a huge design burden, so maybe @xaleeks can consider pulling this into 1.21?

absoludity commented 2 years ago

Hi @sunjayBhatia

The still-alpha Envoy matching APIs may offer a way out of this. These would enable us to skip the gRPC-Web filter if for example requests contained a certain header, or some other attribute. However, of course this feature is still alpha (as of this issue creation).

It sounds like this could be used to skip the gRPC-Web filter for requests to the Kubeapps frontend (host header?) I'd like to try this out (and don't mind it being alpha, as long as it's on the road to beta :) )

absoludity commented 2 years ago

JFYI: https://github.com/kubeapps/kubeapps/issues/3716#issuecomment-1067532124 - TL;DR: It is possible to run Kubeapps with the current contour (ie. without https://github.com/projectcontour/contour/issues/4290 ) when using the demo-only, insecure, token authentication. I am not able to get Kubeapps working with Contour with OIDC.

youngnick commented 2 years ago

Reading that thread, it seems like kubeapps has its own OIDC proxy that requires the traffic to flow through it? I'm currently working on a design that will allow Contour's Envoys to do that flow for you to some extent, see #2664 for details.

Regardless, it seems like having a way to disable the GRPC-web filter on a per-route basis would be useful - it may be easier to add a field to HTTPProxy's Route struct than to start using the route matching API. @sunjayBhatia, thoughts? I'm thinking of a "disable GRPC-web filter" style boolean.

absoludity commented 2 years ago

Reading that thread, it seems like kubeapps has its own OIDC proxy that requires the traffic to flow through it?

We just use oauth2-proxy rather than our own. but yes, it switches the cookie for the Authorization header when requests go through it, so without it the requests arrive at our grpc backend without any authentication header.

I'm currently working on a design that will allow Contour's Envoys to do that flow for you to some extent, see #2664 for details.

Excellent, let us know if you need any early testers - that'd be an interesting alternative to our dependency on oauth2-proxy.

sunjayBhatia commented 2 years ago

Regardless, it seems like having a way to disable the GRPC-web filter on a per-route basis would be useful - it may be easier to add a field to HTTPProxy's Route struct than to start using the route matching API. @sunjayBhatia, thoughts? I'm thinking of a "disable GRPC-web filter" style boolean.

for HTTP only routes this gets complicated as there is only one HTTPConnectionManager and set of filters, it’s doable for HTTPS routes but more complicated otherwise

mfridman commented 2 years ago

We ran into this issue and need to disable gRPC-Web filtering on a per route basis. A global wouldn't be ideal because we are serving some traffic that uses this translation, whereas other traffic we'd like it to be disabled.

... having a way to disable the GRPC-web filter on a per-route basis would be useful - it may be easier to add a field to HTTPProxy's Route struct than to start using the route matching API. @sunjayBhatia, thoughts? I'm thinking of a "disable GRPC-web filter" style boolean.

This exactly would fit our use-case. Something like this, not sure where, but scoped to the route object.

- conditions:
    - prefix: /api.v1.UserService/
  services:
    - name: service1
      port: 3000
      protocol: h2c
      disableGRPCWebFilter: true # <-- on by default with the ability to disable

jorgemoralespou commented 2 years ago

For including Kubeapps into TCE we need a solution to this, as otherwise we need to deploy, along with kubeapps another ingress controller that supports our grpc communication between client and server (currently we use nginx), and it looks awful to have a different ingress controller (for this use case) than the one for the cluster globally, which we prefer to be contour.

vrabbi commented 2 years ago

Or suggest the use of SVC LB for the meantime or nodeport

jorgemoralespou commented 2 years ago

In the scenario we're working on, that can't happen as web ui will run on a different place as backend bits, and those other options, although possible are not nice to have.

ppbaena commented 2 years ago

Any update here? Is this being planned for a relatively near release? As mentioned before it is something needed for Kubeapps in TCE (as otherwise we need to deploy nginx along with kubeapps...).

sunjayBhatia commented 2 years ago

we can add this to the 1.22 release, PRs welcome of course if we don't get to it!

sunjayBhatia commented 2 years ago

one clarification question to potential users: Which of these scenarios represent how you intend to configure your gPRC-web services?

gRPC-web over HTTP to Envoy proxy, gRPC-web over HTTP/S to upstream service
- with other services that require gRPC-web translation to gRPC
- without other services that require gPRC-web translation to gRPC
gRPC-web over HTTPS to Envoy proxy, gRPC-web over HTTP/S to upstream service
- with other services that require gRPC-web translation to gRPC
- without other services that require gPRC-web translation to gRPC
needing to use the gRPC-web filter in conjunction with enabling fallback TLS certificates

the motivation behind this is that disabling the filter per plain HTTP gRPC-web host or "fallback" enabled host isn't really possible given how Contour works today (each of these has only one filter chain that services all virtualhosts that apply), however per regular TLS vhost it is possible

if it is ok to only apply this feature to TLS vhosts this will likely be straightforward, otherwise it needs more thinking

Also for clarification, we will likely be adding a field per-root HTTPProxy and not per-HTTPProxy Route since this translates into removing an Envoy HTTP filter which is at a higher level in request processing before Envoy gets to routing, e.g.

apiVersion: projectcontour.io/v1
kind: HTTPProxy
metadata:
  name: example
spec:
  virtualhost:
    fqdn: foo.com
  disableGRPCWebTranslation: true
  routes:
  - ...

absoludity commented 2 years ago

one clarification question to potential users: Which of these scenarios represent how you intend to configure your gPRC-web services?
* gRPC-web over HTTP to Envoy proxy, gRPC-web over HTTP/S to upstream service
  * with other services that require gRPC-web translation to gRPC
  * without other services that require gPRC-web translation to gRPC

This would be useful for dev environments without TLS ingress, but not essential.

* gRPC-web over HTTPS to Envoy proxy, gRPC-web over HTTP/S to upstream service

This is the one that is essential for us - and would ensure that people can use Kubeapps securely with OIDC auth. (FWIW, we terminate TLS at ingress so incoming gRPCWeb requests continue to our frontend reverse proxy and on to the backend gRPC service which handles grpcWeb as well).

  * with other services that require gRPC-web translation to gRPC
  * without other services that require gPRC-web translation to gRPC

The choice of the above 2 doesn't affect us, but in terms of not wanting to change things for other services running on the contour-enabled cluster, I'd assume the former?

* needing to use the gRPC-web filter in conjunction with enabling fallback TLS certificates

I don't think we need to enable fallback TLS certificates.

if it is ok to only apply this feature to TLS vhosts this will likely be straightforward, otherwise it needs more thinking

Fine for us to apply the feature to TLS vhosts only - that's a much better option than not having a solution for a longer time, IMO.

Thanks!

mfridman commented 2 years ago

... we will likely be adding a field per-root HTTPProxy and not per-HTTPProxy Route since this translates into removing an Envoy HTTP filter which is at a higher level in request processing before Envoy gets to routing

This would be perfect for our use case.

We no longer use gPRC on the backend, and have a framework (connect-go) that speaks grpc-web natively. So, being able to disable the translation for the entire HTTPProxy would be great.

vrabbi commented 2 years ago

Any updates on when this will be implemented? Currently this is blocking us from switching to use contour in some of our environments.

skriss commented 2 years ago

Any updates on when this will be implemented? Currently this is blocking us from switching to use contour in some of our environments.

@vrabbi no updates right now but I will add it to the list for consideration for v1.24. Does supporting this only for TLS-enabled vhosts work for your use case as well?

If anyone is interested in contributing here, it could help accelerate this work.

vrabbi commented 2 years ago

Supporting only tls vhosts would be fine for our use case

vrabbi commented 1 year ago

Any updates around this?

kahirokunn commented 1 year ago

I need this feature.

kahirokunn commented 1 year ago

I have a problem with tls only support because I use ALB.

skriss commented 1 year ago

Tentatively adding this to the 1.26 milestone given the interest.

kahirokunn commented 1 year ago

@skriss That's awesome! 😍

skriss commented 1 year ago

This issue needs a contributor to do some design work to figure out the requirements and come up with a solution, please let us know if you are interested in working on it.

projectcontour / contour

Add ability to optionally disable gRPC-Web filter #4290