URL: https://github.com/SSSD/sssd/pull/132
Author: fidencio
 Title: #132: Add "Wants=" to sssd unit
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/132/head:pr132
git checkout pr132
From f4f8df520184aa51cad48b718923f1c15e9eef78 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com>
Date: Tue, 24 Jan 2017 09:36:34 +0100
Subject: [PATCH 1/4] sssd: add a list of dependent services to sssd.service
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Let's add a list of dependent services to the sssd unit file so we can
have all those services enable by default when enabling sssd unit.

As it differs from our first approach were all services were disabled by
default, the manuals have also been updated.

Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com>
---
 Makefile.am                      | 14 +++++++++++++-
 src/man/sssd-sudo.5.xml          |  3 +--
 src/man/sssd.conf.5.xml          | 12 +++++++++---
 src/sysv/systemd/sssd.service.in |  2 +-
 4 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 1b4f0443b..8f93c63c1 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -4880,6 +4880,7 @@ endif
 init_SCRIPTS =
 systemdunit_DATA =
 systemdconf_DATA =
+sssd_dependent_services =
 if HAVE_SYSTEMD_UNIT
     systemdunit_DATA += \
         src/sysv/systemd/sssd.service \
@@ -4889,11 +4890,15 @@ if HAVE_SYSTEMD_UNIT
         src/sysv/systemd/sssd-pam-priv.socket \
         src/sysv/systemd/sssd-pam.service \
         $(NULL)
+
+sssd_dependent_services += sssd-nss.socket sssd-pam.socket
 if BUILD_AUTOFS
     systemdunit_DATA += \
         src/sysv/systemd/sssd-autofs.socket \
         src/sysv/systemd/sssd-autofs.service \
         $(NULL)
+
+sssd_dependent_services += sssd-autofs.socket
 endif
 if BUILD_IFP
     systemdunit_DATA += \
@@ -4905,6 +4910,8 @@ if BUILD_PAC_RESPONDER
         src/sysv/systemd/sssd-pac.socket \
         src/sysv/systemd/sssd-pac.service \
         $(NULL)
+
+sssd_dependent_services += sssd-pac.socket
 endif
 if BUILD_SECRETS
     systemdunit_DATA += \
@@ -4917,12 +4924,16 @@ if BUILD_SSH
         src/sysv/systemd/sssd-ssh.socket \
         src/sysv/systemd/sssd-ssh.service \
         $(NULL)
+
+sssd_dependent_services += sssd-ssh.socket
 endif
 if BUILD_SUDO
     systemdunit_DATA += \
         src/sysv/systemd/sssd-sudo.socket \
         src/sysv/systemd/sssd-sudo.service \
         $(NULL)
+
+sssd_dependent_services += sssd-sudo.socket
 endif
 if BUILD_KCM
     systemdunit_DATA += \
@@ -4969,7 +4980,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|@sssd_dependent_services[@]|${sssd_dependent_services}|g'
 
 replace_script = \
     @rm -f $@ $@.tmp; \
diff --git a/src/man/sssd-sudo.5.xml b/src/man/sssd-sudo.5.xml
index 5bc56c463..cb085419a 100644
--- a/src/man/sssd-sudo.5.xml
+++ b/src/man/sssd-sudo.5.xml
@@ -110,8 +110,7 @@ ldap_sudo_search_base = ou=sudoers,dc=example,dc=com
             <phrase condition="have_systemd">
                 It's important to note that on platforms where systemd is supported
                 there's no need to add the "sudo" provider to the list of services,
-                as it became optional. However, sssd-sudo.socket must be enabled
-                instead.
+                as it became optional.
             </phrase>
         </para>
         <para>
diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml
index 881ffc6ab..23f59e0e8 100644
--- a/src/man/sssd.conf.5.xml
+++ b/src/man/sssd.conf.5.xml
@@ -220,9 +220,15 @@
                             </para>
                             <para>
                                 <phrase condition="have_systemd">
-                                    By default, all services are disabled and the administrator
-                                    must enable the ones allowed to be used by executing:
-                                    "systemctl enable sssd-@service@.socket".
+                                    By default, the following services are enabled: nss, pam
+                                    <phrase condition="with_sudo">, sudo</phrase>
+                                    <phrase condition="with_autofs">, autofs</phrase>
+                                    <phrase condition="with_ssh">, ssh</phrase>
+                                    <phrase condition="with_pac_responder">, pac</phrase>
+                                    <phrase condition="with_ifp">, ifp</phrase>
+                                    In case the Administrator wants to persistently disable
+                                    one of them, it can be done by running:
+                                    "systemctl mask sssd-@service@.socket"
                                 </phrase>
                             </para>
                         </listitem>
diff --git a/src/sysv/systemd/sssd.service.in b/src/sysv/systemd/sssd.service.in
index 0c515d34c..49c09ea58 100644
--- a/src/sysv/systemd/sssd.service.in
+++ b/src/sysv/systemd/sssd.service.in
@@ -2,7 +2,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
+Wants=nss-user-lookup.target @sssd_dependent_services@
 
 [Service]
 Environment=DEBUG_LOGGER=--logger=files

From bd9f8579181f383b08824e3250e4586b2d1d4ca6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com>
Date: Thu, 9 Aug 2018 16:31:19 +0200
Subject: [PATCH 2/4] monitor: remove add_implicit_services()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

As the services are socket-activated and enabled by default by SSSD,
there's no need to keep this code which has the only purpose to add the
PAC responder to the services list when needed.

Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com>
---
 src/monitor/monitor.c | 78 -------------------------------------------
 1 file changed, 78 deletions(-)

diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c
index 335b2070b..4aae11da1 100644
--- a/src/monitor/monitor.c
+++ b/src/monitor/monitor.c
@@ -779,77 +779,6 @@ static int check_local_domain_unique(struct sss_domain_info *domains)
     return EOK;
 }
 
-static errno_t add_implicit_services(struct confdb_ctx *cdb, TALLOC_CTX *mem_ctx,
-                                     char ***_services)
-{
-    int ret;
-    char **domain_names;
-    TALLOC_CTX *tmp_ctx;
-    size_t c;
-    char *conf_path;
-    char *id_provider;
-    bool add_pac = false;
-
-    tmp_ctx = talloc_new(NULL);
-    if (tmp_ctx == NULL) {
-        DEBUG(SSSDBG_OP_FAILURE, "talloc_new failed.\n");
-        return ENOMEM;
-    }
-
-    ret = confdb_get_string_as_list(cdb, tmp_ctx,
-                                    CONFDB_MONITOR_CONF_ENTRY,
-                                    CONFDB_MONITOR_ACTIVE_DOMAINS,
-                                    &domain_names);
-    if (ret == ENOENT) {
-        DEBUG(SSSDBG_OP_FAILURE, "No domains configured!\n");
-        goto done;
-    }
-
-    for (c = 0; domain_names[c] != NULL; c++) {
-        conf_path = talloc_asprintf(tmp_ctx, CONFDB_DOMAIN_PATH_TMPL,
-                                    domain_names[c]);
-        if (conf_path == NULL) {
-            DEBUG(SSSDBG_OP_FAILURE, "talloc_asprintf failed.\n");
-            ret = ENOMEM;
-            goto done;
-        }
-
-        ret = confdb_get_string(cdb, tmp_ctx, conf_path,
-                                CONFDB_DOMAIN_ID_PROVIDER, NULL, &id_provider);
-        if (ret == EOK) {
-            if (id_provider == NULL) {
-                DEBUG(SSSDBG_OP_FAILURE, "id_provider is not set for "
-                      "domain [%s], trying next domain.\n", domain_names[c]);
-                continue;
-            }
-
-            if (strcasecmp(id_provider, "IPA") == 0) {
-                add_pac = true;
-            }
-        } else {
-            DEBUG(SSSDBG_OP_FAILURE, "Failed to get id_provider for " \
-                                      "domain [%s], trying next domain.\n",
-                                      domain_names[c]);
-        }
-    }
-
-    if (BUILD_WITH_PAC_RESPONDER && add_pac &&
-        !string_in_list("pac", *_services, false)) {
-        ret = add_string_to_list(mem_ctx, "pac", _services);
-        if (ret != EOK) {
-            DEBUG(SSSDBG_OP_FAILURE, "add_string_to_list failed.\n");
-            goto done;
-        }
-    }
-
-    ret = EOK;
-
-done:
-    talloc_free(tmp_ctx);
-
-    return ret;
-}
-
 static char *check_service(char *service)
 {
     const char * const *known_services = get_known_services();
@@ -942,13 +871,6 @@ static int get_monitor_config(struct mt_ctx *ctx)
     }
 #endif
 
-    ret = add_implicit_services(ctx->cdb, ctx, &ctx->services);
-    if (ret != EOK) {
-        DEBUG(SSSDBG_OP_FAILURE, "Failed to add implicit configured "
-                                 "services. Some functionality might "
-                                 "be missing\n");
-    }
-
     badsrv = check_services(ctx->services);
     if (badsrv != NULL) {
         DEBUG(SSSDBG_FATAL_FAILURE, "Invalid service %s\n", badsrv);

From 8045ddfa532ca2ed78cb30da2453da8bb08a3ad6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com>
Date: Wed, 22 Aug 2018 13:32:31 +0200
Subject: [PATCH 3/4] socket_activated_responders: deal with conf less setup
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

In case there's no sssd.conf, let's just start the services' sockets.

Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com>
---
 src/tools/sssd_check_socket_activated_responders.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/tools/sssd_check_socket_activated_responders.c b/src/tools/sssd_check_socket_activated_responders.c
index fb9df3909..2d8a14d24 100644
--- a/src/tools/sssd_check_socket_activated_responders.c
+++ b/src/tools/sssd_check_socket_activated_responders.c
@@ -54,7 +54,10 @@ static errno_t check_socket_activated_responder(const char *responder)
     }
 
     ret = ini_config_file_open(SSSD_CONFIG_FILE, 0, &file_ctx);
-    if (ret != 0) {
+    if (ret == ENOENT) {
+        ret = EOK;
+        goto done;
+    } else if (ret != 0) {
         DEBUG(SSSDBG_CRIT_FAILURE, "ini_config_file_open() failed [%d][%s]\n",
               ret, sss_strerror(ret));
         goto done;

From 570dae9fa156a62676f77fa176939793ffb3de09 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com>
Date: Wed, 15 Aug 2018 23:00:01 +0200
Subject: [PATCH 4/4] socket_activated_responders: improve tool's error log
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Improve the sssd_check_socket_activated_responders' error log in case of
misconfiguration and also start to log it into the syslog instead of
logging it only into the responders' log files.

Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com>
---
 .../sssd_check_socket_activated_responders.c  | 42 +++++++++++++++----
 1 file changed, 33 insertions(+), 9 deletions(-)

diff --git a/src/tools/sssd_check_socket_activated_responders.c b/src/tools/sssd_check_socket_activated_responders.c
index 2d8a14d24..f948c8053 100644
--- a/src/tools/sssd_check_socket_activated_responders.c
+++ b/src/tools/sssd_check_socket_activated_responders.c
@@ -149,10 +149,12 @@ static errno_t check_socket_activated_responder(const char *responder)
 
 int main(int argc, const char *argv[])
 {
+    TALLOC_CTX *tmp_ctx;
     int ret;
     int opt;
     poptContext pc;
     char *responder = NULL;
+    char *err_msg = NULL;
 
     struct poptOption long_options[] = {
         POPT_AUTOHELP
@@ -161,6 +163,11 @@ int main(int argc, const char *argv[])
         POPT_TABLEEND
     };
 
+    tmp_ctx = talloc_new(NULL);
+    if (tmp_ctx == NULL) {
+        return ENOMEM;
+    }
+
     pc = poptGetContext(argv[0], argc, argv, long_options, 0);
     while ((opt = poptGetNextOpt(pc)) != -1) {
         switch (opt) {
@@ -181,20 +188,37 @@ int main(int argc, const char *argv[])
 
     ret = check_socket_activated_responder(responder);
     if (ret != EOK) {
-        DEBUG(SSSDBG_DEFAULT,
-              "Misconfiguration found for the %s responder.\n"
-              "The %s responder has been configured to be socket-activated "
-              "but it's still mentioned in the services' line in %s.\n"
-              "Please, consider either adjusting your services' line in %s "
-              "or disabling the %s's socket by calling:\n"
-              "\"systemctl disable sssd-%s.socket\"",
-              responder, responder, SSSD_CONFIG_FILE, SSSD_CONFIG_FILE,
-              responder, responder);
+        err_msg = talloc_asprintf(
+                tmp_ctx,
+                "There's a misconfiguration in the \"services\" line of "
+                "\"%s\"!\n"
+                "The \"services\" line contains \"%s\", meaning that the "
+                "responder's process will be started and managed by SSSD's "
+                "monitor. "
+                "However, SSSD relies on systemd to start "
+                "sssd-%s.socket and then manage the responder's process, "
+                "causing then a configuration conflict.\n"
+                "In order to solve this misconfiguration, please, either "
+                "remove \"%s\" from the \"services\" line in \"%s\" or call "
+                "`systemctl mask sssd-%s.socket`\n"
+                "Please, refer to \"sssd.conf\" man page for more info and "
+                "mind that the recommended way to go is to take advantage "
+                "of systemd, as much as possible, avoiding then to have a "
+                "\"services\" line in \"%s\"!",
+                SSSD_CONFIG_FILE, responder, responder, responder,
+                SSSD_CONFIG_FILE, responder, SSSD_CONFIG_FILE);
+        if (err_msg == NULL) {
+            goto done;
+        }
+
+        DEBUG(SSSDBG_IMPORTANT_INFO, "%s\n", err_msg);
+        sss_log(SSS_LOG_WARNING, "%s\n", err_msg);
         goto done;
     }
 
     ret = EOK;
 done:
     poptFreeContext(pc);
+    talloc_zfree(tmp_ctx);
     return ret;
 }
_______________________________________________
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedoraproject.org/archives/list/sssd-devel@lists.fedorahosted.org/message/ZBQ3GG7XS4PEDMZSUEKL4ZTPESZVTMJP/

Reply via email to