Add pkg/fall for Fallthrough (#1355)

* Add pkg/fall for Fallthrough

Move this into it's own package to facilitate tests. Important bug
was fixed: make the names fully qualified.

Add fall package to hosts, reverse, etcd, and fix kubernetes and any
tests. The k8s tests are still as-is, might need a future cleanup.
This commit is contained in:
Miek Gieben 2018-01-07 16:32:59 +00:00 committed by GitHub
parent 84ebbbc722
commit c6febe6250
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
22 changed files with 217 additions and 110 deletions

View file

@ -41,15 +41,15 @@ We use the Unix manual page style:
### Example Domain Names
Please be sure to use `example.org` or `example.net` in any examples you provide. These are the
standard domain names created for this purpose.
Please be sure to use `example.org` or `example.net` in any examples and tests you provide. These
are the standard domain names created for this purpose.
## Fallthrough
In a perfect world the following would be true for plugin: "Either you are responsible for a zone or
not". If the answer is "not", the plugin should call the next plugin in the chain. If "yes" it
should handle *all* names that fall in this zone and the names below - i.e. it should handle the
entire domain.
entire domain and all sub domains.
~~~ txt
. {
@ -61,7 +61,7 @@ In this example the *file* plugin is handling all names below (and including) `e
a query comes in that is not a subdomain (or equal to) `example.org` the next plugin is called.
Now, the world isn't perfect, and there are good reasons to "fallthrough" to the next middlware,
meaning a plugin is only responsible for a subset of names within the zone. The first of these
meaning a plugin is only responsible for a *subset* of names within the zone. The first of these
to appear was the *reverse* plugin that synthesis PTR and A/AAAA responses (useful with IPv6).
The nature of the *reverse* plugin is such that it only deals with A,AAAA and PTR and then only

View file

@ -29,7 +29,7 @@ If you want to `round robin` A and AAAA responses look at the `loadbalance` plug
~~~
etcd [ZONES...] {
stubzones
fallthrough
fallthrough [ZONES...]
path PATH
endpoint ENDPOINT...
upstream ADDRESS...
@ -40,6 +40,9 @@ etcd [ZONES...] {
* `stubzones` enables the stub zones feature. The stubzone is *only* done in the etcd tree located
under the *first* zone specified.
* `fallthrough` If zone matches but no record can be generated, pass request to the next plugin.
If **[ZONES...]** is omitted, then fallthrough happens for all zones for which the plugin
is authoritative. If specific zones are listed (for example `in-addr.arpa` and `ip6.arpa`), then only
queries for those zones will be subject to fallthrough.
* **PATH** the path inside etcd. Defaults to "/skydns".
* **ENDPOINT** the etcd endpoints. Defaults to "http://localhost:2397".
* `upstream` upstream resolvers to be used resolve external names found in etcd (think CNAMEs)

View file

@ -9,6 +9,7 @@ import (
"github.com/coredns/coredns/plugin"
"github.com/coredns/coredns/plugin/etcd/msg"
"github.com/coredns/coredns/plugin/pkg/fall"
"github.com/coredns/coredns/plugin/proxy"
"github.com/coredns/coredns/request"
@ -19,14 +20,14 @@ import (
// Etcd is a plugin talks to an etcd cluster.
type Etcd struct {
Next plugin.Handler
Fallthrough bool
Zones []string
PathPrefix string
Proxy proxy.Proxy // Proxy for looking up names during the resolution process
Client etcdc.KeysAPI
Ctx context.Context
Stubmap *map[string]proxy.Proxy // list of proxies for stub resolving.
Next plugin.Handler
Fall *fall.F
Zones []string
PathPrefix string
Proxy proxy.Proxy // Proxy for looking up names during the resolution process
Client etcdc.KeysAPI
Ctx context.Context
Stubmap *map[string]proxy.Proxy // list of proxies for stub resolving.
endpoints []string // Stored here as well, to aid in testing.
}

View file

@ -67,7 +67,7 @@ func (e *Etcd) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (
}
if e.IsNameError(err) {
if e.Fallthrough {
if e.Fall.Through(state.Name()) {
return plugin.NextOrFailure(e.Name(), e.Next, ctx, w, r)
}
// Make err nil when returning here, so we don't log spam for NXDOMAIN.

View file

@ -7,6 +7,7 @@ import (
"github.com/coredns/coredns/plugin/etcd/msg"
"github.com/coredns/coredns/plugin/pkg/dnstest"
"github.com/coredns/coredns/plugin/pkg/fall"
"github.com/coredns/coredns/plugin/test"
"github.com/miekg/dns"
@ -15,7 +16,7 @@ import (
func TestMultiLookup(t *testing.T) {
etc := newEtcdPlugin()
etc.Zones = []string{"skydns.test.", "miek.nl."}
etc.Fallthrough = true
etc.Fall = fall.New()
etc.Next = test.ErrorHandler()
for _, serv := range servicesMulti {

View file

@ -6,6 +6,7 @@ import (
"github.com/coredns/coredns/core/dnsserver"
"github.com/coredns/coredns/plugin"
"github.com/coredns/coredns/plugin/pkg/dnsutil"
"github.com/coredns/coredns/plugin/pkg/fall"
mwtls "github.com/coredns/coredns/plugin/pkg/tls"
"github.com/coredns/coredns/plugin/proxy"
@ -73,7 +74,8 @@ func etcdParse(c *caddy.Controller) (*Etcd, bool, error) {
case "stubzones":
stubzones = true
case "fallthrough":
etc.Fallthrough = true
etc.Fall = fall.New()
etc.Fall.SetZones(c.RemainingArgs())
case "debug":
/* it is a noop now */
case "path":

View file

@ -16,12 +16,12 @@ available hosts files that block access to advertising servers.
~~~
hosts [FILE [ZONES...]] {
[INLINE]
fallthrough
fallthrough [ZONES...]
}
~~~
* **FILE** the hosts file to read and parse. If the path is relative the path from the *root*
directive will be prepended to it. Defaults to /etc/hosts if omitted. We scan the file for changes
directive will be prepended to it. Defaults to /etc/hosts if omitted. We scan the file for changes
every 5 seconds.
* **ZONES** zones it should be authoritative for. If empty, the zones from the configuration block
are used.
@ -29,6 +29,9 @@ hosts [FILE [ZONES...]] {
then all of them will be treated as the additional content for hosts file. The specified hosts
file path will still be read but entries will be overrided.
* `fallthrough` If zone matches and no record can be generated, pass request to the next plugin.
If **[ZONES...]** is omitted, then fallthrough happens for all zones for which the plugin
is authoritative. If specific zones are listed (for example `in-addr.arpa` and `ip6.arpa`), then only
queries for those zones will be subject to fallthrough.
## Examples

View file

@ -7,6 +7,7 @@ import (
"github.com/coredns/coredns/plugin"
"github.com/coredns/coredns/plugin/pkg/dnsutil"
"github.com/coredns/coredns/plugin/pkg/fall"
"github.com/coredns/coredns/request"
"github.com/miekg/dns"
)
@ -16,7 +17,7 @@ type Hosts struct {
Next plugin.Handler
*Hostsfile
Fallthrough bool
Fall *fall.F
}
// ServeDNS implements the plugin.Handle interface.
@ -52,7 +53,7 @@ func (h Hosts) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (
}
if len(answers) == 0 {
if h.Fallthrough {
if h.Fall.Through(qname) {
return plugin.NextOrFailure(h.Name(), h.Next, ctx, w, r)
}
if !h.otherRecordsExist(state.QType(), qname) {

View file

@ -9,6 +9,7 @@ import (
"github.com/coredns/coredns/core/dnsserver"
"github.com/coredns/coredns/plugin"
"github.com/coredns/coredns/plugin/pkg/fall"
"github.com/mholt/caddy"
)
@ -105,14 +106,10 @@ func hostsParse(c *caddy.Controller) (Hosts, error) {
for c.NextBlock() {
switch c.Val() {
case "fallthrough":
args := c.RemainingArgs()
if len(args) == 0 {
h.Fallthrough = true
continue
}
return h, c.ArgErr()
h.Fall = fall.New()
h.Fall.SetZones(c.RemainingArgs())
default:
if !h.Fallthrough {
if h.Fall.IsNil() {
line := strings.Join(append([]string{c.Val()}, c.RemainingArgs()...), " ")
inline = append(inline, line)
continue

View file

@ -3,6 +3,8 @@ package hosts
import (
"testing"
"github.com/coredns/coredns/plugin/pkg/fall"
"github.com/mholt/caddy"
)
@ -12,48 +14,48 @@ func TestHostsParse(t *testing.T) {
shouldErr bool
expectedPath string
expectedOrigins []string
expectedFallthrough bool
expectedFallthrough *fall.F
}{
{
`hosts
`,
false, "/etc/hosts", nil, false,
false, "/etc/hosts", nil, nil,
},
{
`hosts /tmp`,
false, "/tmp", nil, false,
false, "/tmp", nil, nil,
},
{
`hosts /etc/hosts miek.nl.`,
false, "/etc/hosts", []string{"miek.nl."}, false,
false, "/etc/hosts", []string{"miek.nl."}, nil,
},
{
`hosts /etc/hosts miek.nl. pun.gent.`,
false, "/etc/hosts", []string{"miek.nl.", "pun.gent."}, false,
false, "/etc/hosts", []string{"miek.nl.", "pun.gent."}, nil,
},
{
`hosts {
fallthrough
}`,
false, "/etc/hosts", nil, true,
false, "/etc/hosts", nil, fall.Zero(),
},
{
`hosts /tmp {
fallthrough
}`,
false, "/tmp", nil, true,
false, "/tmp", nil, fall.Zero(),
},
{
`hosts /etc/hosts miek.nl. {
fallthrough
}`,
false, "/etc/hosts", []string{"miek.nl."}, true,
false, "/etc/hosts", []string{"miek.nl."}, fall.Zero(),
},
{
`hosts /etc/hosts miek.nl 10.0.0.9/8 {
fallthrough
}`,
false, "/etc/hosts", []string{"miek.nl.", "10.in-addr.arpa."}, true,
false, "/etc/hosts", []string{"miek.nl.", "10.in-addr.arpa."}, fall.Zero(),
},
}
@ -70,8 +72,8 @@ func TestHostsParse(t *testing.T) {
t.Fatalf("Test %d expected %v, got %v", i, test.expectedPath, h.path)
}
} else {
if h.Fallthrough != test.expectedFallthrough {
t.Fatalf("Test %d expected fallthrough of %v, got %v", i, test.expectedFallthrough, h.Fallthrough)
if !h.Fall.Equal(test.expectedFallthrough) {
t.Fatalf("Test %d expected fallthrough of %v, got %v", i, test.expectedFallthrough, h.Fall)
}
if len(h.Origins) != len(test.expectedOrigins) {
t.Fatalf("Test %d expected %v, got %v", i, test.expectedOrigins, h.Origins)
@ -90,7 +92,7 @@ func TestHostsInlineParse(t *testing.T) {
inputFileRules string
shouldErr bool
expectedbyAddr map[string][]string
expectedFallthrough bool
expectedFallthrough *fall.F
}{
{
`hosts highly_unlikely_to_exist_hosts_file example.org {
@ -103,28 +105,28 @@ func TestHostsInlineParse(t *testing.T) {
`example.org.`,
},
},
true,
fall.Zero(),
},
{
`hosts highly_unlikely_to_exist_hosts_file example.org {
10.0.0.1 example.org
}`,
10.0.0.1 example.org
}`,
false,
map[string][]string{
`10.0.0.1`: {
`example.org.`,
},
},
false,
nil,
},
{
`hosts highly_unlikely_to_exist_hosts_file example.org {
fallthrough
10.0.0.1 example.org
}`,
fallthrough
10.0.0.1 example.org
}`,
true,
map[string][]string{},
true,
fall.Zero(),
},
}
@ -137,8 +139,8 @@ func TestHostsInlineParse(t *testing.T) {
} else if err != nil && !test.shouldErr {
t.Fatalf("Test %d expected no errors, but got '%v'", i, err)
} else if !test.shouldErr {
if h.Fallthrough != test.expectedFallthrough {
t.Fatalf("Test %d expected fallthrough of %v, got %v", i, test.expectedFallthrough, h.Fallthrough)
if !h.Fall.Equal(test.expectedFallthrough) {
t.Fatalf("Test %d expected fallthrough of %v, got %v", i, test.expectedFallthrough, h.Fall)
}
for k, expectedVal := range test.expectedbyAddr {
if val, ok := h.hmap.byAddr[k]; !ok {

View file

@ -59,7 +59,7 @@ func (k Kubernetes) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.M
}
if k.IsNameError(err) {
if plugin.Fallthrough(k.Fallthrough, state.Name()) {
if k.Fall.Through(state.Name()) {
return plugin.NextOrFailure(k.Name(), k.Next, ctx, w, r)
}
return plugin.BackendError(&k, zone, dns.RcodeNameError, state, nil /* err */, plugin.Options{})

View file

@ -12,6 +12,7 @@ import (
"github.com/coredns/coredns/plugin"
"github.com/coredns/coredns/plugin/etcd/msg"
"github.com/coredns/coredns/plugin/pkg/dnsutil"
"github.com/coredns/coredns/plugin/pkg/fall"
"github.com/coredns/coredns/plugin/pkg/healthcheck"
"github.com/coredns/coredns/plugin/proxy"
"github.com/coredns/coredns/request"
@ -40,7 +41,7 @@ type Kubernetes struct {
Namespaces map[string]bool
podMode string
endpointNameMode bool
Fallthrough *[]string // nil = disabled, empty = all zones, o/w zones
Fall *fall.F
ttl uint32
primaryZoneIndex int

View file

@ -10,6 +10,7 @@ import (
"github.com/coredns/coredns/core/dnsserver"
"github.com/coredns/coredns/plugin"
"github.com/coredns/coredns/plugin/pkg/dnsutil"
"github.com/coredns/coredns/plugin/pkg/fall"
"github.com/coredns/coredns/plugin/proxy"
"github.com/mholt/caddy"
@ -172,8 +173,8 @@ func kubernetesParse(c *caddy.Controller) (*Kubernetes, dnsControlOpts, error) {
}
return nil, opts, c.ArgErr()
case "fallthrough":
zones := c.RemainingArgs()
k8s.Fallthrough = &zones
k8s.Fall = fall.New()
k8s.Fall.SetZones(c.RemainingArgs())
case "upstream":
args := c.RemainingArgs()
if len(args) == 0 {

View file

@ -347,7 +347,7 @@ func TestKubernetesParse(t *testing.T) {
defaultResyncPeriod,
"",
podModeDisabled,
&[]string{"ip6.arpa", "inaddr.arpa", "foo.com"},
&[]string{"ip6.arpa.", "inaddr.arpa.", "foo.com."},
nil,
},
// Valid upstream
@ -443,7 +443,7 @@ func TestKubernetesParse(t *testing.T) {
}
// fallthrough
foundFallthrough := k8sController.Fallthrough
foundFallthrough := k8sController.Fall
if foundFallthrough != nil {
failed := false
if test.expectedFallthrough == nil {

77
plugin/pkg/fall/fall.go Normal file
View file

@ -0,0 +1,77 @@
// Package fall handles the fallthrough logic used in plugins that support it.
package fall
import (
"github.com/coredns/coredns/plugin"
)
// F can be nil to allow for no fallthrough, empty allow all zones to fallthrough or
// contain a zone list that is checked.
type F []string
// New returns a new F.
func New() *F { return new(F) }
// Through will check if we should fallthrough for qname. Note that we've named the
// variable in each plugin "Fall", so this then reads Fall.Through().
func (f *F) Through(qname string) bool {
if f == nil {
return false
}
if len(*f) == 0 {
return true
}
zone := plugin.Zones(*f).Matches(qname)
return zone != ""
}
// SetZones will set zones in f.
func (f *F) SetZones(zones []string) {
for i := range zones {
zones[i] = plugin.Host(zones[i]).Normalize()
}
*f = zones
}
// Example returns an F with example.org. as the zone name.
var Example = func() *F {
f := F([]string{"example.org."})
return &f
}()
// Zero returns a zero valued F.
var Zero = func() *F {
f := F([]string{})
return &f
}
// IsNil returns true is f is nil.
func (f *F) IsNil() bool { return f == nil }
// IsZero returns true is f is zero (and not nil).
func (f *F) IsZero() bool {
if f == nil {
return false
}
return len(*f) == 0
}
// Equal returns true if f and g are equal. Only useful in tests, The (possible) zones
// are *not* checked.
func (f *F) Equal(g *F) bool {
if f.IsNil() {
if g.IsNil() {
return true
}
return false
}
if f.IsZero() {
if g.IsZero() {
return true
}
}
if len(*f) != len(*g) {
return false
}
return true
}

View file

@ -0,0 +1,48 @@
package fall
import "testing"
func TestIsNil(t *testing.T) {
var f *F
if !f.IsNil() {
t.Errorf("F should be nil")
}
}
func TestIsZero(t *testing.T) {
f := New()
if !f.IsZero() {
t.Errorf("F should be zero")
}
}
func TestFallThroughExample(t *testing.T) {
if !Example.Through("example.org.") {
t.Errorf("example.org. should fall through")
}
if Example.Through("example.net.") {
t.Errorf("example.net. should not fall through")
}
}
func TestFallthrough(t *testing.T) {
var fall *F
if fall.Through("foo.com.") {
t.Errorf("Expected false, got true for nil fallthrough")
}
fall = New()
if !fall.Through("foo.net.") {
t.Errorf("Expected true, got false for all zone fallthrough")
}
fall.SetZones([]string{"foo.com", "bar.com"})
if fall.Through("foo.net.") {
t.Errorf("Expected false, got true for non-matching fallthrough zone")
}
if !fall.Through("bar.com.") {
t.Errorf("Expected true, got false for matching fallthrough zone")
}
}

View file

@ -83,21 +83,6 @@ func NextOrFailure(name string, next Handler, ctx context.Context, w dns.Respons
return dns.RcodeServerFailure, Error(name, errors.New("no next plugin found"))
}
// Fallthrough handles the fallthrough logic used in plugins that support it
func Fallthrough(ftzones *[]string, qname string) bool {
if ftzones == nil {
return false
}
if len(*ftzones) == 0 {
return true
}
zone := Zones(*ftzones).Matches(qname)
if zone != "" {
return true
}
return false
}
// ClientWrite returns true if the response has been written to the client.
// Each plugin to adhire to this protocol.
func ClientWrite(rcode int) bool {

View file

@ -1,21 +0,0 @@
package plugin
import "testing"
func TestFallthrough(t *testing.T) {
if Fallthrough(nil, "foo.com.") {
t.Errorf("Expected false, got true for nil fallthrough")
}
if !Fallthrough(&[]string{}, "foo.net.") {
t.Errorf("Expected true, got false for all zone fallthrough")
}
if Fallthrough(&[]string{"foo.com", "bar.com"}, "foo.net") {
t.Errorf("Expected false, got true for non-matching fallthrough zone")
}
if !Fallthrough(&[]string{"foo.com.", "bar.com."}, "bar.com.") {
t.Errorf("Expected true, got false for matching fallthrough zone")
}
}

View file

@ -15,7 +15,7 @@ response. This is only done for "address" records (PTR, A and AAAA).
reverse NETWORK... {
hostname TEMPLATE
[ttl TTL]
[fallthrough]
[fallthrough [ZONES...]]
[wildcard]
~~~
@ -23,6 +23,9 @@ reverse NETWORK... {
* `hostname` injects the IP and zone to a template for the hostname. Defaults to "ip-{IP}.{zone[1]}". See below for template.
* `ttl` defaults to 60
* `fallthrough` if zone matches and no record can be generated, pass request to the next plugin.
If **[ZONES...]** is omitted, then fallthrough happens for all zones for which the plugin
is authoritative. If specific zones are listed (for example `in-addr.arpa` and `ip6.arpa`), then only
queries for those zones will be subject to fallthrough.
* `wildcard` allows matches to catch all subdomains as well.
### Template Syntax

View file

@ -5,6 +5,7 @@ import (
"github.com/coredns/coredns/plugin"
"github.com/coredns/coredns/plugin/pkg/dnsutil"
"github.com/coredns/coredns/plugin/pkg/fall"
"github.com/coredns/coredns/request"
"github.com/miekg/dns"
@ -13,9 +14,10 @@ import (
// Reverse provides dynamic reverse DNS and the related forward RR.
type Reverse struct {
Next plugin.Handler
Networks networks
Fallthrough bool
Next plugin.Handler
Networks networks
Fall *fall.F
}
// ServeDNS implements the plugin.Handler interface.
@ -97,7 +99,7 @@ func (re Reverse) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg
return dns.RcodeSuccess, nil
}
if re.Fallthrough {
if re.Fall.Through(state.Name()) {
return plugin.NextOrFailure(re.Name(), re.Next, ctx, w, r)
}
return dns.RcodeServerFailure, nil

View file

@ -24,7 +24,6 @@ func TestReverse(t *testing.T) {
Template: "ip-{ip}.example.org.",
RegexMatchIP: regexIP4,
}},
Fallthrough: false,
}
tests := []struct {

View file

@ -9,6 +9,7 @@ import (
"github.com/coredns/coredns/core/dnsserver"
"github.com/coredns/coredns/plugin"
"github.com/coredns/coredns/plugin/pkg/fall"
"github.com/mholt/caddy"
)
@ -21,19 +22,19 @@ func init() {
}
func setupReverse(c *caddy.Controller) error {
networks, fallThrough, err := reverseParse(c)
networks, fall, err := reverseParse(c)
if err != nil {
return plugin.Error("reverse", err)
}
dnsserver.GetConfig(c).AddPlugin(func(next plugin.Handler) plugin.Handler {
return Reverse{Next: next, Networks: networks, Fallthrough: fallThrough}
return Reverse{Next: next, Networks: networks, Fall: fall}
})
return nil
}
func reverseParse(c *caddy.Controller) (nets networks, fall bool, err error) {
func reverseParse(c *caddy.Controller) (nets networks, f *fall.F, err error) {
zones := make([]string, len(c.ServerBlockKeys))
wildcard := false
@ -52,12 +53,12 @@ func reverseParse(c *caddy.Controller) (nets networks, fall bool, err error) {
}
_, ipnet, err := net.ParseCIDR(cidr)
if err != nil {
return nil, false, c.Errf("network needs to be CIDR formatted: %q\n", cidr)
return nil, f, c.Errf("network needs to be CIDR formatted: %q\n", cidr)
}
cidrs = append(cidrs, ipnet)
}
if len(cidrs) == 0 {
return nil, false, c.ArgErr()
return nil, f, c.ArgErr()
}
// set defaults
@ -69,27 +70,28 @@ func reverseParse(c *caddy.Controller) (nets networks, fall bool, err error) {
switch c.Val() {
case "hostname":
if !c.NextArg() {
return nil, false, c.ArgErr()
return nil, f, c.ArgErr()
}
template = c.Val()
case "ttl":
if !c.NextArg() {
return nil, false, c.ArgErr()
return nil, f, c.ArgErr()
}
ttl, err = strconv.Atoi(c.Val())
if err != nil {
return nil, false, err
return nil, f, err
}
case "wildcard":
wildcard = true
case "fallthrough":
fall = true
f = fall.New()
f.SetZones(c.RemainingArgs())
default:
return nil, false, c.ArgErr()
return nil, f, c.ArgErr()
}
}
@ -107,7 +109,7 @@ func reverseParse(c *caddy.Controller) (nets networks, fall bool, err error) {
// extract zone from template
templateZone := strings.SplitAfterN(template, ".", 2)
if len(templateZone) != 2 || templateZone[1] == "" {
return nil, false, c.Errf("cannot find domain in template '%v'", template)
return nil, f, c.Errf("cannot find domain in template '%v'", template)
}
// Create for each configured network in this stanza
@ -128,7 +130,7 @@ func reverseParse(c *caddy.Controller) (nets networks, fall bool, err error) {
regexIP,
1) + "$")
if err != nil {
return nil, false, err
return nil, f, err
}
nets = append(nets, network{
@ -143,5 +145,5 @@ func reverseParse(c *caddy.Controller) (nets networks, fall bool, err error) {
// sort by cidr
sort.Sort(nets)
return nets, fall, nil
return nets, f, nil
}