fix(dnsserver): allow view server blocks in any declaration order (#8001)

When using the view plugin, filtered and unfiltered server blocks can
share the same zone and port. The zone overlap validation rejected this
configuration when the unfiltered block was not declared last, because
filtered configs treated an already-registered zone as an error.

Skip the 'already defined' check for configs that have filter functions,
since they are expected to coexist with an unfiltered catch-all block on
the same zone/port.

Fixes #7733

Signed-off-by: umut-polat <52835619+umut-polat@users.noreply.github.com>

core/dnsserver/register.go

@@ -237,11 +237,13 @@ func (h *dnsContext) validateZonesAndListeningAddresses() error {
 			akey := zoneAddr{Transport: conf.Transport, Zone: conf.Zone, Address: h, Port: conf.Port}
 			var existZone, overlapZone *zoneAddr
 			if len(conf.FilterFuncs) > 0 {
-				// This config has filters. Check for overlap with other (unfiltered) configs.
-				existZone, overlapZone = checker.check(akey)
+				// This config has filters (e.g. view plugin). It is allowed to
+				// share a zone/port with an unfiltered server block, so we only
+				// check without registering and skip the "already defined" error.
+				_, overlapZone = checker.check(akey)
 			} else {
-				// This config has no filters. Check for overlap with other (unfiltered) configs,
-				// and register the zone to prevent subsequent zones from overlapping with it.
+				// This config has no filters. Check for overlap with other
+				// unfiltered configs and register the zone.
 				existZone, overlapZone = checker.registerAndCheck(akey)
 			}
 			if existZone != nil {

plugin/view/README.md

@@ -25,6 +25,15 @@ view NAME {
 
 For expression syntax and examples, see the Expressions and Examples sections.
 
+## Server Block Ordering
+
+Server blocks sharing the same zone and port are evaluated **top to bottom**. The first block whose
+view expression matches (or that has no view) handles the query. An unfiltered catch-all block
+declared *before* a filtered block will shadow it, because the catch-all matches every query.
+
+To get the expected split-DNS behavior, declare all filtered (view) blocks first and the unfiltered
+catch-all block last.
+
 ## Examples
 
 Implement CIDR based split DNS routing.  This will return a different

test/view_test.go

@@ -162,3 +162,134 @@ func viewTest(t *testing.T, testName, addr, qname string, qtype uint16, expectRc
 		}
 	})
 }
+
+func TestViewServerBlockOrdering(t *testing.T) {
+	// Verify that filtered and unfiltered server blocks sharing the same
+	// zone/port start without error regardless of declaration order, and
+	// that queries are routed correctly.
+	//
+	// Declaration order matters: server blocks are evaluated top-to-bottom
+	// and the first block whose filter matches (or has no filter) handles
+	// the query. An unfiltered block declared before a filtered block will
+	// catch all queries, shadowing the filtered block.
+	//
+	// See https://github.com/coredns/coredns/issues/7733
+
+	corefile := `example.org:0 {
+		erratic
+	}`
+	tmp, addr, _, err := CoreDNSServerAndPorts(corefile)
+	if err != nil {
+		t.Fatalf("Could not get CoreDNS serving instance: %s", err)
+	}
+	port := addr[strings.LastIndex(addr, ":")+1:]
+	tmp.Stop()
+
+	t.Run("filtered blocks before unfiltered", func(t *testing.T) {
+		// Filtered blocks are listed first, unfiltered catch-all is last.
+		// Each view handles its matching queries; the catch-all handles the rest.
+		corefile := `
+      order-test:` + port + ` {
+        view v-a {
+          expr type() == 'A'
+        }
+        hosts {
+          1.2.3.4 test.order-test
+        }
+      }
+      order-test:` + port + ` {
+        view v-aaaa {
+          expr type() == 'AAAA'
+        }
+        hosts {
+          1:2:3::4 test.order-test
+        }
+      }
+      order-test:` + port + ` {
+        hosts {
+          5.6.7.8 test.order-test
+        }
+      }
+    `
+		i, addr, _, err := CoreDNSServerAndPorts(corefile)
+		if err != nil {
+			t.Fatalf("Could not start server: %s", err)
+		}
+		defer i.Stop()
+
+		viewTest(t, "A routed to v-a", addr, "test.order-test.", dns.TypeA, dns.RcodeSuccess,
+			[]dns.RR{test.A("test.order-test.	303	IN	A	1.2.3.4")})
+		viewTest(t, "AAAA routed to v-aaaa", addr, "test.order-test.", dns.TypeAAAA, dns.RcodeSuccess,
+			[]dns.RR{test.AAAA("test.order-test.	303	IN	AAAA	1:2:3::4")})
+	})
+
+	t.Run("unfiltered block first", func(t *testing.T) {
+		// Unfiltered block is declared first. It matches all queries, so the
+		// filtered block below it is effectively shadowed.
+		corefile := `
+      order-test2:` + port + ` {
+        hosts {
+          5.6.7.8 test.order-test2
+        }
+      }
+      order-test2:` + port + ` {
+        view v-a {
+          expr type() == 'A'
+        }
+        hosts {
+          1.2.3.4 test.order-test2
+        }
+      }
+    `
+		i, addr, _, err := CoreDNSServerAndPorts(corefile)
+		if err != nil {
+			t.Fatalf("Could not start server: %s", err)
+		}
+		defer i.Stop()
+
+		// The unfiltered block catches everything, so A goes to it (5.6.7.8).
+		viewTest(t, "A hits unfiltered", addr, "test.order-test2.", dns.TypeA, dns.RcodeSuccess,
+			[]dns.RR{test.A("test.order-test2.	303	IN	A	5.6.7.8")})
+	})
+
+	t.Run("unfiltered block in the middle", func(t *testing.T) {
+		// A filtered block, then unfiltered, then another filtered block.
+		// The first view catches A queries. The unfiltered block catches
+		// everything else, shadowing the second filtered block.
+		corefile := `
+      order-test3:` + port + ` {
+        view v-a {
+          expr type() == 'A'
+        }
+        hosts {
+          1.2.3.4 test.order-test3
+        }
+      }
+      order-test3:` + port + ` {
+        hosts {
+          5.6.7.8 test.order-test3
+        }
+      }
+      order-test3:` + port + ` {
+        view v-aaaa {
+          expr type() == 'AAAA'
+        }
+        hosts {
+          1:2:3::4 test.order-test3
+        }
+      }
+    `
+		i, addr, _, err := CoreDNSServerAndPorts(corefile)
+		if err != nil {
+			t.Fatalf("Could not start server: %s", err)
+		}
+		defer i.Stop()
+
+		// A is caught by v-a (first block).
+		viewTest(t, "A routed to v-a", addr, "test.order-test3.", dns.TypeA, dns.RcodeSuccess,
+			[]dns.RR{test.A("test.order-test3.	303	IN	A	1.2.3.4")})
+		// MX has no matching view, hits the unfiltered block -> 5.6.7.8 (hosts only has A).
+		// AAAA view is shadowed by unfiltered, so AAAA also hits unfiltered.
+		viewTest(t, "AAAA hits unfiltered (shadowed view)", addr, "test.order-test3.", dns.TypeAAAA, dns.RcodeSuccess, nil)
+	})
+}