plugin/forward: Forward NODATA responses to Next handler#8065
Conversation
Signed-off-by: AliveDevil <AliveDevil@users.noreply.github.com>
bf09f06 to
8e058b0
Compare
|
@AliveDevil Thanks for the contribution and sorry for the delay! Wondering if having an option to selectively control this behavior will make it backward compatible. Also, wondering if having a simple test will help coverage? |
|
Hi @yongtang, thanks for getting back to this :) retaining the current behavior when unspecified, continuing with any noerror upstream response: or adding a separate behavior switch Any preferred option here? For a unit test: I can look into providing one. |
|
@AliveDevil Thanks! I feel next_on_nodata might be easier, as its an extension with better backward compatibility? |
Instead of hijacking `NOERROR` Signed-off-by: AliveDevil <AliveDevil@users.noreply.github.com>
Signed-off-by: AliveDevil <AliveDevil@users.noreply.github.com>
|
Updated opening description, added tests. |
1. Why is this pull request needed and what does it do?
Forward can't handle NODATA-responses, which are technically error responses as well.
Using the NOERROR-case to allow forwarding to Next handler, when upstream returned
NODATA.Unfortunately, NODATA isn't a real rcode, but inferred from the answer section, the plugin code has been updated to inspect NODATA-answers as well.
2. Which issues (if any) are related?
See coredns/alternate#24 - original PR coredns/alternate#48.
Altered the original PR to not include the magic string "NODATA", with a "-1" RCode value.
3. Which documentation changes (if any) need to be made?
See changes to
plugin/forward/README.md, added new config option "next_on_nodata", which activates the new behavior.4. Does this introduce a backward incompatible change or deprecation?
After discussion a new backwards-compatible change was introduced.
Details
Current behavior for: ``` forward . a.b.c.d { next NOERROR } forward . d.e.f.0 ``` would _always_ respond with the response from `d.e.f.0`. The new behavior only uses `d.e.f.0` when `a.b.c.d` returned `NOERROR` with an empty answer set. In some sense it is a backward incompatible change, but I'd argue that this case is rather exotic, forwarding and discarding the answer would be quite wasteful.I've built this locally, and verified it works just like my previous patch to the alternate-plugin, which has been running here for two years unchanged.