Skip to content

Commit

Permalink
Revert "nixos: add option for bind to not resolve local queries (#29503
Browse files Browse the repository at this point in the history
…)"

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.
  • Loading branch information
peti committed Sep 23, 2017
1 parent f7411b8 commit 23a021d
Show file tree
Hide file tree
Showing 4 changed files with 3 additions and 14 deletions.
4 changes: 1 addition & 3 deletions nixos/modules/config/networking.nix
Expand Up @@ -9,9 +9,7 @@ let
cfg = config.networking;
dnsmasqResolve = config.services.dnsmasq.enable &&
config.services.dnsmasq.resolveLocalQueries;
bindResolve = config.services.bind.enable &&
config.services.bind.resolveLocalQueries;
hasLocalResolver = bindResolve || dnsmasqResolve;
hasLocalResolver = config.services.bind.enable || dnsmasqResolve;

resolvconfOptions = cfg.resolvconfOptions
++ optional cfg.dnsSingleRequest "single-request"
Expand Down
9 changes: 0 additions & 9 deletions nixos/modules/services/networking/bind.nix
Expand Up @@ -151,15 +151,6 @@ in
";
};

resolveLocalQueries = mkOption {
type = types.bool;
default = true;
description = ''
Whether bind should resolve local queries (i.e. add 127.0.0.1 to
/etc/resolv.conf, overriding networking.nameserver).
'';
};

};

};
Expand Down
2 changes: 1 addition & 1 deletion nixos/modules/services/networking/dnsmasq.nix
Expand Up @@ -42,7 +42,7 @@ in
default = true;
description = ''
Whether dnsmasq should resolve local queries (i.e. add 127.0.0.1 to
/etc/resolv.conf overriding networking.nameservers).
/etc/resolv.conf).
'';
};

Expand Down
2 changes: 1 addition & 1 deletion nixos/modules/tasks/network-interfaces-scripted.nix
Expand Up @@ -105,7 +105,7 @@ let
''
# Set the static DNS configuration, if given.
${pkgs.openresolv}/sbin/resolvconf -m 1 -a static <<EOF
${optionalString (cfg.domain != null) ''
${optionalString (cfg.nameservers != [] && cfg.domain != null) ''
domain ${cfg.domain}
''}
${optionalString (cfg.search != []) ("search " + concatStringsSep " " cfg.search)}
Expand Down

1 comment on commit 23a021d

@gwitmond
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@peti, Please don't feel that my closing of the old PR as a way to bypass the review process.

I never had that intention. I closed it because I made a mess of the original PR. Besides, I linked both PR's to the other for continuity.

I took your comment of not breaking existing systems to heart and changed the way the Bind-service gets handled to be identical to Dnsmasq. The breaking assertion I removed. In fact my patch makes sure that your system (and those of others) don't break while letting me run Bind in a non-resover role.

Please read my patch again and check if it would break on your configuration.

Please sign in to comment.