diff --git a/plugin/auto/README.md b/plugin/auto/README.md index 7d746cb82..661e419ab 100644 --- a/plugin/auto/README.md +++ b/plugin/auto/README.md @@ -16,7 +16,6 @@ zonefile. New or changed zones are automatically picked up from disk only when S ~~~ auto [ZONES...] { directory DIR [REGEXP ORIGIN_TEMPLATE] - transfer to ADDRESS... reload DURATION } ~~~ @@ -29,14 +28,12 @@ are used. like `{}` are replaced with the respective matches in the file name, e.g. `{1}` is the first match, `{2}` is the second. The default is: `db\.(.*) {1}` i.e. from a file with the name `db.example.com`, the extracted origin will be `example.com`. -* `transfer` enables zone transfers. It may be specified multiples times. `To` or `from` signals - the direction. **ADDRESS** must be denoted in CIDR notation (e.g., 127.0.0.1/32) or just as plain - addresses. The special wildcard `*` means: the entire internet (only valid for 'transfer to'). - When an address is specified a notify message will be send whenever the zone is reloaded. * `reload` interval to perform reloads of zones if SOA version changes and zonefiles. It specifies how often CoreDNS should scan the directory to watch for file removal and addition. Default is one minute. Value of `0` means to not scan for changes and reload. eg. `30s` checks zonefile every 30 seconds and reloads zone when serial changes. +For enabling zone transfers look at the *transfer* plugin. + All directives from the *file* plugin are supported. Note that *auto* will load all zones found, even though the directive might only receive queries for a specific zone. I.e: @@ -59,8 +56,10 @@ notifies to 10.240.1.1 org { auto { directory /etc/coredns/zones/org - transfer to * - transfer to 10.240.1.1 + } + transfer { + to * + to 10.240.1.1 } } ~~~ @@ -76,3 +75,8 @@ org { } } ~~~ + +## Also + +Use the *root* plugin to help you specify the location of the zone files. See the *transfer* plugin +to enable outgoing zone transfers. diff --git a/plugin/auto/auto.go b/plugin/auto/auto.go index 74a48a205..4316d8d2f 100644 --- a/plugin/auto/auto.go +++ b/plugin/auto/auto.go @@ -10,6 +10,7 @@ import ( "github.com/coredns/coredns/plugin/file" "github.com/coredns/coredns/plugin/metrics" "github.com/coredns/coredns/plugin/pkg/upstream" + "github.com/coredns/coredns/plugin/transfer" "github.com/coredns/coredns/request" "github.com/miekg/dns" @@ -21,7 +22,8 @@ type ( Next plugin.Handler *Zones - metrics *metrics.Metrics + metrics *metrics.Metrics + transfer *transfer.Transfer loader } @@ -30,8 +32,6 @@ type ( template string re *regexp.Regexp - // In the future this should be something like ZoneMeta that contains all this stuff. - transferTo []string ReloadInterval time.Duration upstream *upstream.Upstream // Upstream for looking up names during the resolution process. } @@ -59,11 +59,6 @@ func (a Auto) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (i return dns.RcodeServerFailure, nil } - if state.QType() == dns.TypeAXFR || state.QType() == dns.TypeIXFR { - xfr := file.Xfr{Zone: z} - return xfr.ServeDNS(ctx, w, r) - } - answer, ns, extra, result := z.Lookup(ctx, state, qname) m := new(dns.Msg) diff --git a/plugin/auto/setup.go b/plugin/auto/setup.go index 7da3e11a2..bebf3a65b 100644 --- a/plugin/auto/setup.go +++ b/plugin/auto/setup.go @@ -11,8 +11,8 @@ import ( "github.com/coredns/coredns/plugin" "github.com/coredns/coredns/plugin/metrics" clog "github.com/coredns/coredns/plugin/pkg/log" - "github.com/coredns/coredns/plugin/pkg/parse" "github.com/coredns/coredns/plugin/pkg/upstream" + "github.com/coredns/coredns/plugin/transfer" ) var log = clog.NewWithPlugin("auto") @@ -27,10 +27,13 @@ func setup(c *caddy.Controller) error { c.OnStartup(func() error { m := dnsserver.GetConfig(c).Handler("prometheus") - if m == nil { - return nil + if m != nil { + (&a).metrics = m.(*metrics.Metrics) + } + t := dnsserver.GetConfig(c).Handler("transfer") + if t != nil { + (&a).transfer = t.(*transfer.Transfer) } - (&a).metrics = m.(*metrics.Metrics) return nil }) @@ -146,15 +149,6 @@ func autoParse(c *caddy.Controller) (Auto, error) { // remove soon c.RemainingArgs() // eat remaining args - case "transfer": - t, _, e := parse.Transfer(c, false) - if e != nil { - return a, e - } - if t != nil { - a.loader.transferTo = append(a.loader.transferTo, t...) - } - default: return Auto{}, c.Errf("unknown property '%s'", c.Val()) } diff --git a/plugin/auto/setup_test.go b/plugin/auto/setup_test.go index e50b8451e..987411c07 100644 --- a/plugin/auto/setup_test.go +++ b/plugin/auto/setup_test.go @@ -15,48 +15,38 @@ func TestAutoParse(t *testing.T) { expectedTempl string expectedRe string expectedReloadInterval time.Duration - expectedTo []string }{ { `auto example.org { directory /tmp - transfer to 127.0.0.1 }`, - false, "/tmp", "${1}", `db\.(.*)`, 60 * time.Second, []string{"127.0.0.1:53"}, + false, "/tmp", "${1}", `db\.(.*)`, 60 * time.Second, }, { `auto 10.0.0.0/24 { directory /tmp }`, - false, "/tmp", "${1}", `db\.(.*)`, 60 * time.Second, nil, + false, "/tmp", "${1}", `db\.(.*)`, 60 * time.Second, }, { `auto { directory /tmp reload 0 }`, - false, "/tmp", "${1}", `db\.(.*)`, 0 * time.Second, nil, + false, "/tmp", "${1}", `db\.(.*)`, 0 * time.Second, }, { `auto { directory /tmp (.*) bliep }`, - false, "/tmp", "bliep", `(.*)`, 60 * time.Second, nil, + false, "/tmp", "bliep", `(.*)`, 60 * time.Second, }, { `auto { directory /tmp (.*) bliep reload 10s }`, - false, "/tmp", "bliep", `(.*)`, 10 * time.Second, nil, - }, - { - `auto { - directory /tmp (.*) bliep - transfer to 127.0.0.1 - transfer to 127.0.0.2 - }`, - false, "/tmp", "bliep", `(.*)`, 60 * time.Second, []string{"127.0.0.1:53", "127.0.0.2:53"}, + false, "/tmp", "bliep", `(.*)`, 10 * time.Second, }, // errors // NO_RELOAD has been deprecated. @@ -65,42 +55,50 @@ func TestAutoParse(t *testing.T) { directory /tmp no_reload }`, - true, "/tmp", "${1}", `db\.(.*)`, 0 * time.Second, nil, + true, "/tmp", "${1}", `db\.(.*)`, 0 * time.Second, }, // TIMEOUT has been deprecated. { `auto { directory /tmp (.*) bliep 10 }`, - true, "/tmp", "bliep", `(.*)`, 10 * time.Second, nil, + true, "/tmp", "bliep", `(.*)`, 10 * time.Second, + }, + // TRANSFER has been deprecated. + { + `auto { + directory /tmp (.*) bliep 10 + transfer to 127.0.0.1 + }`, + true, "/tmp", "bliep", `(.*)`, 10 * time.Second, }, // no template specified. { `auto { directory /tmp (.*) }`, - true, "/tmp", "", `(.*)`, 60 * time.Second, nil, + true, "/tmp", "", `(.*)`, 60 * time.Second, }, // no directory specified. { `auto example.org { directory }`, - true, "", "${1}", `db\.(.*)`, 60 * time.Second, nil, + true, "", "${1}", `db\.(.*)`, 60 * time.Second, }, // illegal REGEXP. { `auto example.org { directory /tmp * {1} }`, - true, "/tmp", "${1}", ``, 60 * time.Second, nil, + true, "/tmp", "${1}", ``, 60 * time.Second, }, // unexpected argument. { `auto example.org { directory /tmp (.*) {1} aa }`, - true, "/tmp", "${1}", ``, 60 * time.Second, nil, + true, "/tmp", "${1}", ``, 60 * time.Second, }, } @@ -125,13 +123,6 @@ func TestAutoParse(t *testing.T) { if a.loader.ReloadInterval != test.expectedReloadInterval { t.Fatalf("Test %d expected %v, got %v", i, test.expectedReloadInterval, a.loader.ReloadInterval) } - if test.expectedTo != nil { - for j, got := range a.loader.transferTo { - if got != test.expectedTo[j] { - t.Fatalf("Test %d expected %v, got %v", i, test.expectedTo[j], got) - } - } - } } } } diff --git a/plugin/auto/walk.go b/plugin/auto/walk.go index 40c62e514..9108fe941 100644 --- a/plugin/auto/walk.go +++ b/plugin/auto/walk.go @@ -53,15 +53,14 @@ func (a Auto) Walk() error { zo.ReloadInterval = a.loader.ReloadInterval zo.Upstream = a.loader.upstream - zo.TransferTo = a.loader.transferTo - a.Zones.Add(zo, origin) + a.Zones.Add(zo, origin, a.transfer) if a.metrics != nil { a.metrics.AddZone(origin) } - zo.Notify() + a.transfer.Notify(origin) log.Infof("Inserting zone `%s' from: %s", origin, path) diff --git a/plugin/auto/xfr.go b/plugin/auto/xfr.go new file mode 100644 index 000000000..6fef8b9e8 --- /dev/null +++ b/plugin/auto/xfr.go @@ -0,0 +1,19 @@ +package auto + +import ( + "github.com/coredns/coredns/plugin/transfer" + + "github.com/miekg/dns" +) + +// Transfer implements the transfer.Transfer interface. +func (a Auto) Transfer(zone string, serial uint32) (<-chan []dns.RR, error) { + a.Zones.RLock() + z, ok := a.Zones.Z[zone] + a.Zones.RUnlock() + + if !ok || z == nil { + return nil, transfer.ErrNotAuthoritative + } + return z.Transfer(serial) +} diff --git a/plugin/auto/zone.go b/plugin/auto/zone.go index 0a12ca39f..dff376bf9 100644 --- a/plugin/auto/zone.go +++ b/plugin/auto/zone.go @@ -5,6 +5,7 @@ import ( "sync" "github.com/coredns/coredns/plugin/file" + "github.com/coredns/coredns/plugin/transfer" ) // Zones maps zone names to a *Zone. This keeps track of what zones we have loaded at @@ -42,7 +43,7 @@ func (z *Zones) Zones(name string) *file.Zone { // Add adds a new zone into z. If zo.NoReload is false, the // reload goroutine is started. -func (z *Zones) Add(zo *file.Zone, name string) { +func (z *Zones) Add(zo *file.Zone, name string, t *transfer.Transfer) { z.Lock() if z.Z == nil { @@ -51,7 +52,7 @@ func (z *Zones) Add(zo *file.Zone, name string) { z.Z[name] = zo z.names = append(z.names, name) - zo.Reload() + zo.Reload(t) z.Unlock() } diff --git a/plugin/backend.go b/plugin/backend.go index 43d07d99c..a0217c961 100644 --- a/plugin/backend.go +++ b/plugin/backend.go @@ -29,20 +29,11 @@ type ServiceBackend interface { // IsNameError returns true if err indicated a record not found condition IsNameError(err error) bool - Transferer -} - -// Transferer defines an interface for backends that provide AXFR of all records. -type Transferer interface { // Serial returns a SOA serial number to construct a SOA record. Serial(state request.Request) uint32 // MinTTL returns the minimum TTL to be used in the SOA record. MinTTL(state request.Request) uint32 - - // Transfer handles a zone transfer it writes to the client just - // like any other handler. - Transfer(ctx context.Context, state request.Request) (int, error) } // Options are extra options that can be specified for a lookup. diff --git a/plugin/etcd/xfr.go b/plugin/etcd/xfr.go index 358ff7a38..87a4d7838 100644 --- a/plugin/etcd/xfr.go +++ b/plugin/etcd/xfr.go @@ -1,25 +1,17 @@ package etcd import ( - "context" "time" "github.com/coredns/coredns/request" - - "github.com/miekg/dns" ) -// Serial implements the Transferer interface. +// Serial returns the serial number to use. func (e *Etcd) Serial(state request.Request) uint32 { return uint32(time.Now().Unix()) } -// MinTTL implements the Transferer interface. +// MinTTL returns the minimal TTL. func (e *Etcd) MinTTL(state request.Request) uint32 { return 30 } - -// Transfer implements the Transferer interface. -func (e *Etcd) Transfer(ctx context.Context, state request.Request) (int, error) { - return dns.RcodeServerFailure, nil -} diff --git a/plugin/file/README.md b/plugin/file/README.md index e80b6b0dc..3923322dd 100644 --- a/plugin/file/README.md +++ b/plugin/file/README.md @@ -26,19 +26,16 @@ If you want to round-robin A and AAAA responses look at the *loadbalance* plugin ~~~ file DBFILE [ZONES... ] { - transfer to ADDRESS... reload DURATION } ~~~ -* `transfer` enables zone transfers. It may be specified multiples times. `To` or `from` signals - the direction. **ADDRESS** must be denoted in CIDR notation (e.g., 127.0.0.1/32) or just as plain - addresses. The special wildcard `*` means: the entire internet (only valid for 'transfer to'). - When an address is specified a notify message will be sent whenever the zone is reloaded. * `reload` interval to perform a reload of the zone if the SOA version changes. Default is one minute. Value of `0` means to not scan for changes and reload. For example, `30s` checks the zonefile every 30 seconds and reloads the zone when serial changes. +If you need outgoing zone transfers, take a look at the *transfer* plugin. + ## Examples Load the `example.org` zone from `example.org.signed` and allow transfers to the internet, but send @@ -46,9 +43,9 @@ notifies to 10.240.1.1 ~~~ corefile example.org { - file example.org.signed { - transfer to * - transfer to 10.240.1.1 + file example.org.signed + transfer { + to * 10.240.1.1 } } ~~~ @@ -57,9 +54,9 @@ Or use a single zone file for multiple zones: ~~~ corefile . { - file example.org.signed example.org example.net { - transfer to * - transfer to 10.240.1.1 + file example.org.signed example.org example.net + transfer example.org example.net { + to * 10.240.1.1 } } ~~~ @@ -94,4 +91,5 @@ example.org { ## Also See -See the *loadbalance* plugin if you need simple record shuffling. +See the *loadbalance* plugin if you need simple record shuffling. And the *transfer* plugin for zone +transfers. Lastly the *root* plugin can help you specificy the location of the zone files. diff --git a/plugin/file/file.go b/plugin/file/file.go index d8de85cd3..1c586dd6d 100644 --- a/plugin/file/file.go +++ b/plugin/file/file.go @@ -8,6 +8,7 @@ import ( "github.com/coredns/coredns/plugin" clog "github.com/coredns/coredns/plugin/pkg/log" + "github.com/coredns/coredns/plugin/transfer" "github.com/coredns/coredns/request" "github.com/miekg/dns" @@ -20,6 +21,7 @@ type ( File struct { Next plugin.Handler Zones + transfer *transfer.Transfer } // Zones maps zone names to a *Zone. @@ -77,11 +79,6 @@ func (f File) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (i return dns.RcodeServerFailure, nil } - if state.QType() == dns.TypeAXFR || state.QType() == dns.TypeIXFR { - xfr := Xfr{z} - return xfr.ServeDNS(ctx, w, r) - } - answer, ns, extra, result := z.Lookup(ctx, state, qname) m := new(dns.Msg) diff --git a/plugin/file/notify.go b/plugin/file/notify.go index 83d73ee6f..7d4e35cc3 100644 --- a/plugin/file/notify.go +++ b/plugin/file/notify.go @@ -1,10 +1,8 @@ package file import ( - "fmt" "net" - "github.com/coredns/coredns/plugin/pkg/rcode" "github.com/coredns/coredns/request" "github.com/miekg/dns" @@ -33,48 +31,3 @@ func (z *Zone) isNotify(state request.Request) bool { } return false } - -// Notify will send notifies to all configured TransferTo IP addresses. -func (z *Zone) Notify() { - go notify(z.origin, z.TransferTo) -} - -// notify sends notifies to the configured remote servers. It will try up to three times -// before giving up on a specific remote. We will sequentially loop through "to" -// until they all have replied (or have 3 failed attempts). -func notify(zone string, to []string) error { - m := new(dns.Msg) - m.SetNotify(zone) - c := new(dns.Client) - - for _, t := range to { - if t == "*" { - continue - } - if err := notifyAddr(c, m, t); err != nil { - log.Error(err.Error()) - } - } - log.Infof("Sent notifies for zone %q to %v", zone, to) - return nil -} - -func notifyAddr(c *dns.Client, m *dns.Msg, s string) error { - var err error - - code := dns.RcodeServerFailure - for i := 0; i < 3; i++ { - ret, _, err := c.Exchange(m, s) - if err != nil { - continue - } - code = ret.Rcode - if code == dns.RcodeSuccess { - return nil - } - } - if err != nil { - return fmt.Errorf("notify for zone %q was not accepted by %q: %q", m.Question[0].Name, s, err) - } - return fmt.Errorf("notify for zone %q was not accepted by %q: rcode was %q", m.Question[0].Name, s, rcode.ToString(code)) -} diff --git a/plugin/file/reload.go b/plugin/file/reload.go index 79db040fe..426a986b0 100644 --- a/plugin/file/reload.go +++ b/plugin/file/reload.go @@ -3,10 +3,12 @@ package file import ( "os" "time" + + "github.com/coredns/coredns/plugin/transfer" ) // Reload reloads a zone when it is changed on disk. If z.NoReload is true, no reloading will be done. -func (z *Zone) Reload() error { +func (z *Zone) Reload(t *transfer.Transfer) error { if z.ReloadInterval == 0 { return nil } @@ -40,7 +42,11 @@ func (z *Zone) Reload() error { z.Unlock() log.Infof("Successfully reloaded zone %q in %q with %d SOA serial", z.origin, zFile, z.Apex.SOA.Serial) - z.Notify() + if t != nil { + if err := t.Notify(z.origin); err != nil { + log.Warningf("Failed sending notifies: %s", err) + } + } case <-z.reloadShutdown: tick.Stop() diff --git a/plugin/file/reload_test.go b/plugin/file/reload_test.go index f9e544372..0c644d484 100644 --- a/plugin/file/reload_test.go +++ b/plugin/file/reload_test.go @@ -9,6 +9,7 @@ import ( "time" "github.com/coredns/coredns/plugin/test" + "github.com/coredns/coredns/plugin/transfer" "github.com/coredns/coredns/request" "github.com/miekg/dns" @@ -30,7 +31,7 @@ func TestZoneReload(t *testing.T) { } z.ReloadInterval = 500 * time.Millisecond - z.Reload() + z.Reload(&transfer.Transfer{}) time.Sleep(time.Second) ctx := context.TODO() diff --git a/plugin/file/secondary_test.go b/plugin/file/secondary_test.go index 820c9b9d0..67d151e53 100644 --- a/plugin/file/secondary_test.go +++ b/plugin/file/secondary_test.go @@ -11,10 +11,6 @@ import ( "github.com/miekg/dns" ) -// TODO(miek): should test notifies as well, ie start test server (a real coredns one)... -// setup other test server that sends notify, see if CoreDNS comes calling for a zone -// transfer - func TestLess(t *testing.T) { const ( min = 0 diff --git a/plugin/file/setup.go b/plugin/file/setup.go index c1dc853d6..ed19aedfe 100644 --- a/plugin/file/setup.go +++ b/plugin/file/setup.go @@ -8,8 +8,8 @@ import ( "github.com/coredns/caddy" "github.com/coredns/coredns/core/dnsserver" "github.com/coredns/coredns/plugin" - "github.com/coredns/coredns/plugin/pkg/parse" "github.com/coredns/coredns/plugin/pkg/upstream" + "github.com/coredns/coredns/plugin/transfer" ) func init() { plugin.Register("file", setup) } @@ -20,26 +20,43 @@ func setup(c *caddy.Controller) error { return plugin.Error("file", err) } - // Add startup functions to notify the master(s). - for _, n := range zones.Names { - z := zones.Z[n] - c.OnStartup(func() error { - z.StartupOnce.Do(func() { - if len(z.TransferTo) > 0 { - z.Notify() - } - z.Reload() - }) + f := File{Zones: zones} + // get the transfer plugin, so we can send notifies and send notifies on startup as well. + c.OnStartup(func() error { + t := dnsserver.GetConfig(c).Handler("transfer") + if t == nil { return nil - }) - } + } + f.transfer = t.(*transfer.Transfer) // if found this must be OK. + for _, n := range zones.Names { + f.transfer.Notify(n) + } + return nil + }) + + c.OnRestartFailed(func() error { + t := dnsserver.GetConfig(c).Handler("transfer") + if t == nil { + return nil + } + for _, n := range zones.Names { + f.transfer.Notify(n) + } + return nil + }) + for _, n := range zones.Names { z := zones.Z[n] c.OnShutdown(z.OnShutdown) + c.OnStartup(func() error { + z.StartupOnce.Do(func() { z.Reload(f.transfer) }) + return nil + }) } dnsserver.GetConfig(c).AddPlugin(func(next plugin.Handler) plugin.Handler { - return File{Next: next, Zones: zones} + f.Next = next + return f }) return nil @@ -92,24 +109,14 @@ func fileParse(c *caddy.Controller) (Zones, error) { names = append(names, origins[i]) } - t := []string{} - var e error - for c.NextBlock() { switch c.Val() { - case "transfer": - t, _, e = parse.Transfer(c, false) - if e != nil { - return Zones{}, e - } - case "reload": d, err := time.ParseDuration(c.RemainingArgs()[0]) if err != nil { return Zones{}, plugin.Error("file", err) } reload = d - case "upstream": // remove soon c.RemainingArgs() @@ -117,12 +124,6 @@ func fileParse(c *caddy.Controller) (Zones, error) { default: return Zones{}, c.Errf("unknown property '%s'", c.Val()) } - - for _, origin := range origins { - if t != nil { - z[origin].TransferTo = append(z[origin].TransferTo, t...) - } - } } } diff --git a/plugin/file/xfr.go b/plugin/file/xfr.go index f7192165b..28c3a3a9d 100644 --- a/plugin/file/xfr.go +++ b/plugin/file/xfr.go @@ -1,118 +1,45 @@ package file import ( - "context" - "fmt" - "sync" - - "github.com/coredns/coredns/plugin" "github.com/coredns/coredns/plugin/file/tree" - "github.com/coredns/coredns/request" + "github.com/coredns/coredns/plugin/transfer" "github.com/miekg/dns" ) -// Xfr serves up an AXFR. -type Xfr struct { - *Zone +// Transfer implements the transfer.Transfer interface. +func (f File) Transfer(zone string, serial uint32) (<-chan []dns.RR, error) { + z, ok := f.Zones.Z[zone] + if !ok || z == nil { + return nil, transfer.ErrNotAuthoritative + } + return z.Transfer(serial) } -// ServeDNS implements the plugin.Handler interface. -func (x Xfr) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (int, error) { - state := request.Request{W: w, Req: r} - if !x.TransferAllowed(state) { - return dns.RcodeServerFailure, nil - } - if state.QType() != dns.TypeAXFR && state.QType() != dns.TypeIXFR { - return 0, plugin.Error(x.Name(), fmt.Errorf("xfr called with non transfer type: %d", state.QType())) - } - - // For IXFR we take the SOA in the IXFR message (if there), compare it what we have and then decide to do an - // AXFR or just reply with one SOA message back. - if state.QType() == dns.TypeIXFR { - code, _ := x.ServeIxfr(ctx, w, r) - if plugin.ClientWrite(code) { - return code, nil - } - } - +// Transfer transfers a zone with serial in the returned channel and implements IXFR fallback, by just +// sending a single SOA record. +func (z *Zone) Transfer(serial uint32) (<-chan []dns.RR, error) { // get soa and apex - apex, err := x.ApexIfDefined() + apex, err := z.ApexIfDefined() if err != nil { - return dns.RcodeServerFailure, nil + return nil, err } - ch := make(chan *dns.Envelope) - tr := new(dns.Transfer) - wg := new(sync.WaitGroup) - wg.Add(1) + ch := make(chan []dns.RR) go func() { - tr.Out(w, r, ch) - wg.Done() + if serial != 0 && apex[0].(*dns.SOA).Serial == serial { // ixfr fallback, only send SOA + ch <- []dns.RR{apex[0]} + + close(ch) + return + } + + ch <- apex + z.Walk(func(e *tree.Elem, _ map[uint16][]dns.RR) error { ch <- e.All(); return nil }) + ch <- []dns.RR{apex[0]} + + close(ch) }() - rrs := []dns.RR{} - l := len(apex) - - ch <- &dns.Envelope{RR: apex} - - x.Walk(func(e *tree.Elem, _ map[uint16][]dns.RR) error { - rrs = append(rrs, e.All()...) - if len(rrs) > 500 { - ch <- &dns.Envelope{RR: rrs} - l += len(rrs) - rrs = []dns.RR{} - } - return nil - }) - - if len(rrs) > 0 { - ch <- &dns.Envelope{RR: rrs} - l += len(rrs) - rrs = []dns.RR{} - } - - ch <- &dns.Envelope{RR: []dns.RR{apex[0]}} // closing SOA. - l++ - - close(ch) // Even though we close the channel here, we still have - wg.Wait() // to wait before we can return and close the connection. - - log.Infof("Outgoing transfer of %d records of zone %s to %s done with %d SOA serial", l, x.origin, state.IP(), apex[0].(*dns.SOA).Serial) - return dns.RcodeSuccess, nil -} - -// Name implements the plugin.Handler interface. -func (x Xfr) Name() string { return "xfr" } - -// ServeIxfr checks if we need to serve a simpler IXFR for the incoming message. -// See RFC 1995 Section 3: "... and the authority section containing the SOA record of client's version of the zone." -// and Section 2, paragraph 4 where we only need to echo the SOA record back. -// This function must be called when the qtype is IXFR. It returns a plugin.ClientWrite(code) == false, when it didn't -// write anything and we should perform an AXFR. -func (x Xfr) ServeIxfr(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (int, error) { - if len(r.Ns) != 1 { - return dns.RcodeServerFailure, nil - } - soa, ok := r.Ns[0].(*dns.SOA) - if !ok { - return dns.RcodeServerFailure, nil - } - - x.RLock() - if x.Apex.SOA == nil { - x.RUnlock() - return dns.RcodeServerFailure, nil - } - serial := x.Apex.SOA.Serial - x.RUnlock() - - if soa.Serial == serial { // Section 2, para 4; echo SOA back. We have the same zone - m := new(dns.Msg) - m.SetReply(r) - m.Answer = []dns.RR{soa} - w.WriteMsg(m) - return 0, nil - } - return dns.RcodeServerFailure, nil + return ch, nil } diff --git a/plugin/file/zone.go b/plugin/file/zone.go index 62720abb4..aa5f3cac0 100644 --- a/plugin/file/zone.go +++ b/plugin/file/zone.go @@ -2,7 +2,6 @@ package file import ( "fmt" - "net" "path/filepath" "strings" "sync" @@ -10,7 +9,6 @@ import ( "github.com/coredns/coredns/plugin/file/tree" "github.com/coredns/coredns/plugin/pkg/upstream" - "github.com/coredns/coredns/request" "github.com/miekg/dns" ) @@ -26,7 +24,6 @@ type Zone struct { sync.RWMutex - TransferTo []string StartupOnce sync.Once TransferFrom []string @@ -58,7 +55,6 @@ func NewZone(name, file string) *Zone { // Copy copies a zone. func (z *Zone) Copy() *Zone { z1 := NewZone(z.origin, z.file) - z1.TransferTo = z.TransferTo z1.TransferFrom = z.TransferFrom z1.Expired = z.Expired @@ -69,7 +65,6 @@ func (z *Zone) Copy() *Zone { // CopyWithoutApex copies zone z without the Apex records. func (z *Zone) CopyWithoutApex() *Zone { z1 := NewZone(z.origin, z.file) - z1.TransferTo = z.TransferTo z1.TransferFrom = z.TransferFrom z1.Expired = z.Expired @@ -134,26 +129,6 @@ func (z *Zone) SetFile(path string) { z.Unlock() } -// TransferAllowed checks if incoming request for transferring the zone is allowed according to the ACLs. -func (z *Zone) TransferAllowed(state request.Request) bool { - for _, t := range z.TransferTo { - if t == "*" { - return true - } - // If remote IP matches we accept. - remote := state.IP() - to, _, err := net.SplitHostPort(t) - if err != nil { - continue - } - if to == remote { - return true - } - } - // TODO(miek): future matching against IP/CIDR notations - return false -} - // ApexIfDefined returns the apex nodes from z. The SOA record is the first record, if it does not exist, an error is returned. func (z *Zone) ApexIfDefined() ([]dns.RR, error) { z.RLock() diff --git a/plugin/kubernetes/README.md b/plugin/kubernetes/README.md index 654e6526f..007e4bd72 100644 --- a/plugin/kubernetes/README.md +++ b/plugin/kubernetes/README.md @@ -40,7 +40,6 @@ kubernetes [ZONES...] { endpoint_pod_names ttl TTL noendpoints - transfer to ADDRESS... fallthrough [ZONES...] ignore empty_service } @@ -90,11 +89,6 @@ kubernetes [ZONES...] { 0 seconds, and the maximum is capped at 3600 seconds. Setting TTL to 0 will prevent records from being cached. * `noendpoints` will turn off the serving of endpoint records by disabling the watch on endpoints. All endpoint queries and headless service queries will result in an NXDOMAIN. -* `transfer` enables zone transfers. It may be specified multiples times. `To` signals the direction - (only `to` is allowed). **ADDRESS** must be denoted in CIDR notation (127.0.0.1/32 etc.) or just as - plain addresses. The special wildcard `*` means: the entire internet. - Sending DNS notifies is not supported. - [Deprecated](https://github.com/kubernetes/dns/blob/master/docs/specification.md#26---deprecated-records) pod records in the subdomain `pod.cluster.local` are not transferred. * `fallthrough` **[ZONES...]** If a query for a record in the zones for which the plugin is authoritative results in NXDOMAIN, normally that is what the response will be. However, if you specify this option, the query will instead be passed on down the plugin chain, which can include another plugin to handle @@ -105,6 +99,8 @@ kubernetes [ZONES...] { This allows the querying pod to continue searching for the service in the search path. The search path could, for example, include another Kubernetes cluster. +Enabling zone transfer is done by using the *transfer* plugin. + ## Ready This plugin reports readiness to the ready plugin. This will happen after it has synced to the @@ -238,3 +234,8 @@ If monitoring is enabled (via the *prometheus* plugin) then the following metric ## Bugs The duration metric only supports the "headless\_with\_selector" service currently. + +## Also See + +See the *autopath* plugin to enable search path optimizations. And use the *transfer* plugin to +enable outgoing zone transfers. diff --git a/plugin/kubernetes/handler.go b/plugin/kubernetes/handler.go index 78761d303..57e9ac528 100644 --- a/plugin/kubernetes/handler.go +++ b/plugin/kubernetes/handler.go @@ -28,8 +28,6 @@ func (k Kubernetes) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.M ) switch state.QType() { - case dns.TypeAXFR, dns.TypeIXFR: - k.Transfer(ctx, state) case dns.TypeA: records, err = plugin.A(ctx, &k, zone, state, nil, plugin.Options{}) case dns.TypeAAAA: diff --git a/plugin/kubernetes/handler_test.go b/plugin/kubernetes/handler_test.go index 302136837..12ebc2765 100644 --- a/plugin/kubernetes/handler_test.go +++ b/plugin/kubernetes/handler_test.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "testing" - "time" "github.com/coredns/coredns/plugin/kubernetes/object" "github.com/coredns/coredns/plugin/pkg/dnstest" @@ -525,7 +524,7 @@ func (APIConnServeTest) Run() {} func (APIConnServeTest) Stop() error { return nil } func (APIConnServeTest) EpIndexReverse(string) []*object.Endpoints { return nil } func (APIConnServeTest) SvcIndexReverse(string) []*object.Service { return nil } -func (APIConnServeTest) Modified() int64 { return time.Now().Unix() } +func (APIConnServeTest) Modified() int64 { return int64(3) } func (APIConnServeTest) PodIndex(ip string) []*object.Pod { if ip != "10.240.0.1" { diff --git a/plugin/kubernetes/kubernetes.go b/plugin/kubernetes/kubernetes.go index 7805bff3d..83b4b02d1 100644 --- a/plugin/kubernetes/kubernetes.go +++ b/plugin/kubernetes/kubernetes.go @@ -46,7 +46,6 @@ type Kubernetes struct { primaryZoneIndex int localIPs []net.IP autoPathSearch []string // Local search path from /etc/resolv.conf. Needed for autopath. - TransferTo []string } // New returns a initialized Kubernetes. It default interfaceAddrFunc to return 127.0.0.1. All other @@ -495,6 +494,12 @@ func (k *Kubernetes) findServices(r recordRequest, zone string) (services []msg. return services, err } +// Serial return the SOA serial. +func (k *Kubernetes) Serial(state request.Request) uint32 { return uint32(k.APIConn.Modified()) } + +// MinTTL returns the minimal TTL. +func (k *Kubernetes) MinTTL(state request.Request) uint32 { return k.ttl } + // match checks if a and b are equal taking wildcards into account. func match(a, b string) bool { if wildcard(a) { diff --git a/plugin/kubernetes/setup.go b/plugin/kubernetes/setup.go index a412f57f5..03bbc6330 100644 --- a/plugin/kubernetes/setup.go +++ b/plugin/kubernetes/setup.go @@ -14,7 +14,6 @@ import ( "github.com/coredns/coredns/plugin" "github.com/coredns/coredns/plugin/pkg/dnsutil" clog "github.com/coredns/coredns/plugin/pkg/log" - "github.com/coredns/coredns/plugin/pkg/parse" "github.com/coredns/coredns/plugin/pkg/upstream" "github.com/miekg/dns" @@ -235,15 +234,6 @@ func ParseStanza(c *caddy.Controller) (*Kubernetes, error) { return nil, c.Errf("ttl must be in range [0, 3600]: %d", t) } k8s.ttl = uint32(t) - case "transfer": - tos, froms, err := parse.Transfer(c, false) - if err != nil { - return nil, err - } - if len(froms) != 0 { - return nil, c.Errf("transfer from is not supported with this plugin") - } - k8s.TransferTo = tos case "noendpoints": if len(c.RemainingArgs()) != 0 { return nil, c.ArgErr() diff --git a/plugin/kubernetes/setup_transfer_test.go b/plugin/kubernetes/setup_transfer_test.go deleted file mode 100644 index 57d665821..000000000 --- a/plugin/kubernetes/setup_transfer_test.go +++ /dev/null @@ -1,47 +0,0 @@ -package kubernetes - -import ( - "testing" - - "github.com/coredns/caddy" -) - -func TestKubernetesParseTransfer(t *testing.T) { - tests := []struct { - input string // Corefile data as string - expected string - shouldErr bool - }{ - {`kubernetes cluster.local { - transfer to 1.2.3.4 - }`, "1.2.3.4:53", false}, - {`kubernetes cluster.local { - transfer to 1.2.3.4:53 - }`, "1.2.3.4:53", false}, - {`kubernetes cluster.local { - transfer to * - }`, "*", false}, - {`kubernetes cluster.local { - transfer - }`, "", true}, - } - - for i, tc := range tests { - c := caddy.NewTestController("dns", tc.input) - k, err := kubernetesParse(c) - if err != nil && !tc.shouldErr { - t.Fatalf("Test %d: Expected no error, got %q", i, err) - } - if err == nil && tc.shouldErr { - t.Fatalf("Test %d: Expected error, got none", i) - } - if err != nil && tc.shouldErr { - // input should error - continue - } - - if k.TransferTo[0] != tc.expected { - t.Errorf("Test %d: Expected Transfer To to be %s, got %s", i, tc.expected, k.TransferTo[0]) - } - } -} diff --git a/plugin/kubernetes/xfr.go b/plugin/kubernetes/xfr.go index a3a0d4a4a..0392f0252 100644 --- a/plugin/kubernetes/xfr.go +++ b/plugin/kubernetes/xfr.go @@ -4,207 +4,151 @@ import ( "context" "math" "net" + "sort" "strings" "github.com/coredns/coredns/plugin" "github.com/coredns/coredns/plugin/etcd/msg" + "github.com/coredns/coredns/plugin/transfer" "github.com/coredns/coredns/request" "github.com/miekg/dns" api "k8s.io/api/core/v1" ) -const transferLength = 2000 - -// Serial implements the Transferer interface. -func (k *Kubernetes) Serial(state request.Request) uint32 { return uint32(k.APIConn.Modified()) } - -// MinTTL implements the Transferer interface. -func (k *Kubernetes) MinTTL(state request.Request) uint32 { return k.ttl } - -// Transfer implements the Transferer interface. -func (k *Kubernetes) Transfer(ctx context.Context, state request.Request) (int, error) { - - if !k.transferAllowed(state) { - return dns.RcodeRefused, nil - } - - // Get all services. - rrs := make(chan dns.RR) - go k.transfer(rrs, state.Zone) - - records := []dns.RR{} - for r := range rrs { - records = append(records, r) - } - - if len(records) == 0 { - return dns.RcodeServerFailure, nil - } - - ch := make(chan *dns.Envelope) - tr := new(dns.Transfer) - - soa, err := plugin.SOA(ctx, k, state.Zone, state, plugin.Options{}) +// Transfer implements the transfer.Transfer interface. +func (k *Kubernetes) Transfer(zone string, serial uint32) (<-chan []dns.RR, error) { + // state is not used here, hence the empty request.Request{] + soa, err := plugin.SOA(context.TODO(), k, zone, request.Request{}, plugin.Options{}) if err != nil { - return dns.RcodeServerFailure, nil + return nil, transfer.ErrNotAuthoritative } - records = append(soa, records...) - records = append(records, soa...) - go func(ch chan *dns.Envelope) { - j, l := 0, 0 - log.Infof("Outgoing transfer of %d records of zone %s to %s started", len(records), state.Zone, state.IP()) - for i, r := range records { - l += dns.Len(r) - if l > transferLength { - ch <- &dns.Envelope{RR: records[j:i]} - l = 0 - j = i - } - } - if j < len(records) { - ch <- &dns.Envelope{RR: records[j:]} - } - close(ch) - }(ch) - - tr.Out(state.W, state.Req, ch) - // Defer closing to the client - state.W.Hijack() - return dns.RcodeSuccess, nil -} - -// transferAllowed checks if incoming request for transferring the zone is allowed according to the ACLs. -// Note: This is copied from zone.transferAllowed, but should eventually be factored into a common transfer pkg. -func (k *Kubernetes) transferAllowed(state request.Request) bool { - for _, t := range k.TransferTo { - if t == "*" { - return true - } - // If remote IP matches we accept. - remote := state.IP() - to, _, err := net.SplitHostPort(t) - if err != nil { - continue - } - if to == remote { - return true - } - } - return false -} - -func (k *Kubernetes) transfer(c chan dns.RR, zone string) { - - defer close(c) + ch := make(chan []dns.RR) zonePath := msg.Path(zone, "coredns") serviceList := k.APIConn.ServiceList() - for _, svc := range serviceList { - if !k.namespaceExposed(svc.Namespace) { - continue + + go func() { + // ixfr fallback + if serial != 0 && soa[0].(*dns.SOA).Serial == serial { + ch <- soa + close(ch) + return } - svcBase := []string{zonePath, Svc, svc.Namespace, svc.Name} - switch svc.Type { - case api.ServiceTypeClusterIP, api.ServiceTypeNodePort, api.ServiceTypeLoadBalancer: - clusterIP := net.ParseIP(svc.ClusterIP) - if clusterIP != nil { - s := msg.Service{Host: svc.ClusterIP, TTL: k.ttl} - s.Key = strings.Join(svcBase, "/") + ch <- soa - // Change host from IP to Name for SRV records - host := emitAddressRecord(c, s) + sort.Slice(serviceList, func(i, j int) bool { + return serviceList[i].Name < serviceList[j].Name + }) - for _, p := range svc.Ports { - s := msg.Service{Host: host, Port: int(p.Port), TTL: k.ttl} - s.Key = strings.Join(svcBase, "/") - - // Need to generate this to handle use cases for peer-finder - // ref: https://github.com/coredns/coredns/pull/823 - c <- s.NewSRV(msg.Domain(s.Key), 100) - - // As per spec unnamed ports do not have a srv record - // https://github.com/kubernetes/dns/blob/master/docs/specification.md#232---srv-records - if p.Name == "" { - continue - } - - s.Key = strings.Join(append(svcBase, strings.ToLower("_"+string(p.Protocol)), strings.ToLower("_"+string(p.Name))), "/") - - c <- s.NewSRV(msg.Domain(s.Key), 100) - } - - // Skip endpoint discovery if clusterIP is defined + for _, svc := range serviceList { + if !k.namespaceExposed(svc.Namespace) { continue } + svcBase := []string{zonePath, Svc, svc.Namespace, svc.Name} + switch svc.Type { - endpointsList := k.APIConn.EpIndex(svc.Name + "." + svc.Namespace) + case api.ServiceTypeClusterIP, api.ServiceTypeNodePort, api.ServiceTypeLoadBalancer: + clusterIP := net.ParseIP(svc.ClusterIP) + if clusterIP != nil { + s := msg.Service{Host: svc.ClusterIP, TTL: k.ttl} + s.Key = strings.Join(svcBase, "/") - for _, ep := range endpointsList { - if ep.Name != svc.Name || ep.Namespace != svc.Namespace { + // Change host from IP to Name for SRV records + host := emitAddressRecord(ch, s) + + for _, p := range svc.Ports { + s := msg.Service{Host: host, Port: int(p.Port), TTL: k.ttl} + s.Key = strings.Join(svcBase, "/") + + // Need to generate this to handle use cases for peer-finder + // ref: https://github.com/coredns/coredns/pull/823 + ch <- []dns.RR{s.NewSRV(msg.Domain(s.Key), 100)} + + // As per spec unnamed ports do not have a srv record + // https://github.com/kubernetes/dns/blob/master/docs/specification.md#232---srv-records + if p.Name == "" { + continue + } + + s.Key = strings.Join(append(svcBase, strings.ToLower("_"+string(p.Protocol)), strings.ToLower("_"+string(p.Name))), "/") + + ch <- []dns.RR{s.NewSRV(msg.Domain(s.Key), 100)} + } + + // Skip endpoint discovery if clusterIP is defined continue } - for _, eps := range ep.Subsets { - srvWeight := calcSRVWeight(len(eps.Addresses)) - for _, addr := range eps.Addresses { - s := msg.Service{Host: addr.IP, TTL: k.ttl} - s.Key = strings.Join(svcBase, "/") - // We don't need to change the msg.Service host from IP to Name yet - // so disregard the return value here - emitAddressRecord(c, s) + endpointsList := k.APIConn.EpIndex(svc.Name + "." + svc.Namespace) - s.Key = strings.Join(append(svcBase, endpointHostname(addr, k.endpointNameMode)), "/") - // Change host from IP to Name for SRV records - host := emitAddressRecord(c, s) - s.Host = host + for _, ep := range endpointsList { + if ep.Name != svc.Name || ep.Namespace != svc.Namespace { + continue + } - for _, p := range eps.Ports { - // As per spec unnamed ports do not have a srv record - // https://github.com/kubernetes/dns/blob/master/docs/specification.md#232---srv-records - if p.Name == "" { - continue + for _, eps := range ep.Subsets { + srvWeight := calcSRVWeight(len(eps.Addresses)) + for _, addr := range eps.Addresses { + s := msg.Service{Host: addr.IP, TTL: k.ttl} + s.Key = strings.Join(svcBase, "/") + // We don't need to change the msg.Service host from IP to Name yet + // so disregard the return value here + emitAddressRecord(ch, s) + + s.Key = strings.Join(append(svcBase, endpointHostname(addr, k.endpointNameMode)), "/") + // Change host from IP to Name for SRV records + host := emitAddressRecord(ch, s) + s.Host = host + + for _, p := range eps.Ports { + // As per spec unnamed ports do not have a srv record + // https://github.com/kubernetes/dns/blob/master/docs/specification.md#232---srv-records + if p.Name == "" { + continue + } + + s.Port = int(p.Port) + + s.Key = strings.Join(append(svcBase, strings.ToLower("_"+string(p.Protocol)), strings.ToLower("_"+string(p.Name))), "/") + ch <- []dns.RR{s.NewSRV(msg.Domain(s.Key), srvWeight)} } - - s.Port = int(p.Port) - - s.Key = strings.Join(append(svcBase, strings.ToLower("_"+string(p.Protocol)), strings.ToLower("_"+string(p.Name))), "/") - c <- s.NewSRV(msg.Domain(s.Key), srvWeight) } } } - } - case api.ServiceTypeExternalName: + case api.ServiceTypeExternalName: - s := msg.Service{Key: strings.Join(svcBase, "/"), Host: svc.ExternalName, TTL: k.ttl} - if t, _ := s.HostType(); t == dns.TypeCNAME { - c <- s.NewCNAME(msg.Domain(s.Key), s.Host) + s := msg.Service{Key: strings.Join(svcBase, "/"), Host: svc.ExternalName, TTL: k.ttl} + if t, _ := s.HostType(); t == dns.TypeCNAME { + ch <- []dns.RR{s.NewCNAME(msg.Domain(s.Key), s.Host)} + } } } - } + ch <- soa + close(ch) + }() + return ch, nil } -// emitAddressRecord generates a new A or AAAA record based on the msg.Service and writes it to -// a channel. +// emitAddressRecord generates a new A or AAAA record based on the msg.Service and writes it to a channel. // emitAddressRecord returns the host name from the generated record. -func emitAddressRecord(c chan dns.RR, message msg.Service) string { - ip := net.ParseIP(message.Host) - var host string - dnsType, _ := message.HostType() +func emitAddressRecord(c chan<- []dns.RR, s msg.Service) string { + ip := net.ParseIP(s.Host) + dnsType, _ := s.HostType() switch dnsType { case dns.TypeA: - arec := message.NewA(msg.Domain(message.Key), ip) - host = arec.Hdr.Name - c <- arec + r := s.NewA(msg.Domain(s.Key), ip) + c <- []dns.RR{r} + return r.Hdr.Name case dns.TypeAAAA: - arec := message.NewAAAA(msg.Domain(message.Key), ip) - host = arec.Hdr.Name - c <- arec + r := s.NewAAAA(msg.Domain(s.Key), ip) + c <- []dns.RR{r} + return r.Hdr.Name } - return host + return "" } // calcSrvWeight borrows the logic implemented in plugin.SRV for dynamically diff --git a/plugin/kubernetes/xfr_test.go b/plugin/kubernetes/xfr_test.go index 1ada4f7aa..b5f13ad6e 100644 --- a/plugin/kubernetes/xfr_test.go +++ b/plugin/kubernetes/xfr_test.go @@ -1,229 +1,126 @@ package kubernetes import ( - "context" "strings" "testing" - "github.com/coredns/coredns/plugin/kubernetes/object" - "github.com/coredns/coredns/plugin/pkg/dnstest" - "github.com/coredns/coredns/plugin/test" - "github.com/miekg/dns" ) -func TestKubernetesXFR(t *testing.T) { +func TestKubernetesAXFR(t *testing.T) { k := New([]string{"cluster.local."}) k.APIConn = &APIConnServeTest{} - k.TransferTo = []string{"10.240.0.1:53"} k.Namespaces = map[string]struct{}{"testns": {}} - ctx := context.TODO() - w := dnstest.NewMultiRecorder(&test.ResponseWriter{}) dnsmsg := &dns.Msg{} dnsmsg.SetAxfr(k.Zones[0]) - _, err := k.ServeDNS(ctx, w, dnsmsg) + ch, err := k.Transfer(k.Zones[0], 0) + if err != nil { + t.Error(err) + } + validateAXFR(t, ch) +} + +func TestKubernetesIXFRFallback(t *testing.T) { + k := New([]string{"cluster.local."}) + k.APIConn = &APIConnServeTest{} + k.Namespaces = map[string]struct{}{"testns": {}} + + dnsmsg := &dns.Msg{} + dnsmsg.SetAxfr(k.Zones[0]) + + ch, err := k.Transfer(k.Zones[0], 1) + if err != nil { + t.Error(err) + } + validateAXFR(t, ch) +} + +func TestKubernetesIXFRCurrent(t *testing.T) { + k := New([]string{"cluster.local."}) + k.APIConn = &APIConnServeTest{} + k.Namespaces = map[string]struct{}{"testns": {}} + + dnsmsg := &dns.Msg{} + dnsmsg.SetAxfr(k.Zones[0]) + + ch, err := k.Transfer(k.Zones[0], 3) if err != nil { t.Error(err) } - if len(w.Msgs) == 0 { - t.Logf("%+v\n", w) - t.Fatal("Did not get back a zone response") + var gotRRs []dns.RR + for rrs := range ch { + gotRRs = append(gotRRs, rrs...) } - if len(w.Msgs[0].Answer) == 0 { - t.Logf("%+v\n", w) - t.Fatal("Did not get back an answer") + // ensure only one record is returned + if len(gotRRs) > 1 { + t.Errorf("Expected only one answer, got %d", len(gotRRs)) } - // Ensure xfr starts with SOA - if w.Msgs[0].Answer[0].Header().Rrtype != dns.TypeSOA { - t.Error("Invalid XFR, does not start with SOA record") - } - - // Ensure xfr starts with SOA - // Last message is empty, so we need to go back one further - if w.Msgs[len(w.Msgs)-2].Answer[len(w.Msgs[len(w.Msgs)-2].Answer)-1].Header().Rrtype != dns.TypeSOA { - t.Error("Invalid XFR, does not end with SOA record") - } - - testRRs := []dns.RR{} - for _, tc := range dnsTestCases { - if tc.Rcode != dns.RcodeSuccess { - continue - } - - for _, ans := range tc.Answer { - // Exclude wildcard searches - if strings.Contains(ans.Header().Name, "*") { - continue - } - - // Exclude TXT records - if ans.Header().Rrtype == dns.TypeTXT { - continue - } - testRRs = append(testRRs, ans) - } - } - - gotRRs := []dns.RR{} - for _, resp := range w.Msgs { - for _, ans := range resp.Answer { - // Skip SOA records since these - // test cases do not exist - if ans.Header().Rrtype == dns.TypeSOA { - continue - } - - gotRRs = append(gotRRs, ans) - } - - } - - diff := difference(testRRs, gotRRs) - if len(diff) != 0 { - t.Errorf("Got back %d records that do not exist in test cases, should be 0:", len(diff)) - for _, rec := range diff { - t.Errorf("%+v", rec) - } - } - - diff = difference(gotRRs, testRRs) - if len(diff) != 0 { - t.Errorf("Found %d records we're missing, should be 0:", len(diff)) - for _, rec := range diff { - t.Errorf("%+v", rec) - } + // Ensure first record is a SOA + if gotRRs[0].Header().Rrtype != dns.TypeSOA { + t.Error("Invalid transfer response, does not start with SOA record") } } -func TestKubernetesXFRNotAllowed(t *testing.T) { - k := New([]string{"cluster.local."}) - k.APIConn = &APIConnServeTest{} - k.TransferTo = []string{"1.2.3.4:53"} - k.Namespaces = map[string]struct{}{"testns": {}} - - ctx := context.TODO() - w := dnstest.NewMultiRecorder(&test.ResponseWriter{}) - dnsmsg := &dns.Msg{} - dnsmsg.SetAxfr(k.Zones[0]) - - _, err := k.ServeDNS(ctx, w, dnsmsg) - if err != nil { - t.Error(err) +func validateAXFR(t *testing.T, ch <-chan []dns.RR) { + xfr := []dns.RR{} + for rrs := range ch { + xfr = append(xfr, rrs...) + } + if xfr[0].Header().Rrtype != dns.TypeSOA { + t.Error("Invalid transfer response, does not start with SOA record") } - if len(w.Msgs) == 0 { - t.Logf("%+v\n", w) - t.Fatal("Did not get back a zone response") + zp := dns.NewZoneParser(strings.NewReader(expectedZone), "", "") + i := 0 + for rr, ok := zp.Next(); ok; rr, ok = zp.Next() { + if !dns.IsDuplicate(rr, xfr[i]) { + t.Fatalf("Record %d, expected\n%v\n, got\n%v", i, rr, xfr[i]) + } + i++ } - if len(w.Msgs[0].Answer) != 0 { - t.Logf("%+v\n", w) - t.Fatal("Got an answer, should not have") + if err := zp.Err(); err != nil { + t.Fatal(err) } } -// difference shows what we're missing when comparing two RR slices -func difference(testRRs []dns.RR, gotRRs []dns.RR) []dns.RR { - expectedRRs := map[string]struct{}{} - for _, rr := range testRRs { - expectedRRs[rr.String()] = struct{}{} - } - - foundRRs := []dns.RR{} - for _, rr := range gotRRs { - if _, ok := expectedRRs[rr.String()]; !ok { - foundRRs = append(foundRRs, rr) - } - } - return foundRRs -} - -func TestEndpointsEquivalent(t *testing.T) { - epA := object.Endpoints{ - Subsets: []object.EndpointSubset{{ - Addresses: []object.EndpointAddress{{IP: "1.2.3.4", Hostname: "foo"}}, - }}, - } - epB := object.Endpoints{ - Subsets: []object.EndpointSubset{{ - Addresses: []object.EndpointAddress{{IP: "1.2.3.4", Hostname: "foo"}}, - }}, - } - epC := object.Endpoints{ - Subsets: []object.EndpointSubset{{ - Addresses: []object.EndpointAddress{{IP: "1.2.3.5", Hostname: "foo"}}, - }}, - } - epD := object.Endpoints{ - Subsets: []object.EndpointSubset{{ - Addresses: []object.EndpointAddress{{IP: "1.2.3.5", Hostname: "foo"}}, - }, - { - Addresses: []object.EndpointAddress{{IP: "1.2.2.2", Hostname: "foofoo"}}, - }}, - } - epE := object.Endpoints{ - Subsets: []object.EndpointSubset{{ - Addresses: []object.EndpointAddress{{IP: "1.2.3.5", Hostname: "foo"}, {IP: "1.1.1.1"}}, - }}, - } - epF := object.Endpoints{ - Subsets: []object.EndpointSubset{{ - Addresses: []object.EndpointAddress{{IP: "1.2.3.4", Hostname: "foofoo"}}, - }}, - } - epG := object.Endpoints{ - Subsets: []object.EndpointSubset{{ - Addresses: []object.EndpointAddress{{IP: "1.2.3.4", Hostname: "foo"}}, - Ports: []object.EndpointPort{{Name: "http", Port: 80, Protocol: "TCP"}}, - }}, - } - epH := object.Endpoints{ - Subsets: []object.EndpointSubset{{ - Addresses: []object.EndpointAddress{{IP: "1.2.3.4", Hostname: "foo"}}, - Ports: []object.EndpointPort{{Name: "newportname", Port: 80, Protocol: "TCP"}}, - }}, - } - epI := object.Endpoints{ - Subsets: []object.EndpointSubset{{ - Addresses: []object.EndpointAddress{{IP: "1.2.3.4", Hostname: "foo"}}, - Ports: []object.EndpointPort{{Name: "http", Port: 8080, Protocol: "TCP"}}, - }}, - } - epJ := object.Endpoints{ - Subsets: []object.EndpointSubset{{ - Addresses: []object.EndpointAddress{{IP: "1.2.3.4", Hostname: "foo"}}, - Ports: []object.EndpointPort{{Name: "http", Port: 80, Protocol: "UDP"}}, - }}, - } - - tests := []struct { - equiv bool - a *object.Endpoints - b *object.Endpoints - }{ - {true, &epA, &epB}, - {false, &epA, &epC}, - {false, &epA, &epD}, - {false, &epA, &epE}, - {false, &epA, &epF}, - {false, &epF, &epG}, - {false, &epG, &epH}, - {false, &epG, &epI}, - {false, &epG, &epJ}, - } - - for i, tc := range tests { - if tc.equiv && !endpointsEquivalent(tc.a, tc.b) { - t.Errorf("Test %d: expected endpoints to be equivalent and they are not.", i) - } - if !tc.equiv && endpointsEquivalent(tc.a, tc.b) { - t.Errorf("Test %d: expected endpoints to be seen as different but they were not.", i) - } - } -} +const expectedZone = ` +cluster.local. 5 IN SOA ns.dns.cluster.local. hostmaster.cluster.local. 3 7200 1800 86400 5 +external.testns.svc.cluster.local. 5 IN CNAME ext.interwebs.test. +external-to-service.testns.svc.cluster.local. 5 IN CNAME svc1.testns.svc.cluster.local. +hdls1.testns.svc.cluster.local. 5 IN A 172.0.0.2 +172-0-0-2.hdls1.testns.svc.cluster.local. 5 IN A 172.0.0.2 +_http._tcp.hdls1.testns.svc.cluster.local. 5 IN SRV 0 16 80 172-0-0-2.hdls1.testns.svc.cluster.local. +hdls1.testns.svc.cluster.local. 5 IN A 172.0.0.3 +172-0-0-3.hdls1.testns.svc.cluster.local. 5 IN A 172.0.0.3 +_http._tcp.hdls1.testns.svc.cluster.local. 5 IN SRV 0 16 80 172-0-0-3.hdls1.testns.svc.cluster.local. +hdls1.testns.svc.cluster.local. 5 IN A 172.0.0.4 +dup-name.hdls1.testns.svc.cluster.local. 5 IN A 172.0.0.4 +_http._tcp.hdls1.testns.svc.cluster.local. 5 IN SRV 0 16 80 dup-name.hdls1.testns.svc.cluster.local. +hdls1.testns.svc.cluster.local. 5 IN A 172.0.0.5 +dup-name.hdls1.testns.svc.cluster.local. 5 IN A 172.0.0.5 +_http._tcp.hdls1.testns.svc.cluster.local. 5 IN SRV 0 16 80 dup-name.hdls1.testns.svc.cluster.local. +hdls1.testns.svc.cluster.local. 5 IN AAAA 5678:abcd::1 +5678-abcd--1.hdls1.testns.svc.cluster.local. 5 IN AAAA 5678:abcd::1 +_http._tcp.hdls1.testns.svc.cluster.local. 5 IN SRV 0 16 80 5678-abcd--1.hdls1.testns.svc.cluster.local. +hdls1.testns.svc.cluster.local. 5 IN AAAA 5678:abcd::2 +5678-abcd--2.hdls1.testns.svc.cluster.local. 5 IN AAAA 5678:abcd::2 +_http._tcp.hdls1.testns.svc.cluster.local. 5 IN SRV 0 16 80 5678-abcd--2.hdls1.testns.svc.cluster.local. +hdlsprtls.testns.svc.cluster.local. 5 IN A 172.0.0.20 +172-0-0-20.hdlsprtls.testns.svc.cluster.local. 5 IN A 172.0.0.20 +svc1.testns.svc.cluster.local. 5 IN A 10.0.0.1 +svc1.testns.svc.cluster.local. 5 IN SRV 0 100 80 svc1.testns.svc.cluster.local. +_http._tcp.svc1.testns.svc.cluster.local. 5 IN SRV 0 100 80 svc1.testns.svc.cluster.local. +svc6.testns.svc.cluster.local. 5 IN AAAA 1234:abcd::1 +svc6.testns.svc.cluster.local. 5 IN SRV 0 100 80 svc6.testns.svc.cluster.local. +_http._tcp.svc6.testns.svc.cluster.local. 5 IN SRV 0 100 80 svc6.testns.svc.cluster.local. +svcempty.testns.svc.cluster.local. 5 IN A 10.0.0.1 +svcempty.testns.svc.cluster.local. 5 IN SRV 0 100 80 svcempty.testns.svc.cluster.local. +_http._tcp.svcempty.testns.svc.cluster.local. 5 IN SRV 0 100 80 svcempty.testns.svc.cluster.local. +cluster.local. 5 IN SOA ns.dns.cluster.local. hostmaster.cluster.local. 3 7200 1800 86400 5 +` diff --git a/plugin/pkg/parse/parse.go b/plugin/pkg/parse/parse.go index 356d6c21c..300a57a8b 100644 --- a/plugin/pkg/parse/parse.go +++ b/plugin/pkg/parse/parse.go @@ -8,41 +8,31 @@ import ( "github.com/coredns/coredns/plugin/pkg/transport" ) -// Transfer parses transfer statements: 'transfer [to|from] [address...]'. -func Transfer(c *caddy.Controller, secondary bool) (tos, froms []string, err error) { +// TransferIn parses transfer statements: 'transfer from [address...]'. +func TransferIn(c *caddy.Controller) (froms []string, err error) { if !c.NextArg() { - return nil, nil, c.ArgErr() + return nil, c.ArgErr() } value := c.Val() switch value { - case "to": - tos = c.RemainingArgs() - for i := range tos { - if tos[i] != "*" { - normalized, err := HostPort(tos[i], transport.Port) - if err != nil { - return nil, nil, err - } - tos[i] = normalized - } - } - + default: + return nil, c.Errf("unknown property %s", value) case "from": - if !secondary { - return nil, nil, fmt.Errorf("can't use `transfer from` when not being a secondary") - } froms = c.RemainingArgs() + if len(froms) == 0 { + return nil, c.ArgErr() + } for i := range froms { if froms[i] != "*" { normalized, err := HostPort(froms[i], transport.Port) if err != nil { - return nil, nil, err + return nil, err } froms[i] = normalized } else { - return nil, nil, fmt.Errorf("can't use '*' in transfer from") + return nil, fmt.Errorf("can't use '*' in transfer from") } } } - return + return froms, nil } diff --git a/plugin/pkg/parse/parse_test.go b/plugin/pkg/parse/parse_test.go index e0f817f49..b0ab2c601 100644 --- a/plugin/pkg/parse/parse_test.go +++ b/plugin/pkg/parse/parse_test.go @@ -6,65 +6,41 @@ import ( "github.com/coredns/caddy" ) -func TestTransfer(t *testing.T) { +func TestTransferIn(t *testing.T) { tests := []struct { inputFileRules string shouldErr bool - secondary bool - expectedTo []string expectedFrom []string }{ - // OK transfer to - { - `to 127.0.0.1`, - false, false, []string{"127.0.0.1:53"}, []string{}, - }, - // OK transfer tos - { - `to 127.0.0.1 127.0.0.2`, - false, false, []string{"127.0.0.1:53", "127.0.0.2:53"}, []string{}, - }, - // OK transfer from { `from 127.0.0.1`, - false, true, []string{}, []string{"127.0.0.1:53"}, + false, []string{"127.0.0.1:53"}, }, // OK transfer froms { `from 127.0.0.1 127.0.0.2`, - false, true, []string{}, []string{"127.0.0.1:53", "127.0.0.2:53"}, - }, - // OK transfer tos/froms - { - `to 127.0.0.1 127.0.0.2 - from 127.0.0.1 127.0.0.2`, - false, true, []string{"127.0.0.1:53", "127.0.0.2:53"}, []string{"127.0.0.1:53", "127.0.0.2:53"}, - }, - // Bad transfer from, secondary false - { - `from 127.0.0.1`, - true, false, []string{}, []string{}, + false, []string{"127.0.0.1:53", "127.0.0.2:53"}, }, // Bad transfer from garbage { `from !@#$%^&*()`, - true, true, []string{}, []string{}, + true, []string{}, }, // Bad transfer from no args { `from`, - true, false, []string{}, []string{}, + true, []string{}, }, // Bad transfer from * { `from *`, - true, true, []string{}, []string{}, + true, []string{}, }, } for i, test := range tests { c := caddy.NewTestController("dns", test.inputFileRules) - tos, froms, err := Transfer(c, test.secondary) + froms, err := TransferIn(c) if err == nil && test.shouldErr { t.Fatalf("Test %d expected errors, but got no error %+v %+v", i, err, test) @@ -72,13 +48,6 @@ func TestTransfer(t *testing.T) { t.Fatalf("Test %d expected no errors, but got '%v'", i, err) } - if test.expectedTo != nil { - for j, got := range tos { - if got != test.expectedTo[j] { - t.Fatalf("Test %d expected %v, got %v", i, test.expectedTo[j], got) - } - } - } if test.expectedFrom != nil { for j, got := range froms { if got != test.expectedFrom[j] { diff --git a/plugin/secondary/README.md b/plugin/secondary/README.md index dab740284..1405627c0 100644 --- a/plugin/secondary/README.md +++ b/plugin/secondary/README.md @@ -24,17 +24,16 @@ A working syntax would be: ~~~ secondary [zones...] { transfer from ADDRESS - transfer to ADDRESS } ~~~ * `transfer from` specifies from which address to fetch the zone. It can be specified multiple times; - if one does not work, another will be tried. -* `transfer to` can be enabled to allow this secondary zone to be transferred again. + if one does not work, another will be tried. Transfering this zone outwards again can be done by + enableing the *transfer* plugin. When a zone is due to be refreshed (Refresh timer fires) a random jitter of 5 seconds is applied, before fetching. In the case of retry this will be 2 seconds. If there are any errors -during the transfer the transfer fails; this will be logged. +during the transfer in, the transfer fails; this will be logged. ## Examples @@ -43,8 +42,7 @@ Transfer `example.org` from 10.0.1.1, and if that fails try 10.1.2.1. ~~~ corefile example.org { secondary { - transfer from 10.0.1.1 - transfer from 10.1.2.1 + transfer from 10.0.1.1 10.1.2.1 } } ~~~ @@ -52,10 +50,12 @@ example.org { Or re-export the retrieved zone to other secondaries. ~~~ corefile -. { - secondary example.net { +example.net { + secondary { transfer from 10.1.2.1 - transfer to * + } + transfer { + to * } } ~~~ @@ -63,3 +63,7 @@ Or re-export the retrieved zone to other secondaries. ## Bugs Only AXFR is supported and the retrieved zone is not committed to disk. + +## Also See + +See the *transfer* plugin to enable zone transfers _to_ other servers. diff --git a/plugin/secondary/setup.go b/plugin/secondary/setup.go index 108415290..83d8e56ef 100644 --- a/plugin/secondary/setup.go +++ b/plugin/secondary/setup.go @@ -43,7 +43,6 @@ func setup(c *caddy.Controller) error { func secondaryParse(c *caddy.Controller) (file.Zones, error) { z := make(map[string]*file.Zone) names := []string{} - upstr := upstream.New() for c.Next() { if c.Val() == "secondary" { @@ -62,30 +61,24 @@ func secondaryParse(c *caddy.Controller) (file.Zones, error) { for c.NextBlock() { - t, f := []string{}, []string{} - var e error + f := []string{} switch c.Val() { case "transfer": - t, f, e = parse.Transfer(c, true) - if e != nil { - return file.Zones{}, e + var err error + f, err = parse.TransferIn(c) + if err != nil { + return file.Zones{}, err } - case "upstream": - // remove soon - c.RemainingArgs() default: return file.Zones{}, c.Errf("unknown property '%s'", c.Val()) } for _, origin := range origins { - if t != nil { - z[origin].TransferTo = append(z[origin].TransferTo, t...) - } if f != nil { z[origin].TransferFrom = append(z[origin].TransferFrom, f...) } - z[origin].Upstream = upstr + z[origin].Upstream = upstream.New() } } } diff --git a/plugin/secondary/setup_test.go b/plugin/secondary/setup_test.go index bd60ba69c..4985ec5b6 100644 --- a/plugin/secondary/setup_test.go +++ b/plugin/secondary/setup_test.go @@ -22,7 +22,6 @@ func TestSecondaryParse(t *testing.T) { { `secondary { transfer from 127.0.0.1 - transfer to 127.0.0.1 }`, false, "127.0.0.1:53", @@ -31,7 +30,6 @@ func TestSecondaryParse(t *testing.T) { { `secondary example.org { transfer from 127.0.0.1 - transfer to 127.0.0.1 }`, false, "127.0.0.1:53", diff --git a/plugin/transfer/README.md b/plugin/transfer/README.md index 30f797e5b..8924a1e79 100644 --- a/plugin/transfer/README.md +++ b/plugin/transfer/README.md @@ -2,34 +2,38 @@ ## Name -*transfer* - perform zone transfers for other plugins. +*transfer* - perform (outgoing) zone transfers for other plugins. ## Description -This plugin answers zone transfers for authoritative plugins that implement -`transfer.Transferer`. Currently, no internal plugins implement this interface. +This plugin answers zone transfers for authoritative plugins that implement `transfer.Transferer`. -Transfer answers full zone transfer (AXFR) requests and incremental zone transfer (IXFR) requests +*transfer* answers full zone transfer (AXFR) requests and incremental zone transfer (IXFR) requests with AXFR fallback if the zone has changed. -Notifies are not currently supported. +When a plugin wants to notify it's secondaries it will call back into the *transfer* plugin. + +The following plugins implement zone transfers using this plugin: *file*, *auto*, *secondary*, and +*kubernetes*. See `transfer.go` for implementation details if you are a plugin author that wants to +use this plugin. ## Syntax ~~~ transfer [ZONE...] { - to HOST... + to ADDRESS... } ~~~ -* **ZONES** The zones *transfer* will answer zone requests for. If left blank, - the zones are inherited from the enclosing server block. To answer zone - transfers for a given zone, there must be another plugin in the same server - block that serves the same zone, and implements `transfer.Transferer`. + * **ZONE** The zones *transfer* will answer zone transfer requests for. If left blank, the zones + are inherited from the enclosing server block. To answer zone transfers for a given zone, + there must be another plugin in the same server block that serves the same zone, and implements + `transfer.Transferer`. -* `to ` **HOST...** The hosts *transfer* will transfer to. Use `*` to permit - transfers to all hosts. + * `to` **ADDRESS...** The hosts *transfer* will transfer to. Use `*` to permit transfers to all + addresses. **ADDRESS** must be denoted in CIDR notation (e.g., 127.0.0.1/32) or just as plain + addresses. `to` may be specified multiple times. ## Examples -TODO +See the specific plugins using this plugin for examples on it's usage. diff --git a/plugin/transfer/notify.go b/plugin/transfer/notify.go new file mode 100644 index 000000000..b024a3aa1 --- /dev/null +++ b/plugin/transfer/notify.go @@ -0,0 +1,58 @@ +package transfer + +import ( + "fmt" + + "github.com/coredns/coredns/plugin/pkg/rcode" + + "github.com/miekg/dns" +) + +// Notify will send notifies to all configured to hosts IP addresses. If the zone isn't known +// to t an error will be returned. The string zone must be lowercased. +func (t *Transfer) Notify(zone string) error { + if t == nil { // t might be nil, mostly expected in tests, so intercept and to a noop in that case + return nil + } + + m := new(dns.Msg) + m.SetNotify(zone) + c := new(dns.Client) + + x := longestMatch(t.xfrs, zone) + if x == nil { + return fmt.Errorf("no such zone registred in the transfer plugin: %s", zone) + } + + var err1 error + for _, t := range x.to { + if t == "*" { + continue + } + if err := sendNotify(c, m, t); err != nil { + err1 = err + } + } + log.Debugf("Sent notifies for zone %q to %v", zone, x.to) + return err1 // this only captures the last error +} + +func sendNotify(c *dns.Client, m *dns.Msg, s string) error { + var err error + + code := dns.RcodeServerFailure + for i := 0; i < 3; i++ { + ret, _, err := c.Exchange(m, s) + if err != nil { + continue + } + code = ret.Rcode + if code == dns.RcodeSuccess { + return nil + } + } + if err != nil { + return fmt.Errorf("notify for zone %q was not accepted by %q: %q", m.Question[0].Name, s, err) + } + return fmt.Errorf("notify for zone %q was not accepted by %q: rcode was %q", m.Question[0].Name, s, rcode.ToString(code)) +} diff --git a/plugin/transfer/select_test.go b/plugin/transfer/select_test.go new file mode 100644 index 000000000..6cb0d7681 --- /dev/null +++ b/plugin/transfer/select_test.go @@ -0,0 +1,58 @@ +package transfer + +import ( + "context" + "fmt" + "testing" + + "github.com/coredns/coredns/plugin/pkg/dnstest" + "github.com/coredns/coredns/plugin/test" + + "github.com/miekg/dns" +) + +type ( + t1 struct{} + t2 struct{} +) + +func (t t1) Transfer(zone string, serial uint32) (<-chan []dns.RR, error) { + const z = "example.org." + if zone != z { + return nil, ErrNotAuthoritative + } + return nil, fmt.Errorf(z) +} +func (t t2) Transfer(zone string, serial uint32) (<-chan []dns.RR, error) { + const z = "sub.example.org." + if zone != z { + return nil, ErrNotAuthoritative + } + return nil, fmt.Errorf(z) +} + +func TestZoneSelection(t *testing.T) { + tr := &Transfer{ + Transferers: []Transferer{t1{}, t2{}}, + xfrs: []*xfr{ + { + Zones: []string{"example.org."}, + to: []string{"192.0.2.1"}, // RFC 5737 IP, no interface should have this address. + }, + { + Zones: []string{"sub.example.org."}, + to: []string{"*"}, + }, + }, + } + r := new(dns.Msg) + r.SetAxfr("sub.example.org.") + w := dnstest.NewRecorder(&test.ResponseWriter{}) + _, err := tr.ServeDNS(context.TODO(), w, r) + if err == nil { + t.Fatal("Expected error, got nil") + } + if x := err.Error(); x != "sub.example.org." { + t.Errorf("Expected transfer for zone %s, got %s", "sub.example.org", x) + } +} diff --git a/plugin/transfer/setup.go b/plugin/transfer/setup.go index 528e53847..e92d442bc 100644 --- a/plugin/transfer/setup.go +++ b/plugin/transfer/setup.go @@ -4,7 +4,7 @@ import ( "github.com/coredns/caddy" "github.com/coredns/coredns/core/dnsserver" "github.com/coredns/coredns/plugin" - parsepkg "github.com/coredns/coredns/plugin/pkg/parse" + "github.com/coredns/coredns/plugin/pkg/parse" "github.com/coredns/coredns/plugin/pkg/transport" ) @@ -16,7 +16,7 @@ func init() { } func setup(c *caddy.Controller) error { - t, err := parse(c) + t, err := parseTransfer(c) if err != nil { return plugin.Error("transfer", err) @@ -43,8 +43,7 @@ func setup(c *caddy.Controller) error { return nil } -func parse(c *caddy.Controller) (*Transfer, error) { - +func parseTransfer(c *caddy.Controller) (*Transfer, error) { t := &Transfer{} for c.Next() { x := &xfr{} @@ -82,14 +81,14 @@ func parse(c *caddy.Controller) (*Transfer, error) { x.to = append(x.to, host) continue } - normalized, err := parsepkg.HostPort(host, transport.Port) + normalized, err := parse.HostPort(host, transport.Port) if err != nil { return nil, err } x.to = append(x.to, normalized) } default: - return nil, plugin.Error("transfer", c.Errf("unknown property '%s'", c.Val())) + return nil, plugin.Error("transfer", c.Errf("unknown property %q", c.Val())) } } if len(x.to) == 0 { diff --git a/plugin/transfer/setup_test.go b/plugin/transfer/setup_test.go index dfbbc3950..d46eec17c 100644 --- a/plugin/transfer/setup_test.go +++ b/plugin/transfer/setup_test.go @@ -6,12 +6,6 @@ import ( "github.com/coredns/caddy" ) -func newTestControllerWithZones(input string, zones []string) *caddy.Controller { - ctr := caddy.NewTestController("dns", input) - ctr.ServerBlockKeys = append(ctr.ServerBlockKeys, zones...) - return ctr -} - func TestParse(t *testing.T) { tests := []struct { input string @@ -75,8 +69,10 @@ func TestParse(t *testing.T) { }, } for i, tc := range tests { - c := newTestControllerWithZones(tc.input, tc.zones) - transfer, err := parse(c) + c := caddy.NewTestController("dns", tc.input) + c.ServerBlockKeys = append(c.ServerBlockKeys, tc.zones...) + + transfer, err := parseTransfer(c) if err == nil && tc.shouldErr { t.Fatalf("Test %d expected errors, but got no error", i) diff --git a/plugin/transfer/transfer.go b/plugin/transfer/transfer.go index 1112ffb9e..af27cc717 100644 --- a/plugin/transfer/transfer.go +++ b/plugin/transfer/transfer.go @@ -3,7 +3,6 @@ package transfer import ( "context" "errors" - "fmt" "net" "sync" @@ -18,9 +17,9 @@ var log = clog.NewWithPlugin("transfer") // Transfer is a plugin that handles zone transfers. type Transfer struct { - Transferers []Transferer // the list of plugins that implement Transferer + Transferers []Transferer // List of plugins that implement Transferer xfrs []*xfr - Next plugin.Handler // the next plugin in the chain + Next plugin.Handler } type xfr struct { @@ -32,53 +31,53 @@ type xfr struct { type Transferer interface { // Transfer returns a channel to which it writes responses to the transfer request. // If the plugin is not authoritative for the zone, it should immediately return the - // Transfer.ErrNotAuthoritative error. + // transfer.ErrNotAuthoritative error. This is important otherwise the transfer plugin can + // use plugin X while it should transfer the data from plugin Y. // // If serial is 0, handle as an AXFR request. Transfer should send all records // in the zone to the channel. The SOA should be written to the channel first, followed - // by all other records, including all NS + glue records. + // by all other records, including all NS + glue records. The implemenation is also responsible + // for sending the last SOA record (to signal end of the transfer). This plugin will just grab + // these records and send them back to the requester, there is little validation done. // - // If serial is not 0, handle as an IXFR request. If the serial is equal to or greater (newer) than - // the current serial for the zone, send a single SOA record to the channel. + // If serial is not 0, it will be handled as an IXFR request. If the serial is equal to or greater (newer) than + // the current serial for the zone, send a single SOA record to the channel and then close it. // If the serial is less (older) than the current serial for the zone, perform an AXFR fallback // by proceeding as if an AXFR was requested (as above). Transfer(zone string, serial uint32) (<-chan []dns.RR, error) } var ( - // ErrNotAuthoritative is returned by Transfer() when the plugin is not authoritative for the zone + // ErrNotAuthoritative is returned by Transfer() when the plugin is not authoritative for the zone. ErrNotAuthoritative = errors.New("not authoritative for zone") ) // ServeDNS implements the plugin.Handler interface. -func (t Transfer) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (int, error) { +func (t *Transfer) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (int, error) { state := request.Request{W: w, Req: r} if state.QType() != dns.TypeAXFR && state.QType() != dns.TypeIXFR { return plugin.NextOrFailure(t.Name(), t.Next, ctx, w, r) } - // Find the first transfer instance for which the queried zone is a subdomain. - var x *xfr - for _, xfr := range t.xfrs { - zone := plugin.Zones(xfr.Zones).Matches(state.Name()) - if zone == "" { - continue - } - x = xfr - } + x := longestMatch(t.xfrs, state.QName()) if x == nil { - // Requested zone did not match any transfer instance zones. - // Pass request down chain in case later plugins are capable of handling transfer requests themselves. return plugin.NextOrFailure(t.Name(), t.Next, ctx, w, r) } if !x.allowed(state) { - return dns.RcodeRefused, nil + // write msg here, so logging will pick it up + m := new(dns.Msg) + m.SetRcode(r, dns.RcodeRefused) + w.WriteMsg(m) + return 0, nil } - // Get serial from request if this is an IXFR + // Get serial from request if this is an IXFR. var serial uint32 if state.QType() == dns.TypeIXFR { + if len(r.Ns) != 1 { + return dns.RcodeServerFailure, nil + } soa, ok := r.Ns[0].(*dns.SOA) if !ok { return dns.RcodeServerFailure, nil @@ -86,11 +85,11 @@ func (t Transfer) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg serial = soa.Serial } - // Get a receiving channel from the first Transferer plugin that returns one - var fromPlugin <-chan []dns.RR + // Get a receiving channel from the first Transferer plugin that returns one. + var pchan <-chan []dns.RR + var err error for _, p := range t.Transferers { - var err error - fromPlugin, err = p.Transfer(state.QName(), serial) + pchan, err = p.Transfer(state.QName(), serial) if err == ErrNotAuthoritative { // plugin was not authoritative for the zone, try next plugin continue @@ -101,7 +100,7 @@ func (t Transfer) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg break } - if fromPlugin == nil { + if pchan == nil { return plugin.NextOrFailure(t.Name(), t.Next, ctx, w, r) } @@ -115,26 +114,51 @@ func (t Transfer) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg wg.Done() }() - var soa *dns.SOA rrs := []dns.RR{} l := 0 - -receive: - for records := range fromPlugin { - for _, record := range records { - if soa == nil { - if soa = record.(*dns.SOA); soa == nil { - break receive - } - serial = soa.Serial - } - rrs = append(rrs, record) - if len(rrs) > 500 { - ch <- &dns.Envelope{RR: rrs} - l += len(rrs) - rrs = []dns.RR{} - } + var soa *dns.SOA + for records := range pchan { + if x, ok := records[0].(*dns.SOA); ok && soa == nil { + soa = x } + rrs = append(rrs, records...) + if len(rrs) > 500 { + ch <- &dns.Envelope{RR: rrs} + l += len(rrs) + rrs = []dns.RR{} + } + } + + // if we are here and we only hold 1 soa (len(rrs) == 1) and soa != nil, and IXFR fallback should + // be performed. We haven't send anything on ch yet, so that can be closed (and waited for), and we only + // need to return the SOA back to the client and return. + if len(rrs) == 1 && soa != nil { // soa should never be nil... + close(ch) + wg.Wait() + + m := new(dns.Msg) + m.SetReply(r) + m.Answer = []dns.RR{soa} + w.WriteMsg(m) + + log.Infof("Outgoing incremental transfer for up to date zone %q to %s for %d SOA serial", state.QName(), state.IP(), serial) + return 0, nil + } + + // if we are here and we only hold 1 soa (len(rrs) == 1) and soa != nil, and IXFR fallback should + // be performed. We haven't send anything on ch yet, so that can be closed (and waited for), and we only + // need to return the SOA back to the client and return. + if len(rrs) == 1 && soa != nil { // soa should never be nil... + close(ch) + wg.Wait() + + m := new(dns.Msg) + m.SetReply(r) + m.Answer = []dns.RR{soa} + w.WriteMsg(m) + + log.Infof("Outgoing noop, incremental transfer for up to date zone %q to %s for %d SOA serial", state.QName(), state.IP(), soa.Serial) + return 0, nil } if len(rrs) > 0 { @@ -142,20 +166,15 @@ receive: l += len(rrs) } - if soa != nil { - ch <- &dns.Envelope{RR: []dns.RR{soa}} // closing SOA. - l++ - } - close(ch) // Even though we close the channel here, we still have wg.Wait() // to wait before we can return and close the connection. - if soa == nil { - return dns.RcodeServerFailure, fmt.Errorf("first record in zone %s is not SOA", state.QName()) + logserial := uint32(0) + if soa != nil { + logserial = soa.Serial } - - log.Infof("Outgoing transfer of %d records of zone %s to %s with %d SOA serial", l, state.QName(), state.IP(), serial) - return dns.RcodeSuccess, nil + log.Infof("Outgoing transfer of %d records of zone %q to %s for %d SOA serial", l, state.QName(), state.IP(), logserial) + return 0, nil } func (x xfr) allowed(state request.Request) bool { @@ -167,14 +186,30 @@ func (x xfr) allowed(state request.Request) bool { if err != nil { return false } - // If remote IP matches we accept. - remote := state.IP() - if to == remote { + // If remote IP matches we accept. TODO(): make this works with ranges + if to == state.IP() { return true } } return false } +// Find the first transfer instance for which the queried zone is the longest match. When nothing +// is found nil is returned. +func longestMatch(xfrs []*xfr, name string) *xfr { + // TODO(xxx): optimize and make it a map (or maps) + var x *xfr + zone := "" // longest zone match wins + for _, xfr := range xfrs { + if z := plugin.Zones(xfr.Zones).Matches(name); z != "" { + if z > zone { + zone = z + x = xfr + } + } + } + return x +} + // Name implements the Handler interface. func (Transfer) Name() string { return "transfer" } diff --git a/plugin/transfer/transfer_test.go b/plugin/transfer/transfer_test.go index 8dce4c6e1..c4b3891db 100644 --- a/plugin/transfer/transfer_test.go +++ b/plugin/transfer/transfer_test.go @@ -12,18 +12,18 @@ import ( "github.com/miekg/dns" ) -// transfererPlugin implements transfer.Transferer and plugin.Handler +// transfererPlugin implements transfer.Transferer and plugin.Handler. type transfererPlugin struct { Zone string Serial uint32 Next plugin.Handler } -// Name implements plugin.Handler -func (transfererPlugin) Name() string { return "transfererplugin" } +// Name implements plugin.Handler. +func (*transfererPlugin) Name() string { return "transfererplugin" } -// ServeDNS implements plugin.Handler -func (p transfererPlugin) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (int, error) { +// ServeDNS implements plugin.Handler. +func (p *transfererPlugin) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (int, error) { if r.Question[0].Name != p.Zone { return p.Next.ServeDNS(ctx, w, r) } @@ -31,12 +31,12 @@ func (p transfererPlugin) ServeDNS(ctx context.Context, w dns.ResponseWriter, r } // Transfer implements transfer.Transferer - it returns a static AXFR response, or -// if serial is current, an abbreviated IXFR response -func (p transfererPlugin) Transfer(zone string, serial uint32) (<-chan []dns.RR, error) { +// if serial is current, an abbreviated IXFR response. +func (p *transfererPlugin) Transfer(zone string, serial uint32) (<-chan []dns.RR, error) { if zone != p.Zone { return nil, ErrNotAuthoritative } - ch := make(chan []dns.RR, 2) + ch := make(chan []dns.RR, 3) // sending 3 bits and don't want to block, nor do a waitgroup defer close(ch) ch <- []dns.RR{test.SOA(fmt.Sprintf("%s 100 IN SOA ns.dns.%s hostmaster.%s %d 7200 1800 86400 100", p.Zone, p.Zone, p.Zone, p.Serial))} if serial >= p.Serial { @@ -46,30 +46,31 @@ func (p transfererPlugin) Transfer(zone string, serial uint32) (<-chan []dns.RR, test.NS(fmt.Sprintf("%s 100 IN NS ns.dns.%s", p.Zone, p.Zone)), test.A(fmt.Sprintf("ns.dns.%s 100 IN A 1.2.3.4", p.Zone)), } + ch <- []dns.RR{test.SOA(fmt.Sprintf("%s 100 IN SOA ns.dns.%s hostmaster.%s %d 7200 1800 86400 100", p.Zone, p.Zone, p.Zone, p.Serial))} return ch, nil } type terminatingPlugin struct{} -// Name implements plugin.Handler -func (terminatingPlugin) Name() string { return "testplugin" } +// Name implements plugin.Handler. +func (*terminatingPlugin) Name() string { return "testplugin" } -// ServeDNS implements plugin.Handler that returns NXDOMAIN for all requests -func (terminatingPlugin) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (int, error) { +// ServeDNS implements plugin.Handler that returns NXDOMAIN for all requests. +func (*terminatingPlugin) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (int, error) { m := new(dns.Msg) m.SetRcode(r, dns.RcodeNameError) w.WriteMsg(m) return dns.RcodeNameError, nil } -func newTestTransfer() Transfer { +func newTestTransfer() *Transfer { nextPlugin1 := transfererPlugin{Zone: "example.com.", Serial: 12345} nextPlugin2 := transfererPlugin{Zone: "example.org.", Serial: 12345} - nextPlugin2.Next = terminatingPlugin{} - nextPlugin1.Next = nextPlugin2 + nextPlugin2.Next = &terminatingPlugin{} + nextPlugin1.Next = &nextPlugin2 - transfer := Transfer{ - Transferers: []Transferer{nextPlugin1, nextPlugin2}, + transfer := &Transfer{ + Transferers: []Transferer{&nextPlugin1, &nextPlugin2}, xfrs: []*xfr{ { Zones: []string{"example.org."}, @@ -80,22 +81,21 @@ func newTestTransfer() Transfer { to: []string{"*"}, }, }, - Next: nextPlugin1, + Next: &nextPlugin1, } return transfer } func TestTransferNonZone(t *testing.T) { - transfer := newTestTransfer() ctx := context.TODO() for _, tc := range []string{"sub.example.org.", "example.test."} { w := dnstest.NewRecorder(&test.ResponseWriter{}) - dnsmsg := &dns.Msg{} - dnsmsg.SetAxfr(tc) + m := &dns.Msg{} + m.SetAxfr(tc) - _, err := transfer.ServeDNS(ctx, w, dnsmsg) + _, err := transfer.ServeDNS(ctx, w, m) if err != nil { t.Error(err) } @@ -111,15 +111,14 @@ func TestTransferNonZone(t *testing.T) { } func TestTransferNotAXFRorIXFR(t *testing.T) { - transfer := newTestTransfer() ctx := context.TODO() w := dnstest.NewRecorder(&test.ResponseWriter{}) - dnsmsg := &dns.Msg{} - dnsmsg.SetQuestion("test.domain.", dns.TypeA) + m := &dns.Msg{} + m.SetQuestion("test.domain.", dns.TypeA) - _, err := transfer.ServeDNS(ctx, w, dnsmsg) + _, err := transfer.ServeDNS(ctx, w, m) if err != nil { t.Error(err) } @@ -134,15 +133,14 @@ func TestTransferNotAXFRorIXFR(t *testing.T) { } func TestTransferAXFRExampleOrg(t *testing.T) { - transfer := newTestTransfer() ctx := context.TODO() w := dnstest.NewMultiRecorder(&test.ResponseWriter{}) - dnsmsg := &dns.Msg{} - dnsmsg.SetAxfr(transfer.xfrs[0].Zones[0]) + m := &dns.Msg{} + m.SetAxfr(transfer.xfrs[0].Zones[0]) - _, err := transfer.ServeDNS(ctx, w, dnsmsg) + _, err := transfer.ServeDNS(ctx, w, m) if err != nil { t.Error(err) } @@ -151,39 +149,14 @@ func TestTransferAXFRExampleOrg(t *testing.T) { } func TestTransferAXFRExampleCom(t *testing.T) { - transfer := newTestTransfer() ctx := context.TODO() w := dnstest.NewMultiRecorder(&test.ResponseWriter{}) - dnsmsg := &dns.Msg{} - dnsmsg.SetAxfr(transfer.xfrs[1].Zones[0]) + m := &dns.Msg{} + m.SetAxfr(transfer.xfrs[1].Zones[0]) - _, err := transfer.ServeDNS(ctx, w, dnsmsg) - if err != nil { - t.Error(err) - } - - validateAXFRResponse(t, w) -} - -func TestTransferIXFRFallback(t *testing.T) { - - transfer := newTestTransfer() - - testPlugin := transfer.Transferers[0].(transfererPlugin) - - ctx := context.TODO() - w := dnstest.NewMultiRecorder(&test.ResponseWriter{}) - dnsmsg := &dns.Msg{} - dnsmsg.SetIxfr( - transfer.xfrs[0].Zones[0], - testPlugin.Serial-1, - "ns.dns."+testPlugin.Zone, - "hostmaster.dns."+testPlugin.Zone, - ) - - _, err := transfer.ServeDNS(ctx, w, dnsmsg) + _, err := transfer.ServeDNS(ctx, w, m) if err != nil { t.Error(err) } @@ -192,28 +165,21 @@ func TestTransferIXFRFallback(t *testing.T) { } func TestTransferIXFRCurrent(t *testing.T) { - transfer := newTestTransfer() - testPlugin := transfer.Transferers[0].(transfererPlugin) + testPlugin := transfer.Transferers[0].(*transfererPlugin) ctx := context.TODO() w := dnstest.NewMultiRecorder(&test.ResponseWriter{}) - dnsmsg := &dns.Msg{} - dnsmsg.SetIxfr( - transfer.xfrs[0].Zones[0], - testPlugin.Serial, - "ns.dns."+testPlugin.Zone, - "hostmaster.dns."+testPlugin.Zone, - ) + m := &dns.Msg{} + m.SetIxfr(transfer.xfrs[0].Zones[0], testPlugin.Serial, "ns.dns."+testPlugin.Zone, "hostmaster.dns."+testPlugin.Zone) - _, err := transfer.ServeDNS(ctx, w, dnsmsg) + _, err := transfer.ServeDNS(ctx, w, m) if err != nil { t.Error(err) } if len(w.Msgs) == 0 { - t.Logf("%+v\n", w) t.Fatal("Did not get back a zone response") } @@ -228,9 +194,31 @@ func TestTransferIXFRCurrent(t *testing.T) { } } +func TestTransferIXFRFallback(t *testing.T) { + transfer := newTestTransfer() + + testPlugin := transfer.Transferers[0].(*transfererPlugin) + + ctx := context.TODO() + w := dnstest.NewMultiRecorder(&test.ResponseWriter{}) + m := &dns.Msg{} + m.SetIxfr( + transfer.xfrs[0].Zones[0], + testPlugin.Serial-1, + "ns.dns."+testPlugin.Zone, + "hostmaster.dns."+testPlugin.Zone, + ) + + _, err := transfer.ServeDNS(ctx, w, m) + if err != nil { + t.Error(err) + } + + validateAXFRResponse(t, w) +} + func validateAXFRResponse(t *testing.T, w *dnstest.MultiRecorder) { if len(w.Msgs) == 0 { - t.Logf("%+v\n", w) t.Fatal("Did not get back a zone response") } @@ -263,29 +251,28 @@ func TestTransferNotAllowed(t *testing.T) { nextPlugin := transfererPlugin{Zone: "example.org.", Serial: 12345} transfer := Transfer{ - Transferers: []Transferer{nextPlugin}, + Transferers: []Transferer{&nextPlugin}, xfrs: []*xfr{ { Zones: []string{"example.org."}, to: []string{"1.2.3.4"}, }, }, - Next: nextPlugin, + Next: &nextPlugin, } ctx := context.TODO() - w := dnstest.NewMultiRecorder(&test.ResponseWriter{}) - dnsmsg := &dns.Msg{} - dnsmsg.SetAxfr(transfer.xfrs[0].Zones[0]) + w := dnstest.NewRecorder(&test.ResponseWriter{}) + m := &dns.Msg{} + m.SetAxfr(transfer.xfrs[0].Zones[0]) - rcode, err := transfer.ServeDNS(ctx, w, dnsmsg) + _, err := transfer.ServeDNS(ctx, w, m) if err != nil { t.Error(err) } - if rcode != dns.RcodeRefused { - t.Errorf("Expected REFUSED response code, got %s", dns.RcodeToString[rcode]) + if w.Msg.Rcode != dns.RcodeRefused { + t.Errorf("Expected REFUSED response code, got %s", dns.RcodeToString[w.Msg.Rcode]) } - } diff --git a/test/auto_test.go b/test/auto_test.go index f7e5faecd..3dd0fdf79 100644 --- a/test/auto_test.go +++ b/test/auto_test.go @@ -120,7 +120,9 @@ func TestAutoAXFR(t *testing.T) { auto { directory ` + tmpdir + ` db\.(.*) {1} reload 1s - transfer to * + } + transfer { + to * } }` diff --git a/test/file_upstream_test.go b/test/file_upstream_test.go index 9232b44a1..4d13a2ca0 100644 --- a/test/file_upstream_test.go +++ b/test/file_upstream_test.go @@ -29,9 +29,7 @@ www 3600 IN CNAME www.example.net. defer rm() corefile := `.:0 { - file ` + name + ` example.org { - upstream - } + file ` + name + ` example.org hosts { 10.0.0.1 www.example.net. fallthrough @@ -88,12 +86,8 @@ www 3600 IN A 127.0.0.53 defer rm2() corefile := `.:0 { - file ` + name + ` example.org { - upstream - } - file ` + name2 + ` foo.example.org { - upstream - } + file ` + name + ` example.org + file ` + name2 + ` foo.example.org }` i, udp, _, err := CoreDNSServerAndPorts(corefile) diff --git a/test/file_xfr_test.go b/test/file_xfr_test.go index b564417f0..0298f5e68 100644 --- a/test/file_xfr_test.go +++ b/test/file_xfr_test.go @@ -29,8 +29,9 @@ func TestLargeAXFR(t *testing.T) { defer rm() corefile := `example.com:0 { - file ` + name + ` { - transfer to * + file ` + name + ` + transfer { + to * } }` diff --git a/test/secondary_test.go b/test/secondary_test.go index bacc989b5..02fee0b3c 100644 --- a/test/secondary_test.go +++ b/test/secondary_test.go @@ -43,7 +43,9 @@ func TestSecondaryZoneTransfer(t *testing.T) { corefile := `example.org:0 { file ` + name + ` { - transfer to * + } + transfer { + to * } }` @@ -92,7 +94,9 @@ func TestIxfrResponse(t *testing.T) { corefile := `example.org:0 { file ` + name + ` { - transfer to * + } + transfer { + to * } }`