Log the deferral reason when no handler answers and keep one logrus test hook

This commit is contained in:
Viktor Liu
2026-07-27 11:57:59 +02:00
parent 4546908ceb
commit f7af852851
3 changed files with 22 additions and 10 deletions
+5 -2
View File
@@ -367,8 +367,11 @@ func (c *HandlerChain) dispatch(w dns.ResponseWriter, r *dns.Msg, maxPriority in
resutil.AttachEDE(resp, resutil.EDENetbirdSoftenedNegative, resutil.AttachEDE(resp, resutil.EDENetbirdSoftenedNegative,
"netbird: name is served locally, no fallthrough resolver for this query type") "netbird: name is served locally, no fallthrough resolver for this query type")
} }
logger.Tracef("no handler below the deferring one for domain=%s type=%s, answering NODATA", // logResponse never runs on this path, so the carried metadata is
qname, dns.TypeToString[question.Qtype]) // appended here or the reason for the deferral is lost in exactly the
// case that is hardest to diagnose.
logger.Tracef("no handler below the deferring one for domain=%s type=%s, answering NODATA%s",
qname, dns.TypeToString[question.Qtype], carried.format())
} }
if err := w.WriteMsg(resp); err != nil { if err := w.WriteMsg(resp); err != nil {
logger.Errorf("failed to write DNS response: %v", err) logger.Errorf("failed to write DNS response: %v", err)
+7 -5
View File
@@ -1344,6 +1344,12 @@ func responseLineFor(hook *logtest.Hook, qname string) string {
// without it an empty answer is indistinguishable from a real "no such record" // without it an empty answer is indistinguishable from a real "no such record"
// in a dig output or a capture taken next to the application. // in a dig output or a capture taken next to the application.
func TestHandlerChain_SoftNegative_IsVisibleToTheClient(t *testing.T) { func TestHandlerChain_SoftNegative_IsVisibleToTheClient(t *testing.T) {
// One hook for the whole test: logtest installs it on the standard logger
// and logrus has no way to take it off again, so a hook per subtest would
// leave several behind buffering every later line in the package.
hook := logtest.NewGlobal()
t.Cleanup(hook.Reset)
newChain := func() *nbdns.HandlerChain { newChain := func() *nbdns.HandlerChain {
chain := nbdns.NewHandlerChain() chain := nbdns.NewHandlerChain()
chain.AddHandler("*.example.com.", &deferringHandler{softNegative: true}, nbdns.PriorityDNSRoute) chain.AddHandler("*.example.com.", &deferringHandler{softNegative: true}, nbdns.PriorityDNSRoute)
@@ -1387,9 +1393,6 @@ func TestHandlerChain_SoftNegative_IsVisibleToTheClient(t *testing.T) {
t.Run("log fields keep a stable order", func(t *testing.T) { t.Run("log fields keep a stable order", func(t *testing.T) {
const qname = "_mongodb._tcp.stable.example.com." const qname = "_mongodb._tcp.stable.example.com."
hook := logtest.NewGlobal()
t.Cleanup(hook.Reset)
prev := log.GetLevel() prev := log.GetLevel()
log.SetLevel(log.TraceLevel) log.SetLevel(log.TraceLevel)
t.Cleanup(func() { log.SetLevel(prev) }) t.Cleanup(func() { log.SetLevel(prev) })
@@ -1422,8 +1425,7 @@ func TestHandlerChain_SoftNegative_IsVisibleToTheClient(t *testing.T) {
t.Run("logged with the reason it was deferred", func(t *testing.T) { t.Run("logged with the reason it was deferred", func(t *testing.T) {
const qname = "_mongodb._tcp.reason.example.com." const qname = "_mongodb._tcp.reason.example.com."
hook := logtest.NewGlobal() hook.Reset()
t.Cleanup(hook.Reset)
prev := log.GetLevel() prev := log.GetLevel()
log.SetLevel(log.TraceLevel) log.SetLevel(log.TraceLevel)
@@ -135,10 +135,17 @@ func TestDispositionFor(t *testing.T) {
t.Run(dns.TypeToString[tt.qtype], func(t *testing.T) { t.Run(dns.TypeToString[tt.qtype], func(t *testing.T) {
assert.Equal(t, tt.want, dispositionFor(tt.qtype), "disposition for %s", dns.TypeToString[tt.qtype]) assert.Equal(t, tt.want, dispositionFor(tt.qtype), "disposition for %s", dns.TypeToString[tt.qtype])
if tt.want != dispositionPeer { // Address types are resolved by the forwarder's address path, not by
assert.Equal(t, tt.want == dispositionPeerFirst, resutil.SupportedRecordQtype(tt.qtype), // its record path, so they are deliberately outside
"only types a forwarder resolves may be sent to a peer") // SupportedRecordQtype and their disposition does not depend on it.
if tt.want == dispositionPeer {
assert.False(t, resutil.SupportedRecordQtype(tt.qtype),
"address types take the forwarder's address path")
return
} }
assert.Equal(t, tt.want == dispositionPeerFirst, resutil.SupportedRecordQtype(tt.qtype),
"only types a forwarder resolves may be sent to a peer")
}) })
} }
} }