Add MINTTL parameter to cache configuration. (#2055)

* Add success min TTL parameter to cache.

* Add MINTTL to README.

* Update README.

* Add MINTTL to negative cache.

* Remove unnecessary variable name.

* Address review comments.

* Configure cache in TestCacheZeroTTL to have 0 min ttl.

5 files changed, 111 insertions, 31 deletions

diff --git a/plugin/cache/README.md b/plugin/cache/README.md
index fa790185e..5f1980794 100644
--- a/plugin/cache/README.md
+++ b/plugin/cache/README.md
@@ -32,8 +32,8 @@ If you want more control:
 
 ~~~ txt
 cache [TTL] [ZONES...] {
-    success CAPACITY [TTL]
-    denial CAPACITY [TTL]
+    success CAPACITY [TTL] [MINTTL]
+    denial CAPACITY [TTL] [MINTTL]
     prefetch AMOUNT [[DURATION] [PERCENTAGE%]]
 }
 ~~~
@@ -41,8 +41,10 @@ cache [TTL] [ZONES...] {
 * **TTL**  and **ZONES** as above.
 * `success`, override the settings for caching successful responses. **CAPACITY** indicates the maximum
   number of packets we cache before we start evicting (*randomly*). **TTL** overrides the cache maximum TTL.
+  **MINTTL** overrides the cache minimum TTL, which can be useful to limit queries to the backend.
 * `denial`, override the settings for caching denial of existence responses. **CAPACITY** indicates the maximum
   number of packets we cache before we start evicting (LRU). **TTL** overrides the cache maximum TTL.
+  **MINTTL** overrides the cache minimum TTL, which can be useful to limit queries to the backend.
   There is a third category (`error`) but those responses are never cached.
 * `prefetch` will prefetch popular items when they are about to be expunged from the cache.
   Popular means **AMOUNT** queries have been seen with no gaps of **DURATION** or more between them.
diff --git a/plugin/cache/cache.go b/plugin/cache/cache.go
index 654841b23..9ff43dac2 100644
--- a/plugin/cache/cache.go
+++ b/plugin/cache/cache.go
@@ -22,13 +22,15 @@ type Cache struct {
 	Next  plugin.Handler
 	Zones []string
 
-	ncache *cache.Cache
-	ncap   int
-	nttl   time.Duration
+	ncache  *cache.Cache
+	ncap    int
+	nttl    time.Duration
+	minnttl time.Duration
 
-	pcache *cache.Cache
-	pcap   int
-	pttl   time.Duration
+	pcache  *cache.Cache
+	pcap    int
+	pttl    time.Duration
+	minpttl time.Duration
 
 	// Prefetch.
 	prefetch   int
@@ -47,9 +49,11 @@ func New() *Cache {
 		pcap:       defaultCap,
 		pcache:     cache.New(defaultCap),
 		pttl:       maxTTL,
+		minpttl:    minTTL,
 		ncap:       defaultCap,
 		ncache:     cache.New(defaultCap),
 		nttl:       maxNTTL,
+		minnttl:    minNTTL,
 		prefetch:   0,
 		duration:   1 * time.Minute,
 		percentage: 10,
@@ -100,6 +104,17 @@ func hash(qname string, qtype uint16, do bool) uint64 {
 	return h.Sum64()
 }
 
+func computeTTL(msgTTL, minTTL, maxTTL time.Duration) time.Duration {
+	ttl := msgTTL
+	if ttl < minTTL {
+		ttl = minTTL
+	}
+	if ttl > maxTTL {
+		ttl = maxTTL
+	}
+	return ttl
+}
+
 // ResponseWriter is a response writer that caches the reply message.
 type ResponseWriter struct {
 	dns.ResponseWriter
@@ -154,14 +169,12 @@ func (w *ResponseWriter) WriteMsg(res *dns.Msg) error {
 	// key returns empty string for anything we don't want to cache.
 	hasKey, key := key(res, mt, do)
 
-	duration := w.pttl
-	if mt == response.NameError || mt == response.NoData {
-		duration = w.nttl
-	}
-
 	msgTTL := dnsutil.MinimalTTL(res, mt)
-	if msgTTL < duration {
-		duration = msgTTL
+	var duration time.Duration
+	if mt == response.NameError || mt == response.NoData {
+		duration = computeTTL(msgTTL, w.minnttl, w.nttl)
+	} else {
+		duration = computeTTL(msgTTL, w.minpttl, w.pttl)
 	}
 
 	if hasKey && duration > 0 {
@@ -226,7 +239,9 @@ func (w *ResponseWriter) Write(buf []byte) (int, error) {
 
 const (
 	maxTTL  = dnsutil.MaximumDefaulTTL
+	minTTL  = dnsutil.MinimalDefaultTTL
 	maxNTTL = dnsutil.MaximumDefaulTTL / 2
+	minNTTL = dnsutil.MinimalDefaultTTL
 
 	defaultCap = 10000 // default capacity of the cache.
 
diff --git a/plugin/cache/cache_test.go b/plugin/cache/cache_test.go
index 08ff667a7..6ef8c8c3a 100644
--- a/plugin/cache/cache_test.go
+++ b/plugin/cache/cache_test.go
@@ -205,6 +205,8 @@ func TestCache(t *testing.T) {
 
 func TestCacheZeroTTL(t *testing.T) {
 	c := New()
+	c.minpttl = 0
+	c.minnttl = 0
 	c.Next = zeroTTLBackend()
 
 	req := new(dns.Msg)
@@ -270,3 +272,23 @@ func zeroTTLBackend() plugin.Handler {
 		return dns.RcodeSuccess, nil
 	})
 }
+
+func TestComputeTTL(t *testing.T) {
+	tests := []struct {
+		msgTTL      time.Duration
+		minTTL      time.Duration
+		maxTTL      time.Duration
+		expectedTTL time.Duration
+	}{
+		{1800 * time.Second, 300 * time.Second, 3600 * time.Second, 1800 * time.Second},
+		{299 * time.Second, 300 * time.Second, 3600 * time.Second, 300 * time.Second},
+		{299 * time.Second, 0 * time.Second, 3600 * time.Second, 299 * time.Second},
+		{3601 * time.Second, 300 * time.Second, 3600 * time.Second, 3600 * time.Second},
+	}
+	for i, test := range tests {
+		ttl := computeTTL(test.msgTTL, test.minTTL, test.maxTTL)
+		if ttl != test.expectedTTL {
+			t.Errorf("Test %v: Expected ttl %v but found: %v", i, test.expectedTTL, ttl)
+		}
+	}
+}
diff --git a/plugin/cache/setup.go b/plugin/cache/setup.go
index 17a588885..8464c27d9 100644
--- a/plugin/cache/setup.go
+++ b/plugin/cache/setup.go
@@ -101,6 +101,17 @@ func cacheParse(c *caddy.Controller) (*Cache, error) {
 						return nil, fmt.Errorf("cache TTL can not be zero or negative: %d", pttl)
 					}
 					ca.pttl = time.Duration(pttl) * time.Second
+					if len(args) > 2 {
+						minpttl, err := strconv.Atoi(args[2])
+						if err != nil {
+							return nil, err
+						}
+						// Reserve < 0
+						if minpttl < 0 {
+							return nil, fmt.Errorf("cache min TTL can not be negative: %d", minpttl)
+						}
+						ca.minpttl = time.Duration(minpttl) * time.Second
+					}
 				}
 			case Denial:
 				args := c.RemainingArgs()
@@ -122,6 +133,17 @@ func cacheParse(c *caddy.Controller) (*Cache, error) {
 						return nil, fmt.Errorf("cache TTL can not be zero or negative: %d", nttl)
 					}
 					ca.nttl = time.Duration(nttl) * time.Second
+					if len(args) > 2 {
+						minnttl, err := strconv.Atoi(args[2])
+						if err != nil {
+							return nil, err
+						}
+						// Reserve < 0
+						if minnttl < 0 {
+							return nil, fmt.Errorf("cache min TTL can not be negative: %d", minnttl)
+						}
+						ca.minnttl = time.Duration(minnttl) * time.Second
+					}
 				}
 			case "prefetch":
 				args := c.RemainingArgs()
diff --git a/plugin/cache/setup_test.go b/plugin/cache/setup_test.go
index c735e0bb2..bc1c163d1 100644
--- a/plugin/cache/setup_test.go
+++ b/plugin/cache/setup_test.go
@@ -14,54 +14,67 @@ func TestSetup(t *testing.T) {
 		expectedNcap     int
 		expectedPcap     int
 		expectedNttl     time.Duration
+		expectedMinNttl  time.Duration
 		expectedPttl     time.Duration
+		expectedMinPttl  time.Duration
 		expectedPrefetch int
 	}{
-		{`cache`, false, defaultCap, defaultCap, maxNTTL, maxTTL, 0},
-		{`cache {}`, false, defaultCap, defaultCap, maxNTTL, maxTTL, 0},
+		{`cache`, false, defaultCap, defaultCap, maxNTTL, minNTTL, maxTTL, minTTL, 0},
+		{`cache {}`, false, defaultCap, defaultCap, maxNTTL, minNTTL, maxTTL, minTTL, 0},
 		{`cache example.nl {
 				success 10
-			}`, false, defaultCap, 10, maxNTTL, maxTTL, 0},
+			}`, false, defaultCap, 10, maxNTTL, minNTTL, maxTTL, minTTL, 0},
+		{`cache example.nl {
+				success 10 1800 30
+			}`, false, defaultCap, 10, maxNTTL, minNTTL, 1800 * time.Second, 30 * time.Second, 0},
 		{`cache example.nl {
 				success 10
 				denial 10 15
-			}`, false, 10, 10, 15 * time.Second, maxTTL, 0},
+			}`, false, 10, 10, 15 * time.Second, minNTTL, maxTTL, minTTL, 0},
+		{`cache example.nl {
+				success 10
+				denial 10 15 2
+			}`, false, 10, 10, 15 * time.Second, 2 * time.Second, maxTTL, minTTL, 0},
 		{`cache 25 example.nl {
 				success 10
 				denial 10 15
-			}`, false, 10, 10, 15 * time.Second, 25 * time.Second, 0},
-		{`cache aaa example.nl`, false, defaultCap, defaultCap, maxNTTL, maxTTL, 0},
+			}`, false, 10, 10, 15 * time.Second, minNTTL, 25 * time.Second, minTTL, 0},
+		{`cache 25 example.nl {
+				success 10
+				denial 10 15 5
+			}`, false, 10, 10, 15 * time.Second, 5 * time.Second, 25 * time.Second, minTTL, 0},
+		{`cache aaa example.nl`, false, defaultCap, defaultCap, maxNTTL, minNTTL, maxTTL, minTTL, 0},
 		{`cache	{
 				prefetch 10
-			}`, false, defaultCap, defaultCap, maxNTTL, maxTTL, 10},
+			}`, false, defaultCap, defaultCap, maxNTTL, minNTTL, maxTTL, minTTL, 10},
 
 		// fails
 		{`cache example.nl {
 				success
 				denial 10 15
-			}`, true, defaultCap, defaultCap, maxTTL, maxTTL, 0},
+			}`, true, defaultCap, defaultCap, maxTTL, minNTTL, maxTTL, minTTL, 0},
 		{`cache example.nl {
 				success 15
 				denial aaa
-			}`, true, defaultCap, defaultCap, maxTTL, maxTTL, 0},
+			}`, true, defaultCap, defaultCap, maxTTL, minNTTL, maxTTL, minTTL, 0},
 		{`cache example.nl {
 				positive 15
 				negative aaa
-			}`, true, defaultCap, defaultCap, maxTTL, maxTTL, 0},
-		{`cache 0 example.nl`, true, defaultCap, defaultCap, maxTTL, maxTTL, 0},
-		{`cache -1 example.nl`, true, defaultCap, defaultCap, maxTTL, maxTTL, 0},
+			}`, true, defaultCap, defaultCap, maxTTL, minNTTL, maxTTL, minTTL, 0},
+		{`cache 0 example.nl`, true, defaultCap, defaultCap, maxTTL, minNTTL, maxTTL, minTTL, 0},
+		{`cache -1 example.nl`, true, defaultCap, defaultCap, maxTTL, minNTTL, maxTTL, minTTL, 0},
 		{`cache 1 example.nl {
 				positive 0
-			}`, true, defaultCap, defaultCap, maxTTL, maxTTL, 0},
+			}`, true, defaultCap, defaultCap, maxTTL, minNTTL, maxTTL, minTTL, 0},
 		{`cache 1 example.nl {
 				positive 0
 				prefetch -1
-			}`, true, defaultCap, defaultCap, maxTTL, maxTTL, 0},
+			}`, true, defaultCap, defaultCap, maxTTL, minNTTL, maxTTL, minTTL, 0},
 		{`cache 1 example.nl {
 				prefetch 0 blurp
-			}`, true, defaultCap, defaultCap, maxTTL, maxTTL, 0},
+			}`, true, defaultCap, defaultCap, maxTTL, minNTTL, maxTTL, minTTL, 0},
 		{`cache
-		  cache`, true, defaultCap, defaultCap, maxTTL, maxTTL, 0},
+		  cache`, true, defaultCap, defaultCap, maxTTL, minNTTL, maxTTL, minTTL, 0},
 	}
 	for i, test := range tests {
 		c := caddy.NewTestController("dns", test.input)
@@ -86,9 +99,15 @@ func TestSetup(t *testing.T) {
 		if ca.nttl != test.expectedNttl {
 			t.Errorf("Test %v: Expected nttl %v but found: %v", i, test.expectedNttl, ca.nttl)
 		}
+		if ca.minnttl != test.expectedMinNttl {
+			t.Errorf("Test %v: Expected minnttl %v but found: %v", i, test.expectedMinNttl, ca.minnttl)
+		}
 		if ca.pttl != test.expectedPttl {
 			t.Errorf("Test %v: Expected pttl %v but found: %v", i, test.expectedPttl, ca.pttl)
 		}
+		if ca.minpttl != test.expectedMinPttl {
+			t.Errorf("Test %v: Expected minpttl %v but found: %v", i, test.expectedMinPttl, ca.minpttl)
+		}
 		if ca.prefetch != test.expectedPrefetch {
 			t.Errorf("Test %v: Expected prefetch %v but found: %v", i, test.expectedPrefetch, ca.prefetch)
 		}