feat(v2dns): add partial support for SOA records (#20320)

agent/discovery/discovery.go

@@ -12,10 +12,38 @@ import (
 	"github.com/hashicorp/consul/agent/structs"
 )
 
+var (
+	ErrNoData       = fmt.Errorf("no data")
+	ErrECSNotGlobal = fmt.Errorf("ECS response is not global")
+)
+
+// ECSNotGlobalError may be used to wrap an error or nil, to indicate that the
+// EDNS client subnet source scope is not global.
+// TODO (v2-dns): prepared queries errors are wrapped by this
+type ECSNotGlobalError struct {
+	error
+}
+
+func (e ECSNotGlobalError) Error() string {
+	if e.error == nil {
+		return ""
+	}
+	return e.error.Error()
+}
+
+func (e ECSNotGlobalError) Is(other error) bool {
+	return other == ErrECSNotGlobal
+}
+
+func (e ECSNotGlobalError) Unwrap() error {
+	return e.error
+}
+
 // Query is used to request a name-based Service Discovery lookup.
 type Query struct {
 	QueryType    QueryType
 	QueryPayload QueryPayload
+	Limit        int
 }
 
 // QueryType is used to filter service endpoints.

agent/dns/router.go

@@ -8,6 +8,7 @@ import (
 	"errors"
 	"fmt"
 	"net"
+	"strings"
 	"sync/atomic"
 	"time"
 
@@ -144,9 +145,10 @@ func (r *Router) HandleRequest(req *dns.Msg, reqCtx discovery.Context, remoteAdd
 		return createServerFailureResponse(req, configCtx, false)
 	}
 
-	reqType, responseDomain, needRecurse := r.parseDomain(req)
+	responseDomain, needRecurse := r.parseDomain(req)
 	if needRecurse && !canRecurse(configCtx) {
-		return createServerFailureResponse(req, configCtx, true)
+		// This is the same error as an unmatched domain
+		return createRefusedResponse(req)
 	}
 
 	if needRecurse {
@@ -161,15 +163,23 @@ func (r *Router) HandleRequest(req *dns.Msg, reqCtx discovery.Context, remoteAdd
 		return resp
 	}
 
+	reqType := parseRequestType(req)
 	results, err := r.getQueryResults(req, reqCtx, reqType, configCtx)
-
+	switch {
-	if err != nil && errors.Is(err, errNameNotFound) {
+	case errors.Is(err, errNameNotFound):
 		r.logger.Error("name not found", "name", req.Question[0].Name)
-		return createNameErrorResponse(req, configCtx, responseDomain)
+
-	}
+		ecsGlobal := !errors.Is(err, discovery.ErrECSNotGlobal)
-	if err != nil {
+		return createAuthoritativeResponse(req, configCtx, responseDomain, dns.RcodeNameError, ecsGlobal)
+	// TODO (v2-dns): there is another case here where the discovery service returns "name not found"
+	case errors.Is(err, discovery.ErrNoData):
+		r.logger.Debug("no data available", "name", req.Question[0].Name)
+
+		ecsGlobal := !errors.Is(err, discovery.ErrECSNotGlobal)
+		return createAuthoritativeResponse(req, configCtx, responseDomain, dns.RcodeSuccess, ecsGlobal)
+	case err != nil:
 		r.logger.Error("error processing discovery query", "error", err)
-		return createServerFailureResponse(req, configCtx, false)
+		return createServerFailureResponse(req, configCtx, canRecurse(configCtx))
 	}
 
 	// This needs the question information because it affects the serialization format.
@@ -185,6 +195,17 @@ func (r *Router) HandleRequest(req *dns.Msg, reqCtx discovery.Context, remoteAdd
 // getQueryResults returns a discovery.Result from a DNS message.
 func (r *Router) getQueryResults(req *dns.Msg, reqCtx discovery.Context, reqType requestType, cfgCtx *RouterDynamicConfig) ([]*discovery.Result, error) {
 	switch reqType {
+	case requestTypeConsul:
+		// This is a special case of discovery.QueryByName where we know that we need to query the consul service
+		// regardless of the question name.
+		query := &discovery.Query{
+			QueryType: discovery.QueryTypeService,
+			QueryPayload: discovery.QueryPayload{
+				Name: structs.ConsulServiceName,
+			},
+			Limit: 3, // TODO (v2-dns): need to thread this through to the backend and make sure we shuffle the results
+		}
+		return r.processor.QueryByName(query, reqCtx)
 	case requestTypeName:
 		query, err := buildQueryFromDNSMessage(req, r.domain, r.altDomain, cfgCtx, r.defaultEntMeta)
 		if err != nil {
@@ -224,44 +245,58 @@ func (r *Router) ReloadConfig(newCfg *config.RuntimeConfig) error {
 type requestType string
 
 const (
-	requestTypeName    requestType = "NAME" // A/AAAA/CNAME/SRV/SOA
+	requestTypeName    requestType = "NAME"   // A/AAAA/CNAME/SRV
-	requestTypeIP      requestType = "IP"
+	requestTypeIP      requestType = "IP"     // PTR
-	requestTypeAddress requestType = "ADDR"
+	requestTypeAddress requestType = "ADDR"   // Custom addr. A/AAAA lookups
+	requestTypeConsul  requestType = "CONSUL" // SOA/NS
 )
 
-// parseQuery converts a DNS message into a generic discovery request.
+// parseDomain converts a DNS message into a generic discovery request.
 // If the request domain does not match "consul." or the alternative domain,
 // it will return true for needRecurse. The logic is based on miekg/dns.ServeDNS matcher.
 // The implementation assumes that the only valid domains are "consul." and the alternative domain, and
 // that DS query types are not supported.
-func (r *Router) parseDomain(req *dns.Msg) (requestType, string, bool) {
+func (r *Router) parseDomain(req *dns.Msg) (string, bool) {
 	target := dns.CanonicalName(req.Question[0].Name)
 	target, _ = stripSuffix(target)
 
 	for offset, overflow := 0, false; !overflow; offset, overflow = dns.NextLabel(target, offset) {
 		subdomain := target[offset:]
 		switch subdomain {
+		case ".":
+			// We don't support consul having a domain or altdomain attached to the root.
+			return "", true
 		case r.domain:
-			if isAddrSubdomain(target) {
+			return r.domain, false
-				return requestTypeAddress, r.domain, false
-			}
-			return requestTypeName, r.domain, false
-
 		case r.altDomain:
-			// TODO (v2-dns): the default, unspecified alt domain should be ".". Next label should never return this
+			return r.altDomain, false
-			// but write a test to verify that.
-			if isAddrSubdomain(target) {
-				return requestTypeAddress, r.altDomain, false
-			}
-			return requestTypeName, r.altDomain, false
 		case arpaDomain:
 			// PTR queries always respond with the primary domain.
-			return requestTypeIP, r.domain, false
+			return r.domain, false
 			// Default: fallthrough
 		}
 	}
 	// No match found; recurse if possible
-	return "", "", true
+	return "", true
+}
+
+// parseRequestType inspects the DNS message type and question name to determine the requestType of request.
+// We assume by the time this is called, we are responding to a question with a domain we serve.
+// This is used internally to determine which query processor method (if any) to invoke.
+func parseRequestType(req *dns.Msg) requestType {
+	switch {
+	case req.Question[0].Qtype == dns.TypeSOA || req.Question[0].Qtype == dns.TypeNS:
+		// SOA and NS type supersede the domain
+		// NOTE!: In V1 of the DNS server it was possible to serve a PTR lookup using the arpa domain but a SOA question type.
+		// This also included the SOA record. This seemed inconsistent and unnecessary - it was removed for simplicity.
+		return requestTypeConsul
+	case isPTRSubdomain(req.Question[0].Name):
+		return requestTypeIP
+	case isAddrSubdomain(req.Question[0].Name):
+		return requestTypeAddress
+	default:
+		return requestTypeName
+	}
 }
 
 // serializeQueryResults converts a discovery.Result into a DNS message.
@@ -272,7 +307,10 @@ func (r *Router) serializeQueryResults(req *dns.Msg, results []*discovery.Result
 	resp.Authoritative = true
 	resp.RecursionAvailable = canRecurse(cfg)
 
-	// TODO (v2-dns): add SOA if that is the question type
+	// Always add the SOA record if requested.
+	if req.Question[0].Qtype == dns.TypeSOA {
+		resp.Answer = append(resp.Answer, makeSOARecord(responseDomain, cfg))
+	}
 
 	for _, result := range results {
 		appendResultToDNSResponse(result, req, resp, responseDomain, cfg)
@@ -334,6 +372,18 @@ func isAddrSubdomain(domain string) bool {
 	return false
 }
 
+// isPTRSubdomain returns true if the domain ends in the PTR domain, "in-addr.arpa.".
+func isPTRSubdomain(domain string) bool {
+	labels := dns.SplitDomainName(domain)
+	labelCount := len(labels)
+
+	if labelCount < 3 {
+		return false
+	}
+
+	return fmt.Sprintf("%s.%s.", labels[labelCount-2], labels[labelCount-1]) == arpaDomain
+}
+
 // getDynamicRouterConfig takes agent config and creates/resets the config used by DNS Router
 func getDynamicRouterConfig(conf *config.RuntimeConfig) (*RouterDynamicConfig, error) {
 	cfg := &RouterDynamicConfig{
@@ -402,7 +452,7 @@ func setEDNS(request *dns.Msg, response *dns.Msg, ecsGlobal bool) {
 	ednsResp.Hdr.Rrtype = dns.TypeOPT
 	ednsResp.SetUDPSize(edns.UDPSize())
 
-	// Setup the ECS option if present
+	// Set up the ECS option if present
 	if subnet := ednsSubnetForRequest(request); subnet != nil {
 		subOp := new(dns.EDNS0_SUBNET)
 		subOp.Code = dns.EDNS0SUBNET
@@ -426,7 +476,6 @@ func setEDNS(request *dns.Msg, response *dns.Msg, ecsGlobal bool) {
 func ednsSubnetForRequest(req *dns.Msg) *dns.EDNS0_SUBNET {
 	// IsEdns0 returns the EDNS RR if present or nil otherwise
 	edns := req.IsEdns0()
-
 	if edns == nil {
 		return nil
 	}
@@ -436,11 +485,11 @@ func ednsSubnetForRequest(req *dns.Msg) *dns.EDNS0_SUBNET {
 			return subnet
 		}
 	}
-
 	return nil
 }
 
-// createRefusedResponse returns a REFUSED message.
+// createRefusedResponse returns a REFUSED message. This is the default behavior for unmatched queries in
+// upstream miekg/dns.
 func createRefusedResponse(req *dns.Msg) *dns.Msg {
 	// Return a REFUSED message
 	m := &dns.Msg{}
@@ -448,32 +497,20 @@ func createRefusedResponse(req *dns.Msg) *dns.Msg {
 	return m
 }
 
-// createNameErrorResponse returns a NXDOMAIN message.
+// createAuthoritativeResponse returns an authoritative message that contains the SOA in the event that data is
-func createNameErrorResponse(req *dns.Msg, cfg *RouterDynamicConfig, domain string) *dns.Msg {
+// not return for a query. There can be multiple reasons for not returning data, hence the rcode argument.
-	// Return a NXDOMAIN message
+func createAuthoritativeResponse(req *dns.Msg, cfg *RouterDynamicConfig, domain string, rcode int, ecsGlobal bool) *dns.Msg {
 	m := &dns.Msg{}
-	m.SetRcode(req, dns.RcodeNameError)
+	m.SetRcode(req, rcode)
 	m.Compress = !cfg.DisableCompression
 	m.Authoritative = true
+	m.RecursionAvailable = canRecurse(cfg)
+	if edns := req.IsEdns0(); edns != nil {
+		setEDNS(req, m, ecsGlobal)
+	}
 
 	// We add the SOA on NameErrors
-	// TODO (v2-dns): refactor into a common function
+	soa := makeSOARecord(domain, cfg)
-	soa := &dns.SOA{
-		Hdr: dns.RR_Header{
-			Name:   domain,
-			Rrtype: dns.TypeSOA,
-			Class:  dns.ClassINET,
-			// Has to be consistent with MinTTL to avoid invalidation
-			Ttl: cfg.SOAConfig.Minttl,
-		},
-		Ns:      "ns." + domain,
-		Serial:  uint32(time.Now().Unix()),
-		Mbox:    "hostmaster." + domain,
-		Refresh: cfg.SOAConfig.Refresh,
-		Retry:   cfg.SOAConfig.Retry,
-		Expire:  cfg.SOAConfig.Expire,
-		Minttl:  cfg.SOAConfig.Minttl,
-	}
 	m.Ns = append(m.Ns, soa)
 
 	return m
@@ -504,7 +541,7 @@ func buildAddressResults(req *dns.Msg) ([]*discovery.Result, error) {
 }
 
 // buildQueryFromDNSMessage appends the discovery result to the dns message.
-func appendResultToDNSResponse(result *discovery.Result, req *dns.Msg, resp *dns.Msg, _ string, cfg *RouterDynamicConfig) {
+func appendResultToDNSResponse(result *discovery.Result, req *dns.Msg, resp *dns.Msg, domain string, cfg *RouterDynamicConfig) {
 	ip, ok := convertToIp(result)
 
 	// if the result is not an IP, we can try to recurse on the hostname.
@@ -516,7 +553,7 @@ func appendResultToDNSResponse(result *discovery.Result, req *dns.Msg, resp *dns
 
 	var ttl uint32
 	switch result.Type {
-	case discovery.ResultTypeNode, discovery.ResultTypeVirtual:
+	case discovery.ResultTypeNode, discovery.ResultTypeVirtual, discovery.ResultTypeWorkload:
 		ttl = uint32(cfg.NodeTTL / time.Second)
 	case discovery.ResultTypeService:
 		// TODO (v2-dns): implement service TTL using the radix tree
@@ -527,12 +564,32 @@ func appendResultToDNSResponse(result *discovery.Result, req *dns.Msg, resp *dns
 
 	record, isIPV4 := makeRecord(qName, ip, ttl)
 
-	if qType == dns.TypeSRV {
+	// TODO (v2-dns): skip records that refer to a workload/node that don't have a valid DNS name.
+
+	// Special case responses
+	switch qType {
+	case dns.TypeSOA:
+		// TODO (v2-dns): fqdn in V1 has the datacenter included, this would need to be added to discovery.Result
+		// to be returned in the result.
+		fqdn := fmt.Sprintf("%s.%s.%s", result.Target, strings.ToLower(string(result.Type)), domain)
+		extraRecord, _ := makeRecord(fqdn, ip, ttl) // TODO (v2-dns): this is not sufficient, because recursion and CNAMES are supported
+
+		resp.Ns = append(resp.Ns, makeNSRecord(domain, fqdn, ttl))
+		resp.Extra = append(resp.Extra, extraRecord)
+		return
+	case dns.TypeNS:
+		// TODO (v2-dns): fqdn in V1 has the datacenter included, this would need to be added to discovery.Result
+		fqdn := fmt.Sprintf("%s.%s.%s.", result.Target, strings.ToLower(string(result.Type)), domain)
+		extraRecord, _ := makeRecord(fqdn, ip, ttl) // TODO (v2-dns): this is not sufficient, because recursion and CNAMES are supported
+
+		resp.Answer = append(resp.Ns, makeNSRecord(domain, fqdn, ttl))
+		resp.Extra = append(resp.Extra, extraRecord)
+		return
+	case dns.TypeSRV:
 		// We put A/AAAA/CNAME records in the additional section for SRV requests
 		resp.Extra = append(resp.Extra, record)
 
 		// TODO (v2-dns): implement SRV records for the answer section
-
 		return
 	}
 
@@ -558,11 +615,41 @@ func convertToIp(result *discovery.Result) (net.IP, bool) {
 	return ip, true
 }
 
-// n A or AAAA record for the given name and IP.
+func makeSOARecord(domain string, cfg *RouterDynamicConfig) dns.RR {
+	return &dns.SOA{
+		Hdr: dns.RR_Header{
+			Name:   domain,
+			Rrtype: dns.TypeSOA,
+			Class:  dns.ClassINET,
+			// Has to be consistent with MinTTL to avoid invalidation
+			Ttl: cfg.SOAConfig.Minttl,
+		},
+		Ns:      "ns." + domain,
+		Serial:  uint32(time.Now().Unix()),
+		Mbox:    "hostmaster." + domain,
+		Refresh: cfg.SOAConfig.Refresh,
+		Retry:   cfg.SOAConfig.Retry,
+		Expire:  cfg.SOAConfig.Expire,
+		Minttl:  cfg.SOAConfig.Minttl,
+	}
+}
+
+func makeNSRecord(domain, fqdn string, ttl uint32) dns.RR {
+	return &dns.NS{
+		Hdr: dns.RR_Header{
+			Name:   domain,
+			Rrtype: dns.TypeNS,
+			Class:  dns.ClassINET,
+			Ttl:    ttl,
+		},
+		Ns: fqdn,
+	}
+}
+
+// makeRecord an A or AAAA record for the given name and IP.
 // Note: we might want to pass in the Query Name here, which is used in addr. and virtual. queries
 // since there is only ever one result. Right now choosing to leave it off for simplification.
 func makeRecord(name string, ip net.IP, ttl uint32) (dns.RR, bool) {
-
 	isIPV4 := ip.To4() != nil
 
 	if isIPV4 {

agent/dns/router_test.go

@@ -17,15 +17,16 @@ import (
 	"github.com/hashicorp/consul/acl"
 	"github.com/hashicorp/consul/agent/config"
 	"github.com/hashicorp/consul/agent/discovery"
+	"github.com/hashicorp/consul/agent/structs"
 )
 
 // TODO (v2-dns)
 
-// Test Parameters
+// TBD Test Cases
-// 	1. Domain vs AltDomain vs non-consul Main domain
+//  1. Reload the configuration (e.g. SOA)
-//  2. Reload the configuration (e.g. SOA)
+//  2. Something to check the token makes it through to the data fetcher
-//  3. Something to check the token makes it through to the data fetcher
+//  3. Something case-insensitive
-// 4. Something case insensitive
+//  4. Test the edns settings.
 
 func Test_HandleRequest(t *testing.T) {
 	type testCase struct {
@@ -59,13 +60,10 @@ func Test_HandleRequest(t *testing.T) {
 			// configureRecursor: call not expected.
 			response: &dns.Msg{
 				MsgHdr: dns.MsgHdr{
-					Opcode:             dns.OpcodeQuery,
+					Opcode:   dns.OpcodeQuery,
-					Response:           true,
+					Response: true,
-					Authoritative:      false,
+					Rcode:    dns.RcodeRefused,
-					Rcode:              dns.RcodeServerFailure,
-					RecursionAvailable: true,
 				},
-				Compress: true,
 				Question: []dns.Question{
 					{
 						Name:   "google.com.",
@@ -148,7 +146,7 @@ func Test_HandleRequest(t *testing.T) {
 			},
 		},
 		{
-			name: "recursors configured, matching domain",
+			name: "recursors configured, no matching domain",
 			request: &dns.Msg{
 				MsgHdr: dns.MsgHdr{
 					Opcode: dns.OpcodeQuery,
@@ -226,6 +224,93 @@ func Test_HandleRequest(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "recursors configured, the root domain is handled by the recursor",
+			request: &dns.Msg{
+				MsgHdr: dns.MsgHdr{
+					Opcode: dns.OpcodeQuery,
+				},
+				Question: []dns.Question{
+					{
+						Name:   ".",
+						Qtype:  dns.TypeA,
+						Qclass: dns.ClassINET,
+					},
+				},
+			},
+			agentConfig: &config.RuntimeConfig{
+				DNSRecursors: []string{"8.8.8.8"},
+			},
+			configureRecursor: func(recursor dnsRecursor) {
+				// this response is modeled after `dig .`
+				resp := &dns.Msg{
+					MsgHdr: dns.MsgHdr{
+						Opcode:        dns.OpcodeQuery,
+						Response:      true,
+						Authoritative: true,
+						Rcode:         dns.RcodeSuccess,
+					},
+					Question: []dns.Question{
+						{
+							Name:   ".",
+							Qtype:  dns.TypeA,
+							Qclass: dns.ClassINET,
+						},
+					},
+					Answer: []dns.RR{
+						&dns.SOA{
+							Hdr: dns.RR_Header{
+								Name:   ".",
+								Rrtype: dns.TypeSOA,
+								Class:  dns.ClassINET,
+								Ttl:    86391,
+							},
+							Ns:      "a.root-servers.net.",
+							Serial:  2024012200,
+							Mbox:    "nstld.verisign-grs.com.",
+							Refresh: 1800,
+							Retry:   900,
+							Expire:  604800,
+							Minttl:  86400,
+						},
+					},
+				}
+				recursor.(*mockDnsRecursor).On("handle",
+					mock.Anything, mock.Anything, mock.Anything).Return(resp, nil)
+			},
+			response: &dns.Msg{
+				MsgHdr: dns.MsgHdr{
+					Opcode:        dns.OpcodeQuery,
+					Response:      true,
+					Authoritative: true,
+					Rcode:         dns.RcodeSuccess,
+				},
+				Question: []dns.Question{
+					{
+						Name:   ".",
+						Qtype:  dns.TypeA,
+						Qclass: dns.ClassINET,
+					},
+				},
+				Answer: []dns.RR{
+					&dns.SOA{
+						Hdr: dns.RR_Header{
+							Name:   ".",
+							Rrtype: dns.TypeSOA,
+							Class:  dns.ClassINET,
+							Ttl:    86391,
+						},
+						Ns:      "a.root-servers.net.",
+						Serial:  2024012200,
+						Mbox:    "nstld.verisign-grs.com.",
+						Refresh: 1800,
+						Retry:   900,
+						Expire:  604800,
+						Minttl:  86400,
+					},
+				},
+			},
+		},
 		// addr queries
 		{
 			name: "test A 'addr.' query, ipv4 response",
@@ -709,10 +794,303 @@ func Test_HandleRequest(t *testing.T) {
 				},
 			},
 		},
+		// SOA Queries
+		{
+			name: "vanilla SOA query",
+			request: &dns.Msg{
+				MsgHdr: dns.MsgHdr{
+					Opcode: dns.OpcodeQuery,
+				},
+				Question: []dns.Question{
+					{
+						Name:   "consul.",
+						Qtype:  dns.TypeSOA,
+						Qclass: dns.ClassINET,
+					},
+				},
+			},
+			configureDataFetcher: func(fetcher discovery.CatalogDataFetcher) {
+				fetcher.(*discovery.MockCatalogDataFetcher).
+					On("FetchEndpoints", mock.Anything, mock.Anything, mock.Anything).
+					Return([]*discovery.Result{
+						{
+							Address: "1.2.3.4",
+							Type:    discovery.ResultTypeWorkload,
+							Target:  "server-one", // This would correlate to the workload name
+						},
+						{
+							Address: "4.5.6.7",
+							Type:    discovery.ResultTypeWorkload,
+							Target:  "server-two", // This would correlate to the workload name
+						},
+					}, nil).
+					Run(func(args mock.Arguments) {
+						req := args.Get(1).(*discovery.QueryPayload)
+						reqType := args.Get(2).(discovery.LookupType)
+
+						require.Equal(t, discovery.LookupTypeService, reqType)
+						require.Equal(t, structs.ConsulServiceName, req.Name)
+					})
+			},
+			response: &dns.Msg{
+				MsgHdr: dns.MsgHdr{
+					Opcode:        dns.OpcodeQuery,
+					Response:      true,
+					Authoritative: true,
+				},
+				Compress: true,
+				Question: []dns.Question{
+					{
+						Name:   "consul.",
+						Qtype:  dns.TypeSOA,
+						Qclass: dns.ClassINET,
+					},
+				},
+				Answer: []dns.RR{
+					&dns.SOA{
+						Hdr: dns.RR_Header{
+							Name:   "consul.",
+							Rrtype: dns.TypeSOA,
+							Class:  dns.ClassINET,
+							Ttl:    4,
+						},
+						Ns:      "ns.consul.",
+						Serial:  uint32(time.Now().Unix()),
+						Mbox:    "hostmaster.consul.",
+						Refresh: 1,
+						Expire:  3,
+						Retry:   2,
+						Minttl:  4,
+					},
+				},
+				Ns: []dns.RR{
+					&dns.NS{
+						Hdr: dns.RR_Header{
+							Name:   "consul.",
+							Rrtype: dns.TypeNS,
+							Class:  dns.ClassINET,
+							Ttl:    123,
+						},
+						Ns: "server-one.workload.consul.",
+					},
+					&dns.NS{
+						Hdr: dns.RR_Header{
+							Name:   "consul.",
+							Rrtype: dns.TypeNS,
+							Class:  dns.ClassINET,
+							Ttl:    123,
+						},
+						Ns: "server-two.workload.consul.",
+					},
+				},
+				Extra: []dns.RR{
+					&dns.A{
+						Hdr: dns.RR_Header{
+							Name:   "server-one.workload.consul.",
+							Rrtype: dns.TypeA,
+							Class:  dns.ClassINET,
+							Ttl:    123,
+						},
+						A: net.ParseIP("1.2.3.4"),
+					},
+					&dns.A{
+						Hdr: dns.RR_Header{
+							Name:   "server-two.workload.consul.",
+							Rrtype: dns.TypeA,
+							Class:  dns.ClassINET,
+							Ttl:    123,
+						},
+						A: net.ParseIP("4.5.6.7"),
+					},
+				},
+			},
+		},
+		{
+			name: "SOA query against alternate domain",
+			request: &dns.Msg{
+				MsgHdr: dns.MsgHdr{
+					Opcode: dns.OpcodeQuery,
+				},
+				Question: []dns.Question{
+					{
+						Name:   "testdomain.",
+						Qtype:  dns.TypeSOA,
+						Qclass: dns.ClassINET,
+					},
+				},
+			},
+			agentConfig: &config.RuntimeConfig{
+				DNSDomain:    "consul",
+				DNSAltDomain: "testdomain",
+				DNSNodeTTL:   123 * time.Second,
+				DNSSOA: config.RuntimeSOAConfig{
+					Refresh: 1,
+					Retry:   2,
+					Expire:  3,
+					Minttl:  4,
+				},
+			},
+			configureDataFetcher: func(fetcher discovery.CatalogDataFetcher) {
+				fetcher.(*discovery.MockCatalogDataFetcher).
+					On("FetchEndpoints", mock.Anything, mock.Anything, mock.Anything).
+					Return([]*discovery.Result{
+						{
+							Address: "1.2.3.4",
+							Type:    discovery.ResultTypeWorkload,
+							Target:  "server-one", // This would correlate to the workload name
+						},
+						{
+							Address: "4.5.6.7",
+							Type:    discovery.ResultTypeWorkload,
+							Target:  "server-two", // This would correlate to the workload name
+						},
+					}, nil).
+					Run(func(args mock.Arguments) {
+						req := args.Get(1).(*discovery.QueryPayload)
+						reqType := args.Get(2).(discovery.LookupType)
+
+						require.Equal(t, discovery.LookupTypeService, reqType)
+						require.Equal(t, structs.ConsulServiceName, req.Name)
+					})
+			},
+			response: &dns.Msg{
+				MsgHdr: dns.MsgHdr{
+					Opcode:        dns.OpcodeQuery,
+					Response:      true,
+					Authoritative: true,
+				},
+				Compress: true,
+				Question: []dns.Question{
+					{
+						Name:   "testdomain.",
+						Qtype:  dns.TypeSOA,
+						Qclass: dns.ClassINET,
+					},
+				},
+				Answer: []dns.RR{
+					&dns.SOA{
+						Hdr: dns.RR_Header{
+							Name:   "testdomain.",
+							Rrtype: dns.TypeSOA,
+							Class:  dns.ClassINET,
+							Ttl:    4,
+						},
+						Ns:      "ns.testdomain.",
+						Serial:  uint32(time.Now().Unix()),
+						Mbox:    "hostmaster.testdomain.",
+						Refresh: 1,
+						Expire:  3,
+						Retry:   2,
+						Minttl:  4,
+					},
+				},
+				Ns: []dns.RR{
+					&dns.NS{
+						Hdr: dns.RR_Header{
+							Name:   "testdomain.",
+							Rrtype: dns.TypeNS,
+							Class:  dns.ClassINET,
+							Ttl:    123,
+						},
+						Ns: "server-one.workload.testdomain.",
+					},
+					&dns.NS{
+						Hdr: dns.RR_Header{
+							Name:   "testdomain.",
+							Rrtype: dns.TypeNS,
+							Class:  dns.ClassINET,
+							Ttl:    123,
+						},
+						Ns: "server-two.workload.testdomain.",
+					},
+				},
+				Extra: []dns.RR{
+					&dns.A{
+						Hdr: dns.RR_Header{
+							Name:   "server-one.workload.testdomain.",
+							Rrtype: dns.TypeA,
+							Class:  dns.ClassINET,
+							Ttl:    123,
+						},
+						A: net.ParseIP("1.2.3.4"),
+					},
+					&dns.A{
+						Hdr: dns.RR_Header{
+							Name:   "server-two.workload.testdomain.",
+							Rrtype: dns.TypeA,
+							Class:  dns.ClassINET,
+							Ttl:    123,
+						},
+						A: net.ParseIP("4.5.6.7"),
+					},
+				},
+			},
+		},
+		// Service Lookup
+		{
+			name: "When no data is return from a query, send SOA",
+			request: &dns.Msg{
+				MsgHdr: dns.MsgHdr{
+					Opcode: dns.OpcodeQuery,
+				},
+				Question: []dns.Question{
+					{
+						Name:   "foo.service.consul.",
+						Qtype:  dns.TypeA,
+						Qclass: dns.ClassINET,
+					},
+				},
+			},
+			configureDataFetcher: func(fetcher discovery.CatalogDataFetcher) {
+				fetcher.(*discovery.MockCatalogDataFetcher).
+					On("FetchEndpoints", mock.Anything, mock.Anything, mock.Anything).
+					Return(nil, discovery.ErrNoData).
+					Run(func(args mock.Arguments) {
+						req := args.Get(1).(*discovery.QueryPayload)
+						reqType := args.Get(2).(discovery.LookupType)
+
+						require.Equal(t, discovery.LookupTypeService, reqType)
+						require.Equal(t, "foo", req.Name)
+					})
+			},
+			response: &dns.Msg{
+				MsgHdr: dns.MsgHdr{
+					Opcode:        dns.OpcodeQuery,
+					Response:      true,
+					Authoritative: true,
+				},
+				Compress: true,
+				Question: []dns.Question{
+					{
+						Name:   "foo.service.consul.",
+						Qtype:  dns.TypeA,
+						Qclass: dns.ClassINET,
+					},
+				},
+				Ns: []dns.RR{
+					&dns.SOA{
+						Hdr: dns.RR_Header{
+							Name:   "consul.",
+							Rrtype: dns.TypeSOA,
+							Class:  dns.ClassINET,
+							Ttl:    4,
+						},
+						Ns:      "ns.consul.",
+						Serial:  uint32(time.Now().Unix()),
+						Mbox:    "hostmaster.consul.",
+						Refresh: 1,
+						Expire:  3,
+						Retry:   2,
+						Minttl:  4,
+					},
+				},
+			},
+		},
+		// TODO (v2-dns): add a test to make sure only 3 records are returned
 	}
 
 	run := func(t *testing.T, tc testCase) {
-		cdf := &discovery.MockCatalogDataFetcher{}
+		cdf := discovery.NewMockCatalogDataFetcher(t)
 		if tc.configureDataFetcher != nil {
 			tc.configureDataFetcher(cdf)
 		}

agent/dns_test.go

@@ -285,6 +285,7 @@ func TestDNSCycleRecursorCheck(t *testing.T) {
 					A:   []byte{0xAC, 0x15, 0x2D, 0x43}, // 172 , 21, 45, 67
 				},
 			}
+			require.NotNil(t, in)
 			require.Equal(t, wantAnswer, in.Answer)
 		})
 	}
@@ -323,6 +324,7 @@ func TestDNSCycleRecursorCheckAllFail(t *testing.T) {
 			in, _, err := client.Exchange(m, agent.DNSAddr())
 			require.NoError(t, err)
 			// Verify if we hit SERVFAIL from Consul
+			require.NotNil(t, in)
 			require.Equal(t, dns.RcodeServerFailure, in.Rcode)
 		})
 	}