grpc
/
grpc-go
mirror of https://github.com/grpc/grpc-go.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

resolver: replace resolver.Target.Endpoint field with Endpoint() method (#5852) 

Fixes https://github.com/grpc/grpc-go/issues/5796
This commit is contained in:
Kyle J. Burda 2023-01-24 15:03:56 -05:00 committed by GitHub
parent 894816c487
commit 3930549b38
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
14 changed files with 114 additions and 107 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
balancer/grpclb/grpclb.go
View File

@ -136,8 +136,8 @@ func (b *lbBuilder) Build(cc balancer.ClientConn, opt balancer.BuildOptions) bal
lb := &lbBalancer{ lb := &lbBalancer{
cc: newLBCacheClientConn(cc), cc: newLBCacheClientConn(cc),
dialTarget: opt.Target.Endpoint, dialTarget: opt.Target.Endpoint(),
target: opt.Target.Endpoint, target: opt.Target.Endpoint(),
opt: opt, opt: opt,
fallbackTimeout: b.fallbackTimeout, fallbackTimeout: b.fallbackTimeout,
doneCh: make(chan struct{}), doneCh: make(chan struct{}),

2
balancer/rls/balancer.go
View File

@ -481,7 +481,7 @@ func (b *rlsBalancer) sendNewPickerLocked() {
} }
picker := &rlsPicker{ picker := &rlsPicker{
kbm: b.lbCfg.kbMap, kbm: b.lbCfg.kbMap,
origEndpoint: b.bopts.Target.Endpoint, origEndpoint: b.bopts.Target.Endpoint(),
lb: b, lb: b,
defaultPolicy: b.defaultPolicy, defaultPolicy: b.defaultPolicy,
ctrlCh: b.ctrlCh, ctrlCh: b.ctrlCh,

19
clientconn.go
View File

@ -256,7 +256,7 @@ func DialContext(ctx context.Context, target string, opts ...DialOption) (conn *
if err != nil { if err != nil {
return nil, err return nil, err
} }
cc.authority, err = determineAuthority(cc.parsedTarget.Endpoint, cc.target, cc.dopts) cc.authority, err = determineAuthority(cc.parsedTarget.Endpoint(), cc.target, cc.dopts)
if err != nil { if err != nil {
return nil, err return nil, err
} }
@ -1587,30 +1587,17 @@ func (cc *ClientConn) parseTargetAndFindResolver() (resolver.Builder, error) {
} }
// parseTarget uses RFC 3986 semantics to parse the given target into a // parseTarget uses RFC 3986 semantics to parse the given target into a
// resolver.Target struct containing scheme, authority and endpoint. Query // resolver.Target struct containing scheme, authority and url. Query
// params are stripped from the endpoint. // params are stripped from the endpoint.
func parseTarget(target string) (resolver.Target, error) { func parseTarget(target string) (resolver.Target, error) {
u, err := url.Parse(target) u, err := url.Parse(target)
if err != nil { if err != nil {
return resolver.Target{}, err return resolver.Target{}, err
} }
// For targets of the form "[scheme]://[authority]/endpoint, the endpoint
// value returned from url.Parse() contains a leading "/". Although this is
// in accordance with RFC 3986, we do not want to break existing resolver
// implementations which expect the endpoint without the leading "/". So, we
// end up stripping the leading "/" here. But this will result in an
// incorrect parsing for something like "unix:///path/to/socket". Since we
// own the "unix" resolver, we can workaround in the unix resolver by using
// the `URL` field instead of the `Endpoint` field.
endpoint := u.Path
if endpoint == "" {
endpoint = u.Opaque
}
endpoint = strings.TrimPrefix(endpoint, "/")
return resolver.Target{ return resolver.Target{
Scheme: u.Scheme, Scheme: u.Scheme,
Authority: u.Host, Authority: u.Host,
Endpoint: endpoint,
URL: *u, URL: *u,
}, nil }, nil
} }

90
clientconn_parsed_target_test.go
View File

@ -21,6 +21,7 @@ package grpc
import ( import (
"context" "context"
"errors" "errors"
"fmt"
"net" "net"
"net/url" "net/url"
"testing" "testing"
@ -28,6 +29,7 @@ import (
"github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp"
"google.golang.org/grpc/credentials/insecure" "google.golang.org/grpc/credentials/insecure"
"google.golang.org/grpc/internal/testutils"
"google.golang.org/grpc/resolver" "google.golang.org/grpc/resolver"
) )
@ -40,46 +42,46 @@ func (s) TestParsedTarget_Success_WithoutCustomDialer(t *testing.T) {
wantParsed resolver.Target wantParsed resolver.Target
}{ }{
// No scheme is specified. // No scheme is specified.
{target: "://", badScheme: true, wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "://"}}, {target: "://", badScheme: true, wantParsed: resolver.Target{Scheme: defScheme, Authority: "", URL: *testutils.MustParseURL(fmt.Sprintf("%s:///%s", defScheme, "://"))}},
{target: ":///", badScheme: true, wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: ":///"}}, {target: ":///", badScheme: true, wantParsed: resolver.Target{Scheme: defScheme, Authority: "", URL: *testutils.MustParseURL(fmt.Sprintf("%s:///%s", defScheme, ":///"))}},
{target: "://a/", badScheme: true, wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "://a/"}}, {target: "://a/", badScheme: true, wantParsed: resolver.Target{Scheme: defScheme, Authority: "", URL: *testutils.MustParseURL(fmt.Sprintf("%s:///%s", defScheme, "://a/"))}},
{target: ":///a", badScheme: true, wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: ":///a"}}, {target: ":///a", badScheme: true, wantParsed: resolver.Target{Scheme: defScheme, Authority: "", URL: *testutils.MustParseURL(fmt.Sprintf("%s:///%s", defScheme, ":///a"))}},
{target: "://a/b", badScheme: true, wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "://a/b"}}, {target: "://a/b", badScheme: true, wantParsed: resolver.Target{Scheme: defScheme, Authority: "", URL: *testutils.MustParseURL(fmt.Sprintf("%s:///%s", defScheme, "://a/b"))}},
{target: "/", badScheme: true, wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "/"}}, {target: "/", badScheme: true, wantParsed: resolver.Target{Scheme: defScheme, Authority: "", URL: *testutils.MustParseURL(fmt.Sprintf("%s:///%s", defScheme, "/"))}},
{target: "a/b", badScheme: true, wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "a/b"}}, {target: "a/b", badScheme: true, wantParsed: resolver.Target{Scheme: defScheme, Authority: "", URL: *testutils.MustParseURL(fmt.Sprintf("%s:///%s", defScheme, "a/b"))}},
{target: "a//b", badScheme: true, wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "a//b"}}, {target: "a//b", badScheme: true, wantParsed: resolver.Target{Scheme: defScheme, Authority: "", URL: *testutils.MustParseURL(fmt.Sprintf("%s:///%s", defScheme, "a//b"))}},
{target: "google.com", badScheme: true, wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "google.com"}}, {target: "google.com", badScheme: true, wantParsed: resolver.Target{Scheme: defScheme, Authority: "", URL: *testutils.MustParseURL(fmt.Sprintf("%s:///%s", defScheme, "google.com"))}},
{target: "google.com/?a=b", badScheme: true, wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "google.com/"}}, {target: "google.com/?a=b", badScheme: true, wantParsed: resolver.Target{Scheme: defScheme, Authority: "", URL: *testutils.MustParseURL(fmt.Sprintf("%s:///%s", defScheme, "google.com/?a=b"))}},
{target: "/unix/socket/address", badScheme: true, wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "/unix/socket/address"}}, {target: "/unix/socket/address", badScheme: true, wantParsed: resolver.Target{Scheme: defScheme, Authority: "", URL: *testutils.MustParseURL(fmt.Sprintf("%s:///%s", defScheme, "/unix/socket/address"))}},
// An unregistered scheme is specified. // An unregistered scheme is specified.
{target: "a:///", badScheme: true, wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "a:///"}}, {target: "a:///", badScheme: true, wantParsed: resolver.Target{Scheme: defScheme, Authority: "", URL: *testutils.MustParseURL(fmt.Sprintf("%s:///%s", defScheme, "a:///"))}},
{target: "a://b/", badScheme: true, wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "a://b/"}}, {target: "a://b/", badScheme: true, wantParsed: resolver.Target{Scheme: defScheme, Authority: "", URL: *testutils.MustParseURL(fmt.Sprintf("%s:///%s", defScheme, "a://b/"))}},
{target: "a:///b", badScheme: true, wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "a:///b"}}, {target: "a:///b", badScheme: true, wantParsed: resolver.Target{Scheme: defScheme, Authority: "", URL: *testutils.MustParseURL(fmt.Sprintf("%s:///%s", defScheme, "a:///b"))}},
{target: "a://b/c", badScheme: true, wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "a://b/c"}}, {target: "a://b/c", badScheme: true, wantParsed: resolver.Target{Scheme: defScheme, Authority: "", URL: *testutils.MustParseURL(fmt.Sprintf("%s:///%s", defScheme, "a://b/c"))}},
{target: "a:b", badScheme: true, wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "a:b"}}, {target: "a:b", badScheme: true, wantParsed: resolver.Target{Scheme: defScheme, Authority: "", URL: *testutils.MustParseURL(fmt.Sprintf("%s:///%s", defScheme, "a:b"))}},
{target: "a:/b", badScheme: true, wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "a:/b"}}, {target: "a:/b", badScheme: true, wantParsed: resolver.Target{Scheme: defScheme, Authority: "", URL: *testutils.MustParseURL(fmt.Sprintf("%s:///%s", defScheme, "a:/b"))}},
{target: "a://b", badScheme: true, wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "a://b"}}, {target: "a://b", badScheme: true, wantParsed: resolver.Target{Scheme: defScheme, Authority: "", URL: *testutils.MustParseURL(fmt.Sprintf("%s:///%s", defScheme, "a://b"))}},
// A registered scheme is specified. // A registered scheme is specified.
{target: "dns:///google.com", wantParsed: resolver.Target{Scheme: "dns", Authority: "", Endpoint: "google.com"}}, {target: "dns:///google.com", wantParsed: resolver.Target{Scheme: "dns", Authority: "", URL: *testutils.MustParseURL("dns:///google.com")}},
{target: "dns://a.server.com/google.com", wantParsed: resolver.Target{Scheme: "dns", Authority: "a.server.com", Endpoint: "google.com"}}, {target: "dns://a.server.com/google.com", wantParsed: resolver.Target{Scheme: "dns", Authority: "a.server.com", URL: *testutils.MustParseURL("dns://a.server.com/google.com")}},
{target: "dns://a.server.com/google.com/?a=b", wantParsed: resolver.Target{Scheme: "dns", Authority: "a.server.com", Endpoint: "google.com/"}}, {target: "dns://a.server.com/google.com/?a=b", wantParsed: resolver.Target{Scheme: "dns", Authority: "a.server.com", URL: *testutils.MustParseURL("dns://a.server.com/google.com/?a=b")}},
{target: "unix:///a/b/c", wantParsed: resolver.Target{Scheme: "unix", Authority: "", Endpoint: "a/b/c"}}, {target: "unix:///a/b/c", wantParsed: resolver.Target{Scheme: "unix", Authority: "", URL: *testutils.MustParseURL("unix:///a/b/c")}},
{target: "unix-abstract:a/b/c", wantParsed: resolver.Target{Scheme: "unix-abstract", Authority: "", Endpoint: "a/b/c"}}, {target: "unix-abstract:a/b/c", wantParsed: resolver.Target{Scheme: "unix-abstract", Authority: "", URL: *testutils.MustParseURL("unix-abstract:a/b/c")}},
{target: "unix-abstract:a b", wantParsed: resolver.Target{Scheme: "unix-abstract", Authority: "", Endpoint: "a b"}}, {target: "unix-abstract:a b", wantParsed: resolver.Target{Scheme: "unix-abstract", Authority: "", URL: *testutils.MustParseURL("unix-abstract:a b")}},
{target: "unix-abstract:a:b", wantParsed: resolver.Target{Scheme: "unix-abstract", Authority: "", Endpoint: "a:b"}}, {target: "unix-abstract:a:b", wantParsed: resolver.Target{Scheme: "unix-abstract", Authority: "", URL: *testutils.MustParseURL("unix-abstract:a:b")}},
{target: "unix-abstract:a-b", wantParsed: resolver.Target{Scheme: "unix-abstract", Authority: "", Endpoint: "a-b"}}, {target: "unix-abstract:a-b", wantParsed: resolver.Target{Scheme: "unix-abstract", Authority: "", URL: *testutils.MustParseURL("unix-abstract:a-b")}},
{target: "unix-abstract:/ a///://::!@#$%25^&*()b", wantParsed: resolver.Target{Scheme: "unix-abstract", Authority: "", Endpoint: " a///://::!@"}}, {target: "unix-abstract:/ a///://::!@#$%25^&*()b", wantParsed: resolver.Target{Scheme: "unix-abstract", Authority: "", URL: *testutils.MustParseURL("unix-abstract:/ a///://::!@#$%25^&*()b")}},
{target: "unix-abstract:passthrough:abc", wantParsed: resolver.Target{Scheme: "unix-abstract", Authority: "", Endpoint: "passthrough:abc"}}, {target: "unix-abstract:passthrough:abc", wantParsed: resolver.Target{Scheme: "unix-abstract", Authority: "", URL: *testutils.MustParseURL("unix-abstract:passthrough:abc")}},
{target: "unix-abstract:unix:///abc", wantParsed: resolver.Target{Scheme: "unix-abstract", Authority: "", Endpoint: "unix:///abc"}}, {target: "unix-abstract:unix:///abc", wantParsed: resolver.Target{Scheme: "unix-abstract", Authority: "", URL: *testutils.MustParseURL("unix-abstract:unix:///abc")}},
{target: "unix-abstract:///a/b/c", wantParsed: resolver.Target{Scheme: "unix-abstract", Authority: "", Endpoint: "a/b/c"}}, {target: "unix-abstract:///a/b/c", wantParsed: resolver.Target{Scheme: "unix-abstract", Authority: "", URL: *testutils.MustParseURL("unix-abstract:///a/b/c")}},
{target: "unix-abstract:///", wantParsed: resolver.Target{Scheme: "unix-abstract", Authority: "", Endpoint: ""}}, {target: "unix-abstract:///", wantParsed: resolver.Target{Scheme: "unix-abstract", Authority: "", URL: *testutils.MustParseURL("unix-abstract:///")}},
{target: "passthrough:///unix:///a/b/c", wantParsed: resolver.Target{Scheme: "passthrough", Authority: "", Endpoint: "unix:///a/b/c"}}, {target: "passthrough:///unix:///a/b/c", wantParsed: resolver.Target{Scheme: "passthrough", Authority: "", URL: *testutils.MustParseURL("passthrough:///unix:///a/b/c")}},
// Cases for `scheme:absolute-path`. // Cases for `scheme:absolute-path`.
{target: "dns:/a/b/c", wantParsed: resolver.Target{Scheme: "dns", Authority: "", Endpoint: "a/b/c"}}, {target: "dns:/a/b/c", wantParsed: resolver.Target{Scheme: "dns", Authority: "", URL: *testutils.MustParseURL("dns:/a/b/c")}},
{target: "unregistered:/a/b/c", badScheme: true, wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "unregistered:/a/b/c"}}, {target: "unregistered:/a/b/c", badScheme: true, wantParsed: resolver.Target{Scheme: defScheme, Authority: "", URL: *testutils.MustParseURL("unregistered:/a/b/c")}},
} }
for _, test := range tests { for _, test := range tests {
@ -138,56 +140,56 @@ func (s) TestParsedTarget_WithCustomDialer(t *testing.T) {
// different behaviors with a custom dialer. // different behaviors with a custom dialer.
{ {
target: "unix:a/b/c", target: "unix:a/b/c",
wantParsed: resolver.Target{Scheme: "unix", Authority: "", Endpoint: "a/b/c"}, wantParsed: resolver.Target{Scheme: "unix", Authority: "", URL: *testutils.MustParseURL("unix:a/b/c")},
wantDialerAddress: "unix:a/b/c", wantDialerAddress: "unix:a/b/c",
}, },
{ {
target: "unix:/a/b/c", target: "unix:/a/b/c",
wantParsed: resolver.Target{Scheme: "unix", Authority: "", Endpoint: "a/b/c"}, wantParsed: resolver.Target{Scheme: "unix", Authority: "", URL: *testutils.MustParseURL("unix:/a/b/c")},
wantDialerAddress: "unix:///a/b/c", wantDialerAddress: "unix:///a/b/c",
}, },
{ {
target: "unix:///a/b/c", target: "unix:///a/b/c",
wantParsed: resolver.Target{Scheme: "unix", Authority: "", Endpoint: "a/b/c"}, wantParsed: resolver.Target{Scheme: "unix", Authority: "", URL: *testutils.MustParseURL("unix:///a/b/c")},
wantDialerAddress: "unix:///a/b/c", wantDialerAddress: "unix:///a/b/c",
}, },
{ {
target: "dns:///127.0.0.1:50051", target: "dns:///127.0.0.1:50051",
wantParsed: resolver.Target{Scheme: "dns", Authority: "", Endpoint: "127.0.0.1:50051"}, wantParsed: resolver.Target{Scheme: "dns", Authority: "", URL: *testutils.MustParseURL("dns:///127.0.0.1:50051")},
wantDialerAddress: "127.0.0.1:50051", wantDialerAddress: "127.0.0.1:50051",
}, },
{ {
target: ":///127.0.0.1:50051", target: ":///127.0.0.1:50051",
badScheme: true, badScheme: true,
wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: ":///127.0.0.1:50051"}, wantParsed: resolver.Target{Scheme: defScheme, Authority: "", URL: *testutils.MustParseURL(fmt.Sprintf("%s:///%s", defScheme, ":///127.0.0.1:50051"))},
wantDialerAddress: ":///127.0.0.1:50051", wantDialerAddress: ":///127.0.0.1:50051",
}, },
{ {
target: "dns://authority/127.0.0.1:50051", target: "dns://authority/127.0.0.1:50051",
wantParsed: resolver.Target{Scheme: "dns", Authority: "authority", Endpoint: "127.0.0.1:50051"}, wantParsed: resolver.Target{Scheme: "dns", Authority: "authority", URL: *testutils.MustParseURL("dns://authority/127.0.0.1:50051")},
wantDialerAddress: "127.0.0.1:50051", wantDialerAddress: "127.0.0.1:50051",
}, },
{ {
target: "://authority/127.0.0.1:50051", target: "://authority/127.0.0.1:50051",
badScheme: true, badScheme: true,
wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "://authority/127.0.0.1:50051"}, wantParsed: resolver.Target{Scheme: defScheme, Authority: "", URL: *testutils.MustParseURL(fmt.Sprintf("%s:///%s", defScheme, "://authority/127.0.0.1:50051"))},
wantDialerAddress: "://authority/127.0.0.1:50051", wantDialerAddress: "://authority/127.0.0.1:50051",
}, },
{ {
target: "/unix/socket/address", target: "/unix/socket/address",
badScheme: true, badScheme: true,
wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: "/unix/socket/address"}, wantParsed: resolver.Target{Scheme: defScheme, Authority: "", URL: *testutils.MustParseURL(fmt.Sprintf("%s:///%s", defScheme, "/unix/socket/address"))},
wantDialerAddress: "/unix/socket/address", wantDialerAddress: "/unix/socket/address",
}, },
{ {
target: "", target: "",
badScheme: true, badScheme: true,
wantParsed: resolver.Target{Scheme: defScheme, Authority: "", Endpoint: ""}, wantParsed: resolver.Target{Scheme: defScheme, Authority: "", URL: *testutils.MustParseURL(fmt.Sprintf("%s:///%s", defScheme, ""))},
wantDialerAddress: "", wantDialerAddress: "",
}, },
{ {
target: "passthrough://a.server.com/google.com", target: "passthrough://a.server.com/google.com",
wantParsed: resolver.Target{Scheme: "passthrough", Authority: "a.server.com", Endpoint: "google.com"}, wantParsed: resolver.Target{Scheme: "passthrough", Authority: "a.server.com", URL: *testutils.MustParseURL("passthrough://a.server.com/google.com")},
wantDialerAddress: "google.com", wantDialerAddress: "google.com",
}, },
} }

2
examples/features/load_balancing/client/main.go
View File

@ -111,7 +111,7 @@ type exampleResolver struct {
} }
func (r *exampleResolver) start() { func (r *exampleResolver) start() {
addrStrs := r.addrsStore[r.target.Endpoint] addrStrs := r.addrsStore[r.target.Endpoint()]
addrs := make([]resolver.Address, len(addrStrs)) addrs := make([]resolver.Address, len(addrStrs))
for i, s := range addrStrs { for i, s := range addrStrs {
addrs[i] = resolver.Address{Addr: s} addrs[i] = resolver.Address{Addr: s}

2
examples/features/name_resolving/client/main.go
View File

@ -119,7 +119,7 @@ type exampleResolver struct {
} }
func (r *exampleResolver) start() { func (r *exampleResolver) start() {
addrStrs := r.addrsStore[r.target.Endpoint] addrStrs := r.addrsStore[r.target.Endpoint()]
addrs := make([]resolver.Address, len(addrStrs)) addrs := make([]resolver.Address, len(addrStrs))
for i, s := range addrStrs { for i, s := range addrStrs {
addrs[i] = resolver.Address{Addr: s} addrs[i] = resolver.Address{Addr: s}

2
internal/resolver/dns/dns_resolver.go
View File

@ -116,7 +116,7 @@ type dnsBuilder struct{}
// Build creates and starts a DNS resolver that watches the name resolution of the target. // Build creates and starts a DNS resolver that watches the name resolution of the target.
func (b *dnsBuilder) Build(target resolver.Target, cc resolver.ClientConn, opts resolver.BuildOptions) (resolver.Resolver, error) { func (b *dnsBuilder) Build(target resolver.Target, cc resolver.ClientConn, opts resolver.BuildOptions) (resolver.Resolver, error) {
host, port, err := parseTarget(target.Endpoint, defaultPort) host, port, err := parseTarget(target.Endpoint(), defaultPort)
if err != nil { if err != nil {
return nil, err return nil, err
} }

31
internal/resolver/dns/dns_resolver_test.go
View File

@ -23,7 +23,6 @@ import (
"errors" "errors"
"fmt" "fmt"
"net" "net"
"net/url"
"os" "os"
"reflect" "reflect"
"strings" "strings"
@ -735,7 +734,7 @@ func testDNSResolver(t *testing.T) {
for _, a := range tests { for _, a := range tests {
b := NewBuilder() b := NewBuilder()
cc := &testClientConn{target: a.target} cc := &testClientConn{target: a.target}
r, err := b.Build(resolver.Target{Endpoint: a.target}, cc, resolver.BuildOptions{}) r, err := b.Build(resolver.Target{URL: *testutils.MustParseURL(fmt.Sprintf("scheme:///%s", a.target))}, cc, resolver.BuildOptions{})
if err != nil { if err != nil {
t.Fatalf("%v\n", err) t.Fatalf("%v\n", err)
} }
@ -807,7 +806,7 @@ func TestDNSResolverExponentialBackoff(t *testing.T) {
cc := &testClientConn{target: test.target} cc := &testClientConn{target: test.target}
// Cause ClientConn to return an error. // Cause ClientConn to return an error.
cc.updateStateErr = balancer.ErrBadResolverState cc.updateStateErr = balancer.ErrBadResolverState
r, err := b.Build(resolver.Target{Endpoint: test.target}, cc, resolver.BuildOptions{}) r, err := b.Build(resolver.Target{URL: *testutils.MustParseURL(fmt.Sprintf("scheme:///%s", test.target))}, cc, resolver.BuildOptions{})
if err != nil { if err != nil {
t.Fatalf("Error building resolver for target %v: %v", test.target, err) t.Fatalf("Error building resolver for target %v: %v", test.target, err)
} }
@ -962,7 +961,7 @@ func testDNSResolverWithSRV(t *testing.T) {
for _, a := range tests { for _, a := range tests {
b := NewBuilder() b := NewBuilder()
cc := &testClientConn{target: a.target} cc := &testClientConn{target: a.target}
r, err := b.Build(resolver.Target{Endpoint: a.target}, cc, resolver.BuildOptions{}) r, err := b.Build(resolver.Target{URL: *testutils.MustParseURL(fmt.Sprintf("scheme:///%s", a.target))}, cc, resolver.BuildOptions{})
if err != nil { if err != nil {
t.Fatalf("%v\n", err) t.Fatalf("%v\n", err)
} }
@ -1046,7 +1045,7 @@ func testDNSResolveNow(t *testing.T) {
for _, a := range tests { for _, a := range tests {
b := NewBuilder() b := NewBuilder()
cc := &testClientConn{target: a.target} cc := &testClientConn{target: a.target}
r, err := b.Build(resolver.Target{Endpoint: a.target}, cc, resolver.BuildOptions{}) r, err := b.Build(resolver.Target{URL: *testutils.MustParseURL(fmt.Sprintf("scheme:///%s", a.target))}, cc, resolver.BuildOptions{})
if err != nil { if err != nil {
t.Fatalf("%v\n", err) t.Fatalf("%v\n", err)
} }
@ -1124,7 +1123,7 @@ func testIPResolver(t *testing.T) {
for _, v := range tests { for _, v := range tests {
b := NewBuilder() b := NewBuilder()
cc := &testClientConn{target: v.target} cc := &testClientConn{target: v.target}
r, err := b.Build(resolver.Target{Endpoint: v.target}, cc, resolver.BuildOptions{}) r, err := b.Build(resolver.Target{URL: *testutils.MustParseURL(fmt.Sprintf("scheme:///%s", v.target))}, cc, resolver.BuildOptions{})
if err != nil { if err != nil {
t.Fatalf("%v\n", err) t.Fatalf("%v\n", err)
} }
@ -1175,7 +1174,7 @@ func TestResolveFunc(t *testing.T) {
{"[2001:db8:a0b:12f0::1]:21", nil}, {"[2001:db8:a0b:12f0::1]:21", nil},
{":80", nil}, {":80", nil},
{"127.0.0...1:12345", nil}, {"127.0.0...1:12345", nil},
{"[fe80::1%lo0]:80", nil}, {"[fe80::1%25lo0]:80", nil},
{"golang.org:http", nil}, {"golang.org:http", nil},
{"[2001:db8::1]:http", nil}, {"[2001:db8::1]:http", nil},
{"[2001:db8::1]:", errEndsWithColon}, {"[2001:db8::1]:", errEndsWithColon},
@ -1187,7 +1186,7 @@ func TestResolveFunc(t *testing.T) {
b := NewBuilder() b := NewBuilder()
for _, v := range tests { for _, v := range tests {
cc := &testClientConn{target: v.addr, errChan: make(chan error, 1)} cc := &testClientConn{target: v.addr, errChan: make(chan error, 1)}
r, err := b.Build(resolver.Target{Endpoint: v.addr}, cc, resolver.BuildOptions{}) r, err := b.Build(resolver.Target{URL: *testutils.MustParseURL(fmt.Sprintf("scheme:///%s", v.addr))}, cc, resolver.BuildOptions{})
if err == nil { if err == nil {
r.Close() r.Close()
} }
@ -1226,7 +1225,7 @@ func TestDisableServiceConfig(t *testing.T) {
for _, a := range tests { for _, a := range tests {
b := NewBuilder() b := NewBuilder()
cc := &testClientConn{target: a.target} cc := &testClientConn{target: a.target}
r, err := b.Build(resolver.Target{Endpoint: a.target}, cc, resolver.BuildOptions{DisableServiceConfig: a.disableServiceConfig}) r, err := b.Build(resolver.Target{URL: *testutils.MustParseURL(fmt.Sprintf("scheme:///%s", a.target))}, cc, resolver.BuildOptions{DisableServiceConfig: a.disableServiceConfig})
if err != nil { if err != nil {
t.Fatalf("%v\n", err) t.Fatalf("%v\n", err)
} }
@ -1264,7 +1263,7 @@ func TestTXTError(t *testing.T) {
envconfig.TXTErrIgnore = ignore envconfig.TXTErrIgnore = ignore
b := NewBuilder() b := NewBuilder()
cc := &testClientConn{target: "ipv4.single.fake"} // has A records but not TXT records. cc := &testClientConn{target: "ipv4.single.fake"} // has A records but not TXT records.
r, err := b.Build(resolver.Target{Endpoint: "ipv4.single.fake"}, cc, resolver.BuildOptions{}) r, err := b.Build(resolver.Target{URL: *testutils.MustParseURL(fmt.Sprintf("scheme:///%s", "ipv4.single.fake"))}, cc, resolver.BuildOptions{})
if err != nil { if err != nil {
t.Fatalf("%v\n", err) t.Fatalf("%v\n", err)
} }
@ -1300,7 +1299,7 @@ func TestDNSResolverRetry(t *testing.T) {
b := NewBuilder() b := NewBuilder()
target := "ipv4.single.fake" target := "ipv4.single.fake"
cc := &testClientConn{target: target} cc := &testClientConn{target: target}
r, err := b.Build(resolver.Target{Endpoint: target}, cc, resolver.BuildOptions{}) r, err := b.Build(resolver.Target{URL: *testutils.MustParseURL(fmt.Sprintf("scheme:///%s", target))}, cc, resolver.BuildOptions{})
if err != nil { if err != nil {
t.Fatalf("%v\n", err) t.Fatalf("%v\n", err)
} }
@ -1438,12 +1437,12 @@ func TestCustomAuthority(t *testing.T) {
} }
} }
mockEndpointTarget := "foo.bar.com"
b := NewBuilder() b := NewBuilder()
cc := &testClientConn{target: "foo.bar.com", errChan: make(chan error, 1)} cc := &testClientConn{target: mockEndpointTarget, errChan: make(chan error, 1)}
target := resolver.Target{ target := resolver.Target{
Endpoint: "foo.bar.com",
Authority: a.authority, Authority: a.authority,
URL: url.URL{Host: a.authority}, URL: *testutils.MustParseURL(fmt.Sprintf("scheme://%s/%s", a.authority, mockEndpointTarget)),
} }
r, err := b.Build(target, cc, resolver.BuildOptions{}) r, err := b.Build(target, cc, resolver.BuildOptions{})
@ -1501,7 +1500,7 @@ func TestRateLimitedResolve(t *testing.T) {
b := NewBuilder() b := NewBuilder()
cc := &testClientConn{target: target} cc := &testClientConn{target: target}
r, err := b.Build(resolver.Target{Endpoint: target}, cc, resolver.BuildOptions{}) r, err := b.Build(resolver.Target{URL: *testutils.MustParseURL(fmt.Sprintf("scheme:///%s", target))}, cc, resolver.BuildOptions{})
if err != nil { if err != nil {
t.Fatalf("resolver.Build() returned error: %v\n", err) t.Fatalf("resolver.Build() returned error: %v\n", err)
} }
@ -1610,7 +1609,7 @@ func TestReportError(t *testing.T) {
cc := &testClientConn{target: target, errChan: make(chan error)} cc := &testClientConn{target: target, errChan: make(chan error)}
totalTimesCalledError := 0 totalTimesCalledError := 0
b := NewBuilder() b := NewBuilder()
r, err := b.Build(resolver.Target{Endpoint: target}, cc, resolver.BuildOptions{}) r, err := b.Build(resolver.Target{URL: *testutils.MustParseURL(fmt.Sprintf("scheme:///%s", target))}, cc, resolver.BuildOptions{})
if err != nil { if err != nil {
t.Fatalf("Error building resolver for target %v: %v", target, err) t.Fatalf("Error building resolver for target %v: %v", target, err)
} }

4
internal/resolver/passthrough/passthrough.go
View File

@ -31,7 +31,7 @@ const scheme = "passthrough"
type passthroughBuilder struct{} type passthroughBuilder struct{}
func (*passthroughBuilder) Build(target resolver.Target, cc resolver.ClientConn, opts resolver.BuildOptions) (resolver.Resolver, error) { func (*passthroughBuilder) Build(target resolver.Target, cc resolver.ClientConn, opts resolver.BuildOptions) (resolver.Resolver, error) {
if target.Endpoint == "" && opts.Dialer == nil { if target.Endpoint() == "" && opts.Dialer == nil {
return nil, errors.New("passthrough: received empty target in Build()") return nil, errors.New("passthrough: received empty target in Build()")
} }
r := &passthroughResolver{ r := &passthroughResolver{
@ -52,7 +52,7 @@ type passthroughResolver struct {
} }
func (r *passthroughResolver) start() { func (r *passthroughResolver) start() {
r.cc.UpdateState(resolver.State{Addresses: []resolver.Address{{Addr: r.target.Endpoint}}}) r.cc.UpdateState(resolver.State{Addresses: []resolver.Address{{Addr: r.target.Endpoint()}}})
} }
func (*passthroughResolver) ResolveNow(o resolver.ResolveNowOptions) {} func (*passthroughResolver) ResolveNow(o resolver.ResolveNowOptions) {}

22
resolver/resolver.go
View File

@ -24,6 +24,7 @@ import (
"context" "context"
"net" "net"
"net/url" "net/url"
"strings"
"google.golang.org/grpc/attributes" "google.golang.org/grpc/attributes"
"google.golang.org/grpc/credentials" "google.golang.org/grpc/credentials"
@ -247,9 +248,6 @@ type Target struct {
Scheme string Scheme string
// Deprecated: use URL.Host instead. // Deprecated: use URL.Host instead.
Authority string Authority string
// Deprecated: use URL.Path or URL.Opaque instead. The latter is set when
// the former is empty.
Endpoint string
// URL contains the parsed dial target with an optional default scheme added // URL contains the parsed dial target with an optional default scheme added
// to it if the original dial target contained no scheme or contained an // to it if the original dial target contained no scheme or contained an
// unregistered scheme. Any query params specified in the original dial // unregistered scheme. Any query params specified in the original dial
@ -257,6 +255,24 @@ type Target struct {
URL url.URL URL url.URL
} }
// Endpoint retrieves endpoint without leading "/" from either `URL.Path`
// or `URL.Opaque`. The latter is used when the former is empty.
func (t Target) Endpoint() string {
endpoint := t.URL.Path
if endpoint == "" {
endpoint = t.URL.Opaque
}
// For targets of the form "[scheme]://[authority]/endpoint, the endpoint
// value returned from url.Parse() contains a leading "/". Although this is
// in accordance with RFC 3986, we do not want to break existing resolver
// implementations which expect the endpoint without the leading "/". So, we
// end up stripping the leading "/" here. But this will result in an
// incorrect parsing for something like "unix:///path/to/socket". Since we
// own the "unix" resolver, we can workaround in the unix resolver by using
// the `URL` field.
return strings.TrimPrefix(endpoint, "/")
}
// Builder creates a resolver that will be used to watch name resolution updates. // Builder creates a resolver that will be used to watch name resolution updates.
type Builder interface { type Builder interface {
// Build creates a new resolver for the given target. // Build creates a new resolver for the given target.

5
xds/internal/balancer/clusterresolver/priority_test.go
View File

@ -62,7 +62,8 @@ func setupTestEDS(t *testing.T, initChild *internalserviceconfig.BalancerConfig)
xdsC := fakeclient.NewClientWithName(testBalancerNameFooBar) xdsC := fakeclient.NewClientWithName(testBalancerNameFooBar)
cc := testutils.NewTestClientConn(t) cc := testutils.NewTestClientConn(t)
builder := balancer.Get(Name) builder := balancer.Get(Name)
edsb := builder.Build(cc, balancer.BuildOptions{Target: resolver.Target{Endpoint: testEDSServcie}}) // TODO: @kylejb will fix typo for 'testEDSServcie' in another PR
edsb := builder.Build(cc, balancer.BuildOptions{Target: resolver.Target{URL: *testutils.MustParseURL("dns:///" + testEDSServcie)}})
if edsb == nil { if edsb == nil {
t.Fatalf("builder.Build(%s) failed and returned nil", Name) t.Fatalf("builder.Build(%s) failed and returned nil", Name)
} }
@ -853,7 +854,7 @@ func (s) TestFallbackToDNS(t *testing.T) {
defer ctxCancel() defer ctxCancel()
select { select {
case target := <-dnsTargetCh: case target := <-dnsTargetCh:
if diff := cmp.Diff(target, resolver.Target{Scheme: "dns", Endpoint: testDNSTarget}); diff != "" { if diff := cmp.Diff(target, resolver.Target{Scheme: "dns", URL: *testutils.MustParseURL("dns:///" + testDNSTarget)}); diff != "" {
t.Fatalf("got unexpected DNS target to watch, diff (-got, +want): %v", diff) t.Fatalf("got unexpected DNS target to watch, diff (-got, +want): %v", diff)
} }
case <-ctx.Done(): case <-ctx.Done():

3
xds/internal/balancer/clusterresolver/resource_resolver_dns.go
View File

@ -21,6 +21,7 @@ package clusterresolver
import ( import (
"fmt" "fmt"
"google.golang.org/grpc/internal/testutils"
"google.golang.org/grpc/resolver" "google.golang.org/grpc/resolver"
"google.golang.org/grpc/serviceconfig" "google.golang.org/grpc/serviceconfig"
) )
@ -50,7 +51,7 @@ func newDNSResolver(target string, topLevelResolver *resourceResolver) *dnsDisco
target: target, target: target,
topLevelResolver: topLevelResolver, topLevelResolver: topLevelResolver,
} }
r, err := newDNS(resolver.Target{Scheme: "dns", Endpoint: target}, ret, resolver.BuildOptions{}) r, err := newDNS(resolver.Target{Scheme: "dns", URL: *testutils.MustParseURL("dns:///" + target)}, ret, resolver.BuildOptions{})
if err != nil { if err != nil {
select { select {
case <-topLevelResolver.updateChannel: case <-topLevelResolver.updateChannel:

17
xds/internal/balancer/clusterresolver/resource_resolver_test.go
View File

@ -24,9 +24,10 @@ import (
"testing" "testing"
"github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp"
"google.golang.org/grpc/internal/testutils"
"google.golang.org/grpc/resolver" "google.golang.org/grpc/resolver"
"google.golang.org/grpc/resolver/manual" "google.golang.org/grpc/resolver/manual"
"google.golang.org/grpc/xds/internal/testutils" xdstestutils "google.golang.org/grpc/xds/internal/testutils"
"google.golang.org/grpc/xds/internal/testutils/fakeclient" "google.golang.org/grpc/xds/internal/testutils/fakeclient"
"google.golang.org/grpc/xds/internal/xdsclient/xdsresource" "google.golang.org/grpc/xds/internal/xdsclient/xdsresource"
) )
@ -40,10 +41,10 @@ var (
) )
func init() { func init() {
clab1 := testutils.NewClusterLoadAssignmentBuilder(testClusterNames[0], nil) clab1 := xdstestutils.NewClusterLoadAssignmentBuilder(testClusterNames[0], nil)
clab1.AddLocality(testSubZones[0], 1, 0, testEndpointAddrs[:1], nil) clab1.AddLocality(testSubZones[0], 1, 0, testEndpointAddrs[:1], nil)
testEDSUpdates = append(testEDSUpdates, parseEDSRespProtoForTesting(clab1.Build())) testEDSUpdates = append(testEDSUpdates, parseEDSRespProtoForTesting(clab1.Build()))
clab2 := testutils.NewClusterLoadAssignmentBuilder(testClusterNames[0], nil) clab2 := xdstestutils.NewClusterLoadAssignmentBuilder(testClusterNames[0], nil)
clab2.AddLocality(testSubZones[1], 1, 0, testEndpointAddrs[1:2], nil) clab2.AddLocality(testSubZones[1], 1, 0, testEndpointAddrs[1:2], nil)
testEDSUpdates = append(testEDSUpdates, parseEDSRespProtoForTesting(clab2.Build())) testEDSUpdates = append(testEDSUpdates, parseEDSRespProtoForTesting(clab2.Build()))
} }
@ -156,7 +157,7 @@ func (s) TestResourceResolverOneDNSResource(t *testing.T) {
{ {
name: "watch DNS", name: "watch DNS",
target: testDNSTarget, target: testDNSTarget,
wantTarget: resolver.Target{Scheme: "dns", Endpoint: testDNSTarget}, wantTarget: resolver.Target{Scheme: "dns", URL: *testutils.MustParseURL("dns:///" + testDNSTarget)},
addrs: []resolver.Address{{Addr: "1.1.1.1"}, {Addr: "2.2.2.2"}}, addrs: []resolver.Address{{Addr: "1.1.1.1"}, {Addr: "2.2.2.2"}},
want: []priorityConfig{{ want: []priorityConfig{{
mechanism: DiscoveryMechanism{ mechanism: DiscoveryMechanism{
@ -614,7 +615,7 @@ func (s) TestResourceResolverEDSAndDNS(t *testing.T) {
} }
select { select {
case target := <-dnsTargetCh: case target := <-dnsTargetCh:
if diff := cmp.Diff(target, resolver.Target{Scheme: "dns", Endpoint: testDNSTarget}); diff != "" { if diff := cmp.Diff(target, resolver.Target{Scheme: "dns", URL: *testutils.MustParseURL("dns:///" + testDNSTarget)}); diff != "" {
t.Fatalf("got unexpected DNS target to watch, diff (-got, +want): %v", diff) t.Fatalf("got unexpected DNS target to watch, diff (-got, +want): %v", diff)
} }
case <-ctx.Done(): case <-ctx.Done():
@ -719,7 +720,7 @@ func (s) TestResourceResolverChangeFromEDSToDNS(t *testing.T) {
}}) }})
select { select {
case target := <-dnsTargetCh: case target := <-dnsTargetCh:
if diff := cmp.Diff(target, resolver.Target{Scheme: "dns", Endpoint: testDNSTarget}); diff != "" { if diff := cmp.Diff(target, resolver.Target{Scheme: "dns", URL: *testutils.MustParseURL("dns:///" + testDNSTarget)}); diff != "" {
t.Fatalf("got unexpected DNS target to watch, diff (-got, +want): %v", diff) t.Fatalf("got unexpected DNS target to watch, diff (-got, +want): %v", diff)
} }
case <-ctx.Done(): case <-ctx.Done():
@ -786,7 +787,7 @@ func (s) TestResourceResolverError(t *testing.T) {
} }
select { select {
case target := <-dnsTargetCh: case target := <-dnsTargetCh:
if diff := cmp.Diff(target, resolver.Target{Scheme: "dns", Endpoint: testDNSTarget}); diff != "" { if diff := cmp.Diff(target, resolver.Target{Scheme: "dns", URL: *testutils.MustParseURL("dns:///" + testDNSTarget)}); diff != "" {
t.Fatalf("got unexpected DNS target to watch, diff (-got, +want): %v", diff) t.Fatalf("got unexpected DNS target to watch, diff (-got, +want): %v", diff)
} }
case <-ctx.Done(): case <-ctx.Done():
@ -847,7 +848,7 @@ func (s) TestResourceResolverDNSResolveNow(t *testing.T) {
defer ctxCancel() defer ctxCancel()
select { select {
case target := <-dnsTargetCh: case target := <-dnsTargetCh:
if diff := cmp.Diff(target, resolver.Target{Scheme: "dns", Endpoint: testDNSTarget}); diff != "" { if diff := cmp.Diff(target, resolver.Target{Scheme: "dns", URL: *testutils.MustParseURL("dns:///" + testDNSTarget)}); diff != "" {
t.Fatalf("got unexpected DNS target to watch, diff (-got, +want): %v", diff) t.Fatalf("got unexpected DNS target to watch, diff (-got, +want): %v", diff)
} }
case <-ctx.Done(): case <-ctx.Done():

18
xds/internal/resolver/xds_resolver_test.go
View File

@ -75,7 +75,7 @@ const (
defaultTestShortTimeout = 100 * time.Microsecond defaultTestShortTimeout = 100 * time.Microsecond
) )
var target = resolver.Target{Endpoint: targetStr, URL: url.URL{Scheme: "xds", Path: "/" + targetStr}} var target = resolver.Target{URL: *testutils.MustParseURL("xds:///" + targetStr)}
var routerFilter = xdsresource.HTTPFilter{Name: "rtr", Filter: httpfilter.Get(router.TypeURL)} var routerFilter = xdsresource.HTTPFilter{Name: "rtr", Filter: httpfilter.Get(router.TypeURL)}
var routerFilterList = []xdsresource.HTTPFilter{routerFilter} var routerFilterList = []xdsresource.HTTPFilter{routerFilter}
@ -907,14 +907,14 @@ func (s) TestResolverRemovedWithRPCs(t *testing.T) {
{ {
"cds_experimental": { "cds_experimental": {
"cluster": "test-cluster-1" "cluster": "test-cluster-1"
} }
} }
] ]
} }
} }
} }
} }
] ]
}`) }`)
if !internal.EqualServiceConfigForTesting(rState.ServiceConfig.Config, wantSCParsed.Config) { if !internal.EqualServiceConfigForTesting(rState.ServiceConfig.Config, wantSCParsed.Config) {
t.Errorf("Received unexpected service config") t.Errorf("Received unexpected service config")