Merge ~sergiodj/ubuntu/+source/sssd:bug1900642-condpathexists-hirsute into ubuntu/+source/sssd:ubuntu/devel

[Sergio Durigan Junior]

This is a MP to address bug 1900642 on hirsute. It includes a patch that comes directly from upstream, which accepted my solution (after some tweaking). This MP basically replaces an old approach to solve the problem by a new one, which is more complete.

In a nutshell, what the new patch does is to include some conditions in the sssd.service file that are responsible for checking whether the user as (a) a file named /etc/sssd/sssd.conf, or (b) some configuration snippet under /etc/sssd/conf.d/. If either is true, then the service can be started.

It's important to mention that these conditions will only be added to the service file if sssd has been compiled without --enable-files-domain support. This is true for the Debian/Ubuntu sssd packages.

[Bryce Harrington (bryce)]

Looks good. I reviewed bug report (and SRU template), and past work on this bug. I did not test out the behavior but assume you've tested it thoroughly already. The approach looks correct and I verified the systemd logic against documentation.

review: Approve

[Sergio Durigan Junior (sergiodj)]

Thanks for the review.

$ git push pkg upload/2.4.0-1ubuntu2
Enumerating objects: 16, done.
Counting objects: 100% (16/16), done.
Delta compression using up to 8 threads
Compressing objects: 100% (11/11), done.
Writing objects: 100% (11/11), 2.96 KiB | 378.00 KiB/s, done.
Total 11 (delta 5), reused 0 (delta 0)
To ssh://git.launchpad.net/ubuntu/+source/sssd
 * [new tag] upload/2.4.0-1ubuntu2 -> upload/2.4.0-1ubuntu2

$ dput sssd_2.4.0-1ubuntu2_source.changes
Trying to upload package to ubuntu
Checking signature on .changes
gpg: /home/sergio/work/sssd/sssd_2.4.0-1ubuntu2_source.changes: Valid signature from 106DA1C8C3CBBF14
Checking signature on .dsc
gpg: /home/sergio/work/sssd/sssd_2.4.0-1ubuntu2.dsc: Valid signature from 106DA1C8C3CBBF14
Uploading to ubuntu (via ftp to upload.ubuntu.com):
  Uploading sssd_2.4.0-1ubuntu2.dsc: done.
  Uploading sssd_2.4.0-1ubuntu2.debian.tar.xz: done.
  Uploading sssd_2.4.0-1ubuntu2_source.buildinfo: done.
  Uploading sssd_2.4.0-1ubuntu2_source.changes: done.
Successfully uploaded packages.

diff --git a/debian/changelog b/debian/changelog
index b718179..568e3cc 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,12 @@
+sssd (2.4.0-1ubuntu2) hirsute; urgency=medium
+
+  * d/p/0003-Only-start-sssd.service-if-there-s-a-configuration-f.patch:
+    Upstream patch to make sssd.service only able to start when there
+    is a configuration file present.  (LP: #1900642)
+  * d/p/condition-path-exists-sssd-conf.patch: Remove.
+
+ -- Sergio Durigan Junior <[e-mail]>  Tue, 12 Jan 2021 16:17:38 -0500
+
 sssd (2.4.0-1ubuntu1) hirsute; urgency=medium
 
   * d/p/condition-path-exists-sssd-conf.patch: Only start
diff --git a/debian/patches/0003-Only-start-sssd.service-if-there-s-a-configuration-f.patch b/debian/patches/0003-Only-start-sssd.service-if-there-s-a-configuration-f.patch
new file mode 100644
index 0000000..622ddc3
--- /dev/null
+++ b/debian/patches/0003-Only-start-sssd.service-if-there-s-a-configuration-f.patch
@@ -0,0 +1,86 @@
+From: Sergio Durigan Junior <[e-mail]>
+Date: Wed, 9 Dec 2020 22:54:21 -0500
+Subject: Only start sssd.service if there's a configuration file present
+
+This commit is the follow-up of the discussion that is happening here:
+
+https://bugs.launchpad.net/ubuntu/+source/sssd/+bug/1900642
+
+In a nutshell, SSSD is compile with --disable-files-domain and
+installed without a configuration file by default, which means that
+it's impossible to start it successfully unless the user has actively
+created/copied a sssd.conf inside /etc/sssd.
+
+There are two possible ways to have sssd.service successfully start:
+
+1) If SSSD is configured with --enable-files-domain, then no
+   configuration file is required, and the service can start normally.
+
+2) If SSSD is configured with --disable-files-domain, then a
+   configuration file is required.  This can be either
+   /etc/sssd/sssd.conf, or a snippet under /etc/sssd/conf.d/.
+
+For this reason, I'd like to suggest that we conditionally add the
+following lines to sssd.service:
+
+  ConditionPathExists=|/etc/sssd/sssd.conf
+  ConditionDirectoryNotEmpty=|/etc/sssd/conf.d/
+
+These lines will be added only if SSSD is not configured with
+--enable-files-domain.
+
+Signed-off-by: Sergio Durigan Junior <[e-mail]>
+
+Reviewed-by: Alexey Tikhonov <[e-mail]>
+
+Author: Sergio Durigan Junior <[e-mail]>
+Origin: upstream, https://github.com/SSSD/sssd/commit/a25256fe22dd0b976093d15a5c7c73e1dc64bbcc
+Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/sssd/+bug/1900642
+Last-Updated: 2021-01-11
+---
+ Makefile.am                      | 12 +++++++++++-
+ src/sysv/systemd/sssd.service.in |  1 +
+ 2 files changed, 12 insertions(+), 1 deletion(-)
+
+diff --git a/Makefile.am b/Makefile.am
+index 4bacabd..1e5c0e8 100644
+--- a/Makefile.am
++++ b/Makefile.am
+@@ -95,6 +95,15 @@ if HAVE_SYSTEMD_UNIT
+ ifp_exec_cmd = $(sssdlibexecdir)/sssd_ifp --uid 0 --gid 0 --dbus-activated
+ ifp_systemdservice = SystemdService=sssd-ifp.service
+ ifp_restart = Restart=on-failure
++# If sssd is configured with --enable-files-domain, the service is
++# able to start even without a configuration file.  Otherwise, sssd
++# requires a configuration file (either /etc/sssd/sssd.conf, or some
++# snippet under /etc/sssd/sssd.conf.d/) to be present.
++if ADD_FILES_DOMAIN
++condconfigexists =
++else
++condconfigexists = ConditionPathExists=\|/etc/sssd/sssd.conf\nConditionDirectoryNotEmpty=\|/etc/sssd/conf.d/
++endif
+ else
+ ifp_exec_cmd = $(sssdlibexecdir)/sss_signal
+ ifp_systemdservice =
+@@ -5128,7 +5137,8 @@ edit_cmd = $(SED) \
+         -e 's|@libexecdir[@]|$(libexecdir)|g' \
+         -e 's|@pipepath[@]|$(pipepath)|g' \
+         -e 's|@prefix[@]|$(prefix)|g' \
+-        -e 's|@SSSD_USER[@]|$(SSSD_USER)|g'
++        -e 's|@SSSD_USER[@]|$(SSSD_USER)|g' \
++        -e 's|@condconfigexists[@]|$(condconfigexists)|g'
+ 
+ replace_script = \
+     @rm -f $@ $@.tmp; \
+diff --git a/src/sysv/systemd/sssd.service.in b/src/sysv/systemd/sssd.service.in
+index 7a4b7c7..aae36f7 100644
+--- a/src/sysv/systemd/sssd.service.in
++++ b/src/sysv/systemd/sssd.service.in
+@@ -3,6 +3,7 @@ Description=System Security Services Daemon
+ # SSSD must be running before we permit user sessions
+ Before=systemd-user-sessions.service nss-user-lookup.target
+ Wants=nss-user-lookup.target
++@condconfigexists@
+ 
+ [Service]
+ Environment=DEBUG_LOGGER=--logger=files
diff --git a/debian/patches/condition-path-exists-sssd-conf.patch b/debian/patches/condition-path-exists-sssd-conf.patch
deleted file mode 100644
index 7e297c6..0000000
--- a/debian/patches/condition-path-exists-sssd-conf.patch
+++ /dev/null
@@ -1,36 +0,0 @@
-From: Sergio Durigan Junior <[e-mail]>
-Date: Thu, 10 Dec 2020 14:17:09 -0500
-Subject: Only start sssd.service if there's a configuration file present
-
-This commit is the follow-up of the discussion that is happening here:
-
-https://bugs.launchpad.net/ubuntu/+source/sssd/+bug/1900642
-
-In a nutshell, SSSD is installed without a configuration file by
-default, which means that it's impossible to start it successfully
-unless the user has actively created/copied a sssd.conf inside
-/etc/sssd.  For this reason, I'd like to suggest that we add
-"ConditionPathExists=/etc/sssd/sssd.conf" to sssd.service, which
-mitigates the problem of SSSD not properly starting and generating
-error messages in the system log.
-
-Author: Sergio Durigan Junior <[e-mail]>
-Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/sssd/+bug/1900642
-Forwarded: yes, https://github.com/SSSD/sssd/pull/5433
-Last-Updated: 2020-12-10
----
- src/sysv/systemd/sssd.service.in | 1 +
- 1 file changed, 1 insertion(+)
-
-diff --git a/src/sysv/systemd/sssd.service.in b/src/sysv/systemd/sssd.service.in
-index 7a4b7c7..4b0fe98 100644
---- a/src/sysv/systemd/sssd.service.in
-+++ b/src/sysv/systemd/sssd.service.in
-@@ -3,6 +3,7 @@ Description=System Security Services Daemon
- # SSSD must be running before we permit user sessions
- Before=systemd-user-sessions.service nss-user-lookup.target
- Wants=nss-user-lookup.target
-+ConditionPathExists=/etc/sssd/sssd.conf
- 
- [Service]
- Environment=DEBUG_LOGGER=--logger=files
diff --git a/debian/patches/series b/debian/patches/series
index 18be75c..b8ec7a5 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -1,3 +1,3 @@
 fix-whitespace-test.diff
 default-to-socket-activated-services.diff
-condition-path-exists-sssd-conf.patch
+0003-Only-start-sssd.service-if-there-s-a-configuration-f.patch