Fix the automatic override for networking.nameservers when services.bind is enabled. #29503
Conversation
It's better to break with a comment than to silently ignore a user's setting.
When the user specifies the networking.nameservers setting in the configuration file, it must take precedence over automatically derived settings. This patch prevents the service.bind and dnsmasq.resolveLocalQueries settings from overriding the users' settings. Also, when the user specifies a domain to search, it must be set in the resolver configuration, even if the user does not specify any nameservers.
The culprit was services.bind that made the resolver set to 127.0.0.1 and ignore the nameserver setting. This patch adds a flag to services.bind to override the nameserver to localhost. It defaults to true. There is also a warning when a user sets the nameserver and this setting. I added an explanation to the dnsmasq documentation to specify this behaviour.
|
@gwitmond, thanks for your PR! By analyzing the history of the files in this pull request, we identified @joachifm, @edolstra and @kampfschlaefer to be potential reviewers. |
| type = types.bool; | ||
| default = true; | ||
| description = '' | ||
| Whether dnsmasq should resolve local queries (i.e. add 127.0.0.1 to |
There was a problem hiding this comment.
Thanks for spotting. Updated.
|
Thanks! Merged and squashed your commits into one. In future PRs, please try to make commits self-contained and remove/squash commits whose code was removed or are merely small fixes to previous commits. Oh, and just noticed this is your contribution! Welcome on bord! 👋 |
|
@fpletz indeed, my first contribution to Nixos. Does this get merged with master later or do I need to submit a PR against master as well? |
|
Oh, damn, I didn't notice this PR was against 17.09. You should base all your PR against master. They will be picked into the release branches as needed. I'll pick it into master. |
When the user specifies the networking.nameservers setting in the configuration file, it must take precedence over automatically derived settings. The culprit was services.bind that made the resolver set to 127.0.0.1 and ignore the nameserver setting. This patch adds a flag to services.bind to override the nameserver to localhost. It defaults to true. Setting this to false prevents the service.bind and dnsmasq.resolveLocalQueries settings from overriding the users' settings. Also, when the user specifies a domain to search, it must be set in the resolver configuration, even if the user does not specify any nameservers. (cherry picked from commit 670b4e2) This commit was accidentally merged to 17.09 but was intended for master. This is the cherry-pick to master.
|
@fpletz, thanks for explaining the procedure. Will abide. |
|
This change breaks my name server: #29205 (review). @fpletz, can you please revert this merge? I don't think we should add this PR as-is. |
|
@peti Really? Have you tested it? |
…)" This reverts commit 670b4e2. The change added in this commit was controversial when it was originally suggested in #29205. Then that PR was closed and a new one opened, #29503, effectively circumventing the review process. I don't agree with this modification. Adding an option 'resolveLocalQueries' to tell the locally running name server that it should resolve local DNS queries feels outright nuts. I agree that the current state is unsatisfactory and that it should be improved, but this is not the right way.
|
I agree that the problem this PR tries to solve is real, but I don't agree with the proposed solution. |
…)" This reverts commit 670b4e2. The change added in this commit was controversial when it was originally suggested in #29205. Then that PR was closed and a new one opened, #29503, effectively circumventing the review process. I don't agree with this modification. Adding an option 'resolveLocalQueries' to tell the locally running name server that it should resolve local DNS queries feels outright nuts. I agree that the current state is unsatisfactory and that it should be improved, but this is not the right way. (cherry picked from commit 23a021d)
|
@peti, please don't take my change to a different PR as a way to avoid the review process. On the contrary. I closed the old one because I messed it up. Then, I linked both to each other for continuity. Apologies for causing the confusion. I took your remarks of not breaking existing setups to heart and removed the assertion. That would have broken your situation and forced you to comment out the I then changed the way Nixos handles Bind to do it identical to Dnsmasq. That leaves existing installations intact while lets me run Bind in a master-only role. Please read this patch again to check whether it would break your situation. |
Motivation for this change
The Nixos-networking code assumes that when
services.bindis enabled it means it runs in local resolver mode. This is not always true as I run Bind in master mode to host a domain and I pointnetworking.nameserversto an external name server.What happens is that my system cannot run Bind in master mode and resolve names at a remote name server.
Things done
This PR changes the Nixos networking configurator to check for a new attribute
resolveLocalQueriesto services.bind. It defaults totrueto keep the existing behaviour of setting 127.0.0.1 as nameserver in /etc/resolv.confWhen that flag is set to false, it doesn't set the resolver and the standard behaviour applies. I.e.: If
networking.nameserversis set, choose that, otherwise let resolveconf handle DHCP. etc.See for more discussion my previous PR: #29205
Closes #29202