Ignore HTTP_PROXY in reverse tunnels, part 2 #12335
Conversation
|
@r0mant Can you take another look at this? |
There was a problem hiding this comment.
@atburke The code lgtm in general but I'm having a bit of a hard time understanding in which cases we'll be respecting vs not respecting proxies with these changes. Mostly because these clients are used throughout the code.
Before we merge this, could you please help me clarify my understanding regarding the following:
- In which scenarios the proxy will be respected? When using tsh only?
- In which scenarios the proxy will not be respected? For inter-process communication?
- In which scenarios the proxy was respected prior to your original HTTP_PROXY changes? Never?
| @@ -55,6 +55,8 @@ func Connect(ctx context.Context, cfg *Config) (auth.ClientI, error) { | |||
| Credentials: []apiclient.Credentials{ | |||
| apiclient.LoadTLS(cfg.TLS), | |||
| }, | |||
| // Deliberately ignore HTTP proxies for backwards compatibility. | |||
| IgnoreHTTPProxy: true, | |||
There was a problem hiding this comment.
I'm probably missing something, but I'm still not sure why we need to ignore it in the authclient too. Isn't lib/reversetunnel/resolver enough?
There was a problem hiding this comment.
It isn't enough. authclient was also affected by the original HTTP_PROXY change. TestMultiPortNoProxy doesn't pass unless authclient ignores proxies.
|
Previously, HTTP_PROXY was only supported (as far as I can tell) for inter-process communication in single-port mode. My original HTTP_PROXY PR added support deliberately for |
|
We have a docs page for http proxy. I'll update it with the current behavior in a new PR. |
This change disables HTTP_PROXY in a few places that were missed in #11990.
This change disables HTTP_PROXY in a few places that were missed in #11990.
This change disables HTTP_PROXY in a few places that were missed in #11990.
This change disables HTTP_PROXY in a few places that were missed in #11990.
This PR disables HTTP_PROXY in a few places that were missed in #11990.