From b91517cd568fea519f7bce5afb5d23ee285ea386 Mon Sep 17 00:00:00 2001
From: Doug Fawley <dfawley@google.com>
Date: Tue, 7 Jan 2020 13:08:22 -0800
Subject: [PATCH] dns: ignore TXT errors unless GRPC_GO_IGNORE_TXT_ERRORS=false
 (#3299)

---
 internal/envconfig/envconfig.go            |  7 ++--
 internal/resolver/dns/dns_resolver.go      |  4 +++
 internal/resolver/dns/dns_resolver_test.go | 39 +++++++++++++++++++++-
 3 files changed, 47 insertions(+), 3 deletions(-)

diff --git a/internal/envconfig/envconfig.go b/internal/envconfig/envconfig.go
index 3ee8740f1..ae6c8972f 100644
--- a/internal/envconfig/envconfig.go
+++ b/internal/envconfig/envconfig.go
@@ -25,11 +25,14 @@ import (
 )
 
 const (
-	prefix   = "GRPC_GO_"
-	retryStr = prefix + "RETRY"
+	prefix          = "GRPC_GO_"
+	retryStr        = prefix + "RETRY"
+	txtErrIgnoreStr = prefix + "IGNORE_TXT_ERRORS"
 )
 
 var (
 	// Retry is set if retry is explicitly enabled via "GRPC_GO_RETRY=on".
 	Retry = strings.EqualFold(os.Getenv(retryStr), "on")
+	// TXTErrIgnore is set if TXT errors should be ignored ("GRPC_GO_IGNORE_TXT_ERRORS" is not "false").
+	TXTErrIgnore = !strings.EqualFold(os.Getenv(retryStr), "false")
 )
diff --git a/internal/resolver/dns/dns_resolver.go b/internal/resolver/dns/dns_resolver.go
index 1505754b0..c368db62e 100644
--- a/internal/resolver/dns/dns_resolver.go
+++ b/internal/resolver/dns/dns_resolver.go
@@ -33,6 +33,7 @@ import (
 	"time"
 
 	"google.golang.org/grpc/grpclog"
+	"google.golang.org/grpc/internal/envconfig"
 	"google.golang.org/grpc/internal/grpcrand"
 	"google.golang.org/grpc/resolver"
 	"google.golang.org/grpc/serviceconfig"
@@ -278,6 +279,9 @@ func handleDNSError(err error, lookupType string) error {
 func (d *dnsResolver) lookupTXT() *serviceconfig.ParseResult {
 	ss, err := d.resolver.LookupTXT(d.ctx, txtPrefix+d.host)
 	if err != nil {
+		if envconfig.TXTErrIgnore {
+			return nil
+		}
 		if err = handleDNSError(err, "TXT"); err != nil {
 			return &serviceconfig.ParseResult{Err: err}
 		}
diff --git a/internal/resolver/dns/dns_resolver_test.go b/internal/resolver/dns/dns_resolver_test.go
index aeb16de60..b7b39a7f6 100644
--- a/internal/resolver/dns/dns_resolver_test.go
+++ b/internal/resolver/dns/dns_resolver_test.go
@@ -30,6 +30,7 @@ import (
 	"testing"
 	"time"
 
+	"google.golang.org/grpc/internal/envconfig"
 	"google.golang.org/grpc/internal/leakcheck"
 	"google.golang.org/grpc/resolver"
 	"google.golang.org/grpc/serviceconfig"
@@ -830,7 +831,11 @@ func mutateTbl(target string) func() {
 		hostLookupTbl.tbl[target] = oldHostTblEntry
 		hostLookupTbl.Unlock()
 		txtLookupTbl.Lock()
-		txtLookupTbl.tbl[txtPrefix+target] = oldTxtTblEntry
+		if len(oldTxtTblEntry) == 0 {
+			delete(txtLookupTbl.tbl, txtPrefix+target)
+		} else {
+			txtLookupTbl.tbl[txtPrefix+target] = oldTxtTblEntry
+		}
 		txtLookupTbl.Unlock()
 	}
 }
@@ -1039,6 +1044,38 @@ func TestDisableServiceConfig(t *testing.T) {
 	}
 }
 
+func TestTXTError(t *testing.T) {
+	defer leakcheck.Check(t)
+	defer func(v bool) { envconfig.TXTErrIgnore = v }(envconfig.TXTErrIgnore)
+	for _, ignore := range []bool{false, true} {
+		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{})
+		if err != nil {
+			t.Fatalf("%v\n", err)
+		}
+		defer r.Close()
+		var cnt int
+		var state resolver.State
+		for i := 0; i < 2000; i++ {
+			state, cnt = cc.getState()
+			if cnt > 0 {
+				break
+			}
+			time.Sleep(time.Millisecond)
+		}
+		if cnt == 0 {
+			t.Fatalf("UpdateState not called after 2s; aborting")
+		}
+		if !ignore && (state.ServiceConfig == nil || state.ServiceConfig.Err == nil) {
+			t.Errorf("state.ServiceConfig = %v; want non-nil error", state.ServiceConfig)
+		} else if ignore && state.ServiceConfig != nil {
+			t.Errorf("state.ServiceConfig = %v; want nil", state.ServiceConfig)
+		}
+	}
+}
+
 func TestDNSResolverRetry(t *testing.T) {
 	b := NewBuilder()
 	target := "ipv4.single.fake"