diff --git a/plugin/forward/forward.go b/plugin/forward/forward.go index f95c77bdb..bfe810738 100644 --- a/plugin/forward/forward.go +++ b/plugin/forward/forward.go @@ -10,6 +10,7 @@ import ( "errors" "net" "net/http" + "slices" "sync/atomic" "time" @@ -126,6 +127,7 @@ func (f *Forward) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg } fails := 0 + failoverAttempts := 0 var span, child ot.Span var upstreamErr error span = ot.SpanFromContext(ctx) @@ -236,16 +238,11 @@ func (f *Forward) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg // Check if we have a failover Rcode defined, check if we match on the code tryNext := false - for _, failoverRcode := range f.failoverRcodes { - // if we match, we continue to the next upstream in the list - if failoverRcode == ret.Rcode { - if fails < len(f.proxies) { - tryNext = true - } - } + if slices.Contains(f.failoverRcodes, ret.Rcode) { + failoverAttempts++ + tryNext = failoverAttempts < len(f.proxies) } if tryNext { - fails++ continue } diff --git a/plugin/forward/forward_test.go b/plugin/forward/forward_test.go index c5fb9cad0..d472f11ea 100644 --- a/plugin/forward/forward_test.go +++ b/plugin/forward/forward_test.go @@ -5,6 +5,7 @@ import ( "fmt" "net" "strings" + "sync/atomic" "testing" "time" @@ -238,3 +239,68 @@ func TestForward_NextOnNodata(t *testing.T) { }) } } + +func TestForwardFailoverStopsAfterAllUpstreams(t *testing.T) { + var first atomic.Int32 + var second atomic.Int32 + + s1 := dnstest.NewMultipleServer(func(w dns.ResponseWriter, r *dns.Msg) { + first.Add(1) + + m := new(dns.Msg) + m.SetRcode(r, dns.RcodeServerFailure) + w.WriteMsg(m) + }) + defer s1.Close() + + s2 := dnstest.NewMultipleServer(func(w dns.ResponseWriter, r *dns.Msg) { + second.Add(1) + + m := new(dns.Msg) + m.SetRcode(r, dns.RcodeServerFailure) + w.WriteMsg(m) + }) + defer s2.Close() + + c := caddy.NewTestController("dns", fmt.Sprintf(`forward . %s %s { + policy sequential + failover SERVFAIL + }`, s1.Addr, s2.Addr)) + + fs, err := parseForward(c) + if err != nil { + t.Fatal(err) + } + + f := fs[0] + if err := f.OnStartup(); err != nil { + t.Fatal(err) + } + defer f.OnShutdown() + + req := new(dns.Msg) + req.SetQuestion("example.org.", dns.TypeA) + + rec := dnstest.NewRecorder(&test.ResponseWriter{}) + + ctx, cancel := context.WithTimeout( + context.Background(), + 200*time.Millisecond, + ) + defer cancel() + + if _, err := f.ServeDNS(ctx, rec, req); err != nil { + t.Fatalf("Expected the last SERVFAIL response, got error: %v", err) + } + + if rec.Rcode != dns.RcodeServerFailure { + t.Fatalf("Expected SERVFAIL, got %d", rec.Rcode) + } + + if got := first.Load(); got != 1 { + t.Errorf("Expected first upstream to be queried once, got %d", got) + } + if got := second.Load(); got != 1 { + t.Errorf("Expected second upstream to be queried once, got %d", got) + } +}