-
Notifications
You must be signed in to change notification settings - Fork 352
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
rpmostree: use nss files from sysroot for tmpfiles #1976
Conversation
Jenkins, it's ok to test. |
@cgwalters could you please look on this and say what do you think about it? |
nssfiles = ["passwd", "group"] | ||
srcfiles = [util.getSysroot() + "/usr/lib/" + f for f in nssfiles] | ||
cp_args = ["-a", "-bS", ".orig"] + srcfiles + ["/etc"] | ||
self._safe_exec_with_redirect("cp", cp_args) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First, please have this code silently do nothing if the files don't exist, so things continue to work after we switch to sysusers.
Second...I think you need to temporarily concatenate them, otherwise the root
user won't be present, etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the review @cgwalters
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for your contribution!
Could you please make the change below before merge?
Could you please explain how we can test this? Also is there a bug you are trying to fix by this contribution? |
jenkins, test this please |
Executing systemd-tmpfiles with --root does not recognize users and groups from the root filesystem. To overcome this, copy the nss files from the sysroot over to the stage2 filesystem just before systemd-tmpfiles Signed-off-by: Yuval Turgeman <yturgema@redhat.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should also restore the files after the systemd-tmpfiles
call. There is a similar pull request at #1683. Anyway, this issue is reported at systemd/systemd#7032. The problem is that nss doesn't support the root
option. Have you filed a bug for nss?
Hi guys, thanks for the review, I didn't file a bug, but the issue that is reported in systemd/systemd#7032 seems to fit. This patch will concatenate nss files from /etc and /usr/lib from the image and in case one of them is missing it will fail silently as @cgwalters asked so it won't break the move to sysusers. As for testing, I created my own ostree image with custom file ownership under /var, repacked stage2 with this change, then created my own iso, and saw that the installation was successful with correct ownership under /var. Why do we need to restore the old files ? |
@yuvalturg, Anaconda doesn't run only in the stage2 environment. It can be installed on your system and used for installation to a directory or an image. Also, the users and groups of the stage2 environment can be set up with a kickstart file to configure login via ssh. Therefore, it is not desirable to change the users and groups of the host system. Even with restored files, the installation can fail before that and leave the host system changed. We recommend to file a bug to libc nss, so they can implement support for the root option and systemd-tmpfiles can use it. I am closing this pull request. |
Executing systemd-tmpfiles with --root does not recognize users and
groups from the root filesystem. To overcome this, copy the nss files
from the sysroot over to the stage2 filesystem just before
systemd-tmpfiles
Signed-off-by: Yuval Turgeman yturgema@redhat.com