-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
userdb: Fix use of memberships defined in .user/.group dropins and userdbctl outputs #32871
base: main
Are you sure you want to change the base?
Conversation
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. |
I don't think these failures are related. It seems like they fail a lot. |
Failing because the checking of .user and .group dropins by the membership iterator was never implemented and some of userdbctl's output formats don't use the iterator anyway.
This was broken because it was never actually implemented. Only .membership dropins were checked. This enhances the membership iterator to step through each user/group in these dropins before moving on to the next dropin. It does not deduplicate the users or groups as it iterates. Some users of the iterator do deduplication, but userdbctl's friendly output and the NSS module do not. Perhaps this is not an issue in practise. Fixes systemd#24381.
The friendly renderer was using a membership iterator while the JSON renderer was not. Fixes systemd#24381.
The friendly renderer was using a membership iterator while the classic renderer was not. Fixes systemd#24381.
Hmm, the intention was actually that programs that drop in .user/.group files would also drop in .membership files. But I guess we never properly documented that. i.e. for efficiency reasons it kinda sucks having to parse through so many files, hence the idea was to just have that separately, and expect that the tool populating these dirs would generate the right files. |
That's not really what I wanted to hear after such an effort. 😞 I'm wondering if there's a middle ground here, but I can't think of one. I must admit that I was surprised it reparses files as often as it does, but I figured I'd just follow suit. |
Would caching the users/groups against the dropin path and checking the mtime be an option? If not, I guess I can at least salvage the classic/JSON output fixes, along with the tests. |
Well, the stuff is in /run/ anyway, it's not really a slow disk, but a fast tmpfs, given how slow the rest of the passwd database access is it seems a bit unnecessary to cache things here. Maintaining a cache in NSS modules is a bit nasty too, because it will show up in people's memory profilers, and they tend to complain about that. |
I was thinking more in terms of time spent parsing than disk access. I thought the cached data would be held by the systemd-userwork processes rather than the module, but I've had another idea in any case. What if the .membership dropins served as the cache? It could read the .user/.group dropins and create .membership files under /run/userdb on the fly. These could be created as symlinks, both to differentiate them from manually-created .membership dropins, and to show which dropin led to their creation. That makes it work more like how it does now, but also ensures consistency between the different files. |
The membership iterator was broken because checking the .user and .group dropins was never actually implemented. Only .membership dropins were checked.
This enhances the membership iterator to step through each user/group in these dropins before moving on to the next dropin.
It does not deduplicate the users or groups as it iterates. Some users of the iterator do deduplication, but
userdbctl
's friendly output and the NSS module do not. Perhaps this is not an issue in practise.The JSON and classic outputs of
userdbctl
were also not consistent with the friendly output because they did not use the membership iterator. This is now fixed.Comprehensive tests are included and passing.
Fixes #24381.