fix: set internal.UserSetDefaultScheme inside of SetDefaultScheme()

clientconn.go

@@ -712,7 +712,6 @@ func init() {
 	internal.ExitIdleModeForTesting = func(cc *ClientConn) error {
 		return cc.idlenessMgr.ExitIdleMode()
 	}
-	internal.UserSetDefaultScheme = resolver.GetDefaultScheme() != "dns"
 }
 
 func (cc *ClientConn) maybeApplyDefaultServiceConfig(addrs []resolver.Address) {
@@ -1751,7 +1750,7 @@ func (cc *ClientConn) parseTargetAndFindResolver() error {
 	// always use passthrough scheme. For either case, we need to respect any overridden
 	// global defaults set by the user.
 	defScheme := cc.dopts.defScheme
-	if defScheme == "" || internal.UserSetDefaultScheme {
+	if internal.UserSetDefaultScheme {
 		defScheme = resolver.GetDefaultScheme()
 	}
 

clientconn_parsed_target_test.go

@@ -33,29 +33,75 @@ import (
 	"google.golang.org/grpc/resolver"
 )
 
+func generateTarget(scheme string, target string) resolver.Target {
+	return resolver.Target{URL: *testutils.MustParseURL(fmt.Sprintf("%s:///%s", scheme, target))}
+}
+
 func (s) TestParsedTarget_Success_WithoutCustomDialer(t *testing.T) {
-	defScheme := "passthrough"
+	dialScheme := resolver.GetDefaultScheme()
+	newClientScheme := "dns"
 	tests := []struct {
-		target     string
-		wantParsed resolver.Target
+		target             string
+		wantDialParse      resolver.Target
+		wantNewClientParse resolver.Target
 	}{
 		// No scheme is specified.
-		{target: "://a/b", wantParsed: resolver.Target{URL: *testutils.MustParseURL(fmt.Sprintf("%s:///%s", defScheme, "://a/b"))}},
-		{target: "a//b", wantParsed: resolver.Target{URL: *testutils.MustParseURL(fmt.Sprintf("%s:///%s", defScheme, "a//b"))}},
+		{
+			target:             "://a/b",
+			wantDialParse:      generateTarget(dialScheme, "://a/b"),
+			wantNewClientParse: generateTarget(newClientScheme, "://a/b"),
+		},
+		{
+			target:             "a//b",
+			wantDialParse:      generateTarget(dialScheme, "a//b"),
+			wantNewClientParse: generateTarget(newClientScheme, "a//b"),
+		},
 
 		// An unregistered scheme is specified.
-		{target: "a:///", wantParsed: resolver.Target{URL: *testutils.MustParseURL(fmt.Sprintf("%s:///%s", defScheme, "a:///"))}},
-		{target: "a:b", wantParsed: resolver.Target{URL: *testutils.MustParseURL(fmt.Sprintf("%s:///%s", defScheme, "a:b"))}},
+		{
+			target:             "a:///",
+			wantDialParse:      generateTarget(dialScheme, "a:///"),
+			wantNewClientParse: generateTarget(newClientScheme, "a:///"),
+		},
+		{
+			target:             "a:b",
+			wantDialParse:      generateTarget(dialScheme, "a:b"),
+			wantNewClientParse: generateTarget(newClientScheme, "a:b"),
+		},
 
 		// A registered scheme is specified.
-		{target: "dns://a.server.com/google.com", wantParsed: resolver.Target{URL: *testutils.MustParseURL("dns://a.server.com/google.com")}},
-		{target: "unix-abstract:/ a///://::!@#$%25^&*()b", wantParsed: resolver.Target{URL: *testutils.MustParseURL("unix-abstract:/ a///://::!@#$%25^&*()b")}},
-		{target: "unix-abstract:passthrough:abc", wantParsed: resolver.Target{URL: *testutils.MustParseURL("unix-abstract:passthrough:abc")}},
-		{target: "passthrough:///unix:///a/b/c", wantParsed: resolver.Target{URL: *testutils.MustParseURL("passthrough:///unix:///a/b/c")}},
+		{
+			target:             "dns://a.server.com/google.com",
+			wantDialParse:      resolver.Target{URL: *testutils.MustParseURL("dns://a.server.com/google.com")},
+			wantNewClientParse: resolver.Target{URL: *testutils.MustParseURL("dns://a.server.com/google.com")},
+		},
+		{
+			target:             "unix-abstract:/ a///://::!@#$%25^&*()b",
+			wantDialParse:      resolver.Target{URL: *testutils.MustParseURL("unix-abstract:/ a///://::!@#$%25^&*()b")},
+			wantNewClientParse: resolver.Target{URL: *testutils.MustParseURL("unix-abstract:/ a///://::!@#$%25^&*()b")},
+		},
+		{
+			target:             "unix-abstract:passthrough:abc",
+			wantDialParse:      resolver.Target{URL: *testutils.MustParseURL("unix-abstract:passthrough:abc")},
+			wantNewClientParse: resolver.Target{URL: *testutils.MustParseURL("unix-abstract:passthrough:abc")},
+		},
+		{
+			target:             "passthrough:///unix:///a/b/c",
+			wantDialParse:      resolver.Target{URL: *testutils.MustParseURL("passthrough:///unix:///a/b/c")},
+			wantNewClientParse: resolver.Target{URL: *testutils.MustParseURL("passthrough:///unix:///a/b/c")},
+		},
 
 		// Cases for `scheme:absolute-path`.
-		{target: "dns:/a/b/c", wantParsed: resolver.Target{URL: *testutils.MustParseURL("dns:/a/b/c")}},
-		{target: "unregistered:/a/b/c", wantParsed: resolver.Target{URL: *testutils.MustParseURL(fmt.Sprintf("%s:///%s", defScheme, "unregistered:/a/b/c"))}},
+		{
+			target:             "dns:/a/b/c",
+			wantDialParse:      resolver.Target{URL: *testutils.MustParseURL("dns:/a/b/c")},
+			wantNewClientParse: resolver.Target{URL: *testutils.MustParseURL("dns:/a/b/c")},
+		},
+		{
+			target:             "unregistered:/a/b/c",
+			wantDialParse:      generateTarget(dialScheme, "unregistered:/a/b/c"),
+			wantNewClientParse: generateTarget(newClientScheme, "unregistered:/a/b/c"),
+		},
 	}
 
 	for _, test := range tests {
@@ -66,8 +112,18 @@ func (s) TestParsedTarget_Success_WithoutCustomDialer(t *testing.T) {
 			}
 			defer cc.Close()
 
-			if !cmp.Equal(cc.parsedTarget, test.wantParsed) {
-				t.Errorf("cc.parsedTarget for dial target %q = %+v, want %+v", test.target, cc.parsedTarget, test.wantParsed)
+			if !cmp.Equal(cc.parsedTarget, test.wantDialParse) {
+				t.Errorf("cc.parsedTarget for dial target %q = %+v, want %+v", test.target, cc.parsedTarget, test.wantDialParse)
+			}
+
+			cc, err = NewClient(test.target, WithTransportCredentials(insecure.NewCredentials()))
+			if err != nil {
+				t.Fatalf("NewClient(%q) failed: %v", test.target, err)
+			}
+			defer cc.Close()
+
+			if !cmp.Equal(cc.parsedTarget, test.wantNewClientParse) {
+				t.Errorf("cc.parsedTarget for newClient target %q = %+v, want %+v", test.target, cc.parsedTarget, test.wantNewClientParse)
 			}
 		})
 	}
@@ -93,7 +149,7 @@ func (s) TestParsedTarget_Failure_WithoutCustomDialer(t *testing.T) {
 }
 
 func (s) TestParsedTarget_WithCustomDialer(t *testing.T) {
-	defScheme := "passthrough"
+	defScheme := resolver.GetDefaultScheme()
 	tests := []struct {
 		target            string
 		wantParsed        resolver.Target

credentials/google/google_test.go

@@ -20,11 +20,11 @@ package google
 
 import (
 	"context"
+	"google.golang.org/grpc/internal/xds"
 	"net"
 	"testing"
 
 	"google.golang.org/grpc/credentials"
-	"google.golang.org/grpc/internal"
 	icredentials "google.golang.org/grpc/internal/credentials"
 	"google.golang.org/grpc/internal/grpctest"
 	"google.golang.org/grpc/resolver"
@@ -109,31 +109,31 @@ func (s) TestClientHandshakeBasedOnClusterName(t *testing.T) {
 			{
 				name: "with non-CFE cluster name",
 				ctx: icredentials.NewClientHandshakeInfoContext(context.Background(), credentials.ClientHandshakeInfo{
-					Attributes: internal.SetXDSHandshakeClusterName(resolver.Address{}, "lalala").Attributes,
+					Attributes: xds.SetXDSHandshakeClusterName(resolver.Address{}, "lalala").Attributes,
 				}),
 				// non-CFE backends should use alts.
 				wantTyp: "alts",
 			},
 			{
 				name: "with CFE cluster name",
 				ctx: icredentials.NewClientHandshakeInfoContext(context.Background(), credentials.ClientHandshakeInfo{
-					Attributes: internal.SetXDSHandshakeClusterName(resolver.Address{}, "google_cfe_bigtable.googleapis.com").Attributes,
+					Attributes: xds.SetXDSHandshakeClusterName(resolver.Address{}, "google_cfe_bigtable.googleapis.com").Attributes,
 				}),
 				// CFE should use tls.
 				wantTyp: "tls",
 			},
 			{
 				name: "with xdstp CFE cluster name",
 				ctx: icredentials.NewClientHandshakeInfoContext(context.Background(), credentials.ClientHandshakeInfo{
-					Attributes: internal.SetXDSHandshakeClusterName(resolver.Address{}, "xdstp://traffic-director-c2p.xds.googleapis.com/envoy.config.cluster.v3.Cluster/google_cfe_bigtable.googleapis.com").Attributes,
+					Attributes: xds.SetXDSHandshakeClusterName(resolver.Address{}, "xdstp://traffic-director-c2p.xds.googleapis.com/envoy.config.cluster.v3.Cluster/google_cfe_bigtable.googleapis.com").Attributes,
 				}),
 				// CFE should use tls.
 				wantTyp: "tls",
 			},
 			{
 				name: "with xdstp non-CFE cluster name",
 				ctx: icredentials.NewClientHandshakeInfoContext(context.Background(), credentials.ClientHandshakeInfo{
-					Attributes: internal.SetXDSHandshakeClusterName(resolver.Address{}, "xdstp://other.com/envoy.config.cluster.v3.Cluster/google_cfe_bigtable.googleapis.com").Attributes,
+					Attributes: xds.SetXDSHandshakeClusterName(resolver.Address{}, "xdstp://other.com/envoy.config.cluster.v3.Cluster/google_cfe_bigtable.googleapis.com").Attributes,
 				}),
 				// non-CFE should use atls.
 				wantTyp: "alts",

credentials/google/xds.go

@@ -20,12 +20,12 @@ package google
 
 import (
 	"context"
+	"google.golang.org/grpc/internal/xds"
 	"net"
 	"net/url"
 	"strings"
 
 	"google.golang.org/grpc/credentials"
-	"google.golang.org/grpc/internal"
 )
 
 const cfeClusterNamePrefix = "google_cfe_"
@@ -63,7 +63,7 @@ func clusterName(ctx context.Context) string {
 	if chi.Attributes == nil {
 		return ""
 	}
-	cluster, _ := internal.GetXDSHandshakeClusterName(chi.Attributes)
+	cluster, _ := xds.GetXDSHandshakeClusterName(chi.Attributes)
 	return cluster
 }
 

credentials/google/xds_test.go

@@ -20,18 +20,18 @@ package google
 
 import (
 	"context"
+	"google.golang.org/grpc/internal/xds"
 	"testing"
 
 	"google.golang.org/grpc/credentials"
-	"google.golang.org/grpc/internal"
 	icredentials "google.golang.org/grpc/internal/credentials"
 	"google.golang.org/grpc/resolver"
 )
 
 func (s) TestIsDirectPathCluster(t *testing.T) {
 	c := func(cluster string) context.Context {
 		return icredentials.NewClientHandshakeInfoContext(context.Background(), credentials.ClientHandshakeInfo{
-			Attributes: internal.SetXDSHandshakeClusterName(resolver.Address{}, cluster).Attributes,
+			Attributes: xds.SetXDSHandshakeClusterName(resolver.Address{}, cluster).Attributes,
 		})
 	}
 

internal/internal.go

@@ -199,7 +199,7 @@ var (
 	FromOutgoingContextRaw any // func(context.Context) (metadata.MD, [][]string, bool)
 
 	// UserSetDefaultScheme is set to true if the user has overridden the default resolver scheme.
-	UserSetDefaultScheme bool
+	UserSetDefaultScheme bool = false
 )
 
 // HealthChecker defines the signature of the client-side LB channel health checking function.

internal/xds_handshake_cluster.go → internal/xds/xds.go

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-package internal
+package xds
 
 import (
 	"google.golang.org/grpc/attributes"

resolver/resolver.go

@@ -23,6 +23,7 @@ package resolver
 import (
 	"context"
 	"fmt"
+	"google.golang.org/grpc/internal"
 	"net"
 	"net/url"
 	"strings"
@@ -36,7 +37,7 @@ var (
 	// m is a map from scheme to resolver builder.
 	m = make(map[string]Builder)
 	// defaultScheme is the default scheme to use.
-	defaultScheme = "dns"
+	defaultScheme = "passthrough"
 )
 
 // TODO(bar) install dns resolver in init(){}.
@@ -63,17 +64,18 @@ func Get(scheme string) Builder {
 }
 
 // SetDefaultScheme sets the default scheme that will be used. The default
-// scheme is "dns".
+// scheme is "passthrough".
 //
 // NOTE: this function must only be called during initialization time (i.e. in
 // an init() function), and is not thread-safe. The scheme set last overrides
 // previously set values.
 func SetDefaultScheme(scheme string) {
 	defaultScheme = scheme
+	internal.UserSetDefaultScheme = true
 }
 
-// GetDefaultScheme gets the default scheme that will be used, if no scheme was
-// overridden, defaults to the "dns" scheme.
+// GetDefaultScheme gets the default scheme that will be used by grpc.Dial.  If
+// SetDefaultScheme is never called, the default scheme used by grpc.NewClient is "dns" instead.
 func GetDefaultScheme() string {
 	return defaultScheme
 }

xds/internal/balancer/clusterimpl/balancer_test.go

@@ -22,6 +22,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"google.golang.org/grpc/internal/xds"
 	"strings"
 	"testing"
 	"time"
@@ -32,7 +33,6 @@ import (
 	"google.golang.org/grpc/balancer/base"
 	"google.golang.org/grpc/balancer/roundrobin"
 	"google.golang.org/grpc/connectivity"
-	"google.golang.org/grpc/internal"
 	"google.golang.org/grpc/internal/balancer/stub"
 	"google.golang.org/grpc/internal/grpctest"
 	internalserviceconfig "google.golang.org/grpc/internal/serviceconfig"
@@ -464,7 +464,7 @@ func (s) TestClusterNameInAddressAttributes(t *testing.T) {
 	if got, want := addrs1[0].Addr, testBackendAddrs[0].Addr; got != want {
 		t.Fatalf("sc is created with addr %v, want %v", got, want)
 	}
-	cn, ok := internal.GetXDSHandshakeClusterName(addrs1[0].Attributes)
+	cn, ok := xds.GetXDSHandshakeClusterName(addrs1[0].Attributes)
 	if !ok || cn != testClusterName {
 		t.Fatalf("sc is created with addr with cluster name %v, %v, want cluster name %v", cn, ok, testClusterName)
 	}
@@ -495,7 +495,7 @@ func (s) TestClusterNameInAddressAttributes(t *testing.T) {
 		t.Fatalf("sc is created with addr %v, want %v", got, want)
 	}
 	// New addresses should have the new cluster name.
-	cn2, ok := internal.GetXDSHandshakeClusterName(addrs2[0].Attributes)
+	cn2, ok := xds.GetXDSHandshakeClusterName(addrs2[0].Attributes)
 	if !ok || cn2 != testClusterName2 {
 		t.Fatalf("sc is created with addr with cluster name %v, %v, want cluster name %v", cn2, ok, testClusterName2)
 	}

xds/internal/balancer/clusterimpl/clusterimpl.go

@@ -26,12 +26,12 @@ package clusterimpl
 import (
 	"encoding/json"
 	"fmt"
+	"google.golang.org/grpc/internal/xds"
 	"sync"
 	"sync/atomic"
 
 	"google.golang.org/grpc/balancer"
 	"google.golang.org/grpc/connectivity"
-	"google.golang.org/grpc/internal"
 	"google.golang.org/grpc/internal/balancer/gracefulswitch"
 	"google.golang.org/grpc/internal/buffer"
 	"google.golang.org/grpc/internal/grpclog"
@@ -359,7 +359,7 @@ func (b *clusterImplBalancer) NewSubConn(addrs []resolver.Address, opts balancer
 	newAddrs := make([]resolver.Address, len(addrs))
 	var lID xdsinternal.LocalityID
 	for i, addr := range addrs {
-		newAddrs[i] = internal.SetXDSHandshakeClusterName(addr, clusterName)
+		newAddrs[i] = xds.SetXDSHandshakeClusterName(addr, clusterName)
 		lID = xdsinternal.GetLocalityID(newAddrs[i])
 	}
 	var sc balancer.SubConn
@@ -384,7 +384,7 @@ func (b *clusterImplBalancer) UpdateAddresses(sc balancer.SubConn, addrs []resol
 	newAddrs := make([]resolver.Address, len(addrs))
 	var lID xdsinternal.LocalityID
 	for i, addr := range addrs {
-		newAddrs[i] = internal.SetXDSHandshakeClusterName(addr, clusterName)
+		newAddrs[i] = xds.SetXDSHandshakeClusterName(addr, clusterName)
 		lID = xdsinternal.GetLocalityID(newAddrs[i])
 	}
 	if scw, ok := sc.(*scWrapper); ok {