plugin/cache: add a new keepttl option

[MrFreezeex]

1. Why is this pull request needed and what does it do?

Whenever CoreDNS is used as an authoritative server it could be useful to serve a consistent TTL (not decreased based on time) to clients and still use caching to spare request to the backend.

This commit adds a new option keepttl to the cache plugin to do just that. This new option doesn't update any caching logic but just override the cache plugin answer.

2. Which issues (if any) are related?

3. Which documentation changes (if any) need to be made?

To the cache plugin (included in this PR)

4. Does this introduce a backward incompatible change or deprecation?

no

[chrisohaver]

Whenever CoreDNS is used as an authoritative server it could be useful to serve a consistent TTL to clients and still use caching to spare request to the backend.

It seems like setting the maximum TTL to the longest allowed TTL may be an easier way to solve that situation with the currently available options. It would leave the TTL unchanged, but also keep record in cache per their original TTL value thus sparing requests to the back end more so than caching for a shorted period of time.

[MrFreezeex]

It seems like setting the maximum TTL to the longest allowed TTL may be an easier way to solve that situation with the currently available options. It would leave the TTL unchanged, but also keep record in cache per their original TTL value thus sparing requests to the back end more so than caching for a shorted period of time.

Hi! thanks for the quick reply :). I am not exactly sure what you mean though. The TTL seem to be always computed by subtracting the time in the cache plugin AFAIU:

coredns/plugin/cache/item.go

Line 98 in d3e4fc7

ttl := int(i.origTTL) - int(now.UTC().Sub(i.stored).Seconds())

. So you would get the "original ttl" the first time it gets cached and then it would always be decreased.

We are trying to do something similar to the dontAge option from dnsdist (https://dnsdist.org/reference/config.html?highlight=dontage):

dontAge (bool) – Don’t reduce TTLs when serving from the cache. Use this when dnsdist fronts a cluster of authoritative servers

EDIT: I just updated the doc it might be slightly clearer like that

[chrisohaver]

Thanks for the clarification. I think the feature would be problematic applied to any other situations. E.g. when caching responses retrieved from upstream servers.

I think we should also add a note in the README that enabling this feature could result in downstream caching stale answers from upstream servers. Or perhaps that we don't recommend using this option for caching answers from upstream dns servers. IOW, it should not be used when caching records CoreDNS is not authoritative for because it could result in downstream clients using stale answers.

[MrFreezeex]

Indeed better documentation is always good :D. I just added a note in the cache plugin readme like you advised!

[chrisohaver]

Thanks! Just suggesting some tweaks to the wording.

[MrFreezeex]

Thanks for the wording suggestions, looks indeed better this way!

[codecov-commenter]

Codecov Report

Merging #5879 (c59ee24) into master (93c57b6) will increase coverage by 1.58%.
The diff coverage is 56.64%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##           master    #5879      +/-   ##
==========================================
+ Coverage   55.70%   57.28%   +1.58%     
==========================================
  Files         224      245      +21     
  Lines       10016    15738    +5722     
==========================================
+ Hits         5579     9015    +3436     
- Misses       3978     6175    +2197     
- Partials      459      548      +89     

Impacted Files	Coverage Δ
core/dnsserver/config.go	0.00% <ø> (ø)
core/dnsserver/register.go	13.79% <0.00%> (-4.19%)	⬇️
plugin/auto/auto.go	0.00% <0.00%> (ø)
plugin/auto/zone.go	78.94% <ø> (+0.68%)	⬆️
plugin/autopath/autopath.go	73.61% <ø> (-1.39%)	⬇️
plugin/azure/azure.go	10.61% <0.00%> (-0.57%)	⬇️
plugin/azure/setup.go	56.36% <0.00%> (-4.14%)	⬇️
plugin/backend_lookup.go	0.00% <0.00%> (ø)
plugin/bind/bind.go	50.00% <0.00%> (-50.00%)	⬇️
plugin/clouddns/gcp.go	0.00% <0.00%> (ø)
... and 317 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more