Improving code

internal/resolver/delegatingresolver/delegatingresolver.go

@@ -36,8 +36,9 @@ import (
 var (
 	// HTTPSProxyFromEnvironment will be used and overwritten in the tests.
 	httpProxyFromEnvironmentFunc = http.ProxyFromEnvironment
-
-	logger = grpclog.Component("delegating-resolver")
+	// ProxyScheme will be ovwewritten in tests
+	ProxyScheme = "dns"
+	logger      = grpclog.Component("delegating-resolver")
 )
 
 // delegatingResolver implements the `resolver.Resolver` interface. It uses child
@@ -81,6 +82,12 @@ func parsedURLForProxy(address string) (*url.URL, error) {
 	return url, nil
 }
 
+// OnClientResolution is a no-op function in non-test code. In tests, it can
+// be overwritten to send a signal to a channel, indicating that client-side
+// name resolution was triggered.  This enables tests to verify that resolution
+// is bypassed when a proxy is in use.
+var OnClientResolution = func(int) { /* no-op */ }
+
 // New creates a new delegating resolver that is used to call the target and
 // proxy child resolver. If proxy is configured, both proxy and target resolvers
 // are used else only target resolver is used.
@@ -109,6 +116,7 @@ func New(target resolver.Target, cc resolver.ClientConn, opts resolver.BuildOpti
 	// proxy is not configured or proxy address excluded using `NO_PROXY` env var,
 	// so only target resolver is used.
 	if r.proxyURL == nil {
+		OnClientResolution(1)
 		return targetResolverBuilder.Build(target, cc, opts)
 	}
 
@@ -123,6 +131,7 @@ func New(target resolver.Target, cc resolver.ClientConn, opts resolver.BuildOpti
 		r.targetAddrs = []resolver.Address{{Addr: target.Endpoint()}}
 		r.targetResolverReady = true
 	} else {
+		OnClientResolution(1)
 		wcc := &wrappingClientConn{
 			parent:       r,
 			resolverType: targetResolverType,
@@ -142,7 +151,7 @@ func New(target resolver.Target, cc resolver.ClientConn, opts resolver.BuildOpti
 // "dns" scheme. It adjusts the proxyURL to conform to the "dns:///" format and
 // builds a resolver with a wrappingClientConn to capture resolved addresses.
 func (r *delegatingResolver) proxyURIResolver(opts resolver.BuildOptions) (resolver.Resolver, error) {
-	proxyBuilder := resolver.Get("dns")
+	proxyBuilder := resolver.Get(ProxyScheme)
 	if proxyBuilder == nil {
 		panic(fmt.Sprintln("delegating_resolver: resolver for proxy not found for scheme dns"))
 	}

internal/transport/http2_client.go

@@ -37,7 +37,7 @@ import (
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/credentials"
 	"google.golang.org/grpc/internal"
-	"google.golang.org/grpc/internal/attributes"
+	"google.golang.org/grpc/internal/proxyattributes"
 	"google.golang.org/grpc/internal/channelz"
 	icredentials "google.golang.org/grpc/internal/credentials"
 	"google.golang.org/grpc/internal/grpclog"
@@ -157,8 +157,8 @@ type http2Client struct {
 func dial(ctx context.Context, fn func(context.Context, string) (net.Conn, error), addr resolver.Address, grpcUA string) (net.Conn, error) {
 	address := addr.Addr
 
-	//if the ProxyConnectAddr is set in the aattribute, do a proxy dial.
-	if attributes.ProxyConnectAddr(addr) != "" {
+	//if the ProxyConnectAddr is set in the attribute, do a proxy dial.
+	if proxyattributes.ProxyConnectAddr(addr) != "" {
 		return proxyDial(ctx, addr, grpcUA)
 	}
 	networkType, ok := networktype.Get(addr)

internal/transport/proxy.go

@@ -30,7 +30,7 @@ import (
 	"net/url"
 
 	"google.golang.org/grpc/internal"
-	"google.golang.org/grpc/internal/attributes"
+	"google.golang.org/grpc/internal/proxyattributes"
 	"google.golang.org/grpc/resolver"
 )
 
@@ -62,13 +62,13 @@ func doHTTPConnectHandshake(ctx context.Context, conn net.Conn, address resolver
 		}
 	}()
 
-	backendAddr := attributes.ProxyConnectAddr(address)
+	backendAddr := proxyattributes.ProxyConnectAddr(address)
 	req := &http.Request{
 		Method: http.MethodConnect,
 		URL:    &url.URL{Host: backendAddr},
 		Header: map[string][]string{"User-Agent": {grpcUA}},
 	}
-	if user := attributes.User(address); user != nil {
+	if user := proxyattributes.User(address); user != nil {
 		u := user.Username()
 		p, _ := user.Password()
 		req.Header.Add(proxyAuthHeaderKey, "Basic "+basicAuth(u, p))
@@ -101,7 +101,7 @@ func doHTTPConnectHandshake(ctx context.Context, conn net.Conn, address resolver
 	return conn, nil
 }
 
-// proxyDial dials, connecting to a proxy first if necessary. Dials, does the
+// proxyDial dials, connecting to a proxy first.It dials, does the
 // HTTP CONNECT handshake, and returns the connection.
 func proxyDial(ctx context.Context, address resolver.Address, grpcUA string) (net.Conn, error) {
 	conn, err := internal.NetDialerWithTCPKeepalive().DialContext(ctx, "tcp", address.Addr)

resolver_wrapper.go

@@ -79,8 +79,10 @@ func (ccr *ccResolverWrapper) start() error {
 			Authority:            ccr.cc.authority,
 		}
 		var err error
-		// The delegating resolver is not used if WithNoProxy or WithCustomDialer is set.
-		// It is not used when explicitly disabled with TargetResolutionEnabled as well.
+		// The delegating resolver is used unless
+		//   - A custom dialer is set using WithContextDialer dialoption.
+		//   - Proxy usage is explicitly disabled using WithNoProxy dialoption.
+		//   - Client-side resolution is explicitly enforced using WithTargetResolutionEnabled.
 		// In these cases, the normal name resolver determined by the scheme will be used directly.
 		if ccr.cc.dopts.copts.Dialer != nil || !ccr.cc.dopts.UseProxy {
 			ccr.resolver, err = ccr.cc.resolverBuilder.Build(ccr.cc.parsedTarget, ccr, opts)

test/proxy_test.go

@@ -29,6 +29,7 @@ import (
 
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/credentials/insecure"
+	"google.golang.org/grpc/internal"
 	"google.golang.org/grpc/internal/resolver/delegatingresolver"
 	"google.golang.org/grpc/internal/stubserver"
 	"google.golang.org/grpc/internal/testutils"
@@ -42,13 +43,12 @@ const (
 	unresolvedProxyURI  = "proxyExample.com"
 )
 
-// overwriteAndRestore temporarily replaces `HTTPSProxyFromEnvironment` with a
-// custom function and returns a function to restore the original.
-func overwriteAndRestore(customFunc func(req *http.Request) (*url.URL, error)) func() {
-	originalFunc := delegatingresolver.HTTPSProxyFromEnvironment
-	delegatingresolver.HTTPSProxyFromEnvironment = customFunc
+// overrideHTTPSProxyFromEnvironment function overwrites HTTPSProxyFromEnvironment and
+// returns a function to restore the default values.
+func overrideHTTPSProxyFromEnvironment(hpfe func(req *http.Request) (*url.URL, error)) func() {
+	internal.HTTPSProxyFromEnvironmentForTesting = hpfe
 	return func() {
-		delegatingresolver.HTTPSProxyFromEnvironment = originalFunc
+		internal.HTTPSProxyFromEnvironmentForTesting = nil
 	}
 }
 
@@ -100,7 +100,7 @@ func setupProxy(t *testing.T, backendAddr string, resolutionOnClient bool, reqCh
 }
 
 // TestGrpcDialWithProxy tests grpc.Dial using a proxy and default
-// resolver in the target URI.and verifies that it connects to the proxy server
+// resolver in the target URI and verifies that it connects to the proxy server
 // and sends unresolved target URI in the HTTP CONNECT request and then
 // connects to the backend server.
 func (s) TestGrpcDialWithProxy(t *testing.T) {
@@ -124,7 +124,7 @@ func (s) TestGrpcDialWithProxy(t *testing.T) {
 		}
 		return nil, nil
 	}
-	defer overwriteAndRestore(hpfe)()
+	defer overrideHTTPSProxyFromEnvironment(hpfe)()
 
 	ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
 	defer cancel()
@@ -175,7 +175,7 @@ func (s) TestGrpcDialWithProxyAndResolution(t *testing.T) {
 		}
 		return nil, nil
 	}
-	defer overwriteAndRestore(hpfe)()
+	defer overrideHTTPSProxyFromEnvironment(hpfe)()
 
 	// Set up a manual resolver for proxy resolution.
 	mrProxy := setupDNS(t)
@@ -248,7 +248,7 @@ func (s) TestGrpcNewClientWithProxy(t *testing.T) {
 		}
 		return nil, nil
 	}
-	defer overwriteAndRestore(hpfe)()
+	defer overrideHTTPSProxyFromEnvironment(hpfe)()
 
 	// Set up a manual resolver for proxy resolution.
 	mrProxy := setupDNS(t)
@@ -333,7 +333,7 @@ func (s) TestGrpcNewClientWithProxyAndCustomResolver(t *testing.T) {
 		}
 		return nil, nil
 	}
-	defer overwriteAndRestore(hpfe)()
+	defer overrideHTTPSProxyFromEnvironment(hpfe)()
 
 	// Dial options for the gRPC client.
 	dopts := []grpc.DialOption{
@@ -426,16 +426,15 @@ func (s) TestGrpcNewClientWithProxyAndTargetResolutionEnabled(t *testing.T) {
 		}
 		return nil, nil
 	}
-	defer overwriteAndRestore(hpfe)()
+	defer overrideHTTPSProxyFromEnvironment(hpfe)()
 
-	// Configure manual resolvers for both proxy and target backends.
+	// Configure manual resolvers for both proxy and target backends
+	targetResolver := setupDNS(t)
+	targetResolver.InitialState(resolver.State{Addresses: []resolver.Address{{Addr: backendAddr}}})
 	proxyResolver := manual.NewBuilderWithScheme("whatever")
 	resolver.Register(proxyResolver)
 	proxyResolver.InitialState(resolver.State{Addresses: []resolver.Address{{Addr: proxyLis.Addr().String()}}})
 
-	targetResolver := setupDNS(t)
-	targetResolver.InitialState(resolver.State{Addresses: []resolver.Address{{Addr: backendAddr}}})
-
 	// Dial options with target resolution enabled.
 	dopts := []grpc.DialOption{
 		grpc.WithTransportCredentials(insecure.NewCredentials()),
@@ -620,7 +619,7 @@ func (s) TestBasicAuthInGrpcNewClientWithProxy(t *testing.T) {
 		}
 		return nil, nil
 	}
-	defer overwriteAndRestore(hpfe)()
+	defer overrideHTTPSProxyFromEnvironment(hpfe)()
 
 	// Set up a manual resolver for proxy resolution.
 	mrProxy := setupDNS(t)