envoyproxy / envoy

Cloud-native high-performance edge/middle/service proxy
https://www.envoyproxy.io
Apache License 2.0
24.74k stars 4.75k forks source link

Support TCP Proxy over HTTP/2 POST (Encap and Decap) #14234

Closed wlhee closed 3 years ago

wlhee commented 3 years ago

Title: Support TCP Proxy over HTTP/2 POST

Description: https://github.com/envoyproxy/envoy/issues/13185 has original context. With https://github.com/envoyproxy/envoy/pull/13548, Envoy could supports the encapping TCP over HTTP/2 POST with a custom TCP upstream.

Based on our further investigation, we notice that in addition to a custom TCP upstream for encapping for the client side envoy, we also need to a symmetric decapping filter on the server side envoy.

We would like to contribute to Envoy upstream because we think it will benefit other Envoy users as well, as many edge proxies don't have good support for HTTP/2 CONNECT (for example, Google edge frontend).

If this proposal is accepted, I would like some guidance of

  1. whether the custom upstream should be an independent class or it can be an option to the existing tcp upstream?

  2. which directory is the proper place for adding the custom upstream (if needed) and the new filter?

wlhee commented 3 years ago

@htuch @alyssawilk

alyssawilk commented 3 years ago

I'll let other maintainers weigh in on if this goes upstream or not :-)

but to the other questions, the HTTP filter would go in source/extensions/filters/http/filter_name/ and the tcp work (if not done inline, with a config option) would go in source/extensions/upstreams/tcp/custom_name

mattklein123 commented 3 years ago

So is the proposal here to have: 1) TCP -> HTTP Post for upstream 2) HTTP filter -> raw TCP (tcp_proxy) decap in place of the router filter? I wonder could this somehow be turned into a terminated "websocket" connection and get most of that for free?

wlhee commented 3 years ago

So is the proposal here to have:

  1. TCP -> HTTP Post for upstream

Correct, basically we need to set the method to POST and stop setting "protocol: bytestream" header: https://github.com/envoyproxy/envoy/blob/1c0696770325926cd1498bb0c991ca85aa42b050/source/common/tcp_proxy/upstream.cc#L250-L251

nghttp2 rejects requests when method = POST and protocol header is also set.

  1. HTTP filter -> raw TCP (tcp_proxy) decap in place of the router filter? I wonder could this somehow be turned into a terminated "websocket" connection and get most of that for free?

My thought was to add a new HTTP filter which overrides the method to CONNECT and also set the protocol header. I am not sure if it fits into router filter. But I guess it also needs a way to optionally trigger the logic.

I am not quite familiar of the logic of 'terminated "websockets" connection', I am happy to learn more about it if it fits this use case.

mattklein123 commented 3 years ago

OK it seems like (1) could just be a config option versus something new if all it needs to do is some header changes?

For (2) I will defer to @alyssawilk who is the expert here, but we already have code that terminates CONNECT and I think also WebSocket, and translates to raw tcp_proxy. I just didn't want us to reimplment that part of the code. If a small filter would do the trick so the normal connect or websocket code would work that sgtm.

alyssawilk commented 3 years ago

yeah, I think it'll be more work than the in-house PR I saw, because we'll want the full suite of CONNECT integraiton tests running for both CONNECT and POST, and checks on the new HTTP/1.1 implementation plus new corner cases like making sure we handle pipelined tunneling-POSTs in a way we're happy with.
If Matt is happy with it being a config option we could have an optional enum in tunneling config for which method to use, which defaults to CONNECT but can be set to POST.

wlhee commented 3 years ago

Thanks, I am open to add an enum to tunneling config.

I wonder what would be decapping side? should I add a dedicated HTTP filter or bundle the logic in an existing filter?

mattklein123 commented 3 years ago

I wonder what would be decapping side? should I add a dedicated HTTP filter or bundle the logic in an existing filter?

This is the part that I would discuss with @alyssawilk. It seems like this could also be a config option that just translates POST to CONNECT? Otherwise yeah I think we need some kind of small filter that effectively does this.

wlhee commented 3 years ago

I would think the filter does three simple things:

  1. overrides the method header from POST to connect
  2. adds "protocol: bytestream" header
  3. a configuration that control the filter to do the above, for example, a configurable header match. Same config on the encap tcp proxy side.
mattklein123 commented 3 years ago

The only part that I don't know about is whether a filter can actually convert POST to CONNECT and have it work. You might have to do an internal redirect but I'm not sure. @alyssawilk will know the details.

alyssawilk commented 3 years ago

If Matt's OK with config, I think the same way we allow CONNECT as a one off in upgrade configs, we can allow POST Again it'll need some extra docs and testing, but start there and throw something my way?

mattklein123 commented 3 years ago

Yeah I think for this config seems reasonable.

wlhee commented 3 years ago

Thanks for the suggestion, so I will start with:

  1. add configs to the tcp proxy
  2. add a new small filter
  3. add integration tests
github-actions[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

alyssawilk commented 3 years ago

Unfortunate that the in flight PR doesn't prevent stale tagging. Fixed manually.