Skip to content

Commit

Permalink
cleanup: usages of resolver.Target.Endpoint
Browse files Browse the repository at this point in the history
* fix(internal/dns_resolver_test): remove reliance on Target.Endpoint

* feat(internal/testutils): helper function for dns_resolver_test
  • Loading branch information
kylejb committed Dec 9, 2022
1 parent a9709c3 commit 00aad9b
Show file tree
Hide file tree
Showing 10 changed files with 83 additions and 24 deletions.
4 changes: 2 additions & 2 deletions balancer/grpclb/grpclb.go
Expand Up @@ -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{}),
Expand Down
2 changes: 1 addition & 1 deletion balancer/rls/balancer.go
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion clientconn.go
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion examples/features/load_balancing/client/main.go
Expand Up @@ -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}
Expand Down
2 changes: 1 addition & 1 deletion examples/features/name_resolving/client/main.go
Expand Up @@ -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}
Expand Down
2 changes: 1 addition & 1 deletion internal/resolver/dns/dns_resolver.go
Expand Up @@ -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
}
Expand Down
26 changes: 13 additions & 13 deletions internal/resolver/dns/dns_resolver_test.go
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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},
Expand All @@ -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()
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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{})

Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down
4 changes: 2 additions & 2 deletions internal/resolver/passthrough/passthrough.go
Expand Up @@ -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{
Expand All @@ -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) {}
Expand Down
50 changes: 50 additions & 0 deletions 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,
}
}
13 changes: 11 additions & 2 deletions resolver/resolver.go
Expand Up @@ -24,6 +24,7 @@ import (
"context"
"net"
"net/url"
"strings"

"google.golang.org/grpc/attributes"
"google.golang.org/grpc/credentials"
Expand Down Expand Up @@ -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
Expand All @@ -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.
Expand Down

0 comments on commit 00aad9b

Please sign in to comment.