diff --git a/plugin/etcd/etcd.go b/plugin/etcd/etcd.go index 14c24d06e..fc0b60774 100644 --- a/plugin/etcd/etcd.go +++ b/plugin/etcd/etcd.go @@ -21,7 +21,7 @@ import ( // Etcd is a plugin talks to an etcd cluster. type Etcd struct { Next plugin.Handler - Fall *fall.F + Fall fall.F Zones []string PathPrefix string Proxy proxy.Proxy // Proxy for looking up names during the resolution process diff --git a/plugin/etcd/multi_test.go b/plugin/etcd/multi_test.go index d58d22de0..b06d0bc7e 100644 --- a/plugin/etcd/multi_test.go +++ b/plugin/etcd/multi_test.go @@ -7,7 +7,6 @@ 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" @@ -16,7 +15,6 @@ import ( func TestMultiLookup(t *testing.T) { etc := newEtcdPlugin() etc.Zones = []string{"skydns.test.", "miek.nl."} - etc.Fall = fall.New() etc.Next = test.ErrorHandler() for _, serv := range servicesMulti { diff --git a/plugin/etcd/setup.go b/plugin/etcd/setup.go index 5fbfe4d18..7d34c43f6 100644 --- a/plugin/etcd/setup.go +++ b/plugin/etcd/setup.go @@ -6,7 +6,6 @@ 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" @@ -74,8 +73,7 @@ func etcdParse(c *caddy.Controller) (*Etcd, bool, error) { case "stubzones": stubzones = true case "fallthrough": - etc.Fall = fall.New() - etc.Fall.SetZones(c.RemainingArgs()) + etc.Fall.SetZonesFromArgs(c.RemainingArgs()) case "debug": /* it is a noop now */ case "path": diff --git a/plugin/hosts/hosts.go b/plugin/hosts/hosts.go index 5f9766d47..dcd66d4be 100644 --- a/plugin/hosts/hosts.go +++ b/plugin/hosts/hosts.go @@ -17,7 +17,7 @@ type Hosts struct { Next plugin.Handler *Hostsfile - Fall *fall.F + Fall fall.F } // ServeDNS implements the plugin.Handle interface. diff --git a/plugin/hosts/setup.go b/plugin/hosts/setup.go index 35b0c5483..57413feed 100644 --- a/plugin/hosts/setup.go +++ b/plugin/hosts/setup.go @@ -9,7 +9,6 @@ import ( "github.com/coredns/coredns/core/dnsserver" "github.com/coredns/coredns/plugin" - "github.com/coredns/coredns/plugin/pkg/fall" "github.com/mholt/caddy" ) @@ -106,10 +105,9 @@ func hostsParse(c *caddy.Controller) (Hosts, error) { for c.NextBlock() { switch c.Val() { case "fallthrough": - h.Fall = fall.New() - h.Fall.SetZones(c.RemainingArgs()) + h.Fall.SetZonesFromArgs(c.RemainingArgs()) default: - if h.Fall.IsNil() { + if len(h.Fall.Zones) == 0 { line := strings.Join(append([]string{c.Val()}, c.RemainingArgs()...), " ") inline = append(inline, line) continue diff --git a/plugin/hosts/setup_test.go b/plugin/hosts/setup_test.go index 58351cc52..3c4f94948 100644 --- a/plugin/hosts/setup_test.go +++ b/plugin/hosts/setup_test.go @@ -14,48 +14,48 @@ func TestHostsParse(t *testing.T) { shouldErr bool expectedPath string expectedOrigins []string - expectedFallthrough *fall.F + expectedFallthrough fall.F }{ { `hosts `, - false, "/etc/hosts", nil, nil, + false, "/etc/hosts", nil, fall.Zero(), }, { `hosts /tmp`, - false, "/tmp", nil, nil, + false, "/tmp", nil, fall.Zero(), }, { `hosts /etc/hosts miek.nl.`, - false, "/etc/hosts", []string{"miek.nl."}, nil, + false, "/etc/hosts", []string{"miek.nl."}, fall.Zero(), }, { `hosts /etc/hosts miek.nl. pun.gent.`, - false, "/etc/hosts", []string{"miek.nl.", "pun.gent."}, nil, + false, "/etc/hosts", []string{"miek.nl.", "pun.gent."}, fall.Zero(), }, { `hosts { fallthrough }`, - false, "/etc/hosts", nil, fall.Zero(), + false, "/etc/hosts", nil, fall.Root(), }, { `hosts /tmp { fallthrough }`, - false, "/tmp", nil, fall.Zero(), + false, "/tmp", nil, fall.Root(), }, { `hosts /etc/hosts miek.nl. { fallthrough }`, - false, "/etc/hosts", []string{"miek.nl."}, fall.Zero(), + false, "/etc/hosts", []string{"miek.nl."}, fall.Root(), }, { `hosts /etc/hosts miek.nl 10.0.0.9/8 { fallthrough }`, - false, "/etc/hosts", []string{"miek.nl.", "10.in-addr.arpa."}, fall.Zero(), + false, "/etc/hosts", []string{"miek.nl.", "10.in-addr.arpa."}, fall.Root(), }, } @@ -92,7 +92,7 @@ func TestHostsInlineParse(t *testing.T) { inputFileRules string shouldErr bool expectedbyAddr map[string][]string - expectedFallthrough *fall.F + expectedFallthrough fall.F }{ { `hosts highly_unlikely_to_exist_hosts_file example.org { @@ -105,7 +105,7 @@ func TestHostsInlineParse(t *testing.T) { `example.org.`, }, }, - fall.Zero(), + fall.Root(), }, { `hosts highly_unlikely_to_exist_hosts_file example.org { @@ -117,7 +117,7 @@ func TestHostsInlineParse(t *testing.T) { `example.org.`, }, }, - nil, + fall.Zero(), }, { `hosts highly_unlikely_to_exist_hosts_file example.org { @@ -126,14 +126,13 @@ func TestHostsInlineParse(t *testing.T) { }`, true, map[string][]string{}, - fall.Zero(), + fall.Root(), }, } for i, test := range tests { c := caddy.NewTestController("dns", test.inputFileRules) h, err := hostsParse(c) - if err == nil && test.shouldErr { t.Fatalf("Test %d expected errors, but got no error", i) } else if err != nil && !test.shouldErr { diff --git a/plugin/kubernetes/kubernetes.go b/plugin/kubernetes/kubernetes.go index a41397848..0cc88b38d 100644 --- a/plugin/kubernetes/kubernetes.go +++ b/plugin/kubernetes/kubernetes.go @@ -41,7 +41,7 @@ type Kubernetes struct { Namespaces map[string]bool podMode string endpointNameMode bool - Fall *fall.F + Fall fall.F ttl uint32 primaryZoneIndex int diff --git a/plugin/kubernetes/setup.go b/plugin/kubernetes/setup.go index f4de8d72a..cdb3b0eac 100644 --- a/plugin/kubernetes/setup.go +++ b/plugin/kubernetes/setup.go @@ -10,7 +10,6 @@ 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" @@ -173,8 +172,7 @@ func kubernetesParse(c *caddy.Controller) (*Kubernetes, dnsControlOpts, error) { } return nil, opts, c.ArgErr() case "fallthrough": - k8s.Fall = fall.New() - k8s.Fall.SetZones(c.RemainingArgs()) + k8s.Fall.SetZonesFromArgs(c.RemainingArgs()) case "upstream": args := c.RemainingArgs() if len(args) == 0 { diff --git a/plugin/kubernetes/setup_test.go b/plugin/kubernetes/setup_test.go index c7c6c15ce..fa6de3e7e 100644 --- a/plugin/kubernetes/setup_test.go +++ b/plugin/kubernetes/setup_test.go @@ -5,6 +5,8 @@ import ( "testing" "time" + "github.com/coredns/coredns/plugin/pkg/fall" + "github.com/mholt/caddy" meta "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -19,7 +21,7 @@ func TestKubernetesParse(t *testing.T) { expectedResyncPeriod time.Duration // expected resync period value expectedLabelSelector string // expected label selector value expectedPodMode string - expectedFallthrough *[]string + expectedFallthrough fall.F expectedUpstreams []string }{ // positive @@ -32,7 +34,7 @@ func TestKubernetesParse(t *testing.T) { defaultResyncPeriod, "", podModeDisabled, - nil, + fall.Zero(), nil, }, { @@ -44,7 +46,7 @@ func TestKubernetesParse(t *testing.T) { defaultResyncPeriod, "", podModeDisabled, - nil, + fall.Zero(), nil, }, { @@ -57,7 +59,7 @@ func TestKubernetesParse(t *testing.T) { defaultResyncPeriod, "", podModeDisabled, - nil, + fall.Zero(), nil, }, { @@ -71,7 +73,7 @@ func TestKubernetesParse(t *testing.T) { defaultResyncPeriod, "", podModeDisabled, - nil, + fall.Zero(), nil, }, { @@ -85,7 +87,7 @@ func TestKubernetesParse(t *testing.T) { defaultResyncPeriod, "", podModeDisabled, - nil, + fall.Zero(), nil, }, { @@ -99,7 +101,7 @@ func TestKubernetesParse(t *testing.T) { defaultResyncPeriod, "", podModeDisabled, - nil, + fall.Zero(), nil, }, { @@ -113,7 +115,7 @@ func TestKubernetesParse(t *testing.T) { 30 * time.Second, "", podModeDisabled, - nil, + fall.Zero(), nil, }, { @@ -127,7 +129,7 @@ func TestKubernetesParse(t *testing.T) { 15 * time.Minute, "", podModeDisabled, - nil, + fall.Zero(), nil, }, { @@ -141,7 +143,7 @@ func TestKubernetesParse(t *testing.T) { defaultResyncPeriod, "environment=prod", podModeDisabled, - nil, + fall.Zero(), nil, }, { @@ -155,7 +157,7 @@ func TestKubernetesParse(t *testing.T) { defaultResyncPeriod, "application=nginx,environment in (production,qa,staging)", podModeDisabled, - nil, + fall.Zero(), nil, }, { @@ -173,7 +175,7 @@ func TestKubernetesParse(t *testing.T) { 15 * time.Minute, "application=nginx,environment in (production,qa,staging)", podModeDisabled, - &[]string{}, + fall.Root(), nil, }, // negative @@ -188,7 +190,7 @@ func TestKubernetesParse(t *testing.T) { defaultResyncPeriod, "", podModeDisabled, - nil, + fall.Zero(), nil, }, { @@ -202,7 +204,7 @@ func TestKubernetesParse(t *testing.T) { defaultResyncPeriod, "", podModeDisabled, - nil, + fall.Zero(), nil, }, { @@ -216,7 +218,7 @@ func TestKubernetesParse(t *testing.T) { 0 * time.Minute, "", podModeDisabled, - nil, + fall.Zero(), nil, }, { @@ -230,7 +232,7 @@ func TestKubernetesParse(t *testing.T) { 0 * time.Second, "", podModeDisabled, - nil, + fall.Zero(), nil, }, { @@ -244,7 +246,7 @@ func TestKubernetesParse(t *testing.T) { 0 * time.Second, "", podModeDisabled, - nil, + fall.Zero(), nil, }, { @@ -258,7 +260,7 @@ func TestKubernetesParse(t *testing.T) { 0 * time.Second, "", podModeDisabled, - nil, + fall.Zero(), nil, }, { @@ -272,7 +274,7 @@ func TestKubernetesParse(t *testing.T) { 0 * time.Second, "", podModeDisabled, - nil, + fall.Zero(), nil, }, // pods disabled @@ -287,7 +289,7 @@ func TestKubernetesParse(t *testing.T) { defaultResyncPeriod, "", podModeDisabled, - nil, + fall.Zero(), nil, }, // pods insecure @@ -302,7 +304,7 @@ func TestKubernetesParse(t *testing.T) { defaultResyncPeriod, "", podModeInsecure, - nil, + fall.Zero(), nil, }, // pods verified @@ -317,7 +319,7 @@ func TestKubernetesParse(t *testing.T) { defaultResyncPeriod, "", podModeVerified, - nil, + fall.Zero(), nil, }, // pods invalid @@ -332,7 +334,7 @@ func TestKubernetesParse(t *testing.T) { defaultResyncPeriod, "", podModeVerified, - nil, + fall.Zero(), nil, }, // fallthrough with zones @@ -347,7 +349,7 @@ func TestKubernetesParse(t *testing.T) { defaultResyncPeriod, "", podModeDisabled, - &[]string{"ip6.arpa.", "inaddr.arpa.", "foo.com."}, + fall.F{Zones: []string{"ip6.arpa.", "inaddr.arpa.", "foo.com."}}, nil, }, // Valid upstream @@ -362,7 +364,7 @@ func TestKubernetesParse(t *testing.T) { defaultResyncPeriod, "", podModeDisabled, - nil, + fall.Zero(), []string{"13.14.15.16:53"}, }, // Invalid upstream @@ -377,7 +379,7 @@ func TestKubernetesParse(t *testing.T) { defaultResyncPeriod, "", podModeDisabled, - nil, + fall.Zero(), nil, }, } @@ -443,23 +445,8 @@ func TestKubernetesParse(t *testing.T) { } // fallthrough - foundFallthrough := k8sController.Fall - if foundFallthrough != nil { - failed := false - if test.expectedFallthrough == nil { - failed = true - } else if len(*foundFallthrough) != len(*test.expectedFallthrough) { - failed = true - } else { - for i := range *foundFallthrough { - if (*foundFallthrough)[i] != (*test.expectedFallthrough)[i] { - failed = true - } - } - } - if failed { - t.Errorf("Test %d: Expected kubernetes controller to be initialized with fallthrough '%v'. Instead found fallthrough '%v' for input '%s'", i, test.expectedFallthrough, foundFallthrough, test.input) - } + if !k8sController.Fall.Equal(test.expectedFallthrough) { + t.Errorf("Test %d: Expected kubernetes controller to be initialized with fallthrough '%v'. Instead found fallthrough '%v' for input '%s'", i, test.expectedFallthrough, k8sController.Fall, test.input) } // upstream foundUpstreams := k8sController.Proxy.Upstreams diff --git a/plugin/pkg/fall/fall.go b/plugin/pkg/fall/fall.go index 99ad34618..adecc4c89 100644 --- a/plugin/pkg/fall/fall.go +++ b/plugin/pkg/fall/fall.go @@ -7,71 +7,52 @@ import ( // 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) } +type F struct { + Zones []string +} // 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 != "" +func (f F) Through(qname string) bool { + return plugin.Zones(f.Zones).Matches(qname) != "" } -// SetZones will set zones in f. -func (f *F) SetZones(zones []string) { +// 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 + f.Zones = 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 +// SetZonesFromArgs sets zones in f to the passed value or to "." if the slice is empty. +func (f *F) SetZonesFromArgs(zones []string) { + if len(zones) == 0 { + f.setZones(Root().Zones) + return + } + f.setZones(zones) } -// 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 { +// Equal returns true if f and g are equal. +func (f F) Equal(g F) bool { + if len(f.Zones) != len(g.Zones) { 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 + for i := range f.Zones { + if f.Zones[i] != g.Zones[i] { + return false } - return false - } - if f.IsZero() { - if g.IsZero() { - return true - } - } - if len(*f) != len(*g) { - return false } return true } + +// Zero returns a zero valued F. +var Zero = func() F { + return F{[]string{}} +} + +// Root returns F set to only ".". +var Root = func() F { + return F{[]string{"."}} +} diff --git a/plugin/pkg/fall/fall_test.go b/plugin/pkg/fall/fall_test.go index 4cc043a38..9988578f6 100644 --- a/plugin/pkg/fall/fall_test.go +++ b/plugin/pkg/fall/fall_test.go @@ -2,41 +2,58 @@ package fall import "testing" -func TestIsNil(t *testing.T) { - var f *F - if !f.IsNil() { - t.Errorf("F should be nil") +func TestEqual(t *testing.T) { + var z F + f := F{Zones: []string{"example.com."}} + g := F{Zones: []string{"example.net."}} + h := F{Zones: []string{"example.com."}} + + if !f.Equal(h) { + t.Errorf("%v should equal %v", f, h) + } + + if z.Equal(f) { + t.Errorf("%v should not be equal to %v", z, f) + } + + if f.Equal(g) { + t.Errorf("%v should not be equal to %v", f, g) } } -func TestIsZero(t *testing.T) { - f := New() - if !f.IsZero() { +func TestZero(t *testing.T) { + var f F + if !f.Equal(Zero()) { t.Errorf("F should be zero") } } -func TestFallThroughExample(t *testing.T) { - if !Example.Through("example.org.") { - t.Errorf("example.org. should fall through") +func TestSetZonesFromArgs(t *testing.T) { + var f F + f.SetZonesFromArgs([]string{}) + if !f.Equal(Root()) { + t.Errorf("F should have the root zone") } - if Example.Through("example.net.") { - t.Errorf("example.net. should not fall through") + + f.SetZonesFromArgs([]string{"example.com", "example.net."}) + expected := F{Zones: []string{"example.com.", "example.net."}} + if !f.Equal(expected) { + t.Errorf("F should be %v but is %v", expected, f) } } func TestFallthrough(t *testing.T) { - var fall *F + var fall F if fall.Through("foo.com.") { - t.Errorf("Expected false, got true for nil fallthrough") + t.Errorf("Expected false, got true for zero fallthrough") } - fall = New() + fall.SetZonesFromArgs([]string{}) if !fall.Through("foo.net.") { t.Errorf("Expected true, got false for all zone fallthrough") } - fall.SetZones([]string{"foo.com", "bar.com"}) + fall.SetZonesFromArgs([]string{"foo.com", "bar.com"}) if fall.Through("foo.net.") { t.Errorf("Expected false, got true for non-matching fallthrough zone") diff --git a/plugin/reverse/reverse.go b/plugin/reverse/reverse.go index 912d998b8..8b9e403d5 100644 --- a/plugin/reverse/reverse.go +++ b/plugin/reverse/reverse.go @@ -17,7 +17,7 @@ type Reverse struct { Next plugin.Handler Networks networks - Fall *fall.F + Fall fall.F } // ServeDNS implements the plugin.Handler interface. diff --git a/plugin/reverse/setup.go b/plugin/reverse/setup.go index b45d3cada..db7ddd665 100644 --- a/plugin/reverse/setup.go +++ b/plugin/reverse/setup.go @@ -34,7 +34,7 @@ func setupReverse(c *caddy.Controller) error { return nil } -func reverseParse(c *caddy.Controller) (nets networks, f *fall.F, err error) { +func reverseParse(c *caddy.Controller) (nets networks, f fall.F, err error) { zones := make([]string, len(c.ServerBlockKeys)) wildcard := false @@ -87,8 +87,7 @@ func reverseParse(c *caddy.Controller) (nets networks, f *fall.F, err error) { wildcard = true case "fallthrough": - f = fall.New() - f.SetZones(c.RemainingArgs()) + f.SetZonesFromArgs(c.RemainingArgs()) default: return nil, f, c.ArgErr()