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

systemd: Allow setting the transient hostname via DHCP #91232

Merged
merged 1 commit into from Jul 11, 2020

Conversation

primeos
Copy link
Member

@primeos primeos commented Jun 21, 2020

This permits using method_set_hostname but still denies
method_set_static_hostname. As a result DHCP clients can now always set
the transient hostname via the SetHostname method of the D-Bus interface
of systemd-hostnamed (org.freedesktop.hostname1.set-hostname).
If the NixOS option networking.hostName is set to an empty string (or
"localhost") the static hostname (kernel.hostname but NOT /etc/hostname)
will additionally be updated (this is intended).

From "man hostnamectl": The transient hostname is a fallback value
received from network configuration. If a static hostname is set, and is
valid (something other than localhost), then the transient hostname is
not used.

Fix #74847.

Note: It's possible to restrict access to the org.freedesktop.hostname1
interface using Polkit rules.

Motivation for this change
Examples
[root@quorra:/var/nixpkgs-test]# hostnamectl --pretty

[root@quorra:/var/nixpkgs-test]# hostnamectl --static
quorra

[root@quorra:/var/nixpkgs-test]# hostnamectl --transient
quorra

[root@quorra:/var/nixpkgs-test]# hostnamectl --transient set-hostname nixos
Could not set property: Changing system settings via systemd is not supported on NixOS.

[root@quorra:/var/nixpkgs-test]# # With this PR:

[root@quorra:/var/nixpkgs-test]# hostnamectl --transient set-hostname nixos

[root@quorra:/var/nixpkgs-test]# hostnamectl --pretty

[root@quorra:/var/nixpkgs-test]# hostnamectl --static
quorra

[root@quorra:/var/nixpkgs-test]# hostnamectl --transient
nixos

[root@quorra:/var/nixpkgs-test]# cat /etc/hostname
quorra

[root@quorra:/var/nixpkgs-test]# sysctl kernel.hostname
kernel.hostname = quorra

[root@quorra:/var/nixpkgs-test]# # With an empty hostname (networking.hostName):

[root@quorra:/var/nixpkgs-test]# cat /etc/hostname
cat: /etc/hostname: No such file or directory

[root@quorra:/var/nixpkgs-test]# sysctl kernel.hostname
kernel.hostname = quorra

[root@quorra:/var/nixpkgs-test]# hostnamectl --pretty

[root@quorra:/var/nixpkgs-test]# hostnamectl --static

[root@quorra:/var/nixpkgs-test]# hostnamectl --transient
quorra

[root@quorra:/var/nixpkgs-test]# hostnamectl --transient set-hostname test

[root@quorra:/var/nixpkgs-test]# cat /etc/hostname
cat: /etc/hostname: No such file or directory

[root@quorra:/var/nixpkgs-test]# sysctl kernel.hostname
kernel.hostname = test

[root@quorra:/var/nixpkgs-test]# hostnamectl --pretty

[root@quorra:/var/nixpkgs-test]# hostnamectl --static

[root@quorra:/var/nixpkgs-test]# hostnamectl --transient
test

[root@quorra:/var/nixpkgs-test]# hostname
test

[root@quorra:/var/nixpkgs-test]# hostnamectl --static set-hostname test
Could not set property: Changing system settings via systemd is not supported on NixOS.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • 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 nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

Copy link
Member

@lheckemann lheckemann left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@flokli flokli left a comment

Choose a reason for hiding this comment

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

I didn't test this yet, but this needs to:

  • direct staging
  • be added to the release notes
  • be added to the tests (running hostnamectl set-hostname --transient foobar on a host with a config allowing it, and one ensuring it fails on another)

@primeos primeos force-pushed the systemd-allow-transient-hostname branch from a6b762f to 1f0ef84 Compare June 24, 2020 15:18
@primeos primeos changed the base branch from master to staging June 24, 2020 15:18
@primeos
Copy link
Member Author

primeos commented Jun 24, 2020

I messed up the switch to staging, sorry for the automated review requrests, etc.

@flokli I don't need this change and only created the PR because no one else volunteered in #74847 and since I already proposed a few possible solutions. As I'm already busy enough with other stuff, I unfortunately won't have time (i.e. enough priority) for the second two items:

  • be added to the release notes
  • be added to the tests (running hostnamectl set-hostname --transient foobar on a host with a config allowing it, and one ensuring it fails on another)

But tbh: IMO these are indeed nice to have but also optional as I'd consider this a bug fix and not a new feature etc. If someone has time for the necessary changes: Feel free to re-use my commit.

@flokli
Copy link
Contributor

flokli commented Jun 24, 2020

No worries, thanks. Better to say you don't have the time than not replying :-)

I'll try to get to this over the weekend, and address the changes I requested by myself - it's probably not urgent to merge it before that.

@lheckemann
Copy link
Member

I don't think this needs a release notes entry, or is there a workflow this would break? All for a test though :)

@FRidh FRidh added this to WIP in Staging via automation Jul 2, 2020
@FRidh FRidh moved this from WIP to Needs review in Staging Jul 2, 2020
@flokli
Copy link
Contributor

flokli commented Jul 9, 2020

I didn't forget this, just haven't gotten to it yet. Will soon, promise!

This permits using method_set_hostname but still denies
method_set_static_hostname. As a result DHCP clients can now always set
the transient hostname via the SetHostname method of the D-Bus interface
of systemd-hostnamed (org.freedesktop.hostname1.set-hostname).
If the NixOS option networking.hostName is set to an empty string (or
"localhost") the static hostname (kernel.hostname but NOT /etc/hostname)
will additionally be updated (this is intended).

From "man hostnamectl": The transient hostname is a fallback value
received from network configuration. If a static hostname is set, and is
valid (something other than localhost), then the transient hostname is
not used.

Fix NixOS#74847.

Note: It's possible to restrict access to the org.freedesktop.hostname1
interface using Polkit rules.
@flokli flokli force-pushed the systemd-allow-transient-hostname branch from 1f0ef84 to 483dbe9 Compare July 10, 2020 22:06
@flokli
Copy link
Contributor

flokli commented Jul 10, 2020

I added a regression test to nixos/tests/systemd.nix, and things look good.

Couldn't yet test with a UseHostname=yes in networkd, or NetworkManagers setting of transient hostnames, but as they should go via the same dbus interface as hostnamectl, this should be fine. 👍

Staging automation moved this from Needs review to Ready Jul 10, 2020
@flokli flokli merged commit 12834b3 into NixOS:staging Jul 11, 2020
Staging automation moved this from Ready to Done Jul 11, 2020
@bjornfor
Copy link
Contributor

Thanks!

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

Successfully merging this pull request may close these issues.

NixOS doesn't support getting hostname from DHCP server
5 participants