plugin/forward: Forward NODATA responses to Next handler (#8065)

plugin/forward/README.md

@@ -111,6 +111,7 @@ forward FROM TO... {
   at least greater than the expected *upstream query rate* * *latency* of the upstream servers.
   As an upper bound for **MAX**, consider that each concurrent query will use about 2kb of memory.
 * `next` If the `RCODE` (i.e. `NXDOMAIN`) is returned by the remote then execute the next plugin. If no next plugin is defined, or the next plugin is not a `forward` plugin, this setting is ignored
+* `next_on_nodata` If `NOERROR` is returned by the remote, but an empty answer section (`NODATA`) was provided, execute the next `forward` plugin, if configured.
 * `failfast_all_unhealthy_upstreams` - determines the handling of requests when all upstream servers are unhealthy and unresponsive to health checks. Enabling this option will immediately return SERVFAIL responses for all requests. By default, requests are sent to a random upstream.
 * `failover` - By default when a DNS lookup fails to return a DNS response (e.g. timeout), _forward_ will attempt a lookup on the next upstream server. The `failover` option will make _forward_ do the same for any response with a response code matching an `RCODE` ( e.g. `SERVFAIL`、`REFUSED`). `NOERROR` cannot be used. If all upstreams have been tried, the response from the last attempt is returned.
 

plugin/forward/forward.go

@@ -44,6 +44,7 @@ type Forward struct {
 	ignored []string
 
 	nextAlternateRcodes []int
+	nextOnNodata        bool
 
 	tlsConfig                  *tls.Config
 	tlsServerName              string
@@ -245,6 +246,14 @@ func (f *Forward) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg
 			}
 		}
 
+		if f.nextOnNodata && f.Next != nil {
+			if ret.Rcode == dns.RcodeSuccess && isEmpty(ret) {
+				if _, ok := f.Next.(*Forward); ok {
+					return plugin.NextOrFailure(f.Name(), f.Next, ctx, w, r)
+				}
+			}
+		}
+
 		w.WriteMsg(ret)
 		return 0, nil
 	}
@@ -277,6 +286,19 @@ func (f *Forward) isAllowedDomain(name string) bool {
 	return true
 }
 
+func isEmpty(r *dns.Msg) bool {
+	if len(r.Answer) == 0 {
+		return true
+	}
+
+	for _, r := range r.Answer {
+		if r != nil {
+			return false
+		}
+	}
+	return true
+}
+
 // ForceTCP returns if TCP is forced to be used even when the request comes in over UDP.
 func (f *Forward) ForceTCP() bool { return f.opts.ForceTCP }
 

plugin/forward/forward_test.go

@@ -2,6 +2,7 @@ package forward
 
 import (
 	"context"
+	"fmt"
 	"net"
 	"strings"
 	"testing"
@@ -11,8 +12,10 @@ import (
 	"github.com/coredns/caddy/caddyfile"
 	"github.com/coredns/coredns/core/dnsserver"
 	"github.com/coredns/coredns/plugin/dnstap"
+	"github.com/coredns/coredns/plugin/pkg/dnstest"
 	"github.com/coredns/coredns/plugin/pkg/proxy"
 	"github.com/coredns/coredns/plugin/pkg/transport"
+	"github.com/coredns/coredns/plugin/test"
 
 	"github.com/miekg/dns"
 	"github.com/opentracing/opentracing-go"
@@ -157,3 +160,81 @@ func TestForward_Regression_NoBusyLoop(t *testing.T) {
 		})
 	}
 }
+
+func TestForward_NextOnNodata(t *testing.T) {
+	tests := []struct {
+		name         string
+		nextOnNodata bool
+	}{
+		{name: "serveEmpty", nextOnNodata: false},
+		{name: "nextNotEmpty", nextOnNodata: true},
+	}
+
+	s1 := dnstest.NewMultipleServer(func(w dns.ResponseWriter, r *dns.Msg) {
+		ret := new(dns.Msg)
+		ret.SetReply(r)
+		w.WriteMsg(ret)
+	})
+	s2 := dnstest.NewMultipleServer(func(w dns.ResponseWriter, r *dns.Msg) {
+		ret := new(dns.Msg)
+		ret.SetReply(r)
+		ret.Answer = append(ret.Answer, test.A("example.org. IN A 127.0.0.1"))
+		w.WriteMsg(ret)
+	})
+	defer s1.Close()
+	defer s2.Close()
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			var config string
+			if tc.nextOnNodata {
+				config = `
+					forward . %s {
+						next_on_nodata
+					}
+					forward . %s
+					`
+			} else {
+				config = `
+					forward . %s
+					forward . %s
+					`
+			}
+			c := caddy.NewTestController("dns", fmt.Sprintf(config, s1.Addr, s2.Addr))
+			fs, err := parseForward(c)
+			if err != nil {
+				t.Errorf("Failed to create forwarder: %s", err)
+			}
+			if x := len(fs); x != 2 {
+				t.Errorf("Failed to create two forward instances")
+			}
+			f := fs[0]
+			f.Next = fs[1]
+			f.OnStartup()
+			defer f.OnShutdown()
+
+			m := new(dns.Msg)
+			m.SetQuestion("example.org.", dns.TypeA)
+			rec := dnstest.NewRecorder(&test.ResponseWriter{})
+
+			if _, err := f.ServeDNS(context.TODO(), rec, m); err != nil {
+				t.Fatal("Expected to receive reply, but didn't")
+			}
+			if x := rec.Rcode; x != dns.RcodeSuccess {
+				t.Errorf("Expected %v, got %+v instead", dns.RcodeSuccess, rec)
+			}
+			if tc.nextOnNodata {
+				if x := len(rec.Msg.Answer); x != 1 {
+					t.Errorf("Expected answer, got %d instead", x)
+				}
+				if x := rec.Msg.Answer[0].Header().Name; x != "example.org." {
+					t.Errorf("Expected %s, got %s", "example.org.", x)
+				}
+			} else {
+				if x := len(rec.Msg.Answer); x != 0 {
+					t.Errorf("Expected zero length answer, got %d instead", x)
+				}
+			}
+		})
+	}
+}

plugin/forward/setup.go

@@ -395,6 +395,11 @@ func parseBlock(c *caddy.Controller, f *Forward) error {
 
 			f.nextAlternateRcodes = append(f.nextAlternateRcodes, rc)
 		}
+	case "next_on_nodata":
+		if c.NextArg() {
+			return c.ArgErr()
+		}
+		f.nextOnNodata = true
 	case "failfast_all_unhealthy_upstreams":
 		args := c.RemainingArgs()
 		if len(args) != 0 {