Skip to content

Commit

Permalink
fix review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
lyuxuan committed Sep 21, 2017
1 parent 0af96da commit ef970f7
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 11 deletions.
26 changes: 16 additions & 10 deletions resolver/dns/dns_resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,24 @@ import (
"net"
"os"
"strconv"
"strings"
"time"

"golang.org/x/net/context"
"google.golang.org/grpc/grpclog"
"google.golang.org/grpc/resolver"
)

func init() {
resolver.Register(NewBuilder())
}

const (
defaultPort = "443"
defaultFreq = time.Minute * 30
golang = "GO"
defaultPort = "443"
defaultFreq = time.Minute * 30
golang = "GO"
// In DNS, service config is encoded in a TXT record via the mechanism
// described in RFC-1464 using the attribute name grpc_config.
txtAttribute = "grpc_config="
)

Expand Down Expand Up @@ -165,7 +172,7 @@ func (d *dnsResolver) watcher() {
case <-d.rn:
}
result, sc := d.lookup()
// Next lookup should happen after an interval defined by d.b.freq.
// Next lookup should happen after an interval defined by d.freq.
d.t.Reset(d.freq)
d.cc.NewServiceConfig(string(sc))
d.cc.NewAddress(result)
Expand Down Expand Up @@ -210,11 +217,11 @@ func (d *dnsResolver) lookupTXT() string {
}

// TXT record must have "grpc_config=" attribute in order to be used as service config.
if len(res) < len(txtAttribute) || res[:len(txtAttribute)] != txtAttribute {
if !strings.HasPrefix(res, txtAttribute) {
grpclog.Warningf("grpc: TXT record %p missing %p attribute", res, txtAttribute)
return ""
}
return res[len(txtAttribute):]
return strings.TrimPrefix(res, txtAttribute)
}

func (d *dnsResolver) lookupHost() []resolver.Address {
Expand Down Expand Up @@ -273,12 +280,11 @@ func parseTarget(target string) (host, port string, err error) {
if target == "" {
return "", "", errMissingAddr
}

if ip := net.ParseIP(target); ip != nil {
// target is an IPv4 or IPv6(without brackets) address
return target, defaultPort, nil
}
if host, port, err := net.SplitHostPort(target); err == nil {
if host, port, err = net.SplitHostPort(target); err == nil {
// target has port, i.e ipv4-host:port, [ipv6-host]:port, host-name:port
if host == "" {
// Keep consistent with net.Dial(): If the host is empty, as in ":80", the local system is assumed.
Expand All @@ -290,11 +296,11 @@ func parseTarget(target string) (host, port string, err error) {
}
return host, port, nil
}
if host, port, err := net.SplitHostPort(target + ":" + defaultPort); err == nil {
if host, port, err = net.SplitHostPort(target + ":" + defaultPort); err == nil {
// target doesn't have port
return host, port, nil
}
return "", "", fmt.Errorf("invalid target address %v", target)
return "", "", fmt.Errorf("invalid target address %v, error info: %v", target, err)
}

type rawChoice struct {
Expand Down
2 changes: 1 addition & 1 deletion resolver/dns/dns_resolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,7 @@ func TestResolveFunc(t *testing.T) {
{"[2001:db8::1]:http", nil},
{":", nil},
{"", errMissingAddr},
{"[2001:db8:a0b:12f0::1", fmt.Errorf("invalid target address %v", "[2001:db8:a0b:12f0::1")},
{"[2001:db8:a0b:12f0::1", fmt.Errorf("invalid target address [2001:db8:a0b:12f0::1, error info: address [2001:db8:a0b:12f0::1:443: missing ']' in address")},
}

b := NewBuilder()
Expand Down

0 comments on commit ef970f7

Please sign in to comment.