plugin/etcd: avoid apex fallback on backend errors (#8465)

Signed-off-by: houyuwushang <liuluoqianqiu@outlook.com>

plugin/backend_lookup.go

@@ -523,12 +523,11 @@ func checkForApex(ctx context.Context, b ServiceBackend, zone string, state requ
 	state.Req.Question[0].Name = dnsutil.Join("apex.dns", zone)
 
 	services, err := b.Services(ctx, state, false, opt)
-	if err == nil {
+	state.Req.Question[0].Name = old
-		state.Req.Question[0].Name = old
+	if err == nil || !b.IsNameError(err) {
 		return services, err
 	}
 
-	state.Req.Question[0].Name = old
 	return b.Services(ctx, state, false, opt)
 }
 

plugin/backend_lookup_test.go

@@ -2,6 +2,7 @@ package plugin
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"net"
 	"testing"
@@ -18,12 +19,13 @@ import (
 var _ ServiceBackend = &mockBackend{}
 
 type mockBackend struct {
-	mockServices func(ctx context.Context, state request.Request, exact bool, opt Options) ([]msg.Service, error)
+	mockServices  func(ctx context.Context, state request.Request, exact bool, opt Options) ([]msg.Service, error)
-	mockReverse  func(ctx context.Context, state request.Request, exact bool, opt Options) ([]msg.Service, error)
+	mockReverse   func(ctx context.Context, state request.Request, exact bool, opt Options) ([]msg.Service, error)
-	mockLookup   func(ctx context.Context, state request.Request, name string, typ uint16) (*dns.Msg, error)
+	mockLookup    func(ctx context.Context, state request.Request, name string, typ uint16) (*dns.Msg, error)
-	mockRecords  func(ctx context.Context, state request.Request, exact bool) ([]msg.Service, error)
+	mockRecords   func(ctx context.Context, state request.Request, exact bool) ([]msg.Service, error)
-	minTTL       uint32
+	mockNameError func(err error) bool
-	serial       uint32
+	minTTL        uint32
+	serial        uint32
 }
 
 func (m *mockBackend) Serial(_state request.Request) uint32 {
@@ -52,7 +54,10 @@ func (m *mockBackend) Lookup(ctx context.Context, state request.Request, name st
 	return m.mockLookup(ctx, state, name, typ)
 }
 
-func (m *mockBackend) IsNameError(_err error) bool {
+func (m *mockBackend) IsNameError(err error) bool {
+	if m.mockNameError != nil {
+		return m.mockNameError(err)
+	}
 	return false
 }
 
@@ -569,13 +574,17 @@ func TestCheckForApex(t *testing.T) {
 }
 
 func TestCheckForApexFallback(t *testing.T) {
+	errName := errors.New("name not found")
+	calls := 0
 	b := &mockBackend{
 		mockServices: func(_ctx context.Context, state request.Request, _exact bool, _opt Options) ([]msg.Service, error) {
+			calls++
 			if state.QName() == "apex.dns.example.org." {
-				return nil, dns.ErrRcode
+				return nil, errName
 			}
 			return []msg.Service{{Host: "::1", TTL: 20}}, nil
 		},
+		mockNameError: func(err error) bool { return errors.Is(err, errName) },
 	}
 	req := new(dns.Msg)
 	req.SetQuestion("example.org.", dns.TypeA)
@@ -587,6 +596,42 @@ func TestCheckForApexFallback(t *testing.T) {
 	if len(services) != 1 || services[0].Host != "::1" {
 		t.Fatalf("expected fallback services, got %+v", services)
 	}
+	if calls != 2 {
+		t.Fatalf("expected apex and fallback lookups, got %d calls", calls)
+	}
+	if state.QName() != "example.org." {
+		t.Fatalf("query name was not restored: %s", state.QName())
+	}
+}
+
+func TestCheckForApexBackendError(t *testing.T) {
+	errBackend := context.DeadlineExceeded
+	calls := 0
+	b := &mockBackend{
+		mockServices: func(_ctx context.Context, state request.Request, _exact bool, _opt Options) ([]msg.Service, error) {
+			calls++
+			if state.QName() != "apex.dns.example.org." {
+				t.Fatalf("unexpected fallback lookup for %s", state.QName())
+			}
+			return nil, errBackend
+		},
+	}
+	req := new(dns.Msg)
+	req.SetQuestion("example.org.", dns.TypeA)
+	state := request.Request{Req: req, W: &test.ResponseWriter{}}
+	services, err := checkForApex(context.Background(), b, "example.org.", state, Options{})
+	if !errors.Is(err, errBackend) {
+		t.Fatalf("expected backend error, got %v", err)
+	}
+	if len(services) != 0 {
+		t.Fatalf("expected no services, got %+v", services)
+	}
+	if calls != 1 {
+		t.Fatalf("expected one backend lookup, got %d", calls)
+	}
+	if state.QName() != "example.org." {
+		t.Fatalf("query name was not restored: %s", state.QName())
+	}
 }
 
 func TestIsDuplicate(t *testing.T) {

plugin/etcd/README.md

@@ -80,6 +80,24 @@ find entries like `/skydns/test/skydns/mx1`.
 
 This causes two lookups from CoreDNS to etcd in certain cases.
 
+### Backend load
+
+The *etcd* plugin does not watch or poll etcd when data changes. Record lookups are driven by
+incoming DNS queries. An uncached lookup may issue multiple etcd requests when an exact-key fallback
+or an internal CNAME lookup is needed.
+
+Non-exact lookups use an etcd prefix range and return the complete subtree by design. Querying a name
+high in the hierarchy can therefore read many keys. In particular, an A or AAAA query for a configured
+zone name first looks below `apex.dns.ZONE`. If that entry does not exist, CoreDNS falls back to the
+zone's root prefix for compatibility with older SkyDNS layouts. On a large zone, that fallback scans
+the entire zone subtree.
+
+Store zone-apex address records below the `dns/apex` path, as shown in the examples below, to avoid
+the zone-wide fallback. The *cache* plugin reduces repeated backend reads for the same DNS question,
+but does not reduce the size of the first prefix response. Use the *log* plugin to identify the client
+and question name that trigger a lookup, and `coredns_dns_requests_total` from the *prometheus* plugin
+to measure query volume by zone and type.
+
 ## Examples
 
 This is the default SkyDNS setup, with everything specified in full:
@@ -170,7 +188,7 @@ entries to the ETCD path of your zone. If your zone is named `skydns.local` for
 create an `A` record for this zone as follows:
 
 ~~~
-% etcdctl put /skydns/local/skydns/ '{"host":"1.1.1.1","ttl":60}'
+% etcdctl put /skydns/local/skydns/dns/apex/x1 '{"host":"1.1.1.1","ttl":60}'
 ~~~
 
 If you query the zone name itself, you will receive the created `A` record:
@@ -182,8 +200,8 @@ If you query the zone name itself, you will receive the created `A` record:
 
 If you would like to use DNS RR for the zone name, you can set the following:
 ~~~
-% etcdctl put /skydns/local/skydns/x1 '{"host":"1.1.1.1","ttl":60}'
+% etcdctl put /skydns/local/skydns/dns/apex/x1 '{"host":"1.1.1.1","ttl":60}'
-% etcdctl put /skydns/local/skydns/x2 '{"host":"1.1.1.2","ttl":60}'
+% etcdctl put /skydns/local/skydns/dns/apex/x2 '{"host":"1.1.1.2","ttl":60}'
 ~~~
 
 If you query the zone name now, you will get the following response:
@@ -198,8 +216,8 @@ If you query the zone name now, you will get the following response:
 
 If you would like to use `AAAA` records for the zone name too, you can set the following:
 ~~~
-% etcdctl put /skydns/local/skydns/x3 '{"host":"2003::8:1","ttl":60}'
+% etcdctl put /skydns/local/skydns/dns/apex/x3 '{"host":"2003::8:1","ttl":60}'
-% etcdctl put /skydns/local/skydns/x4 '{"host":"2003::8:2","ttl":60}'
+% etcdctl put /skydns/local/skydns/dns/apex/x4 '{"host":"2003::8:2","ttl":60}'
 ~~~
 
 If you query the zone name for `AAAA` now, you will get the following response: