From f4f0d55dcec3b0b555a603348013177c46c063d2 Mon Sep 17 00:00:00 2001 From: Chris O'Haver Date: Mon, 30 Sep 2024 10:32:13 -0400 Subject: [PATCH] only create PTR records for endpoints with hostname defined (#6898) Signed-off-by: Chris O'Haver --- plugin/kubernetes/reverse.go | 9 +++++++-- plugin/kubernetes/reverse_test.go | 27 ++++++++++++++------------- 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/plugin/kubernetes/reverse.go b/plugin/kubernetes/reverse.go index 26fc3b429..d63754315 100644 --- a/plugin/kubernetes/reverse.go +++ b/plugin/kubernetes/reverse.go @@ -44,8 +44,13 @@ func (k *Kubernetes) serviceRecordForIP(ip, name string) []msg.Service { } for _, eps := range ep.Subsets { for _, addr := range eps.Addresses { - if addr.IP == ip { - domain := strings.Join([]string{endpointHostname(addr, k.endpointNameMode), ep.Index, Svc, k.primaryZone()}, ".") + // The endpoint's Hostname will be set if this endpoint is supposed to generate a PTR. + // So only return reverse records that match the IP AND have a non-empty hostname. + // Kubernetes more or less keeps this to one canonical service/endpoint per IP, but in the odd event there + // are multiple endpoints for the same IP with hostname set, return them all rather than selecting one + // arbitrarily. + if addr.IP == ip && addr.Hostname != "" { + domain := strings.Join([]string{addr.Hostname, ep.Index, Svc, k.primaryZone()}, ".") svcs = append(svcs, msg.Service{Host: domain, TTL: k.ttl}) } } diff --git a/plugin/kubernetes/reverse_test.go b/plugin/kubernetes/reverse_test.go index 370b9f9b5..d13337d6c 100644 --- a/plugin/kubernetes/reverse_test.go +++ b/plugin/kubernetes/reverse_test.go @@ -60,8 +60,10 @@ func (APIConnReverseTest) EpIndexReverse(ip string) []*object.Endpoints { Subsets: []object.EndpointSubset{ { Addresses: []object.EndpointAddress{ - {IP: "10.0.0.100", Hostname: "ep1a"}, - {IP: "10.0.0.99", Hostname: "double-ep"}, // this endpoint is used by two services + {IP: "10.0.0.100", Hostname: "ep1a"}, // this endpoint is duplicated across two slices, which can happen in k8s + + // 10.0.0.99 is used by two services. Only one will have hostname defined, as is typically enforced by K8s + {IP: "10.0.0.99", Hostname: "double-ep"}, }, Ports: []object.EndpointPort{ {Port: 80, Protocol: "tcp", Name: "http"}, @@ -93,7 +95,7 @@ func (APIConnReverseTest) EpIndexReverse(ip string) []*object.Endpoints { Subsets: []object.EndpointSubset{ { Addresses: []object.EndpointAddress{ - {IP: "10.0.0.100", Hostname: "ep1a"}, // duplicate endpointslice address + {IP: "10.0.0.100", Hostname: "ep1a"}, // This endpoint duplicated across used by two slices, see above }, Ports: []object.EndpointPort{ {Port: 80, Protocol: "tcp", Name: "http"}, @@ -108,7 +110,7 @@ func (APIConnReverseTest) EpIndexReverse(ip string) []*object.Endpoints { Subsets: []object.EndpointSubset{ { Addresses: []object.EndpointAddress{ - {IP: "10.0.0.99", Hostname: "double-ep"}, // this endpoint is used by two services + {IP: "10.0.0.99", Hostname: ""}, // This endpoint is used by two services, see above }, Ports: []object.EndpointPort{ {Port: 80, Protocol: "tcp", Name: "http"}, @@ -153,6 +155,13 @@ func TestReverse(t *testing.T) { k.APIConn = &APIConnReverseTest{} tests := []test.Case{ + { + Qname: "99.0.0.10.in-addr.arpa.", Qtype: dns.TypePTR, + Rcode: dns.RcodeSuccess, + Answer: []dns.RR{ + test.PTR("99.0.0.10.in-addr.arpa. 5 IN PTR double-ep.svc1.testns.svc.cluster.local."), + }, + }, { Qname: "100.0.0.10.in-addr.arpa.", Qtype: dns.TypePTR, Rcode: dns.RcodeSuccess, @@ -223,14 +232,6 @@ func TestReverse(t *testing.T) { test.SOA("cluster.local. 5 IN SOA ns.dns.cluster.local. hostmaster.cluster.local. 1502989566 7200 1800 86400 5"), }, }, - { - Qname: "99.0.0.10.in-addr.arpa.", Qtype: dns.TypePTR, - Rcode: dns.RcodeSuccess, - Answer: []dns.RR{ - test.PTR("99.0.0.10.in-addr.arpa. 5 IN PTR double-ep.svc1.testns.svc.cluster.local."), - test.PTR("99.0.0.10.in-addr.arpa. 5 IN PTR double-ep.svc2.testns.svc.cluster.local."), - }, - }, } ctx := context.TODO() @@ -250,7 +251,7 @@ func TestReverse(t *testing.T) { t.Fatalf("Test %d: got nil message and no error for: %s %d", i, r.Question[0].Name, r.Question[0].Qtype) } if err := test.SortAndCheck(resp, tc); err != nil { - t.Error(err) + t.Errorf("Test %d error: %s", i, err) } } }