middleware/etcd: add setup_test.go (#304)

Add tests for parsing etcd Corefile stanza. Discover a bug in the code,
fix that as well.

middleware/etcd/etcd.go

@@ -27,6 +27,8 @@ type Etcd struct {
 	Inflight   *singleflight.Group
 	Stubmap    *map[string]proxy.Proxy // list of proxies for stub resolving.
 	Debug      bool                    // Do we allow debug queries.
+
+	endpoints []string // Stored here as well, to aid in testing.
 }
 
 // Records looks up records in etcd. If exact is true, it will lookup just

middleware/etcd/setup.go

@@ -100,7 +100,6 @@ func etcdParse(c *caddy.Controller) (*Etcd, bool, error) {
 							args[i] = h + ":53"
 						}
 					}
-					endpoints = args
 					etc.Proxy = proxy.New(args)
 				case "tls": // cert key cacertfile
 					args := c.RemainingArgs()
@@ -108,6 +107,10 @@ func etcdParse(c *caddy.Controller) (*Etcd, bool, error) {
 						return &Etcd{}, false, c.ArgErr()
 					}
 					tlsCertFile, tlsKeyFile, tlsCAcertFile = args[0], args[1], args[2]
+				default:
+					if c.Val() != "}" {
+						return &Etcd{}, false, c.Errf("unknown property '%s'", c.Val())
+					}
 				}
 				for c.Next() {
 					switch c.Val() {
@@ -144,14 +147,20 @@ func etcdParse(c *caddy.Controller) (*Etcd, bool, error) {
 							return &Etcd{}, false, c.ArgErr()
 						}
 						tlsCertFile, tlsKeyFile, tlsCAcertFile = args[0], args[1], args[2]
+					default:
+						if c.Val() != "}" { // TODO(miek): this feels like I'm doing it completely wrong.
+							return &Etcd{}, false, c.Errf("unknown property '%s'", c.Val())
 						}
 					}
 				}
+
+			}
 			client, err := newEtcdClient(endpoints, tlsCertFile, tlsKeyFile, tlsCAcertFile)
 			if err != nil {
 				return &Etcd{}, false, err
 			}
 			etc.Client = client
+			etc.endpoints = endpoints
 			return &etc, stubzones, nil
 		}
 	}

middleware/etcd/setup_test.go

@@ -5,9 +5,11 @@ package etcd
 import (
 	"encoding/json"
 	"sort"
+	"strings"
 	"testing"
 	"time"
 
+	"github.com/mholt/caddy"
 	"github.com/miekg/coredns/middleware/etcd/msg"
 	"github.com/miekg/coredns/middleware/pkg/dnsrecorder"
 	"github.com/miekg/coredns/middleware/pkg/singleflight"
@@ -23,7 +25,6 @@ func init() {
 	ctxt, _ = context.WithTimeout(context.Background(), etcdTimeout)
 }
 
-//	etc    *Etcd
 func newEtcdMiddleware() *Etcd {
 	ctxt, _ = context.WithTimeout(context.Background(), etcdTimeout)
 
@@ -93,4 +94,60 @@ func TestLookup(t *testing.T) {
 	}
 }
 
+func TestSetupEtcd(t *testing.T) {
+	tests := []struct {
+		input              string
+		shouldErr          bool
+		expectedPath       string
+		expectedEndpoint   string
+		expectedErrContent string // substring from the expected error. Empty for positive cases.
+	}{
+		// positive
+		{
+			`etcd`, false, "skydns", "http://localhost:2379", "",
+		},
+		{
+			`etcd skydns.local {
+	endpoint localhost:300
+}
+`, false, "skydns", "localhost:300", "",
+		},
+		// negative
+		{
+			`etcd {
+	endpoints localhost:300
+}
+`, true, "", "", "unknown property 'endpoints'",
+		},
+	}
+
+	for i, test := range tests {
+		c := caddy.NewTestController("dns", test.input)
+		etcd, _ /*stubzones*/, err := etcdParse(c)
+
+		if test.shouldErr && err == nil {
+			t.Errorf("Test %d: Expected error but found %s for input %s", i, err, test.input)
+		}
+
+		if err != nil {
+			if !test.shouldErr {
+				t.Errorf("Test %d: Expected no error but found one for input %s. Error was: %v", i, test.input, err)
+				continue
+			}
+
+			if !strings.Contains(err.Error(), test.expectedErrContent) {
+				t.Errorf("Test %d: Expected error to contain: %v, found error: %v, input: %s", i, test.expectedErrContent, err, test.input)
+				continue
+			}
+		}
+
+		if !test.shouldErr && etcd.PathPrefix != test.expectedPath {
+			t.Errorf("Etcd not correctly set for input %s. Expected: %s, actual: %s", test.input, test.expectedPath, etcd.PathPrefix)
+		}
+		if !test.shouldErr && etcd.endpoints[0] != test.expectedEndpoint { // only checks the first
+			t.Errorf("Etcd not correctly set for input %s. Expected: '%s', actual: '%s'", test.input, test.expectedEndpoint, etcd.endpoints[0])
+		}
+	}
+}
+
 var ctxt context.Context