Fix dns provider tets and flakiness

hashicorp · Aug 17, 2022 · 297a5d6 · 297a5d6
1 parent abc8544
commit 297a5d6
Show file tree

Hide file tree

Showing 2 changed files with 59 additions and 27 deletions.
diff --git a/provider/dns/dns_provider.go b/provider/dns/dns_provider.go
@@ -3,7 +3,7 @@ package dns
 
 import (
 	"fmt"
-	"io/ioutil"
+	"io"
 	"log"
 	"net"
 	"strconv"
@@ -33,13 +33,12 @@ func (p *Provider) Addrs(args map[string]string, l *log.Logger) ([]string, error
 
 	// default to null logger
 	if l == nil {
-		l = log.New(ioutil.Discard, "", 0)
+		l = log.New(io.Discard, "", 0)
 	}
 
 	// validate and set service record
 	if args["query"] == "" {
-		return nil, fmt.Errorf("discover-dns: Query not provided." +
-			"  Please specify a valid dns query for the DNS lookup.")
+		return nil, fmt.Errorf("discover-dns: Query not provided")
 	}
 
 	var server string
@@ -77,7 +76,11 @@ func (p *Provider) Addrs(args map[string]string, l *log.Logger) ([]string, error
 	m1.Id = dns.Id()
 	m1.RecursionDesired = true
 	m1.Question = make([]dns.Question, 1)
-	m1.Question[0] = dns.Question{args["query"], dns.TypeA, dns.ClassINET}
+	m1.Question[0] = dns.Question{
+		Name:   args["query"],
+		Qtype:  dns.TypeA,
+		Qclass: dns.ClassINET,
+	}
 
 	c := new(dns.Client)
 
@@ -93,7 +96,7 @@ func (p *Provider) Addrs(args map[string]string, l *log.Logger) ([]string, error
 	raddr := fmt.Sprintf("%s:%d", server, port)
 	in, _, err := c.Exchange(m1, raddr)
 	if err != nil {
-		return nil, fmt.Errorf("discover-mdns: Failed to process query: %s", err)
+		return nil, fmt.Errorf("discover-dns: Failed to process query: %s", err)
 	}
 
 	for _, answer := range in.Answer {

diff --git a/provider/dns/dns_provider_test.go b/provider/dns/dns_provider_test.go
@@ -3,8 +3,10 @@ package dns_test
 import (
 	"fmt"
 	"log"
+	"net"
 	"os"
 	"testing"
+	"time"
 
 	"github.com/miekg/dns"
 
@@ -20,27 +22,54 @@ var (
 	testAddresses = []string{"127.0.0.1"}
 )
 
-func newTestServer() (*dns.Server, error) {
+func pickUnusedUDPPort() (int, error) {
+	addr, err := net.ResolveUDPAddr("udp", ":0")
+	if err != nil {
+		return 0, err
+	}
+
+	l, err := net.ListenUDP("udp", addr)
+	if err != nil {
+		return 0, err
+	}
+	port := l.LocalAddr().(*net.UDPAddr).Port
+	if err := l.Close(); err != nil {
+		return 0, err
+	}
+	return port, nil
+}
+
+func newTestServer(port int) (*dns.Server, error) {
 	var rr []dns.RR
 
 	for _, testAddress := range testAddresses {
 		a, _ := dns.NewRR(fmt.Sprintf("tasks.%s. IN A %s", testService, testAddress))
 		rr = append(rr, a)
 	}
 
-	server := &dns.Server{Addr: ":5300", Net: "udp"}
+	server := &dns.Server{Addr: fmt.Sprintf(":%d", port), Net: "udp"}
 	go server.ListenAndServe()
 	dns.HandleFunc(fmt.Sprintf("tasks.%s.", testService), func(w dns.ResponseWriter, r *dns.Msg) {
 		m := new(dns.Msg)
 		m.SetReply(r)
-		m.Insert(rr)
+		m.Answer = rr
 		w.WriteMsg(m)
 	})
 
+	// XXX: This test harness seems flakey without a small timeout
+	time.Sleep(time.Second * 1)
+
 	return server, nil
 }
 
 func TestDiscover(t *testing.T) {
+	port, err := pickUnusedUDPPort()
+	if err != nil {
+		t.Fatalf("unable to find free udp port for test dns server harness: %s", err)
+		return
+	}
+	testPort := fmt.Sprintf("%d", port)
+
 	type testCases []struct {
 		desc  string
 		args  discover.Config
@@ -53,10 +82,10 @@ func TestDiscover(t *testing.T) {
 			"valid config - no addresses",
 			discover.Config{
 				"provider": "dns",
-				"query":    "tasks.fake-service",
+				"server":   "127.0.0.1",
+				"port":     testPort,
+				"query":    "tasks.fake-service.",
 				"timeout":  "1s",
-				"v6":       "false",
-				"v4":       "false",
 			},
 			false,
 			0,
@@ -65,10 +94,10 @@ func TestDiscover(t *testing.T) {
 			"valid config - one address",
 			discover.Config{
 				"provider": "dns",
-				"query":    "tasks.test-service",
+				"server":   "127.0.0.1",
+				"port":     testPort,
+				"query":    "tasks.test-service.",
 				"timeout":  "10s",
-				"v6":       "true",
-				"v4":       "true",
 			},
 			false,
 			1,
@@ -77,10 +106,10 @@ func TestDiscover(t *testing.T) {
 			"invalid config - missing query option",
 			discover.Config{
 				"provider": "dns",
+				"server":   "127.0.0.1",
+				"port":     testPort,
 				"query":    "",
 				"timeout":  "1s",
-				"v6":       "false",
-				"v4":       "false",
 			},
 			true,
 			0,
@@ -89,10 +118,10 @@ func TestDiscover(t *testing.T) {
 			"invalid config - bad timeout option",
 			discover.Config{
 				"provider": "dns",
-				"query":    "tasks.fake-service",
+				"server":   "127.0.0.1",
+				"port":     testPort,
+				"query":    "tasks.fake-service.",
 				"timeout":  "1z",
-				"v6":       "false",
-				"v4":       "false",
 			},
 			true,
 			0,
@@ -101,10 +130,10 @@ func TestDiscover(t *testing.T) {
 			"invalid config - bad v6 option",
 			discover.Config{
 				"provider": "dns",
-				"service":  "tasks.fake-service",
+				"server":   "127.0.0.1",
+				"port":     testPort,
+				"query":    "tasks.fake-service.",
 				"timeout":  "1s",
-				"v6":       "xxxx",
-				"v4":       "false",
 			},
 			true,
 			0,
@@ -113,10 +142,10 @@ func TestDiscover(t *testing.T) {
 			"invalid config - bad v4 option",
 			discover.Config{
 				"provider": "dns",
-				"service":  "tasks.fake-service",
+				"server":   "127.0.0.1",
+				"port":     testPort,
+				"query":    "tasks.fake-service.",
 				"timeout":  "1s",
-				"v6":       "false",
-				"v4":       "xxxx",
 			},
 			true,
 			0,
@@ -126,7 +155,7 @@ func TestDiscover(t *testing.T) {
 	p := &provider.Provider{}
 	l := log.New(os.Stderr, "", log.LstdFlags)
 
-	svr, err := newTestServer()
+	svr, err := newTestServer(port)
 	if err != nil {
 		t.Fatalf("Failed to start test server: %s", err.Error())
 		return