Skip to content
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

sysusers: add a treefile option in rpm-ostree #4680

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

HuijingHei
Copy link
Member

@HuijingHei HuijingHei commented Oct 30, 2023

Add sysusers option in treefile, if true,

  • turns off nss-altfiles support
  • disables the passwd / group files migration to /usr/lib

Xref to coreos/fedora-coreos-tracker#155 (comment)

@openshift-ci
Copy link

openshift-ci bot commented Oct 30, 2023

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@HuijingHei
Copy link
Member Author

Build fcos with coreos/fedora-coreos-config#2698, run fcos_groups and fcos_users passed.

[coreos-assembler]$ kola run -E src/config/tests/ ext.config.files.fcos_groups
=== RUN   ext.config.files.fcos_groups
--- PASS: ext.config.files.fcos_groups (42.60s)
PASS, output in tmp/kola/qemu-2023-10-30-0933-16127
[coreos-assembler]$ kola run -E src/config/tests/ ext.config.files.fcos_users
=== RUN   ext.config.files.fcos_users
--- PASS: ext.config.files.fcos_users (42.68s)
PASS, output in tmp/kola/qemu-2023-10-30-0934-16164

docs/treefile.md Outdated Show resolved Hide resolved
Copy link
Member

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So...no opposition to experimenting with this.

However my overall concern is that sysusers won't handle cases where we have a dynamic UID/GID included in the ostree commit/image content.

I think we started to add a check for this as part of the build system.

new_entities.add_passwd_content(rootfs.as_raw_fd(), "usr/lib/passwd")?;
new_entities.add_group_content(rootfs.as_raw_fd(), "usr/lib/group")?;
} else {
new_entities.add_passwd_content(rootfs.as_raw_fd(), "usr/etc/passwd")?;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this right though? I thought we'd be relying on systemd-sysusers creating the users/groups on firstboot?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe should add both? Do you mean with empty passwd / group (and no check-passwd/check-groups)? Maybe that is the final goal.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, I thought the goal was that we start with an empty passwd file if we were going all-in on sysusers.

db.add_group_content(rootfs.as_raw_fd(), "usr/etc/group")?;
db.add_group_content(rootfs.as_raw_fd(), "usr/lib/group")?;
if has_usrlib_passwd(&rootfs)? {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this change can land now if you want

@HuijingHei HuijingHei changed the title sysusers: add a treefile option in rpm-ostree to turn off nss-altfiles support and the passwd / group files migration to /usr/lib sysusers: add a treefile option in rpm-ostree Oct 31, 2023
docs/treefile.md Outdated Show resolved Hide resolved
Default is `false`, if `true`:
- turns off nss-altfiles support
- disables the passwd / group files migration to /usr/lib

Xref to coreos/fedora-coreos-tracker#155 (comment)
@HuijingHei
Copy link
Member Author

HuijingHei commented Nov 1, 2023

Tried to remove altfiles in composepost_nsswitch_altfiles with passing arg sysusers, but this does not work, find that if checking usr/etc/nsswitch.conf is symlink, then will not update the configuration file (refer to https://github.com/coreos/rpm-ostree/blob/main/rust/src/composepost.rs#L689), maybe transfer to authselect to create the configuration (see https://src.fedoraproject.org/rpms/authselect/blob/rawhide/f/authselect.spec#_315)?

Does this mean should remove altfiles in postprocess?

@cgwalters
Copy link
Member

I'm pretty sure we need to also figure out how to disable https://src.fedoraproject.org/rpms/systemd/blob/rawhide/f/systemd.spec#_940

Add an environment variable e.g.?

@cgwalters
Copy link
Member

cgwalters commented Nov 1, 2023

Also per discussion I'd say this treefile option should also enable RPMOSTREE_EXP_BRIDGE_SYSUSERS

Edit: And if we have this enabled we also ignore (or error out) if the static check-passwd is specified.

@HuijingHei
Copy link
Member Author

HuijingHei commented Nov 1, 2023

I'm pretty sure we need to also figure out how to disable https://src.fedoraproject.org/rpms/systemd/blob/rawhide/f/systemd.spec#_940

Add an environment variable e.g.?

Another problem is when installing a package which requires a systemd users, that will also create the user during pre-script, should we also disable it? for example tcpdump:

$ sudo rpm -q --scripts tcpdump
preinstall scriptlet (using /bin/sh):

# generated from tcpdump-sysusers.conf
getent group 'tcpdump' >/dev/null || groupadd -f -g '72' -r 'tcpdump' || :
if ! getent passwd 'tcpdump' >/dev/null; then
if ! getent passwd '72' >/dev/null; then
useradd -r -u '72' -g 'tcpdump' -d '/' -s '/usr/sbin/nologin' -c 'tcpdump' 'tcpdump' || :
else
useradd -r -g 'tcpdump' -d '/' -s '/usr/sbin/nologin' -c 'tcpdump' 'tcpdump' || :
fi
fi 

exit 0

@cgwalters
Copy link
Member

Tried to remove altfiles in composepost_nsswitch_altfiles with passing arg sysusers, but this does not work, find that if checking usr/etc/nsswitch.conf is symlink, then will not update the configuration file (refer to https://github.com/coreos/rpm-ostree/blob/main/rust/src/composepost.rs#L689), maybe transfer to authselect to create the configuration (see https://src.fedoraproject.org/rpms/authselect/blob/rawhide/f/authselect.spec#_315)?

Ah wow yes...messy. We have "dueling" sources of truth here. I guess for now we could copy the file to /etc and edit it there?

@HuijingHei HuijingHei force-pushed the systemd-sysusers branch 5 times, most recently from ef30c3b to 7401b70 Compare November 3, 2023 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants