Skip to content

Commit

Permalink
dns: do not call NewServiceConfig when lookups are disabled (#3201)
Browse files Browse the repository at this point in the history
  • Loading branch information
@dfawley
dfawley authored Nov 22, 2019
1 parent da649b3 commit cb47f38
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 100 deletions.
4 changes: 3 additions & 1 deletion internal/resolver/dns/dns_resolver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,9 @@ func (d *dnsResolver) watcher() {
d.retryCount = 0
d.t.Reset(d.freq)
}
d.cc.NewServiceConfig(sc)
if sc != "" { // We get empty string when disabled or the TXT lookup failed.
d.cc.NewServiceConfig(sc)
}
d.cc.NewAddress(result)

// Sleep to prevent excessive re-resolutions. Incoming resolution requests
Expand Down
174 changes: 75 additions & 99 deletions internal/resolver/dns/dns_resolver_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,20 +183,6 @@ func srvLookup(service, proto, name string) (string, []*net.SRV, error) {
return "", nil, fmt.Errorf("failed to lookup srv record for %s in srvLookupTbl", cname)
}

// div divides a byte slice into a slice of strings, each of which is of maximum
// 255 bytes length, which is the length limit per TXT record in DNS.
func div(b []byte) []string {
var r []string
for i := 0; i < len(b); i += txtBytesLimit {
if i+txtBytesLimit > len(b) {
r = append(r, string(b[i:]))
} else {
r = append(r, string(b[i:i+txtBytesLimit]))
}
}
return r
}

// scfs contains an array of service config file string in JSON format.
// Notes about the scfs contents and usage:
// scfs contains 4 service config file JSON strings for testing. Inside each
Expand Down Expand Up @@ -605,65 +591,48 @@ var scs = []string{
}`,
}

// scLookupTbl is a set, which contains targets that have service config. Target
// not in this set should not have service config.
var scLookupTbl = map[string]bool{
txtPrefix + "foo.bar.com": true,
txtPrefix + "srv.ipv4.single.fake": true,
txtPrefix + "srv.ipv4.multi.fake": true,
txtPrefix + "no.attribute": true,
}

// generateSCF generates a slice of strings (aggregately representing a single
// service config file) for the input name, which mocks the result from a real
// DNS TXT record lookup.
func generateSCF(name string) []string {
var b []byte
switch name {
case "foo.bar.com":
b = []byte(scfs[0])
case "srv.ipv4.single.fake":
b = []byte(scfs[1])
case "srv.ipv4.multi.fake":
b = []byte(scfs[2])
default:
b = []byte(scfs[3])
}
if name == "no.attribute" {
return div(b)
}
return div(append([]byte(txtAttribute), b...))
// scLookupTbl is a map, which contains targets that have service config to
// their configs. Targets not in this set should not have service config.
var scLookupTbl = map[string]string{
"foo.bar.com": scs[0],
"srv.ipv4.single.fake": scs[1],
"srv.ipv4.multi.fake": scs[2],
}

// generateSC returns a service config string in JSON format for the input name.
func generateSC(name string) string {
_, cnt := scLookupTbl[name]
if !cnt || name == "no.attribute" {
return ""
}
switch name {
case "foo.bar.com":
return scs[0]
case "srv.ipv4.single.fake":
return scs[1]
case "srv.ipv4.multi.fake":
return scs[2]
default:
return ""
return scLookupTbl[name]
}

// generateSCF generates a slice of strings (aggregately representing a single
// service config file) for the input config string, which mocks the result
// from a real DNS TXT record lookup.
func generateSCF(cfg string) []string {
b := append([]byte(txtAttribute), []byte(cfg)...)

// Split b into multiple strings, each with a max of 255 bytes, which is
// the DNS TXT record limit.
var r []string
for i := 0; i < len(b); i += txtBytesLimit {
if i+txtBytesLimit > len(b) {
r = append(r, string(b[i:]))
} else {
r = append(r, string(b[i:i+txtBytesLimit]))
}
}
return r
}

var txtLookupTbl = struct {
sync.Mutex
tbl map[string][]string
}{
tbl: map[string][]string{
"foo.bar.com": generateSCF("foo.bar.com"),
"srv.ipv4.single.fake": generateSCF("srv.ipv4.single.fake"),
"srv.ipv4.multi.fake": generateSCF("srv.ipv4.multi.fake"),
"srv.ipv6.single.fake": generateSCF("srv.ipv6.single.fake"),
"srv.ipv6.multi.fake": generateSCF("srv.ipv6.multi.fake"),
"no.attribute": generateSCF("no.attribute"),
txtPrefix + "foo.bar.com": generateSCF(scfs[0]),
txtPrefix + "srv.ipv4.single.fake": generateSCF(scfs[1]),
txtPrefix + "srv.ipv4.multi.fake": generateSCF(scfs[2]),
txtPrefix + "srv.ipv6.single.fake": generateSCF(scfs[3]),
txtPrefix + "srv.ipv6.multi.fake": generateSCF(scfs[3]),
},
}

Expand Down Expand Up @@ -720,11 +689,6 @@ func testDNSResolver(t *testing.T) {
nil,
generateSC("srv.ipv6.multi.fake"),
},
{
"no.attribute",
nil,
generateSC("no.attribute"),
},
}

for _, a := range tests {
Expand All @@ -744,12 +708,18 @@ func testDNSResolver(t *testing.T) {
time.Sleep(time.Millisecond)
}
var sc string
for {
sc, cnt = cc.getSc()
if cnt > 0 {
break
if a.scWant != "" {
for {
sc, cnt = cc.getSc()
if cnt > 0 {
break
}
time.Sleep(time.Millisecond)
}
time.Sleep(time.Millisecond)
} else {
// A new service config should never be produced; call getSc once
// just in case.
sc, _ = cc.getSc()
}
if !reflect.DeepEqual(a.addrWant, addrs) {
t.Errorf("Resolved addresses of target: %q = %+v, want %+v\n", a.target, addrs, a.addrWant)
Expand Down Expand Up @@ -810,11 +780,6 @@ func testDNSResolverWithSRV(t *testing.T) {
},
generateSC("srv.ipv6.multi.fake"),
},
{
"no.attribute",
nil,
generateSC("no.attribute"),
},
}

for _, a := range tests {
Expand All @@ -834,12 +799,18 @@ func testDNSResolverWithSRV(t *testing.T) {
time.Sleep(time.Millisecond)
}
var sc string
for {
sc, cnt = cc.getSc()
if cnt > 0 {
break
if a.scWant != "" {
for {
sc, cnt = cc.getSc()
if cnt > 0 {
break
}
time.Sleep(time.Millisecond)
}
time.Sleep(time.Millisecond)
} else {
// A new service config should never be produced; call getSc once
// just in case.
sc, _ = cc.getSc()
}
if !reflect.DeepEqual(a.addrWant, addrs) {
t.Errorf("Resolved addresses of target: %q = %+v, want %+v\n", a.target, addrs, a.addrWant)
Expand All @@ -857,16 +828,16 @@ func mutateTbl(target string) func() {
hostLookupTbl.tbl[target] = hostLookupTbl.tbl[target][:len(oldHostTblEntry)-1]
hostLookupTbl.Unlock()
txtLookupTbl.Lock()
oldTxtTblEntry := txtLookupTbl.tbl[target]
txtLookupTbl.tbl[target] = []string{""}
oldTxtTblEntry := txtLookupTbl.tbl[txtPrefix+target]
txtLookupTbl.tbl[txtPrefix+target] = []string{txtAttribute + `[{"serviceConfig":{"loadBalancingPolicy": "grpclb"}}]`}
txtLookupTbl.Unlock()

return func() {
hostLookupTbl.Lock()
hostLookupTbl.tbl[target] = oldHostTblEntry
hostLookupTbl.Unlock()
txtLookupTbl.Lock()
txtLookupTbl.tbl[target] = oldTxtTblEntry
txtLookupTbl.tbl[txtPrefix+target] = oldTxtTblEntry
txtLookupTbl.Unlock()
}
}
Expand All @@ -885,7 +856,7 @@ func testDNSResolveNow(t *testing.T) {
[]resolver.Address{{Addr: "1.2.3.4" + colonDefaultPort}, {Addr: "5.6.7.8" + colonDefaultPort}},
[]resolver.Address{{Addr: "1.2.3.4" + colonDefaultPort}},
generateSC("foo.bar.com"),
"",
`{"loadBalancingPolicy": "grpclb"}`,
},
}

Expand Down Expand Up @@ -921,16 +892,18 @@ func testDNSResolveNow(t *testing.T) {
}
revertTbl := mutateTbl(a.target)
r.ResolveNow(resolver.ResolveNowOptions{})
for {
for i := 0; i < 1000; i++ {
addrs, cnt = cc.getAddress()
if cnt == 2 {
// Break if the address list changes or enough redundant updates happen.
if !reflect.DeepEqual(addrs, a.addrWant) || cnt > 10 {
break
}
time.Sleep(time.Millisecond)
}
for {
for i := 0; i < 1000; i++ {
sc, cnt = cc.getSc()
if cnt == 2 {
// Break if the service config changes or enough redundant updates happen.
if !reflect.DeepEqual(sc, a.scWant) || cnt > 10 {
break
}
time.Sleep(time.Millisecond)
Expand Down Expand Up @@ -1066,13 +1039,20 @@ func TestDisableServiceConfig(t *testing.T) {
}
var cnt int
var sc string
for {
sc, cnt = cc.getSc()
// First wait for addresses. We know service configs are reported
// first, so once addresses have been reported, we can then check to
// see whether any configs have been reported..
for i := 0; i < 1000; i++ {
_, cnt = cc.getAddress()
if cnt > 0 {
break
}
time.Sleep(time.Millisecond)
}
sc, cnt = cc.getSc()
if a.disableServiceConfig && cnt > 0 {
t.Errorf("Resolver reported a service config even though lookups are disabled: sc=%v, cnt=%v", sc, cnt)
}
if !reflect.DeepEqual(a.scWant, sc) {
t.Errorf("Resolved service config of target: %q = %+v, want %+v\n", a.target, sc, a.scWant)
}
Expand Down Expand Up @@ -1114,22 +1094,18 @@ func TestDNSResolverRetry(t *testing.T) {
revertTbl()
// wait for the retry to happen in two seconds.
timer := time.NewTimer(2 * time.Second)
loop:
for {
b := false
select {
case <-timer.C:
b = true
break loop
default:
addrs, _ = cc.getAddress()
if len(addrs) == 1 {
b = true
break
if len(addrs) != 0 {
break loop
}
time.Sleep(time.Millisecond)
}
if b {
break
}
}
if !reflect.DeepEqual(want, addrs) {
t.Errorf("Resolved addresses of target: %q = %+v, want %+v\n", target, addrs, want)
Expand Down
4 changes: 4 additions & 0 deletions resolver_conn_wrapper.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,10 @@ func (ccr *ccResolverWrapper) NewServiceConfig(sc string) {
return
}
grpclog.Infof("ccResolverWrapper: got new service config: %v", sc)
if ccr.cc.dopts.disableServiceConfig {
grpclog.Infof("Service config lookups disabled; ignoring config")
return
}
scpr := parseServiceConfig(sc)
if scpr.Err != nil {
grpclog.Warningf("ccResolverWrapper: error parsing service config: %v", scpr.Err)
Expand Down

0 comments on commit cb47f38

Please sign in to comment.