pop-os / distinst

Installer Backend
GNU Lesser General Public License v3.0
221 stars 43 forks source link

fix: Stop populating hostname in /etc/hosts by default #322

Closed jacobgkau closed 1 year ago

jacobgkau commented 1 year ago

systemd-resolved resolves localhost and the hostname by default; libnss-myhostname additionally takes care of this. Creating the file leads to problems if hostnamectl is used to update the hostname later, since the file is not updated by that tool. Ref: https://github.com/pop-os/systemd/issues/5

I wasn't sure if there might still be a reason to keep the function around. I removed it since this was the only place it was used, but engineering feedback is welcome on that.

leviport commented 1 year ago

Looks like this is having some CI issues

mmstick commented 1 year ago

Just needs to remove the line with hosts => "error writing hosts"; a couple lines down.

leviport commented 1 year ago

I did an install with this distinst version and it still created an /etc/hosts file. I'll try again in case it was something I screwed up while I was multitasking.

leviport commented 1 year ago

Yeah it's still creating that file. Here's my process:

Guessing either I'm doing something wrong, or something else is creating that file during install.

jacobgkau commented 1 year ago

I'm still trying to figure out where the default etc/hosts is coming from. However, the new default that's still being created does not contain the hostname, only variants of localhost:

image

Therefore, this would still fix https://github.com/pop-os/systemd/issues/5, since that issue is about changing the hostname.

If we wanted the file to be empty, we could probably keep the function in distinst but just have it overwrite the file with an empty one instead of what's currently used. However, I'm not sure if that's great form (any tools that check the existence of /etc/hosts would think it's there and potentially assume it contains common configuration, for example.) On the other hand, we could add a comment about why it's empty, similar to what we have in /etc/apt/sources.list by default.

jacobgkau commented 1 year ago

Ok, I've located where the new default comes from. It's the netbase package:

image

This can be confirmed by removing /etc/hosts and seeing it get recreated with sudo dpkg --purge --force-all netbase and then sudo apt install netbase.

We don't currently package that, so it would be best to either leave this new default configuration, or continue to populate an /etc/hosts file with distinst to prevent netbase from creating one.

jacobgkau commented 1 year ago

Upon doing further research, I found out that even though netbase comes from Debian, the netcfg part of Debian's installer actually creates a default /etc/hosts file on installation that overrides the netbase version, similar to our installer. Debian does attempt to keep their contents of netbase's and netcfg's default file in sync, but the netcfg version includes a hostname, since the installer collects that information.

It seems that the additional IPv6 entries in the netbase version are not actually standardized (for example, ff02::1 is a standardized IP address to reach all IPv6 nodes, but there's no standard saying that ip6-allnodes should resolve to that IP address.) Some other distros and operating systems use different names for some of the same IPs.

I also found that Arch Linux ships an /etc/hosts file with only comments for new installations. (The commit to do that was six years ago; I've got an install slightly newer than that that doesn't have that hosts file, but I've got an install from last year that does have it.)

jacobgkau commented 1 year ago

After a lot of consideration, it's probably simplest to reduce the diff here and just remove the line that's causing a problem (the hostname). If we're going to keep the function to write the file around anyway, then we might as well still put localhost in it to try and reduce the impact of configuration corner cases, potential future systemd/nss-myhostname bugs, etc.

If anyone disagrees, I can update the file to include comments instead, or we can remove the function like I did before and trust the Debian/Ubuntu packagers to continue creating it the way they currently do.

Edit: I did go ahead and add a comment explaining why the hostname's not present (and that localhost is optional).

jacobgkau commented 1 year ago

The line breaks where I used \n weren't working, fixed that now. (The string was previously marked raw, but I don't see why that has to be the case-- maybe it was to prevent certain hostnames from causing problems before, but that's no longer relevant.)

image