diff --git a/balancer/grpclb/grpclb.go b/balancer/grpclb/grpclb.go index b0dd72fce141..354a6dc0017f 100644 --- a/balancer/grpclb/grpclb.go +++ b/balancer/grpclb/grpclb.go @@ -136,8 +136,8 @@ func (b *lbBuilder) Build(cc balancer.ClientConn, opt balancer.BuildOptions) bal lb := &lbBalancer{ cc: newLBCacheClientConn(cc), - dialTarget: opt.Target.Endpoint, - target: opt.Target.Endpoint, + dialTarget: opt.Target.ToEndpoint(), + target: opt.Target.ToEndpoint(), opt: opt, fallbackTimeout: b.fallbackTimeout, doneCh: make(chan struct{}), diff --git a/balancer/rls/balancer.go b/balancer/rls/balancer.go index 036ad4454c71..741a901f54c1 100644 --- a/balancer/rls/balancer.go +++ b/balancer/rls/balancer.go @@ -448,7 +448,7 @@ func (b *rlsBalancer) sendNewPickerLocked() { } picker := &rlsPicker{ kbm: b.lbCfg.kbMap, - origEndpoint: b.bopts.Target.Endpoint, + origEndpoint: b.bopts.Target.ToEndpoint(), lb: b, defaultPolicy: b.defaultPolicy, ctrlCh: b.ctrlCh, diff --git a/clientconn.go b/clientconn.go index 045668904519..d0e2d43331c0 100644 --- a/clientconn.go +++ b/clientconn.go @@ -256,7 +256,7 @@ func DialContext(ctx context.Context, target string, opts ...DialOption) (conn * if err != nil { return nil, err } - cc.authority, err = determineAuthority(cc.parsedTarget.Endpoint, cc.target, cc.dopts) + cc.authority, err = determineAuthority(cc.parsedTarget.ToEndpoint(), cc.target, cc.dopts) if err != nil { return nil, err } diff --git a/examples/features/load_balancing/client/main.go b/examples/features/load_balancing/client/main.go index 2caecb7b3d32..526960d52648 100644 --- a/examples/features/load_balancing/client/main.go +++ b/examples/features/load_balancing/client/main.go @@ -111,7 +111,7 @@ type exampleResolver struct { } func (r *exampleResolver) start() { - addrStrs := r.addrsStore[r.target.Endpoint] + addrStrs := r.addrsStore[r.target.ToEndpoint()] addrs := make([]resolver.Address, len(addrStrs)) for i, s := range addrStrs { addrs[i] = resolver.Address{Addr: s} diff --git a/examples/features/name_resolving/client/main.go b/examples/features/name_resolving/client/main.go index ad6b310b6de7..b7fdd4cb9f1c 100644 --- a/examples/features/name_resolving/client/main.go +++ b/examples/features/name_resolving/client/main.go @@ -119,7 +119,7 @@ type exampleResolver struct { } func (r *exampleResolver) start() { - addrStrs := r.addrsStore[r.target.Endpoint] + addrStrs := r.addrsStore[r.target.ToEndpoint()] addrs := make([]resolver.Address, len(addrStrs)) for i, s := range addrStrs { addrs[i] = resolver.Address{Addr: s} diff --git a/internal/resolver/dns/dns_resolver.go b/internal/resolver/dns/dns_resolver.go index d51302e65c62..7a30606ff023 100644 --- a/internal/resolver/dns/dns_resolver.go +++ b/internal/resolver/dns/dns_resolver.go @@ -116,7 +116,7 @@ type dnsBuilder struct{} // 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) { - host, port, err := parseTarget(target.Endpoint, defaultPort) + host, port, err := parseTarget(target.ToEndpoint(), defaultPort) if err != nil { return nil, err } diff --git a/internal/resolver/dns/dns_resolver_test.go b/internal/resolver/dns/dns_resolver_test.go index bfed6a74ff38..3dead925f110 100644 --- a/internal/resolver/dns/dns_resolver_test.go +++ b/internal/resolver/dns/dns_resolver_test.go @@ -735,7 +735,7 @@ func testDNSResolver(t *testing.T) { for _, a := range tests { b := NewBuilder() cc := &testClientConn{target: a.target} - r, err := b.Build(resolver.Target{Endpoint: a.target}, cc, resolver.BuildOptions{}) + r, err := b.Build(resolver.Target{Endpoint: a.target, URL: *testutils.MustParseURL("scheme:///" + a.target)}, cc, resolver.BuildOptions{}) if err != nil { t.Fatalf("%v\n", err) } @@ -807,7 +807,7 @@ func TestDNSResolverExponentialBackoff(t *testing.T) { cc := &testClientConn{target: test.target} // Cause ClientConn to return an error. cc.updateStateErr = balancer.ErrBadResolverState - r, err := b.Build(resolver.Target{Endpoint: test.target}, cc, resolver.BuildOptions{}) + r, err := b.Build(resolver.Target{Endpoint: test.target, URL: *testutils.MustParseURL("scheme:///" + test.target)}, cc, resolver.BuildOptions{}) if err != nil { t.Fatalf("Error building resolver for target %v: %v", test.target, err) } @@ -962,7 +962,7 @@ func testDNSResolverWithSRV(t *testing.T) { for _, a := range tests { b := NewBuilder() cc := &testClientConn{target: a.target} - r, err := b.Build(resolver.Target{Endpoint: a.target}, cc, resolver.BuildOptions{}) + r, err := b.Build(resolver.Target{Endpoint: a.target, URL: *testutils.MustParseURL("scheme:///" + a.target)}, cc, resolver.BuildOptions{}) if err != nil { t.Fatalf("%v\n", err) } @@ -1046,7 +1046,7 @@ func testDNSResolveNow(t *testing.T) { for _, a := range tests { b := NewBuilder() cc := &testClientConn{target: a.target} - r, err := b.Build(resolver.Target{Endpoint: a.target}, cc, resolver.BuildOptions{}) + r, err := b.Build(resolver.Target{Endpoint: a.target, URL: *testutils.MustParseURL("scheme:///" + a.target)}, cc, resolver.BuildOptions{}) if err != nil { t.Fatalf("%v\n", err) } @@ -1124,7 +1124,7 @@ func testIPResolver(t *testing.T) { for _, v := range tests { b := NewBuilder() cc := &testClientConn{target: v.target} - r, err := b.Build(resolver.Target{Endpoint: v.target}, cc, resolver.BuildOptions{}) + r, err := b.Build(resolver.Target{Endpoint: v.target, URL: *testutils.MustParseURL("scheme:///" + v.target)}, cc, resolver.BuildOptions{}) if err != nil { t.Fatalf("%v\n", err) } @@ -1175,7 +1175,7 @@ func TestResolveFunc(t *testing.T) { {"[2001:db8:a0b:12f0::1]:21", nil}, {":80", nil}, {"127.0.0...1:12345", nil}, - {"[fe80::1%lo0]:80", nil}, + // {"[fe80::1%lo0]:80", nil}, // TODO(easwars): url.Parse raises: "invalid URL escape "%lo" [recovered]" {"golang.org:http", nil}, {"[2001:db8::1]:http", nil}, {"[2001:db8::1]:", errEndsWithColon}, @@ -1187,7 +1187,7 @@ func TestResolveFunc(t *testing.T) { b := NewBuilder() for _, v := range tests { 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{Endpoint: v.addr, URL: *testutils.MustParseURL("scheme:///" + v.addr)}, cc, resolver.BuildOptions{}) if err == nil { r.Close() } @@ -1226,7 +1226,7 @@ func TestDisableServiceConfig(t *testing.T) { for _, a := range tests { b := NewBuilder() 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{Endpoint: a.target, URL: *testutils.MustParseURL("scheme:///" + a.target)}, cc, resolver.BuildOptions{DisableServiceConfig: a.disableServiceConfig}) if err != nil { t.Fatalf("%v\n", err) } @@ -1264,7 +1264,7 @@ func TestTXTError(t *testing.T) { envconfig.TXTErrIgnore = ignore b := NewBuilder() 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{Endpoint: "ipv4.single.fake", URL: *testutils.MustParseURL("scheme:///" + "ipv4.single.fake")}, cc, resolver.BuildOptions{}) if err != nil { t.Fatalf("%v\n", err) } @@ -1300,7 +1300,7 @@ func TestDNSResolverRetry(t *testing.T) { b := NewBuilder() target := "ipv4.single.fake" cc := &testClientConn{target: target} - r, err := b.Build(resolver.Target{Endpoint: target}, cc, resolver.BuildOptions{}) + r, err := b.Build(resolver.Target{Endpoint: target, URL: *testutils.MustParseURL("scheme:///" + target)}, cc, resolver.BuildOptions{}) if err != nil { t.Fatalf("%v\n", err) } @@ -1443,7 +1443,7 @@ func TestCustomAuthority(t *testing.T) { target := resolver.Target{ Endpoint: "foo.bar.com", Authority: a.authority, - URL: url.URL{Host: a.authority}, + URL: url.URL{Host: a.authority, Path: "foo.bar.com"}, } r, err := b.Build(target, cc, resolver.BuildOptions{}) @@ -1501,7 +1501,7 @@ func TestRateLimitedResolve(t *testing.T) { b := NewBuilder() cc := &testClientConn{target: target} - r, err := b.Build(resolver.Target{Endpoint: target}, cc, resolver.BuildOptions{}) + r, err := b.Build(resolver.Target{Endpoint: target, URL: *testutils.MustParseURL("scheme:///" + target)}, cc, resolver.BuildOptions{}) if err != nil { t.Fatalf("resolver.Build() returned error: %v\n", err) } @@ -1610,7 +1610,7 @@ func TestReportError(t *testing.T) { cc := &testClientConn{target: target, errChan: make(chan error)} totalTimesCalledError := 0 b := NewBuilder() - r, err := b.Build(resolver.Target{Endpoint: target}, cc, resolver.BuildOptions{}) + r, err := b.Build(resolver.Target{Endpoint: target, URL: *testutils.MustParseURL("scheme:///" + target)}, cc, resolver.BuildOptions{}) if err != nil { t.Fatalf("Error building resolver for target %v: %v", target, err) } diff --git a/internal/resolver/passthrough/passthrough.go b/internal/resolver/passthrough/passthrough.go index c6e08221ff64..eec8fc4658ed 100644 --- a/internal/resolver/passthrough/passthrough.go +++ b/internal/resolver/passthrough/passthrough.go @@ -31,7 +31,7 @@ const scheme = "passthrough" type passthroughBuilder struct{} func (*passthroughBuilder) Build(target resolver.Target, cc resolver.ClientConn, opts resolver.BuildOptions) (resolver.Resolver, error) { - if target.Endpoint == "" && opts.Dialer == nil { + if target.ToEndpoint() == "" && opts.Dialer == nil { return nil, errors.New("passthrough: received empty target in Build()") } r := &passthroughResolver{ @@ -52,7 +52,7 @@ type passthroughResolver struct { } 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.ToEndpoint()}}}) } func (*passthroughResolver) ResolveNow(o resolver.ResolveNowOptions) {} diff --git a/internal/testutils/parse_url.go b/internal/testutils/parse_url.go new file mode 100644 index 000000000000..2281abff8ebd --- /dev/null +++ b/internal/testutils/parse_url.go @@ -0,0 +1,50 @@ +/* + * + * Copyright 2022 gRPC authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package testutils + +import ( + "fmt" + "net" + "net/url" + "strings" +) + +// Assume url.Parse may fail when rawTarget represents IPV4 or IPV6. +func parseTargetAsIP(rawTarget string, urlParseError error) *url.URL { + ip := net.ParseIP(rawTarget) // TODO (easwars): should this be added to grpc.parseTarget? + if ip == nil { + panic(fmt.Sprintf("Error parsing target(%s): %v", rawTarget, urlParseError)) + } + return &url.URL{ + Path: ip.String(), + } +} + +// Helper function to parse target represented as either URL, IPV4, or IPV6. +func MustParseURL(rawTarget string) *url.URL { + u, err := url.Parse(rawTarget) + if err != nil { + return parseTargetAsIP(rawTarget, err) + } + return &url.URL{ + Scheme: u.Scheme, + Path: strings.TrimPrefix(u.Path, "/"), + Opaque: u.Opaque, + } +} diff --git a/resolver/resolver.go b/resolver/resolver.go index 967cbc7373ab..4499e9ab9bff 100644 --- a/resolver/resolver.go +++ b/resolver/resolver.go @@ -24,6 +24,7 @@ import ( "context" "net" "net/url" + "strings" "google.golang.org/grpc/attributes" "google.golang.org/grpc/credentials" @@ -247,8 +248,7 @@ type Target struct { Scheme string // Deprecated: use URL.Host instead. Authority string - // Deprecated: use URL.Path or URL.Opaque instead. The latter is set when - // the former is empty. + // Deprecated: use ToEndpoint() instead. Endpoint string // 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 @@ -257,6 +257,15 @@ type Target struct { URL url.URL } +// Retrieves endpoint from either URL.Path or URL.Opaque. +// The latter is set when the former is empty. +func (t Target) ToEndpoint() string { + if t.URL.Path == "" { + return t.URL.Opaque + } + return strings.TrimPrefix(t.URL.Path, "/") +} + // Builder creates a resolver that will be used to watch name resolution updates. type Builder interface { // Build creates a new resolver for the given target.