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

Proposed by Sergio Durigan Junior
Status: Merged
Approved by: Sergio Durigan Junior
Approved revision: 3ead5e9a451cd682c339c702d8f50869659b1d2b
Merged at revision: 3ead5e9a451cd682c339c702d8f50869659b1d2b
Proposed branch: ~sergiodj/ubuntu/+source/sssd:bug1900642-condpathexists-focal
Merge into: ubuntu/+source/sssd:ubuntu/focal-devel
Diff against target: 117 lines (+95/-0)
3 files modified
debian/changelog (+8/-0)
debian/patches/0003-Only-start-sssd.service-if-there-s-a-configuration-f.patch (+86/-0)
debian/patches/series (+1/-0)
Reviewer Review Type Date Requested Status
Bryce Harrington Approve
Canonical Server Team Pending
Review via email: mp+396099@code.launchpad.net

Description of the change

This is a MP to address bug 1900642. It includes a patch that comes directly from upstream, which accepted my solution (after some tweaking).

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.

autopkgtest is still happy:

autopkgtest [17:03:52]: @@@@@@@@@@@@@@@@@@@@ summary
ldap-user-group-ldap-auth PASS
ldap-user-group-krb5-auth PASS

To post a comment you must log in.
Revision history for this message
Bryce Harrington (bryce) wrote :

Same as the groovy MP, approved for same reason and same condition that a fix for hirsute is likewise needed before this can be accepted for SRU.

review: Approve
Revision history for this message
Sergio Durigan Junior (sergiodj) wrote :

Thanks for the review, Bryce.

I've just created an MP for hirsute, and tagged you as a reviewer:

https://code.launchpad.net/~sergiodj/ubuntu/+source/sssd/+git/sssd/+merge/396206

If you can, please have a look :-). Thanks!

Revision history for this message
Sergio Durigan Junior (sergiodj) wrote :

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

$ dput sssd_2.2.3-3ubuntu0.2_source.changes
Trying to upload package to ubuntu
Checking signature on .changes
gpg: /home/sergio/work/sssd/sssd-focal/sssd_2.2.3-3ubuntu0.2_source.changes: Valid signature from 106DA1C8C3CBBF14
Checking signature on .dsc
gpg: /home/sergio/work/sssd/sssd-focal/sssd_2.2.3-3ubuntu0.2.dsc: Valid signature from 106DA1C8C3CBBF14
Uploading to ubuntu (via ftp to upload.ubuntu.com):
  Uploading sssd_2.2.3-3ubuntu0.2.dsc: done.
  Uploading sssd_2.2.3-3ubuntu0.2.diff.gz: done.
  Uploading sssd_2.2.3-3ubuntu0.2_source.buildinfo: done.
  Uploading sssd_2.2.3-3ubuntu0.2_source.changes: done.
Successfully uploaded packages.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/debian/changelog b/debian/changelog
2index 21cd451..b6366f5 100644
3--- a/debian/changelog
4+++ b/debian/changelog
5@@ -1,3 +1,11 @@
6+sssd (2.2.3-3ubuntu0.2) focal; urgency=medium
7+
8+ * d/p/0003-Only-start-sssd.service-if-there-s-a-configuration-f.patch:
9+ Upstream patch to make sssd.service only able to start when there
10+ is a configuration file present. (LP: #1900642)
11+
12+ -- Sergio Durigan Junior <sergio.durigan@canonical.com> Mon, 11 Jan 2021 14:33:54 -0500
13+
14 sssd (2.2.3-3ubuntu0.1) focal; urgency=medium
15
16 * Enable support for "ad_use_ldaps" for new Active Directory
17diff --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
18new file mode 100644
19index 0000000..622ddc3
20--- /dev/null
21+++ b/debian/patches/0003-Only-start-sssd.service-if-there-s-a-configuration-f.patch
22@@ -0,0 +1,86 @@
23+From: Sergio Durigan Junior <sergio.durigan@canonical.com>
24+Date: Wed, 9 Dec 2020 22:54:21 -0500
25+Subject: Only start sssd.service if there's a configuration file present
26+
27+This commit is the follow-up of the discussion that is happening here:
28+
29+https://bugs.launchpad.net/ubuntu/+source/sssd/+bug/1900642
30+
31+In a nutshell, SSSD is compile with --disable-files-domain and
32+installed without a configuration file by default, which means that
33+it's impossible to start it successfully unless the user has actively
34+created/copied a sssd.conf inside /etc/sssd.
35+
36+There are two possible ways to have sssd.service successfully start:
37+
38+1) If SSSD is configured with --enable-files-domain, then no
39+ configuration file is required, and the service can start normally.
40+
41+2) If SSSD is configured with --disable-files-domain, then a
42+ configuration file is required. This can be either
43+ /etc/sssd/sssd.conf, or a snippet under /etc/sssd/conf.d/.
44+
45+For this reason, I'd like to suggest that we conditionally add the
46+following lines to sssd.service:
47+
48+ ConditionPathExists=|/etc/sssd/sssd.conf
49+ ConditionDirectoryNotEmpty=|/etc/sssd/conf.d/
50+
51+These lines will be added only if SSSD is not configured with
52+--enable-files-domain.
53+
54+Signed-off-by: Sergio Durigan Junior <sergio.durigan@canonical.com>
55+
56+Reviewed-by: Alexey Tikhonov <atikhono@redhat.com>
57+
58+Author: Sergio Durigan Junior <sergio.durigan@canonical.com>
59+Origin: upstream, https://github.com/SSSD/sssd/commit/a25256fe22dd0b976093d15a5c7c73e1dc64bbcc
60+Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/sssd/+bug/1900642
61+Last-Updated: 2021-01-11
62+---
63+ Makefile.am | 12 +++++++++++-
64+ src/sysv/systemd/sssd.service.in | 1 +
65+ 2 files changed, 12 insertions(+), 1 deletion(-)
66+
67+diff --git a/Makefile.am b/Makefile.am
68+index 4bacabd..1e5c0e8 100644
69+--- a/Makefile.am
70++++ b/Makefile.am
71+@@ -95,6 +95,15 @@ if HAVE_SYSTEMD_UNIT
72+ ifp_exec_cmd = $(sssdlibexecdir)/sssd_ifp --uid 0 --gid 0 --dbus-activated
73+ ifp_systemdservice = SystemdService=sssd-ifp.service
74+ ifp_restart = Restart=on-failure
75++# If sssd is configured with --enable-files-domain, the service is
76++# able to start even without a configuration file. Otherwise, sssd
77++# requires a configuration file (either /etc/sssd/sssd.conf, or some
78++# snippet under /etc/sssd/sssd.conf.d/) to be present.
79++if ADD_FILES_DOMAIN
80++condconfigexists =
81++else
82++condconfigexists = ConditionPathExists=\|/etc/sssd/sssd.conf\nConditionDirectoryNotEmpty=\|/etc/sssd/conf.d/
83++endif
84+ else
85+ ifp_exec_cmd = $(sssdlibexecdir)/sss_signal
86+ ifp_systemdservice =
87+@@ -5128,7 +5137,8 @@ edit_cmd = $(SED) \
88+ -e 's|@libexecdir[@]|$(libexecdir)|g' \
89+ -e 's|@pipepath[@]|$(pipepath)|g' \
90+ -e 's|@prefix[@]|$(prefix)|g' \
91+- -e 's|@SSSD_USER[@]|$(SSSD_USER)|g'
92++ -e 's|@SSSD_USER[@]|$(SSSD_USER)|g' \
93++ -e 's|@condconfigexists[@]|$(condconfigexists)|g'
94+
95+ replace_script = \
96+ @rm -f $@ $@.tmp; \
97+diff --git a/src/sysv/systemd/sssd.service.in b/src/sysv/systemd/sssd.service.in
98+index 7a4b7c7..aae36f7 100644
99+--- a/src/sysv/systemd/sssd.service.in
100++++ b/src/sysv/systemd/sssd.service.in
101+@@ -3,6 +3,7 @@ Description=System Security Services Daemon
102+ # SSSD must be running before we permit user sessions
103+ Before=systemd-user-sessions.service nss-user-lookup.target
104+ Wants=nss-user-lookup.target
105++@condconfigexists@
106+
107+ [Service]
108+ Environment=DEBUG_LOGGER=--logger=files
109diff --git a/debian/patches/series b/debian/patches/series
110index 4b402cf..2ec352f 100644
111--- a/debian/patches/series
112+++ b/debian/patches/series
113@@ -6,3 +6,4 @@ lp-1868703-01-ad-allow-booleans-for-ad_inherit_opts_if_needed.patch
114 lp-1868703-02-ad-add-ad_use_ldaps.patch
115 lp-1868703-03-ldap-add-new-option-ldap_sasl_maxssf.patch
116 lp-1868703-04-ad-set-min-and-max-ssf-for-ldaps.patch
117+0003-Only-start-sssd.service-if-there-s-a-configuration-f.patch

Subscribers

People subscribed via source and target branches