mkosi
mkosi copied to clipboard
Implement support for generated files ownership management.
Implement support for generated file ownership management:
- Add a new
--map-permissions
option to requestmkosi
to change ownership of generated files and directories to eitherSUDO_UID
orPKEXEC_UIC
(depending on which one is available). - Bind mount using
X-mount.idmap
option to remap root owner to the owner of the directory. All the files subsequently created bymkosi
(as root) will be owned but the owner for the mounted directory. - systemd-nspawn is invoked with new
rootidmap
bind option (https://github.com/systemd/systemd/pull/24467).
Mount's X-mount.idmap
feature is not yet merged (https://github.com/util-linux/util-linux/pull/1661). If X-mount.idmap
is not supported, mount
will silently ignore it and mount the directory without it. This means --map-permissions
could fail silently. What would be the best way to handle this? Check for mount
's version to explicitly fail when --map-permission
is called?
I've only had a quick look for now, but some thoughts:
- Maybe I missed it, but I don't see a reasoning for making the mount executable configurable. Could you maybe explain that a bit? Since only the executable would be switched, it would still need to adhere to the CLI of regular
mount
, no? - I'm not opposed to chowning the more persistent directories - this is something that people have asked for in the past -, but I'd rather not stuff the user running mkosi into the args. Stuff like
int(os.getenv("SUDO_UID") or os.getenv("PKEXEC_UID") or '0')
could just be a function or possibly include the chown call into somechown_to_running_user
. - The idmapping option for nspawn should check whether the nspawn version is recent enough, because most users will probably not run the most recent systemd releases.
I've only had a quick look for now, but some thoughts:
Maybe I missed it, but I don't see a reasoning for making the mount executable configurable. Could you maybe explain that a bit? Since only the executable would be switched, it would still need to adhere to the CLI of regular
mount
, no?
Making mount executable configurable was useful to me during development, I thought I might as well include it in the PR. nspawn executable is configurable already (MKOSI_NSPAWN_EXECUTABLE
), I assume the benefits are identical.
I'm not opposed to chowning the more persistent directories - this is something that people have asked for in the past -, but I'd rather not stuff the user running mkosi into the args. Stuff like
int(os.getenv("SUDO_UID") or os.getenv("PKEXEC_UID") or '0')
could just be a function or possibly include the chown call into somechown_to_running_user
.
What you suggest would be to only be aware of the running user when chown
is actually called, instead of storing it within MkosiArgs
, is that what you mean?
The idmapping option for nspawn should check whether the nspawn version is recent enough, because most users will probably not run the most recent systemd releases.
I agree, although rootidmap
is not yet in any nspawn release, so I don't know yet which version number to check for. Ideally, we should check for mount
version too, as not all versions support X-mount.idmap
.
Making mount executable configurable was useful to me during development, I thought I might as well include it in the PR. nspawn executable is configurable already (
MKOSI_NSPAWN_EXECUTABLE
), I assume the benefits are identical.
Ah, that was to have an escape hatch when working on nspawn itself. Yeah, sure. I'm ±0 on this change.
What you suggest would be to only be aware of the running user when
chown
is actually called, instead of storing it withinMkosiArgs
, is that what you mean?
Yes, mkosi is either run by root or those environment variables are in the env during the whole of mkosi's run.
I agree, although
rootidmap
is not yet in any nspawn release, so I don't know yet which version number to check for. Ideally, we should check formount
version too, as not all versions supportX-mount.idmap
.
I see, then this being merged is contingent on that being merged first and ending in a release.
What you suggest would be to only be aware of the running user when
chown
is actually called, instead of storing it withinMkosiArgs
, is that what you mean?Yes, mkosi is either run by root or those environment variables are in the env during the whole of mkosi's run.
Sounds good to me,
I agree, although
rootidmap
is not yet in any nspawn release, so I don't know yet which version number to check for. Ideally, we should check formount
version too, as not all versions supportX-mount.idmap
.I see, then this being merged is contingent on that being merged first and ending in a release.
This PR is composed of 4 commits, the 3rd one requires https://github.com/systemd/systemd/pull/24467 and the 4th one requires https://github.com/util-linux/util-linux/pull/1661. The 1st and 2nd one can be merged.
This pull request introduces 1 alert when merging 1a85b959c4707e3bd941de643200499e70deb537 into b7cfe85a262ceeacbf77772e00050abd39a4bbad - view on LGTM.com
new alerts:
- 1 for Unused local variable
Rebased on main branch, fixed the merge conflict and added a check in Use rootidmap with nspawn's bind mounts to ensure we use systemd-nspawn 252 or later.
Last commit (Use idmapping for package cache bind mount) still needs to wait for X-mount.idmap
support to be merged.
Also, the MOUNT_EXECUTABLE stuff can be dropped, #1190 allows you to use --extra-search-paths instead to override the mount executable.
instead of the mount binary stuff I think we should people towards using ExtraSearchPaths= instead. Or maybe add mkosi.path/ or so as special dir people can link files or dirs into that shall be preferred over the system ones.