Improve conformance with DNS behavior

.vscode/launch.json

@@ -0,0 +1,27 @@
+{
+    "version": "0.2.0",
+    "configurations": [
+        {
+            "name": "Debug CoreDNS",
+            "type": "go",
+            "request": "launch",
+            "mode": "exec",
+            "program": "${workspaceFolder}/../coredns/coredns",
+            "args": [],
+            "cwd": "${workspaceFolder}/testdata",
+            "env": {
+                "GOWORK": "${workspaceFolder}/../coredns/go.work"
+            },
+            "preLaunchTask": "Build CoreDNS",
+            "showLog": true
+        },
+        {
+            "name": "Attach to CoreDNS",
+            "type": "go",
+            "request": "attach",
+            "mode": "remote",
+            "port": 2345,
+            "host": "127.0.0.1"
+        }
+    ]
+}

.vscode/tasks.json

@@ -0,0 +1,21 @@
+{
+    "version": "2.0.0",
+    "tasks": [
+        {
+            "label": "Build CoreDNS",
+            "type": "shell",
+            "command": "go generate && go build -gcflags='all=-N -l' -o coredns",
+            "options": {
+                "cwd": "${workspaceFolder}/../coredns",
+                "env": {
+                    "GOWORK": "${workspaceFolder}/../coredns/go.work"
+                }
+            },
+            "group": {
+                "kind": "build",
+                "isDefault": true
+            },
+            "problemMatcher": ["$go"]
+        }
+    ]
+}

testdata/Corefile

@@ -0,0 +1,5 @@
+example.com example.org {
+	debug
+  bind 127.0.0.36
+	yaml zones.yaml
+}

yamlplugin.go

@@ -78,11 +78,10 @@ func setup(c *caddy.Controller) error {
 func init() { plugin.Register("yaml", setup) }
 
 func (y YamlPlugin) lookupRRs(qname string, qtype string, reply *dns.Msg) (int, error) {
-	var rcode int
 	res, ok := y.Zone.LookupType(qname, qtype)
 	if !ok {
 		// NXDOMAIN
-		rcode = dns.RcodeNameError
+		reply.Rcode = dns.RcodeNameError
 	}
 	if res.IsReferral {
 		reply.Authoritative = false
@@ -101,12 +100,14 @@ func (y YamlPlugin) lookupRRs(qname string, qtype string, reply *dns.Msg) (int,
 			if ttl == 0 {
 				ttl = y.Config.DefaultTtl
 			}
-			rr, err := dns.NewRR(fmt.Sprintf("%s %d %s %s", record.Name, ttl, record.Record.Type, record.Record.Value))
+			rrString := fmt.Sprintf("%s %d %s %s", record.Name, ttl, record.Record.Type, record.Record.Value)
+			rr, err := dns.NewRR(rrString)
 			if err != nil {
-				return rcode, fmt.Errorf("failed to generate RR for %s %s %s: %w", record.Name, record.Record.Type, record.Record.Value, err)
+				log.Errorf("Failed to parse RR string: %q", rrString)
+				return reply.Rcode, fmt.Errorf("failed to generate RR for %s %s %s: %w", record.Name, record.Record.Type, record.Record.Value, err)
 			}
 			*section.dest = append(*section.dest, rr)
 		}
 	}
-	return rcode, nil
+	return reply.Rcode, nil
 }

yamlzone.go

@@ -76,7 +76,7 @@ func LoadZoneBytes(data []byte, filename string) (*Zone, error) {
 	return z, nil
 }
 
-func (z *Zone) Validate(name string, soa *Record, ns *[]Record) error {
+func (z *Zone) Validate(name string, soa *NamedRecord, ns *[]Record) error {
 	nameservers, otherRecords := []Record{}, []Record{}
 	nameserversMap := map[string]bool{}
 	isZoneApex := false
@@ -85,8 +85,7 @@ func (z *Zone) Validate(name string, soa *Record, ns *[]Record) error {
 		switch record.Type {
 		case "SOA":
 			isZoneApex = true
-			soa = &record
+			soa = &NamedRecord{Name: name, Record: record}
-			z.SOA = NamedRecord{Name: name, Record: record}
 		case "NS":
 			nameservers = append(nameservers, record)
 			nameserversMap[record.Value] = true
@@ -97,12 +96,12 @@ func (z *Zone) Validate(name string, soa *Record, ns *[]Record) error {
 		}
 	}
 
+	z.IsDelegationPoint = !isZoneApex && len(nameservers) > 0
+
 	if cnameCount > 1 || (cnameCount > 0 && len(otherRecords) > 0) {
 		return fmt.Errorf("%s: extraneous records found next to CNAME", name)
 	}
 
-	z.IsDelegationPoint = !isZoneApex && len(nameservers) > 0
-
 	if soa == nil {
 		// Outside zone
 		if z.IsDelegationPoint {
@@ -115,9 +114,10 @@ func (z *Zone) Validate(name string, soa *Record, ns *[]Record) error {
 		if len(nameservers) == 0 {
 			return fmt.Errorf("%s: zone apex missing NS records", name)
 		}
+		z.SOA = *soa
 		ns = &nameservers
 	} else if len(nameservers) > 0 {
-		// Delegation point (does not fall through to subzone validation)
+		// Delegation point (does not fall through)
 		if len(otherRecords) > 0 {
 			return fmt.Errorf("%s: non-glue, non-NS records found at delegation point: %v", name, otherRecords)
 		}
@@ -129,9 +129,14 @@ func (z *Zone) Validate(name string, soa *Record, ns *[]Record) error {
 			}
 		}
 		return nil
+	} else {
+		// Normal node
+		// Cache SOA at each node in the zone tree
+		z.SOA = *soa
 	}
-	// Subzone validation
+
 	// Either we're outside a zone, at a zone apex, or at a non-delegated subzone
+
 	for subname, subzone := range z.Subzones {
 		if err := subzone.Validate(concatName(name, subname), soa, ns); err != nil {
 			return err
@@ -275,6 +280,11 @@ func nameToPath(name string) []string {
 }
 
 func (z *Zone) Lookup(name string) (LookupResult, bool) {
+	res, ok := z.LookupType(name, "")
+	return res, ok
+}
+
+func (z *Zone) LookupType(name string, recordType string) (LookupResult, bool) {
 	res := LookupResult{}
 	path := nameToPath(name)
 	for _, label := range path {
@@ -282,18 +292,6 @@ func (z *Zone) Lookup(name string) (LookupResult, bool) {
 		if label == "" {
 			continue
 		}
-		if z.IsDelegationPoint {
-			res.IsReferral = true
-			// Capture NS records
-			for _, record := range z.Records {
-				if record.Type == "NS" {
-					res.Ns = append(res.Ns, NamedRecord{Name: name, Record: record})
-				}
-			}
-			// Retrieve glue records from cache
-			res.Extra = append(res.Extra, z.GlueRecords...)
-			return res, true
-		}
 		if sz, ok := z.Subzones[label]; !ok {
 			// NXDOMAIN
 			res.Ns = []NamedRecord{z.SOA}
@@ -301,11 +299,28 @@ func (z *Zone) Lookup(name string) (LookupResult, bool) {
 		} else {
 			z = sz
 		}
+		if z.IsDelegationPoint {
+			// Retrieve glue records from cache, whether referral or NS query
+			res.Extra = append(res.Extra, z.GlueRecords...)
+			if recordType != "NS" {
+				// Treat as referral unless client requested NS records
+				res.IsReferral = true
+				// Capture NS records
+				for _, record := range z.Records {
+					if record.Type == "NS" {
+						res.Ns = append(res.Ns, NamedRecord{Name: name, Record: record})
+					}
+				}
+				return res, true
+			}
+		}
 	}
 	res.Answer = []NamedRecord{}
 	for _, record := range z.Records {
+		if recordType == "" || record.Type == recordType {
 			res.Answer = append(res.Answer, NamedRecord{Name: name, Record: record})
 		}
+	}
 	if len(res.Answer) == 0 {
 		// NODATA
 		res.Ns = []NamedRecord{z.SOA}
@@ -314,29 +329,6 @@ func (z *Zone) Lookup(name string) (LookupResult, bool) {
 	return res, true
 }
 
-func (z *Zone) FilterRecords(res LookupResult, recordType string) LookupResult {
-	filtered := []NamedRecord{}
-	for _, nr := range res.Answer {
-		if nr.Record.Type == recordType {
-			filtered = append(filtered, nr)
-		}
-	}
-	res.Answer = filtered
-	if len(res.Answer) == 0 && !res.IsReferral {
-		// This ends up as NODATA even if it had data before filtering
-		res.Ns = []NamedRecord{z.SOA}
-	}
-	return res
-}
-
-func (z *Zone) LookupType(name string, recordType string) (LookupResult, bool) {
-	res, ok := z.Lookup(name)
-	if !ok {
-		return LookupResult{}, false
-	}
-	return z.FilterRecords(res, recordType), true
-}
-
 func concatName(name string, subname string) string {
 	if name == "." {
 		return subname + "."

yamlzone_test.go

@@ -195,8 +195,8 @@ func TestFullZone(t *testing.T) {
 		assertRecord(t, res.Answer[1].Record, "A", 3600, "203.0.113.24")
 	})
 
-	t.Run("Lookup partner.example.com", func(t *testing.T) {
+	t.Run("Lookup partner.example.com NS", func(t *testing.T) {
-		res, ok := zFull.Lookup("partner.example.com")
+		res, ok := zFull.LookupType("partner.example.com", "NS")
 		assertOk(t, ok)
 		assertRecordCount(t, res, 2)
 		assertRecord(t, res.Answer[0].Record, "NS", 3600, "ns1.example.org")