Skip to content

Commit

Permalink
grpc: Canonicalize string returned by ClientConn.Target() and resolve…
Browse files Browse the repository at this point in the history
…r.Address.String() (grpc#6923)
  • Loading branch information
zasweq authored Feb 15, 2024
1 parent 29997a0 commit 3ae77e6
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 13 deletions.
5 changes: 2 additions & 3 deletions clientconn.go
Original file line number Diff line number Diff line change
Expand Up @@ -883,14 +883,14 @@ func (cc *ClientConn) channelzMetric() *channelz.ChannelInternalMetric {
}
}

// Target returns the target string of the ClientConn.
// Target returns the canonical target string of the ClientConn.
//
// # Experimental
//
// Notice: This API is EXPERIMENTAL and may be changed or removed in a
// later release.
func (cc *ClientConn) Target() string {
return cc.target
return cc.parsedTarget.String()
}

func (cc *ClientConn) incrCallsStarted() {
Expand Down Expand Up @@ -1744,7 +1744,6 @@ func (cc *ClientConn) parseTargetAndFindResolver() error {
defScheme := resolver.GetDefaultScheme()
channelz.Infof(logger, cc.channelzID, "fallback to scheme %q", defScheme)
canonicalTarget := defScheme + ":///" + cc.target

parsedTarget, err = parseTarget(canonicalTarget)
if err != nil {
channelz.Infof(logger, cc.channelzID, "dial target %q parse failed: %v", canonicalTarget, err)
Expand Down
45 changes: 37 additions & 8 deletions clientconn_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -808,15 +808,44 @@ func (s) TestMethodConfigDefaultService(t *testing.T) {
}
}

func (s) TestGetClientConnTarget(t *testing.T) {
addr := "nonexist:///non.existent"
cc, err := Dial(addr, WithTransportCredentials(insecure.NewCredentials()))
if err != nil {
t.Fatalf("Dial(%s, _) = _, %v, want _, <nil>", addr, err)
func (s) TestClientConn_Target(t *testing.T) {
tests := []struct {
name string
addr string
targetWant string
}{
{
name: "normal-case",
addr: "dns://a.server.com/google.com",
targetWant: "dns://a.server.com/google.com",
},
{
name: "canonical-target-not-specified",
addr: "no.scheme",
targetWant: "passthrough:///no.scheme",
},
{
name: "canonical-target-nonexistent",
addr: "nonexist:///non.existent",
targetWant: "passthrough:///nonexist:///non.existent",
},
{
name: "canonical-target-add-colon-slash",
addr: "dns:hostname:port",
targetWant: "dns:///hostname:port",
},
}
defer cc.Close()
if cc.Target() != addr {
t.Fatalf("Target() = %s, want %s", cc.Target(), addr)
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
cc, err := Dial(test.addr, WithTransportCredentials(insecure.NewCredentials()))
if err != nil {
t.Fatalf("Dial(%s, _) = _, %v, want _, <nil>", test.addr, err)
}
defer cc.Close()
if cc.Target() != test.targetWant {
t.Fatalf("Target() = %s, want %s", cc.Target(), test.targetWant)
}
})
}
}

Expand Down
4 changes: 2 additions & 2 deletions resolver/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -281,9 +281,9 @@ func (t Target) Endpoint() string {
return strings.TrimPrefix(endpoint, "/")
}

// String returns a string representation of Target.
// String returns the canonical string representation of Target.
func (t Target) String() string {
return t.URL.String()
return t.URL.Scheme + "://" + t.URL.Host + "/" + t.Endpoint()
}

// Builder creates a resolver that will be used to watch name resolution updates.
Expand Down

0 comments on commit 3ae77e6

Please sign in to comment.