Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

martian: allow to use http2 connection to upstream #239

Open
mmatczuk opened this issue Feb 22, 2023 · 3 comments
Open

martian: allow to use http2 connection to upstream #239

mmatczuk opened this issue Feb 22, 2023 · 3 comments
Labels
Milestone

Comments

@mmatczuk
Copy link
Contributor

Upstream requests don't interact well with Transport: connections could always be reused, but Transport thinks they go to different Hosts, so it spawns tons of useless connections. Just use dialContext, which will multiplex via single connection, if http/2.

@mmatczuk mmatczuk added this to the v1.0 milestone Feb 22, 2023
@mmatczuk mmatczuk added the high label Feb 22, 2023
@mmatczuk
Copy link
Contributor Author

Currently the http/2 proxy is not supported in http.Transport golang/go#26479.
However, it can be worked around by a custom transport that manages http2 connections.

In a simplified case of a single upstream proxy it would be sufficient to have a http2.ClientConn to the proxy passed to Martian as a http.RoundTripper.
The more complete solution is to have http2.Transport with a custom connection pool and manage connection lifecycle i.e. dial connections when needed.

Another example of similar, but different, approach can be github.com/caddyserver/forwardproxy/httpclient.
There for each request we establish new CONNECT tunnel over http2 stream.
This solution does not work well with proxy chaining and only works for http1 backends.
The last proxy enters the CONNECT path and it's difficult to coordinate.

@mmatczuk
Copy link
Contributor Author

One thing to note is not to use the http2 conn if the handling with connection upgrade.

https://github.com/kubernetes/kubernetes/pull/88781/files#diff-09ebe223122bee0cad4dde23b1c6b36ea2e30fe45e25ca4cd2e04b32a13948d6

@mmatczuk mmatczuk modified the milestones: v1.0, v1.1 Feb 23, 2023
@mmatczuk
Copy link
Contributor Author

Another thing to note is that this approach can be used to elegantly implement #99.

@mmatczuk mmatczuk modified the milestones: v1.1, v1.2 Aug 8, 2023
@mmatczuk mmatczuk modified the milestones: v1.2, v1.3 Feb 2, 2024
@Choraden Choraden modified the milestones: v1.3, v1.4 Apr 15, 2024
@mmatczuk mmatczuk modified the milestones: v1.4, v1.5 Sep 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants