Fix build and polish codebase to OSS quality

Dockerfile used golang:1.21 but go.mod requires 1.25; CI was broken.
Refactored plugin code with proper godoc comments, extracted helpers to
eliminate duplicated response-building logic, modernized tests with
t.Run subtests and shared assertion helpers, and rewrote README with
configuration reference table and professional structure.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

synthetic_test.go

@@ -2,378 +2,295 @@ package synthetic
 
 import (
 	"context"
-	"fmt"
+	"net"
+	"testing"
+
 	"github.com/coredns/coredns/plugin/pkg/dnstest"
 	"github.com/coredns/coredns/plugin/test"
 	"github.com/miekg/dns"
-	"net"
-
-	"testing"
 )
 
-func TestServeDNSv4(t *testing.T) {
-	ip, ipNet, err := net.ParseCIDR("192.0.2.0/24")
-	if err != nil {
-		log.Fatal(err)
+// testCase describes a single DNS query and the expected response.
+type testCase struct {
+	name       string
+	qname      string
+	qtype      uint16
+	wantRcode  int
+	wantAnswer []string
+	wantTTL    uint32
+}
+
+// assertResponse validates the DNS response from ServeDNS against tc.
+func assertResponse(t *testing.T, tc testCase, rc int, w *dnstest.Recorder) {
+	t.Helper()
+
+	if rc != tc.wantRcode {
+		t.Errorf("rcode: got %d, want %d", rc, tc.wantRcode)
 	}
 
-	s := synthetic{
-		Next: test.ErrorHandler(),
-		Config: syntheticConfig{
-			net:     []*net.IPNet{{IP: ip, Mask: ipNet.Mask}},
-			forward: "example.com",
-			prefix:  "ip-",
-		},
+	if tc.wantAnswer == nil {
+		if len(w.Msg.Answer) > 0 {
+			t.Errorf("answer: got %v, want none", w.Msg.Answer[0])
+		}
+		return
 	}
 
-	testCases := []struct {
-		qname      string
-		qtype      uint16
-		wantrcode  int
-		wantAnswer []string
-		wantTTL    uint32
-	}{
-		{
-			qname:      "ip-192-0-2-0.example.com",
-			qtype:      dns.TypeA,
-			wantrcode:  dns.RcodeSuccess,
-			wantAnswer: []string{"192.0.2.0"},
-			wantTTL:    0,
-		},
-		{
-			qname:      "ip-192-0-2-255.example.com",
-			qtype:      dns.TypeA,
-			wantrcode:  dns.RcodeSuccess,
-			wantAnswer: []string{"192.0.2.255"},
-			wantTTL:    0,
-		},
-		{
-			qname:      "ip-203-0-113-100.example.com",
-			qtype:      dns.TypeA,
-			wantrcode:  dns.RcodeServerFailure,
-			wantAnswer: nil,
-			wantTTL:    0,
-		},
-		{
-			qname:      "123.2.0.192.in-addr.arpa.",
-			qtype:      dns.TypePTR,
-			wantrcode:  dns.RcodeSuccess,
-			wantAnswer: []string{"ip-192-0-2-123.example.com."},
-			wantTTL:    0,
-		},
-		{
-			qname:      "ip-2001-db8-abcd--.example.com",
-			qtype:      dns.TypeA,
-			wantrcode:  dns.RcodeServerFailure,
-			wantAnswer: nil,
-			wantTTL:    0,
-		},
+	if len(w.Msg.Answer) == 0 {
+		t.Fatal("answer: got none, want an answer")
 	}
 
-	for i, tc := range testCases {
-		errorMsgPrefix := fmt.Sprintf("Test case %v for '%v' failed. Expected", i, tc.qname)
-		ctx := context.TODO()
-		w := dnstest.NewRecorder(&test.ResponseWriter{})
-		r := new(dns.Msg)
-		r.SetQuestion(tc.qname, tc.qtype)
+	hdr := w.Msg.Answer[0].Header()
+	if hdr.Name != tc.qname {
+		t.Errorf("name: got %s, want %s", hdr.Name, tc.qname)
+	}
+	if hdr.Rrtype != tc.qtype {
+		t.Errorf("rrtype: got %d, want %d", hdr.Rrtype, tc.qtype)
+	}
+	if hdr.Ttl != tc.wantTTL {
+		t.Errorf("ttl: got %d, want %d", hdr.Ttl, tc.wantTTL)
+	}
 
-		rc, err := s.ServeDNS(ctx, w, r)
-
-		if err != nil {
-			t.Errorf("%v no error, but got %v", errorMsgPrefix, err)
+	switch rr := w.Msg.Answer[0].(type) {
+	case *dns.A:
+		if rr.A.String() != tc.wantAnswer[0] {
+			t.Errorf("A record: got %s, want %s", rr.A, tc.wantAnswer[0])
 		}
-		if rc != tc.wantrcode {
-			t.Errorf("%v rcode %v, but got %v", errorMsgPrefix, tc.wantrcode, rc)
+	case *dns.AAAA:
+		if rr.AAAA.String() != tc.wantAnswer[0] {
+			t.Errorf("AAAA record: got %s, want %s", rr.AAAA, tc.wantAnswer[0])
 		}
-		if len(w.Msg.Answer) == 0 && tc.wantAnswer != nil {
-			t.Errorf("%v an answer, but got none", errorMsgPrefix)
-			continue
-		}
-		if tc.wantAnswer == nil {
-			if len(w.Msg.Answer) > 0 {
-				t.Errorf("%v no answer, but got %v", errorMsgPrefix, w.Msg.Answer[0])
-			}
-			continue
-		}
-		if w.Msg.Answer[0].Header().Ttl != 0 {
-			t.Errorf("%v TTL to be 0, but got %v", errorMsgPrefix, w.Msg.Answer[0].Header().Ttl)
-		}
-		if w.Msg.Answer[0].Header().Name != tc.qname {
-			t.Errorf("%v Name to be %s, but got %s", errorMsgPrefix, tc.qname, w.Msg.Answer[0].Header().Name)
-		}
-		if w.Msg.Answer[0].Header().Rrtype != tc.qtype {
-			t.Errorf("%v Type to be %d, but got %d", errorMsgPrefix, tc.qtype, w.Msg.Answer[0].Header().Rrtype)
-		}
-		if w.Msg.Answer[0].Header().Ttl != tc.wantTTL {
-			t.Errorf("%v TTL to be %d, but got %d", errorMsgPrefix, tc.wantTTL, w.Msg.Answer[0].Header().Ttl)
-		}
-		switch tc.qtype {
-		case dns.TypeA:
-			if w.Msg.Answer[0].(*dns.A).A.String() != tc.wantAnswer[0] {
-				t.Errorf("%v answer %s, but got %s", errorMsgPrefix, tc.wantAnswer[0], w.Msg.Answer[0].(*dns.A).A.String())
-			}
-		case dns.TypePTR:
-			if w.Msg.Answer[0].(*dns.PTR).Ptr != tc.wantAnswer[0] {
-				t.Errorf("%v answer %s, but got %s", errorMsgPrefix, tc.wantAnswer[0], w.Msg.Answer[0].(*dns.PTR).Ptr)
-			}
+	case *dns.PTR:
+		if rr.Ptr != tc.wantAnswer[0] {
+			t.Errorf("PTR record: got %s, want %s", rr.Ptr, tc.wantAnswer[0])
 		}
 	}
 }
 
-func TestServeDNSv6(t *testing.T) {
-	ip, ipNet, err := net.ParseCIDR("2001:db8:abcd::/48")
+// runTestCases executes a table of test cases against the given synthetic plugin.
+func runTestCases(t *testing.T, s synthetic, cases []testCase) {
+	t.Helper()
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			w := dnstest.NewRecorder(&test.ResponseWriter{})
+			r := new(dns.Msg)
+			r.SetQuestion(tc.qname, tc.qtype)
+
+			rc, err := s.ServeDNS(context.TODO(), w, r)
+			if err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+
+			assertResponse(t, tc, rc, w)
+		})
+	}
+}
+
+func TestServeDNSv4(t *testing.T) {
+	_, ipNet, err := net.ParseCIDR("192.0.2.0/24")
 	if err != nil {
-		log.Fatal(err)
+		t.Fatal(err)
+	}
+
+	s := synthetic{
+		Next:   test.ErrorHandler(),
+		Config: syntheticConfig{net: []*net.IPNet{ipNet}, forward: "example.com", prefix: "ip-"},
+	}
+
+	runTestCases(t, s, []testCase{
+		{
+			name:       "v4 first address",
+			qname:      "ip-192-0-2-0.example.com",
+			qtype:      dns.TypeA,
+			wantRcode:  dns.RcodeSuccess,
+			wantAnswer: []string{"192.0.2.0"},
+		},
+		{
+			name:       "v4 last address",
+			qname:      "ip-192-0-2-255.example.com",
+			qtype:      dns.TypeA,
+			wantRcode:  dns.RcodeSuccess,
+			wantAnswer: []string{"192.0.2.255"},
+		},
+		{
+			name:      "v4 out of range",
+			qname:     "ip-203-0-113-100.example.com",
+			qtype:     dns.TypeA,
+			wantRcode: dns.RcodeServerFailure,
+		},
+		{
+			name:       "v4 PTR reverse",
+			qname:      "123.2.0.192.in-addr.arpa.",
+			qtype:      dns.TypePTR,
+			wantRcode:  dns.RcodeSuccess,
+			wantAnswer: []string{"ip-192-0-2-123.example.com."},
+		},
+		{
+			name:      "v6 address with v4 config",
+			qname:     "ip-2001-db8-abcd--.example.com",
+			qtype:     dns.TypeA,
+			wantRcode: dns.RcodeServerFailure,
+		},
+	})
+}
+
+func TestServeDNSv6(t *testing.T) {
+	_, ipNet, err := net.ParseCIDR("2001:db8:abcd::/48")
+	if err != nil {
+		t.Fatal(err)
 	}
 
 	s := synthetic{
 		Next: test.ErrorHandler(),
 		Config: syntheticConfig{
-			net:     []*net.IPNet{{IP: ip, Mask: ipNet.Mask}},
+			net:     []*net.IPNet{ipNet},
 			forward: "example.com.",
 			ttl:     1800,
 			prefix:  "ip6-",
 		},
 	}
 
-	testCases := []struct {
-		qname      string
-		qtype      uint16
-		wantrcode  int
-		wantAnswer []string
-		wantTTL    uint32
-	}{
+	runTestCases(t, s, []testCase{
 		{
+			name:       "v6 zero-padded",
 			qname:      "ip6-2001-db8-abcd--.example.com",
 			qtype:      dns.TypeAAAA,
-			wantrcode:  dns.RcodeSuccess,
+			wantRcode:  dns.RcodeSuccess,
 			wantAnswer: []string{"2001:db8:abcd::"},
 			wantTTL:    1800,
 		},
 		{
+			name:       "v6 fully expanded",
 			qname:      "ip6-2001-db8-abcd-1234-4567-890a-bcde-f123.example.com",
 			qtype:      dns.TypeAAAA,
-			wantrcode:  dns.RcodeSuccess,
+			wantRcode:  dns.RcodeSuccess,
 			wantAnswer: []string{"2001:db8:abcd:1234:4567:890a:bcde:f123"},
 			wantTTL:    1800,
 		},
 		{
-			qname:      "ip6-2001-db8-1234--.example.com",
-			qtype:      dns.TypeAAAA,
-			wantrcode:  dns.RcodeServerFailure,
-			wantAnswer: nil,
-			wantTTL:    1800,
+			name:      "v6 out of range",
+			qname:     "ip6-2001-db8-1234--.example.com",
+			qtype:     dns.TypeAAAA,
+			wantRcode: dns.RcodeServerFailure,
 		},
 		{
+			name:       "v6 PTR reverse",
 			qname:      "3.2.1.f.e.d.c.b.a.0.9.8.7.6.5.4.4.3.2.1.d.c.b.a.8.b.d.0.1.0.0.2.ip6.arpa.",
 			qtype:      dns.TypePTR,
-			wantrcode:  dns.RcodeSuccess,
+			wantRcode:  dns.RcodeSuccess,
 			wantAnswer: []string{"ip6-2001-db8-abcd-1234-4567-890a-bcde-f123.example.com."},
 			wantTTL:    1800,
 		},
 		{
-			qname:      "ip6-192-0-2-0.example.com",
-			qtype:      dns.TypeAAAA,
-			wantrcode:  dns.RcodeServerFailure,
-			wantAnswer: nil,
-			wantTTL:    0,
+			name:      "v4 address with v6 config",
+			qname:     "ip6-192-0-2-0.example.com",
+			qtype:     dns.TypeAAAA,
+			wantRcode: dns.RcodeServerFailure,
 		},
-	}
-
-	for i, tc := range testCases {
-		errorMsgPrefix := fmt.Sprintf("Test case %v for '%v' failed. Expected", i, tc.qname)
-		ctx := context.TODO()
-		w := dnstest.NewRecorder(&test.ResponseWriter{})
-		r := new(dns.Msg)
-		r.SetQuestion(tc.qname, tc.qtype)
-
-		rc, err := s.ServeDNS(ctx, w, r)
-
-		if err != nil {
-			t.Errorf("%v no error, but got %v", errorMsgPrefix, err)
-		}
-		if rc != tc.wantrcode {
-			t.Errorf("%v rcode %v, but got %v", errorMsgPrefix, tc.wantrcode, rc)
-		}
-		if len(w.Msg.Answer) == 0 && tc.wantAnswer != nil {
-			t.Errorf("%v an answer, but got none", errorMsgPrefix)
-			continue
-		}
-		if tc.wantAnswer == nil {
-			if len(w.Msg.Answer) > 0 {
-				t.Errorf("%v no answer, but got %v", errorMsgPrefix, w.Msg.Answer[0])
-			}
-			continue
-		}
-		if w.Msg.Answer[0].Header().Name != tc.qname {
-			t.Errorf("%v Name to be %s, but got %s", errorMsgPrefix, tc.qname, w.Msg.Answer[0].Header().Name)
-		}
-		if w.Msg.Answer[0].Header().Rrtype != tc.qtype {
-			t.Errorf("%v Type to be %d, but got %d", errorMsgPrefix, tc.qtype, w.Msg.Answer[0].Header().Rrtype)
-		}
-		if w.Msg.Answer[0].Header().Ttl != tc.wantTTL {
-			t.Errorf("%v TTL to be %d, but got %d", errorMsgPrefix, tc.wantTTL, w.Msg.Answer[0].Header().Ttl)
-		}
-		switch tc.qtype {
-		case dns.TypeA:
-			if w.Msg.Answer[0].(*dns.A).A.String() != tc.wantAnswer[0] {
-				t.Errorf("%v answer %s, but got %s", errorMsgPrefix, tc.wantAnswer[0], w.Msg.Answer[0].(*dns.A).A.String())
-			}
-		case dns.TypePTR:
-			if w.Msg.Answer[0].(*dns.PTR).Ptr != tc.wantAnswer[0] {
-				t.Errorf("%v answer %s, but got %s", errorMsgPrefix, tc.wantAnswer[0], w.Msg.Answer[0].(*dns.PTR).Ptr)
-			}
-		}
-	}
+	})
 }
 
-// MockSuccessPlugin is a mock plugin that always responds with a successful DNS response.
-type MockSuccessPlugin struct{}
+// mockSuccessPlugin always returns a successful answer, simulating a static
+// zone file or upstream resolver that provides authoritative records.
+type mockSuccessPlugin struct{}
 
-// Name returns the plugin name.
-func (m MockSuccessPlugin) Name() string { return "mock" }
+func (m mockSuccessPlugin) Name() string { return "mock" }
+
+func (m mockSuccessPlugin) ServeDNS(_ context.Context, w dns.ResponseWriter, r *dns.Msg) (int, error) {
+	msg := new(dns.Msg)
+	msg.SetReply(r)
+	hdr := dns.RR_Header{
+		Name:   r.Question[0].Name,
+		Rrtype: r.Question[0].Qtype,
+		Class:  r.Question[0].Qclass,
+	}
 
-// ServeDNS simulates a successful DNS response.
-func (m MockSuccessPlugin) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (int, error) {
-	m1 := new(dns.Msg)
-	m1.SetReply(r)
-	hdr := dns.RR_Header{Name: r.Question[0].Name, Rrtype: r.Question[0].Qtype, Class: r.Question[0].Qclass, Ttl: 0}
 	switch r.Question[0].Qtype {
 	case dns.TypeA:
-		m1.Answer = append(m1.Answer, &dns.A{Hdr: hdr, A: net.ParseIP("192.0.2.100")})
+		msg.Answer = append(msg.Answer, &dns.A{Hdr: hdr, A: net.ParseIP("192.0.2.100")})
 	case dns.TypeAAAA:
-		m1.Answer = append(m1.Answer, &dns.AAAA{Hdr: hdr, AAAA: net.ParseIP("2001:db8::100")})
+		msg.Answer = append(msg.Answer, &dns.AAAA{Hdr: hdr, AAAA: net.ParseIP("2001:db8::100")})
 	}
-	w.WriteMsg(m1)
+
+	w.WriteMsg(msg)
 	return dns.RcodeSuccess, nil
 }
 
-func TestServeDNSNextPluginRespondsSuccess(t *testing.T) {
-	ip, ipNet, err := net.ParseCIDR("192.0.2.0/24")
+// TestServeDNSNextPluginPrecedence verifies that answers from the next plugin
+// in the chain take priority over synthetic responses for reverse lookups,
+// while forward lookups still produce synthetic answers directly.
+func TestServeDNSNextPluginPrecedence(t *testing.T) {
+	_, ipNet4, err := net.ParseCIDR("192.0.2.0/24")
 	if err != nil {
-		log.Fatal(err)
+		t.Fatal(err)
 	}
-	ip6, ipNet6, err := net.ParseCIDR("2001:db8:abcd::/48")
+	_, ipNet6, err := net.ParseCIDR("2001:db8:abcd::/48")
 	if err != nil {
-		log.Fatal(err)
+		t.Fatal(err)
 	}
 
 	s := synthetic{
-		Next: MockSuccessPlugin{},
+		Next: mockSuccessPlugin{},
 		Config: syntheticConfig{
-			net:     []*net.IPNet{{IP: ip, Mask: ipNet.Mask}, {IP: ip6, Mask: ipNet6.Mask}},
+			net:     []*net.IPNet{ipNet4, ipNet6},
 			forward: "example.com",
 			prefix:  "ip-",
 		},
 	}
 
-	testCases := []struct {
-		qname      string
-		qtype      uint16
-		wantrcode  int
-		wantAnswer []string
-		wantTTL    uint32
-	}{
+	runTestCases(t, s, []testCase{
 		{
+			name:       "forward v4 synthetic wins",
 			qname:      "ip-192-0-2-1.example.com",
 			qtype:      dns.TypeA,
-			wantrcode:  dns.RcodeSuccess,
+			wantRcode:  dns.RcodeSuccess,
 			wantAnswer: []string{"192.0.2.1"},
-			wantTTL:    0,
 		},
 		{
+			name:       "forward non-synthetic uses next plugin",
 			qname:      "foobar.example.com",
 			qtype:      dns.TypeA,
-			wantrcode:  dns.RcodeSuccess,
+			wantRcode:  dns.RcodeSuccess,
 			wantAnswer: []string{"192.0.2.100"},
-			wantTTL:    0,
 		},
 		{
+			name:       "forward v6 synthetic wins",
 			qname:      "ip-2001-db8-abcd--1.example.com",
 			qtype:      dns.TypeAAAA,
-			wantrcode:  dns.RcodeSuccess,
+			wantRcode:  dns.RcodeSuccess,
 			wantAnswer: []string{"2001:db8:abcd::1"},
-			wantTTL:    0,
 		},
 		{
+			name:       "forward non-synthetic AAAA uses next plugin",
 			qname:      "foobar.example.com",
 			qtype:      dns.TypeAAAA,
-			wantrcode:  dns.RcodeSuccess,
-			wantAnswer: []string{"2001:db8:abcd::100"},
-			wantTTL:    0,
+			wantRcode:  dns.RcodeSuccess,
+			wantAnswer: []string{"2001:db8::100"},
 		},
 		{
-			qname:      "ip-2001-db8-abcd--1.example.com",
-			qtype:      dns.TypeA,
-			wantrcode:  dns.RcodeSuccess,
-			wantAnswer: nil,
-			wantTTL:    0,
+			name:      "v6 address queried as A returns empty",
+			qname:     "ip-2001-db8-abcd--1.example.com",
+			qtype:     dns.TypeA,
+			wantRcode: dns.RcodeSuccess,
 		},
 		{
-			qname:      "ip-192-0-2-1.example.com",
-			qtype:      dns.TypeAAAA,
-			wantrcode:  dns.RcodeSuccess,
-			wantAnswer: nil,
-			wantTTL:    0,
+			name:      "v4 address queried as AAAA returns empty",
+			qname:     "ip-192-0-2-1.example.com",
+			qtype:     dns.TypeAAAA,
+			wantRcode: dns.RcodeSuccess,
 		},
 		{
+			name:       "PTR v6 synthetic (mock has no PTR)",
 			qname:      "3.2.1.f.e.d.c.b.a.0.9.8.7.6.5.4.4.3.2.1.d.c.b.a.8.b.d.0.1.0.0.2.ip6.arpa.",
 			qtype:      dns.TypePTR,
-			wantrcode:  dns.RcodeSuccess,
-			wantAnswer: []string{"ip6-2001-db8-abcd-1234-4567-890a-bcde-f123.example.com."},
-			wantTTL:    0,
+			wantRcode:  dns.RcodeSuccess,
+			wantAnswer: []string{"ip-2001-db8-abcd-1234-4567-890a-bcde-f123.example.com."},
 		},
 		{
+			name:       "PTR v4 synthetic (mock has no PTR)",
 			qname:      "123.2.0.192.in-addr.arpa.",
 			qtype:      dns.TypePTR,
-			wantrcode:  dns.RcodeSuccess,
+			wantRcode:  dns.RcodeSuccess,
 			wantAnswer: []string{"ip-192-0-2-123.example.com."},
-			wantTTL:    0,
 		},
-	}
-
-	for i, tc := range testCases {
-		errorMsgPrefix := fmt.Sprintf("Test case %v for '%v' failed. Expected", i, tc.qname)
-		ctx := context.TODO()
-		w := dnstest.NewRecorder(&test.ResponseWriter{})
-		r := new(dns.Msg)
-		r.SetQuestion(tc.qname, tc.qtype)
-
-		rc, err := s.ServeDNS(ctx, w, r)
-
-		if err != nil {
-			t.Errorf("%v no error, but got %v", errorMsgPrefix, err)
-		}
-		if rc != tc.wantrcode {
-			t.Errorf("%v rcode %v, but got %v", errorMsgPrefix, tc.wantrcode, rc)
-		}
-		if len(w.Msg.Answer) == 0 && tc.wantAnswer != nil {
-			t.Errorf("%v an answer, but got none", errorMsgPrefix)
-			continue
-		}
-		if tc.wantAnswer == nil {
-			if len(w.Msg.Answer) > 0 {
-				t.Errorf("%v no answer, but got %v", errorMsgPrefix, w.Msg.Answer[0])
-			}
-			continue
-		}
-		if w.Msg.Answer[0].Header().Ttl != 0 {
-			t.Errorf("%v TTL to be 0, but got %v", errorMsgPrefix, w.Msg.Answer[0].Header().Ttl)
-		}
-		if w.Msg.Answer[0].Header().Name != tc.qname {
-			t.Errorf("%v Name to be %s, but got %s", errorMsgPrefix, tc.qname, w.Msg.Answer[0].Header().Name)
-		}
-		if w.Msg.Answer[0].Header().Rrtype != tc.qtype {
-			t.Errorf("%v Type to be %d, but got %d", errorMsgPrefix, tc.qtype, w.Msg.Answer[0].Header().Rrtype)
-		}
-		if w.Msg.Answer[0].Header().Ttl != tc.wantTTL {
-			t.Errorf("%v TTL to be %d, but got %d", errorMsgPrefix, tc.wantTTL, w.Msg.Answer[0].Header().Ttl)
-		}
-		switch tc.qtype {
-		case dns.TypeA:
-			if w.Msg.Answer[0].(*dns.A).A.String() != tc.wantAnswer[0] {
-				t.Errorf("%v answer %s, but got %s", errorMsgPrefix, tc.wantAnswer[0], w.Msg.Answer[0].(*dns.A).A.String())
-			}
-		}
-	}
+	})
 }