Allow users to be omitted from sysusers.d/basic.conf #32796
Conversation
There were two parts, for templated and non-templated files, and they were more different than they should be.
Fedora (and derivates) is moving towards a model where all users are defined via a sysusers file. To implement this cleanly, rpm (the package manager itself) will take care of precreating users and groups for any package which caries a sysusers file [1]. This is implemented by generating a virtual Provides line at package build time that contains the details of the sysusers config line, and then using creating appropriate entries when the package with such provides is installed. The Provides can also be used by other packages in a Requires to ensure a user or group is available. This brings us the problem: systemd contains duplicates of sysusers config that is also provided by the 'setup' package that are expected to be available "always". We want the Provides to only be generated by 'setup', for clarity and because we don't want to acidentally pull in the full systemd stack from some other package if it defines a dependency on one of the common groups. The precise list of users that should be defined "elsewhere" will vary between distros and over time. This patch adds a config option that contains a list of ids to skip, allowing the distro maintainer to remove any that are duplicates. The conditionalization is only applied to 'basic.conf', i.e. ids that could reasonably be defined elsewhere. Our own like 'systemd-journal*' are defined unconditionally. (The solution is a bit involved, but I think that it'll allow us to handle the following issue quite well: systemd adds new uids and groups occasionally, and if we just skipped 'basic.conf', things would be subtly broken. OTOH, with the current approach, things will "just work". We will build systemd with --suppress-sysusers=<everything that is known to be defined in setup>. We can add the id to the other place, if desired, and in the transition period the only bad thing that might happen is a duplicate definition.) [1] https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/message/IKWECWMBWN2IDKLHK3Q2TZKVSVFTXUNA/ The generated file has some empty lines. This is a bit ugly, but it seems that we cannot make jinja2 not do that.
github-actions
bot
added
build-system
meson
please-review
|
Important An -rc1 tag has been created and a release is being prepared, so please note that PRs introducing new features and APIs will be held back until the new version has been released. |
| @@ -317,6 +317,8 @@ option('systemd-resolve-uid', type : 'integer', value : 0, | |||
| description : 'soft-static allocation for the systemd-resolve user') | |||
| option('systemd-timesync-uid', type : 'integer', value : 0, | |||
| description : 'soft-static allocation for the systemd-timesync user') | |||
| option('suppress-sysusers', type : 'string', | |||
| description : 'list of users/groups to omit in sysusers config') | |||
There was a problem hiding this comment.
Maybe mention that the list is comma-separated.
Also, better to mention that the conf name basic.conf.
There was a problem hiding this comment.
Actually, things will work fine if the list is whitespace separate too. I had this info initially in the description, and I removed it, because it's not really needed (we have comma-separate lists pretty much everywhere, so people will just use that), and meson doesn't deal with long descriptions very well. So I think it's better to keep this implicit.
Also, I don't want to mention basic.conf, because it's possible that we could use the same option for groups in other files later on (e.g. for some groups needed by udev).
|
Last time we tried to change basic.conf for debian it didn't go too well: #25186 |
This approach is different. It allows a distro to move group definitions one-by-one to a different package. It's up to the distro maintainers to opt-in to removal individual groups if they are defined in a different place. It could be misused, but actually the goal is almost the opposite of #25186, because those groups are always defined in every Fedora/CentOS/RHEL installation, we just want to get rid of the duplicate definition in systemd.rpm.
I still hope both can go in ;) It's simple enough. |
It's a new option, so should be material for v257 no? As you'll need to wait for Lennart anyway, and I wanted to do RC2 today. The first commit on the other hand doesn't need to wait. |
|
I think it's totally fine to merge this at any point. It is a new build option, but it only matters if used. If not used, the whole series has no effect, apart from some whitespace changes in |
|
I am kinda afraid that people will start to think that these groups really are optional. I don't think they should think that though. I mean, iirc debian patches out a couple of groups because of internal politics, but I realy don't want to send the wrong message here, suggesting that this was cool and everything would fine that way. i.e. I do think distros should just stop deviating on trivialities like basic groups. And for me it's much more appropriate to force distros to patch upstream if they want to keep deviating on this, rather than us accomodate for them. (and yes, I think we should even drop the ability to name the nobody user/group differently. that debian still uses "nogroup", and we help them with that is just bad) i mean, i can see the benefit for fedora, i.e. accept the reason "we define them all, just elsewhere". I just fear that people will misunderstand these config option as "i like my bikeshed green, let's not define this one group and name this other group pink". wouldn't it make more sense for fedora to simply not include the whole basic.conf in their rpms if they define them elsewhere? How does the rpm thing work btw? does it synthesize a proper sysusers.conf file and write it somewhere? where precisely? I hope not /etc and not /var? (because this would break boots with just /usr/ further) |
|
in fedora, would this have the effect that certain of these basic groups will only exist if some package listing them in their deps asks for them? that would suck though, I am pretty sure these basic groups should just unconditionally exist on any system. |
poettering
added
the
needs-reporter-feedback ❓
That is the fallback option. I wanted to do things as in this PR because of two reasons: right now some groups that are defined in basic.conf are missing from the sysusers definitions provided by setup.rpm. I filed a pull request and this will most likely get resolved pretty soon, but before that's done, I would keep the definitions in systemd.rpm. The second reason is that systemd will add new groups at some point, and then we have the annoying situation where we need to synchronize with setup.rpm to provide those groups before we can use them in systemd. But if y'all think this patch is too complicated and not worth the trouble, I can make do with some hack in the systemd package downstream, it's not a big issue.
You cannot install a system without |
|
I split out the first patch. |
keszybz
removed
the
needs-reporter-feedback ❓
|
i might be more sympathetic to this if we'd change the messaging a bit. i.e. rather than calling this "omit" or "suppress" maybe call it "elsewhere" and make sure that all comments say that this is about "providing equivalent sysusers.d definitions elsewhere", to at least underline that these definitions really should exist, but it's ok if they do exist elsewhere. |
poettering
added
reviewed/needs-rework 🔨
|
I decided to take a simpler approach. This patch is rather messy. In particular, with the jinja2 templating, we end up with a bunch of empty lines when conditional content is skipped. Maybe I'm doing it wrong, but anyway, the formatting doesn't look ideal. So instead, I'll just kill the file altogether in Fedora. We added all groups that systemd needs to setup.rpm (https://pagure.io/setup/pull-request/50, https://src.fedoraproject.org/rpms/setup/pull-request/10), so they are "always defined". The removal of |
github-actions
bot
removed
the
reviewed/needs-rework 🔨
No description provided.