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

Proposed by Sergio Durigan Junior
Status: Merged
Approved by: Sergio Durigan Junior
Approved revision: e923697337a03b6e315f842f2e6be162ee349bd5
Merged at revision: e923697337a03b6e315f842f2e6be162ee349bd5
Proposed branch: ~sergiodj/ubuntu/+source/sssd:bug1900642-condpathexists-hirsute
Merge into: ubuntu/+source/sssd:ubuntu/devel
Diff against target: 80 lines (+47/-1)
4 files modified
debian/changelog (+8/-0)
debian/control (+2/-1)
debian/patches/condition-path-exists-sssd-conf.patch (+36/-0)
debian/patches/series (+1/-0)
Reviewer Review Type Date Requested Status
Lucas Kanashiro Approve
Canonical Server Core Reviewers Pending
Review via email: mp+395170@code.launchpad.net

Description of the change

This is the fix for bug 1900642 on hirsute. I will create another MP (with its corresponding SRU) for groovy as well.

The problem here is that SSSD by default doesn't make any assumptions regarding the user setup and a desired configuration. However, the sssd service requires a valid configuration present at /etc/sssd/sssd.conf in order to successfully start.

Starting from groovy, sssd became a dependency of ubuntu-desktop, which means that it will be installed automatically when the user installs Ubuntu (with a desktop). Because of what I explained above, the user will see a bunch of error messages in the log files (journalctl) during boot time because sssd will fail to start (unless the user has configured Ubuntu to be part of an Active Directory setup, in which case sssd will have been configured during installation time).

In order to avoid these scenarios from happening, we're proposing that sssd.service has a "ConditionPathExists" directive which will only attempt to start the service if /etc/sssd/sssd.conf is present.

I've rebuilt and installed the new sssd in a pristine system, and verified that the error messages are not present in the logs anymore. The user will still see a bunch of warning messages regarding the socket-activated units:

Dec 09 18:44:29 focal-desktop systemd[1]: sssd-nss.socket: Bound to unit sssd.service, but unit isn't active.
Dec 09 18:44:29 focal-desktop systemd[1]: Dependency failed for SSSD NSS Service responder socket.
Dec 09 18:44:29 focal-desktop systemd[1]: sssd-nss.socket: Job sssd-nss.socket/start failed with result 'dependency'.
Dec 09 18:44:29 focal-desktop systemd[1]: sssd-autofs.socket: Bound to unit sssd.service, but unit isn't active.
Dec 09 18:44:29 focal-desktop systemd[1]: Dependency failed for SSSD AutoFS Service responder socket.
Dec 09 18:44:29 focal-desktop systemd[1]: sssd-autofs.socket: Job sssd-autofs.socket/start failed with result 'dependency'.
...

but these messages are harmless, and Timo Aaltonen (Debian maintainer of the sssd package) has already mentioned that he will get rid of those units soon.

Last, but not least, I submitted a PR to the sssd upstream project proposing this same modification to the sssd.service file:

https://github.com/SSSD/sssd/pull/5433

There is a PPA with the proposed build here:

https://launchpad.net/~sergiodj/+archive/ubuntu/sssd-bug1900642

autopkgtest is still happy:

autopkgtest [14:40:25]: @@@@@@@@@@@@@@@@@@@@ summary
ldap-user-group-ldap-auth PASS
ldap-user-group-krb5-auth PASS

To post a comment you must log in.
Lucas Kanashiro (lucaskanashiro) wrote :

The solution looks good and the packing changes as well. No additions to the SRU bug description.

LGTM, +1.

review: Approve
Sergio Durigan Junior (sergiodj) wrote :

$ git push pkg upload/2.4.0-1ubuntu1
Enumerating objects: 21, done.
Counting objects: 100% (21/21), done.
Delta compression using up to 8 threads
Compressing objects: 100% (14/14), done.
Writing objects: 100% (15/15), 2.58 KiB | 440.00 KiB/s, done.
Total 15 (delta 8), reused 1 (delta 1)
To ssh://git.launchpad.net/ubuntu/+source/sssd
 * [new tag] upload/2.4.0-1ubuntu1 -> upload/2.4.0-1ubuntu1

$ dput ubuntu sssd_2.4.0-1ubuntu1_source.changes
Checking signature on .changes
gpg: /home/sergio/work/sssd/sssd_2.4.0-1ubuntu1_source.changes: Valid signature from 106DA1C8C3CBBF14
Checking signature on .dsc
gpg: /home/sergio/work/sssd/sssd_2.4.0-1ubuntu1.dsc: Valid signature from 106DA1C8C3CBBF14
Uploading to ubuntu (via ftp to upload.ubuntu.com):
  Uploading sssd_2.4.0-1ubuntu1.dsc: done.
  Uploading sssd_2.4.0-1ubuntu1.debian.tar.xz: done.
  Uploading sssd_2.4.0-1ubuntu1_source.buildinfo: done.
  Uploading sssd_2.4.0-1ubuntu1_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 7583fdc..b718179 100644
3--- a/debian/changelog
4+++ b/debian/changelog
5@@ -1,3 +1,11 @@
6+sssd (2.4.0-1ubuntu1) hirsute; urgency=medium
7+
8+ * d/p/condition-path-exists-sssd-conf.patch: Only start
9+ sssd.service if there is a configuration file present.
10+ (LP: #1900642)
11+
12+ -- Sergio Durigan Junior <sergio.durigan@canonical.com> Thu, 10 Dec 2020 14:20:24 -0500
13+
14 sssd (2.4.0-1) unstable; urgency=medium
15
16 * New upstream release.
17diff --git a/debian/control b/debian/control
18index 4ed6a14..2fba43d 100644
19--- a/debian/control
20+++ b/debian/control
21@@ -1,7 +1,8 @@
22 Source: sssd
23 Section: utils
24 Priority: optional
25-Maintainer: Debian SSSD Team <pkg-sssd-devel@alioth-lists.debian.net>
26+Maintainer: Ubuntu Developers <ubuntu-devel-discuss@lists.ubuntu.com>
27+XSBC-Original-Maintainer: Debian SSSD Team <pkg-sssd-devel@alioth-lists.debian.net>
28 Uploaders: Timo Aaltonen <tjaalton@debian.org>,
29 Dominik George <natureshadow@debian.org>
30 Build-Depends:
31diff --git a/debian/patches/condition-path-exists-sssd-conf.patch b/debian/patches/condition-path-exists-sssd-conf.patch
32new file mode 100644
33index 0000000..7e297c6
34--- /dev/null
35+++ b/debian/patches/condition-path-exists-sssd-conf.patch
36@@ -0,0 +1,36 @@
37+From: Sergio Durigan Junior <sergio.durigan@canonical.com>
38+Date: Thu, 10 Dec 2020 14:17:09 -0500
39+Subject: Only start sssd.service if there's a configuration file present
40+
41+This commit is the follow-up of the discussion that is happening here:
42+
43+https://bugs.launchpad.net/ubuntu/+source/sssd/+bug/1900642
44+
45+In a nutshell, SSSD is installed without a configuration file by
46+default, which means that it's impossible to start it successfully
47+unless the user has actively created/copied a sssd.conf inside
48+/etc/sssd. For this reason, I'd like to suggest that we add
49+"ConditionPathExists=/etc/sssd/sssd.conf" to sssd.service, which
50+mitigates the problem of SSSD not properly starting and generating
51+error messages in the system log.
52+
53+Author: Sergio Durigan Junior <sergio.durigan@canonical.com>
54+Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/sssd/+bug/1900642
55+Forwarded: yes, https://github.com/SSSD/sssd/pull/5433
56+Last-Updated: 2020-12-10
57+---
58+ src/sysv/systemd/sssd.service.in | 1 +
59+ 1 file changed, 1 insertion(+)
60+
61+diff --git a/src/sysv/systemd/sssd.service.in b/src/sysv/systemd/sssd.service.in
62+index 7a4b7c7..4b0fe98 100644
63+--- a/src/sysv/systemd/sssd.service.in
64++++ b/src/sysv/systemd/sssd.service.in
65+@@ -3,6 +3,7 @@ Description=System Security Services Daemon
66+ # SSSD must be running before we permit user sessions
67+ Before=systemd-user-sessions.service nss-user-lookup.target
68+ Wants=nss-user-lookup.target
69++ConditionPathExists=/etc/sssd/sssd.conf
70+
71+ [Service]
72+ Environment=DEBUG_LOGGER=--logger=files
73diff --git a/debian/patches/series b/debian/patches/series
74index d83fab8..18be75c 100644
75--- a/debian/patches/series
76+++ b/debian/patches/series
77@@ -1,2 +1,3 @@
78 fix-whitespace-test.diff
79 default-to-socket-activated-services.diff
80+condition-path-exists-sssd-conf.patch

Subscribers

People subscribed via source and target branches