middleware/autopath: some fixes (#883)

* middleware/autopath: some fixes

This fix a small issue in autopath, but unearthed a bigger one.
See #881.

* Fix test
This commit is contained in:
Miek Gieben 2017-08-10 19:27:54 +01:00 committed by GitHub
parent 6cc3f47d46
commit 4d0dae8deb
3 changed files with 29 additions and 16 deletions

View file

@ -64,6 +64,11 @@ func (a *AutoPath) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Ms
return dns.RcodeServerFailure, middleware.Error(a.Name(), errors.New("can only deal with ClassINET")) return dns.RcodeServerFailure, middleware.Error(a.Name(), errors.New("can only deal with ClassINET"))
} }
zone := middleware.Zones(a.Zones).Matches(state.Name())
if zone == "" {
return middleware.NextOrFailure(a.Name(), a.Next, ctx, w, r)
}
// Check if autopath should be done, searchFunc takes precedence over the local configured // Check if autopath should be done, searchFunc takes precedence over the local configured
// search path. // search path.
var err error var err error
@ -83,7 +88,7 @@ func (a *AutoPath) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Ms
origQName := state.QName() origQName := state.QName()
// Establish base name of the query. I.e what was originally asked. // Establish base name of the query. I.e what was originally asked.
base, err := dnsutil.TrimZone(state.QName(), a.search[0]) // TOD(miek): we loose the original case of the query here. base, err := dnsutil.TrimZone(state.QName(), a.search[0]) // TODO(miek): we loose the original case of the query here.
if err != nil { if err != nil {
return dns.RcodeServerFailure, err return dns.RcodeServerFailure, err
} }
@ -91,15 +96,21 @@ func (a *AutoPath) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Ms
firstReply := new(dns.Msg) firstReply := new(dns.Msg)
firstRcode := 0 firstRcode := 0
var firstErr error var firstErr error
ar := r.Copy()
// Walk the search path and see if we can get a non-nxdomain - if they all fail we return the first // Walk the search path and see if we can get a non-nxdomain - if they all fail we return the first
// query we've done and return that as-is. This means the client will do the search path walk again... // query we've done and return that as-is. This means the client will do the search path walk again...
for i, s := range searchpath { for i, s := range searchpath {
newQName := base + "." + s newQName := base + "." + s
r.Question[0].Name = newQName ar.Question[0].Name = newQName
nw := NewNonWriter(w) nw := NewNonWriter(w)
rcode, err := middleware.NextOrFailure(a.Name(), a.Next, ctx, nw, r) rcode, err := middleware.NextOrFailure(a.Name(), a.Next, ctx, nw, ar)
if err != nil {
// Return now - not sure if this is the best. We should also check if the write has happened.
return rcode, err
}
if i == 0 { if i == 0 {
firstReply = nw.Msg firstReply = nw.Msg
firstRcode = rcode firstRcode = rcode
@ -139,4 +150,4 @@ func (a *AutoPath) FirstInSearchPath(name string) bool {
return false return false
} }
func (a AutoPath) Name() string { return "autopath" } func (a *AutoPath) Name() string { return "autopath" }

View file

@ -31,6 +31,7 @@ var autopathTestCases = []test.Case{
func newTestAutoPath() *AutoPath { func newTestAutoPath() *AutoPath {
ap := new(AutoPath) ap := new(AutoPath)
ap.Zones = []string{"."}
ap.Next = nextHandler(map[string]int{ ap.Next = nextHandler(map[string]int{
"b.example.org.": dns.RcodeNameError, "b.example.org.": dns.RcodeNameError,
"b.com.": dns.RcodeSuccess, "b.com.": dns.RcodeSuccess,

View file

@ -5,7 +5,6 @@ import (
"github.com/coredns/coredns/core/dnsserver" "github.com/coredns/coredns/core/dnsserver"
"github.com/coredns/coredns/middleware" "github.com/coredns/coredns/middleware"
"github.com/coredns/coredns/middleware/kubernetes"
"github.com/mholt/caddy" "github.com/mholt/caddy"
"github.com/miekg/dns" "github.com/miekg/dns"
@ -20,7 +19,7 @@ func init() {
} }
func setup(c *caddy.Controller) error { func setup(c *caddy.Controller) error {
ap, mw, err := autoPathParse(c) ap, _, err := autoPathParse(c)
if err != nil { if err != nil {
return middleware.Error("autopath", err) return middleware.Error("autopath", err)
} }
@ -28,13 +27,15 @@ func setup(c *caddy.Controller) error {
c.OnStartup(func() error { c.OnStartup(func() error {
// Do this in OnStartup, so all middleware has been initialized. // Do this in OnStartup, so all middleware has been initialized.
// TODO(miek): fabricate test to proof this is not thread safe. // TODO(miek): fabricate test to proof this is not thread safe.
m := dnsserver.GetMiddleware(c, mw) // TODO(miek): disable this for now: See https://github.com/coredns/coredns/issues/881
switch mw { /*
case "kubernetes": switch mw {
if k, ok := m.(kubernetes.Kubernetes); ok { case "kubernetes":
ap.searchFunc = k.AutoPath if k, ok := m.(kubernetes.Kubernetes); ok {
ap.searchFunc = k.AutoPath
}
} }
} */
return nil return nil
}) })
@ -51,13 +52,13 @@ var allowedMiddleware = map[string]bool{
} }
func autoPathParse(c *caddy.Controller) (*AutoPath, string, error) { func autoPathParse(c *caddy.Controller) (*AutoPath, string, error) {
ap := new(AutoPath) ap := &AutoPath{}
mw := "" mw := ""
for c.Next() { for c.Next() {
zoneAndresolv := c.RemainingArgs() zoneAndresolv := c.RemainingArgs()
if len(zoneAndresolv) < 1 { if len(zoneAndresolv) < 1 {
return nil, "", fmt.Errorf("no resolv-conf specified") return ap, "", fmt.Errorf("no resolv-conf specified")
} }
resolv := zoneAndresolv[len(zoneAndresolv)-1] resolv := zoneAndresolv[len(zoneAndresolv)-1]
if resolv[0] == '@' { if resolv[0] == '@' {
@ -69,7 +70,7 @@ func autoPathParse(c *caddy.Controller) (*AutoPath, string, error) {
// assume file on disk // assume file on disk
rc, err := dns.ClientConfigFromFile(resolv) rc, err := dns.ClientConfigFromFile(resolv)
if err != nil { if err != nil {
return nil, "", fmt.Errorf("failed to parse %q: %v", resolv, err) return ap, "", fmt.Errorf("failed to parse %q: %v", resolv, err)
} }
ap.search = rc.Search ap.search = rc.Search
middleware.Zones(ap.search).Normalize() middleware.Zones(ap.search).Normalize()