Skip to content

Commit

Permalink
Responded to Doug's comments
Browse files Browse the repository at this point in the history
  • Loading branch information
zasweq committed May 16, 2024
1 parent d8a39a4 commit 0e061b3
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 18 deletions.
14 changes: 5 additions & 9 deletions clientconn.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ func NewClient(target string, opts ...DialOption) (conn *ClientConn, err error)
}

for _, opt := range globalPerTargetDialOptions {
opt.DialOption(cc.parsedTarget.URL).apply(&cc.dopts)
opt.DialOptionForTarget(cc.parsedTarget.URL).apply(&cc.dopts)
}

chainUnaryClientInterceptors(cc)
Expand All @@ -182,8 +182,10 @@ func NewClient(target string, opts ...DialOption) (conn *ClientConn, err error)
return nil, err
}

// Register ClientConn with channelz.
// Register ClientConn with channelz. Note that this is only done after
// channel creation cannot fail.
cc.channelzRegistration(target)
channelz.Infof(logger, cc.channelz, "parsed dial target is: %#v", cc.parsedTarget)
channelz.Infof(logger, cc.channelz, "Channel authority set to %q", cc.authority)

cc.csMgr = newConnectivityStateManager(cc.ctx, cc.channelz)
Expand Down Expand Up @@ -1684,10 +1686,7 @@ func (cc *ClientConn) parseTargetAndFindResolver() error {

var rb resolver.Builder
parsedTarget, err := parseTarget(cc.target)
if err != nil {
logger.Infof("dial target %q parse failed: %v", cc.target, err)
} else {
logger.Infof("parsed dial target is: %#v", parsedTarget)
if err == nil {
rb = cc.getResolver(parsedTarget.URL.Scheme)
if rb != nil {
cc.parsedTarget = parsedTarget
Expand All @@ -1706,15 +1705,12 @@ func (cc *ClientConn) parseTargetAndFindResolver() error {
defScheme = resolver.GetDefaultScheme()
}

logger.Infof("fallback to scheme %q", defScheme)
canonicalTarget := defScheme + ":///" + cc.target

parsedTarget, err = parseTarget(canonicalTarget)
if err != nil {
logger.Infof("dial target %q parse failed: %v", canonicalTarget, err)
return err
}
logger.Infof("parsed dial target is: %+v", parsedTarget)
rb = cc.getResolver(parsedTarget.URL.Scheme)
if rb == nil {
return fmt.Errorf("could not get resolver for default scheme: %q", parsedTarget.URL.Scheme)
Expand Down
17 changes: 9 additions & 8 deletions default_dial_option_server_option_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ func (s) TestAddGlobalDialOptions(t *testing.T) {
// Set and check the DialOptions
opts := []DialOption{WithTransportCredentials(insecure.NewCredentials()), WithTransportCredentials(insecure.NewCredentials()), WithTransportCredentials(insecure.NewCredentials())}
internal.AddGlobalDialOptions.(func(opt ...DialOption))(opts...)
defer internal.ClearGlobalDialOptions()
for i, opt := range opts {
if globalDialOptions[i] != opt {
t.Fatalf("Unexpected global dial option at index %d: %v != %v", i, globalDialOptions[i], opt)
Expand All @@ -65,19 +66,19 @@ func (s) TestAddGlobalDialOptions(t *testing.T) {
func (s) TestDisableGlobalOptions(t *testing.T) {
// Set transport credentials as a global option.
internal.AddGlobalDialOptions.(func(opt ...DialOption))(WithTransportCredentials(insecure.NewCredentials()))
defer internal.ClearGlobalDialOptions()
// Dial with the disable global options dial option. This dial should fail
// due to the global dial options with credentials not being picked up due
// to global options being disabled.
noTSecStr := "no transport security set"
if _, err := Dial("fake", internal.DisableGlobalDialOptions.(func() DialOption)()); !strings.Contains(fmt.Sprint(err), noTSecStr) {
t.Fatalf("Dialing received unexpected error: %v, want error containing \"%v\"", err, noTSecStr)
}
internal.ClearGlobalDialOptions()
}

type testPerTargetDialOption struct{}

func (do *testPerTargetDialOption) DialOption(parsedTarget url.URL) DialOption {
func (do *testPerTargetDialOption) DialOptionForTarget(parsedTarget url.URL) DialOption {
if parsedTarget.Scheme == "passthrough" {
return WithTransportCredentials(insecure.NewCredentials()) // credentials provided, should pass NewClient.
}
Expand All @@ -86,11 +87,12 @@ func (do *testPerTargetDialOption) DialOption(parsedTarget url.URL) DialOption {

// TestGlobalPerTargetDialOption configures a global per target dial option that
// produces transport credentials for channels using "passthrough" scheme.
// Channels that use the passthrough scheme should this be successfully created
// due to picking up transport credentials, whereas other channels should fail
// at creation due to not having transport credentials.
// Channels that use the passthrough scheme should be successfully created due
// to picking up transport credentials, whereas other channels should fail at
// creation due to not having transport credentials.
func (s) TestGlobalPerTargetDialOption(t *testing.T) {
internal.AddGlobalPerTargetDialOptions.(func(opt any))(&testPerTargetDialOption{})
defer internal.ClearGlobalPerTargetDialOptions()
noTSecStr := "no transport security set"
if _, err := NewClient("dns:///fake"); !strings.Contains(fmt.Sprint(err), noTSecStr) {
t.Fatalf("Dialing received unexpected error: %v, want error containing \"%v\"", err, noTSecStr)
Expand All @@ -99,15 +101,15 @@ func (s) TestGlobalPerTargetDialOption(t *testing.T) {
if err != nil {
t.Fatalf("Dialing with insecure credentials failed: %v", err)
}
defer cc.Close()
internal.ClearGlobalPerTargetDialOptions()
cc.Close()
}

func (s) TestAddGlobalServerOptions(t *testing.T) {
const maxRecvSize = 998765
// Set and check the ServerOptions
opts := []ServerOption{Creds(insecure.NewCredentials()), MaxRecvMsgSize(maxRecvSize)}
internal.AddGlobalServerOptions.(func(opt ...ServerOption))(opts...)
defer internal.ClearGlobalServerOptions()
for i, opt := range opts {
if globalServerOptions[i] != opt {
t.Fatalf("Unexpected global server option at index %d: %v != %v", i, globalServerOptions[i], opt)
Expand All @@ -120,7 +122,6 @@ func (s) TestAddGlobalServerOptions(t *testing.T) {
t.Fatalf("Unexpected s.opts.maxReceiveMessageSize: %d != %d", s.opts.maxReceiveMessageSize, maxRecvSize)
}

internal.ClearGlobalServerOptions()
if len(globalServerOptions) != 0 {
t.Fatalf("Unexpected len of globalServerOptions: %d != 0", len(globalServerOptions))
}
Expand Down
2 changes: 1 addition & 1 deletion dialoptions.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ var globalDialOptions []DialOption
// configuration set through a returned DialOption.
type perTargetDialOption interface {
// DialOption returns a Dial Option to apply.
DialOption(parsedTarget url.URL) DialOption
DialOptionForTarget(parsedTarget url.URL) DialOption
}

var globalPerTargetDialOptions []perTargetDialOption
Expand Down

0 comments on commit 0e061b3

Please sign in to comment.