Handle K8s middleware NS record (#662)

* commit for testing in cluster

* commit for testing in cluster

* refactor and add ns.dns record

* Release 007

* reduce heap allocations

* gofmt

* revert accidental Makefile commits

* restore prior rcode for disabled pod mode

* revert Makefile deltas

* add unit tests

* more unit tests

* make isRequestInReverseRange easier to test

* more unit tests

* addressing review feedback

* commit setup.go

middleware/kubernetes/controller.go

@@ -35,7 +35,15 @@ func (s *storeToNamespaceLister) List() (ns api.NamespaceList, err error) {
 	return ns, nil
 }
 
-type dnsController struct {
+type dnsController interface {
+	ServiceList() []*api.Service
+	PodIndex(string) []interface{}
+	EndpointsList() api.EndpointsList
+	Run()
+	Stop() error
+}
+
+type dnsControl struct {
 	client *kubernetes.Clientset
 
 	selector *labels.Selector
@@ -59,8 +67,8 @@ type dnsController struct {
 }
 
 // newDNSController creates a controller for CoreDNS.
-func newdnsController(kubeClient *kubernetes.Clientset, resyncPeriod time.Duration, lselector *labels.Selector, initPodCache bool) *dnsController {
-	dns := dnsController{
+func newdnsController(kubeClient *kubernetes.Clientset, resyncPeriod time.Duration, lselector *labels.Selector, initPodCache bool) *dnsControl {
+	dns := dnsControl{
 		client:   kubeClient,
 		selector: lselector,
 		stopCh:   make(chan struct{}),
@@ -286,12 +294,12 @@ func endpointsWatchFunc(c *kubernetes.Clientset, ns string, s *labels.Selector)
 	}
 }
 
-func (dns *dnsController) controllersInSync() bool {
+func (dns *dnsControl) controllersInSync() bool {
 	return dns.svcController.HasSynced()
 }
 
 // Stop stops the  controller.
-func (dns *dnsController) Stop() error {
+func (dns *dnsControl) Stop() error {
 	dns.stopLock.Lock()
 	defer dns.stopLock.Unlock()
 
@@ -307,7 +315,7 @@ func (dns *dnsController) Stop() error {
 }
 
 // Run starts the controller.
-func (dns *dnsController) Run() {
+func (dns *dnsControl) Run() {
 	go dns.svcController.Run(dns.stopCh)
 	go dns.nsController.Run(dns.stopCh)
 	go dns.epController.Run(dns.stopCh)
@@ -317,7 +325,7 @@ func (dns *dnsController) Run() {
 	<-dns.stopCh
 }
 
-func (dns *dnsController) NamespaceList() *api.NamespaceList {
+func (dns *dnsControl) NamespaceList() *api.NamespaceList {
 	nsList, err := dns.nsLister.List()
 	if err != nil {
 		return &api.NamespaceList{}
@@ -326,7 +334,7 @@ func (dns *dnsController) NamespaceList() *api.NamespaceList {
 	return &nsList
 }
 
-func (dns *dnsController) ServiceList() []*api.Service {
+func (dns *dnsControl) ServiceList() []*api.Service {
 	svcs, err := dns.svcLister.List(labels.Everything())
 	if err != nil {
 		return []*api.Service{}
@@ -335,10 +343,28 @@ func (dns *dnsController) ServiceList() []*api.Service {
 	return svcs
 }
 
+func (dns *dnsControl) PodIndex(ip string) []interface{} {
+	pods, err := dns.podLister.Indexer.ByIndex(podIPIndex, ip)
+	if err != nil {
+		return nil
+	}
+
+	return pods
+}
+
+func (dns *dnsControl) EndpointsList() api.EndpointsList {
+	epl, err := dns.epLister.List()
+	if err != nil {
+		return api.EndpointsList{}
+	}
+
+	return epl
+}
+
 // ServicesByNamespace returns a map of:
 //
 // namespacename :: [ kubernetesService ]
-func (dns *dnsController) ServicesByNamespace() map[string][]api.Service {
+func (dns *dnsControl) ServicesByNamespace() map[string][]api.Service {
 	k8sServiceList := dns.ServiceList()
 	items := make(map[string][]api.Service, len(k8sServiceList))
 	for _, i := range k8sServiceList {
@@ -350,7 +376,7 @@ func (dns *dnsController) ServicesByNamespace() map[string][]api.Service {
 }
 
 // ServiceInNamespace returns the Service that matches servicename in the namespace
-func (dns *dnsController) ServiceInNamespace(namespace, servicename string) *api.Service {
+func (dns *dnsControl) ServiceInNamespace(namespace, servicename string) *api.Service {
 	svcObj, err := dns.svcLister.Services(namespace).Get(servicename)
 	if err != nil {
 		// TODO(...): should return err here

middleware/kubernetes/handler.go

@@ -32,7 +32,7 @@ func (k Kubernetes) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.M
 		// If this is a PTR request, and the request is in a defined
 		// pod/service cidr range, process the request in this middleware,
 		// otherwise pass to next middleware.
-		if !k.isRequestInReverseRange(state) {
+		if !k.isRequestInReverseRange(state.Name()) {
 			return middleware.NextOrFailure(k.Name(), k.Next, ctx, w, r)
 		}
 		// Set the zone to this specific request.

middleware/kubernetes/kubernetes_test.go

 package kubernetes
 
-import "testing"
-import "reflect"
-
-// Test data for TestSymbolContainsWildcard cases.
-var testdataSymbolContainsWildcard = []struct {
-	Symbol         string
-	ExpectedResult bool
-}{
-	{"mynamespace", false},
-	{"*", true},
-	{"any", true},
-	{"my*space", false},
-	{"*space", false},
-	{"myname*", false},
+import (
+	"errors"
+	"net"
+	"reflect"
+	"testing"
+
+	"github.com/miekg/dns"
+	"k8s.io/client-go/1.5/pkg/api"
+
+	"github.com/coredns/coredns/middleware/etcd/msg"
+)
+
+func TestRecordForTXT(t *testing.T) {
+	k := Kubernetes{Zones: []string{"inter.webs.test"}}
+	r, _ := k.parseRequest("dns-version.inter.webs.test", dns.TypeTXT)
+	expected := DNSSchemaVersion
+
+	var svcs []msg.Service
+	k.recordsForTXT(r, &svcs)
+	if svcs[0].Text != expected {
+		t.Errorf("Expected result '%v'. Instead got result '%v'.", expected, svcs[0].Text)
+	}
+}
+
+func TestPrimaryZone(t *testing.T) {
+	k := Kubernetes{Zones: []string{"inter.webs.test", "inter.nets.test"}}
+	expected := "inter.webs.test"
+	result := k.PrimaryZone()
+	if result != expected {
+		t.Errorf("Expected result '%v'. Instead got result '%v'.", expected, result)
+	}
+}
+
+func TestIsRequestInReverseRange(t *testing.T) {
+
+	tests := []struct {
+		cidr     string
+		name     string
+		expected bool
+	}{
+		{"1.2.3.0/24", "4.3.2.1.in-addr.arpa.", true},
+		{"1.2.3.0/24", "5.3.2.1.in-addr.arpa.", true},
+		{"1.2.3.0/24", "5.4.2.1.in-addr.arpa.", false},
+		{"5.6.0.0/16", "5.4.2.1.in-addr.arpa.", false},
+		{"5.6.0.0/16", "5.4.6.5.in-addr.arpa.", true},
+		{"5.6.0.0/16", "5.6.0.1.in-addr.arpa.", false},
+	}
+
+	k := Kubernetes{Zones: []string{"inter.webs.test"}}
+
+	for _, test := range tests {
+		_, cidr, _ := net.ParseCIDR(test.cidr)
+		k.ReverseCidrs = []net.IPNet{*cidr}
+		result := k.isRequestInReverseRange(test.name)
+		if result != test.expected {
+			t.Errorf("Expected '%v' for '%v' in %v.", test.expected, test.name, test.cidr)
+		}
+	}
+}
+
+func TestIsNameError(t *testing.T) {
+	k := Kubernetes{Zones: []string{"inter.webs.test"}}
+	if !k.IsNameError(errNoItems) {
+		t.Errorf("Expected 'true' for '%v'", errNoItems)
+	}
+	if !k.IsNameError(errNsNotExposed) {
+		t.Errorf("Expected 'true' for '%v'", errNsNotExposed)
+	}
+	if !k.IsNameError(errInvalidRequest) {
+		t.Errorf("Expected 'true' for '%v'", errInvalidRequest)
+	}
+	otherErr := errors.New("Some other error occured")
+	if k.IsNameError(otherErr) {
+		t.Errorf("Expected 'true' for '%v'", otherErr)
+	}
 }
 
 func TestSymbolContainsWildcard(t *testing.T) {
+	var testdataSymbolContainsWildcard = []struct {
+		Symbol         string
+		ExpectedResult bool
+	}{
+		{"mynamespace", false},
+		{"*", true},
+		{"any", true},
+		{"my*space", false},
+		{"*space", false},
+		{"myname*", false},
+	}
+
 	for _, example := range testdataSymbolContainsWildcard {
 		actualResult := symbolContainsWildcard(example.Symbol)
 		if actualResult != example.ExpectedResult {
@@ -44,7 +117,7 @@ func TestParseRequest(t *testing.T) {
 	// Test a valid SRV request
 	//
 	query := "_http._tcp.webs.mynamespace.svc.inter.webs.test."
-	r, e := k.parseRequest(query, "SRV")
+	r, e := k.parseRequest(query, dns.TypeSRV)
 	if e != nil {
 		t.Errorf("Expected no error from parseRequest(%v, \"SRV\"). Instead got '%v'.", query, e)
 	}
@@ -65,7 +138,7 @@ func TestParseRequest(t *testing.T) {
 	// Test wildcard acceptance
 	//
 	query = "*.any.*.any.svc.inter.webs.test."
-	r, e = k.parseRequest(query, "SRV")
+	r, e = k.parseRequest(query, dns.TypeSRV)
 	if e != nil {
 		t.Errorf("Expected no error from parseRequest(\"%v\", \"SRV\"). Instead got '%v'.", query, e)
 	}
@@ -85,7 +158,7 @@ func TestParseRequest(t *testing.T) {
 
 	// Test A request of endpoint
 	query = "1-2-3-4.webs.mynamespace.svc.inter.webs.test."
-	r, e = k.parseRequest(query, "A")
+	r, e = k.parseRequest(query, dns.TypeA)
 	if e != nil {
 		t.Errorf("Expected no error from parseRequest(\"%v\", \"A\"). Instead got '%v'.", query, e)
 	}
@@ -102,6 +175,44 @@ func TestParseRequest(t *testing.T) {
 		expectString(t, f, "A", query, &r, field, expected)
 	}
 
+	// Test NS request
+	query = "inter.webs.test."
+	r, e = k.parseRequest(query, dns.TypeNS)
+	if e != nil {
+		t.Errorf("Expected no error from parseRequest(\"%v\", \"NS\"). Instead got '%v'.", query, e)
+	}
+	tcs = map[string]string{
+		"port":      "",
+		"protocol":  "",
+		"endpoint":  "",
+		"service":   "",
+		"namespace": "",
+		"typeName":  "",
+		"zone":      "inter.webs.test",
+	}
+	for field, expected := range tcs {
+		expectString(t, f, "NS", query, &r, field, expected)
+	}
+
+	// Test TXT request
+	query = "dns-version.inter.webs.test."
+	r, e = k.parseRequest(query, dns.TypeTXT)
+	if e != nil {
+		t.Errorf("Expected no error from parseRequest(\"%v\", \"TXT\"). Instead got '%v'.", query, e)
+	}
+	tcs = map[string]string{
+		"port":      "",
+		"protocol":  "",
+		"endpoint":  "",
+		"service":   "",
+		"namespace": "",
+		"typeName":  "dns-version",
+		"zone":      "inter.webs.test",
+	}
+	for field, expected := range tcs {
+		expectString(t, f, "TXT", query, &r, field, expected)
+	}
+
 	// Invalid query tests
 	invalidAQueries := []string{
 		"_http._tcp.webs.mynamespace.svc.inter.webs.test.", // A requests cannot have port or protocol
@@ -109,7 +220,7 @@ func TestParseRequest(t *testing.T) {
 
 	}
 	for _, q := range invalidAQueries {
-		_, e = k.parseRequest(q, "A")
+		_, e = k.parseRequest(q, dns.TypeA)
 		if e == nil {
 			t.Errorf("Expected error from %v(\"%v\", \"A\").", f, q)
 		}
@@ -126,9 +237,43 @@ func TestParseRequest(t *testing.T) {
 	}
 
 	for _, q := range invalidSRVQueries {
-		_, e = k.parseRequest(q, "SRV")
+		_, e = k.parseRequest(q, dns.TypeSRV)
 		if e == nil {
 			t.Errorf("Expected error from %v(\"%v\", \"SRV\").", f, q)
 		}
 	}
 }
+
+func TestEndpointHostname(t *testing.T) {
+	var tests = []struct {
+		ip       string
+		hostname string
+		expected string
+	}{
+		{"10.11.12.13", "", "10-11-12-13"},
+		{"10.11.12.13", "epname", "epname"},
+	}
+	for _, test := range tests {
+		result := endpointHostname(api.EndpointAddress{IP: test.ip, Hostname: test.hostname})
+		if result != test.expected {
+			t.Errorf("Expected endpoint name for (ip:%v hostname:%v) to be '%v', but got '%v'", test.ip, test.hostname, test.expected, result)
+		}
+	}
+}
+
+func TestIpFromPodName(t *testing.T) {
+	var tests = []struct {
+		ip       string
+		expected string
+	}{
+		{"10-11-12-13", "10.11.12.13"},
+		{"1-2-3-4", "1.2.3.4"},
+		{"1-2-3--A-B-C", "1:2:3::A:B:C"},
+	}
+	for _, test := range tests {
+		result := ipFromPodName(test.ip)
+		if result != test.expected {
+			t.Errorf("Expected ip for podname '%v' to be '%v', but got '%v'", test.ip, test.expected, result)
+		}
+	}
+}

middleware/kubernetes/lookup.go

@@ -12,7 +12,7 @@ import (
 )
 
 func (k Kubernetes) records(state request.Request, exact bool) ([]msg.Service, error) {
-	r, err := k.parseRequest(state.Name(), state.Type())
+	r, err := k.parseRequest(state.Name(), state.QType())
 	if err != nil {
 		return nil, err
 	}

middleware/kubernetes/ns.go

+package kubernetes
+
+import (
+	"net"
+	"strings"
+
+	"github.com/coredns/coredns/middleware/etcd/msg"
+
+	"github.com/miekg/dns"
+	"k8s.io/client-go/1.5/pkg/api"
+)
+
+const DefaultNSName = "ns.dns."
+
+var corednsRecord dns.A
+
+type InterfaceAddrser interface {
+	InterfaceAddrs() ([]net.Addr, error)
+}
+
+type InterfaceAddrs struct{}
+
+func (i InterfaceAddrs) InterfaceAddrs() ([]net.Addr, error) {
+	return net.InterfaceAddrs()
+}
+
+func (k *Kubernetes) recordsForNS(r recordRequest, svcs *[]msg.Service) error {
+	ns := k.CoreDNSRecord()
+	s := msg.Service{
+		Host: ns.A.String(),
+		Key:  msg.Path(strings.Join([]string{ns.Hdr.Name, r.zone}, "."), "coredns")}
+	*svcs = append(*svcs, s)
+	return nil
+}
+
+// DefaultNSMsg returns an msg.Service representing an A record for
+// ns.dns.[zone] -> dns service ip. This A record is needed to legitimize
+// the SOA response in middleware.NS(), which is hardcoded at ns.dns.[zone].
+func (k *Kubernetes) defaultNSMsg(r recordRequest) msg.Service {
+	ns := k.CoreDNSRecord()
+	s := msg.Service{
+		Key:  msg.Path(strings.Join([]string{DefaultNSName, r.zone}, "."), "coredns"),
+		Host: ns.A.String(),
+	}
+	return s
+}
+
+func isDefaultNS(name string, r recordRequest) bool {
+	return strings.Index(name, DefaultNSName) == 0 && strings.Index(name, r.zone) == len(DefaultNSName)
+}
+
+func (k *Kubernetes) CoreDNSRecord() dns.A {
+	var localIP net.IP
+	var svcName string
+	var svcNamespace string
+	var dnsIP net.IP
+
+	if len(corednsRecord.Hdr.Name) == 0 || corednsRecord.A == nil {
+		// get local Pod IP
+		addrs, _ := k.interfaceAddrs.InterfaceAddrs()
+
+		for _, addr := range addrs {
+			ip, _, _ := net.ParseCIDR(addr.String())
+			ip = ip.To4()
+
+			if ip == nil || ip.IsLoopback() {
+				continue
+			}
+			localIP = ip
+			break
+		}
+		// Find endpoint matching IP to get service and namespace
+		endpointsList := k.APIConn.EndpointsList()
+
+	FindEndpoint:
+		for _, ep := range endpointsList.Items {
+			for _, eps := range ep.Subsets {
+				for _, addr := range eps.Addresses {
+					if localIP.Equal(net.ParseIP(addr.IP)) {
+
+						svcNamespace = ep.ObjectMeta.Namespace
+						svcName = ep.ObjectMeta.Name
+						break FindEndpoint
+					}
+				}
+			}
+		}
+
+		if len(svcName) == 0 {
+			corednsRecord.Hdr.Name = DefaultNSName
+			corednsRecord.A = localIP
+			return corednsRecord
+		}
+		// Find service to get ClusterIP
+		serviceList := k.APIConn.ServiceList()
+	FindService:
+		for _, svc := range serviceList {
+			if svcName == svc.Name && svcNamespace == svc.Namespace {
+				if svc.Spec.ClusterIP == api.ClusterIPNone {
+					dnsIP = localIP
+				} else {
+					dnsIP = net.ParseIP(svc.Spec.ClusterIP)
+				}
+				break FindService
+			}
+		}
+		if dnsIP == nil {
+			dnsIP = localIP
+		}
+
+		corednsRecord.Hdr.Name = strings.Join([]string{svcName, svcNamespace, "svc."}, ".")
+		corednsRecord.A = dnsIP
+	}
+	return corednsRecord
+}

middleware/kubernetes/ns_test.go

+package kubernetes
+
+import "testing"
+import "net"
+
+import "github.com/coredns/coredns/middleware/etcd/msg"
+import "k8s.io/client-go/1.5/pkg/api"
+import "github.com/miekg/dns"
+
+func TestRecordForNS(t *testing.T) {
+	k := Kubernetes{Zones: []string{"inter.webs.test"}}
+	corednsRecord.Hdr.Name = "coredns.kube-system."
+	corednsRecord.A = net.IP("1.2.3.4")
+	r, _ := k.parseRequest("inter.webs.test", dns.TypeNS)
+	expected := "/coredns/test/webs/inter/kube-system/coredns"
+
+	var svcs []msg.Service
+	k.recordsForNS(r, &svcs)
+	if svcs[0].Key != expected {
+		t.Errorf("Expected  result '%v'. Instead got result '%v'.", expected, svcs[0].Key)
+	}
+}
+
+func TestDefaultNSMsg(t *testing.T) {
+	k := Kubernetes{Zones: []string{"inter.webs.test"}}
+	corednsRecord.Hdr.Name = "coredns.kube-system."
+	corednsRecord.A = net.IP("1.2.3.4")
+	r, _ := k.parseRequest("ns.dns.inter.webs.test", dns.TypeA)
+	expected := "/coredns/test/webs/inter/dns/ns"
+
+	svc := k.defaultNSMsg(r)
+	if svc.Key != expected {
+		t.Errorf("Expected  result '%v'. Instead got result '%v'.", expected, svc.Key)
+	}
+}
+
+func TestIsDefaultNS(t *testing.T) {
+	k := Kubernetes{Zones: []string{"inter.webs.test"}}
+	r, _ := k.parseRequest("ns.dns.inter.webs.test", dns.TypeA)
+
+	var name string
+	var expected bool
+
+	name = "ns.dns.inter.webs.test"
+	expected = true
+	if isDefaultNS(name, r) != expected {
+		t.Errorf("Expected IsDefaultNS('%v') to be '%v'.", name, expected)
+	}
+	name = "ns.dns.blah.inter.webs.test"
+	expected = false
+	if isDefaultNS(name, r) != expected {
+		t.Errorf("Expected IsDefaultNS('%v') to be '%v'.", name, expected)
+	}
+}
+
+type APIConnTest struct{}
+
+func (APIConnTest) Run() {
+	return
+}
+
+func (APIConnTest) Stop() error {
+	return nil
+}
+
+func (APIConnTest) ServiceList() []*api.Service {
+	svc := api.Service{
+		ObjectMeta: api.ObjectMeta{
+			Name:      "dns-service",
+			Namespace: "kube-system",
+		},
+		Spec: api.ServiceSpec{
+			ClusterIP: "10.0.0.111",
+		},
+	}
+
+	return []*api.Service{&svc}
+
+}
+
+func (APIConnTest) PodIndex(string) []interface{} {
+	return nil
+}
+
+func (APIConnTest) EndpointsList() api.EndpointsList {
+	return api.EndpointsList{
+		Items: []api.Endpoints{
+			{
+				Subsets: []api.EndpointSubset{
+					{
+						Addresses: []api.EndpointAddress{
+							{
+								IP: "172.0.40.10",
+							},
+						},
+					},
+				},
+				ObjectMeta: api.ObjectMeta{
+					Name:      "dns-service",
+					Namespace: "kube-system",
+				},
+			},
+		},
+	}
+}
+
+type InterfaceAddrsTest struct{}
+
+func (i InterfaceAddrsTest) InterfaceAddrs() ([]net.Addr, error) {
+	_, ipnet, _ := net.ParseCIDR("172.0.40.10/32")
+	return []net.Addr{ipnet}, nil
+}
+
+func TestDoCoreDNSRecord(t *testing.T) {
+
+	corednsRecord = dns.A{}
+	k := Kubernetes{Zones: []string{"inter.webs.test"}}
+
+	k.interfaceAddrs = &InterfaceAddrsTest{}
+	k.APIConn = &APIConnTest{}
+
+	cdr := k.CoreDNSRecord()
+
+	expected := "10.0.0.111"
+
+	if cdr.A.String() != expected {
+		t.Errorf("Expected A to be '%v', got '%v'", expected, cdr.A.String())
+	}
+	expected = "dns-service.kube-system.svc."
+	if cdr.Hdr.Name != expected {
+		t.Errorf("Expected Hdr.Name to be '%v', got '%v'", expected, cdr.Hdr.Name)
+	}
+}

middleware/kubernetes/setup.go

@@ -51,8 +51,11 @@ func setup(c *caddy.Controller) error {
 }
 
 func kubernetesParse(c *caddy.Controller) (*Kubernetes, error) {
-	k8s := &Kubernetes{ResyncPeriod: defaultResyncPeriod}
-	k8s.PodMode = PodModeDisabled
+	k8s := &Kubernetes{
+		ResyncPeriod:   defaultResyncPeriod,
+		interfaceAddrs: &InterfaceAddrs{},
+		PodMode:        PodModeDisabled,
+	}
 
 	for c.Next() {
 		if c.Val() == "kubernetes" {

test/kubernetes_test.go

@@ -233,6 +233,13 @@ var dnsTestCases = []test.Case{
 			test.A("next-in-chain.              0       IN      A       192.0.2.53"),
 		},
 	},
+	{
+		Qname: "cluster.local.", Qtype: dns.TypeNS,
+		Rcode: dns.RcodeSuccess,
+		Answer: []dns.RR{
+			test.NS("cluster.local.          0       IN      NS      kubernetes.default.svc.cluster.local."),
+		},
+	},
 }
 
 var dnsTestCasesPodsInsecure = []test.Case{