aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGravatar John Belamaric <jbelamaric@infoblox.com> 2018-01-07 14:51:32 -0500
committerGravatar GitHub <noreply@github.com> 2018-01-07 14:51:32 -0500
commitc59f5f6e86e1d041a9aa7b324b90582b3a4b277a (patch)
treec8091f7101125f066d5bc59c20dbf6b4b192311d
parentc6febe6250361eee580dbb8a601a444f23ed7ac2 (diff)
downloadcoredns-c59f5f6e86e1d041a9aa7b324b90582b3a4b277a.tar.gz
coredns-c59f5f6e86e1d041a9aa7b324b90582b3a4b277a.tar.zst
coredns-c59f5f6e86e1d041a9aa7b324b90582b3a4b277a.zip
Simplify plugin/pkg/fall (#1358)
* Simplify plugin/pkg/fall * Remove unused import * Fix fall_test * Get fall coverage to 100% just because * gofmt. sigh.
-rw-r--r--plugin/etcd/etcd.go2
-rw-r--r--plugin/etcd/multi_test.go2
-rw-r--r--plugin/etcd/setup.go4
-rw-r--r--plugin/hosts/hosts.go2
-rw-r--r--plugin/hosts/setup.go6
-rw-r--r--plugin/hosts/setup_test.go27
-rw-r--r--plugin/kubernetes/kubernetes.go2
-rw-r--r--plugin/kubernetes/setup.go4
-rw-r--r--plugin/kubernetes/setup_test.go73
-rw-r--r--plugin/pkg/fall/fall.go79
-rw-r--r--plugin/pkg/fall/fall_test.go49
-rw-r--r--plugin/reverse/reverse.go2
-rw-r--r--plugin/reverse/setup.go5
13 files changed, 116 insertions, 141 deletions
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
-}
-
-// 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
+// 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
}
- return len(*f) == 0
+ f.setZones(zones)
}
-// 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
- }
+// 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
}
- if f.IsZero() {
- if g.IsZero() {
- return true
+ for i := range f.Zones {
+ if f.Zones[i] != g.Zones[i] {
+ return false
}
}
- 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()