Skip to content

nixos/virtualisation: increase priority for libvirt NSS modules#322980

Merged
flokli merged 1 commit intoNixOS:masterfrom
michaelfranzl:fix-nss-libvirt
Jun 28, 2024
Merged

nixos/virtualisation: increase priority for libvirt NSS modules#322980
flokli merged 1 commit intoNixOS:masterfrom
michaelfranzl:fix-nss-libvirt

Conversation

@michaelfranzl
Copy link
Copy Markdown
Contributor

When services.resolved is enabled, then resolve [!UNAVAIL=return] is added to system.nssDatabases.hosts with priority 501, which prevents lower-priority NSS modules from running unless systemd-resolved is not available.

Quoting from man nss-resolve:

To activate the NSS module, add "resolve [!UNAVAIL=return]" to the line
starting with "hosts:" in /etc/nsswitch.conf. Specifically, it is
recommended to place "resolve" early in /etc/nsswitch.conf's "hosts:"
line. It should be before the "files" entry, since systemd-resolved
supports /etc/hosts internally, but with caching. To the contrary, it
should be after "mymachines", to give hostnames given to local VMs and
containers precedence over names received over DNS. Finally, we
recommend placing "dns" somewhere after "resolve", to fall back to
nss-dns if systemd-resolved.service is not available.

Note that the man page (just) recommends "early" and means with this "before the 'files' and 'dns' entries". It does not insist on being first or excluding other modules.

For this reason, libvirt NSS modules should run before the resolve module. They should come right next to mymachines because both are conceptually very similar -- they resolve local VMs/containers.

Since the data source of the libvirt NSS modules are local plain text files (see source code of the libvirt NSS module), no performance impact is expected form this raise of priorities.

Other NSS modules in NixOS also explicitly set their priority, which is why this change increases consistency.

Fixes #322022

Description of changes

The NSS modules libvirt and libvirt_guest now run even when services.resolved is enabled.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

When `services.resolved` is enabled, then `resolve [!UNAVAIL=return]`
is added to `system.nssDatabases.hosts` with priority 501,
which prevents lower-priority NSS modules from running
unless systemd-resolved is not available.

Quoting from `man nss-resolve`:

> To activate the NSS module, add "resolve [!UNAVAIL=return]" to the line
> starting with "hosts:" in /etc/nsswitch.conf. Specifically, it is
> recommended to place "resolve" early in /etc/nsswitch.conf's "hosts:"
> line. It should be before the "files" entry, since systemd-resolved
> supports /etc/hosts internally, but with caching. To the contrary, it
> should be after "mymachines", to give hostnames given to local VMs and
> containers precedence over names received over DNS. Finally, we
> recommend placing "dns" somewhere after "resolve", to fall back to
> nss-dns if systemd-resolved.service is not available.

Note that the man page (just) recommends "early" and means with this
"before the 'files' and 'dns' entries". It does not insist on being
first or excluding other modules.

For this reason, libvirt NSS modules should run before the `resolve`
module. They should come right next to `mymachines` because both are
conceptually very similar -- they resolve local VMs/containers.

Since the data source of the libvirt NSS modules are local
plain text files (see source code of the libvirt NSS module),
no performance impact is expected form this raise of priorities.

Other NSS modules in NixOS also explicitly set their priority, which is
why this change increases consistency.

Fixes NixOS#322022
@michaelfranzl michaelfranzl requested a review from a team as a code owner June 27, 2024 18:45
@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Jun 27, 2024
@NixOSInfra NixOSInfra added the 12.first-time contribution This PR is the author's first one; please be gentle! label Jun 27, 2024
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Jun 27, 2024
Copy link
Copy Markdown
Contributor

@jchv jchv left a comment

Choose a reason for hiding this comment

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

LGTM. Seems to work, of course, and seems to be closer to the general recommendation anyways. I ran the libvirtd test and it still passes as expected.

@flokli
Copy link
Copy Markdown
Member

flokli commented Jun 28, 2024

Please include the test for the NSS lookup into the libvirt VM test, so we can make sure this doesn't regress unnoticed.

@jchv
Copy link
Copy Markdown
Contributor

jchv commented Jun 28, 2024

@flokli If I understand what you're asking for, I already did that in #268887. If we wanted to make sure this PR doesn't regress, the libvirtd test would need to also have resolved enabled on the host side.

@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one person. label Jun 28, 2024
@flokli
Copy link
Copy Markdown
Member

flokli commented Jun 28, 2024

Fair enough. Running two tests just to that is probably a bit overkill. I assume you'll notice once it regresses ;-)

@flokli flokli merged commit 0ee2243 into NixOS:master Jun 28, 2024
@michaelfranzl
Copy link
Copy Markdown
Contributor Author

Please include the test for the NSS lookup into the libvirt VM test, so we can make sure this doesn't regress unnoticed.

@flokli I did now in #323728

FYI @jchv

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 12.approvals: 1 This PR was reviewed and approved by one person. 12.first-time contribution This PR is the author's first one; please be gentle!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

libvirtd NSS module not working (nscd, nsncd, libnss, libc)

5 participants