From c2ca1b2c23723c2b51582dad8c61f3d3849d0f64 Mon Sep 17 00:00:00 2001 From: rpb-ant Date: Mon, 10 Aug 2026 16:24:50 -0400 Subject: [PATCH] plugin/kubernetes: Add support for topology-aware headless services via "az-pinned" subdomains (#8388) --- plugin/kubernetes/README.md | 87 +++++++++ plugin/kubernetes/controller.go | 18 +- plugin/kubernetes/kubernetes.go | 81 +++++++-- plugin/kubernetes/metadata.go | 8 +- plugin/kubernetes/object/endpoint.go | 31 ++++ plugin/kubernetes/parse.go | 50 ++++- plugin/kubernetes/parse_test.go | 57 ++++-- plugin/kubernetes/setup.go | 12 ++ plugin/kubernetes/setup_test.go | 56 ++++++ plugin/kubernetes/zonal_test.go | 263 +++++++++++++++++++++++++++ 10 files changed, 624 insertions(+), 39 deletions(-) create mode 100644 plugin/kubernetes/zonal_test.go diff --git a/plugin/kubernetes/README.md b/plugin/kubernetes/README.md index b20b36ffc..70913ae0b 100644 --- a/plugin/kubernetes/README.md +++ b/plugin/kubernetes/README.md @@ -46,6 +46,7 @@ kubernetes [ZONES...] { fallthrough [ZONES...] ignore empty_service multicluster [ZONES...] + zonal startup_timeout DURATION } ``` @@ -118,11 +119,97 @@ kubernetes [ZONES...] { Services API (MCS-API). Specifying this option is generally paired with the installation of an MCS-API implementation and the ServiceImport and ServiceExport CRDs. The plugin MUST be authoritative for the zones listed here. +* `zonal` enables zone-scoped names for headless services (see the Zonal + Names section below). It also publishes the `kubernetes/zone` metadata + label (the requested topology zone, empty for non-zonal queries) when the + *metadata* plugin is enabled. * `startup_timeout` specifies the **DURATION** value that limits the time to wait for informer cache synced when the kubernetes plugin starts. If not specified, the default timeout will be 5s. Enabling zone transfer is done by using the *transfer* plugin. +## Zonal Names + +With the `zonal` option, headless services additionally answer zone-scoped +forms of their name: + +~~~ +topology-zone.pin._zone.service.namespace.svc.zone +topology-zone.prefer._zone.service.namespace.svc.zone +~~~ + +e.g. `us-west-2a.pin._zone.db.prod.svc.cluster.local` returns only the +`db` endpoints whose EndpointSlice `zone` field is `us-west-2a`. The zone +value is every label left of the directive, joined, since Kubernetes zone +label values may themselves contain dots +(`corp.example.com.pin._zone.db.prod.svc.cluster.local` selects the zone +`corp.example.com`). Headless +services have no ClusterIP for kube-proxy's `trafficDistribution` to act +on — every client receives every address — so the zone selector in the +query name lets a client scope an answer to its own zone. Plain service +names are not affected in any way, and short relative names still work +from pods (`us-west-2a.pin._zone.db` completes via the first search list +entry in the same namespace). + +The directive label chooses the fallback semantics, so a client states in +the name whether an empty zone is an error or a shrug: + +* `pin` — zone-local endpoints only. A zone label no endpoint of the + service carries (a drained zone and a mistyped one alike) answers + NODATA: "no endpoints carry that zone" is true either way, the answer + is identical on every replica, and resolution still fails visibly. +* `prefer` — zone-local endpoints if there are any, otherwise every + endpoint of the service. One query, no client-side fallback logic; + the widening is chosen in the name, never applied silently to a pin. + +Both directives answer A/AAAA and SRV (filtering happens at endpoint +selection, so SRV records and their glue are zone-filtered too), answer +NODATA for other query types, and are answered identically by every +replica. A nonexistent service is NXDOMAIN as ever; ClusterIP and +ExternalName services are NXDOMAIN — zone-scoped names are defined for +headless services only; use `trafficDistribution` for VIP topology. +Unknown directives keep the stock too-long NXDOMAIN, as does the entire +shape when the option is off. Zonal names are not defined inside +`multicluster` zones. Endpoints whose EndpointSlices carry no zone are +never matched by any zone selector. + +Only names of existing headless services answer at all, so the grammar +adds no capture surface beyond the one service creation itself has always +had: a relative name shaped `x.pin._zone.` +stops a resolver search walk with NODATA, exactly as creating a service +captures colliding relative names today. + +Relationship to [Topology Aware +Routing](https://kubernetes.io/docs/concepts/services-networking/topology-aware-routing/): +`pin` and `prefer` are a topology *addressing* primitive, not an +extension of `trafficDistribution`. They select on the endpoint's +physical topology zone (`Endpoint.Zone`), which the EndpointSlice +controller publishes without any Service-side opt-in — not on the routing +hints +(`Endpoint.Hints.ForZones`), which exist only when a Service opts in via +`trafficDistribution` or the legacy `service.kubernetes.io/topology-mode: +Auto` annotation, and which encode the zone tier of a routing decision +rather than placement (under `Auto` an endpoint can be hinted for a zone +it is not in, and the controller withdraws hints entirely when its +safeguards trip). A client naming a zone under these directives gets the +endpoints that are actually there. A hints-consuming selector is a +distinct primitive with distinct semantics (kube-proxy ignores hints +entirely for unhinted, partially-hinted, and safeguard-withdrawn +services); if one is added, it takes its own directive label in this +grammar. Unknown directives answer the stock too-long NXDOMAIN today, so +that addition is compatible and nothing here forecloses it. + +The option requires the endpoint cache: combining `zonal` with +`noendpoints` is a configuration error, since zone-scoped answers come +from endpoint data and the `noendpoints` contract (NXDOMAIN for all +headless queries) could not hold for them. + +Deployment notes: enable the option on every replica behind a shared +Service before pointing clients at `_zone` names — replicas without the +option answer NXDOMAIN for them, which clients negative-cache per name for +the SOA minttl (this follows the `ttl` option). Zonal names are answered +at query time only; they are not included in zone transfers. + ## Startup When CoreDNS starts with the *kubernetes* plugin enabled, it will delay serving DNS for up to 5 seconds diff --git a/plugin/kubernetes/controller.go b/plugin/kubernetes/controller.go index 4a4ffa5f0..31b6c937f 100644 --- a/plugin/kubernetes/controller.go +++ b/plugin/kubernetes/controller.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "maps" "sync" "sync/atomic" "time" @@ -114,6 +115,10 @@ type dnsControlOpts struct { initPodCache bool initEndpointsCache bool ignoreEmptyService bool + // zonal enables the zone-scoped name grammar + // (topozone.pin|prefer._zone.service.namespace.svc.zone) for headless + // services. + zonal bool // Label handling. labelSelector *meta.LabelSelector @@ -171,6 +176,10 @@ func newdnsController(ctx context.Context, kubeClient kubernetes.Interface, mcsC dns.podController = podController } + epTransform := object.EndpointSliceToEndpoints + if opts.zonal { + epTransform = object.EndpointSliceToEndpointsWithZones + } epLister, epController := object.NewIndexerInformer( cache.ToListWatcherWithWatchListSemantics( &cache.ListWatch{ @@ -182,7 +191,7 @@ func newdnsController(ctx context.Context, kubeClient kubernetes.Interface, mcsC &discovery.EndpointSlice{}, cache.ResourceEventHandlerFuncs{AddFunc: dns.Add, UpdateFunc: dns.Update, DeleteFunc: dns.Delete}, cache.Indexers{epNameNamespaceIndex: epNameNamespaceIndexFunc, epIPIndex: epIPIndexFunc}, - object.DefaultProcessor(object.EndpointSliceToEndpoints, dns.EndpointSliceLatencyRecorder()), + object.DefaultProcessor(epTransform, dns.EndpointSliceLatencyRecorder()), ) dns.epLister = epLister if opts.initEndpointsCache { @@ -781,6 +790,13 @@ func endpointsEquivalent(a, b *object.Endpoints) bool { return false } + // Zones is nil unless the zonal option is on, so this is a no-op for + // default configurations — and with the option on, zone changes alter + // served answers and must bump the serial like any other change. + if !maps.Equal(a.Zones, b.Zones) { + return false + } + // we should be able to rely on // these being sorted and able to be compared // they are supposed to be in a canonical format diff --git a/plugin/kubernetes/kubernetes.go b/plugin/kubernetes/kubernetes.go index 9d9c4ae02..940c0c3d8 100644 --- a/plugin/kubernetes/kubernetes.go +++ b/plugin/kubernetes/kubernetes.go @@ -112,13 +112,27 @@ func (k *Kubernetes) Services(ctx context.Context, state request.Request, _exact } // Check if we have an existing record for this query of another type - services, _ := k.Records(ctx, state, false) + services, err := k.Records(ctx, state, false) if len(services) > 0 { // If so we return an empty NOERROR return nil, nil } + // A zonal name in its exists-but-empty state answers NODATA for + // every query type. NXDOMAIN here is per-name (RFC 2308/8020): + // clients that pair query types (HTTPS+A) re-poison the name's + // address lookups on every cycle regardless of TTL, and resolvers + // cache the denial for the SOA minttl — which follows the ttl + // option, not a fixed small constant. Names findServices rejected + // (unknown service, non-headless) carry errNoItems and stay + // NXDOMAIN. + if err == nil && k.opts.zonal { + if r, e := parseRequest(state.Name(), state.Zone, k.isMultiClusterZone(state.Zone), true); e == nil && r.zone != "" { + return nil, nil + } + } + // Return NXDOMAIN for no match return nil, errNoItems @@ -331,7 +345,7 @@ func (k *Kubernetes) InitKubeCache(ctx context.Context) (onStart func() error, o // Records looks up services in kubernetes. func (k *Kubernetes) Records(_ctx context.Context, state request.Request, _exact bool) ([]msg.Service, error) { multicluster := k.isMultiClusterZone(state.Zone) - r, e := parseRequest(state.Name(), state.Zone, multicluster) + r, e := parseRequest(state.Name(), state.Zone, multicluster, k.opts.zonal) if e != nil { return nil, e } @@ -490,6 +504,13 @@ func (k *Kubernetes) findServices(r recordRequest, zone string) (services []msg. } } + // Zone-scoped names are defined for headless services only: a + // ClusterIP's VIP has no zone, and answering it under a pinned name + // would silently discard the pin. NXDOMAIN, as before the option. + if r.zone != "" && !svc.Headless() { + continue + } + // External service if svc.Type == api.ServiceTypeExternalName { // External services do not have endpoints, nor can we accept port/protocol pseudo subdomains in an SRV query, so skip this service if endpoint, port, or protocol is non-empty in the request @@ -508,37 +529,57 @@ func (k *Kubernetes) findServices(r recordRequest, zone string) (services []msg. // Endpoint query or headless service if svc.Headless() || r.endpoint != "" { + if r.zone != "" { + // The name exists (headless service, any zone label): an + // empty result set is NODATA, not NXDOMAIN — "no endpoints + // carry that zone" is true for drained zones and mistyped + // ones alike, and identically on every replica. + err = nil + } if endpointsList == nil { endpointsList = endpointsListFunc() } - for _, ep := range endpointsList { - if object.EndpointsKey(svc.Name, svc.Namespace) != ep.Index { - continue - } + addForZone := func(topoZone string) (added int) { + for _, ep := range endpointsList { + if object.EndpointsKey(svc.Name, svc.Namespace) != ep.Index { + continue + } - for _, eps := range ep.Subsets { - for _, addr := range eps.Addresses { - // See comments in parse.go parseRequest about the endpoint handling. - if r.endpoint != "" { - if !match(r.endpoint, endpointHostname(addr, k.endpointNameMode)) { + for _, eps := range ep.Subsets { + for _, addr := range eps.Addresses { + // See comments in parse.go parseRequest about the endpoint handling. + if topoZone != "" && ep.Zones[addr.IP] != topoZone { continue } - } - - for _, p := range eps.Ports { - if !(matchPortAndProtocol(r.port, p.Name, r.protocol, p.Protocol)) { - continue + if r.endpoint != "" { + if !match(r.endpoint, endpointHostname(addr, k.endpointNameMode)) { + continue + } } - s := msg.Service{Host: addr.IP, Port: int(p.Port), TTL: k.ttl} - s.Key = strings.Join([]string{zonePath, Svc, svc.Namespace, svc.Name, endpointHostname(addr, k.endpointNameMode)}, "/") - err = nil + for _, p := range eps.Ports { + if !(matchPortAndProtocol(r.port, p.Name, r.protocol, p.Protocol)) { + continue + } + s := msg.Service{Host: addr.IP, Port: int(p.Port), TTL: k.ttl} + s.Key = strings.Join([]string{zonePath, Svc, svc.Namespace, svc.Name, endpointHostname(addr, k.endpointNameMode)}, "/") - services = append(services, s) + err = nil + + services = append(services, s) + added++ + } } } } + return added + } + if addForZone(r.zone) == 0 && r.zonePrefer { + // The prefer directive falls back to the whole service when + // the zone holds nothing. The fallback is in the NAME the + // client chose, so it is never a silent widening of a pin. + addForZone("") } continue } diff --git a/plugin/kubernetes/metadata.go b/plugin/kubernetes/metadata.go index c0e369e37..6832ee411 100644 --- a/plugin/kubernetes/metadata.go +++ b/plugin/kubernetes/metadata.go @@ -36,7 +36,7 @@ func (k *Kubernetes) Metadata(ctx context.Context, state request.Request) contex multicluster = true } // possible optimization: cache r so it doesn't need to be calculated again in ServeDNS - r, err := parseRequest(state.Name(), zone, multicluster) + r, err := parseRequest(state.Name(), zone, multicluster, k.opts.zonal) if err != nil { metadata.SetValueFunc(ctx, "kubernetes/parse-error", func() string { return err.Error() @@ -62,6 +62,12 @@ func (k *Kubernetes) Metadata(ctx context.Context, state request.Request) contex }) } + if k.opts.zonal { + metadata.SetValueFunc(ctx, "kubernetes/zone", func() string { + return r.zone + }) + } + metadata.SetValueFunc(ctx, "kubernetes/service", func() string { return r.service }) diff --git a/plugin/kubernetes/object/endpoint.go b/plugin/kubernetes/object/endpoint.go index e7c45f5d9..1cdb306b6 100644 --- a/plugin/kubernetes/object/endpoint.go +++ b/plugin/kubernetes/object/endpoint.go @@ -2,6 +2,8 @@ package object import ( "fmt" + "maps" + "strings" discovery "k8s.io/api/discovery/v1" meta "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -17,6 +19,12 @@ type Endpoints struct { Index string IndexIP []string Subsets []EndpointSubset + // Zones maps address IPs to their topology.kubernetes.io/zone, + // lowercased. Nil unless the kubernetes plugin's `zonal` option + // selected the zone-retaining transform, so default configurations + // carry one nil pointer per slice and their addresses stay exactly + // as slim as before. + Zones map[string]string *Empty } @@ -48,6 +56,18 @@ func EndpointsKey(name, namespace string) string { return name + "." + namespace // EndpointSliceToEndpoints converts a *discovery.EndpointSlice to a *Endpoints. func EndpointSliceToEndpoints(obj meta.Object) (meta.Object, error) { + return endpointSliceToEndpoints(obj, false /* withZones */) +} + +// EndpointSliceToEndpointsWithZones is EndpointSliceToEndpoints, but also +// retains each endpoint's topology zone. Used only when the kubernetes +// plugin's zonal option is enabled, so the default configuration's cache +// stays exactly as slim as before. +func EndpointSliceToEndpointsWithZones(obj meta.Object) (meta.Object, error) { + return endpointSliceToEndpoints(obj, true /* withZones */) +} + +func endpointSliceToEndpoints(obj meta.Object, withZones bool) (meta.Object, error) { ends, ok := obj.(*discovery.EndpointSlice) if !ok { return nil, fmt.Errorf("unexpected object %v", obj) @@ -92,6 +112,14 @@ func EndpointSliceToEndpoints(obj meta.Object) (meta.Object, error) { if end.Hostname != nil { ea.Hostname = *end.Hostname } + if withZones && end.Zone != nil { + if e.Zones == nil { + e.Zones = make(map[string]string) + } + // Lowercased once here: qnames arrive case-folded, so + // lookups compare without folding per query. + e.Zones[a] = strings.ToLower(*end.Zone) + } // ignore pod names that are too long to be a valid label if end.TargetRef != nil && len(end.TargetRef.Name) < 64 { ea.TargetRefName = end.TargetRef.Name @@ -144,6 +172,9 @@ func (e *Endpoints) DeepCopyObject() runtime.Object { Subsets: make([]EndpointSubset, len(e.Subsets)), } copy(e1.IndexIP, e.IndexIP) + if e.Zones != nil { + e1.Zones = maps.Clone(e.Zones) + } for i, eps := range e.Subsets { sub := EndpointSubset{ diff --git a/plugin/kubernetes/parse.go b/plugin/kubernetes/parse.go index f87a64e03..f0d22aa70 100644 --- a/plugin/kubernetes/parse.go +++ b/plugin/kubernetes/parse.go @@ -17,6 +17,14 @@ type recordRequest struct { protocol string endpoint string cluster string + // The topology zone from a zone-scoped name + // (zone.pin._zone.service.namespace.svc.zone); only set when the zonal + // option is enabled. + zone string + // zonePrefer is set for the prefer directive: a zone holding no + // endpoints falls back to all endpoints. The zero value is the pin + // directive, which answers NODATA instead. + zonePrefer bool // The servicename used in Kubernetes. service string // The namespace used in Kubernetes. @@ -25,15 +33,31 @@ type recordRequest struct { podOrSvc string } +// zoneLabel anchors a zone-scoped name: +// topozone.DIRECTIVE._zone.service.namespace.svc.zone. It sits three labels +// left of the service — a shape that has always been "query too long" +// (NXDOMAIN) — and the underscore keeps it out of every hostname-shaped +// grammar, so nothing served or servable collides with it. The directive +// label selects the semantics; bare words are safe there because the +// subtree is only reachable through the anchor. +const zoneLabel = "_zone" + +// Zone-scoped name directives. +const ( + directivePin = "pin" // zone-local endpoints, NODATA if none + directivePrefer = "prefer" // zone-local endpoints, all endpoints if none +) + // 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 parseRequest(name, zone string, multicluster bool) (r recordRequest, err error) { - // 4 Possible cases: +func parseRequest(name, zone string, multicluster, zonal bool) (r recordRequest, err error) { + // 5 Possible cases: // 1. _port._protocol.service.namespace.pod|svc.zone // 2. (endpoint): endpoint.service.namespace.pod|svc.zone // 3. (service): service.namespace.pod|svc.zone // 4. (endpoint multicluster): endpoint.cluster.service.namespace.pod|svc.zone + // 5. (zonal): topozone.pin|prefer._zone.service.namespace.svc.zone base, _ := dnsutil.TrimZone(name, zone) // return NODATA for apex queries @@ -67,7 +91,8 @@ func parseRequest(name, zone string, multicluster bool) (r recordRequest, err er return r, nil } - // Because of ambiguity we check the labels left: 1: an endpoint. 2: port and protocol or endpoint and clusterid. + // Because of ambiguity we check the labels left: 1: an endpoint. 2: port and protocol or endpoint + // and clusterid. 3 or more: a zone-scoped name (the zone value may span labels). // 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 @@ -81,8 +106,23 @@ func parseRequest(name, zone string, multicluster bool) (r recordRequest, err er r.endpoint = segs[last-1] } - default: // too long - return r, errInvalidRequest + default: // zone-scoped name (topozone.pin|prefer._zone), or too long + // Kubernetes zone label values may contain dots, so the zone is + // every label left of the directive, joined. Not defined in + // multicluster zones; everything this arm rejects keeps the stock + // too-long NXDOMAIN, so behavior with the option off (or for + // unknown directives) is byte-identical to today. + if !zonal || multicluster || segs[last] != zoneLabel || r.podOrSvc != Svc { + return r, errInvalidRequest + } + switch segs[last-1] { + case directivePin: + case directivePrefer: + r.zonePrefer = true + default: + return r, errInvalidRequest + } + r.zone = strings.Join(segs[:last-1], ".") } return r, nil diff --git a/plugin/kubernetes/parse_test.go b/plugin/kubernetes/parse_test.go index 4cee8538a..cb7298f89 100644 --- a/plugin/kubernetes/parse_test.go +++ b/plugin/kubernetes/parse_test.go @@ -13,28 +13,38 @@ func TestParseRequest(t *testing.T) { query string expected string // output from r.String() multicluster bool + zonal bool + zone string // expected r.zone }{ // valid SRV request - {"_http._tcp.webs.mynamespace.svc.inter.webs.tests.", "http.tcp...webs.mynamespace.svc", false}, + {"_http._tcp.webs.mynamespace.svc.inter.webs.tests.", "http.tcp...webs.mynamespace.svc", false, false, ""}, // A request of endpoint - {"1-2-3-4.webs.mynamespace.svc.inter.webs.tests.", "..1-2-3-4..webs.mynamespace.svc", false}, + {"1-2-3-4.webs.mynamespace.svc.inter.webs.tests.", "..1-2-3-4..webs.mynamespace.svc", false, false, ""}, // bare zone - {"inter.webs.tests.", "......", false}, + {"inter.webs.tests.", "......", false, false, ""}, // bare svc type - {"svc.inter.webs.tests.", "......", false}, + {"svc.inter.webs.tests.", "......", false, false, ""}, // bare pod type - {"pod.inter.webs.tests.", "......", false}, + {"pod.inter.webs.tests.", "......", false, false, ""}, // SRV request with empty segments - {"..webs.mynamespace.svc.inter.webs.tests.", "....webs.mynamespace.svc", false}, + {"..webs.mynamespace.svc.inter.webs.tests.", "....webs.mynamespace.svc", false, false, ""}, // A multicluster request with a clusterid - {"1-2-3-4.cluster1.webs.mynamespace.svc.inter.webs.tests.", "..1-2-3-4.cluster1.webs.mynamespace.svc", true}, + {"1-2-3-4.cluster1.webs.mynamespace.svc.inter.webs.tests.", "..1-2-3-4.cluster1.webs.mynamespace.svc", true, false, ""}, + // zone-scoped names, both directives + {"us-west-2a.pin._zone.webs.mynamespace.svc.inter.webs.tests.", "....webs.mynamespace.svc", false, true, "us-west-2a"}, + {"us-west-2a.prefer._zone.webs.mynamespace.svc.inter.webs.tests.", "....webs.mynamespace.svc", false, true, "us-west-2a"}, + // zone label values may contain dots; the value spans every label + // left of the directive + {"corp.example.com.pin._zone.webs.mynamespace.svc.inter.webs.tests.", "....webs.mynamespace.svc", false, true, "corp.example.com"}, + // two-labels-left with an underscore still reads as port/protocol + {"us-west-2a._zone.webs.mynamespace.svc.inter.webs.tests.", "us-west-2a.zone...webs.mynamespace.svc", false, true, ""}, } for i, tc := range tests { m := new(dns.Msg) m.SetQuestion(tc.query, dns.TypeA) state := request.Request{Zone: zone, Req: m} - r, e := parseRequest(state.Name(), state.Zone, tc.multicluster) + r, e := parseRequest(state.Name(), state.Zone, tc.multicluster, tc.zonal) if e != nil { t.Errorf("Test %d, expected no error, got '%v'.", i, e) } @@ -42,13 +52,36 @@ func TestParseRequest(t *testing.T) { if rs != tc.expected { t.Errorf("Test %d, expected (stringified) recordRequest: %s, got %s", i, tc.expected, rs) } + if r.zone != tc.zone { + t.Errorf("Test %d, expected zone %q, got %q", i, tc.zone, r.zone) + } } } func TestParseInvalidRequest(t *testing.T) { 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. + "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. + "us-west-2a.pin._zone.webs.mynamespace.svc.inter.webs.tests.", // Zonal shape without the zonal option. + } + + // The zonal-shaped rejections that must hold even WITH the option on: + // wrong subtree, unknown directive, and multicluster zones. + zonalInvalid := []struct { + query string + multicluster bool + }{ + {"us-west-2a.pin._zone.webs.mynamespace.pod.inter.webs.tests.", false}, + {"us-west-2a.florp._zone.webs.mynamespace.svc.inter.webs.tests.", false}, + {"us-west-2a.pin._zone.webs.mynamespace.svc.inter.webs.tests.", true}, + } + for i, tc := range zonalInvalid { + m := new(dns.Msg) + m.SetQuestion(tc.query, dns.TypeA) + state := request.Request{Zone: zone, Req: m} + if _, e := parseRequest(state.Name(), state.Zone, tc.multicluster, true); e == nil { + t.Errorf("Zonal-invalid test %d: expected error from %s, got none", i, tc.query) + } } for i, query := range invalid { @@ -56,7 +89,7 @@ func TestParseInvalidRequest(t *testing.T) { m.SetQuestion(query, dns.TypeA) state := request.Request{Zone: zone, Req: m} - if _, e := parseRequest(state.Name(), state.Zone, false); e == nil { + if _, e := parseRequest(state.Name(), state.Zone, false, false); e == nil { t.Errorf("Test %d: expected error from %s, got none", i, query) } } @@ -67,6 +100,6 @@ const zone = "inter.webs.tests." func BenchmarkParseRequest(b *testing.B) { b.ReportAllocs() for b.Loop() { - _, _ = parseRequest("1-2-3-4.webs.mynamespace.svc.inter.webs.tests.", zone, false) + _, _ = parseRequest("1-2-3-4.webs.mynamespace.svc.inter.webs.tests.", zone, false, false) } } diff --git a/plugin/kubernetes/setup.go b/plugin/kubernetes/setup.go index d74a96af3..765a80964 100644 --- a/plugin/kubernetes/setup.go +++ b/plugin/kubernetes/setup.go @@ -207,6 +207,11 @@ func ParseStanza(c *caddy.Controller) (*Kubernetes, error) { return nil, c.ArgErr() } k8s.opts.initEndpointsCache = false + case "zonal": + if len(c.RemainingArgs()) != 0 { + return nil, c.ArgErr() + } + k8s.opts.zonal = true case "ignore": args := c.RemainingArgs() if len(args) > 0 { @@ -297,6 +302,13 @@ func ParseStanza(c *caddy.Controller) (*Kubernetes, error) { } } + if k8s.opts.zonal && !k8s.opts.initEndpointsCache { + // Zone-scoped names are answered from the endpoint cache; + // without it every zonal name would contradict the documented + // noendpoints behavior (NXDOMAIN for all headless queries). + return nil, c.Errf("zonal requires the endpoint cache; remove noendpoints") + } + return k8s, nil } diff --git a/plugin/kubernetes/setup_test.go b/plugin/kubernetes/setup_test.go index f9359bd2c..6a38dce62 100644 --- a/plugin/kubernetes/setup_test.go +++ b/plugin/kubernetes/setup_test.go @@ -829,3 +829,59 @@ func TestBoundIPs(t *testing.T) { }) } } + +func TestKubernetesParseZonal(t *testing.T) { + tests := []struct { + input string + shouldErr bool + expectedZonal bool + }{ + { + `kubernetes coredns.local { + zonal +}`, + false, + true, + }, + { + `kubernetes coredns.local { + zonal us-west-2a +}`, + true, + false, + }, + { + `kubernetes coredns.local { + zonal + noendpoints +}`, + true, + false, + }, + { + `kubernetes coredns.local { +}`, + false, + false, + }, + } + + for i, test := range tests { + c := caddy.NewTestController("dns", test.input) + k8sController, err := kubernetesParse(c) + + if test.shouldErr { + if err == nil { + t.Errorf("Test %d: Expected error, got none for input '%s'", i, test.input) + } + continue + } + if err != nil { + t.Errorf("Test %d: Expected no error, got '%v' for input '%s'", i, err, test.input) + continue + } + if k8sController.opts.zonal != test.expectedZonal { + t.Errorf("Test %d: Expected zonal=%v, got %v", i, test.expectedZonal, k8sController.opts.zonal) + } + } +} diff --git a/plugin/kubernetes/zonal_test.go b/plugin/kubernetes/zonal_test.go new file mode 100644 index 000000000..a3bc337d1 --- /dev/null +++ b/plugin/kubernetes/zonal_test.go @@ -0,0 +1,263 @@ +package kubernetes + +import ( + "context" + "testing" + + "github.com/coredns/coredns/plugin/kubernetes/object" + "github.com/coredns/coredns/plugin/pkg/dnstest" + "github.com/coredns/coredns/plugin/test" + + "github.com/miekg/dns" + api "k8s.io/api/core/v1" +) + +type APIConnZonalTest struct{} + +func (APIConnZonalTest) HasSynced() bool { return true } +func (APIConnZonalTest) Run() {} +func (APIConnZonalTest) Stop() error { return nil } +func (APIConnZonalTest) PodIndex(string) []*object.Pod { return nil } +func (APIConnZonalTest) SvcIndexReverse(string) []*object.Service { return nil } +func (APIConnZonalTest) SvcExtIndexReverse(string) []*object.Service { return nil } +func (APIConnZonalTest) ServiceImportList() []*object.ServiceImport { return nil } +func (APIConnZonalTest) SvcImportIndex(string) []*object.ServiceImport { return nil } +func (APIConnZonalTest) EpIndexReverse(string) []*object.Endpoints { return nil } +func (APIConnZonalTest) McEpIndex(string) []*object.MultiClusterEndpoints { return nil } +func (APIConnZonalTest) Modified(ModifiedMode) int64 { return int64(1499347823) } + +func (a APIConnZonalTest) ServiceList() []*object.Service { + return []*object.Service{ + { + Name: "hdls", + Namespace: "testns", + ClusterIPs: []string{api.ClusterIPNone}, + }, + { + Name: "clstr", + Namespace: "testns", + ClusterIPs: []string{"10.0.0.10"}, + Ports: []api.ServicePort{{Name: "http", Protocol: "tcp", Port: 80}}, + }, + } +} + +func (a APIConnZonalTest) SvcIndex(idx string) []*object.Service { + switch idx { + case "hdls.testns": + return a.ServiceList()[:1] + case "clstr.testns": + return a.ServiceList()[1:] + } + return nil +} + +func (a APIConnZonalTest) EndpointsList() []*object.Endpoints { + return []*object.Endpoints{ + { + Subsets: []object.EndpointSubset{ + { + Addresses: []object.EndpointAddress{ + {IP: "172.0.0.1"}, + {IP: "172.0.0.2"}, + {IP: "172.0.0.3"}, + }, + Ports: []object.EndpointPort{{Port: 80, Name: "http", Protocol: "tcp"}}, + }, + }, + Zones: map[string]string{ + "172.0.0.1": "us-west-2a", + "172.0.0.2": "us-west-2b", + "172.0.0.3": "us-west-2b", + }, + Name: "hdls-slice", + Namespace: "testns", + Index: object.EndpointsKey("hdls", "testns"), + }, + } +} + +func (a APIConnZonalTest) EpIndex(idx string) []*object.Endpoints { + if idx == "hdls.testns" { + return a.EndpointsList() + } + return nil +} + +func (APIConnZonalTest) GetNodeByName(_ context.Context, _ string) (*api.Node, error) { + return &api.Node{}, nil +} + +func (APIConnZonalTest) GetNamespaceByName(name string) (*object.Namespace, error) { + return &object.Namespace{Name: name}, nil +} + +var zonalTestCases = []test.Case{ + { // pin: endpoints narrowed to the requested zone + Qname: "us-west-2a.pin._zone.hdls.testns.svc.cluster.local.", Qtype: dns.TypeA, + Rcode: dns.RcodeSuccess, + Answer: []dns.RR{ + test.A("us-west-2a.pin._zone.hdls.testns.svc.cluster.local. 5 IN A 172.0.0.1"), + }, + }, + { + Qname: "us-west-2b.pin._zone.hdls.testns.svc.cluster.local.", Qtype: dns.TypeA, + Rcode: dns.RcodeSuccess, + Answer: []dns.RR{ + test.A("us-west-2b.pin._zone.hdls.testns.svc.cluster.local. 5 IN A 172.0.0.2"), + test.A("us-west-2b.pin._zone.hdls.testns.svc.cluster.local. 5 IN A 172.0.0.3"), + }, + }, + { // pin: any zone label without matching endpoints — drained zones and + // typos alike — is the same determinate empty answer + Qname: "us-west-2c.pin._zone.hdls.testns.svc.cluster.local.", Qtype: dns.TypeA, + Rcode: dns.RcodeSuccess, + Ns: []dns.RR{ + test.SOA("cluster.local. 5 IN SOA ns.dns.cluster.local. hostmaster.cluster.local. 1499347823 7200 1800 86400 5"), + }, + }, + { // prefer: narrows exactly like pin when the zone is populated + Qname: "us-west-2a.prefer._zone.hdls.testns.svc.cluster.local.", Qtype: dns.TypeA, + Rcode: dns.RcodeSuccess, + Answer: []dns.RR{ + test.A("us-west-2a.prefer._zone.hdls.testns.svc.cluster.local. 5 IN A 172.0.0.1"), + }, + }, + { // prefer: an empty zone falls back to every endpoint — the fallback + // is chosen in the name, so it is not a silent widening of a pin + Qname: "us-west-2c.prefer._zone.hdls.testns.svc.cluster.local.", Qtype: dns.TypeA, + Rcode: dns.RcodeSuccess, + Answer: []dns.RR{ + test.A("us-west-2c.prefer._zone.hdls.testns.svc.cluster.local. 5 IN A 172.0.0.1"), + test.A("us-west-2c.prefer._zone.hdls.testns.svc.cluster.local. 5 IN A 172.0.0.2"), + test.A("us-west-2c.prefer._zone.hdls.testns.svc.cluster.local. 5 IN A 172.0.0.3"), + }, + }, + { // a nonexistent service stays NXDOMAIN under any directive + Qname: "us-west-2a.pin._zone.ghost.testns.svc.cluster.local.", Qtype: dns.TypeA, + Rcode: dns.RcodeNameError, + Ns: []dns.RR{ + test.SOA("cluster.local. 5 IN SOA ns.dns.cluster.local. hostmaster.cluster.local. 1499347823 7200 1800 86400 5"), + }, + }, + { // an unknown directive keeps the stock too-long NXDOMAIN + Qname: "us-west-2a.florp._zone.hdls.testns.svc.cluster.local.", Qtype: dns.TypeA, + Rcode: dns.RcodeNameError, + Ns: []dns.RR{ + test.SOA("cluster.local. 5 IN SOA ns.dns.cluster.local. hostmaster.cluster.local. 1499347823 7200 1800 86400 5"), + }, + }, + { // zone-scoped names are defined for headless services only + Qname: "us-west-2a.pin._zone.clstr.testns.svc.cluster.local.", Qtype: dns.TypeA, + Rcode: dns.RcodeNameError, + Ns: []dns.RR{ + test.SOA("cluster.local. 5 IN SOA ns.dns.cluster.local. hostmaster.cluster.local. 1499347823 7200 1800 86400 5"), + }, + }, + { // pin: exists-but-empty stays NODATA for non-address qtypes too (TXT + // has its own lookup branch; NXDOMAIN would be negative-cached per name) + Qname: "us-west-2c.pin._zone.hdls.testns.svc.cluster.local.", Qtype: dns.TypeTXT, + Rcode: dns.RcodeSuccess, + Ns: []dns.RR{ + test.SOA("cluster.local. 5 IN SOA ns.dns.cluster.local. hostmaster.cluster.local. 1499347823 7200 1800 86400 5"), + }, + }, + { // prefer: SRV narrows when populated and falls back when not, same as A + Qname: "us-west-2c.prefer._zone.hdls.testns.svc.cluster.local.", Qtype: dns.TypeSRV, + Rcode: dns.RcodeSuccess, + Answer: []dns.RR{ + test.SRV("us-west-2c.prefer._zone.hdls.testns.svc.cluster.local. 5 IN SRV 0 33 80 172-0-0-1.hdls.testns.svc.cluster.local."), + test.SRV("us-west-2c.prefer._zone.hdls.testns.svc.cluster.local. 5 IN SRV 0 33 80 172-0-0-2.hdls.testns.svc.cluster.local."), + test.SRV("us-west-2c.prefer._zone.hdls.testns.svc.cluster.local. 5 IN SRV 0 33 80 172-0-0-3.hdls.testns.svc.cluster.local."), + }, + Extra: []dns.RR{ + test.A("172-0-0-1.hdls.testns.svc.cluster.local. 5 IN A 172.0.0.1"), + test.A("172-0-0-2.hdls.testns.svc.cluster.local. 5 IN A 172.0.0.2"), + test.A("172-0-0-3.hdls.testns.svc.cluster.local. 5 IN A 172.0.0.3"), + }, + }, + { // prefer: TXT on a zonal name is NODATA like every non-address type + Qname: "us-west-2c.prefer._zone.hdls.testns.svc.cluster.local.", Qtype: dns.TypeTXT, + Rcode: dns.RcodeSuccess, + Ns: []dns.RR{ + test.SOA("cluster.local. 5 IN SOA ns.dns.cluster.local. hostmaster.cluster.local. 1499347823 7200 1800 86400 5"), + }, + }, + { // pin: SRV comes out zone-filtered, since filtering happens at endpoint selection + Qname: "us-west-2b.pin._zone.hdls.testns.svc.cluster.local.", Qtype: dns.TypeSRV, + Rcode: dns.RcodeSuccess, + Answer: []dns.RR{ + test.SRV("us-west-2b.pin._zone.hdls.testns.svc.cluster.local. 5 IN SRV 0 50 80 172-0-0-2.hdls.testns.svc.cluster.local."), + test.SRV("us-west-2b.pin._zone.hdls.testns.svc.cluster.local. 5 IN SRV 0 50 80 172-0-0-3.hdls.testns.svc.cluster.local."), + }, + Extra: []dns.RR{ + test.A("172-0-0-2.hdls.testns.svc.cluster.local. 5 IN A 172.0.0.2"), + test.A("172-0-0-3.hdls.testns.svc.cluster.local. 5 IN A 172.0.0.3"), + }, + }, +} + +// Without the option the shape is three-labels-too-long, exactly as it has +// always been: behavior identical to before the feature existed. +var zonalDisabledTestCases = []test.Case{ + { + Qname: "us-west-2a.pin._zone.hdls.testns.svc.cluster.local.", Qtype: dns.TypeA, + Rcode: dns.RcodeNameError, + Ns: []dns.RR{ + test.SOA("cluster.local. 5 IN SOA ns.dns.cluster.local. hostmaster.cluster.local. 1499347823 7200 1800 86400 5"), + }, + }, +} + +// A zone-only endpoint change alters zonal answers, so it must not be +// classified as equivalent (which would skip the serial bump). Zone is +// empty in default configurations, so this costs nothing when the option +// is off. +func TestEndpointsEquivalentZoneChange(t *testing.T) { + eps := func(zone string) *object.Endpoints { + return &object.Endpoints{ + Subsets: []object.EndpointSubset{{ + Addresses: []object.EndpointAddress{{IP: "172.0.0.1"}}, + }}, + Zones: map[string]string{"172.0.0.1": zone}, + } + } + if !endpointsEquivalent(eps("us-west-2a"), eps("us-west-2a")) { + t.Fatal("identical endpoints must be equivalent") + } + if endpointsEquivalent(eps("us-west-2a"), eps("us-west-2b")) { + t.Fatal("a zone-only change alters zonal answers and must not be equivalent") + } +} + +func TestServeDNSZonal(t *testing.T) { + k := New([]string{"cluster.local."}) + k.APIConn = &APIConnZonalTest{} + k.Next = test.NextHandler(dns.RcodeSuccess, nil) + k.Namespaces = map[string]struct{}{"testns": {}} + k.opts.zonal = true + ctx := context.TODO() + + runZonalCases(ctx, t, k, zonalTestCases) + + k.opts.zonal = false + runZonalCases(ctx, t, k, zonalDisabledTestCases) +} + +func runZonalCases(ctx context.Context, t *testing.T, k *Kubernetes, cases []test.Case) { + t.Helper() + for i, tc := range cases { + r := tc.Msg() + w := dnstest.NewRecorder(&test.ResponseWriter{}) + if _, err := k.ServeDNS(ctx, w, r); err != nil { + t.Errorf("Test %d expected no error, got %v", i, err) + continue + } + if w.Msg == nil { + t.Fatalf("Test %d, got nil message for %q", i, r.Question[0].Name) + } + if err := test.SortAndCheck(w.Msg, tc); err != nil { + t.Errorf("Test %d (%s), %v", i, tc.Qname, err) + } + } +}