-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
use OUTBOUND_HTTP_PROXY for for external bypass client #13565
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
Conversation
S3 Image Test Results (AMD64 / ARM64) 2 files 2 suites 7m 41s ⏱️ Results for commit 72fc81b. ♻️ This comment has been updated with latest results. |
Test Results (amd64) - Integration, Bootstrap 5 files 5 suites 2h 34m 43s ⏱️ Results for commit 72fc81b. ♻️ This comment has been updated with latest results. |
c4aaa55 to
27d39af
Compare
|
Test failure is addressed by #13599 |
|
Adding @simonrw, as original author of the DNS Bypass classes |
| } | ||
| ) | ||
| if config: | ||
| proxy_config = config.merge(proxy_config) |
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.
Nit: it feels weird calling this the proxy config, maybe reassign the config variable?
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.
Now that you point it out, you are right. I was trying to avoid using an extra if or else. I will make it a one liner in the super call. It actually reads better to me that way 👍
Motivation
When creating an
ExternalBypassDnsClient, we should remain mindful of users proxy configurations and ensure that those external calls are respecting the Outbound Proxy configuration.Changes
Ensure both the client and the http session are created with the user's http proxy configuration.
Tests
OUTBOUND_HTTP_PROXYandOUTBOUND_HTTPS_PROXYset to your proxies, configuration. It might be required also to setREQUESTS_CA_BUNDLE.Related
relates to UNC-146