Bug#1035733: debian -policy: packages must not use dpkg-divert to override default systemd configuraton files

Mon May 8 13:28:10 BST 2023

Package: debian-policy
X-Debbugs-CC: biebl at debian.org pkg-systemd-maintainers at lists.alioth.debian.org

It has come to my attention that there is one package in Debian using
dpkg-divert to mask a systemd configuration file (an udev rule).
Speaking as one of the maintainers, both upstream and downstream, I
find this greatly undesirable for several reasons that I will outline
later. Hence I would like to propose explicitly mentioning that dpkg-
divert must not be used for systemd configuration files (units, rules,
etc), and instead the supported workflow (drop-ins, masking, etc) must
be used, both by packages and administrators. This is already standard
practice, and again there is only one instance that needs correcting as
far as I understand, and I have already provided a bug and a MR for
that [1][2]. So the impact of this policy change should be minimal, and
it's mostly to ensure more such instances are accidentally added in the
future.

I have a draft policy update, that adds a paragraph to the dpkg-divert
section of the policy. It is attached here, and also available on Salsa
on my fork [3].

The full reasoning below is what I provided on the MR for the one
existing instance, and I'll copy it mostly unchanged as I hope it's
exhaustive enough. It uses the one existing instance I found as
concrete example. This is not intended to single out the maintainer or
assign blame, but merely to illustrate the point with a concrete and
real use case. Quoting from the MR:

One of the main goals behind the systemd (and its udev component)
project is to unify how the low-level userspace components of a Linux
distro work, so that the exact same mechanisms, patterns, behaviours
and interfaces apply to a multitude of use cases, implementations and
tools. A core part of this is the configuration system. The
configuration system supports a complex schema of main contents,
overrides, drop-ins, masking and aliasing. This system is used and
understood by all systemd components, across all Linux distributions,
with the same interface, look and feel, so that users can feel at home
and know how to work the system regardless of the vendor, and so that
programs can rely on a stable and common interface that doesn't have to
be endlessly customized depending on which vendor or distribution it is
used by. The concept of 'masking' a configuration is well understood,
ubiquitous and fully supported by all the tooling, including in input
_and_ output, and logging, and so on. By using the supported mechanism
for masking we ensure that there are no surprises for users, coders and
vendors. When an unsupported masking mechanism is used, as in this
case, the fact that the original item is masked is completely hidden to
the systemd components, and thus to the interface provided to the user.
This causes at best confusion and misunderstanding, and at worst bugs
that will inevitably fall on the systemd maintainers, causing increased
workload for an already over stretched team.

A simple and obvious example of what I am referring to is already
included in the git commit for this change. Consider what happens when
udev parses the vanilla configuration (ie: without amazon-ec2-utils
installed):

$ udevadm test /dev/cdrom 2>&1 | grep 60-cdrom_id
Reading rules file: /lib/udev/rules.d/60-cdrom_id.rules

The user/system/log is clearly notified that the file is parsed, there
are no errors, and thus is used.

Now consider what happens when the current version of amazon-ec2-utils
is installed:

$ udevadm test /dev/cdrom 2>&1 | grep 60-cdrom_id
Reading rules file: /lib/udev/rules.d/60-cdrom_id.rules

It looks exactly the same. But something extremely different is
actually happening, in fact the opposite! The file is empty, so the
vanilla rule is effectively masked, but nothing and nobody is notified
of this very important fact when udev is running. One would have to
query dpkg-divert to figure this out, but this is something that even
someone like me, who can reasonably consider myself a proficient Debian
user, would not think of looking at.

Now finally consider what happens if amazon-ec2-utils with my proposed
change is installed:

$ udevadm test /dev/cdrom 2>&1 | grep 60-cdrom_id
Skipping overridden file '/lib/udev/rules.d/60-cdrom_id.rules'.
Skipping empty file: /etc/udev/rules.d/60-cdrom_id.rules

There is an extremely clear, obvious and expected indication that the
original rule is ignored because it is overridden, _and_ that the
override is empty, as it's a masking operation. Now udev is a bit older
than the other components, so "masking" is not named explicitly as it
would be for example by systemctl, but it is the same operation and the
same result and the same interface. Something I should probably improve
a bit upstream, I will take a note of this.

Here's another example with systemctl and units. I have masked systemd-
homed.service via `systemctl mask` and this is what the interface shows
me when I check the status:

$ systemctl status -l --no-pager systemd-homed.service
○ systemd-homed.service
     Loaded: masked (Reason: Unit systemd-homed.service is masked.)
     Active: inactive (dead) since Sun 2023-05-07 18:59:51 BST; 7s ago
<snip>

Message is clear and unambiguous, I masked the unit so it will not run.
Here's what happens if I unmask it and instead dpkg-divert the vanilla
unit with a /bin/true no-op:

# systemctl status systemd-homed.service
○ systemd-homed.service
     Loaded: loaded (/usr/lib/systemd/system/systemd-homed.service; enabled; preset: enabled)
     Active: inactive (dead) since Sun 2023-05-07 18:59:51 BST; 4min 49s ago
<snip>

From the output, it's all good. The vanilla unit is enabled and should
run and provide its services on the next boot or when I next start it,
except it's not - it's been dpkg-diverted to a no-op, but there's no
indication there. This is poor UI, poor experience, poor integration.
And it's just the most basic, obvious example of what can go wrong: I
cannot guarantee that there won't be more, higher severity issues, but
I can certainly state that I will provide no support when they
inevitably surface, as this is not a supported mechanism.

The supported mechanism for masking, for both local admins and
packages, is masking via symlinks to /dev/null in /etc. And again I
must stress that packages configure things in /etc too, not only
administrators! This is fully expected and supported. For example, deb-
systemd-helper from init-system-helpers
(https://sources.debian.org/src/init-system-helpers/1.65.2/script/deb-systemd-helper/
) is used in Debian/Ubuntu and derivatives to enable/disable/mask/alias
units, and these changes are performed from auto generated (by
debhelper
https://sources.debian.org/src/debhelper/13.11.4/dh_installsystemd/)
maintainer script snippets not too dissimilar to what I have in this
MR. And this does not only apply to files that the packages own! It
also applies cross-packages. For example, by installing the 'dbus-
broker' package, an alias to dbus.service that effectively masks and
takes over dbus.service from the dbus-daemon package is installed in
/etc by the maintainer script:

$ ls -l /etc/systemd/system/dbus.service 
lrwxrwxrwx 1 root root 43 May  6 17:45 /etc/systemd/system/dbus.service
-> /usr/lib/systemd/system/dbus-broker.service

From `/var/lib/dpkg/info/dbus-broker.postinst`:

# Automatically added by dh_installsystemd/13.11.4
if [ "$1" = "configure" ] || [ "$1" = "abort-upgrade" ] || [ "$1" = "abort-deconfigure" ] || [ "$1" = "abort-remove" ] ; then
	# The following line should be removed in trixie or trixie+1
	deb-systemd-helper unmask 'dbus-broker.service' >/dev/null || true

	# was-enabled defaults to true, so new installations run enable.
	if deb-systemd-helper --quiet was-enabled 'dbus-broker.service'; then
		# Enables the unit on first installation, creates new
		# symlinks on upgrades if the unit file has changed.
		deb-systemd-helper enable 'dbus-broker.service' >/dev/null || true
	else
		# Update the statefile to add new symlinks (if any), which need to be
		# cleaned up on purge. Also remove old symlinks.
		deb-systemd-helper update-state 'dbus-broker.service' >/dev/null || true
	fi
fi
# End automatically added section

This is supported, intended and expected, as it's the correct mechanism
to do aliasing and overriding. No local user action is performed,
everything is done by the package being installed. In effect, the user
action IS installing the package.

Ensuring Debian packages use a supported, intended and expected
mechanism to manage and configure systemd files is fundamental for the
ability of the systemd downstream maintainers team to be able to fully
and effectively support Debian and its users. Deviating from the
expected path causes additional workload, unexpected incidents and
general discomfort for an already over-worked and small team, while
providing no tangible benefit, as the net result is the same.

[0] https://salsa.debian.org/cloud-team/amazon-ec2-utils/-/merge_requests/2
[1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1035667
[3] https://salsa.debian.org/bluca/policy/-/tree/systemd_overrides

-- 
Kind regards,
Luca Boccassi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: Forbid-using-dpkg-divert-for-systemd-configuration-files.patch
Type: text/x-patch
Size: 2411 bytes
Desc: not available
URL: <http://alioth-lists.debian.net/pipermail/pkg-systemd-maintainers/attachments/20230508/69da709b/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part
URL: <http://alioth-lists.debian.net/pipermail/pkg-systemd-maintainers/attachments/20230508/69da709b/attachment.sig>