plugin/k8s_external/kubernetes: handle NS records (#3160)

* fix external ns records

* use k8s service name for ns record

* update test, add func comment

* expand nsAddrs() test cases

* support local ipv6 ip

* use less confusing pod ip in test

6 files changed, 180 insertions, 70 deletions

diff --git a/plugin/kubernetes/external.go b/plugin/kubernetes/external.go
index 4b1104460..42495a430 100644
--- a/plugin/kubernetes/external.go
+++ b/plugin/kubernetes/external.go
@@ -85,8 +85,10 @@ func (k *Kubernetes) External(state request.Request) ([]msg.Service, int) {
 
 // ExternalAddress returns the external service address(es) for the CoreDNS service.
 func (k *Kubernetes) ExternalAddress(state request.Request) []dns.RR {
-	// This is probably wrong, because of all the fallback behavior of k.nsAddr, i.e. can get
-	// an address that isn't reachable from outside the cluster.
-	rrs := []dns.RR{k.nsAddr()}
-	return rrs
+	// If CoreDNS is running inside the Kubernetes cluster: k.nsAddrs() will return the external IPs of the services
+	// targeting the CoreDNS Pod.
+	// If CoreDNS is running outside of the Kubernetes cluster: k.nsAddrs() will return the first non-loopback IP
+	// address seen on the local system it is running on. This could be the wrong answer if coredns is using the *bind*
+	// plugin to bind to a different IP address.
+	return k.nsAddrs(true, state.Zone)
 }
diff --git a/plugin/kubernetes/kubernetes.go b/plugin/kubernetes/kubernetes.go
index a02c35698..e7b36917e 100644
--- a/plugin/kubernetes/kubernetes.go
+++ b/plugin/kubernetes/kubernetes.go
@@ -107,25 +107,33 @@ func (k *Kubernetes) Services(ctx context.Context, state request.Request, exact
 
 	case dns.TypeNS:
 		// We can only get here if the qname equals the zone, see ServeDNS in handler.go.
-		ns := k.nsAddr()
-		svc := msg.Service{Host: ns.A.String(), Key: msg.Path(state.QName(), coredns), TTL: k.ttl}
-		return []msg.Service{svc}, nil
+		nss := k.nsAddrs(false, state.Zone)
+		var svcs []msg.Service
+		for _, ns := range nss {
+			if ns.Header().Rrtype == dns.TypeA {
+				svcs = append(svcs, msg.Service{Host: ns.(*dns.A).A.String(), Key: msg.Path(ns.Header().Name, coredns), TTL: k.ttl})
+				continue
+			}
+			if ns.Header().Rrtype == dns.TypeAAAA {
+				svcs = append(svcs, msg.Service{Host: ns.(*dns.AAAA).AAAA.String(), Key: msg.Path(ns.Header().Name, coredns), TTL: k.ttl})
+			}
+		}
+		return svcs, nil
 	}
 
 	if isDefaultNS(state.Name(), state.Zone) {
-		ns := k.nsAddr()
-
-		isIPv4 := ns.A.To4() != nil
-
-		if !((state.QType() == dns.TypeA && isIPv4) || (state.QType() == dns.TypeAAAA && !isIPv4)) {
-			// NODATA
-			return nil, nil
+		nss := k.nsAddrs(false, state.Zone)
+		var svcs []msg.Service
+		for _, ns := range nss {
+			if ns.Header().Rrtype == dns.TypeA && state.QType() == dns.TypeA {
+				svcs = append(svcs, msg.Service{Host: ns.(*dns.A).A.String(), Key: msg.Path(state.QName(), coredns), TTL: k.ttl})
+				continue
+			}
+			if ns.Header().Rrtype == dns.TypeAAAA && state.QType() == dns.TypeAAAA {
+				svcs = append(svcs, msg.Service{Host: ns.(*dns.AAAA).AAAA.String(), Key: msg.Path(state.QName(), coredns), TTL: k.ttl})
+			}
 		}
-
-		// If this is an A request for "ns.dns", respond with a "fake" record for coredns.
-		// SOA records always use this hardcoded name
-		svc := msg.Service{Host: ns.A.String(), Key: msg.Path(state.QName(), coredns), TTL: k.ttl}
-		return []msg.Service{svc}, nil
+		return svcs, nil
 	}
 
 	s, e := k.Records(ctx, state, false)
diff --git a/plugin/kubernetes/kubernetes_test.go b/plugin/kubernetes/kubernetes_test.go
index 57b735d7b..4fd8e22a6 100644
--- a/plugin/kubernetes/kubernetes_test.go
+++ b/plugin/kubernetes/kubernetes_test.go
@@ -327,7 +327,7 @@ func TestServicesAuthority(t *testing.T) {
 
 		state := request.Request{
 			Req:  &dns.Msg{Question: []dns.Question{{Name: test.qname, Qtype: test.qtype}}},
-			Zone: "interwebs.test.", // must match from k.Zones[0]
+			Zone: k.Zones[0],
 		}
 		svcs, e := k.Services(context.TODO(), state, false, plugin.Options{})
 		if e != nil {
diff --git a/plugin/kubernetes/local.go b/plugin/kubernetes/local.go
index c5918a306..199af6f0d 100644
--- a/plugin/kubernetes/local.go
+++ b/plugin/kubernetes/local.go
@@ -12,11 +12,14 @@ func localPodIP() net.IP {
 
 	for _, addr := range addrs {
 		ip, _, _ := net.ParseCIDR(addr.String())
-		ip = ip.To4()
-		if ip == nil || ip.IsLoopback() {
-			continue
+		ip4 := ip.To4()
+		if ip4 != nil && !ip4.IsLoopback() {
+			return ip4
+		}
+		ip6 := ip.To16()
+		if ip6 != nil && !ip6.IsLoopback() {
+			return ip6
 		}
-		return ip
 	}
 	return nil
 }
diff --git a/plugin/kubernetes/ns.go b/plugin/kubernetes/ns.go
index f3d33ee22..6d18c0b77 100644
--- a/plugin/kubernetes/ns.go
+++ b/plugin/kubernetes/ns.go
@@ -4,6 +4,7 @@ import (
 	"net"
 	"strings"
 
+	"github.com/coredns/coredns/plugin/kubernetes/object"
 	"github.com/miekg/dns"
 	api "k8s.io/api/core/v1"
 )
@@ -12,54 +13,74 @@ func isDefaultNS(name, zone string) bool {
 	return strings.Index(name, defaultNSName) == 0 && strings.Index(name, zone) == len(defaultNSName)
 }
 
-// nsAddr return the A record for the CoreDNS service in the cluster. If it fails that it fallsback
-// on the local address of the machine we're running on.
-//
-// This function is rather expensive to run.
-func (k *Kubernetes) nsAddr() *dns.A {
+// nsAddrs returns the A or AAAA records for the CoreDNS service in the cluster. If the service cannot be found,
+// it returns a record for the the local address of the machine we're running on.
+func (k *Kubernetes) nsAddrs(external bool, zone string) []dns.RR {
 	var (
-		svcName      string
-		svcNamespace string
+		svcNames []string
+		svcIPs   []net.IP
 	)
 
-	rr := new(dns.A)
+	// Find the CoreDNS Endpoint
 	localIP := k.interfaceAddrsFunc()
-	rr.A = localIP
+	endpoints := k.APIConn.EpIndexReverse(localIP.String())
 
-FindEndpoint:
-	for _, ep := range k.APIConn.EpIndexReverse(localIP.String()) {
-		for _, eps := range ep.Subsets {
-			for _, addr := range eps.Addresses {
-				if localIP.Equal(net.ParseIP(addr.IP)) {
-					svcNamespace = ep.Namespace
-					svcName = ep.Name
-					break FindEndpoint
+	// If the CoreDNS Endpoint is not found, use the locally bound IP address
+	if len(endpoints) == 0 {
+		svcNames = []string{defaultNSName + zone}
+		svcIPs = []net.IP{localIP}
+	} else {
+		// Collect IPs for all Services of the Endpoints
+		for _, endpoint := range endpoints {
+			svcs := k.APIConn.SvcIndex(object.ServiceKey(endpoint.Name, endpoint.Namespace))
+			for _, svc := range svcs {
+				if external {
+					svcName := strings.Join([]string{svc.Name, svc.Namespace, zone}, ".")
+					for _, exIP := range svc.ExternalIPs {
+						svcNames = append(svcNames, svcName)
+						svcIPs = append(svcIPs, net.ParseIP(exIP))
+					}
+					continue
+				}
+				svcName := strings.Join([]string{svc.Name, svc.Namespace, Svc, zone}, ".")
+				if svc.ClusterIP == api.ClusterIPNone {
+					// For a headless service, use the endpoints IPs
+					for _, s := range endpoint.Subsets {
+						for _, a := range s.Addresses {
+							svcNames = append(svcNames, endpointHostname(a, k.endpointNameMode)+"."+svcName)
+							svcIPs = append(svcIPs, net.ParseIP(a.IP))
+						}
+					}
+				} else {
+					svcNames = append(svcNames, svcName)
+					svcIPs = append(svcIPs, net.ParseIP(svc.ClusterIP))
 				}
 			}
 		}
 	}
 
-	if len(svcName) == 0 {
-		rr.Hdr.Name = defaultNSName
-		rr.A = localIP
-		return rr
-	}
-
-FindService:
-	for _, svc := range k.APIConn.ServiceList() {
-		if svcName == svc.Name && svcNamespace == svc.Namespace {
-			if svc.ClusterIP == api.ClusterIPNone {
-				rr.A = localIP
-			} else {
-				rr.A = net.ParseIP(svc.ClusterIP)
-			}
-			break FindService
+	// Create an RR slice of collected IPs
+	var rrs []dns.RR
+	rrs = make([]dns.RR, len(svcIPs))
+	for i, ip := range svcIPs {
+		if ip.To4() == nil {
+			rr := new(dns.AAAA)
+			rr.Hdr.Class = dns.ClassINET
+			rr.Hdr.Rrtype = dns.TypeAAAA
+			rr.Hdr.Name = svcNames[i]
+			rr.AAAA = ip
+			rrs[i] = rr
+			continue
 		}
+		rr := new(dns.A)
+		rr.Hdr.Class = dns.ClassINET
+		rr.Hdr.Rrtype = dns.TypeA
+		rr.Hdr.Name = svcNames[i]
+		rr.A = ip
+		rrs[i] = rr
 	}
 
-	rr.Hdr.Name = strings.Join([]string{svcName, svcNamespace, "svc."}, ".")
-
-	return rr
+	return rrs
 }
 
 const defaultNSName = "ns.dns."
diff --git a/plugin/kubernetes/ns_test.go b/plugin/kubernetes/ns_test.go
index df7cf5f83..af057c254 100644
--- a/plugin/kubernetes/ns_test.go
+++ b/plugin/kubernetes/ns_test.go
@@ -1,9 +1,11 @@
 package kubernetes
 
 import (
+	"net"
 	"testing"
 
 	"github.com/coredns/coredns/plugin/kubernetes/object"
+	"github.com/miekg/dns"
 
 	api "k8s.io/api/core/v1"
 )
@@ -14,12 +16,23 @@ func (APIConnTest) HasSynced() bool                          { return true }
 func (APIConnTest) Run()                                     { return }
 func (APIConnTest) Stop() error                              { return nil }
 func (APIConnTest) PodIndex(string) []*object.Pod            { return nil }
-func (APIConnTest) SvcIndex(string) []*object.Service        { return nil }
 func (APIConnTest) SvcIndexReverse(string) []*object.Service { return nil }
 func (APIConnTest) EpIndex(string) []*object.Endpoints       { return nil }
 func (APIConnTest) EndpointsList() []*object.Endpoints       { return nil }
 func (APIConnTest) Modified() int64                          { return 0 }
 
+func (a APIConnTest) SvcIndex(s string) []*object.Service {
+	switch s {
+	case "dns-service.kube-system":
+		return []*object.Service{a.ServiceList()[0]}
+	case "hdls-dns-service.kube-system":
+		return []*object.Service{a.ServiceList()[1]}
+	case "dns6-service.kube-system":
+		return []*object.Service{a.ServiceList()[2]}
+	}
+	return nil
+}
+
 func (APIConnTest) ServiceList() []*object.Service {
 	svcs := []*object.Service{
 		{
@@ -27,18 +40,31 @@ func (APIConnTest) ServiceList() []*object.Service {
 			Namespace: "kube-system",
 			ClusterIP: "10.0.0.111",
 		},
+		{
+			Name:      "hdls-dns-service",
+			Namespace: "kube-system",
+			ClusterIP: api.ClusterIPNone,
+		},
+		{
+			Name:      "dns6-service",
+			Namespace: "kube-system",
+			ClusterIP: "10::111",
+		},
 	}
 	return svcs
 }
 
-func (APIConnTest) EpIndexReverse(string) []*object.Endpoints {
+func (APIConnTest) EpIndexReverse(ip string) []*object.Endpoints {
+	if ip != "10.244.0.20" {
+		return nil
+	}
 	eps := []*object.Endpoints{
 		{
 			Subsets: []object.EndpointSubset{
 				{
 					Addresses: []object.EndpointAddress{
 						{
-							IP: "127.0.0.1",
+							IP: "10.244.0.20",
 						},
 					},
 				},
@@ -46,6 +72,32 @@ func (APIConnTest) EpIndexReverse(string) []*object.Endpoints {
 			Name:      "dns-service",
 			Namespace: "kube-system",
 		},
+		{
+			Subsets: []object.EndpointSubset{
+				{
+					Addresses: []object.EndpointAddress{
+						{
+							IP: "10.244.0.20",
+						},
+					},
+				},
+			},
+			Name:      "hdls-dns-service",
+			Namespace: "kube-system",
+		},
+		{
+			Subsets: []object.EndpointSubset{
+				{
+					Addresses: []object.EndpointAddress{
+						{
+							IP: "10.244.0.20",
+						},
+					},
+				},
+			},
+			Name:      "dns6-service",
+			Namespace: "kube-system",
+		},
 	}
 	return eps
 }
@@ -55,19 +107,43 @@ func (APIConnTest) GetNamespaceByName(name string) (*api.Namespace, error) {
 	return &api.Namespace{}, nil
 }
 
-func TestNsAddr(t *testing.T) {
+func TestNsAddrs(t *testing.T) {
 
 	k := New([]string{"inter.webs.test."})
 	k.APIConn = &APIConnTest{}
+	k.interfaceAddrsFunc = func() net.IP { return net.ParseIP("10.244.0.20") }
 
-	cdr := k.nsAddr()
-	expected := "10.0.0.111"
+	cdrs := k.nsAddrs(false, k.Zones[0])
 
-	if cdr.A.String() != expected {
-		t.Errorf("Expected A to be %q, got %q", expected, cdr.A.String())
+	if len(cdrs) != 3 {
+		t.Fatalf("Expected 3 results, got %v", len(cdrs))
+
+	}
+	cdr := cdrs[0]
+	expected := "10.0.0.111"
+	if cdr.(*dns.A).A.String() != expected {
+		t.Errorf("Expected 1st A to be %q, got %q", expected, cdr.(*dns.A).A.String())
+	}
+	expected = "dns-service.kube-system.svc.inter.webs.test."
+	if cdr.Header().Name != expected {
+		t.Errorf("Expected 1st Header Name to be %q, got %q", expected, cdr.Header().Name)
+	}
+	cdr = cdrs[1]
+	expected = "10.244.0.20"
+	if cdr.(*dns.A).A.String() != expected {
+		t.Errorf("Expected 2nd A to be %q, got %q", expected, cdr.(*dns.A).A.String())
+	}
+	expected = "10-244-0-20.hdls-dns-service.kube-system.svc.inter.webs.test."
+	if cdr.Header().Name != expected {
+		t.Errorf("Expected 2nd Header Name to be %q, got %q", expected, cdr.Header().Name)
+	}
+	cdr = cdrs[2]
+	expected = "10::111"
+	if cdr.(*dns.AAAA).AAAA.String() != expected {
+		t.Errorf("Expected AAAA to be %q, got %q", expected, cdr.(*dns.A).A.String())
 	}
-	expected = "dns-service.kube-system.svc."
-	if cdr.Hdr.Name != expected {
-		t.Errorf("Expected Hdr.Name to be %q, got %q", expected, cdr.Hdr.Name)
+	expected = "dns6-service.kube-system.svc.inter.webs.test."
+	if cdr.Header().Name != expected {
+		t.Errorf("Expected AAAA Header Name to be %q, got %q", expected, cdr.Header().Name)
 	}
 }