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 074e4e424953a9d21a0be5b47d9fead348bfd774 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/3] 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 d313957722..e09808b4aa 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -4879,6 +4879,7 @@ endif
 init_SCRIPTS =
 systemdunit_DATA =
 systemdconf_DATA =
+sssd_dependent_services =
 if HAVE_SYSTEMD_UNIT
     systemdunit_DATA += \
         src/sysv/systemd/sssd.service \
@@ -4888,11 +4889,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 += \
@@ -4904,6 +4909,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 += \
@@ -4916,12 +4923,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 += \
@@ -4968,7 +4979,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 5bc56c4633..cb085419ab 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 881ffc6ab3..23f59e0e86 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 0c515d34ca..49c09ea583 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 4ba7b1d9ed346d4835a414668cdfe6c3f73b6f44 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/3] 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 335b2070bf..4aae11da1e 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 a6b6966ed738eaf7fc7f6f98a741e9f5bd7f1c8c 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 3/3] 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 fb9df39091..e83de622b8 100644
--- a/src/tools/sssd_check_socket_activated_responders.c
+++ b/src/tools/sssd_check_socket_activated_responders.c
@@ -146,10 +146,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
@@ -158,6 +160,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) {
@@ -178,20 +185,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 automatically pulls in the \"%s\" socket(s) and "
+                "relies on systemd to start and manage the responder's "
+                "process.\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/JORAOXOBBU2OA7TLKDMI4ZJPY4LJNE7V/

Reply via email to