plugin/cache: fix negative cache masking cases (#3744)

* fix negative cache masking cases Signed-off-by: Chris O'Haver <cohaver@infoblox.com> * remove unecessary param Signed-off-by: Chris O'Haver <cohaver@infoblox.com>
2020-03-20 05:05:09 -04:00 · 2020-03-20 05:05:09 -04:00 · 40c7b9174b
commit 40c7b9174b
parent d18b48e36c
3 changed files with 107 additions and 3 deletions
--- a/plugin/cache/cache_test.go
+++ b/plugin/cache/cache_test.go
@ -296,6 +296,92 @@ func TestServeFromStaleCache(t *testing.T) {
 	}
 }

+func TestNegativeStaleMaskingPositiveCache(t *testing.T) {
+	c := New()
+	c.staleUpTo = time.Minute * 10
+	c.Next = nxDomainBackend(60)
+
+	req := new(dns.Msg)
+	qname := "cached.org."
+	req.SetQuestion(qname, dns.TypeA)
+	ctx := context.TODO()
+
+	// Add an entry to Negative Cache": cached.org. = NXDOMAIN
+	expectedResult := dns.RcodeNameError
+	if ret, _ := c.ServeDNS(ctx, &test.ResponseWriter{}, req); ret != expectedResult {
+		t.Errorf("Test 0 Negative Cache Population: expecting %v; got %v", expectedResult, ret)
+	}
+
+	// Confirm item was added to negative cache and not to positive cache
+	if c.ncache.Len() == 0 {
+		t.Errorf("Test 0 Negative Cache Population: item not added to negative cache")
+	}
+	if c.pcache.Len() != 0 {
+		t.Errorf("Test 0 Negative Cache Population: item added to positive cache")
+	}
+
+	// Set the Backend to return non-cachable errors only
+	c.Next = plugin.HandlerFunc(func(context.Context, dns.ResponseWriter, *dns.Msg) (int, error) {
+		return 255, nil // Below, a 255 means we tried querying upstream.
+	})
+
+	// Confirm we get the NXDOMAIN from the negative cache, not the error form the backend
+	rec := dnstest.NewRecorder(&test.ResponseWriter{})
+	req = new(dns.Msg)
+	req.SetQuestion(qname, dns.TypeA)
+	expectedResult = dns.RcodeNameError
+	if c.ServeDNS(ctx, rec, req); rec.Rcode != expectedResult {
+		t.Errorf("Test 1 NXDOMAIN from Negative Cache: expecting %v; got %v", expectedResult, rec.Rcode)
+	}
+
+	// Jump into the future beyond when the negative cache item would go stale
+	// but before the item goes rotten (exceeds serve stale time)
+	c.now = func() time.Time { return time.Now().Add(time.Duration(5) * time.Minute) }
+
+	// Set Backend to return a positive NOERROR + A record response
+	c.Next = BackendHandler()
+
+	// Make a query for the stale cache item
+	rec = dnstest.NewRecorder(&test.ResponseWriter{})
+	req = new(dns.Msg)
+	req.SetQuestion(qname, dns.TypeA)
+	expectedResult = dns.RcodeNameError
+	if c.ServeDNS(ctx, rec, req); rec.Rcode != expectedResult {
+		t.Errorf("Test 2 NOERROR from Backend: expecting %v; got %v", expectedResult, rec.Rcode)
+	}
+
+	// Confirm that prefetch removes the negative cache item.
+	waitFor := 3
+	for i := 1; i <= waitFor; i++ {
+		if c.ncache.Len() != 0 {
+			if i == waitFor {
+				t.Errorf("Test 2 NOERROR from Backend: item still exists in negative cache")
+			}
+			time.Sleep(time.Second)
+			continue
+		}
+	}
+
+	// Confirm that positive cache has the item
+	if c.pcache.Len() != 1 {
+		t.Errorf("Test 2 NOERROR from Backend: item missing from positive cache")
+	}
+
+	// Backend - Give error only
+	c.Next = plugin.HandlerFunc(func(context.Context, dns.ResponseWriter, *dns.Msg) (int, error) {
+		return 255, nil // Below, a 255 means we tried querying upstream.
+	})
+
+	// Query again, expect that positive cache entry is not masked by a negative cache entry
+	rec = dnstest.NewRecorder(&test.ResponseWriter{})
+	req = new(dns.Msg)
+	req.SetQuestion(qname, dns.TypeA)
+	expectedResult = dns.RcodeSuccess
+	if ret, _ := c.ServeDNS(ctx, rec, req); ret != expectedResult {
+		t.Errorf("Test 3 NOERROR from Cache: expecting %v; got %v", expectedResult, ret)
+	}
+}
+
 func BenchmarkCacheResponse(b *testing.B) {
 	c := New()
 	c.prefetch = 1
@ -334,6 +420,20 @@ func BackendHandler() plugin.Handler {
 	})
 }

+func nxDomainBackend(ttl int) plugin.Handler {
+	return plugin.HandlerFunc(func(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (int, error) {
+		m := new(dns.Msg)
+		m.SetReply(r)
+		m.Response, m.RecursionAvailable = true, true
+
+		m.Ns = []dns.RR{test.SOA(fmt.Sprintf("example.org. %d IN	SOA	sns.dns.icann.org. noc.dns.icann.org. 2016082540 7200 3600 1209600 3600", ttl))}
+
+		m.MsgHdr.Rcode = dns.RcodeNameError
+		w.WriteMsg(m)
+		return dns.RcodeNameError, nil
+	})
+}
+
 func ttlBackend(ttl int) plugin.Handler {
 	return plugin.HandlerFunc(func(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (int, error) {
 		m := new(dns.Msg)