diff --git a/net/dns/resolver/forwarder.go b/net/dns/resolver/forwarder.go index d1d880af4..90c06f3e7 100644 --- a/net/dns/resolver/forwarder.go +++ b/net/dns/resolver/forwarder.go @@ -1199,10 +1199,11 @@ func (f *forwarder) forwardWithDestChan(ctx context.Context, query packet, respo if len(resolvers) == 0 { resolvers = f.resolvers(domain) if len(resolvers) == 0 { + // No upstream resolver for this name isn't a forwarder failure: + // it's split DNS / a name we weren't asked to handle. Count it + // rather than raising dnsForwarderFailing, which is reserved for + // resolvers we found but couldn't reach. See tailscale/tailscale#19931. metricDNSFwdErrorNoUpstream.Add(1) - if f.acceptDNS { - f.health.SetUnhealthy(dnsForwarderFailing, health.Args{health.ArgDNSServers: ""}) - } f.logf("no upstream resolvers set, returning SERVFAIL") res, err := servfailResponse(query) diff --git a/net/dns/resolver/forwarder_test.go b/net/dns/resolver/forwarder_test.go index ebe4041a6..69a2f2ce0 100644 --- a/net/dns/resolver/forwarder_test.go +++ b/net/dns/resolver/forwarder_test.go @@ -1387,6 +1387,52 @@ func TestForwarderHealthOnContextExpiry(t *testing.T) { } } +// TestForwarderHealthNoUpstreamResolvers verifies that a query with no upstream +// resolver never raises dnsForwarderFailing, regardless of acceptDNS; that +// warning is reserved for resolvers we found but couldn't reach (see +// TestForwarderHealthOnContextExpiry). See tailscale/tailscale#19931. +func TestForwarderHealthNoUpstreamResolvers(t *testing.T) { + const domain = "no-resolver.example.com." + + for _, acceptDNS := range []bool{true, false} { + t.Run(fmt.Sprintf("acceptDNS=%v", acceptDNS), func(t *testing.T) { + request := makeTestRequest(t, domain, dns.TypeA, 0) + logf := tstest.WhileTestRunningLogger(t) + bus := eventbustest.NewBus(t) + netMon, err := netmon.New(bus, logf) + if err != nil { + t.Fatal(err) + } + + var dialer tsdial.Dialer + dialer.SetNetMon(netMon) + dialer.SetBus(bus) + + ht := health.NewTracker(bus) + fwd := newForwarder(logf, netMon, nil, &dialer, ht, nil) + fwd.acceptDNS = acceptDNS + // No routes are configured, so the forwarder has no upstream + // resolver for the query and returns SERVFAIL. + + rpkt := packet{ + bs: request, + family: "udp", + addr: netip.MustParseAddrPort("127.0.0.1:12345"), + } + // Buffered so the SERVFAIL response can be sent without a reader. + responseChan := make(chan packet, 1) + + if err := fwd.forwardWithDestChan(context.Background(), rpkt, responseChan); err != nil { + t.Fatalf("forwardWithDestChan: %v", err) + } + + if ht.IsUnhealthy(dnsForwarderFailing) { + t.Error("dnsForwarderFailing was raised for a query with no upstream resolver; want healthy") + } + }) + } +} + func TestResolversCustomScheme(t *testing.T) { t.Parallel() tests := []struct {