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

proxyDial not compatible with domainMatchers for NO_PROXY settings. #7779

Closed
akats7 opened this issue Oct 25, 2024 · 3 comments
Closed

proxyDial not compatible with domainMatchers for NO_PROXY settings. #7779

akats7 opened this issue Oct 25, 2024 · 3 comments

Comments

@akats7
Copy link

akats7 commented Oct 25, 2024

Hi all,
The http2_client uses proxyDial which uses the net/http package's ProxyFromEnvironment function to determine whether to apply proxy settings to the connections. The ProxyFromEnvironment function parses the NO_PROXY string and uses a matcher interface to compare the address against the ips and domains in the NO_PROXY string. However, it looks like only the ip address string is passed to the proxyDial function instead of the resolver.Address which includes the hostName.

Due to this, domain based NO_PROXY settings do not seem to be applied as all matching is done directly against the IP.

What version of gRPC are you using?

1.67.1

What version of Go are you using (go version)?

1.23.2

What operating system (Linux, Windows, …) and version?

AmazonLinux

What did you do?

If possible, provide a recipe for reproducing the error.

What did you expect to see?

PROXY setting to remain unset when including target domain in NO_PROXY

What did you see instead?

HTTPS_PROXY set even when target is included in NO_PROXY

@eshitachandwani
Copy link
Member

Hi @akats7,

This looks related to the #7556 . Earlier we used grpc.Dial with the passthrough name resolver as default, which prevented domain names from being resolved before reaching the proxy.

We've now switched to grpc.NewClient and the DNS resolver as default. And so all hostnames are resolved before being sent to the proxy.

We're actively working on the fix.
Closing it now, feel free to reopen the issue if you have any more concerns.

@akats7
Copy link
Author

akats7 commented Oct 25, 2024

Thanks @eshitachandwani, is there any known timetable for the fix?

@eshitachandwani
Copy link
Member

Hey, it might take a few weeks for the fix. But for the time being, you can try manually specifying passthrough as the scheme in your address as a workaround. Refer this comment. Let me know if this doesn't work or there are any other issues.

bogdandrutu pushed a commit to open-telemetry/opentelemetry-collector that referenced this issue Nov 4, 2024
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description

This is related #11537 and [this grpc
issue](grpc/grpc-go#7779), to reiterate there
is a bug in the grpc-go NewClient that makes the way the hostname is
resolved incompatible with the way proxy setting are applied. Due to
this domain based NO_PROXY setting do not work for grpc connections.
They are working on a fix in the grpc library but until then it seems
like it might be a good idea to revert to using DialContext instead of
NewClient.

If there's a workaround for this that anyone is aware of that could be
suitable as well, the most clear one of using passthrough doesn't work
since we sanitize the endpoint.

<!-- Issue number if applicable -->
#### Link to tracking issue
Fixes #11537 

<!--Describe what testing was performed and which tests were added.-->
#### Testing

I added some logging where the proxy setting are evaluated to show the
behavior.

**DialContext**
______
InMatch
host :  otel*******</DOMAIN-NAME/>
m.host :  </DOMAIN-NAME/> 
hasSuffix :  true
m.matchHost :  true
______

**NewClient**
______
InMatch
host :  10.*.*.*.*
m.host :  </DOMAIN-NAME/> 
hasSuffix :  false
m.matchHost :  false
______

---------

Co-authored-by: Yang Song <[email protected]>
djaglowski pushed a commit to djaglowski/opentelemetry-collector that referenced this issue Nov 21, 2024
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description

This is related open-telemetry#11537 and [this grpc
issue](grpc/grpc-go#7779), to reiterate there
is a bug in the grpc-go NewClient that makes the way the hostname is
resolved incompatible with the way proxy setting are applied. Due to
this domain based NO_PROXY setting do not work for grpc connections.
They are working on a fix in the grpc library but until then it seems
like it might be a good idea to revert to using DialContext instead of
NewClient.

If there's a workaround for this that anyone is aware of that could be
suitable as well, the most clear one of using passthrough doesn't work
since we sanitize the endpoint.

<!-- Issue number if applicable -->
#### Link to tracking issue
Fixes open-telemetry#11537 

<!--Describe what testing was performed and which tests were added.-->
#### Testing

I added some logging where the proxy setting are evaluated to show the
behavior.

**DialContext**
______
InMatch
host :  otel*******</DOMAIN-NAME/>
m.host :  </DOMAIN-NAME/> 
hasSuffix :  true
m.matchHost :  true
______

**NewClient**
______
InMatch
host :  10.*.*.*.*
m.host :  </DOMAIN-NAME/> 
hasSuffix :  false
m.matchHost :  false
______

---------

Co-authored-by: Yang Song <[email protected]>
HongChenTW pushed a commit to HongChenTW/opentelemetry-collector that referenced this issue Dec 19, 2024
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description

This is related open-telemetry#11537 and [this grpc
issue](grpc/grpc-go#7779), to reiterate there
is a bug in the grpc-go NewClient that makes the way the hostname is
resolved incompatible with the way proxy setting are applied. Due to
this domain based NO_PROXY setting do not work for grpc connections.
They are working on a fix in the grpc library but until then it seems
like it might be a good idea to revert to using DialContext instead of
NewClient.

If there's a workaround for this that anyone is aware of that could be
suitable as well, the most clear one of using passthrough doesn't work
since we sanitize the endpoint.

<!-- Issue number if applicable -->
#### Link to tracking issue
Fixes open-telemetry#11537 

<!--Describe what testing was performed and which tests were added.-->
#### Testing

I added some logging where the proxy setting are evaluated to show the
behavior.

**DialContext**
______
InMatch
host :  otel*******</DOMAIN-NAME/>
m.host :  </DOMAIN-NAME/> 
hasSuffix :  true
m.matchHost :  true
______

**NewClient**
______
InMatch
host :  10.*.*.*.*
m.host :  </DOMAIN-NAME/> 
hasSuffix :  false
m.matchHost :  false
______

---------

Co-authored-by: Yang Song <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants