fix(forward): disallow NOERROR in failover (#7622)

Previously the parsing logic in the forward plugin setup failed to
recognise when NOERROR was used as a failover RCODE criteria. The
check was in the wrong code branch. This PR fixes it and adds
validation tests. Also updates the plugin README.

Signed-off-by: Ville Vesilehto <ville@vesilehto.fi>
This commit is contained in:
Ville Vesilehto
2025-10-17 14:37:02 +03:00
committed by GitHub
parent 38c020941b
commit f4ab631ae4
3 changed files with 42 additions and 9 deletions

View File

@@ -100,7 +100,7 @@ forward FROM TO... {
As an upper bound for **MAX**, consider that each concurrent query will use about 2kb of memory.
* `next` If the `RCODE` (i.e. `NXDOMAIN`) is returned by the remote then execute the next plugin. If no next plugin is defined, or the next plugin is not a `forward` plugin, this setting is ignored
* `failfast_all_unhealthy_upstreams` - determines the handling of requests when all upstream servers are unhealthy and unresponsive to health checks. Enabling this option will immediately return SERVFAIL responses for all requests. By default, requests are sent to a random upstream.
* `failover` - By default when a DNS lookup fails to return a DNS response (e.g. timeout), _forward_ will attempt a lookup on the next upstream server. The `failover` option will make _forward_ do the same for any response with a response code matching an `RCODE` ( e.g. `SERVFAIL``REFUSED`). If all upstreams have been tried, the response from the last attempt is returned.
* `failover` - By default when a DNS lookup fails to return a DNS response (e.g. timeout), _forward_ will attempt a lookup on the next upstream server. The `failover` option will make _forward_ do the same for any response with a response code matching an `RCODE` ( e.g. `SERVFAIL``REFUSED`). `NOERROR` cannot be used. If all upstreams have been tried, the response from the last attempt is returned.
Also note the TLS config is "global" for the whole forwarding proxy if you need a different
`tls_servername` for different upstreams you're out of luck.

View File

@@ -320,16 +320,13 @@ func parseBlock(c *caddy.Controller, f *Forward) error {
toRcode := dns.StringToRcode
for _, rcode := range args {
var rc int
var ok bool
if rc, ok = toRcode[strings.ToUpper(rcode)]; !ok {
if rc == dns.RcodeSuccess {
return fmt.Errorf("NoError cannot be used in failover")
}
rc, ok := toRcode[strings.ToUpper(rcode)]
if !ok {
return fmt.Errorf("%s is not a valid rcode", rcode)
}
if rc == dns.RcodeSuccess {
return fmt.Errorf("NoError cannot be used in failover")
}
f.failoverRcodes = append(f.failoverRcodes, rc)
}

View File

@@ -503,3 +503,39 @@ func TestFailover(t *testing.T) {
}
}
}
func TestFailoverValidation(t *testing.T) {
cases := []struct {
name string
input string
wantError string
}{
{
name: "NoErrorDisallowed",
input: `forward . 127.0.0.1 {
failover NOERROR
}`,
wantError: "NoError cannot be used in failover",
},
{
name: "InvalidRcode",
input: `forward . 127.0.0.1 {
failover NOT_A_VALID_RCODE
}`,
wantError: "not a valid rcode",
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
c := caddy.NewTestController("dns", tc.input)
_, err := parseForward(c)
if err == nil {
t.Fatalf("expected error for %s, got nil", tc.name)
}
if !strings.Contains(err.Error(), tc.wantError) {
t.Fatalf("expected error to contain %q, got: %v", tc.wantError, err)
}
})
}
}