Adding test cases for Corefile parsing (#193)

Adding test cases for Corefile parsing.
Some code refactoring to allow test reuse.

7 files changed, 260 insertions, 44 deletions

diff --git a/.gitignore b/.gitignore
index 699137222..8d0749b37 100644
--- a/.gitignore
+++ b/.gitignore
@@ -3,3 +3,4 @@ Corefile
 *.swp
 coredns
 kubectl
+go-test-tmpfile*
diff --git a/core/setup/kubernetes.go b/core/setup/kubernetes.go
index aea9e334a..2f8b5be44 100644
--- a/core/setup/kubernetes.go
+++ b/core/setup/kubernetes.go
@@ -1,23 +1,14 @@
 package setup
 
 import (
-	//"crypto/tls"
-	//"crypto/x509"
 	"log"
-	//"io/ioutil"
-	//"net"
-	//"net/http"
 	"strings"
-	//"time"
 
 	"github.com/miekg/coredns/middleware"
 	"github.com/miekg/coredns/middleware/kubernetes"
 	k8sc "github.com/miekg/coredns/middleware/kubernetes/k8sclient"
 	"github.com/miekg/coredns/middleware/kubernetes/nametemplate"
 	"github.com/miekg/coredns/middleware/proxy"
-	//"github.com/miekg/coredns/middleware/singleflight"
-
-	"golang.org/x/net/context"
 )
 
 const (
@@ -43,19 +34,8 @@ func Kubernetes(c *Controller) (middleware.Middleware, error) {
 }
 
 func kubernetesParse(c *Controller) (kubernetes.Kubernetes, error) {
-
-	/*
-	 * TODO: Remove unused state and simplify.
-	 * Inflight and Ctx might not be needed. Leaving in place until
-	 * we take a pass at API caching and optimizing connector to the
-	 * k8s API. Single flight (or limited upper-bound) for inflight
-	 * API calls may be desirable.
-	 */
-
 	k8s := kubernetes.Kubernetes{
 		Proxy: proxy.New([]string{}),
-		Ctx:   context.Background(),
-		//      Inflight:   &singleflight.Group{},
 	}
 	var (
 		endpoints  = []string{defaultK8sEndpoint}
@@ -78,7 +58,12 @@ func kubernetesParse(c *Controller) (kubernetes.Kubernetes, error) {
 				k8s.Zones = kubernetes.NormalizeZoneList(zones)
 			}
 
+			// TODO: clean this parsing up
+
 			middleware.Zones(k8s.Zones).FullyQualify()
+
+			log.Printf("[debug] c data: %v\n", c)
+
 			if c.NextBlock() {
 				// TODO(miek): 2 switches?
 				switch c.Val() {
@@ -89,6 +74,13 @@ func kubernetesParse(c *Controller) (kubernetes.Kubernetes, error) {
 					}
 					endpoints = args
 					k8s.APIConn = k8sc.NewK8sConnector(endpoints[0])
+				case "namespaces":
+					args := c.RemainingArgs()
+					if len(args) == 0 {
+						return kubernetes.Kubernetes{}, c.ArgErr()
+					}
+					namespaces = args
+					k8s.Namespaces = append(k8s.Namespaces, namespaces...)
 				}
 				for c.Next() {
 					switch c.Val() {
@@ -108,7 +100,7 @@ func kubernetesParse(c *Controller) (kubernetes.Kubernetes, error) {
 							return kubernetes.Kubernetes{}, c.ArgErr()
 						}
 						namespaces = args
-						k8s.Namespaces = &namespaces
+						k8s.Namespaces = append(k8s.Namespaces, namespaces...)
 					}
 				}
 			}
diff --git a/core/setup/kubernetes_test.go b/core/setup/kubernetes_test.go
new file mode 100644
index 000000000..b0141bab0
--- /dev/null
+++ b/core/setup/kubernetes_test.go
@@ -0,0 +1,197 @@
+package setup
+
+import (
+	"strings"
+	"testing"
+)
+
+/*
+kubernetes coredns.local {
+        # Use url for k8s API endpoint
+        endpoint http://localhost:8080
+        # Assemble k8s record names with the template
+        template {service}.{namespace}.{zone}
+        # Only expose the k8s namespace "demo"
+        #namespaces demo
+    }
+*/
+
+func TestKubernetesParse(t *testing.T) {
+	tests := []struct {
+		input              string
+		shouldErr          bool
+		expectedErrContent string // substring from the expected error. Empty for positive cases.
+		expectedZoneCount  int    // expected count of defined zones. '-1' for negative cases.
+		expectedNTValid    bool   // NameTemplate to be initialized and valid
+		expectedNSCount    int    // expected count of namespaces. '-1' for negative cases.
+	}{
+		// positive
+		// TODO: not specifiying a zone maybe should error out.
+		{
+			`kubernetes`,
+			false,
+			"",
+			0,
+			true,
+			0,
+		},
+		{
+			`kubernetes coredns.local`,
+			false,
+			"",
+			1,
+			true,
+			0,
+		},
+		{
+			`kubernetes coredns.local test.local`,
+			false,
+			"",
+			2,
+			true,
+			0,
+		},
+		{
+			`kubernetes coredns.local {
+    endpoint http://localhost:9090
+}`,
+			false,
+			"",
+			1,
+			true,
+			0,
+		},
+		{
+			`kubernetes coredns.local {
+	template {service}.{namespace}.{zone}
+}`,
+			false,
+			"",
+			1,
+			true,
+			0,
+		},
+		{
+			`kubernetes coredns.local {
+	namespaces demo
+}`,
+			false,
+			"",
+			1,
+			true,
+			1,
+		},
+		{
+			`kubernetes coredns.local {
+	namespaces demo test
+}`,
+			false,
+			"",
+			1,
+			true,
+			2,
+		},
+
+		// negative
+		{
+			`kubernetes coredns.local {
+    endpoint
+}`,
+			true,
+			"Wrong argument count or unexpected line ending after 'endpoint'",
+			-1,
+			true,
+			-1,
+		},
+		// No template provided for template line.
+		{
+			`kubernetes coredns.local {
+    template
+}`,
+			true,
+			"",
+			-1,
+			false,
+			-1,
+		},
+		// Invalid template provided
+		{
+			`kubernetes coredns.local {
+    template {namespace}.{zone}
+}`,
+			true,
+			"",
+			-1,
+			false,
+			-1,
+		},
+		/*
+			 		// No valid provided for namespaces
+			   		{
+			   			`kubernetes coredns.local {
+			     namespaces
+			}`,
+			   			true,
+			   			"",
+			   			-1,
+						true,
+			   			-1,
+			   		},
+		*/
+	}
+
+	for i, test := range tests {
+		c := NewTestController(test.input)
+		k8sController, err := kubernetesParse(c)
+		t.Logf("i: %v\n", i)
+		t.Logf("controller: %v\n", k8sController)
+
+		if test.shouldErr && err == nil {
+			t.Errorf("Test %d: Expected error, but found one for input '%s'. Error was: '%v'", i, test.input, err)
+		}
+
+		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)
+			}
+
+			if test.shouldErr && (len(test.expectedErrContent) < 1) {
+				t.Fatalf("Test %d: Test marked as expecting an error, but no expectedErrContent provided for input '%s'. Error was: '%v'", i, test.input, err)
+			}
+
+			if test.shouldErr && (test.expectedZoneCount >= 0) {
+				t.Fatalf("Test %d: Test marked as expecting an error, but provides value for expectedZoneCount!=-1 for input '%s'. Error was: '%v'", i, test.input, err)
+			}
+
+			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)
+			}
+
+			return
+		}
+
+		// No error was raised, so validate initialization of k8sController
+		//     Zones
+		foundZoneCount := len(k8sController.Zones)
+		if foundZoneCount != test.expectedZoneCount {
+			t.Errorf("Test %d: Expected kubernetes controller to be initialized with %d zones, instead found %d zones: '%v' for input '%s'", i, test.expectedZoneCount, foundZoneCount, k8sController.Zones, test.input)
+		}
+
+		//    NameTemplate
+		if k8sController.NameTemplate == nil {
+			t.Errorf("Test %d: Expected kubernetes controller to be initialized with a NameTemplate. Instead found '%v' for input '%s'", i, k8sController.NameTemplate, test.input)
+		} else {
+			foundNTValid := k8sController.NameTemplate.IsValid()
+			if foundNTValid != test.expectedNTValid {
+				t.Errorf("Test %d: Expected NameTemplate validity to be '%v', instead found '%v' for input '%s'", i, test.expectedNTValid, foundNTValid, test.input)
+			}
+		}
+
+		//    Namespaces
+		foundNSCount := len(k8sController.Namespaces)
+		if foundNSCount != test.expectedNSCount {
+			t.Errorf("Test %d: Expected kubernetes controller to be initialized with %d namespaces. Instead found %d namespaces: '%v' for input '%s'", i, test.expectedNSCount, foundNSCount, k8sController.Namespaces, test.input)
+		}
+
+	}
+}
diff --git a/middleware/kubernetes/k8stest/k8stest.go b/middleware/kubernetes/k8stest/k8stest.go
new file mode 100644
index 000000000..f846b0d92
--- /dev/null
+++ b/middleware/kubernetes/k8stest/k8stest.go
@@ -0,0 +1,11 @@
+package k8stest
+
+import (
+	"net/http"
+)
+
+// checkKubernetesRunning performs a basic
+func CheckKubernetesRunning() bool {
+	_, err := http.Get("http://localhost:8080/api/v1")
+	return err == nil
+}
diff --git a/middleware/kubernetes/kubernetes.go b/middleware/kubernetes/kubernetes.go
index b8ae4f5ea..6288cb394 100644
--- a/middleware/kubernetes/kubernetes.go
+++ b/middleware/kubernetes/kubernetes.go
@@ -12,21 +12,17 @@ import (
 	"github.com/miekg/coredns/middleware/kubernetes/nametemplate"
 	"github.com/miekg/coredns/middleware/kubernetes/util"
 	"github.com/miekg/coredns/middleware/proxy"
-	//	"github.com/miekg/coredns/middleware/singleflight"
 
 	"github.com/miekg/dns"
-	"golang.org/x/net/context"
 )
 
 type Kubernetes struct {
-	Next  middleware.Handler
-	Zones []string
-	Proxy proxy.Proxy // Proxy for looking up names during the resolution process
-	Ctx   context.Context
-	//	Inflight   *singleflight.Group
+	Next         middleware.Handler
+	Zones        []string
+	Proxy        proxy.Proxy // Proxy for looking up names during the resolution process
 	APIConn      *k8sc.K8sConnector
 	NameTemplate *nametemplate.NameTemplate
-	Namespaces   *[]string
+	Namespaces   []string
 }
 
 // getZoneForName returns the zone string that matches the name and a
@@ -84,6 +80,8 @@ func (g Kubernetes) Records(name string, exact bool) ([]msg.Service, error) {
 		serviceName = util.WildcardStar
 	}
 
+	log.Printf("[debug] published namespaces: %v\n", g.Namespaces)
+
 	log.Printf("[debug] exact: %v\n", exact)
 	log.Printf("[debug] zone: %v\n", zone)
 	log.Printf("[debug] servicename: %v\n", serviceName)
@@ -96,7 +94,7 @@ func (g Kubernetes) Records(name string, exact bool) ([]msg.Service, error) {
 
 	// Abort if the namespace does not contain a wildcard, and namespace is not published per CoreFile
 	// Case where namespace contains a wildcard is handled in Get(...) method.
-	if (!nsWildcard) && (g.Namespaces != nil && !util.StringInSlice(namespace, *g.Namespaces)) {
+	if (!nsWildcard) && (len(g.Namespaces) > 0) && (!util.StringInSlice(namespace, g.Namespaces)) {
 		log.Printf("[debug] Namespace '%v' is not published by Corefile\n", namespace)
 		return nil, nil
 	}
@@ -157,7 +155,7 @@ func (g Kubernetes) Get(namespace string, nsWildcard bool, servicename string, s
 		if symbolMatches(namespace, item.Metadata.Namespace, nsWildcard) && symbolMatches(servicename, item.Metadata.Name, serviceWildcard) {
 			// If namespace has a wildcard, filter results against Corefile namespace list.
 			// (Namespaces without a wildcard were filtered before the call to this function.)
-			if nsWildcard && (g.Namespaces != nil && !util.StringInSlice(item.Metadata.Namespace, *g.Namespaces)) {
+			if nsWildcard && (len(g.Namespaces) > 0) && (!util.StringInSlice(item.Metadata.Namespace, g.Namespaces)) {
 				log.Printf("[debug] Namespace '%v' is not published by Corefile\n", item.Metadata.Namespace)
 				continue
 			}
diff --git a/middleware/kubernetes/nametemplate/nametemplate.go b/middleware/kubernetes/nametemplate/nametemplate.go
index c2af686cd..5a34ae4ad 100644
--- a/middleware/kubernetes/nametemplate/nametemplate.go
+++ b/middleware/kubernetes/nametemplate/nametemplate.go
@@ -37,6 +37,11 @@ var types = []string{
 	"pod",
 }
 
+var requiredSymbols = []string{
+	"namespace",
+	"service",
+}
+
 // TODO: Validate that provided NameTemplate string only contains:
 //			* valid, known symbols, or
 //			* static strings
@@ -90,6 +95,12 @@ func (t *NameTemplate) SetTemplate(s string) error {
 		}
 	}
 
+	if err == nil && !t.IsValid() {
+		err = errors.New("Record name template does not pass NameTemplate validation")
+		log.Printf("[debug] %v\n", err)
+		return err
+	}
+
 	return err
 }
 
@@ -157,6 +168,20 @@ func (t *NameTemplate) GetRecordNameFromNameValues(values NameValues) string {
 	return strings.Join(recordName, ".")
 }
 
+func (t *NameTemplate) IsValid() bool {
+	result := true
+
+	// Ensure that all requiredSymbols are found in NameTemplate
+	for _, symbol := range requiredSymbols {
+		if _, ok := t.Element[symbol]; !ok {
+			result = false
+			break
+		}
+	}
+
+	return result
+}
+
 type NameValues struct {
 	ServiceName string
 	Namespace   string
diff --git a/test/kubernetes_test.go b/test/kubernetes_test.go
index 6f6599d41..5f543836d 100644
--- a/test/kubernetes_test.go
+++ b/test/kubernetes_test.go
@@ -6,9 +6,9 @@ import (
 	"fmt"
 	"io/ioutil"
 	"log"
-	"net/http"
 	"testing"
 
+	"github.com/miekg/coredns/middleware/kubernetes/k8stest"
 	"github.com/miekg/dns"
 )
 
@@ -63,12 +63,6 @@ var testdataLookupSRV = []struct {
 	{"*.*.coredns.local.", 1, 1},                             // One SRV record, via namespace and service wildcard
 }
 
-// checkKubernetesRunning performs a basic
-func checkKubernetesRunning() bool {
-	_, err := http.Get("http://localhost:8080/api/v1")
-	return err == nil
-}
-
 func TestK8sIntegration(t *testing.T) {
 	t.Log("   === RUN testLookupA")
 	testLookupA(t)
@@ -77,13 +71,12 @@ func TestK8sIntegration(t *testing.T) {
 }
 
 func testLookupA(t *testing.T) {
-	if !checkKubernetesRunning() {
+	if !k8stest.CheckKubernetesRunning() {
 		t.Skip("Skipping Kubernetes Integration tests. Kubernetes is not running")
 	}
 
-	// Note: Use different port to avoid conflict with servers used in other tests.
 	coreFile :=
-		`.:2053 {
+		`.:0 {
     kubernetes coredns.local {
 		endpoint http://localhost:8080
 		namespaces demo
@@ -128,13 +121,12 @@ func testLookupA(t *testing.T) {
 }
 
 func testLookupSRV(t *testing.T) {
-	if !checkKubernetesRunning() {
+	if !k8stest.CheckKubernetesRunning() {
 		t.Skip("Skipping Kubernetes Integration tests. Kubernetes is not running")
 	}
 
-	// Note: Use different port to avoid conflict with servers used in other tests.
 	coreFile :=
-		`.:2054 {
+		`.:0 {
     kubernetes coredns.local {
 		endpoint http://localhost:8080
 		namespaces demo