plugin/cache: cap TTL on first answer (#1092)

Cache would let the first response through and would then cap subsequent
ones to whatever the cache duration was. This would lead to huge drops
in TTL values: 3600 -> 20 for instance, which is not only bad, but can
mess up your careful TTL planning business.

This PR fixes that and applies the cache duration to all replies. As a
bonus I could remove a time.Sleep() from the cache test and just check
for the cache duration as the TTL on the reply.

Fixes #1038

plugin/cache/cache.go

@@ -117,6 +117,19 @@ func (w *ResponseWriter) WriteMsg(res *dns.Msg) error {
 		return nil
 	}
 
+	// Apply capped TTL to this reply to avoid jarring TTL experience 1799 -> 8 (e.g.)
+	ttl := uint32(duration.Seconds())
+	for i := range res.Answer {
+		res.Answer[i].Header().Ttl = ttl
+	}
+	for i := range res.Ns {
+		res.Ns[i].Header().Ttl = ttl
+	}
+	for i := range res.Extra {
+		if res.Extra[i].Header().Rrtype != dns.TypeOPT {
+			res.Extra[i].Header().Ttl = ttl
+		}
+	}
 	return w.ResponseWriter.WriteMsg(res)
 }
 

plugin/cache/handler.go

@@ -19,7 +19,7 @@ func (c *Cache) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg)
 	qtype := state.QType()
 	zone := plugin.Zones(c.Zones).Matches(qname)
 	if zone == "" {
-		return c.Next.ServeDNS(ctx, w, r)
+		return plugin.NextOrFailure(c.Name(), c.Next, ctx, w, r)
 	}
 
 	do := state.Do() // TODO(): might need more from OPT record? Like the actual bufsize?

test/cache_test.go

@@ -1,10 +1,7 @@
 package test
 
 import (
-	"io/ioutil"
-	"log"
 	"testing"
-	"time"
 
 	"github.com/coredns/coredns/plugin/proxy"
 	"github.com/coredns/coredns/plugin/test"
@@ -14,7 +11,6 @@ import (
 )
 
 func TestLookupCache(t *testing.T) {
-	t.Parallel()
 	// Start auth. CoreDNS holding the auth zone.
 	name, rm, err := test.TempFile(".", exampleOrg)
 	if err != nil {
@@ -35,7 +31,7 @@ func TestLookupCache(t *testing.T) {
 	// Start caching proxy CoreDNS that we want to test.
 	corefile = `example.org:0 {
 	proxy . ` + udp + `
-	cache
+	cache 10
 }
 `
 	i, udp, _, err = CoreDNSServerAndPorts(corefile)
@@ -44,8 +40,6 @@ func TestLookupCache(t *testing.T) {
 	}
 	defer i.Stop()
 
-	log.SetOutput(ioutil.Discard)
-
 	p := proxy.NewLookup([]string{udp})
 	state := request.Request{W: &test.ResponseWriter{}, Req: new(dns.Msg)}
 
@@ -59,20 +53,7 @@ func TestLookupCache(t *testing.T) {
 	}
 
 	ttl := resp.Answer[0].Header().Ttl
-
+	if ttl != 10 { // as set in the Corefile
-	time.Sleep(2 * time.Second) // TODO(miek): meh.
+		t.Errorf("Expected TTL to be %d, got %d", 10, ttl)
-
-	resp, err = p.Lookup(state, "example.org.", dns.TypeA)
-	if err != nil {
-		t.Fatal("Expected to receive reply, but didn't")
-	}
-
-	// expect answer section with A record in it
-	if len(resp.Answer) == 0 {
-		t.Error("Expected to at least one RR in the answer section, got none")
-	}
-	newTTL := resp.Answer[0].Header().Ttl
-	if newTTL >= ttl {
-		t.Errorf("Expected TTL to be lower than: %d, got %d", ttl, newTTL)
 	}
 }