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.

Sign up for GitHub

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

use systemd-sysusers in RPMs #6807

Closed
wants to merge 1 commit into from
Closed

use systemd-sysusers in RPMs #6807

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/actions/build-sssd-srpm/action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ runs:
tar -cvzf "$name.tar.gz" --transform "s,^,$name/," *
cp contrib/sssd.spec.in ./sssd.spec
cp contrib/sssd.sysusers ./sssd.sysusers
sed -iE "s/@PACKAGE_NAME@/sssd/g" ./sssd.spec
sed -iE "s/@PACKAGE_VERSION@/${{ steps.sanitize.outputs.version }}/g" ./sssd.spec
Expand Down
13 changes: 10 additions & 3 deletions contrib/sssd.spec.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ Summary: System Security Services Daemon
License: GPLv3+
URL: https://github.com/SSSD/sssd/
Source0: %{url}/archive/%{version}/%{name}-%{version}.tar.gz
Source1: sssd.sysusers
Copy link
Member

@alexey-tikhonov alexey-tikhonov Aug 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Is this file has to be maintained in downstream dist-git like spec-file?

Copy link
Contributor

@justin-stephenson justin-stephenson Aug 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, similar to https://src.fedoraproject.org/rpms/munge/c/dd0498ebb79605ee6a18100db30b07fddfaf31e4?branch=rawhide


### Patches ###
# Place your patches here:
Expand Down Expand Up @@ -188,7 +189,8 @@ Requires: (sssd-nfs-idmap = %{version}-%{release} if libnfsidmap)
Requires: libsss_idmap = %{version}-%{release}
Requires: libsss_certmap = %{version}-%{release}
%if 0%{?rhel}
Requires(pre): shadow-utils
BuildRequires: systemd-rpm-macros
Copy link
Member

Choose a reason for hiding this comment

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

Why is it BuildRequires (and not Requires)?

Copy link
Contributor

Choose a reason for hiding this comment

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

This, and several of your other concerns, are explained in https://docs.fedoraproject.org/en-US/packaging-guidelines/UsersAndGroups/#_allocation_strategies

Copy link
Member

Choose a reason for hiding this comment

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

Guide says "add a BuildRequires" but doesn't actually explain.

%{?sysusers_requires_compat}
%endif
%{?systemd_requires}

Expand Down Expand Up @@ -701,6 +703,8 @@ do
cat $subpackage.lang
done

install -D -p -m 0644 %{SOURCE1} %{buildroot}%{_sysusersdir}/sssd.conf
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 name - 'sssd.conf' - mandated anywhere?
It's confusing because it matches name of SSSD config file.
Would it be possible to use different name for a file with sysuser definition?

Copy link
Contributor

Choose a reason for hiding this comment

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

I checked _sysusersdir location (/usr/lib/sysusers.d/) and I only saw one exception, which I guess makes it valid to use another extension

Copy link
Contributor

Choose a reason for hiding this comment

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

Per man sysusers.d

CONFIGURATION DIRECTORIES AND PRECEDENCE
       Each configuration file shall be named in the style of package.conf or package-part.conf. The second variant should be used when it is desirable to make it easy to override just this part of configuration.


%files
%license COPYING

Expand Down Expand Up @@ -804,6 +808,10 @@ done
%{_datadir}/systemtap/tapset/sssd.stp
%{_datadir}/systemtap/tapset/sssd_functions.stp
%{_mandir}/man5/sssd-systemtap.5*
%if 0%{?rhel} >= 9 || 0%{?fedora}
Copy link
Member

@alexey-tikhonov alexey-tikhonov Aug 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Either I misunderstand something, or this condition is inconsistent:

  • install above is unconditional.
  • config is packaged for rhel >= 9 and fedora
  • below users are created for rhel only (any version)

Frankly I wouldn't touch RHEL8 and RHEL9 and only bring this to F40+/RHEL10

%{_sysusersdir}/sssd.conf
%endif


%if 0%{?rhel}
%files polkit-rules
Expand Down Expand Up @@ -1003,8 +1011,7 @@ done

%if 0%{?rhel}
%pre common
getent group sssd >/dev/null || groupadd -r sssd
getent passwd sssd >/dev/null || useradd -r -g sssd -d / -s /sbin/nologin -c "User for sssd" sssd
%sysusers_create_compat %{SOURCE1}
%endif

%post common
Expand Down
1 change: 1 addition & 0 deletions contrib/sssd.sysusers
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
u sssd - "User for sssd" / /sbin/nologin
jonathanspw marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

At the beginning of the spec-file we have:

# define SSSD user
%if 0%{?rhel}
%global sssd_user sssd
...

Would it be possible to avoid copy-paste of 'sssd' user name?

Loading