kubernetes: Add zone filtering to fallthrough (#1353)

* Add zone filtering to fallthrough

* Doh. gofmt

* Update documentation

plugin.md

@@ -71,6 +71,9 @@ reverse cases and **all other** request are handled by the backing plugin. This
 "fallthrough" does. To keep things explicit we've opted that plugins implement such behavior
 should implement a `fallthrough` keyword.
 
+The `fallthrough` directive should optionally accept a list of zones. Only queries for records
+in one of those zones should be allowed to fallthrough.
+
 ## Qualifying for main repo
 
 Plugins for CoreDNS can live out-of-tree, `plugin.cfg` defaults to CoreDNS' repo but other

plugin/kubernetes/README.md

@@ -38,7 +38,7 @@ kubernetes [ZONES...] {
     endpoint_pod_names
     upstream ADDRESS...
     ttl TTL
-    fallthrough
+    fallthrough [ZONES...]
 }
 ```
 
@@ -85,9 +85,12 @@ kubernetes [ZONES...] {
   to a file structured like resolv.conf.
 * `ttl` allows you to set a custom TTL for responses. The default (and allowed minimum) is to use
   5 seconds, the maximum is capped at 3600 seconds.
-* `fallthrough`  If a query for a record in the cluster zone results in NXDOMAIN, normally that is
-  what the response will be. However, if you specify this option, the query will instead be passed
-  on down the plugin chain, which can include another plugin to handle the query.
+* `fallthrough` **[ZONES...]** If a query for a record in the zones for which the plugin is authoritative
+  results in NXDOMAIN, normally that is what the response will be. However, if you specify this option,
+  the query will instead be passed on down the plugin chain, which can include another plugin to handle
+  the query. If **[ZONES...]** is omitted, then fallthrough happens for all zones for which the plugin
+  is authoritative. If specific zones are listed (for example `in-addr.arpa` and `ip6.arpa`), then only
+  queries for those zones will be subject to fallthrough.
 
 ## Health
 

plugin/kubernetes/handler.go

@@ -59,7 +59,7 @@ func (k Kubernetes) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.M
 	}
 
 	if k.IsNameError(err) {
-		if k.Fallthrough {
+		if plugin.Fallthrough(k.Fallthrough, state.Name()) {
 			return plugin.NextOrFailure(k.Name(), k.Next, ctx, w, r)
 		}
 		return plugin.BackendError(&k, zone, dns.RcodeNameError, state, nil /* err */, plugin.Options{})

plugin/kubernetes/kubernetes.go

@@ -40,7 +40,7 @@ type Kubernetes struct {
 	Namespaces       map[string]bool
 	podMode          string
 	endpointNameMode bool
-	Fallthrough      bool
+	Fallthrough      *[]string // nil = disabled, empty = all zones, o/w zones
 	ttl              uint32
 
 	primaryZoneIndex   int

plugin/kubernetes/setup.go

@@ -172,12 +172,8 @@ func kubernetesParse(c *caddy.Controller) (*Kubernetes, dnsControlOpts, error) {
 				}
 				return nil, opts, c.ArgErr()
 			case "fallthrough":
-				args := c.RemainingArgs()
-				if len(args) == 0 {
-					k8s.Fallthrough = true
-					continue
-				}
-				return nil, opts, c.ArgErr()
+				zones := c.RemainingArgs()
+				k8s.Fallthrough = &zones
 			case "upstream":
 				args := c.RemainingArgs()
 				if len(args) == 0 {

plugin/kubernetes/setup_test.go

@@ -19,7 +19,7 @@ func TestKubernetesParse(t *testing.T) {
 		expectedResyncPeriod  time.Duration // expected resync period value
 		expectedLabelSelector string        // expected label selector value
 		expectedPodMode       string
-		expectedFallthrough   bool
+		expectedFallthrough   *[]string
 		expectedUpstreams     []string
 	}{
 		// positive
@@ -32,7 +32,7 @@ func TestKubernetesParse(t *testing.T) {
 			defaultResyncPeriod,
 			"",
 			podModeDisabled,
-			false,
+			nil,
 			nil,
 		},
 		{
@@ -44,7 +44,7 @@ func TestKubernetesParse(t *testing.T) {
 			defaultResyncPeriod,
 			"",
 			podModeDisabled,
-			false,
+			nil,
 			nil,
 		},
 		{
@@ -57,7 +57,7 @@ func TestKubernetesParse(t *testing.T) {
 			defaultResyncPeriod,
 			"",
 			podModeDisabled,
-			false,
+			nil,
 			nil,
 		},
 		{
@@ -71,7 +71,7 @@ func TestKubernetesParse(t *testing.T) {
 			defaultResyncPeriod,
 			"",
 			podModeDisabled,
-			false,
+			nil,
 			nil,
 		},
 		{
@@ -85,7 +85,7 @@ func TestKubernetesParse(t *testing.T) {
 			defaultResyncPeriod,
 			"",
 			podModeDisabled,
-			false,
+			nil,
 			nil,
 		},
 		{
@@ -99,7 +99,7 @@ func TestKubernetesParse(t *testing.T) {
 			defaultResyncPeriod,
 			"",
 			podModeDisabled,
-			false,
+			nil,
 			nil,
 		},
 		{
@@ -113,7 +113,7 @@ func TestKubernetesParse(t *testing.T) {
 			30 * time.Second,
 			"",
 			podModeDisabled,
-			false,
+			nil,
 			nil,
 		},
 		{
@@ -127,7 +127,7 @@ func TestKubernetesParse(t *testing.T) {
 			15 * time.Minute,
 			"",
 			podModeDisabled,
-			false,
+			nil,
 			nil,
 		},
 		{
@@ -141,7 +141,7 @@ func TestKubernetesParse(t *testing.T) {
 			defaultResyncPeriod,
 			"environment=prod",
 			podModeDisabled,
-			false,
+			nil,
 			nil,
 		},
 		{
@@ -155,7 +155,7 @@ func TestKubernetesParse(t *testing.T) {
 			defaultResyncPeriod,
 			"application=nginx,environment in (production,qa,staging)",
 			podModeDisabled,
-			false,
+			nil,
 			nil,
 		},
 		{
@@ -173,7 +173,7 @@ func TestKubernetesParse(t *testing.T) {
 			15 * time.Minute,
 			"application=nginx,environment in (production,qa,staging)",
 			podModeDisabled,
-			true,
+			&[]string{},
 			nil,
 		},
 		// negative
@@ -188,7 +188,7 @@ func TestKubernetesParse(t *testing.T) {
 			defaultResyncPeriod,
 			"",
 			podModeDisabled,
-			false,
+			nil,
 			nil,
 		},
 		{
@@ -202,7 +202,7 @@ func TestKubernetesParse(t *testing.T) {
 			defaultResyncPeriod,
 			"",
 			podModeDisabled,
-			false,
+			nil,
 			nil,
 		},
 		{
@@ -216,7 +216,7 @@ func TestKubernetesParse(t *testing.T) {
 			0 * time.Minute,
 			"",
 			podModeDisabled,
-			false,
+			nil,
 			nil,
 		},
 		{
@@ -230,7 +230,7 @@ func TestKubernetesParse(t *testing.T) {
 			0 * time.Second,
 			"",
 			podModeDisabled,
-			false,
+			nil,
 			nil,
 		},
 		{
@@ -244,7 +244,7 @@ func TestKubernetesParse(t *testing.T) {
 			0 * time.Second,
 			"",
 			podModeDisabled,
-			false,
+			nil,
 			nil,
 		},
 		{
@@ -258,7 +258,7 @@ func TestKubernetesParse(t *testing.T) {
 			0 * time.Second,
 			"",
 			podModeDisabled,
-			false,
+			nil,
 			nil,
 		},
 		{
@@ -272,7 +272,7 @@ func TestKubernetesParse(t *testing.T) {
 			0 * time.Second,
 			"",
 			podModeDisabled,
-			false,
+			nil,
 			nil,
 		},
 		// pods disabled
@@ -287,7 +287,7 @@ func TestKubernetesParse(t *testing.T) {
 			defaultResyncPeriod,
 			"",
 			podModeDisabled,
-			false,
+			nil,
 			nil,
 		},
 		// pods insecure
@@ -302,7 +302,7 @@ func TestKubernetesParse(t *testing.T) {
 			defaultResyncPeriod,
 			"",
 			podModeInsecure,
-			false,
+			nil,
 			nil,
 		},
 		// pods verified
@@ -317,7 +317,7 @@ func TestKubernetesParse(t *testing.T) {
 			defaultResyncPeriod,
 			"",
 			podModeVerified,
-			false,
+			nil,
 			nil,
 		},
 		// pods invalid
@@ -332,22 +332,22 @@ func TestKubernetesParse(t *testing.T) {
 			defaultResyncPeriod,
 			"",
 			podModeVerified,
-			false,
+			nil,
 			nil,
 		},
-		// fallthrough invalid
+		// fallthrough with zones
 		{
 			`kubernetes coredns.local {
-	fallthrough junk
+	fallthrough ip6.arpa inaddr.arpa foo.com
 }`,
-			true,
+			false,
 			"rong argument count",
-			-1,
+			1,
 			0,
 			defaultResyncPeriod,
 			"",
 			podModeDisabled,
-			false,
+			&[]string{"ip6.arpa", "inaddr.arpa", "foo.com"},
 			nil,
 		},
 		// Valid upstream
@@ -362,7 +362,7 @@ func TestKubernetesParse(t *testing.T) {
 			defaultResyncPeriod,
 			"",
 			podModeDisabled,
-			false,
+			nil,
 			[]string{"13.14.15.16:53"},
 		},
 		// Invalid upstream
@@ -377,7 +377,7 @@ func TestKubernetesParse(t *testing.T) {
 			defaultResyncPeriod,
 			"",
 			podModeDisabled,
-			false,
+			nil,
 			nil,
 		},
 	}
@@ -444,8 +444,22 @@ func TestKubernetesParse(t *testing.T) {
 
 		// fallthrough
 		foundFallthrough := k8sController.Fallthrough
-		if foundFallthrough != test.expectedFallthrough {
-			t.Errorf("Test %d: Expected kubernetes controller to be initialized with fallthrough '%v'. Instead found fallthrough '%v' for input '%s'", i, test.expectedFallthrough, foundFallthrough, test.input)
+		if foundFallthrough != nil {
+			failed := false
+			if test.expectedFallthrough == nil {
+				failed = true
+			} else if len(*foundFallthrough) != len(*test.expectedFallthrough) {
+				failed = true
+			} else {
+				for i := range *foundFallthrough {
+					if (*foundFallthrough)[i] != (*test.expectedFallthrough)[i] {
+						failed = true
+					}
+				}
+			}
+			if failed {
+				t.Errorf("Test %d: Expected kubernetes controller to be initialized with fallthrough '%v'. Instead found fallthrough '%v' for input '%s'", i, test.expectedFallthrough, foundFallthrough, test.input)
+			}
 		}
 		// upstream
 		foundUpstreams := k8sController.Proxy.Upstreams

plugin/plugin.go

@@ -83,6 +83,21 @@ func NextOrFailure(name string, next Handler, ctx context.Context, w dns.Respons
 	return dns.RcodeServerFailure, Error(name, errors.New("no next plugin found"))
 }
 
+// Fallthrough handles the fallthrough logic used in plugins that support it
+func Fallthrough(ftzones *[]string, qname string) bool {
+	if ftzones == nil {
+		return false
+	}
+	if len(*ftzones) == 0 {
+		return true
+	}
+	zone := Zones(*ftzones).Matches(qname)
+	if zone != "" {
+		return true
+	}
+	return false
+}
+
 // ClientWrite returns true if the response has been written to the client.
 // Each plugin to adhire to this protocol.
 func ClientWrite(rcode int) bool {

plugin/plugin_test.go

 package plugin
+
+import "testing"
+
+func TestFallthrough(t *testing.T) {
+	if Fallthrough(nil, "foo.com.") {
+		t.Errorf("Expected false, got true for nil fallthrough")
+	}
+
+	if !Fallthrough(&[]string{}, "foo.net.") {
+		t.Errorf("Expected true, got false for all zone fallthrough")
+	}
+
+	if Fallthrough(&[]string{"foo.com", "bar.com"}, "foo.net") {
+		t.Errorf("Expected false, got true for non-matching fallthrough zone")
+	}
+
+	if !Fallthrough(&[]string{"foo.com.", "bar.com."}, "bar.com.") {
+		t.Errorf("Expected true, got false for matching fallthrough zone")
+	}
+}