From 338d148c7845ca4e3a44253ac8b6f040c394b8c4 Mon Sep 17 00:00:00 2001 From: Chris O'Haver Date: Fri, 23 Aug 2019 12:54:06 -0400 Subject: [PATCH] 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 --- plugin/kubernetes/external.go | 10 +-- plugin/kubernetes/kubernetes.go | 38 ++++++----- plugin/kubernetes/kubernetes_test.go | 2 +- plugin/kubernetes/local.go | 11 ++-- plugin/kubernetes/ns.go | 91 ++++++++++++++++---------- plugin/kubernetes/ns_test.go | 98 ++++++++++++++++++++++++---- 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 len(cdrs) != 3 { + t.Fatalf("Expected 3 results, got %v", len(cdrs)) - if cdr.A.String() != expected { - t.Errorf("Expected A to be %q, got %q", expected, cdr.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) + 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 = "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) } }