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

[Sergio Durigan Junior]

This is a new MP to address bug 1900642. The difference between it and the previous one (here: https://code.launchpad.net/~sergiodj/ubuntu/+source/sssd/+git/sssd/+merge/395175) is that it incorporates a better, more complete version of the patch I used to fix the issue. In fact, this version of the patch comes directly from upstream, which accepted my solution (after some tweaking).

The previous MP had been approved, and the SRU team had even accepted the package, but I decided to not proceed with the verification process and re-do everything. This is the result.

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.

The good thing is that the SRU template can be left unmodified. I asked Robie how I should proceed in the specific case that a package has already been accepted (but not verified) by the SRU team, and he told me I should bump the version number, so that's what I did. Hopefully everything is correct.

[Bryce Harrington (bryce)]

Looks like this needs to be addressed in hirsute, prior to the SRUs? That will need-fixing, since the SRU won't be accepted without it landing in hirsute.

Otherwise, the merge proposal looks good, and is a logical refinement over the previously reviewed & approved MP, https://code.launchpad.net/~sergiodj/ubuntu/+source/sssd/+git/sssd/+merge/395175

Approving, contingent on the fix also going into hirsute.

review: Approve

[Sergio Durigan Junior (sergiodj)]

Thanks for the review. The hirsute MP has been approved, and the package pushed.

$ git push pkg upload/2.3.1-3ubuntu2
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.95 KiB | 377.00 KiB/s, done.
Total 11 (delta 5), reused 0 (delta 0)
To ssh://git.launchpad.net/ubuntu/+source/sssd
 * [new tag] upload/2.3.1-3ubuntu2 -> upload/2.3.1-3ubuntu2

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

diff --git a/debian/changelog b/debian/changelog
index b25292d..4402380 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,12 @@
+sssd (2.3.1-3ubuntu2) groovy; 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]>  Mon, 11 Jan 2021 14:30:55 -0500
+
 sssd (2.3.1-3ubuntu1) groovy; 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