TrueCloudLab/coredns
16
0
Fork 2
Code Issues Pull requests 4 Projects Releases Packages Wiki Activity Actions

mw/kubernetes: Rewrite parseRequest and Readability improvements (#939)

Browse source 
* mw/kubernetes: rewrite parseRequest

Stop looking at the qtype in parseRequest and make k.Namespace a map.
Fallout from this is that pkg/strings as it is not used anymore. Also
add a few helper functions to make unexposed namespaces easier to see in
the code.

Add wildcard tests to the middleware tests.

* Fix tests

Add a whole bunch of comments to document what we are trying to do.

* This is now answered

* up coverage

* duh

* Update testcase

* Make it nodata
This commit is contained in:
Miek Gieben 2017-08-22 20:44:42 +01:00 • committed by Yong Tang
parent 60d5e71a1a
commit 6a4e69eb9f
9 changed files with 189 additions and 168 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

47
middleware/kubernetes/handler_test.go
View file

@ -19,12 +19,37 @@ 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,
"A Service (wildcard)": {
Qname: "svc1.*.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("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.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("*.any.svc1.*.svc.cluster.local. 0 IN A 10.0.0.1"),
},
},
"SRV Service Not udp/tcp": {
Qname: "*._not-udp-or-tcp.svc1.testns.svc.cluster.local.", Qtype: dns.TypeSRV,
Rcode: dns.RcodeSuccess,
Ns: []dns.RR{
test.SOA("cluster.local. 300 IN SOA ns.dns.cluster.local. hostmaster.cluster.local. 1499347823 7200 1800 86400 60"),
},
},
"SRV Service": {
@ -37,6 +62,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,
@ -49,7 +82,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,
@ -153,13 +185,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)
}

75
middleware/kubernetes/kubernetes.go
View file

@ -14,7 +14,6 @@ import (
"github.com/coredns/coredns/middleware/etcd/msg"
"github.com/coredns/coredns/middleware/pkg/dnsutil"
"github.com/coredns/coredns/middleware/pkg/healthcheck"
dnsstrings "github.com/coredns/coredns/middleware/pkg/strings"
"github.com/coredns/coredns/middleware/proxy"
"github.com/coredns/coredns/request"
@ -40,7 +39,7 @@ type Kubernetes struct {
APIClientKey string
APIConn dnsController
ResyncPeriod time.Duration
Namespaces []string
Namespaces map[string]bool
LabelSelector *unversionedapi.LabelSelector
Selector *labels.Selector
PodMode string
@ -56,7 +55,11 @@ type Kubernetes struct {
func New(zones []string) *Kubernetes {
k := new(Kubernetes)
k.Zones = zones
k.Namespaces = make(map[string]bool)
k.interfaceAddrsFunc = func() net.IP { return net.ParseIP("127.0.0.1") }
k.PodMode = PodModeDisabled
k.Proxy = proxy.Proxy{}
k.ResyncPeriod = defaultResyncPeriod
return k
}
@ -298,7 +301,7 @@ func (k *Kubernetes) Records(state request.Request, exact bool) ([]msg.Service,
return nil, e
}
if !k.namespaceExposed(r.namespace) {
if !wildcard(r.namespace) && !k.namespaceExposed(r.namespace) {
return nil, errNsNotExposed
}
@ -387,18 +390,17 @@ func (k *Kubernetes) findPods(namespace, podname string) (pods []kPod, err error
// PodModeVerified
objList := k.APIConn.PodIndex(ip)
nsWildcard := wildcard(namespace)
for _, o := range objList {
p, ok := o.(*api.Pod)
if !ok {
return nil, errAPIBadPodType
}
// If namespace has a wildcard, filter results against Corefile namespace list.
if nsWildcard && (len(k.Namespaces) > 0) && (!dnsstrings.StringInSlice(p.Namespace, k.Namespaces)) {
if wildcard(namespace) && !k.namespaceExposed(p.Namespace) {
continue
}
// check for matching ip and namespace
if ip == p.Status.PodIP && match(namespace, p.Namespace, nsWildcard) {
if ip == p.Status.PodIP && match(namespace, p.Namespace) {
s := kPod{name: podname, namespace: namespace, addr: ip}
pods = append(pods, s)
return pods, nil
@ -409,8 +411,8 @@ func (k *Kubernetes) findPods(namespace, podname string) (pods []kPod, err error
// get retrieves matching data from the cache.
func (k *Kubernetes) get(r recordRequest) (services []kService, pods []kPod, err error) {
switch {
case r.podOrSvc == Pod:
switch r.podOrSvc {
case Pod:
pods, err = k.findPods(r.namespace, r.service)
return nil, pods, err
default:
@ -423,19 +425,14 @@ func (k *Kubernetes) findServices(r recordRequest) ([]kService, error) {
serviceList := k.APIConn.ServiceList()
var resultItems []kService
nsWildcard := wildcard(r.namespace)
serviceWildcard := wildcard(r.service)
portWildcard := wildcard(r.port) || r.port == ""
protocolWildcard := wildcard(r.protocol) || r.protocol == ""
for _, svc := range serviceList {
if !(match(r.namespace, svc.Namespace, nsWildcard) && match(r.service, svc.Name, serviceWildcard)) {
if !(match(r.namespace, svc.Namespace) && match(r.service, svc.Name)) {
continue
}
// If namespace has a wildcard, filter results against Corefile namespace list.
// (Namespaces without a wildcard were filtered before the call to this function.)
if nsWildcard && (len(k.Namespaces) > 0) && (!dnsstrings.StringInSlice(svc.Namespace, k.Namespaces)) {
if wildcard(r.namespace) && !k.namespaceExposed(svc.Namespace) {
continue
}
s := kService{name: svc.Name, namespace: svc.Namespace}
@ -448,14 +445,20 @@ 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
}
if !(match(r.port, p.Name, portWildcard) && match(r.protocol, string(p.Protocol), protocolWildcard)) {
}
for _, p := range eps.Ports {
if !(match(r.port, p.Name) && match(r.protocol, string(p.Protocol))) {
continue
}
s.endpoints = append(s.endpoints, endpoint{addr: addr, port: p})
@ -479,7 +482,7 @@ func (k *Kubernetes) findServices(r recordRequest) ([]kService, error) {
// ClusterIP service
s.addr = svc.Spec.ClusterIP
for _, p := range svc.Spec.Ports {
if !(match(r.port, p.Name, portWildcard) && match(r.protocol, string(p.Protocol), protocolWildcard)) {
if !(match(r.port, p.Name) && match(r.protocol, string(p.Protocol))) {
continue
}
s.ports = append(s.ports, p)
@ -490,21 +493,30 @@ func (k *Kubernetes) findServices(r recordRequest) ([]kService, error) {
return resultItems, nil
}
func match(a, b string, wildcard bool) bool {
if wildcard {
// match checks if a and b are equal taking wildcards into account.
func match(a, b string) bool {
if wildcard(a) {
return true
}
if wildcard(b) {
return true
}
return strings.EqualFold(a, b)
}
// getServiceRecordForIP: Gets a service record with a cluster ip matching the ip argument
// wildcard checks whether s contains a wildcard value defined as "*" or "any".
func wildcard(s string) bool {
return s == "*" || s == "any"
}
// serviceRecordForIP gets a service record with a cluster ip matching the ip argument
// If a service cluster ip does not match, it checks all endpoints
func (k *Kubernetes) getServiceRecordForIP(ip, name string) []msg.Service {
func (k *Kubernetes) serviceRecordForIP(ip, name string) []msg.Service {
// First check services with cluster ips
svcList := k.APIConn.ServiceList()
for _, service := range svcList {
if (len(k.Namespaces) > 0) && !dnsstrings.StringInSlice(service.Namespace, k.Namespaces) {
if (len(k.Namespaces) > 0) && !k.namespaceExposed(service.Namespace) {
continue
}
if service.Spec.ClusterIP == ip {
@ -515,7 +527,7 @@ func (k *Kubernetes) getServiceRecordForIP(ip, name string) []msg.Service {
// If no cluster ips match, search endpoints
epList := k.APIConn.EndpointsList()
for _, ep := range epList.Items {
if (len(k.Namespaces) > 0) && !dnsstrings.StringInSlice(ep.ObjectMeta.Namespace, k.Namespaces) {
if (len(k.Namespaces) > 0) && !k.namespaceExposed(ep.ObjectMeta.Namespace) {
continue
}
for _, eps := range ep.Subsets {
@ -532,20 +544,13 @@ func (k *Kubernetes) getServiceRecordForIP(ip, name string) []msg.Service {
// namespaceExposed returns true when the namespace is exposed.
func (k *Kubernetes) namespaceExposed(namespace string) bool {
// Abort if the namespace does not contain a wildcard, and namespace is
// not published per CoreFile Case where namespace contains a wildcard
// is handled in k.get(...) method.
if (!wildcard(namespace)) && (len(k.Namespaces) > 0) && (!dnsstrings.StringInSlice(namespace, k.Namespaces)) {
_, ok := k.Namespaces[namespace]
if len(k.Namespaces) > 0 && !ok {
return false
}
return true
}
// wildcard checks whether s contains a wildcard value
func wildcard(s string) bool {
return (s == "*" || s == "any")
}
const (
// Svc is the DNS schema for kubernetes services
Svc = "svc"

16
middleware/kubernetes/kubernetes_test.go
View file

@ -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 {
t.Errorf("Test %d, expected expected 1 answer, got %v", i, len(svcs))
continue
}
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)
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("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 key '%v', got '%v'", i, test.answer.key, svcs[0].Key)
}
}
}

109
middleware/kubernetes/parse.go
View file

@ -15,78 +15,85 @@ type recordRequest struct {
// SRV record.
protocol string
endpoint string
// The servicename used in Kubernetes.
service string
// The namespace used in Kubernetes.
namespace string
// A each name can be for a pod or a service, here we track what we've seen. This value is true for
// pods and false for services. If we ever need to extend this well use a typed value.
// A each name can be for a pod or a service, here we track what we've seen, either "pod" or "service".
podOrSvc string
}
// parseRequest parses the qname to find all the elements we need for querying k8s.
// parseRequest parses the qname to find all the elements we need for querying k8s. Anything
// 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.type.zone
// o A Request (endpoint): endpoint.service.namespace.type.zone
// o A Request (service): service.namespace.type.zone
// Federations are handled in the federation middleware.
// 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. And aren't parsed here.
base, _ := dnsutil.TrimZone(state.Name(), state.Zone)
segs := dns.SplitDomainName(base)
offset := 0
if state.QType() == dns.TypeSRV {
// The kubernetes peer-finder expects queries with empty port and service to resolve
// If neither is specified, treat it as a wildcard
if len(segs) == 3 {
r.port = "*"
r.protocol = "*"
r.service = "*"
offset = 0
} else {
if len(segs) != 5 {
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
// * endpoint
// *_protocol._port
last := len(segs) - 1
r.podOrSvc = segs[last]
if r.podOrSvc != Pod && r.podOrSvc != Svc {
return r, errInvalidRequest
}
// This is a SRV style request, get first two elements as port and
// protocol, stripping leading underscores if present.
if segs[0][0] == '_' {
r.port = segs[0][1:]
} else {
r.port = segs[0]
if !wildcard(r.port) {
return r, errInvalidRequest
}
}
if segs[1][0] == '_' {
r.protocol = segs[1][1:]
if r.protocol != "tcp" && r.protocol != "udp" {
return r, errInvalidRequest
}
} else {
r.protocol = segs[1]
if !wildcard(r.protocol) {
return r, errInvalidRequest
}
}
if r.port == "" || r.protocol == "" {
return r, errInvalidRequest
}
offset = 2
}
}
if (state.QType() == dns.TypeA || state.QType() == dns.TypeAAAA) && len(segs) == 4 {
// This is an endpoint A/AAAA record request. Get first element as endpoint.
r.endpoint = segs[0]
offset = 1
last--
if last < 0 {
return r, nil
}
if len(segs) == (offset + 3) {
r.service = segs[offset]
r.namespace = segs[offset+1]
r.podOrSvc = segs[offset+2]
r.namespace = segs[last]
last--
if last < 0 {
return r, nil
}
r.service = segs[last]
last--
if last < 0 {
return r, nil
}
// 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]
case 1: // service and port
r.protocol = stripUnderscore(segs[last])
r.port = stripUnderscore(segs[last-1])
default: // too long
return r, errInvalidRequest
}
return r, nil
}
return r, errInvalidRequest
// 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.

39
middleware/kubernetes/parse_test.go
View file

@ -13,34 +13,18 @@ func TestParseRequest(t *testing.T) {
tests := []struct {
query string
qtype uint16
expected string // output from r.String()
}{
{
// valid SRV request
"_http._tcp.webs.mynamespace.svc.inter.webs.test.", dns.TypeSRV,
"http.tcp..webs.mynamespace.svc",
},
{
{"_http._tcp.webs.mynamespace.svc.inter.webs.test.", "http.tcp..webs.mynamespace.svc"},
// wildcard acceptance
"*.any.*.any.svc.inter.webs.test.", dns.TypeSRV,
"*.any..*.any.svc",
},
{
{"*.any.*.any.svc.inter.webs.test.", "*.any..*.any.svc"},
// A request of endpoint
"1-2-3-4.webs.mynamespace.svc.inter.webs.test.", dns.TypeA,
"..1-2-3-4.webs.mynamespace.svc",
},
{
// 3 segments
"webs.mynamespace.svc.inter.webs.test.", dns.TypeSRV,
"*...webs.mynamespace.svc",
},
{"1-2-3-4.webs.mynamespace.svc.inter.webs.test.", "*.*.1-2-3-4.webs.mynamespace.svc"},
}
for i, tc := range tests {
m := new(dns.Msg)
m.SetQuestion(tc.query, tc.qtype)
m.SetQuestion(tc.query, dns.TypeA)
state := request.Request{Zone: zone, Req: m}
r, e := k.parseRequest(state)
@ -57,21 +41,18 @@ func TestParseRequest(t *testing.T) {
func TestParseInvalidRequest(t *testing.T) {
k := New([]string{zone})
invalid := map[string]uint16{
"_http._tcp.webs.mynamespace.svc.inter.webs.test.": dns.TypeA, // A requests cannot have port or protocol
"_http._pcp.webs.mynamespace.svc.inter.webs.test.": dns.TypeSRV, // SRV protocol must be tcp or udp
"_http._tcp.ep.webs.ns.svc.inter.webs.test.": dns.TypeSRV, // SRV requests cannot have an endpoint
"_*._*.webs.mynamespace.svc.inter.webs.test.": dns.TypeSRV, // SRV request with invalid wildcards
invalid := []string{
"webs.mynamespace.pood.inter.webs.test.", // Request must be for pod or svc subdomain.
"too.long.for.what.I.am.trying.to.pod.inter.webs.tests.", // Too long.
}
for query, qtype := range invalid {
for i, query := range invalid {
m := new(dns.Msg)
m.SetQuestion(query, qtype)
m.SetQuestion(query, dns.TypeA)
state := request.Request{Zone: zone, Req: m}
if _, e := k.parseRequest(state); e == nil {
t.Errorf("Expected error from %s:%d, got none", query, qtype)
t.Errorf("Test %d: expected error from %s, got none", i, query)
}
}
}

2
middleware/kubernetes/reverse.go
View file

@ -15,6 +15,6 @@ func (k *Kubernetes) Reverse(state request.Request, exact bool, opt middleware.O
return nil, nil, nil
}
records := k.getServiceRecordForIP(ip, state.Name())
records := k.serviceRecordForIP(ip, state.Name())
return records, nil, nil
}

14
middleware/kubernetes/setup.go
View file

@ -59,13 +59,9 @@ func setup(c *caddy.Controller) error {
}
func kubernetesParse(c *caddy.Controller) (*Kubernetes, error) {
k8s := &Kubernetes{
ResyncPeriod: defaultResyncPeriod,
interfaceAddrsFunc: localPodIP,
PodMode: PodModeDisabled,
Proxy: proxy.Proxy{},
autoPathSearch: searchFromResolvConf(),
}
k8s := New([]string{""})
k8s.interfaceAddrsFunc = localPodIP
k8s.autoPathSearch = searchFromResolvConf()
for c.Next() {
zones := c.RemainingArgs()
@ -112,7 +108,9 @@ func kubernetesParse(c *caddy.Controller) (*Kubernetes, error) {
case "namespaces":
args := c.RemainingArgs()
if len(args) > 0 {
k8s.Namespaces = append(k8s.Namespaces, args...)
for _, a := range args {
k8s.Namespaces[a] = true
}
continue
}
return nil, c.ArgErr()

11
middleware/pkg/strings/slice.go
View file

@ -1,11 +0,0 @@
package strings
// StringInSlice check whether string a is a member of slice.
func StringInSlice(a string, slice []string) bool {
for _, b := range slice {
if b == a {
return true
}
}
return false
}

16
test/kubernetes_test.go
View file

@ -203,13 +203,21 @@ var dnsTestCases = []test.Case{
},
{
Qname: "*.svc-1-a.test-1.svc.cluster.local.", Qtype: dns.TypeSRV,
Rcode: dns.RcodeNameError,
Answer: []dns.RR{},
Rcode: dns.RcodeSuccess,
Answer: []dns.RR{
test.SRV("*.svc-1-a.test-1.svc.cluster.local. 0 IN SRV 0 50 443 172-17-0-7.svc-1-a.test-1.svc.cluster.local."),
test.SRV("*.svc-1-a.test-1.svc.cluster.local. 0 IN SRV 0 50 80 172-17-0-7.svc-1-a.test-1.svc.cluster.local."),
},
Extra: []dns.RR{
test.A("172-17-0-7.svc-1-a.test-1.svc.cluster.local. 0 IN A 172.17.0.7"),
},
},
{
Qname: "*._not-udp-or-tcp.svc-1-a.test-1.svc.cluster.local.", Qtype: dns.TypeSRV,
Rcode: dns.RcodeNameError,
Answer: []dns.RR{},
Rcode: dns.RcodeSuccess,
Ns: []dns.RR{
test.SOA("cluster.local. 300 IN SOA ns.dns.cluster.local. hostmaster.cluster.local. 1499347823 7200 1800 86400 60"),
},
},
{
Qname: "svc-1-a.test-1.svc.cluster.local.", Qtype: dns.TypeSRV,