-
Notifications
You must be signed in to change notification settings - Fork 154
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
resolve proxy to inject using http package #6675
base: main
Are you sure you want to change the base?
Conversation
This pull request does not have a backport label. Could you fix it @intxgo? 🙏
|
Looking at endpoint's Proxy.cpp I see the following suggesting this needs a change in endpoint as well. Is that correct? // This mimic GO implementation, though we don't support the NO_PROXY list of IP's which should
// be accessed without proxy
//
// https://pkg.go.dev/golang.org/x/net/http/httpproxy This is a separate thing. Indeed if an environment proxy is set on Endpoint, it'll skip the NO_PROXY. However, it doesn't matter for this fix since when Agent injects it's environment proxy into Endpoint config, then it takes priority at Endpoint side over environment proxy. Much like |
proxyURL = os.Getenv("HTTPS_PROXY") | ||
if proxyURL == "" { | ||
proxyURL = os.Getenv("HTTP_PROXY") | ||
proxyURL, err := http.ProxyFromEnvironment(request) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 didn't know this existed!
Quality Gate passedIssues Measures |
I probably found the culprit of the unit tests failure. The Indeed, even locally when I run all tests I get the same failure. Experimenting I've found that environment proxy set in the shell before test execution is not overridden by
Could it be that these env variables are set on the testing nodes but with empty strings? @cmacknz @michel-laterman do you know if we could set those env variables on the test nodes like above? If yes, how to do this? |
The
Edit I can reproduce locally with: |
OK so the problem isn't It includes a do.Once to setup the proxy func and I think this only reads the env vars one time. https://cs.opensource.google/go/go/+/refs/tags/go1.23.5:src/net/http/transport.go;l=907-914 This has something to do with at which point in the tests the variables are read I think.
|
What does this PR do?
Agent is injecting proxy env variables into components configs if no proxy is already specified by the config. Unfortunately with this approach the NO_PROXY variable is lost, so the target URL has to be evaluated against the environment proxy to determine if the proxy should be used. Go http provides such functionality.
Why is it important?
The environment proxy may be defined with exclusion for Fleet and Output addresses.
Checklist
./changelog/fragments
using the changelog toolRelated issues
HTTP_PROXY/HTTPS_PROXY/NO_PROXY
to components. #2602