diff --git a/clientconn.go b/clientconn.go index 3a98f0c2ac29..42950a9a1e92 100644 --- a/clientconn.go +++ b/clientconn.go @@ -225,6 +225,13 @@ func Dial(target string, opts ...DialOption) (*ClientConn, error) { func DialContext(ctx context.Context, target string, opts ...DialOption) (conn *ClientConn, err error) { // At the end of this method, we kick the channel out of idle, rather than // waiting for the first rpc. + // + // WithTargetResolutionEnabled in `grpc.Dial` ensures that it preserves + // behavior: when default scheme passthrough is used, skip hostname + // resolution, when any other scheme like "dns" is used for resolution, + // perform resolution on the client as expected. + opts = append([]DialOption{withDefaultScheme("passthrough"), WithTargetResolutionEnabled()}, opts...) + opts = append([]DialOption{withDefaultScheme("passthrough"), WithTargetResolutionEnabled()}, opts...) cc, err := NewClient(target, opts...) if err != nil { diff --git a/dialoptions.go b/dialoptions.go index 3950cc96853a..7abe6868cd4a 100644 --- a/dialoptions.go +++ b/dialoptions.go @@ -94,11 +94,8 @@ type dialOptions struct { idleTimeout time.Duration defaultScheme string maxCallAttempts int - // targetResolutionEnabled indicates whether the client should perform - // target resolution even when a proxy is enabled. - targetResolutionEnabled bool - // useProxy specifies if a proxy should be used. - useProxy bool + targetResolutionEnabled bool // Specifies if client should perform target resolution when proxy is enabled. + useProxy bool // Specifies if a proxy should be used. } // DialOption configures how we set up the connection. diff --git a/internal/testutils/proxy.go b/internal/testutils/proxy.go index 6559b6f54b82..8fa1238c0390 100644 --- a/internal/testutils/proxy.go +++ b/internal/testutils/proxy.go @@ -33,12 +33,12 @@ const defaultTestTimeout = 10 * time.Second // ProxyServer represents a test proxy server. type ProxyServer struct { lis net.Listener - in net.Conn // The connection from the client to the proxy. - out net.Conn // The connection from the proxy to the backend. - requestCheck func(*http.Request) error // The function to check the request sent to proxy. + in net.Conn // Connection from the client to the proxy. + out net.Conn // Connection from the proxy to the backend. + requestCheck func(*http.Request) error // Function to check the request sent to proxy. } -// Stop closes the ProxyServer and its connectionsto client and server. +// Stop closes the ProxyServer and its connections to client and server. func (p *ProxyServer) Stop() { p.lis.Close() if p.in != nil { @@ -63,6 +63,7 @@ func NewProxyServer(lis net.Listener, reqCheck func(*http.Request) error, errCh return } p.in = in + // This will be used in tests to check if the proxy server is started. if proxyStarted != nil { proxyStarted() } @@ -87,19 +88,21 @@ func NewProxyServer(lis net.Listener, reqCheck func(*http.Request) error, errCh } else { out, err = net.Dial("tcp", backendAddr) } - if err != nil { errCh <- fmt.Errorf("failed to dial to server: %v", err) return } out.SetDeadline(time.Now().Add(defaultTestTimeout)) + // Response OK to client resp := http.Response{StatusCode: http.StatusOK, Proto: "HTTP/1.0"} var buf bytes.Buffer resp.Write(&buf) p.in.Write(buf.Bytes()) p.out = out - // Perform the proxy function, i.e pass the data from client to server and server to client. + + // Perform the proxy function, i.e pass the data from client to server + // and server to client. go io.Copy(p.in, p.out) go io.Copy(p.out, p.in) close(doneCh) diff --git a/internal/transport/proxy.go b/internal/transport/proxy.go index e0751d220ebc..2e84d4b5e11e 100644 --- a/internal/transport/proxy.go +++ b/internal/transport/proxy.go @@ -61,14 +61,14 @@ func doHTTPConnectHandshake(ctx context.Context, conn net.Conn, addr resolver.Ad } }() - a, _ := proxyattributes.ExtractOptions(addr) + opts, _ := proxyattributes.ExtractOptions(addr) req := &http.Request{ Method: http.MethodConnect, - URL: &url.URL{Host: a.ConnectAddr}, + URL: &url.URL{Host: opts.ConnectAddr}, Header: map[string][]string{"User-Agent": {grpcUA}}, } - if user := a.User; user != (url.Userinfo{}) { + if user := opts.User; user != (url.Userinfo{}) { u := user.Username() p, pSet := user.Password() if !pSet { diff --git a/resolver_wrapper.go b/resolver_wrapper.go index c987aff6d6d5..7420ff5a215c 100644 --- a/resolver_wrapper.go +++ b/resolver_wrapper.go @@ -82,7 +82,6 @@ func (ccr *ccResolverWrapper) start() error { // The delegating resolver is used unless: // - A custom dialer is provided via WithContextDialer dialoption. // - Proxy usage is disabled through WithNoProxy dialoption. - // - Client-side resolution is enforced with WithTargetResolutionEnabled. // In these cases, the resolver is built based on the scheme of target, // using the appropriate resolver builder. if ccr.cc.dopts.copts.Dialer != nil || !ccr.cc.dopts.useProxy { diff --git a/test/proxy_test.go b/test/proxy_test.go index c2459db98ec0..97ac3223bada 100644 --- a/test/proxy_test.go +++ b/test/proxy_test.go @@ -44,8 +44,6 @@ const ( unresolvedProxyURI = "proxyexample.com" ) -// createAndStartBackendServer creates and starts a test backend server, -// registering a cleanup to stop it. func createAndStartBackendServer(t *testing.T) string { t.Helper() backend := &stubserver.StubServer{ @@ -121,11 +119,11 @@ func (s) TestGRPCDialWithProxy(t *testing.T) { os.Setenv("HTTPS_PROXY", pLis.Addr().String()) defer func() { os.Setenv("HTTPS_PROXY", proxyEnv) }() - // Use the httpproxy package instead of http.ProxyFromEnvironment because - // the latter reads proxy-related environment variables only once at + // Use the httpproxy package functions instead of `http.ProxyFromEnvironment` + // because the latter reads proxy-related environment variables only once at // initialization. This behavior causes issues when running multiple tests // with different proxy configurations, as changes to environment variables - // during tests would be ignored. By using httpproxy.FromEnvironment(), we + // during tests would be ignored. By using `httpproxy.FromEnvironment()`, we // ensure proxy settings are read dynamically in each test execution. origHTTPSProxyFromEnvironment := delegatingresolver.HTTPSProxyFromEnvironment delegatingresolver.HTTPSProxyFromEnvironment = func(req *http.Request) (*url.URL, error) { @@ -143,6 +141,7 @@ func (s) TestGRPCDialWithProxy(t *testing.T) { } defer conn.Close() + // Send an empty RPC to the backend through the proxy. client := testgrpc.NewTestServiceClient(conn) if _, err := client.EmptyCall(ctx, &testgrpc.Empty{}); err != nil { t.Errorf("EmptyCall failed: %v", err) @@ -171,12 +170,6 @@ func (s) TestGRPCDialWithDNSAndProxy(t *testing.T) { os.Setenv("HTTPS_PROXY", unresolvedProxyURI) defer func() { os.Setenv("HTTPS_PROXY", proxyEnv) }() - // Use the httpproxy package instead of http.ProxyFromEnvironment because - // the latter reads proxy-related environment variables only once at - // initialization. This behavior causes issues when running multiple tests - // with different proxy configurations, as changes to environment variables - // during tests would be ignored. By using httpproxy.FromEnvironment(), we - // ensure proxy settings are read dynamically in each test execution. origHTTPSProxyFromEnvironment := delegatingresolver.HTTPSProxyFromEnvironment delegatingresolver.HTTPSProxyFromEnvironment = func(req *http.Request) (*url.URL, error) { return httpproxy.FromEnvironment().ProxyFunc()(req.URL) @@ -206,7 +199,7 @@ func (s) TestGRPCDialWithDNSAndProxy(t *testing.T) { } defer conn.Close() - // Send an RPC to the backend through the proxy. + // Send an empty RPC to the backend through the proxy. client := testgrpc.NewTestServiceClient(conn) if _, err := client.EmptyCall(ctx, &testgrpc.Empty{}); err != nil { t.Errorf("EmptyCall failed: %v", err) @@ -235,12 +228,6 @@ func (s) TestGRPCNewClientWithProxy(t *testing.T) { os.Setenv("HTTPS_PROXY", unresolvedProxyURI) defer func() { os.Setenv("HTTPS_PROXY", proxyEnv) }() - // Use the httpproxy package instead of http.ProxyFromEnvironment because - // the latter reads proxy-related environment variables only once at - // initialization. This behavior causes issues when running multiple tests - // with different proxy configurations, as changes to environment variables - // during tests would be ignored. By using httpproxy.FromEnvironment(), we - // ensure proxy settings are read dynamically in each test execution. origHTTPSProxyFromEnvironment := delegatingresolver.HTTPSProxyFromEnvironment delegatingresolver.HTTPSProxyFromEnvironment = func(req *http.Request) (*url.URL, error) { return httpproxy.FromEnvironment().ProxyFunc()(req.URL) @@ -251,13 +238,8 @@ func (s) TestGRPCNewClientWithProxy(t *testing.T) { // Set up and update a manual resolver for proxy resolution. proxyResolver := setupDNS(t) - proxyResolver.InitialState(resolver.State{ - Addresses: []resolver.Address{ - {Addr: pLis.Addr().String()}, - }, - }) + proxyResolver.InitialState(resolver.State{Addresses: []resolver.Address{{Addr: pLis.Addr().String()}}}) - // Dial to the proxy server. ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) defer cancel() conn, err := grpc.NewClient(unresolvedTargetURI, grpc.WithTransportCredentials(insecure.NewCredentials())) @@ -266,7 +248,7 @@ func (s) TestGRPCNewClientWithProxy(t *testing.T) { } defer conn.Close() - // Send an RPC to the backend through the proxy. + // Send an empty RPC to the backend through the proxy. client := testgrpc.NewTestServiceClient(conn) if _, err := client.EmptyCall(ctx, &testgrpc.Empty{}); err != nil { t.Errorf("EmptyCall failed: %v", err) @@ -288,7 +270,6 @@ func (s) TestGRPCNewClientWithProxy(t *testing.T) { // establishes a connection to the backend server. func (s) TestGRPCNewClientWithProxyAndCustomResolver(t *testing.T) { backendAddr := createAndStartBackendServer(t) - // Set up and start the proxy server. pLis, errCh, doneCh, _ := setupProxy(t, backendAddr, true, requestCheck(backendAddr)) // Overwrite the proxy environment and restore it after the test. @@ -296,12 +277,6 @@ func (s) TestGRPCNewClientWithProxyAndCustomResolver(t *testing.T) { os.Setenv("HTTPS_PROXY", unresolvedProxyURI) defer func() { os.Setenv("HTTPS_PROXY", proxyEnv) }() - // Use the httpproxy package instead of http.ProxyFromEnvironment because - // the latter reads proxy-related environment variables only once at - // initialization. This behavior causes issues when running multiple tests - // with different proxy configurations, as changes to environment variables - // during tests would be ignored. By using httpproxy.FromEnvironment(), we - // ensure proxy settings are read dynamically in each test execution. origHTTPSProxyFromEnvironment := delegatingresolver.HTTPSProxyFromEnvironment delegatingresolver.HTTPSProxyFromEnvironment = func(req *http.Request) (*url.URL, error) { return httpproxy.FromEnvironment().ProxyFunc()(req.URL) @@ -328,13 +303,13 @@ func (s) TestGRPCNewClientWithProxyAndCustomResolver(t *testing.T) { } t.Cleanup(func() { conn.Close() }) - // Create a test service client and make an RPC call. + // Send an empty RPC to the backend through the proxy. client := testgrpc.NewTestServiceClient(conn) if _, err := client.EmptyCall(ctx, &testgrpc.Empty{}); err != nil { t.Errorf("EmptyCall() failed: %v", err) } - // Check if the proxy encountered any errors. + // Verify if the proxy encountered any errors. select { case err := <-errCh: t.Fatalf("proxy server encountered an error: %v", err) @@ -350,7 +325,6 @@ func (s) TestGRPCNewClientWithProxyAndCustomResolver(t *testing.T) { // HTTP CONNECT request, and establishes a connection to the backend server. func (s) TestGRPCNewClientWithProxyAndCustomResolverWithEndpoints(t *testing.T) { backendAddr := createAndStartBackendServer(t) - // Set up and start the proxy server. pLis, errCh, doneCh, _ := setupProxy(t, backendAddr, true, requestCheck(backendAddr)) // Overwrite the proxy environment and restore it after the test. @@ -358,12 +332,6 @@ func (s) TestGRPCNewClientWithProxyAndCustomResolverWithEndpoints(t *testing.T) os.Setenv("HTTPS_PROXY", unresolvedProxyURI) defer func() { os.Setenv("HTTPS_PROXY", proxyEnv) }() - // Use the httpproxy package instead of http.ProxyFromEnvironment because - // the latter reads proxy-related environment variables only once at - // initialization. This behavior causes issues when running multiple tests - // with different proxy configurations, as changes to environment variables - // during tests would be ignored. By using httpproxy.FromEnvironment(), we - // ensure proxy settings are read dynamically in each test execution. origHTTPSProxyFromEnvironment := delegatingresolver.HTTPSProxyFromEnvironment delegatingresolver.HTTPSProxyFromEnvironment = func(req *http.Request) (*url.URL, error) { return httpproxy.FromEnvironment().ProxyFunc()(req.URL) @@ -376,6 +344,7 @@ func (s) TestGRPCNewClientWithProxyAndCustomResolverWithEndpoints(t *testing.T) targetResolver := manual.NewBuilderWithScheme("test") resolver.Register(targetResolver) targetResolver.InitialState(resolver.State{Endpoints: []resolver.Endpoint{{Addresses: []resolver.Address{{Addr: backendAddr}}}}}) + // Set up and update a manual resolver for proxy resolution. proxyResolver := setupDNS(t) proxyResolver.InitialState(resolver.State{Endpoints: []resolver.Endpoint{{Addresses: []resolver.Address{{Addr: pLis.Addr().String()}}}}}) @@ -389,7 +358,7 @@ func (s) TestGRPCNewClientWithProxyAndCustomResolverWithEndpoints(t *testing.T) } t.Cleanup(func() { conn.Close() }) - // Create a test service client and make an RPC call. + // Send an empty RPC to the backend through the proxy. client := testgrpc.NewTestServiceClient(conn) if _, err := client.EmptyCall(ctx, &testgrpc.Empty{}); err != nil { t.Errorf("EmptyCall() failed: %v", err) @@ -419,12 +388,6 @@ func (s) TestGRPCNewClientWithProxyAndTargetResolutionEnabled(t *testing.T) { os.Setenv("HTTPS_PROXY", unresolvedProxyURI) defer func() { os.Setenv("HTTPS_PROXY", proxyEnv) }() - // Use the httpproxy package instead of http.ProxyFromEnvironment because - // the latter reads proxy-related environment variables only once at - // initialization. This behavior causes issues when running multiple tests - // with different proxy configurations, as changes to environment variables - // during tests would be ignored. By using httpproxy.FromEnvironment(), we - // ensure proxy settings are read dynamically in each test execution. origHTTPSProxyFromEnvironment := delegatingresolver.HTTPSProxyFromEnvironment delegatingresolver.HTTPSProxyFromEnvironment = func(req *http.Request) (*url.URL, error) { return httpproxy.FromEnvironment().ProxyFunc()(req.URL) @@ -446,12 +409,10 @@ func (s) TestGRPCNewClientWithProxyAndTargetResolutionEnabled(t *testing.T) { resolver.Register(proxyResolver) proxyResolver.InitialState(resolver.State{Addresses: []resolver.Address{{Addr: pLis.Addr().String()}}}) - // Dial options with target resolution enabled. dopts := []grpc.DialOption{ grpc.WithTransportCredentials(insecure.NewCredentials()), - grpc.WithTargetResolutionEnabled(), + grpc.WithTargetResolutionEnabled(), // Target resolution on client enabled. } - // Dial to the proxy server. ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) defer cancel() conn, err := grpc.NewClient(unresolvedTargetURI, dopts...) @@ -460,7 +421,7 @@ func (s) TestGRPCNewClientWithProxyAndTargetResolutionEnabled(t *testing.T) { } t.Cleanup(func() { conn.Close() }) - // Create a test service client and make an RPC call. + // Send an empty RPC to the backend through the proxy. client := testgrpc.NewTestServiceClient(conn) if _, err := client.EmptyCall(ctx, &testgrpc.Empty{}); err != nil { t.Errorf("EmptyCall() failed: %v", err) @@ -491,12 +452,6 @@ func (s) TestGRPCNewClientWithNoProxy(t *testing.T) { os.Setenv("HTTPS_PROXY", unresolvedProxyURI) defer func() { os.Setenv("HTTPS_PROXY", proxyEnv) }() - // Use the httpproxy package instead of http.ProxyFromEnvironment because - // the latter reads proxy-related environment variables only once at - // initialization. This behavior causes issues when running multiple tests - // with different proxy configurations, as changes to environment variables - // during tests would be ignored. By using httpproxy.FromEnvironment(), we - // ensure proxy settings are read dynamically in each test execution. origHTTPSProxyFromEnvironment := delegatingresolver.HTTPSProxyFromEnvironment delegatingresolver.HTTPSProxyFromEnvironment = func(req *http.Request) (*url.URL, error) { return httpproxy.FromEnvironment().ProxyFunc()(req.URL) @@ -508,10 +463,9 @@ func (s) TestGRPCNewClientWithNoProxy(t *testing.T) { targetResolver := setupDNS(t) targetResolver.InitialState(resolver.State{Addresses: []resolver.Address{{Addr: backendAddr}}}) - // Dial options with proxy explicitly disabled. dopts := []grpc.DialOption{ grpc.WithTransportCredentials(insecure.NewCredentials()), - grpc.WithNoProxy(), // Disable proxy. + grpc.WithNoProxy(), // Diable proxy. } ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) defer cancel() @@ -527,7 +481,7 @@ func (s) TestGRPCNewClientWithNoProxy(t *testing.T) { t.Errorf("EmptyCall() failed: %v", err) } - // Verify that the proxy was not dialed. + // Verify that the proxy server was not dialed. select { case <-proxyStartedCh: t.Fatal("unexpected dial to proxy server") @@ -552,12 +506,6 @@ func (s) TestGRPCNewClientWithContextDialer(t *testing.T) { os.Setenv("HTTPS_PROXY", unresolvedProxyURI) defer func() { os.Setenv("HTTPS_PROXY", proxyEnv) }() - // Use the httpproxy package instead of http.ProxyFromEnvironment because - // the latter reads proxy-related environment variables only once at - // initialization. This behavior causes issues when running multiple tests - // with different proxy configurations, as changes to environment variables - // during tests would be ignored. By using httpproxy.FromEnvironment(), we - // ensure proxy settings are read dynamically in each test execution. origHTTPSProxyFromEnvironment := delegatingresolver.HTTPSProxyFromEnvironment delegatingresolver.HTTPSProxyFromEnvironment = func(req *http.Request) (*url.URL, error) { return httpproxy.FromEnvironment().ProxyFunc()(req.URL) @@ -566,8 +514,7 @@ func (s) TestGRPCNewClientWithContextDialer(t *testing.T) { delegatingresolver.HTTPSProxyFromEnvironment = origHTTPSProxyFromEnvironment }() - // Create a custom dialer that directly dials the backend. We'll use this - // to bypass any proxy logic. + // Create a custom dialer that directly dials the backend.ß dialerCalled := make(chan struct{}) customDialer := func(_ context.Context, backendAddr string) (net.Conn, error) { close(dialerCalled) @@ -579,7 +526,7 @@ func (s) TestGRPCNewClientWithContextDialer(t *testing.T) { dopts := []grpc.DialOption{ grpc.WithTransportCredentials(insecure.NewCredentials()), - grpc.WithContextDialer(customDialer), + grpc.WithContextDialer(customDialer), // Use a custom dialer. } ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) defer cancel() @@ -646,12 +593,6 @@ func (s) TestBasicAuthInGrpcNewClientWithProxy(t *testing.T) { os.Setenv("HTTPS_PROXY", user+":"+password+"@"+unresolvedProxyURI) defer func() { os.Setenv("HTTPS_PROXY", proxyEnv) }() - // Use the httpproxy package instead of http.ProxyFromEnvironment because - // the latter reads proxy-related environment variables only once at - // initialization. This behavior causes issues when running multiple tests - // with different proxy configurations, as changes to environment variables - // during tests would be ignored. By using httpproxy.FromEnvironment(), we - // ensure proxy settings are read dynamically in each test execution. origHTTPSProxyFromEnvironment := delegatingresolver.HTTPSProxyFromEnvironment delegatingresolver.HTTPSProxyFromEnvironment = func(req *http.Request) (*url.URL, error) { return httpproxy.FromEnvironment().ProxyFunc()(req.URL) @@ -676,7 +617,7 @@ func (s) TestBasicAuthInGrpcNewClientWithProxy(t *testing.T) { } defer conn.Close() - // Send an RPC to the backend through the proxy. + // Send an empty RPC to the backend through the proxy. client := testgrpc.NewTestServiceClient(conn) if _, err := client.EmptyCall(ctx, &testgrpc.Empty{}); err != nil { t.Errorf("EmptyCall failed: %v", err)