Not sure why this is proving so difficult.. pointers are hard? [Was
tempted to rollback all tweaks here, but the original issue we're fixing
it too important to not have a proper fix].
But we need to make a copy of the message at the earliest point in the
handler because we are changing it (adding an opt rr). If we do this on
the original message (which is a pointer) we change it (obvs). When
undoing those changes we do work on a copy.
Re: testing. There isn't a explicit test for this, so I've added on to
the top-level test/ directory, which indeed makes the issue visible:
master:
~~~
go test -v -run=TestLookupCacheWithoutEdns
=== RUN TestLookupCacheWithoutEdns
cache_test.go:154: Expected no OPT RR, but got:
;; OPT PSEUDOSECTION:
; EDNS: version 0; flags: do; udp: 2048
--- FAIL: TestLookupCacheWithoutEdns (0.01s)
FAIL
~~~
This branch:
~~~
% go test -v -run=TestLookupCacheWithoutEdns
=== RUN TestLookupCacheWithoutEdns
--- PASS: TestLookupCacheWithoutEdns (0.01s)
PASS
ok github.com/coredns/coredns/test 0.109s
~~~
Signed-off-by: Miek Gieben <miek@miek.nl>
The filtering of DNSSEC records in the cache plugin was not done
correctly. Also the change to introduced this bug didn't take into
account that the cache - by virtue of differentiating between DNSSEC and
no-DNSSEC - relied on not copying the data from the cache.
This change copies and then filters the data and factors the filtering
into a function that is used in two places (albeit with on ugly boolean
parameters to prevent copying things twice).
Add tests, do_test.go is moved to test/cache_test.go because the OPT
handing is done outside of the cache plugin. The core server re-attaches
the correct OPT when replying, so that makes for a better e2e test.
Added small unit test for filterRRslice and an explicit test that asks
for DNSSEC first and then plain, and vice versa to test cache behavior.
Fixes: #4146
Signed-off-by: Miek Gieben <miek@miek.nl>
* Move *proxy* to external
move the proxy plugin into coredns/proxy and remove it as a default
plugin. Link the proxy to deprecated in plugin.cfg
coredns/proxy doesn't compile because of the vendoring :(
Signed-off-by: Miek Gieben <miek@miek.nl>
* Add github.com/coredns/proxy
Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
* Default to upstream to self
This is a backwards incompatible change.
This is a massive (cleanup) PR where we default to resolving external
names by the coredns process itself, instead of directly forwarding them
to some upstream.
This ignores any arguments `upstream` may have had and makes it depend
on proxy/forward configuration in the Corefile. This allows resolved
upstream names to be cached and we have better healthchecking of the
upstreams. It also means there is only one way to resolve names, by
either using the proxy or forward plugin.
The proxy/forward lookup.go functions have been removed. This also
lessen the dependency on proxy, meaning deprecating proxy will become
easier. Some tests have been removed as well, or moved to the top-level
test directory as they now require a full coredns process instead of
just the plugin.
For the etcd plugin, the entire StubZone resolving is *dropped*! This
was a hacky (but working) solution to say the least. If someone cares
deeply it can be brought back (maybe)?
The pkg/upstream is now very small and almost does nothing. Also the
New() function was changed to return a pointer to upstream.Upstream. It
also returns only one parameter, so any stragglers using it will
encounter a compile error.
All documentation has been adapted. This affected the following plugins:
* etcd
* file
* auto
* secondary
* federation
* template
* route53
A followup PR will make any upstream directives with arguments an error,
right now they are ignored.
Signed-off-by: Miek Gieben <miek@miek.nl>
* Fix etcd build - probably still fails unit test
Signed-off-by: Miek Gieben <miek@miek.nl>
* Slightly smarter lookup check in upstream
Signed-off-by: Miek Gieben <miek@miek.nl>
* Compilez
Signed-off-by: Miek Gieben <miek@miek.nl>
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
* Rename middleware to plugin
first pass; mostly used 'sed', few spots where I manually changed
text.
This still builds a coredns binary.
* fmt error
* Rename AddMiddleware to AddPlugin
* Readd AddMiddleware to remain backwards compat
* tests: CoreDNSServerAndPorts
Copy from kubernetes.go and renamed to fit the style, adapted almost
all callers.
This is a mechanicl change, no testdata was changed.
* typos
By defining and using an proxy.Exchanger interface we make the proxy
more generic and we can then fold back httproxy into proxy.
This overrides #463 and #473 and should make futures extensions rather
trivial
* Add docs that talk about `protocol` and how to set it.
* middleware/proxy: rename New to NewLookup
It's used as a Lookup mechanism not as a completely new proxy,
reflect that in the name.
* Set maxfails to 3 by default when looking up names.
Most of the changes have been copied
from https://github.com/johnbelamaric/coredns/pull/1/files
Create a small speedup running the tests:
PASS
ok github.com/miekg/coredns/test 10.329s
PASS
ok github.com/miekg/coredns/test 6.079s
Skip the etcd ones. Doing the middleware/*/*_test ones doesn't yield
any speedup as these are still done on a per directory basis.
* Add a setup test for middleware/file
This fix adds a setup test for middleware/file so that there is
a basic coverage for the Corefile processing of middleware/file.
This fix is related to 308 (Will look into it).
Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
* middleware/file: use helper function for test
Fixup setup_test.go and use the test.TempFile function to make things
somewhat shorter.
Use clean up the use of testing.T in TempFile - it is not used.
Make the cache memory bounded, by using a LRU cache. Also split the
cache in a positive and negative one - each with its own controls.
Extend the cache stanza to allow for this:
cache {
positive limit [ttl]
negative limit [ttl]
}
is now possible. This also add a cache_test.go in the toplevel test/
directory that exercises the caching path.
Fixes#260
