[SSSD] [PATCH] Split the providers into separate subpackages

Lukas Slebodnik lslebodn at redhat.com
Fri Jun 7 16:24:48 UTC 2013


On (05/06/13 14:50), Jakub Hrozek wrote:
>On Thu, Oct 11, 2012 at 09:39:41AM -0400, Simo Sorce wrote:
>> On Thu, 2012-10-11 at 13:14 +0200, Jakub Hrozek wrote:
>> > 
>> > Hi,
>> > 
>> > the attached patch splits the previously monolithic sssd package into
>> > sssd-common that contains the deamon and the responders and
>> > per-provider
>> > packages such as sssd-ldap or sssd-ipa.
>> > 
>> > This split would benefit two parties:
>> >     1) security auditors who are often trying to find the smallest
>> > package
>> >     set including dependencies needed for the package to function.
>> > They
>> >     would be able to i.e. install sssd-ldap and not bother about
>> >     sssd-ipa or sssd-ad pulling in more dependencies.
>> >     2) 3rd party programs such as realmd or authconfig that would only
>> >     be able to require or install on demand the needed packages.
>> > 
>> > The patch addresses https://fedorahosted.org/sssd/ticket/1510 and must
>> > b
>> > applied on the two specfile patches I sent earlier (the thread subject
>> > included libsss_sudo).
>> 
>> Questions inline.
>> 
>
>Not even nine months after the initial submission, here comes a revised
>patch. I remember we had a discussion on IRC with Simo about this
>problem, but I'll reply to the thread.
>
>With the Radius provider patches on the list and requiring Samba bits in
>the last couple of releases, I think that splitting the providers is
>something we really should do.
>
>> > plain text
>> > document
>> > attachment
>> > (0001-Split-the-providers-into-separate-subpackages.patch)
>> > 
>> > From f59cfde30777a2c46f0ba2d6bd57dff62561851f Mon Sep 17 00:00:00 2001
>> > From: Jakub Hrozek <jhrozek at redhat.com>
>> > Date: Fri, 28 Sep 2012 09:21:18 +0200
>> > Subject: [PATCH] Split the providers into separate subpackages
>> > 
>> > ---
>> >  contrib/sssd.spec.in | 145
>> > ++++++++++++++++++++++++++++++++++++++++-----------
>> >  1 file changed, 115 insertions(+), 30 deletions(-)
>> > 
>> > diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in
>> > index
>> > e194245d166c7dee2f1988019b414e5fb47df2de..9b5a9b475544d245fbad0cbdd056ab55a0df4437 100644
>> > --- a/contrib/sssd.spec.in
>> > +++ b/contrib/sssd.spec.in
>> > @@ -45,17 +45,13 @@ BuildRoot: %(mktemp -ud
>> > %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)
>> >  Patch0001:  sssd-1.9-man-change-default-ccache.patch
>> >  
>> >  ### Dependencies ###
>> > -
>> > -Requires: libldb >= 0.9.3
>> > -Requires: libtdb >= 1.1.3
>> > +Conflicts: sssd < %{version}-%{release}
>> >  Requires: sssd-client%{?_isa} = %{version}-%{release}
>> > -Requires: libipa_hbac = %{version}-%{release}
>> > -Requires: libsss_idmap = %{version}-%{release}
>> > -Requires: cyrus-sasl-gssapi
>> > -Requires: keyutils-libs
>> > -Requires(post): initscripts chkconfig
>> > -Requires(preun):  initscripts chkconfig
>> > -Requires(postun): initscripts chkconfig
>> > +Requires: sssd-common = %{version}-%{release}
>> > +Requires: sssd-ldap = %{version}-%{release}
>> > +Requires: sssd-krb5 = %{version}-%{release}
>> > +Requires: sssd-ipa = %{version}-%{release}
>> > +Requires: sssd-ad = %{version}-%{release}
>> 
>> 
>> Doesn't this set of requires makes the split useless ?
>> If I read it correctly it means sssd will require all subpackages anyway
>> so you cannot pick and choose to install only one as you say the purpose
>> is in the mail message.
>> 
>
>The intent of the sssd package requiring all the dependencies is to make sure
>that any kickstart that specified "sssd" would get the whole set, because
>we can't currently know what functionality and which provider was used.
>
>To pick the minimal set for LDAP, you can run:
># yum install sssd-ldap
>for instance.
>
>> >  %global servicename sssd
>> >  %global sssdstatedir %{_localstatedir}/lib/sss
>> > @@ -126,6 +122,21 @@ the system and a pluggable backend system to
>> > connect to multiple different
>> >  account sources. It is also the basis to provide client auditing and
>> > policy
>> >  services for projects like FreeIPA.
>> >  
>> > +%package common
>> > +Summary: Common files for the SSSD
>> > +Group: Applications/System
>> > +License: GPLv3+
>> > +Requires: libldb >= 0.9.3
>> > +Requires: libtdb >= 1.1.3
>> > +Requires: sssd-client%{?_isa} = %{version}-%{release}
>> > +Requires(post): initscripts chkconfig
>> > +Requires(preun):  initscripts chkconfig
>> > +Requires(postun): initscripts chkconfig
>> > +Conflicts: sssd < %{version}-%{release}
>> > +
>> > +%description common
>> > +Common files for the SSSD.
>> > +
>> >  %package client
>> >  Summary: SSSD Client libraries for NSS and PAM
>> >  Group: Applications/System
>> > @@ -141,7 +152,7 @@ service.
>> >  Summary: Userspace tools for use with the SSSD
>> >  Group: Applications/System
>> >  License: GPLv3+
>> > -Requires: sssd = %{version}-%{release}
>> > +Requires: sssd-common = %{version}-%{release}
>> >  
>> >  %description tools
>> >  Provides userspace tools for manipulating users, groups, and nested
>> > groups in
>> > @@ -153,6 +164,61 @@ Also provides several other administrative tools:
>> >      * sss_seed which pre-creates a user entry for use in kickstarts
>> >      * sss_obfuscate for generating an obfuscated LDAP password
>> >  
>> > +%package ldap
>> > +Summary: The LDAP back end of the SSSD
>> > +Group: Applications/System
>> > +License: GPLv3+
>> > +Conflicts: sssd < %{version}-%{release}
>> > +Requires: cyrus-sasl-gssapi
>> > +Requires: sssd-common = %{version}-%{release}
>> > +Requires: libsss_idmap = %{version}-%{release}
>> > +
>> > +%description ldap
>> > +Provides the LDAP back end that the SSSD can utilize to fetch
>> > identity data
>> > +from and authenticate against an LDAP server.
>> > +
>> > +%package krb5
>> > +Summary: The Kerberos authentication back end for the SSSD
>> > +Group: Applications/System
>> > +License: GPLv3+
>> > +Conflicts: sssd < %{version}-%{release}
>> > +Requires: cyrus-sasl-gssapi
>> > +Requires: sssd-common = %{version}-%{release}
>> > +
>> > +%description krb5
>> > +Provides the Kerberos back end that the SSSD can utilize authenticate
>> > +against a Kerberos server.
>> > +
>> > +%package ipa
>> > +Summary: The IPA back end of the SSSD
>> > +Group: Applications/System
>> > +License: GPLv3+
>> > +Conflicts: sssd < %{version}-%{release}
>> > +Requires: sssd-common = %{version}-%{release}
>> > +Requires: sssd-ldap = %{version}-%{release}
>> > +Requires: sssd-krb5 = %{version}-%{release}
>> > +Requires: libipa_hbac = %{version}-%{release}
>> > +Requires: libsss_idmap = %{version}-%{release}
>> > +Requires: bind-utils
>> 
>> Does the ipa provider really need the sssd-ldap and sssd-krb5
>> subpackages ?
>> IIRC we statically compile the ldap and krb5 packages bits we need in
>> the ipa provider.
>> If you change this you probably want a require on cyrus-sasl-gssapi
>> here.
>> 
>> (if it is just for the ldap and krb child processes shouldn't we simply
>> keep those binaries in the sssd or sssd-common package ?)
>
>Yes, the intent was to make sure the ldap child and krb5 child processes
>are pulled in. But now that we switched to internal shared libraries, I
>think a better solution is to have the krb5_common internal shared
>library along with the ldap and krb5 child in a subpackage of its own
>and let the Kerberos-aware providers pull these in.
>
>> 
>> 
>> > +%description ipa
>> > +Provides the IPA back end that the SSSD can utilize to fetch identity
>> > data
>> > +from and authenticate against an IPA server.
>> > +
>> > +%package ad
>> > +Summary: The AD back end of the SSSD
>> > +Group: Applications/System
>> > +License: GPLv3+
>> > +Conflicts: sssd < %{version}-%{release}
>> > +Requires: sssd-common = %{version}-%{release}
>> > +Requires: sssd-ldap = %{version}-%{release}
>> > +Requires: sssd-krb5 = %{version}-%{release}
>> > +Requires: libsss_idmap = %{version}-%{release}
>> 
>> SAme questions as for the ipa subpackage
>> 
>> > +%description ad
>> > +Provides the Active Directory back end that the SSSD can utilize to
>> > fetch
>> > +identity data from and authenticate against an Active Directory
>> > server.
>> > +
>> >  %package -n libsss_idmap
>> >  Summary: FreeIPA Idmap library
>> >  Group: Development/Libraries
>> > @@ -205,7 +271,7 @@ used by Python applications.
>> >  Summary: A library to allow communication between SUDO and SSSD
>> >  Group: Development/Libraries
>> >  License: LGPLv3+
>> > -Requires: sssd = %{version}-%{release}
>> > +Requires: sssd-ldap = %{version}-%{release}
>> >  Requires(post): /sbin/ldconfig
>> >  Requires(postun): /sbin/ldconfig
>> 
>> why libsss_idmap would require the sssd-ldap subpakage ?
>
>I think this was a mass-replace bug, fixed.
>
>> 
>> Simo.

1. I like idea of divided subpackages. If someone wants only ldap backend, he
needn't install samba-libs (and its dependencies)

2. There isn't any rpmlint warnings.

I tested yum upgrade upgrade with installed sssd and freeipa-client.
New packages were installed for dependencies:
   sssd-ad sssd-common sssd-ipa sssd-krb5 sssd-krb5-common sssd-ldap
Everything worked as expected.

Then I decided to remove sssd-ad:
   yum remove sssd-ad
and packeges "freeipa-client, sssd" were also removed.
I was little bit confused, because I didn't want to remove sssd
and sssd replied to getent command after packages "freeipa-client, sssd" were
removed.

I think, that other users may be also confused with this situation.
Then I looked to the patch and I found out, that:
  --sssd is only "meta package",which require all backedns subpackages
  --sssd doesn't contain any useful files
  --everything important is in package sssd-common.


Maybe we should update package description of sssd and sssd-common.
I hope that system administrators relies on output of "yum info"
and there isn't it very well explained.

Summary:
Everything works well, but I was little bit confused.

Any other opinions?

One nitpick inline


>From 4498f26c78fcba9da4aaa2e8fb34734106f5227d Mon Sep 17 00:00:00 2001
>From: Jakub Hrozek <jhrozek at redhat.com>
>Date: Wed, 5 Jun 2013 12:53:30 +0200
>Subject: [PATCH 3/3] rpm: Split providers into separate subpackages
>
>https://fedorahosted.org/sssd/ticket/1510
>
>This patch splits the previously monolithic sssd package into sssd-common
>that contains the deamon and the responders and per-provider packages
>such as sssd-ldap or sssd-ipa.
>
>This split would benefit two parties:
>1) security auditors who are often trying to find the smallest package
>   set including dependencies needed for the package to function.
>   They would be able to i.e. install sssd-ldap and not bother
>   about sssd-ipa or sssd-ad pulling in more dependencies.
>2) 3rd party programs such as realmd or authconfig
>   that would only be able to require or install on demand the
>   needed packages.
>---
> contrib/sssd.spec.in | 188 +++++++++++++++++++++++++++++++++++++++------------
> 1 file changed, 145 insertions(+), 43 deletions(-)
>
>diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in
>index d59f684400b6eab4f65a93552ae73a85445ac3e2..ddc364f878bf859116ed79a8aa4f55963d90dfc8 100644
>--- a/contrib/sssd.spec.in
>+++ b/contrib/sssd.spec.in
>@@ -56,30 +56,13 @@ Patch0001:  sssd-1.9-man-change-default-ccache.patch
> 
> ### Dependencies ###
> 
>-Requires: libldb >= 0.9.3
>-Requires: libtdb >= 1.1.3
>-Requires: sssd-client%{?_isa} = %{version}-%{release}
>-Requires: libipa_hbac = %{version}-%{release}
>-Requires: libsss_idmap = %{version}-%{release}
>+Conflicts: sssd < %{version}-%{release}
>+Requires: sssd-common = %{version}-%{release}
>+Requires: sssd-ldap = %{version}-%{release}
>+Requires: sssd-krb5 = %{version}-%{release}
>+Requires: sssd-ipa = %{version}-%{release}
>+Requires: sssd-ad = %{version}-%{release}
> Requires: python-sssdconfig = %{version}-%{release}
>-Requires: cyrus-sasl-gssapi
>-%if (0%{?use_systemd} == 1)
>-Requires(post): systemd-units systemd-sysv
>-Requires(preun): systemd-units
>-Requires(postun): systemd-units
>-%else
>-Requires(post): initscripts chkconfig
>-Requires(preun):  initscripts chkconfig
>-Requires(postun): initscripts chkconfig
>-%endif
>-
>-### Provides ###
>-Provides: libsss_sudo = %{version}-%{release}
>-Obsoletes: libsss_sudo < %{version}-%{release}
>-Provides: libsss_sudo-devel = %{version}-%{release}
>-Obsoletes: libsss_sudo-devel < %{version}-%{release}
>-Provides: libsss_autofs = %{version}-%{release}
>-Obsoletes: libsss_autofs < %{version}-%{release}
> 
> %global servicename sssd
> %global sssdstatedir %{_localstatedir}/lib/sss
>@@ -155,6 +138,35 @@ the system and a pluggable backend system to connect to multiple different
> account sources. It is also the basis to provide client auditing and policy
> services for projects like FreeIPA.
> 
>+%package common
>+Summary: Common files for the SSSD
>+Group: Applications/System
>+License: GPLv3+
>+Requires: libldb >= 0.9.3
>+Requires: libtdb >= 1.1.3
>+Requires: sssd-client%{?_isa} = %{version}-%{release}
>+Conflicts: sssd < %{version}-%{release}
>+%if (0%{?use_systemd} == 1)
>+Requires(post): systemd-units systemd-sysv
>+Requires(preun): systemd-units
>+Requires(postun): systemd-units
>+%else
>+Requires(post): initscripts chkconfig
>+Requires(preun):  initscripts chkconfig
>+Requires(postun): initscripts chkconfig
>+%endif
>+
>+### Provides ###
>+Provides: libsss_sudo = %{version}-%{release}
>+Obsoletes: libsss_sudo < %{version}-%{release}
>+Provides: libsss_sudo-devel = %{version}-%{release}
>+Obsoletes: libsss_sudo-devel < %{version}-%{release}
>+Provides: libsss_autofs = %{version}-%{release}
>+Obsoletes: libsss_autofs < %{version}-%{release}
>+
>+%description common
>+Common files for the SSSD.
>+
> %package client
> Summary: SSSD Client libraries for NSS and PAM
> Group: Applications/System
>@@ -170,7 +182,7 @@ service.
> Summary: Userspace tools for use with the SSSD
> Group: Applications/System
> License: GPLv3+
>-Requires: sssd = %{version}-%{release}
>+Requires: sssd-common = %{version}-%{release}
> 
> %description tools
> Provides userspace tools for manipulating users, groups, and nested groups in
>@@ -190,6 +202,73 @@ BuildArch: noarch
> %description -n python-sssdconfig
> Provides python files for manipulation SSSD and IPA configuration files.
> 
>+%package ldap
>+Summary: The LDAP back end of the SSSD
>+Group: Applications/System
>+License: GPLv3+
>+Conflicts: sssd < %{version}-%{release}
>+Requires: sssd-common = %{version}-%{release}
>+Requires: libsss_idmap = %{version}-%{release}
>+Requires: sssd-krb5-common = %{version}-%{release}
>+
>+%description ldap
>+Provides the LDAP back end that the SSSD can utilize to fetch identity data
>+from and authenticate against an LDAP server.
>+
>+%package krb5-common
>+Summary: SSSD helpers needed for Kerberos and GSSAPI authentication
>+Group: Applications/System
>+License: GPLv3+
>+Conflicts: sssd < %{version}-%{release}
>+Requires: cyrus-sasl-gssapi
>+Requires: sssd-common = %{version}-%{release}
>+Requires: sssd-krb5-common = %{version}-%{release}
            ^^^^^^^^^^^^^^^^
Package sssd-krb5-common requires itself?
>+
>+%description krb5-common
>+Provides helper processes that the LDAP and Kerberos back ends can use for
>+Kerberos user or host authentication.
>+
>+%package krb5
>+Summary: The Kerberos authentication back end for the SSSD
>+Group: Applications/System
>+License: GPLv3+
>+Conflicts: sssd < %{version}-%{release}
>+Requires: sssd-common = %{version}-%{release}
>+Requires: sssd-krb5-common = %{version}-%{release}
>+
>+%description krb5
>+Provides the Kerberos back end that the SSSD can utilize authenticate
>+against a Kerberos server.
>+
>+%package ipa
>+Summary: The IPA back end of the SSSD
>+Group: Applications/System
>+License: GPLv3+
>+Conflicts: sssd < %{version}-%{release}
>+Requires: sssd-common = %{version}-%{release}
>+Requires: sssd-krb5-common = %{version}-%{release}
>+Requires: libipa_hbac = %{version}-%{release}
>+Requires: libsss_idmap = %{version}-%{release}
>+Requires: bind-utils
>+
>+%description ipa
>+Provides the IPA back end that the SSSD can utilize to fetch identity data
>+from and authenticate against an IPA server.
>+
>+%package ad
>+Summary: The AD back end of the SSSD
>+Group: Applications/System
>+License: GPLv3+
>+Conflicts: sssd < %{version}-%{release}
>+Requires: sssd-common = %{version}-%{release}
>+Requires: sssd-krb5-common = %{version}-%{release}
>+Requires: libsss_idmap = %{version}-%{release}
>+Requires: bind-utils
>+
>+%description ad
>+Provides the Active Directory back end that the SSSD can utilize to fetch
>+identity data from and authenticate against an Active Directory server.
>+
> %package -n libsss_idmap
> Summary: FreeIPA Idmap library
> Group: Development/Libraries
>@@ -386,7 +465,11 @@ touch $RPM_BUILD_ROOT/%{mcpath}/group
> %clean
> rm -rf $RPM_BUILD_ROOT
> 
>-%files -f sssd.lang
>+%files
>+%defattr(-,root,root,-)
>+%doc COPYING
>+
>+%files common -f sssd.lang
> %defattr(-,root,root,-)
> %doc COPYING
> %doc src/examples/sssd-example.conf
>@@ -398,8 +481,6 @@ rm -rf $RPM_BUILD_ROOT
> %endif
> 
> %dir %{_libexecdir}/%{servicename}
>-%{_libexecdir}/%{servicename}/krb5_child
>-%{_libexecdir}/%{servicename}/ldap_child
> %{_libexecdir}/%{servicename}/proxy_child
> %{_libexecdir}/%{servicename}/sssd_be
> %{_libexecdir}/%{servicename}/sssd_nss
>@@ -409,17 +490,7 @@ rm -rf $RPM_BUILD_ROOT
> %{_libexecdir}/%{servicename}/sssd_ssh
> %{_libexecdir}/%{servicename}/sssd_sudo
> 
>-# RHEL 5 is too old to support the PAC responder
>-%if !0%{?is_rhel5}
>-%{_libexecdir}/%{servicename}/sssd_pac
>-
>-%endif
>-
> %dir %{_libdir}/%{name}
>-%{_libdir}/%{name}/libsss_ad.so
>-%{_libdir}/%{name}/libsss_ipa.so
>-%{_libdir}/%{name}/libsss_krb5.so
>-%{_libdir}/%{name}/libsss_ldap.so
> %{_libdir}/%{name}/libsss_proxy.so
> %{_libdir}/%{name}/libsss_simple.so
> 
>@@ -427,7 +498,6 @@ rm -rf $RPM_BUILD_ROOT
> %{_libdir}/%{name}/libsss_child.so
> %{_libdir}/%{name}/libsss_crypt.so
> %{_libdir}/%{name}/libsss_debug.so
>-%{_libdir}/%{name}/libsss_krb5_common.so
> %{_libdir}/%{name}/libsss_ldap_common.so
> %{_libdir}/%{name}/libsss_util.so
> 
>@@ -448,7 +518,6 @@ rm -rf $RPM_BUILD_ROOT
> %ghost %attr(0644,root,root) %verify(not md5 size mtime) %{mcpath}/group
> %attr(755,root,root) %dir %{pipepath}
> %attr(755,root,root) %dir %{pubconfpath}
>-%attr(755,root,root) %dir %{pubconfpath}/krb5.include.d
> %attr(700,root,root) %dir %{pipepath}/private
> %attr(750,root,root) %dir %{_var}/log/%{name}
> %attr(711,root,root) %dir %{_sysconfdir}/sssd
>@@ -459,10 +528,6 @@ rm -rf $RPM_BUILD_ROOT
> %{_datadir}/sssd/sssd.api.conf
> %{_datadir}/sssd/sssd.api.d
> %{_mandir}/man5/sssd.conf.5*
>-%{_mandir}/man5/sssd-ipa.5*
>-%{_mandir}/man5/sssd-ad.5*
>-%{_mandir}/man5/sssd-krb5.5*
>-%{_mandir}/man5/sssd-ldap.5*
> %{_mandir}/man5/sssd-simple.5*
> %{_mandir}/man5/sssd-sudo.5*
> %{_mandir}/man8/sssd.8*
>@@ -472,6 +537,43 @@ rm -rf $RPM_BUILD_ROOT
> %{python_sitearch}/pysss.so
> %{python_sitearch}/pysss_murmur.so
> 
>+%files ldap
>+%defattr(-,root,root,-)
>+%doc COPYING
>+%{_libdir}/%{name}/libsss_ldap.so
>+%{_mandir}/man5/sssd-ldap.5*
>+
>+%files krb5-common
>+%defattr(-,root,root,-)
>+%doc COPYING
>+%{_libdir}/%{name}/libsss_krb5_common.so
>+%{_libexecdir}/%{servicename}/ldap_child
>+%{_libexecdir}/%{servicename}/krb5_child
>+
>+%files krb5
>+%defattr(-,root,root,-)
>+%doc COPYING
>+%{_libdir}/%{name}/libsss_krb5.so
>+%{_mandir}/man5/sssd-krb5.5*
>+
>+%files ipa
>+%defattr(-,root,root,-)
>+%doc COPYING
>+# RHEL 5 is too old to support the PAC responder
>+%if !0%{?is_rhel5}
>+%{_libexecdir}/%{servicename}/sssd_pac
>+%endif
>+
>+%attr(755,root,root) %dir %{pubconfpath}/krb5.include.d
>+%{_libdir}/%{name}/libsss_ipa.so
>+%{_mandir}/man5/sssd-ipa.5*
>+
>+%files ad
>+%defattr(-,root,root,-)
>+%doc COPYING
>+%{_libdir}/%{name}/libsss_ad.so
>+%{_mandir}/man5/sssd-ad.5*
>+
> %files client -f sssd_client.lang
> %defattr(-,root,root,-)
> %doc src/sss_client/COPYING src/sss_client/COPYING.LESSER
>-- 
>1.8.2.1
>

>_______________________________________________
>sssd-devel mailing list
>sssd-devel at lists.fedorahosted.org
>https://lists.fedorahosted.org/mailman/listinfo/sssd-devel




More information about the sssd-devel mailing list