Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix systemd resolved nsswitch loading and clearly state NSS module's dependency on nscd #26967

Merged

Conversation

florianjacob
Copy link
Contributor

Motivation for this change

nss-resolve fails silently to get loaded when resolved is enabled because of a typo.

I spent way too much time investigating the source DNS issue leading to this typo because the first thing I did was disabling nscd to exclude cashing problems, therefore the note that nscd is required for any not-included modules to be loaded.

I thought about only adding the dynamic modules only if their respective service like resolved and nscd is enabled to make potential issues more obvious when looking at /etc/nsswitch.conf which then would not contain modules that it cannot load, but I'm not sure about that and wanted to discuss it first.

Things done
  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • Linux
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@florianjacob
Copy link
Contributor Author

When resolved / networkd is enabled, you can test this with ping gateway, which is a synthesized dns entry from resolved. Won't work currently, but after applying this patch.

@bjornfor
Copy link
Contributor

Regarding the 2nd commit, can we add an assertion instead of a comment (or in addition)?

@florianjacob
Copy link
Contributor Author

@bjornfor Any idea for a good assertion? Failing an assertion would be nice, but just „if this option is used, assert nscd.enable = true;“ would currently mean all of nsswitch would always fail if nscd is disabled, because it is using this option itself when adding the systemd nss modules:

https://github.com/florianjacob/nixpkgs/blob/9495cef848ba41574983b7e2183bc16d646d42e7/nixos/modules/config/nsswitch.nix#L83

We could make that conditional on whether nscd is enabled, then. But then this still would not fail if a user enables resolved but disables nscd. It would guard nix module authors from using this option without nscd, though, which is better that not doing it.

@bjornfor
Copy link
Contributor

@florianjacob: No idea. I didn't look at the code. I was just hoping there was some NixOS option that could be checked at evaluation time.

@florianjacob
Copy link
Contributor Author

I think what we can do would be:

  • Add assertion to nssModules to depend on nscd
  • Add systemd modules to nssModules only if nscd is enabled
  • Add another assertion in nsswitch for all dynamic modules, like “assert = resolved.enable || … || ldap.enable => nscd.enable”

Does that seem sensible? It operates on the assumption that any of these services are not really useful without nsswitch.conf, but I guess that's the case.

I think I'll try to write the code for that.

@florianjacob florianjacob force-pushed the fix-systemd-resolved-nsswitch-loading branch from 9495cef to 8c89834 Compare June 30, 2017 00:21
@florianjacob florianjacob changed the title Fix systemd resolved nsswitch loading Fix systemd resolved nsswitch loading and clearly state NSS module's dependency on nscd Jun 30, 2017
this had the effect of not being able to load nss-resolve
and falling back to dns module in all cases.
@florianjacob florianjacob force-pushed the fix-systemd-resolved-nsswitch-loading branch from 8c89834 to 4a9fa18 Compare June 30, 2017 00:41
@florianjacob
Copy link
Contributor Author

@bjornfor Assertions added! Should work that way. 😄

Now also includes a proposal commit to only add external stuff to nsswitch.conf when nscd is enabled.

@florianjacob florianjacob force-pushed the fix-systemd-resolved-nsswitch-loading branch from 4a9fa18 to e370e97 Compare June 30, 2017 00:44
@florianjacob
Copy link
Contributor Author

Identified @Mic92 as potential reviewer from the nsswitch.nix history. Maybe you could take a look, or recommend somebody else? 😃

@Mic92 Mic92 merged commit 1266c8f into NixOS:master Jun 30, 2017
@Mic92
Copy link
Member

Mic92 commented Jun 30, 2017

Thanks.

@florianjacob
Copy link
Contributor Author

@Mic92 Thanks for your time. 👍

@florianjacob florianjacob deleted the fix-systemd-resolved-nsswitch-loading branch June 30, 2017 08:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants