diff --git a/middleware/kubernetes/handler_test.go b/middleware/kubernetes/handler_test.go index 1cdc8b619..c01b17bcb 100644 --- a/middleware/kubernetes/handler_test.go +++ b/middleware/kubernetes/handler_test.go @@ -12,7 +12,6 @@ import ( ) var dnsTestCases = map[string](test.Case){ - // *.any.svc-1-a.*.svc.cluster.local., "A Service": { Qname: "svc1.testns.svc.cluster.local.", Qtype: dns.TypeA, Rcode: dns.RcodeSuccess, @@ -24,29 +23,26 @@ var dnsTestCases = map[string](test.Case){ Qname: "svc1.*.svc.cluster.local.", Qtype: dns.TypeA, Rcode: dns.RcodeSuccess, Answer: []dns.RR{ - test.A("svc1.testns.svc.cluster.local. 0 IN A 10.0.0.1"), + test.A("svc1.*.svc.cluster.local. 0 IN A 10.0.0.1"), }, }, + "SRV Service (wildcard)": { + Qname: "svc1.*.svc.cluster.local.", Qtype: dns.TypeSRV, + Rcode: dns.RcodeSuccess, + Answer: []dns.RR{test.SRV("svc1.*.svc.cluster.local. 0 IN SRV 0 100 80 svc1.testns.svc.cluster.local.")}, + Extra: []dns.RR{test.A("svc1.testns.svc.cluster.local. 0 IN A 10.0.0.1")}, + }, "SRV Service (wildcards)": { Qname: "*.any.svc1.*.svc.cluster.local.", Qtype: dns.TypeSRV, Rcode: dns.RcodeSuccess, - Answer: []dns.RR{ - test.A("svc1.testns.svc.cluster.local. 0 IN A 10.0.0.1"), - }, + Answer: []dns.RR{test.SRV("*.any.svc1.*.svc.cluster.local. 0 IN SRV 0 100 80 svc1.testns.svc.cluster.local.")}, + Extra: []dns.RR{test.A("svc1.testns.svc.cluster.local. 0 IN A 10.0.0.1")}, }, "A Service (wildcards)": { Qname: "*.any.svc1.*.svc.cluster.local.", Qtype: dns.TypeA, Rcode: dns.RcodeSuccess, Answer: []dns.RR{ - test.A("svc1.testns.svc.cluster.local. 0 IN A 10.0.0.1"), - }, - }, - "A Service (Headless)": { - Qname: "hdls1.testns.svc.cluster.local.", Qtype: dns.TypeA, - Rcode: dns.RcodeSuccess, - Answer: []dns.RR{ - test.A("hdls1.testns.svc.cluster.local. 0 IN A 172.0.0.2"), - test.A("hdls1.testns.svc.cluster.local. 0 IN A 172.0.0.3"), + test.A("*.any.svc1.*.svc.cluster.local. 0 IN A 10.0.0.1"), }, }, "SRV Service": { @@ -59,6 +55,14 @@ var dnsTestCases = map[string](test.Case){ test.A("svc1.testns.svc.cluster.local. 0 IN A 10.0.0.1"), }, }, + "A Service (Headless)": { + Qname: "hdls1.testns.svc.cluster.local.", Qtype: dns.TypeA, + Rcode: dns.RcodeSuccess, + Answer: []dns.RR{ + test.A("hdls1.testns.svc.cluster.local. 0 IN A 172.0.0.2"), + test.A("hdls1.testns.svc.cluster.local. 0 IN A 172.0.0.3"), + }, + }, "SRV Service (Headless)": { Qname: "_http._tcp.hdls1.testns.svc.cluster.local.", Qtype: dns.TypeSRV, Rcode: dns.RcodeSuccess, @@ -71,7 +75,6 @@ var dnsTestCases = map[string](test.Case){ test.A("172-0-0-3.hdls1.testns.svc.cluster.local. 0 IN A 172.0.0.3"), }, }, - // TODO A External "CNAME External": { Qname: "external.testns.svc.cluster.local.", Qtype: dns.TypeCNAME, Rcode: dns.RcodeSuccess, @@ -175,13 +178,12 @@ func TestServeDNS(t *testing.T) { ctx := context.TODO() runServeDNSTests(ctx, t, dnsTestCases, k) - //Set PodMode to Disabled k.PodMode = PodModeDisabled runServeDNSTests(ctx, t, podModeDisabledCases, k) - //Set PodMode to Insecure + k.PodMode = PodModeInsecure runServeDNSTests(ctx, t, podModeInsecureCases, k) - //Set PodMode to Verified + k.PodMode = PodModeVerified runServeDNSTests(ctx, t, podModeVerifiedCases, k) } diff --git a/middleware/kubernetes/kubernetes.go b/middleware/kubernetes/kubernetes.go index 3ac4293a5..e70a87ba2 100644 --- a/middleware/kubernetes/kubernetes.go +++ b/middleware/kubernetes/kubernetes.go @@ -450,13 +450,19 @@ func (k *Kubernetes) findServices(r recordRequest) ([]kService, error) { if ep.ObjectMeta.Name != svc.Name || ep.ObjectMeta.Namespace != svc.Namespace { continue } + for _, eps := range ep.Subsets { for _, addr := range eps.Addresses { - for _, p := range eps.Ports { - ephostname := endpointHostname(addr) - if r.endpoint != "" && r.endpoint != ephostname { + + // See comments in parse.go parseRequest about the endpoint handling. + + if r.endpoint != "" { + if !match(r.endpoint, endpointHostname(addr)) { continue } + } + + for _, p := range eps.Ports { if !(match(r.port, p.Name) && match(r.protocol, string(p.Protocol))) { continue } diff --git a/middleware/kubernetes/kubernetes_test.go b/middleware/kubernetes/kubernetes_test.go index 7caaa25de..3e6d78f7c 100644 --- a/middleware/kubernetes/kubernetes_test.go +++ b/middleware/kubernetes/kubernetes_test.go @@ -217,24 +217,26 @@ func TestServices(t *testing.T) { {qname: "external.testns.svc.interwebs.test.", qtype: dns.TypeCNAME, answer: svcAns{host: "coredns.io", key: "/coredns/test/interwebs/svc/testns/external"}}, } - for _, test := range tests { + for i, test := range tests { state := request.Request{ Req: &dns.Msg{Question: []dns.Question{{Name: test.qname, Qtype: test.qtype}}}, Zone: "interwebs.test.", // must match from k.Zones[0] } svcs, _, e := k.Services(state, false, middleware.Options{}) if e != nil { - t.Errorf("Query '%v' got error '%v'", test.qname, e) + t.Errorf("Test %d: got error '%v'", i, e) + continue } if len(svcs) != 1 { - t.Errorf("Query %v %v: expected expected 1 answer, got %v", test.qname, dns.TypeToString[test.qtype], len(svcs)) - } else { - if test.answer.host != svcs[0].Host { - t.Errorf("Query %v %v: expected host '%v', got '%v'", test.qname, dns.TypeToString[test.qtype], test.answer.host, svcs[0].Host) - } - if test.answer.key != svcs[0].Key { - t.Errorf("Query %v %v: expected key '%v', got '%v'", test.qname, dns.TypeToString[test.qtype], test.answer.key, svcs[0].Key) - } + t.Errorf("Test %d, expected expected 1 answer, got %v", i, len(svcs)) + continue + } + + if test.answer.host != svcs[0].Host { + t.Errorf("Test %d, expected host '%v', got '%v'", i, test.answer.host, svcs[0].Host) + } + if test.answer.key != svcs[0].Key { + t.Errorf("Test %d, expected key '%v', got '%v'", i, test.answer.key, svcs[0].Key) } } } diff --git a/middleware/kubernetes/parse.go b/middleware/kubernetes/parse.go index c0b5de680..4b09b15f9 100644 --- a/middleware/kubernetes/parse.go +++ b/middleware/kubernetes/parse.go @@ -24,15 +24,15 @@ type recordRequest struct { } // parseRequest parses the qname to find all the elements we need for querying k8s. Anything -// that is not parsed will have the wildcard "*" value. Potential underscores are stripped -// from _port and _protocol. +// that is not parsed will have the wildcard "*" value (except r.endpoint). +// Potential underscores are stripped from _port and _protocol. func (k *Kubernetes) parseRequest(state request.Request) (r recordRequest, err error) { // 3 Possible cases: - // o SRV Request: _port._protocol.service.namespace.pod|svc.zone - // o A Request (endpoint): endpoint.service.namespace.pod|svc.zone - // o A Request (service): service.namespace.pod|svc.zone + // 1. _port._protocol.service.namespace.pod|svc.zone + // 2. (endpoint): endpoint.service.namespace.pod|svc.zone + // 3. (service): service.namespace.pod|svc.zone // - // Federations are handled in the federation middleware. + // Federations are handled in the federation middleware. And aren't parsed here. base, _ := dnsutil.TrimZone(state.Name(), state.Zone) segs := dns.SplitDomainName(base) @@ -40,8 +40,9 @@ func (k *Kubernetes) parseRequest(state request.Request) (r recordRequest, err e r.port = "*" r.protocol = "*" r.service = "*" - r.endpoint = "" // TODO(miek): dangerous; should just work with "*", but "" is checked in k.get() r.namespace = "*" + // r.endpoint is the odd one out, we need to know if it has been set or not. If it is + // empty we should skip the endpoint check in k.get(). Hence we cannot set if to "*". // start at the right and fill out recordRequest with the bits we find, so we look for // pod|svc.namespace.service and then either @@ -70,27 +71,31 @@ func (k *Kubernetes) parseRequest(state request.Request) (r recordRequest, err e return r, nil } - if segs[last][0] == '_' { - r.protocol = segs[last][1:] - } else { + // Becuase of ambiquity we check the labels left: 1: an endpoint. 2: port and protocol. + // Anything else is a query that is too long to answer and can safely be delegated to return an nxdomain. + switch last { + + case 0: // endpoint only r.endpoint = segs[last] - } - last-- - if last < 0 { - return r, nil - } + case 1: // service and port + r.protocol = stripUnderscore(segs[last]) + r.port = stripUnderscore(segs[last-1]) - if segs[last][0] == '_' { - r.port = segs[last][1:] - } - - if last > 0 { // Too long, so NXDOMAIN these. + default: // too long return r, errInvalidRequest - } + return r, nil } +// stripUnderscore removes a prefixed underscore from s. +func stripUnderscore(s string) string { + if s[0] != '_' { + return s + } + return s[1:] +} + // String return a string representation of r, it just returns all fields concatenated with dots. // This is mostly used in tests. func (r recordRequest) String() string { diff --git a/middleware/kubernetes/parse_test.go b/middleware/kubernetes/parse_test.go index 720b831b2..b08d31147 100644 --- a/middleware/kubernetes/parse_test.go +++ b/middleware/kubernetes/parse_test.go @@ -24,7 +24,7 @@ func TestParseRequest(t *testing.T) { { // wildcard acceptance "*.any.*.any.svc.inter.webs.test.", dns.TypeSRV, - "*.*.any.*.any.svc", + "*.any..*.any.svc", }, { // A request of endpoint