plugin/k8s_external: fix external nsAddrs when CoreDNS Service has no External IPs (#4891)

fix external nsAddrs; add tests;

Signed-off-by: Chris O'Haver <cohaver@infoblox.com>

plugin/kubernetes/ns.go

@@ -15,8 +15,9 @@ func isDefaultNS(name, zone string) bool {
 // it returns a record for the local address of the machine we're running on.
 func (k *Kubernetes) nsAddrs(external bool, zone string) []dns.RR {
 	var (
-		svcNames []string
+		svcNames      []string
-		svcIPs   []net.IP
+		svcIPs        []net.IP
+		foundEndpoint bool
 	)
 
 	// Find the CoreDNS Endpoints
@@ -25,6 +26,7 @@ func (k *Kubernetes) nsAddrs(external bool, zone string) []dns.RR {
 
 		// Collect IPs for all Services of the Endpoints
 		for _, endpoint := range endpoints {
+			foundEndpoint = true
 			svcs := k.APIConn.SvcIndex(endpoint.Index)
 			for _, svc := range svcs {
 				if external {
@@ -54,8 +56,8 @@ func (k *Kubernetes) nsAddrs(external bool, zone string) []dns.RR {
 		}
 	}
 
-	// If no local IPs matched any endpoints, use the localIPs directly
+	// If no CoreDNS endpoints were found, use the localIPs directly
-	if len(svcIPs) == 0 {
+	if !foundEndpoint {
 		svcIPs = make([]net.IP, len(k.localIPs))
 		svcNames = make([]string, len(k.localIPs))
 		for i, localIP := range k.localIPs {

plugin/kubernetes/ns_test.go

@@ -35,24 +35,25 @@ func (a APIConnTest) SvcIndex(s string) []*object.Service {
 	return nil
 }
 
+var svcs = []*object.Service{
+	{
+		Name:       "dns-service",
+		Namespace:  "kube-system",
+		ClusterIPs: []string{"10.0.0.111"},
+	},
+	{
+		Name:       "hdls-dns-service",
+		Namespace:  "kube-system",
+		ClusterIPs: []string{api.ClusterIPNone},
+	},
+	{
+		Name:       "dns6-service",
+		Namespace:  "kube-system",
+		ClusterIPs: []string{"10::111"},
+	},
+}
+
 func (APIConnTest) ServiceList() []*object.Service {
-	svcs := []*object.Service{
-		{
-			Name:       "dns-service",
-			Namespace:  "kube-system",
-			ClusterIPs: []string{"10.0.0.111"},
-		},
-		{
-			Name:       "hdls-dns-service",
-			Namespace:  "kube-system",
-			ClusterIPs: []string{api.ClusterIPNone},
-		},
-		{
-			Name:       "dns6-service",
-			Namespace:  "kube-system",
-			ClusterIPs: []string{"10::111"},
-		},
-	}
 	return svcs
 }
 
@@ -136,3 +137,37 @@ func TestNsAddrs(t *testing.T) {
 		t.Errorf("Expected AAAA Header Name to be %q, got %q", expected, cdr.Header().Name)
 	}
 }
+
+func TestNsAddrsExternal(t *testing.T) {
+
+	k := New([]string{"example.com."})
+	k.APIConn = &APIConnTest{}
+	k.localIPs = []net.IP{net.ParseIP("10.244.0.20")}
+
+	// initially no services have an external IP ...
+	cdrs := k.nsAddrs(true, k.Zones[0])
+
+	if len(cdrs) != 0 {
+		t.Fatalf("Expected 0 results, got %v", len(cdrs))
+
+	}
+
+	// Add an external IP to one of the services ...
+	svcs[0].ExternalIPs = []string{"1.2.3.4"}
+	cdrs = k.nsAddrs(true, k.Zones[0])
+
+	if len(cdrs) != 1 {
+		t.Fatalf("Expected 1 results, got %v", len(cdrs))
+
+	}
+	cdr := cdrs[0]
+	expected := "1.2.3.4"
+	if cdr.(*dns.A).A.String() != expected {
+		t.Errorf("Expected A address to be %q, got %q", expected, cdr.(*dns.A).A.String())
+	}
+	expected = "dns-service.kube-system.example.com."
+	if cdr.Header().Name != expected {
+		t.Errorf("Expected record name to be %q, got %q", expected, cdr.Header().Name)
+	}
+
+}