Bug#877512: slapd: enabled systemd integration and convert /etc/default/slapd (untested patches)

[Andreas Henriksson]

Hello Ryan Tandy,

I'm attaching a(n extended) set of patches that tries to implement some
of the suggestions...

On Wed, Jun 28, 2023 at 01:03:33PM -0700, Ryan Tandy wrote:
> On Wed, Jun 28, 2023 at 08:48:14PM +0200, Moritz Mühlenhoff wrote:
> > OTOH, moving to the systemd unit might also be a good opportunity to reduce
> > some complexity? Looking at slapd.default shipped with the current package
> > SLAPD_SENTINEL_FILE, SLAPD_PIDFILE and SLAPD_NO_START are all settings which
> > are no longer relevant with a systemd unit or can equally be achieved with
> > commands built-in to systemd (e.g. systemctl mask).
> 
> Yes, definitely meaning to review and drop at least those, as well as
> migrating /var/run/slapd to tmpfiles.d.

This is still TODO. Also consider `RuntimeDirectory=slapd` instead of
tmpfiles.
https://www.freedesktop.org/software/systemd/man/systemd.exec.html#RuntimeDirectory=

This probably needs the use of User=, Group= though to work correctly,
rather than passing `-u $SLAPD_USER -g $SLAPD_GROUP` on command line.
https://www.freedesktop.org/software/systemd/man/systemd.exec.html#User=

So using tmpfiles might be easier after all (but you'd have to duplicate
the user and group in two different config files).

> 
> > Then there's a handful of settings which IMHO probably very people actually modify
> > (SLAPD_USER, SLAPD_USER, SLAPD_CONF, SLAPD_SERVICES) and which folks wanting
> > to modify can always tweak with a local unit override/dropins.
> 
> Hmm. So on upgrade I suppose we would want to automatically migrate those
> settings to a drop-in? That actually sounds doable; such a drop-in would
> probably not have to be a conffile.

This should be implemented in the attached patch set, but still equally
untested as before (because I'm not actually a slapd user myself).

> 
> SLAPD_CONF is also used (at least) by anyone who still uses a slapd.conf
> file instead of cn=config. Using -f or -F depending on what SLAPD_CONF
> points to was the main reason I assumed we'd need a wrapper script. But that
> could also be replaced by a drop-in.
> 
> > The most commonly used option is probably SLAPD_OPTIONS, which could also
> > be read via an EnvironmentFile from /etc/default.
> 
> Right. Although if that's the only thing still being consumed, I'd be
> tempted to just let it go too. :)

I noticed the upstream service file had (RHism?)
EnvironmentFile=/etc/sysconfig/slapd
which I initially was tempted to just replace with
EnvironmentFile=/etc/default/slapd
but I decided not to do that, because I think it would be way to
confusing for users if some options of /etc/default/slapd worked
but not others.
(Rather than leaving it pointing to a non-existant file, maybe
EnvironmentFile= setting shipped by upstream should be patched out.)

I'm willing to bet that despite documenting the conversion in slapd.NEWS
most users will still miss this information and be confused why
their settings is not honored when editing /etc/default/slapd after
the upgrade. I've seen cases where injecting a banner at the top of the
file isn't even noticed (because people dive directly to the option they
want to fiddle with, rather than read the file from top to bottom) --
and it's also problematic to modify a conffile policy-wise.
Removing /etc/default/slapd via rm_conffile would be an option, but then
non-systemd users would be displeased. (But I can't help to wonder if
the "confused users" group is much bigger and more relevant than
the group of remaining debian sysvinit users.)

> 
> Another (lower priority) thing I meant to look into is the sd_notify(3)
> support. Enabling that means changing the service type and adding the -d
> flag to stop slapd from detaching.

The upstream service file already has `-d 0`... am I missing something?

> 
> Thanks for the input, it really does help. :)
> 
> Ryan

Hope the attached patches gives some inspiration for who ever wants to
take this all the way.

Regards,
Andreas Henriksson
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Enable-systemd-integration.patch
Type: text/x-diff
Size: 1722 bytes
Desc: not available
URL: <http://alioth-lists.debian.net/pipermail/pkg-openldap-devel/attachments/20230629/701b9b13/attachment.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-debian-rules-Work-around-configure.ac-not-finding-pk.patch
Type: text/x-diff
Size: 769 bytes
Desc: not available
URL: <http://alioth-lists.debian.net/pipermail/pkg-openldap-devel/attachments/20230629/701b9b13/attachment-0001.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-debian-slapd.install-Use-correct-systemd-system-unit.patch
Type: text/x-diff
Size: 605 bytes
Desc: not available
URL: <http://alioth-lists.debian.net/pipermail/pkg-openldap-devel/attachments/20230629/701b9b13/attachment-0002.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-debian-slapd.postinst-Generate-overrides-from-etc-de.patch
Type: text/x-diff
Size: 2651 bytes
Desc: not available
URL: <http://alioth-lists.debian.net/pipermail/pkg-openldap-devel/attachments/20230629/701b9b13/attachment-0003.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-debian-slapd.NEWS-Add-entry-about-etc-default-slapd-.patch
Type: text/x-diff
Size: 1042 bytes
Desc: not available
URL: <http://alioth-lists.debian.net/pipermail/pkg-openldap-devel/attachments/20230629/701b9b13/attachment-0004.patch>