[SSSD] [sssd PR#5378][opened] sbus: change socket file name to sbus-dp_DOMAIN
URL: https://github.com/SSSD/sssd/pull/5378
Author: ikerexxe
Title: #5378: sbus: change socket file name to sbus-dp_DOMAIN
Action: opened
PR body:
"""
Socket file name changes from sbus-dp_DOMAIN_PID to sbus-dp_DOMAIN to
avoid leaving orphaned socket files when sssd crashes. As there is only
one socket opened per domain it makes no sense to include PID number in
the file name.
Besides, remove socket's symlink creation.
Resolves:
https://bugzilla.redhat.com/show_bug.cgi?id=1875981
"""
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5378/head:pr5378
git checkout pr5378
From a09ae12c31bcd116e71c426777663fd8cc0c2ad0 Mon Sep 17 00:00:00 2001
From: ikerexxe
Date: Wed, 28 Oct 2020 12:31:33 +0100
Subject: [PATCH] sbus: change socket file name to sbus-dp_DOMAIN
Socket file name changes from sbus-dp_DOMAIN_PID to sbus-dp_DOMAIN to
avoid leaving orphaned socket files when sssd crashes. As there is only
one socket opened per domain it makes no sense to include PID number in
the file name.
Besides, remove socket's symlink creation.
Resolves:
https://bugzilla.redhat.com/show_bug.cgi?id=1875981
---
src/monitor/monitor.c | 2 +-
src/providers/data_provider/dp.c | 2 +-
src/sbus/connection/sbus_connection_connect.c | 3 +-
src/sbus/sbus.h | 4 -
src/sbus/sbus_private.h | 1 -
src/sbus/server/sbus_server.c | 202 +-
6 files changed, 9 insertions(+), 205 deletions(-)
diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c
index d9da05a511..99e4337006 100644
--- a/src/monitor/monitor.c
+++ b/src/monitor/monitor.c
@@ -2008,7 +2008,7 @@ static int monitor_process_init(struct mt_ctx *ctx,
req = sbus_server_create_and_connect_send(ctx, ctx->ev, SSS_BUS_MONITOR,
NULL, SSS_MONITOR_ADDRESS,
- false, 100, ctx->uid, ctx->gid,
+ 100, ctx->uid, ctx->gid,
NULL, NULL);
if (req == NULL) {
ret = ENOMEM;
diff --git a/src/providers/data_provider/dp.c b/src/providers/data_provider/dp.c
index 0858c43d2a..306841fb74 100644
--- a/src/providers/data_provider/dp.c
+++ b/src/providers/data_provider/dp.c
@@ -192,7 +192,7 @@ dp_init_send(TALLOC_CTX *mem_ctx,
talloc_set_destructor(state->provider, dp_destructor);
subreq = sbus_server_create_and_connect_send(state->provider, ev,
-state->sbus_name, NULL, sbus_address, true, 1000, uid, gid,
+state->sbus_name, NULL, sbus_address, 1000, uid, gid,
(sbus_server_on_connection_cb)dp_client_init,
(sbus_server_on_connection_data)state->provider);
if (subreq == NULL) {
diff --git a/src/sbus/connection/sbus_connection_connect.c b/src/sbus/connection/sbus_connection_connect.c
index 9cfe86206b..6e232492d0 100644
--- a/src/sbus/connection/sbus_connection_connect.c
+++ b/src/sbus/connection/sbus_connection_connect.c
@@ -341,7 +341,6 @@ sbus_server_create_and_connect_send(TALLOC_CTX *mem_ctx,
const char *dbus_name,
time_t *last_activity_time,
const char *address,
-bool use_symlink,
uint32_t max_connections,
uid_t uid,
gid_t gid,
@@ -359,7 +358,7 @@ sbus_server_create_and_connect_send(TALLOC_CTX *mem_ctx,
return NULL;
}
-state->server = sbus_server_create(state, ev, address, use_symlink,
+state->server = sbus_server_create(state, ev, address,
max_connections, uid, gid,
on_conn_cb, on_conn_data);
if (state->server == NULL) {
diff --git a/src/sbus/sbus.h b/src/sbus/sbus.h
index 0983879f0e..9c8faa3ed8 100644
--- a/src/sbus/sbus.h
+++ b/src/sbus/sbus.h
@@ -135,7 +135,6 @@ errno_t sbus_connect_private_recv(TALLOC_CTX *mem_ctx,
* @param mem_ctxMemory context.
* @param ev Tevent context.
* @param addressSocket address.
- * @param use_symlinkIf a symlink to @address should be created.
* @param uidSocket owner uid.
* @param gidSocket owner gid.
* @param on_conn_cb On new connection callback function.
@@ -147,7 +146,6 @@ struct sbus_server *
sbus_server_create(TALLOC_CTX *mem_ctx,
struct tevent_context *ev,
const char *address,
- bool use_symlink,
uint32_t max_connections,
uid_t uid,
gi
[SSSD] [sssd PR#5378][edited] WIP: sbus: change socket file name to sbus-dp_DOMAIN
URL: https://github.com/SSSD/sssd/pull/5378 Author: ikerexxe Title: #5378: WIP: sbus: change socket file name to sbus-dp_DOMAIN Action: edited Changed field: title Original value: """ sbus: change socket file name to sbus-dp_DOMAIN """ ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
[SSSD] [sssd PR#5378][synchronized] WIP: sbus: change socket file name to sbus-dp_DOMAIN
URL: https://github.com/SSSD/sssd/pull/5378
Author: ikerexxe
Title: #5378: WIP: sbus: change socket file name to sbus-dp_DOMAIN
Action: synchronized
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5378/head:pr5378
git checkout pr5378
From fee2a910f3140df31d0e437c6d9b66ebaecfdfb3 Mon Sep 17 00:00:00 2001
From: ikerexxe
Date: Wed, 28 Oct 2020 12:31:33 +0100
Subject: [PATCH 1/2] sbus: change socket file name to sbus-dp_DOMAIN
Socket file name changes from sbus-dp_DOMAIN_PID to sbus-dp_DOMAIN to
avoid leaving orphaned socket files when sssd crashes. As there is only
one socket opened per domain it makes no sense to include PID number in
the file name.
Besides, remove socket's symlink creation.
Resolves:
https://github.com/SSSD/sssd/issues/5379
---
src/monitor/monitor.c | 2 +-
src/providers/data_provider/dp.c | 2 +-
src/sbus/connection/sbus_connection_connect.c | 3 +-
src/sbus/sbus.h | 4 -
src/sbus/sbus_private.h | 1 -
src/sbus/server/sbus_server.c | 202 +-
6 files changed, 9 insertions(+), 205 deletions(-)
diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c
index d9da05a511..99e4337006 100644
--- a/src/monitor/monitor.c
+++ b/src/monitor/monitor.c
@@ -2008,7 +2008,7 @@ static int monitor_process_init(struct mt_ctx *ctx,
req = sbus_server_create_and_connect_send(ctx, ctx->ev, SSS_BUS_MONITOR,
NULL, SSS_MONITOR_ADDRESS,
- false, 100, ctx->uid, ctx->gid,
+ 100, ctx->uid, ctx->gid,
NULL, NULL);
if (req == NULL) {
ret = ENOMEM;
diff --git a/src/providers/data_provider/dp.c b/src/providers/data_provider/dp.c
index 0858c43d2a..306841fb74 100644
--- a/src/providers/data_provider/dp.c
+++ b/src/providers/data_provider/dp.c
@@ -192,7 +192,7 @@ dp_init_send(TALLOC_CTX *mem_ctx,
talloc_set_destructor(state->provider, dp_destructor);
subreq = sbus_server_create_and_connect_send(state->provider, ev,
-state->sbus_name, NULL, sbus_address, true, 1000, uid, gid,
+state->sbus_name, NULL, sbus_address, 1000, uid, gid,
(sbus_server_on_connection_cb)dp_client_init,
(sbus_server_on_connection_data)state->provider);
if (subreq == NULL) {
diff --git a/src/sbus/connection/sbus_connection_connect.c b/src/sbus/connection/sbus_connection_connect.c
index 9cfe86206b..6e232492d0 100644
--- a/src/sbus/connection/sbus_connection_connect.c
+++ b/src/sbus/connection/sbus_connection_connect.c
@@ -341,7 +341,6 @@ sbus_server_create_and_connect_send(TALLOC_CTX *mem_ctx,
const char *dbus_name,
time_t *last_activity_time,
const char *address,
-bool use_symlink,
uint32_t max_connections,
uid_t uid,
gid_t gid,
@@ -359,7 +358,7 @@ sbus_server_create_and_connect_send(TALLOC_CTX *mem_ctx,
return NULL;
}
-state->server = sbus_server_create(state, ev, address, use_symlink,
+state->server = sbus_server_create(state, ev, address,
max_connections, uid, gid,
on_conn_cb, on_conn_data);
if (state->server == NULL) {
diff --git a/src/sbus/sbus.h b/src/sbus/sbus.h
index 0983879f0e..9c8faa3ed8 100644
--- a/src/sbus/sbus.h
+++ b/src/sbus/sbus.h
@@ -135,7 +135,6 @@ errno_t sbus_connect_private_recv(TALLOC_CTX *mem_ctx,
* @param mem_ctxMemory context.
* @param ev Tevent context.
* @param addressSocket address.
- * @param use_symlinkIf a symlink to @address should be created.
* @param uidSocket owner uid.
* @param gidSocket owner gid.
* @param on_conn_cb On new connection callback function.
@@ -147,7 +146,6 @@ struct sbus_server *
sbus_server_create(TALLOC_CTX *mem_ctx,
struct tevent_context *ev,
const char *address,
- bool use_symlink,
uint32_t max_connections,
uid_t uid,
gid_t gid,
@@ -163,7 +161,6 @@ sbus_server_create(TALLOC_CTX *mem_ctx,
* @param last_activity_time Pointer to a time that is updated each time
* an event occurs on connection.
* @param addressSocket address.
- * @param use_symlinkIf a symlink to @address should be created.
* @param uidSocket owner uid.
* @p
[SSSD] [sssd PR#5378][edited] sbus: change socket file name to sbus-dp_DOMAIN
URL: https://github.com/SSSD/sssd/pull/5378 Author: ikerexxe Title: #5378: sbus: change socket file name to sbus-dp_DOMAIN Action: edited Changed field: title Original value: """ WIP: sbus: change socket file name to sbus-dp_DOMAIN """ ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
[SSSD] [sssd PR#5378][edited] WIP: sbus: change socket file name to sbus-dp_DOMAIN
URL: https://github.com/SSSD/sssd/pull/5378 Author: ikerexxe Title: #5378: WIP: sbus: change socket file name to sbus-dp_DOMAIN Action: edited Changed field: body Original value: """ Socket file name changes from sbus-dp_DOMAIN_PID to sbus-dp_DOMAIN to avoid leaving orphaned socket files when sssd crashes. As there is only one socket opened per domain it makes no sense to include PID number in the file name. Besides, remove socket's symlink creation. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1875981 """ ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
[SSSD] [sssd PR#5378][comment] sbus: change socket file name to sbus-dp_DOMAIN
URL: https://github.com/SSSD/sssd/pull/5378 Title: #5378: sbus: change socket file name to sbus-dp_DOMAIN ikerexxe commented: """ I was also wondering for the historical reasons of adding the PID as a suffix to the socket file. Unfortunately, @pbrezina wasn't able to give any reason. Nowadays, each data provider has only one socket and even though it would be feasible to keep the PID suffix, in case of sssd crash the socket file would remain until a clean process is executed. This is what's happening right now and the customer reports more than 1000 orphaned socket files. Thus, I think it's more sensible to remove the PID from the suffix and use only the domain part. So that in case of failure there'll be only one file left and when sssd recovers it'll reuse it. """ See the full comment at https://github.com/SSSD/sssd/pull/5378#issuecomment-718815941 ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
[SSSD] [sssd PR#5378][+Waiting for review] sbus: change socket file name to sbus-dp_DOMAIN
URL: https://github.com/SSSD/sssd/pull/5378 Title: #5378: sbus: change socket file name to sbus-dp_DOMAIN Label: +Waiting for review ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
[SSSD] [sssd PR#5378][comment] sbus: change socket file name to sbus-dp_DOMAIN
URL: https://github.com/SSSD/sssd/pull/5378 Title: #5378: sbus: change socket file name to sbus-dp_DOMAIN ikerexxe commented: """ By the way, the tests that are failing in fedora-rawhide are in the multihost-pytest test suite. More specifically, in the setup phase of them. These same tests are working correctly in other fedora versions, so I suspect the problem isn't directly related with my changes. """ See the full comment at https://github.com/SSSD/sssd/pull/5378#issuecomment-718819866 ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
[SSSD] [sssd PR#5378][comment] sbus: change socket file name to sbus-dp_DOMAIN
URL: https://github.com/SSSD/sssd/pull/5378 Title: #5378: sbus: change socket file name to sbus-dp_DOMAIN ikerexxe commented: """ Reproduction steps seem easy: 1. start sssd 2. killall -STOP sssd_be 3. wait until monitor sends TERM to sssd_be and spawns a new one 4. killall -CONT sssd_be 5. getent passwd foobar But I've been unable so far to succeed in step 2 using a vanilla sssd, 2.4.0 more precisely. I send the signal to stop but sssd_be never stops. I've tried waiting some time (~2 minutes) and also checking if it's still alive by checking PID and executing step 5. Any ideas? """ See the full comment at https://github.com/SSSD/sssd/pull/5378#issuecomment-721238722 ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
[SSSD] [sssd PR#5378][comment] sbus: change socket file name to sbus-dp_DOMAIN
URL: https://github.com/SSSD/sssd/pull/5378 Title: #5378: sbus: change socket file name to sbus-dp_DOMAIN ikerexxe commented: """ I forgot to mention that SIGTERM and SIGKILL are correctly handled. """ See the full comment at https://github.com/SSSD/sssd/pull/5378#issuecomment-721239711 ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
[SSSD] [sssd PR#5378][comment] sbus: change socket file name to sbus-dp_DOMAIN
URL: https://github.com/SSSD/sssd/pull/5378 Title: #5378: sbus: change socket file name to sbus-dp_DOMAIN ikerexxe commented: """ As commented out by @alexey-tikhonov there's a great risk of a regress in https://bugzilla.redhat.com/show_bug.cgi?id=743841 with this change so I'll close the PR. """ See the full comment at https://github.com/SSSD/sssd/pull/5378#issuecomment-721617790 ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
[SSSD] [sssd PR#5378][closed] sbus: change socket file name to sbus-dp_DOMAIN
URL: https://github.com/SSSD/sssd/pull/5378 Author: ikerexxe Title: #5378: sbus: change socket file name to sbus-dp_DOMAIN Action: closed To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/5378/head:pr5378 git checkout pr5378 ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
[SSSD] [sssd PR#5381][comment] pytest multihost tests for sssd
URL: https://github.com/SSSD/sssd/pull/5381 Title: #5381: pytest multihost tests for sssd ikerexxe commented: """ There are white space issues. For all the instances that are failing take a look at: https://s3.eu-central-1.amazonaws.com/sssd-ci/PR-5381/5/fedora33/test-suite.log """ See the full comment at https://github.com/SSSD/sssd/pull/5381#issuecomment-723918772 ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
[SSSD] [sssd PR#5407][opened] kcm: check socket path loaded from configuration
URL: https://github.com/SSSD/sssd/pull/5407
Author: ikerexxe
Title: #5407: kcm: check socket path loaded from configuration
Action: opened
PR body:
"""
There are three major execution flows for this change:
1. If kcm socket path is not defined in sssd configuration, then log it and
fall back to the default location.
2. If kcm socket path is defined in sssd configuration but the location is
invalid, then log it and fall back to the default location.
3. If kcm socket path is defined in sssd configuration and the location is
valid, then use it.
Apart from that some unit-tests have been implement to check that the changes
work as expected.
I wonder if the changes included in confdb_get_string() should be ported to all
confdb_get_*() methods.
Resolves: https://github.com/SSSD/sssd/issues/5406
"""
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5407/head:pr5407
git checkout pr5407
From 318ad7e7d8fba67d4b84fd410ed01ff7991e37ae Mon Sep 17 00:00:00 2001
From: ikerexxe
Date: Tue, 17 Nov 2020 12:17:28 +0100
Subject: [PATCH 1/3] confdb: log message when falling back to default
Log message when string attribute is not found in database and value
falls back to default. Moreover, implement unit-test for
confdb_get_string() method.
Resolves: https://github.com/SSSD/sssd/issues/5406
---
src/confdb/confdb.c | 3 +
src/tests/cmocka/confdb/test_confdb.c | 91 +++
2 files changed, 94 insertions(+)
diff --git a/src/confdb/confdb.c b/src/confdb/confdb.c
index d2fc018fd0..7254d8c5ae 100644
--- a/src/confdb/confdb.c
+++ b/src/confdb/confdb.c
@@ -411,6 +411,9 @@ int confdb_get_string(struct confdb_ctx *cdb, TALLOC_CTX *ctx,
return EOK;
}
+DEBUG(SSSDBG_CONF_SETTINGS,
+ "Unable to get [%s] from [%s], falling back to default [%s]\n",
+ attribute, section, defstr);
/* Copy the default string */
restr = talloc_strdup(ctx, defstr);
}
diff --git a/src/tests/cmocka/confdb/test_confdb.c b/src/tests/cmocka/confdb/test_confdb.c
index daff41e5cc..7851945878 100644
--- a/src/tests/cmocka/confdb/test_confdb.c
+++ b/src/tests/cmocka/confdb/test_confdb.c
@@ -251,6 +251,88 @@ static void test_confdb_get_enabled_domain_list(void **state)
}
+static void test_confdb_get_string_value_present(void **state)
+{
+/*
+ * Given config_file_version exists in the configuration database
+ * And a default value is defined
+ * When config_file_version is requested
+ * Then config_file_version value is returned
+ */
+struct test_ctx *test_ctx = talloc_get_type(*state, struct test_ctx);
+TALLOC_CTX* tmp_ctx = talloc_new(NULL);
+char *confdb_value;
+int ret = EOK;
+
+ret = confdb_get_string(test_ctx->confdb,
+tmp_ctx,
+"config/sssd",
+"config_file_version",
+NULL,
+&confdb_value);
+
+assert_int_equal(EOK, ret);
+assert_string_equal("2", confdb_value);
+
+TALLOC_FREE(tmp_ctx);
+}
+
+
+static void test_confdb_get_string_value_not_present_fallback_default(void **state)
+{
+/*
+ * Given non_existing_value doesn't exist in the configuration database
+ * And a default value is defined
+ * When non_existing_value is requested
+ * Then default value is returned
+ */
+struct test_ctx *test_ctx = talloc_get_type(*state, struct test_ctx);
+TALLOC_CTX* tmp_ctx = talloc_new(NULL);
+char *confdb_value;
+const char *default_value = "3";
+int ret = EOK;
+
+ret = confdb_get_string(test_ctx->confdb,
+tmp_ctx,
+"config/sssd",
+"non_existing_value",
+default_value,
+&confdb_value);
+
+assert_int_equal(EOK, ret);
+assert_string_equal(default_value, confdb_value);
+
+TALLOC_FREE(tmp_ctx);
+}
+
+
+static void test_confdb_get_string_value_and_default_not_present(void **state)
+{
+/*
+ * Given non_existing_value doesn't exist in the configuration database
+ * And a default value isn't defined
+ * When non_existing_value is requested
+ * Then NULL is returned
+ */
+struct test_ctx *test_ctx = talloc_get_type(*state, struct test_ctx);
+TALLOC_CTX* tmp_ctx = talloc_new(NULL);
+char *confdb_value;
+int ret = EOK;
+
+ret = confdb_get_string(test_ctx->confdb,
+tmp_ctx,
+"config/sssd",
+"non_existing_value",
+NULL,
+&confdb_value);
+
+assert_int_equal(EOK, ret);
+asse
[SSSD] [sssd PR#5407][comment] kcm: check socket path loaded from configuration
URL: https://github.com/SSSD/sssd/pull/5407 Title: #5407: kcm: check socket path loaded from configuration ikerexxe commented: """ > (2) seems bad. Admin configuration shouldn't be overridden. If it won't work, > log that it won't work and (depending on project) either exit failure or > simply don't provide the functionality. Why should it fail? One advantage that I see when falling back to the default location is that the software continues working, so the user is able to use it. Is there any dependency that I'm missing? """ See the full comment at https://github.com/SSSD/sssd/pull/5407#issuecomment-729793252 ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
[SSSD] [sssd PR#5410][comment] Bz1734040
URL: https://github.com/SSSD/sssd/pull/5410 Title: #5410: Bz1734040 ikerexxe commented: """ I think that you should also clone bugzilla ticket to github ticket and use the last one on the commit message. If you don't know how to do clone the ticket ping me on IRC (@ipedrosa). Moreover, commit message is not very clear, can you improve it following the template? ``` COMPONENT: Subject Explanation Resolves: https://github.com/SSSD/sssd/issues/ ``` Finally, I think that you could squash all commits into one. """ See the full comment at https://github.com/SSSD/sssd/pull/5410#issuecomment-732057070 ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
[SSSD] [sssd PR#5407][synchronized] kcm: check socket path loaded from configuration
URL: https://github.com/SSSD/sssd/pull/5407
Author: ikerexxe
Title: #5407: kcm: check socket path loaded from configuration
Action: synchronized
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5407/head:pr5407
git checkout pr5407
From 318ad7e7d8fba67d4b84fd410ed01ff7991e37ae Mon Sep 17 00:00:00 2001
From: ikerexxe
Date: Tue, 17 Nov 2020 12:17:28 +0100
Subject: [PATCH 1/3] confdb: log message when falling back to default
Log message when string attribute is not found in database and value
falls back to default. Moreover, implement unit-test for
confdb_get_string() method.
Resolves: https://github.com/SSSD/sssd/issues/5406
---
src/confdb/confdb.c | 3 +
src/tests/cmocka/confdb/test_confdb.c | 91 +++
2 files changed, 94 insertions(+)
diff --git a/src/confdb/confdb.c b/src/confdb/confdb.c
index d2fc018fd0..7254d8c5ae 100644
--- a/src/confdb/confdb.c
+++ b/src/confdb/confdb.c
@@ -411,6 +411,9 @@ int confdb_get_string(struct confdb_ctx *cdb, TALLOC_CTX *ctx,
return EOK;
}
+DEBUG(SSSDBG_CONF_SETTINGS,
+ "Unable to get [%s] from [%s], falling back to default [%s]\n",
+ attribute, section, defstr);
/* Copy the default string */
restr = talloc_strdup(ctx, defstr);
}
diff --git a/src/tests/cmocka/confdb/test_confdb.c b/src/tests/cmocka/confdb/test_confdb.c
index daff41e5cc..7851945878 100644
--- a/src/tests/cmocka/confdb/test_confdb.c
+++ b/src/tests/cmocka/confdb/test_confdb.c
@@ -251,6 +251,88 @@ static void test_confdb_get_enabled_domain_list(void **state)
}
+static void test_confdb_get_string_value_present(void **state)
+{
+/*
+ * Given config_file_version exists in the configuration database
+ * And a default value is defined
+ * When config_file_version is requested
+ * Then config_file_version value is returned
+ */
+struct test_ctx *test_ctx = talloc_get_type(*state, struct test_ctx);
+TALLOC_CTX* tmp_ctx = talloc_new(NULL);
+char *confdb_value;
+int ret = EOK;
+
+ret = confdb_get_string(test_ctx->confdb,
+tmp_ctx,
+"config/sssd",
+"config_file_version",
+NULL,
+&confdb_value);
+
+assert_int_equal(EOK, ret);
+assert_string_equal("2", confdb_value);
+
+TALLOC_FREE(tmp_ctx);
+}
+
+
+static void test_confdb_get_string_value_not_present_fallback_default(void **state)
+{
+/*
+ * Given non_existing_value doesn't exist in the configuration database
+ * And a default value is defined
+ * When non_existing_value is requested
+ * Then default value is returned
+ */
+struct test_ctx *test_ctx = talloc_get_type(*state, struct test_ctx);
+TALLOC_CTX* tmp_ctx = talloc_new(NULL);
+char *confdb_value;
+const char *default_value = "3";
+int ret = EOK;
+
+ret = confdb_get_string(test_ctx->confdb,
+tmp_ctx,
+"config/sssd",
+"non_existing_value",
+default_value,
+&confdb_value);
+
+assert_int_equal(EOK, ret);
+assert_string_equal(default_value, confdb_value);
+
+TALLOC_FREE(tmp_ctx);
+}
+
+
+static void test_confdb_get_string_value_and_default_not_present(void **state)
+{
+/*
+ * Given non_existing_value doesn't exist in the configuration database
+ * And a default value isn't defined
+ * When non_existing_value is requested
+ * Then NULL is returned
+ */
+struct test_ctx *test_ctx = talloc_get_type(*state, struct test_ctx);
+TALLOC_CTX* tmp_ctx = talloc_new(NULL);
+char *confdb_value;
+int ret = EOK;
+
+ret = confdb_get_string(test_ctx->confdb,
+tmp_ctx,
+"config/sssd",
+"non_existing_value",
+NULL,
+&confdb_value);
+
+assert_int_equal(EOK, ret);
+assert_null(confdb_value);
+
+TALLOC_FREE(tmp_ctx);
+}
+
+
int main(int argc, const char *argv[])
{
poptContext pc;
@@ -272,6 +354,15 @@ int main(int argc, const char *argv[])
cmocka_unit_test_setup_teardown(test_confdb_get_enabled_domain_list,
confdb_test_setup,
confdb_test_teardown),
+cmocka_unit_test_setup_teardown(test_confdb_get_string_value_present,
+confdb_test_setup,
+confdb_test_teardown),
+cmocka_unit_test_setup_teardown(test_confdb_get_string_value_not_present_fallback_defa
[SSSD] [sssd PR#5407][edited] kcm: check socket path loaded from configuration
URL: https://github.com/SSSD/sssd/pull/5407 Author: ikerexxe Title: #5407: kcm: check socket path loaded from configuration Action: edited Changed field: body Original value: """ There are three major execution flows for this change: 1. If kcm socket path is not defined in sssd configuration, then log it and fall back to the default location. 2. If kcm socket path is defined in sssd configuration but the location is invalid, then log it and fall back to the default location. 3. If kcm socket path is defined in sssd configuration and the location is valid, then use it. Apart from that some unit-tests have been implement to check that the changes work as expected. I wonder if the changes included in confdb_get_string() should be ported to all confdb_get_*() methods. Resolves: https://github.com/SSSD/sssd/issues/5406 """ ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
[SSSD] [sssd PR#5407][comment] kcm: check socket path loaded from configuration
URL: https://github.com/SSSD/sssd/pull/5407 Title: #5407: kcm: check socket path loaded from configuration ikerexxe commented: """ I've updated the PR description and the code taking into account @frozencemetery comments. """ See the full comment at https://github.com/SSSD/sssd/pull/5407#issuecomment-732229823 ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
[SSSD] [sssd PR#5407][+Waiting for review] kcm: check socket path loaded from configuration
URL: https://github.com/SSSD/sssd/pull/5407 Title: #5407: kcm: check socket path loaded from configuration Label: +Waiting for review ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
[SSSD] [sssd PR#5410][comment] Bz1734040
URL: https://github.com/SSSD/sssd/pull/5410 Title: #5410: Bz1734040 ikerexxe commented: """ > > I think that you should also clone bugzilla ticket to github ticket and use > > the last one on the commit message > > ... > > Resolves: https://github.com/SSSD/sssd/issues/ > > This is test automation, this patch itself doesn't resolve the issue. So I'm > not sure if it's correct to state "resolves" here. Definitely `Resolves` is not the correct keyword here but there should be some link. Maybe `Test` or `Check` are better words. > > Moreover, usually BZ is already cloned to the upstream to the moment test is > done (in this case this is #5295) Thanks for pointing the upstream ticket. """ See the full comment at https://github.com/SSSD/sssd/pull/5410#issuecomment-732231499 ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
[SSSD] [sssd PR#5407][synchronized] kcm: check socket path loaded from configuration
URL: https://github.com/SSSD/sssd/pull/5407
Author: ikerexxe
Title: #5407: kcm: check socket path loaded from configuration
Action: synchronized
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5407/head:pr5407
git checkout pr5407
From 318ad7e7d8fba67d4b84fd410ed01ff7991e37ae Mon Sep 17 00:00:00 2001
From: ikerexxe
Date: Tue, 17 Nov 2020 12:17:28 +0100
Subject: [PATCH 1/3] confdb: log message when falling back to default
Log message when string attribute is not found in database and value
falls back to default. Moreover, implement unit-test for
confdb_get_string() method.
Resolves: https://github.com/SSSD/sssd/issues/5406
---
src/confdb/confdb.c | 3 +
src/tests/cmocka/confdb/test_confdb.c | 91 +++
2 files changed, 94 insertions(+)
diff --git a/src/confdb/confdb.c b/src/confdb/confdb.c
index d2fc018fd0..7254d8c5ae 100644
--- a/src/confdb/confdb.c
+++ b/src/confdb/confdb.c
@@ -411,6 +411,9 @@ int confdb_get_string(struct confdb_ctx *cdb, TALLOC_CTX *ctx,
return EOK;
}
+DEBUG(SSSDBG_CONF_SETTINGS,
+ "Unable to get [%s] from [%s], falling back to default [%s]\n",
+ attribute, section, defstr);
/* Copy the default string */
restr = talloc_strdup(ctx, defstr);
}
diff --git a/src/tests/cmocka/confdb/test_confdb.c b/src/tests/cmocka/confdb/test_confdb.c
index daff41e5cc..7851945878 100644
--- a/src/tests/cmocka/confdb/test_confdb.c
+++ b/src/tests/cmocka/confdb/test_confdb.c
@@ -251,6 +251,88 @@ static void test_confdb_get_enabled_domain_list(void **state)
}
+static void test_confdb_get_string_value_present(void **state)
+{
+/*
+ * Given config_file_version exists in the configuration database
+ * And a default value is defined
+ * When config_file_version is requested
+ * Then config_file_version value is returned
+ */
+struct test_ctx *test_ctx = talloc_get_type(*state, struct test_ctx);
+TALLOC_CTX* tmp_ctx = talloc_new(NULL);
+char *confdb_value;
+int ret = EOK;
+
+ret = confdb_get_string(test_ctx->confdb,
+tmp_ctx,
+"config/sssd",
+"config_file_version",
+NULL,
+&confdb_value);
+
+assert_int_equal(EOK, ret);
+assert_string_equal("2", confdb_value);
+
+TALLOC_FREE(tmp_ctx);
+}
+
+
+static void test_confdb_get_string_value_not_present_fallback_default(void **state)
+{
+/*
+ * Given non_existing_value doesn't exist in the configuration database
+ * And a default value is defined
+ * When non_existing_value is requested
+ * Then default value is returned
+ */
+struct test_ctx *test_ctx = talloc_get_type(*state, struct test_ctx);
+TALLOC_CTX* tmp_ctx = talloc_new(NULL);
+char *confdb_value;
+const char *default_value = "3";
+int ret = EOK;
+
+ret = confdb_get_string(test_ctx->confdb,
+tmp_ctx,
+"config/sssd",
+"non_existing_value",
+default_value,
+&confdb_value);
+
+assert_int_equal(EOK, ret);
+assert_string_equal(default_value, confdb_value);
+
+TALLOC_FREE(tmp_ctx);
+}
+
+
+static void test_confdb_get_string_value_and_default_not_present(void **state)
+{
+/*
+ * Given non_existing_value doesn't exist in the configuration database
+ * And a default value isn't defined
+ * When non_existing_value is requested
+ * Then NULL is returned
+ */
+struct test_ctx *test_ctx = talloc_get_type(*state, struct test_ctx);
+TALLOC_CTX* tmp_ctx = talloc_new(NULL);
+char *confdb_value;
+int ret = EOK;
+
+ret = confdb_get_string(test_ctx->confdb,
+tmp_ctx,
+"config/sssd",
+"non_existing_value",
+NULL,
+&confdb_value);
+
+assert_int_equal(EOK, ret);
+assert_null(confdb_value);
+
+TALLOC_FREE(tmp_ctx);
+}
+
+
int main(int argc, const char *argv[])
{
poptContext pc;
@@ -272,6 +354,15 @@ int main(int argc, const char *argv[])
cmocka_unit_test_setup_teardown(test_confdb_get_enabled_domain_list,
confdb_test_setup,
confdb_test_teardown),
+cmocka_unit_test_setup_teardown(test_confdb_get_string_value_present,
+confdb_test_setup,
+confdb_test_teardown),
+cmocka_unit_test_setup_teardown(test_confdb_get_string_value_not_present_fallback_defa
[SSSD] [sssd PR#5407][synchronized] kcm: check socket path loaded from configuration
URL: https://github.com/SSSD/sssd/pull/5407
Author: ikerexxe
Title: #5407: kcm: check socket path loaded from configuration
Action: synchronized
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5407/head:pr5407
git checkout pr5407
From 318ad7e7d8fba67d4b84fd410ed01ff7991e37ae Mon Sep 17 00:00:00 2001
From: ikerexxe
Date: Tue, 17 Nov 2020 12:17:28 +0100
Subject: [PATCH 1/3] confdb: log message when falling back to default
Log message when string attribute is not found in database and value
falls back to default. Moreover, implement unit-test for
confdb_get_string() method.
Resolves: https://github.com/SSSD/sssd/issues/5406
---
src/confdb/confdb.c | 3 +
src/tests/cmocka/confdb/test_confdb.c | 91 +++
2 files changed, 94 insertions(+)
diff --git a/src/confdb/confdb.c b/src/confdb/confdb.c
index d2fc018fd0..7254d8c5ae 100644
--- a/src/confdb/confdb.c
+++ b/src/confdb/confdb.c
@@ -411,6 +411,9 @@ int confdb_get_string(struct confdb_ctx *cdb, TALLOC_CTX *ctx,
return EOK;
}
+DEBUG(SSSDBG_CONF_SETTINGS,
+ "Unable to get [%s] from [%s], falling back to default [%s]\n",
+ attribute, section, defstr);
/* Copy the default string */
restr = talloc_strdup(ctx, defstr);
}
diff --git a/src/tests/cmocka/confdb/test_confdb.c b/src/tests/cmocka/confdb/test_confdb.c
index daff41e5cc..7851945878 100644
--- a/src/tests/cmocka/confdb/test_confdb.c
+++ b/src/tests/cmocka/confdb/test_confdb.c
@@ -251,6 +251,88 @@ static void test_confdb_get_enabled_domain_list(void **state)
}
+static void test_confdb_get_string_value_present(void **state)
+{
+/*
+ * Given config_file_version exists in the configuration database
+ * And a default value is defined
+ * When config_file_version is requested
+ * Then config_file_version value is returned
+ */
+struct test_ctx *test_ctx = talloc_get_type(*state, struct test_ctx);
+TALLOC_CTX* tmp_ctx = talloc_new(NULL);
+char *confdb_value;
+int ret = EOK;
+
+ret = confdb_get_string(test_ctx->confdb,
+tmp_ctx,
+"config/sssd",
+"config_file_version",
+NULL,
+&confdb_value);
+
+assert_int_equal(EOK, ret);
+assert_string_equal("2", confdb_value);
+
+TALLOC_FREE(tmp_ctx);
+}
+
+
+static void test_confdb_get_string_value_not_present_fallback_default(void **state)
+{
+/*
+ * Given non_existing_value doesn't exist in the configuration database
+ * And a default value is defined
+ * When non_existing_value is requested
+ * Then default value is returned
+ */
+struct test_ctx *test_ctx = talloc_get_type(*state, struct test_ctx);
+TALLOC_CTX* tmp_ctx = talloc_new(NULL);
+char *confdb_value;
+const char *default_value = "3";
+int ret = EOK;
+
+ret = confdb_get_string(test_ctx->confdb,
+tmp_ctx,
+"config/sssd",
+"non_existing_value",
+default_value,
+&confdb_value);
+
+assert_int_equal(EOK, ret);
+assert_string_equal(default_value, confdb_value);
+
+TALLOC_FREE(tmp_ctx);
+}
+
+
+static void test_confdb_get_string_value_and_default_not_present(void **state)
+{
+/*
+ * Given non_existing_value doesn't exist in the configuration database
+ * And a default value isn't defined
+ * When non_existing_value is requested
+ * Then NULL is returned
+ */
+struct test_ctx *test_ctx = talloc_get_type(*state, struct test_ctx);
+TALLOC_CTX* tmp_ctx = talloc_new(NULL);
+char *confdb_value;
+int ret = EOK;
+
+ret = confdb_get_string(test_ctx->confdb,
+tmp_ctx,
+"config/sssd",
+"non_existing_value",
+NULL,
+&confdb_value);
+
+assert_int_equal(EOK, ret);
+assert_null(confdb_value);
+
+TALLOC_FREE(tmp_ctx);
+}
+
+
int main(int argc, const char *argv[])
{
poptContext pc;
@@ -272,6 +354,15 @@ int main(int argc, const char *argv[])
cmocka_unit_test_setup_teardown(test_confdb_get_enabled_domain_list,
confdb_test_setup,
confdb_test_teardown),
+cmocka_unit_test_setup_teardown(test_confdb_get_string_value_present,
+confdb_test_setup,
+confdb_test_teardown),
+cmocka_unit_test_setup_teardown(test_confdb_get_string_value_not_present_fallback_defa
[SSSD] [sssd PR#5407][comment] kcm: check socket path loaded from configuration
URL: https://github.com/SSSD/sssd/pull/5407 Title: #5407: kcm: check socket path loaded from configuration ikerexxe commented: """ Fedora-rawhide failures are not related with this change as it's also failing in other PRs. I don't see rhel8 failing anywhere, so maybe there's some problem parsing the results. Moreover, this is also "failing" in other PRs. """ See the full comment at https://github.com/SSSD/sssd/pull/5407#issuecomment-733640636 ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
[SSSD] [sssd PR#5410][comment] Test: AD: For crash in ad_get_account_domain_search
URL: https://github.com/SSSD/sssd/pull/5410 Title: #5410: Test: AD: For crash in ad_get_account_domain_search ikerexxe commented: """ Let's wait for CI results but it also looks good to me. """ See the full comment at https://github.com/SSSD/sssd/pull/5410#issuecomment-736529136 ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
[SSSD] [sssd PR#5407][comment] kcm: check socket path loaded from configuration
URL: https://github.com/SSSD/sssd/pull/5407
Title: #5407: kcm: check socket path loaded from configuration
ikerexxe commented:
"""
> I have several comments to the code. However, I'm sorry, but I don't think
> this is correct way to solve it. This is not a problem of KCM only, but it
> affects all responders because all sockets are activated in
> `activate_unix_sockets()` so it should be also solved here. Obviously, if the
> socket path is invalid then `bind()` should fail, so the question is why it
> succeeds? I don't know, my guess is that systemd socket activation is somehow
> affecting it - maybe because it already created a socket for us and the
> socket name is defined in kcm.socket unit file instead of sssd.conf so the
> one in sssd.conf is ignored? Probably, because if socket is already created
> by systemd, SSSD just skips its creation: (from `set_unix_socket()`:
I think I'm missing something from your point. IIUC, sssd-kcm loads the socket
path from sssd.conf, whilst kcm.socket unit file loads it from its own unit
file configuration. If sssd.conf and unit file path are the same, then both
processes should fail because the path isn't valid. Otherwise, only sssd-kcm
should fail because sssd.conf socket path is invalid. As said, I must be
missing something because either sssd-kcm or kcm.socket unit file should fail.
>
> ```c
> if (rctx->lfd == -1) {
> ret = create_pipe_fd(rctx->sock_name, &rctx->lfd, SCKT_RSP_UMASK);
> if (ret != EOK) {
> return ret;
> }
> }
> ```
>
> So KCM always binds to whatever is set in kcm.socket. So the question is -
> should we take kcm.socket unit file for granted (and print error if systemd
> socket != sssd.conf socket) or should we open a new socket in this case?
> Probably the first thing, otherwise socket activation will stop working.
I don't think opening a new socket is a solution, so I'd prefer to print an
error if paths are different.
And now, going back to the first part of your comment.
> I have several comments to the code. However, I'm sorry, but I don't think
> this is correct way to solve it. This is not a problem of KCM only, but it
> affects all responders because all sockets are activated in
> `activate_unix_sockets()` so it should be also solved here.
If we choose to print an error, should we implement some algorithm that checks
if the responder can be activated by systemd and check the path provided in the
unit file against the one provided in sssd.conf?
"""
See the full comment at
https://github.com/SSSD/sssd/pull/5407#issuecomment-743277752
___
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://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives:
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
[SSSD] [sssd PR#5423][+Changes requested] Offline timeout man update
URL: https://github.com/SSSD/sssd/pull/5423 Title: #5423: Offline timeout man update Label: +Changes requested ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
[SSSD] [sssd PR#5423][-Waiting for review] Offline timeout man update
URL: https://github.com/SSSD/sssd/pull/5423 Title: #5423: Offline timeout man update Label: -Waiting for review ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
[SSSD] [sssd PR#5423][comment] Offline timeout man update
URL: https://github.com/SSSD/sssd/pull/5423 Title: #5423: Offline timeout man update ikerexxe commented: """ I've reopened one of the issues as I haven't seen the change applied. Moreover, the commit message is still pointing to the bugzilla link instead of the github reference. """ See the full comment at https://github.com/SSSD/sssd/pull/5423#issuecomment-749404219 ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
[SSSD] [sssd PR#5423][+Changes requested] Offline timeout man update
URL: https://github.com/SSSD/sssd/pull/5423 Title: #5423: Offline timeout man update Label: +Changes requested ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
[SSSD] [sssd PR#5423][-Waiting for review] Offline timeout man update
URL: https://github.com/SSSD/sssd/pull/5423 Title: #5423: Offline timeout man update Label: -Waiting for review ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
[SSSD] [sssd PR#5423][-Changes requested] Offline timeout man update
URL: https://github.com/SSSD/sssd/pull/5423 Title: #5423: Offline timeout man update Label: -Changes requested ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
[SSSD] [sssd PR#5423][+Accepted] Offline timeout man update
URL: https://github.com/SSSD/sssd/pull/5423 Title: #5423: Offline timeout man update Label: +Accepted ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
[SSSD] [sssd PR#5407][comment] kcm: check socket path loaded from configuration
URL: https://github.com/SSSD/sssd/pull/5407 Title: #5407: kcm: check socket path loaded from configuration ikerexxe commented: """ > Does this make sense? Yes, now it makes sense why systemd activated sssd-kcm works correctly even though the socket path in sssd.conf is incorrect. > Therefore IMHO the solution would be to print error if systemd socket != > sssd.conf socket and document that socket patch should be changed in the unit > file and not in sssd.conf. I also agree that it is best to print an error if the two paths are different. But what happens to those distributions that don't use systemd? Should we add another check to see whether systemd socket is not empty and then compare it with sssd.conf? Moreover, does the previous check also apply to other socket initializations? """ See the full comment at https://github.com/SSSD/sssd/pull/5407#issuecomment-756213201 ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
[SSSD] [sssd PR#5407][comment] kcm: check socket path loaded from configuration
URL: https://github.com/SSSD/sssd/pull/5407 Title: #5407: kcm: check socket path loaded from configuration ikerexxe commented: """ > > Moreover, does the previous check also apply to other socket > > initializations? > > I'm sorry, I am not entirely sure what you ask. Can you rephrase it? Does the check only apply to sssd-kcm? Or does it apply to other responders like sssd-secrets? """ See the full comment at https://github.com/SSSD/sssd/pull/5407#issuecomment-756795660 ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
[SSSD] [sssd PR#5452][comment] RESOLV: handle fail of ares_parse_*_reply() properly
URL: https://github.com/SSSD/sssd/pull/5452 Title: #5452: RESOLV: handle fail of ares_parse_*_reply() properly ikerexxe commented: """ First of all thank you for taking care of this bug. I've reviewed the changes and something worries me. Should we add an else statement with a debug line to indicate that the name resolve has failed? By the way, the CI errors in fedora-rawhide aren't related with your change. """ See the full comment at https://github.com/SSSD/sssd/pull/5452#issuecomment-757772874 ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
[SSSD] [sssd PR#5407][comment] kcm: check socket path loaded from configuration
URL: https://github.com/SSSD/sssd/pull/5407 Title: #5407: kcm: check socket path loaded from configuration ikerexxe commented: """ Annotated. In that case I'll change the PR so that it handles this type of error in all responders (responder_common.c file) and not only in sssd-kcm. """ See the full comment at https://github.com/SSSD/sssd/pull/5407#issuecomment-757896218 ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
[SSSD] [sssd PR#5452][-Waiting for review] RESOLV: handle fail of ares_parse_*_reply() properly
URL: https://github.com/SSSD/sssd/pull/5452 Title: #5452: RESOLV: handle fail of ares_parse_*_reply() properly Label: -Waiting for review ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
[SSSD] [sssd PR#5452][+Accepted] RESOLV: handle fail of ares_parse_*_reply() properly
URL: https://github.com/SSSD/sssd/pull/5452 Title: #5452: RESOLV: handle fail of ares_parse_*_reply() properly Label: +Accepted ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
[SSSD] [sssd PR#5472][+Waiting for review] man: sss_override clarification
URL: https://github.com/SSSD/sssd/pull/5472 Title: #5472: man: sss_override clarification Label: +Waiting for review ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
[SSSD] [sssd PR#5472][-Waiting for review] man: sss_override clarification
URL: https://github.com/SSSD/sssd/pull/5472 Title: #5472: man: sss_override clarification Label: -Waiting for review ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
[SSSD] [sssd PR#5472][+Waiting for review] man: sss_override clarification
URL: https://github.com/SSSD/sssd/pull/5472 Title: #5472: man: sss_override clarification Label: +Waiting for review ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
[SSSD] [sssd PR#5472][+Bugzilla] man: sss_override clarification
URL: https://github.com/SSSD/sssd/pull/5472 Title: #5472: man: sss_override clarification Label: +Bugzilla ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
[SSSD] [sssd PR#5407][synchronized] kcm: check socket path loaded from configuration
URL: https://github.com/SSSD/sssd/pull/5407
Author: ikerexxe
Title: #5407: kcm: check socket path loaded from configuration
Action: synchronized
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5407/head:pr5407
git checkout pr5407
From 318fd555393058de0e1c2d22f4e2f6b4c7b3b174 Mon Sep 17 00:00:00 2001
From: ikerexxe
Date: Tue, 26 Jan 2021 12:37:15 +0100
Subject: [PATCH 1/2] RESPONDER: check that configured sockets match
Check if the sockets defined in systemd unit and sssd.conf match. If
they don't, then print a warning message.
Moreover, change man page regarding socket_path option to indicate that
it will be overwritten by systemd's unit file.
Resolves: https://github.com/SSSD/sssd/issues/5406
---
src/man/sssd-kcm.8.xml | 7 ++
src/responder/common/responder_common.c | 103
2 files changed, 110 insertions(+)
diff --git a/src/man/sssd-kcm.8.xml b/src/man/sssd-kcm.8.xml
index 022a74ba09..535af27375 100644
--- a/src/man/sssd-kcm.8.xml
+++ b/src/man/sssd-kcm.8.xml
@@ -203,6 +203,13 @@ systemctl restart sssd-kcm.service
Default: /var/run/.heim_org.h5l.kcm-socket
+
+
+Note: on platforms where systemd is supported, the
+socket path is overwritten by the one defined in
+the unit file.
+
+
diff --git a/src/responder/common/responder_common.c b/src/responder/common/responder_common.c
index 7061d018a6..3b98b21149 100644
--- a/src/responder/common/responder_common.c
+++ b/src/responder/common/responder_common.c
@@ -895,12 +895,106 @@ int create_pipe_fd(const char *sock_name, int *_fd, mode_t umaskval)
return ret;
}
+static int
+get_socket_path_in_use(struct resp_ctx *rctx, char **socket_path_in_use)
+{
+TALLOC_CTX *tmp_ctx;
+struct stat statbuf;
+char *path_name = NULL;
+char *inode = NULL;
+char *line = NULL;
+char *socket_line = NULL;
+char *inode_pos = NULL;
+errno_t ret;
+FILE *fp;
+size_t len = 0;
+
+tmp_ctx = talloc_new(NULL);
+if (tmp_ctx == NULL) {
+ret = ENOMEM;
+goto done;
+}
+
+path_name = talloc_strdup(tmp_ctx, "");
+path_name = talloc_asprintf_append(path_name, "%s%d", "/proc/self/fd/",
+ rctx->lfd);
+
+ret = stat(path_name, &statbuf);
+if (ret != EOK) {
+goto done;
+}
+
+fp = fopen("/proc/net/unix", "r");
+if (fp == NULL) {
+ret = ENOENT;
+goto done;
+}
+
+inode = talloc_strdup(tmp_ctx, "");
+inode = talloc_asprintf_append(inode, "%lu", statbuf.st_ino);
+
+while ((getline(&line, &len, fp) != -1) && (socket_line == NULL)) {
+inode_pos = strstr(line, inode);
+
+if (inode_pos != NULL) {
+/* copy the string except for the inode number at the beginning
+ * and the breaking line at the end */
+socket_line = talloc_strndup(tmp_ctx,
+ inode_pos + strlen(inode) + 1,
+ strlen(inode_pos) - strlen(inode) - 2);
+}
+}
+
+if (socket_line == NULL) {
+ret = ENOENT;
+goto done;
+}
+
+*socket_path_in_use = talloc_steal(rctx, socket_line);
+ret = EOK;
+
+done:
+fclose(fp);
+talloc_free(tmp_ctx);
+
+return ret;
+}
+
+static bool
+compare_files(const char *file_path1, const char *file_path2)
+{
+errno_t ret;
+struct stat statbuf1;
+struct stat statbuf2;
+
+if (strcmp(file_path1, file_path2) == 0) {
+return true;
+}
+
+ret = stat(file_path1, &statbuf1);
+if (ret != EOK) {
+return false;
+}
+
+ret = stat(file_path2, &statbuf2);
+if (ret != EOK) {
+return false;
+}
+
+if (statbuf1.st_ino == statbuf2.st_ino) {
+return true;
+}
+
+return false;
+}
+
/* create a unix socket and listen to it */
static int set_unix_socket(struct resp_ctx *rctx,
connection_setup_t conn_setup)
{
errno_t ret;
struct accept_fd_ctx *accept_ctx = NULL;
+char *socket_path_in_use = NULL;
/* for future use */
#if 0
@@ -947,6 +1041,15 @@ static int set_unix_socket(struct resp_ctx *rctx,
return ret;
}
}
+/* Check if the sockets defined in systemd unit and sssd.conf match */
+else if (rctx->lfd == SD_LISTEN_FDS_START) {
+if ((get_socket_path_in_use(rctx, &socket_path_in_use) == EOK) &&
+(compare_files(rctx->sock_name, socket_path_in_use) == false)) {
+
[SSSD] [sssd PR#5407][synchronized] kcm: check socket path loaded from configuration
URL: https://github.com/SSSD/sssd/pull/5407
Author: ikerexxe
Title: #5407: kcm: check socket path loaded from configuration
Action: synchronized
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5407/head:pr5407
git checkout pr5407
From 318fd555393058de0e1c2d22f4e2f6b4c7b3b174 Mon Sep 17 00:00:00 2001
From: ikerexxe
Date: Tue, 26 Jan 2021 12:37:15 +0100
Subject: [PATCH 1/2] RESPONDER: check that configured sockets match
Check if the sockets defined in systemd unit and sssd.conf match. If
they don't, then print a warning message.
Moreover, change man page regarding socket_path option to indicate that
it will be overwritten by systemd's unit file.
Resolves: https://github.com/SSSD/sssd/issues/5406
---
src/man/sssd-kcm.8.xml | 7 ++
src/responder/common/responder_common.c | 103
2 files changed, 110 insertions(+)
diff --git a/src/man/sssd-kcm.8.xml b/src/man/sssd-kcm.8.xml
index 022a74ba09..535af27375 100644
--- a/src/man/sssd-kcm.8.xml
+++ b/src/man/sssd-kcm.8.xml
@@ -203,6 +203,13 @@ systemctl restart sssd-kcm.service
Default: /var/run/.heim_org.h5l.kcm-socket
+
+
+Note: on platforms where systemd is supported, the
+socket path is overwritten by the one defined in
+the unit file.
+
+
diff --git a/src/responder/common/responder_common.c b/src/responder/common/responder_common.c
index 7061d018a6..3b98b21149 100644
--- a/src/responder/common/responder_common.c
+++ b/src/responder/common/responder_common.c
@@ -895,12 +895,106 @@ int create_pipe_fd(const char *sock_name, int *_fd, mode_t umaskval)
return ret;
}
+static int
+get_socket_path_in_use(struct resp_ctx *rctx, char **socket_path_in_use)
+{
+TALLOC_CTX *tmp_ctx;
+struct stat statbuf;
+char *path_name = NULL;
+char *inode = NULL;
+char *line = NULL;
+char *socket_line = NULL;
+char *inode_pos = NULL;
+errno_t ret;
+FILE *fp;
+size_t len = 0;
+
+tmp_ctx = talloc_new(NULL);
+if (tmp_ctx == NULL) {
+ret = ENOMEM;
+goto done;
+}
+
+path_name = talloc_strdup(tmp_ctx, "");
+path_name = talloc_asprintf_append(path_name, "%s%d", "/proc/self/fd/",
+ rctx->lfd);
+
+ret = stat(path_name, &statbuf);
+if (ret != EOK) {
+goto done;
+}
+
+fp = fopen("/proc/net/unix", "r");
+if (fp == NULL) {
+ret = ENOENT;
+goto done;
+}
+
+inode = talloc_strdup(tmp_ctx, "");
+inode = talloc_asprintf_append(inode, "%lu", statbuf.st_ino);
+
+while ((getline(&line, &len, fp) != -1) && (socket_line == NULL)) {
+inode_pos = strstr(line, inode);
+
+if (inode_pos != NULL) {
+/* copy the string except for the inode number at the beginning
+ * and the breaking line at the end */
+socket_line = talloc_strndup(tmp_ctx,
+ inode_pos + strlen(inode) + 1,
+ strlen(inode_pos) - strlen(inode) - 2);
+}
+}
+
+if (socket_line == NULL) {
+ret = ENOENT;
+goto done;
+}
+
+*socket_path_in_use = talloc_steal(rctx, socket_line);
+ret = EOK;
+
+done:
+fclose(fp);
+talloc_free(tmp_ctx);
+
+return ret;
+}
+
+static bool
+compare_files(const char *file_path1, const char *file_path2)
+{
+errno_t ret;
+struct stat statbuf1;
+struct stat statbuf2;
+
+if (strcmp(file_path1, file_path2) == 0) {
+return true;
+}
+
+ret = stat(file_path1, &statbuf1);
+if (ret != EOK) {
+return false;
+}
+
+ret = stat(file_path2, &statbuf2);
+if (ret != EOK) {
+return false;
+}
+
+if (statbuf1.st_ino == statbuf2.st_ino) {
+return true;
+}
+
+return false;
+}
+
/* create a unix socket and listen to it */
static int set_unix_socket(struct resp_ctx *rctx,
connection_setup_t conn_setup)
{
errno_t ret;
struct accept_fd_ctx *accept_ctx = NULL;
+char *socket_path_in_use = NULL;
/* for future use */
#if 0
@@ -947,6 +1041,15 @@ static int set_unix_socket(struct resp_ctx *rctx,
return ret;
}
}
+/* Check if the sockets defined in systemd unit and sssd.conf match */
+else if (rctx->lfd == SD_LISTEN_FDS_START) {
+if ((get_socket_path_in_use(rctx, &socket_path_in_use) == EOK) &&
+(compare_files(rctx->sock_name, socket_path_in_use) == false)) {
+
[SSSD] [sssd PR#5407][comment] kcm: check socket path loaded from configuration
URL: https://github.com/SSSD/sssd/pull/5407 Title: #5407: kcm: check socket path loaded from configuration ikerexxe commented: """ I've updated the PR with the changes requested by Pavel. The changes include a check of the socket path in use by the responder, which is defined in systemd, and the socket path defined in sssd.conf. If the paths are different then a warning is printed. Moreover, the man page has been updated to take into account that sssd_path option included in sssd.conf can be overwritten by systemd. Finally, a multihost test has been added to test the previous implementation. @sgoveas can you check it? I remember that there are some things to take into account if we don't want to include the test downstream until the patch has been applied, but I don't recall exactly what has to be done. """ See the full comment at https://github.com/SSSD/sssd/pull/5407#issuecomment-767619125 ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
[SSSD] [sssd PR#5506][+Waiting for review] SSSD Log: write_krb5info_file word replacement
URL: https://github.com/SSSD/sssd/pull/5506 Title: #5506: SSSD Log: write_krb5info_file word replacement Label: +Waiting for review ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[SSSD] [sssd PR#5407][synchronized] kcm: check socket path loaded from configuration
URL: https://github.com/SSSD/sssd/pull/5407
Author: ikerexxe
Title: #5407: kcm: check socket path loaded from configuration
Action: synchronized
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5407/head:pr5407
git checkout pr5407
From d5f47e1f1ba2279907c98ce7ff0595443730509a Mon Sep 17 00:00:00 2001
From: ikerexxe
Date: Tue, 26 Jan 2021 12:37:15 +0100
Subject: [PATCH 1/2] RESPONDER: check that configured sockets match
Check if the sockets defined in systemd unit and sssd.conf match. If
they don't, then print a warning message.
Moreover, change man page regarding socket_path option to indicate that
it will be overwritten by systemd's unit file.
Resolves: https://github.com/SSSD/sssd/issues/5406
---
src/man/sssd-kcm.8.xml | 7 +++
src/responder/common/responder_common.c | 84 -
2 files changed, 90 insertions(+), 1 deletion(-)
diff --git a/src/man/sssd-kcm.8.xml b/src/man/sssd-kcm.8.xml
index 022a74ba09..14ba122a5c 100644
--- a/src/man/sssd-kcm.8.xml
+++ b/src/man/sssd-kcm.8.xml
@@ -203,6 +203,13 @@ systemctl restart sssd-kcm.service
Default: /var/run/.heim_org.h5l.kcm-socket
+
+
+Note: on platforms where systemd is supported, the
+socket path is overwritten by the one defined in
+the sssd-kcm.socket unit file.
+
+
diff --git a/src/responder/common/responder_common.c b/src/responder/common/responder_common.c
index 7061d018a6..00f1b3858f 100644
--- a/src/responder/common/responder_common.c
+++ b/src/responder/common/responder_common.c
@@ -895,6 +895,68 @@ int create_pipe_fd(const char *sock_name, int *_fd, mode_t umaskval)
return ret;
}
+static int
+get_socket_path_in_use(TALLOC_CTX *mem_ctx, int fd, char **socket_path_in_use)
+{
+TALLOC_CTX *tmp_ctx;
+struct stat statbuf;
+char *path_name = NULL;
+char *inode = NULL;
+char *line = NULL;
+char *socket_line = NULL;
+char *inode_pos = NULL;
+errno_t ret;
+FILE *fp;
+size_t len = 0;
+
+tmp_ctx = talloc_new(NULL);
+if (tmp_ctx == NULL) {
+ret = ENOMEM;
+goto done;
+}
+
+path_name = talloc_asprintf_append(path_name, "%s%d", "/proc/self/fd/", fd);
+
+ret = stat(path_name, &statbuf);
+if (ret != EOK) {
+goto done;
+}
+
+fp = fopen("/proc/net/unix", "r");
+if (fp == NULL) {
+ret = ENOENT;
+goto done;
+}
+
+inode = talloc_asprintf_append(inode, "%lu", statbuf.st_ino);
+
+while ((getline(&line, &len, fp) != -1) && (socket_line == NULL)) {
+inode_pos = strstr(line, inode);
+
+if (inode_pos != NULL) {
+/* copy the string except for the inode number at the beginning
+ * and the breaking line at the end */
+socket_line = talloc_strndup(tmp_ctx,
+ inode_pos + strlen(inode) + 1,
+ strlen(inode_pos) - strlen(inode) - 2);
+}
+}
+
+if (socket_line == NULL) {
+ret = ENOENT;
+goto done;
+}
+
+*socket_path_in_use = talloc_steal(mem_ctx, socket_line);
+ret = EOK;
+
+done:
+fclose(fp);
+talloc_free(tmp_ctx);
+
+return ret;
+}
+
/* create a unix socket and listen to it */
static int set_unix_socket(struct resp_ctx *rctx,
connection_setup_t conn_setup)
@@ -1000,7 +1062,15 @@ static int set_unix_socket(struct resp_ctx *rctx,
int activate_unix_sockets(struct resp_ctx *rctx,
connection_setup_t conn_setup)
{
+TALLOC_CTX *tmp_ctx;
int ret;
+char *socket_path_in_use = NULL;
+
+tmp_ctx = talloc_new(NULL);
+if (tmp_ctx == NULL) {
+ret = ENOMEM;
+goto done;
+}
#ifdef HAVE_SYSTEMD
if (rctx->lfd == -1 && rctx->priv_lfd == -1) {
@@ -1024,12 +1094,22 @@ int activate_unix_sockets(struct resp_ctx *rctx,
if (ret == numfds) {
rctx->lfd = SD_LISTEN_FDS_START;
-ret = sd_is_socket_unix(rctx->lfd, SOCK_STREAM, 1, NULL, 0);
+
+ret = get_socket_path_in_use(tmp_ctx, rctx->lfd, &socket_path_in_use);
+if (ret != EOK) {
+DEBUG(SSSDBG_MINOR_FAILURE, "Unable to obtain socket path\n");
+}
+
+ret = sd_is_socket_unix(rctx->lfd, SOCK_STREAM, 1, socket_path_in_use, 0);
if (ret < 0) {
DEBUG(SSSDBG_CRIT_FAILURE,
"Activated socket is not a UNIX listening socket\n");
ret = EIO;
goto done;
+
[SSSD] [sssd PR#5407][comment] kcm: check socket path loaded from configuration
URL: https://github.com/SSSD/sssd/pull/5407 Title: #5407: kcm: check socket path loaded from configuration ikerexxe commented: """ @pbrezina I've updated the PR taking into account your feedback. """ See the full comment at https://github.com/SSSD/sssd/pull/5407#issuecomment-780612591 ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[SSSD] [sssd PR#5407][synchronized] kcm: check socket path loaded from configuration
URL: https://github.com/SSSD/sssd/pull/5407
Author: ikerexxe
Title: #5407: kcm: check socket path loaded from configuration
Action: synchronized
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5407/head:pr5407
git checkout pr5407
From 1ad0445e8fd5e03b35ea53353b7b9a07222b4942 Mon Sep 17 00:00:00 2001
From: ikerexxe
Date: Tue, 26 Jan 2021 12:37:15 +0100
Subject: [PATCH 1/2] RESPONDER: check that configured sockets match
Check if the sockets defined in systemd unit and sssd.conf match. If
they don't, then print a warning message.
Moreover, change man page regarding socket_path option to indicate that
it will be overwritten by systemd's unit file.
Resolves: https://github.com/SSSD/sssd/issues/5406
---
src/man/sssd-kcm.8.xml | 7 +++
src/responder/common/responder_common.c | 11 +++
2 files changed, 18 insertions(+)
diff --git a/src/man/sssd-kcm.8.xml b/src/man/sssd-kcm.8.xml
index 022a74ba09..14ba122a5c 100644
--- a/src/man/sssd-kcm.8.xml
+++ b/src/man/sssd-kcm.8.xml
@@ -203,6 +203,13 @@ systemctl restart sssd-kcm.service
Default: /var/run/.heim_org.h5l.kcm-socket
+
+
+Note: on platforms where systemd is supported, the
+socket path is overwritten by the one defined in
+the sssd-kcm.socket unit file.
+
+
diff --git a/src/responder/common/responder_common.c b/src/responder/common/responder_common.c
index 7061d018a6..992d85c6d2 100644
--- a/src/responder/common/responder_common.c
+++ b/src/responder/common/responder_common.c
@@ -1001,6 +1001,8 @@ int activate_unix_sockets(struct resp_ctx *rctx,
connection_setup_t conn_setup)
{
int ret;
+struct sockaddr_un sockaddr;
+socklen_t sockaddr_len = sizeof(sockaddr);
#ifdef HAVE_SYSTEMD
if (rctx->lfd == -1 && rctx->priv_lfd == -1) {
@@ -1032,6 +1034,15 @@ int activate_unix_sockets(struct resp_ctx *rctx,
goto done;
}
+ret = getsockname(rctx->lfd, (struct sockaddr *) &sockaddr, &sockaddr_len);
+if (ret == EOK) {
+if (memcmp(rctx->sock_name, sockaddr.sun_path, strlen(rctx->sock_name)) != 0) {
+DEBUG(SSSDBG_CONF_SETTINGS,
+ "Warning: socket path defined in systemd unit (%s) and sssd.conf (%s) don't match\n",
+ sockaddr.sun_path, rctx->sock_name);
+}
+}
+
ret = sss_fd_nonblocking(rctx->lfd);
if (ret != EOK) goto done;
if (numfds == 2) {
From 11591f76eacf3e98687fd5d264db3ed063d4bded Mon Sep 17 00:00:00 2001
From: ikerexxe
Date: Tue, 26 Jan 2021 16:01:48 +0100
Subject: [PATCH 2/2] TESTS: test socket path when systemd activation
Test socket path when sssd-kcm is activated by systemd. If socket in
systemd unit and sssd.conf is defined in different locations then print a
warning.
Verifies: https://github.com/SSSD/sssd/issues/5406
---
src/tests/multihost/alltests/test_kcm.py | 33
1 file changed, 33 insertions(+)
diff --git a/src/tests/multihost/alltests/test_kcm.py b/src/tests/multihost/alltests/test_kcm.py
index db08dbd8c4..e7182f5d58 100644
--- a/src/tests/multihost/alltests/test_kcm.py
+++ b/src/tests/multihost/alltests/test_kcm.py
@@ -52,3 +52,36 @@ def test_client_timeout(self, multihost, backupsssdconf):
" /var/log/sssd/"
"sssd_kcm.log")
assert 'Terminated client' in grep_cmd.stdout_text
+
+def test_kcm_check_socket_path(self, multihost, enable_kcm):
+"""
+@Title: kcm: Test socket path when sssd-kcm is activated by systemd
+#https://github.com/SSSD/sssd/issues/5406
+"""
+# Start from a known-good state after removing log file and adding a
+# new socket path
+multihost.master[0].service_sssd('stop')
+self._stop_kcm(multihost)
+self._remove_kcm_log_file(multihost)
+server = sssdTools(multihost.master[0])
+server.backup_sssd_conf()
+socket_path = "/some_path/kcm.socket"
+domain_section = "kcm"
+sssd_params = {'socket_path': '%s' % (socket_path)}
+server.sssd_conf(domain_section, sssd_params)
+multihost.master[0].service_sssd('start')
+self._start_kcm(multihost)
+# Give sssd some time to load
+time.sleep(2)
+
+# Check log file for the expected warning message
+d
[SSSD] [sssd PR#5407][+Waiting for review] kcm: check socket path loaded from configuration
URL: https://github.com/SSSD/sssd/pull/5407 Title: #5407: kcm: check socket path loaded from configuration Label: +Waiting for review ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[SSSD] [sssd PR#5407][-Changes requested] kcm: check socket path loaded from configuration
URL: https://github.com/SSSD/sssd/pull/5407 Title: #5407: kcm: check socket path loaded from configuration Label: -Changes requested ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[SSSD] [sssd PR#5407][comment] kcm: check socket path loaded from configuration
URL: https://github.com/SSSD/sssd/pull/5407 Title: #5407: kcm: check socket path loaded from configuration ikerexxe commented: """ I'd prefer to log the paths so I've picked your second proposal. """ See the full comment at https://github.com/SSSD/sssd/pull/5407#issuecomment-781305848 ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[SSSD] [sssd PR#5407][-Ready to push] kcm: check socket path loaded from configuration
URL: https://github.com/SSSD/sssd/pull/5407 Title: #5407: kcm: check socket path loaded from configuration Label: -Ready to push ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[SSSD] [sssd PR#5407][comment] kcm: check socket path loaded from configuration
URL: https://github.com/SSSD/sssd/pull/5407 Title: #5407: kcm: check socket path loaded from configuration ikerexxe commented: """ I've spoken with @sgoveas and the test infrastructure it's not ready yet to avoid running the test in downstream CI. He'll propose something tomorrow to handle this use case. For the moment I've removed the `Ready to push` label. """ See the full comment at https://github.com/SSSD/sssd/pull/5407#issuecomment-781467833 ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[SSSD] [sssd PR#5506][+Waiting for review] SSSD Log: write_krb5info_file word replacement
URL: https://github.com/SSSD/sssd/pull/5506 Title: #5506: SSSD Log: write_krb5info_file word replacement Label: +Waiting for review ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[SSSD] [sssd PR#5506][-Changes requested] SSSD Log: write_krb5info_file word replacement
URL: https://github.com/SSSD/sssd/pull/5506 Title: #5506: SSSD Log: write_krb5info_file word replacement Label: -Changes requested ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[SSSD] [sssd PR#5407][synchronized] kcm: check socket path loaded from configuration
URL: https://github.com/SSSD/sssd/pull/5407
Author: ikerexxe
Title: #5407: kcm: check socket path loaded from configuration
Action: synchronized
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5407/head:pr5407
git checkout pr5407
From 1ad0445e8fd5e03b35ea53353b7b9a07222b4942 Mon Sep 17 00:00:00 2001
From: ikerexxe
Date: Tue, 26 Jan 2021 12:37:15 +0100
Subject: [PATCH] RESPONDER: check that configured sockets match
Check if the sockets defined in systemd unit and sssd.conf match. If
they don't, then print a warning message.
Moreover, change man page regarding socket_path option to indicate that
it will be overwritten by systemd's unit file.
Resolves: https://github.com/SSSD/sssd/issues/5406
---
src/man/sssd-kcm.8.xml | 7 +++
src/responder/common/responder_common.c | 11 +++
2 files changed, 18 insertions(+)
diff --git a/src/man/sssd-kcm.8.xml b/src/man/sssd-kcm.8.xml
index 022a74ba09..14ba122a5c 100644
--- a/src/man/sssd-kcm.8.xml
+++ b/src/man/sssd-kcm.8.xml
@@ -203,6 +203,13 @@ systemctl restart sssd-kcm.service
Default: /var/run/.heim_org.h5l.kcm-socket
+
+
+Note: on platforms where systemd is supported, the
+socket path is overwritten by the one defined in
+the sssd-kcm.socket unit file.
+
+
diff --git a/src/responder/common/responder_common.c b/src/responder/common/responder_common.c
index 7061d018a6..992d85c6d2 100644
--- a/src/responder/common/responder_common.c
+++ b/src/responder/common/responder_common.c
@@ -1001,6 +1001,8 @@ int activate_unix_sockets(struct resp_ctx *rctx,
connection_setup_t conn_setup)
{
int ret;
+struct sockaddr_un sockaddr;
+socklen_t sockaddr_len = sizeof(sockaddr);
#ifdef HAVE_SYSTEMD
if (rctx->lfd == -1 && rctx->priv_lfd == -1) {
@@ -1032,6 +1034,15 @@ int activate_unix_sockets(struct resp_ctx *rctx,
goto done;
}
+ret = getsockname(rctx->lfd, (struct sockaddr *) &sockaddr, &sockaddr_len);
+if (ret == EOK) {
+if (memcmp(rctx->sock_name, sockaddr.sun_path, strlen(rctx->sock_name)) != 0) {
+DEBUG(SSSDBG_CONF_SETTINGS,
+ "Warning: socket path defined in systemd unit (%s) and sssd.conf (%s) don't match\n",
+ sockaddr.sun_path, rctx->sock_name);
+}
+}
+
ret = sss_fd_nonblocking(rctx->lfd);
if (ret != EOK) goto done;
if (numfds == 2) {
___
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://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives:
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
Do not reply to spam on the list, report it:
https://pagure.io/fedora-infrastructure
[SSSD] [sssd PR#5513][opened] TESTS: test socket path when systemd activation
URL: https://github.com/SSSD/sssd/pull/5513
Author: ikerexxe
Title: #5513: TESTS: test socket path when systemd activation
Action: opened
PR body:
"""
Test socket path when sssd-kcm is activated by systemd. If socket in
systemd unit and sssd.conf is defined in different locations then print a
warning.
Verifies: https://github.com/SSSD/sssd/issues/5406
"""
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5513/head:pr5513
git checkout pr5513
From 2b7e72cbe7446117846f9163310fd08d56010942 Mon Sep 17 00:00:00 2001
From: ikerexxe
Date: Tue, 26 Jan 2021 16:01:48 +0100
Subject: [PATCH] TESTS: test socket path when systemd activation
Test socket path when sssd-kcm is activated by systemd. If socket in
systemd unit and sssd.conf is defined in different locations then print a
warning.
Verifies: https://github.com/SSSD/sssd/issues/5406
---
src/tests/multihost/alltests/test_kcm.py | 33
1 file changed, 33 insertions(+)
diff --git a/src/tests/multihost/alltests/test_kcm.py b/src/tests/multihost/alltests/test_kcm.py
index db08dbd8c4..e7182f5d58 100644
--- a/src/tests/multihost/alltests/test_kcm.py
+++ b/src/tests/multihost/alltests/test_kcm.py
@@ -52,3 +52,36 @@ def test_client_timeout(self, multihost, backupsssdconf):
" /var/log/sssd/"
"sssd_kcm.log")
assert 'Terminated client' in grep_cmd.stdout_text
+
+def test_kcm_check_socket_path(self, multihost, enable_kcm):
+"""
+@Title: kcm: Test socket path when sssd-kcm is activated by systemd
+#https://github.com/SSSD/sssd/issues/5406
+"""
+# Start from a known-good state after removing log file and adding a
+# new socket path
+multihost.master[0].service_sssd('stop')
+self._stop_kcm(multihost)
+self._remove_kcm_log_file(multihost)
+server = sssdTools(multihost.master[0])
+server.backup_sssd_conf()
+socket_path = "/some_path/kcm.socket"
+domain_section = "kcm"
+sssd_params = {'socket_path': '%s' % (socket_path)}
+server.sssd_conf(domain_section, sssd_params)
+multihost.master[0].service_sssd('start')
+self._start_kcm(multihost)
+# Give sssd some time to load
+time.sleep(2)
+
+# Check log file for the expected warning message
+domain_log = '/var/log/sssd/sssd_kcm.log'
+log = multihost.master[0].get_file_contents(domain_log).decode('utf-8')
+msg = "Warning: socket path defined in systemd unit "\
+ "\(/run/.heim_org.h5l.kcm-socket\) and sssd.conf \(%s\) don't "\
+ "match" % (socket_path)
+find = re.compile(r'%s' % msg)
+
+server.restore_sssd_conf()
+
+assert find.search(log)
___
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://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives:
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
Do not reply to spam on the list, report it:
https://pagure.io/fedora-infrastructure
[SSSD] [sssd PR#5513][+Blocked] TESTS: test socket path when systemd activation
URL: https://github.com/SSSD/sssd/pull/5513 Title: #5513: TESTS: test socket path when systemd activation Label: +Blocked ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[SSSD] [sssd PR#5513][+Bugzilla] TESTS: test socket path when systemd activation
URL: https://github.com/SSSD/sssd/pull/5513 Title: #5513: TESTS: test socket path when systemd activation Label: +Bugzilla ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[SSSD] [sssd PR#5407][comment] kcm: check socket path loaded from configuration
URL: https://github.com/SSSD/sssd/pull/5407 Title: #5407: kcm: check socket path loaded from configuration ikerexxe commented: """ Done! Test can be found in https://github.com/SSSD/sssd/pull/5513 """ See the full comment at https://github.com/SSSD/sssd/pull/5407#issuecomment-781987710 ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[SSSD] [sssd PR#5513][-Blocked] TESTS: test socket path when systemd activation
URL: https://github.com/SSSD/sssd/pull/5513 Title: #5513: TESTS: test socket path when systemd activation Label: -Blocked ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[SSSD] [sssd PR#5513][+Changes requested] TESTS: test socket path when systemd activation
URL: https://github.com/SSSD/sssd/pull/5513 Title: #5513: TESTS: test socket path when systemd activation Label: +Changes requested ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[SSSD] [sssd PR#5513][synchronized] TESTS: test socket path when systemd activation
URL: https://github.com/SSSD/sssd/pull/5513
Author: ikerexxe
Title: #5513: TESTS: test socket path when systemd activation
Action: synchronized
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5513/head:pr5513
git checkout pr5513
From e3d2dad0b56ff1cc99d81dee84acfb28dac44c10 Mon Sep 17 00:00:00 2001
From: ikerexxe
Date: Tue, 26 Jan 2021 16:01:48 +0100
Subject: [PATCH] TESTS: test socket path when systemd activation
Test socket path when sssd-kcm is activated by systemd. If socket in
systemd unit and sssd.conf is defined in different locations then print a
warning.
Verifies: https://github.com/SSSD/sssd/issues/5406
---
src/tests/multihost/alltests/pytest.ini | 1 +
src/tests/multihost/alltests/test_kcm.py | 34
2 files changed, 35 insertions(+)
diff --git a/src/tests/multihost/alltests/pytest.ini b/src/tests/multihost/alltests/pytest.ini
index 7f625aa262..66519e8003 100644
--- a/src/tests/multihost/alltests/pytest.ini
+++ b/src/tests/multihost/alltests/pytest.ini
@@ -23,6 +23,7 @@ markers =
fips: Tests related to fips when auth_provider is krb5
ssh: Tests related to ssh responder
ldaplibdebuglevel: Test ldap_library_debug_level option
+no_tier: test cases are not executed on any tier
tier1: tier1 test cases with run time of aproximately 60 minutes
tier1_2: tier1 test cases split to keep runtime upto 60 minutes
tier2: tier2 test cases
diff --git a/src/tests/multihost/alltests/test_kcm.py b/src/tests/multihost/alltests/test_kcm.py
index db08dbd8c4..8c975e5c88 100644
--- a/src/tests/multihost/alltests/test_kcm.py
+++ b/src/tests/multihost/alltests/test_kcm.py
@@ -52,3 +52,37 @@ def test_client_timeout(self, multihost, backupsssdconf):
" /var/log/sssd/"
"sssd_kcm.log")
assert 'Terminated client' in grep_cmd.stdout_text
+
+@pytest.mark.no_tier
+def test_kcm_check_socket_path(self, multihost, backupsssdconf):
+"""
+:Title: kcm: Test socket path when sssd-kcm is activated by systemd
+:id: 6425bf2c-d07e-4d65-b15d-946141422f96
+ticket: #https://github.com/SSSD/sssd/issues/5406
+"""
+# Start from a known-good state after removing log file and adding a
+# new socket path
+domain_log = '/var/log/sssd/sssd_kcm.log'
+multihost.client[0].service_sssd('stop')
+multihost.client[0].run_command("systemctl stop sssd-kcm")
+delete_sssd_domain_log("kcm")
+server = sssdTools(multihost.client[0])
+socket_path = "/some_path/kcm.socket"
+domain_section = "kcm"
+sssd_params = {'socket_path': '%s' % (socket_path)}
+server.sssd_conf(domain_section, sssd_params)
+multihost.client[0].service_sssd('start')
+multihost.client[0].run_command("systemctl start sssd-kcm")
+# Give sssd some time to load
+time.sleep(2)
+
+# Check log file for the expected warning message
+log = multihost.client[0].get_file_contents(domain_log).decode('utf-8')
+msg = "Warning: socket path defined in systemd unit "\
+ "\([a-zA-Z0-9\/._-]) and sssd.conf \(%s\) don't "\
+ "match" % (socket_path)
+find = re.compile(r'%s' % msg)
+
+server.restore_sssd_conf()
+
+assert find.search(log)
___
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://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives:
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
Do not reply to spam on the list, report it:
https://pagure.io/fedora-infrastructure
[SSSD] [sssd PR#5513][synchronized] TESTS: test socket path when systemd activation
URL: https://github.com/SSSD/sssd/pull/5513
Author: ikerexxe
Title: #5513: TESTS: test socket path when systemd activation
Action: synchronized
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5513/head:pr5513
git checkout pr5513
From a882bd3bb1ec68bd66fddcd9f7d3648f0f0b14e0 Mon Sep 17 00:00:00 2001
From: ikerexxe
Date: Tue, 26 Jan 2021 16:01:48 +0100
Subject: [PATCH] TESTS: test socket path when systemd activation
Test socket path when sssd-kcm is activated by systemd. If socket in
systemd unit and sssd.conf is defined in different locations then print a
warning.
Verifies: https://github.com/SSSD/sssd/issues/5406
---
src/tests/multihost/alltests/pytest.ini | 1 +
src/tests/multihost/alltests/test_kcm.py | 34
2 files changed, 35 insertions(+)
diff --git a/src/tests/multihost/alltests/pytest.ini b/src/tests/multihost/alltests/pytest.ini
index 7f625aa262..66519e8003 100644
--- a/src/tests/multihost/alltests/pytest.ini
+++ b/src/tests/multihost/alltests/pytest.ini
@@ -23,6 +23,7 @@ markers =
fips: Tests related to fips when auth_provider is krb5
ssh: Tests related to ssh responder
ldaplibdebuglevel: Test ldap_library_debug_level option
+no_tier: test cases are not executed on any tier
tier1: tier1 test cases with run time of aproximately 60 minutes
tier1_2: tier1 test cases split to keep runtime upto 60 minutes
tier2: tier2 test cases
diff --git a/src/tests/multihost/alltests/test_kcm.py b/src/tests/multihost/alltests/test_kcm.py
index db08dbd8c4..3ca9e2ea0f 100644
--- a/src/tests/multihost/alltests/test_kcm.py
+++ b/src/tests/multihost/alltests/test_kcm.py
@@ -52,3 +52,37 @@ def test_client_timeout(self, multihost, backupsssdconf):
" /var/log/sssd/"
"sssd_kcm.log")
assert 'Terminated client' in grep_cmd.stdout_text
+
+@pytest.mark.no_tier
+def test_kcm_check_socket_path(self, multihost, backupsssdconf):
+"""
+:Title: kcm: Test socket path when sssd-kcm is activated by systemd
+:id: 6425bf2c-d07e-4d65-b15d-946141422f96
+ticket: #https://github.com/SSSD/sssd/issues/5406
+"""
+# Start from a known-good state after removing log file and adding a
+# new socket path
+domain_log = '/var/log/sssd/sssd_kcm.log'
+multihost.client[0].service_sssd('stop')
+multihost.client[0].run_command("systemctl stop sssd-kcm")
+remove_sss_cache('/var/log/sssd')
+server = sssdTools(multihost.client[0])
+socket_path = "/some_path/kcm.socket"
+domain_section = "kcm"
+sssd_params = {'socket_path': '%s' % (socket_path)}
+server.sssd_conf(domain_section, sssd_params)
+multihost.client[0].service_sssd('start')
+multihost.client[0].run_command("systemctl start sssd-kcm")
+# Give sssd some time to load
+time.sleep(2)
+
+# Check log file for the expected warning message
+log = multihost.client[0].get_file_contents(domain_log).decode('utf-8')
+msg = "Warning: socket path defined in systemd unit "\
+ "\([a-zA-Z0-9\/._-]) and sssd.conf \(%s\) don't "\
+ "match" % (socket_path)
+find = re.compile(r'%s' % msg)
+
+server.restore_sssd_conf()
+
+assert find.search(log)
___
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://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives:
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
Do not reply to spam on the list, report it:
https://pagure.io/fedora-infrastructure
[SSSD] [sssd PR#5513][synchronized] TESTS: test socket path when systemd activation
URL: https://github.com/SSSD/sssd/pull/5513
Author: ikerexxe
Title: #5513: TESTS: test socket path when systemd activation
Action: synchronized
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5513/head:pr5513
git checkout pr5513
From 3d21eb56b3d7afab3e3e9ccadd370ebc6cb7580f Mon Sep 17 00:00:00 2001
From: ikerexxe
Date: Tue, 26 Jan 2021 16:01:48 +0100
Subject: [PATCH] TESTS: test socket path when systemd activation
Test socket path when sssd-kcm is activated by systemd. If socket in
systemd unit and sssd.conf is defined in different locations then print a
warning.
Verifies: https://github.com/SSSD/sssd/issues/5406
---
src/tests/multihost/alltests/pytest.ini | 1 +
src/tests/multihost/alltests/test_kcm.py | 34
2 files changed, 35 insertions(+)
diff --git a/src/tests/multihost/alltests/pytest.ini b/src/tests/multihost/alltests/pytest.ini
index 7f625aa262..66519e8003 100644
--- a/src/tests/multihost/alltests/pytest.ini
+++ b/src/tests/multihost/alltests/pytest.ini
@@ -23,6 +23,7 @@ markers =
fips: Tests related to fips when auth_provider is krb5
ssh: Tests related to ssh responder
ldaplibdebuglevel: Test ldap_library_debug_level option
+no_tier: test cases are not executed on any tier
tier1: tier1 test cases with run time of aproximately 60 minutes
tier1_2: tier1 test cases split to keep runtime upto 60 minutes
tier2: tier2 test cases
diff --git a/src/tests/multihost/alltests/test_kcm.py b/src/tests/multihost/alltests/test_kcm.py
index db08dbd8c4..667c16b669 100644
--- a/src/tests/multihost/alltests/test_kcm.py
+++ b/src/tests/multihost/alltests/test_kcm.py
@@ -52,3 +52,37 @@ def test_client_timeout(self, multihost, backupsssdconf):
" /var/log/sssd/"
"sssd_kcm.log")
assert 'Terminated client' in grep_cmd.stdout_text
+
+@pytest.mark.no_tier
+def test_kcm_check_socket_path(self, multihost, backupsssdconf):
+"""
+:Title: kcm: Test socket path when sssd-kcm is activated by systemd
+:id: 6425bf2c-d07e-4d65-b15d-946141422f96
+ticket: #https://github.com/SSSD/sssd/issues/5406
+"""
+# Start from a known-good state after removing log file and adding a
+# new socket path
+domain_log = '/var/log/sssd/sssd_kcm.log'
+multihost.client[0].service_sssd('stop')
+multihost.client[0].run_command("systemctl stop sssd-kcm")
+server = sssdTools(multihost.client[0])
+server.remove_sss_cache('/var/log/sssd')
+socket_path = "/some_path/kcm.socket"
+domain_section = "kcm"
+sssd_params = {'socket_path': '%s' % (socket_path)}
+server.sssd_conf(domain_section, sssd_params)
+multihost.client[0].service_sssd('start')
+multihost.client[0].run_command("systemctl start sssd-kcm")
+# Give sssd some time to load
+time.sleep(2)
+
+# Check log file for the expected warning message
+log = multihost.client[0].get_file_contents(domain_log).decode('utf-8')
+msg = "Warning: socket path defined in systemd unit "\
+ "\([a-zA-Z0-9\/._-]) and sssd.conf \(%s\) don't "\
+ "match" % (socket_path)
+find = re.compile(r'%s' % msg)
+
+server.restore_sssd_conf()
+
+assert find.search(log)
___
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://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives:
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
Do not reply to spam on the list, report it:
https://pagure.io/fedora-infrastructure
[SSSD] [sssd PR#5506][-Changes requested] SSSD Log: write_krb5info_file word replacement
URL: https://github.com/SSSD/sssd/pull/5506 Title: #5506: SSSD Log: write_krb5info_file word replacement Label: -Changes requested ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[SSSD] [sssd PR#5506][+Waiting for review] SSSD Log: write_krb5info_file word replacement
URL: https://github.com/SSSD/sssd/pull/5506 Title: #5506: SSSD Log: write_krb5info_file word replacement Label: +Waiting for review ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[SSSD] [sssd PR#5506][-Changes requested] SSSD Log: write_krb5info_file word replacement
URL: https://github.com/SSSD/sssd/pull/5506 Title: #5506: SSSD Log: write_krb5info_file word replacement Label: -Changes requested ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[SSSD] [sssd PR#5506][+Waiting for review] SSSD Log: write_krb5info_file word replacement
URL: https://github.com/SSSD/sssd/pull/5506 Title: #5506: SSSD Log: write_krb5info_file word replacement Label: +Waiting for review ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[SSSD] [sssd PR#5513][synchronized] TESTS: test socket path when systemd activation
URL: https://github.com/SSSD/sssd/pull/5513
Author: ikerexxe
Title: #5513: TESTS: test socket path when systemd activation
Action: synchronized
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5513/head:pr5513
git checkout pr5513
From 7639f9c6d68eeecf86b3205635837cb80c4acd45 Mon Sep 17 00:00:00 2001
From: ikerexxe
Date: Tue, 26 Jan 2021 16:01:48 +0100
Subject: [PATCH] TESTS: test socket path when systemd activation
Test socket path when sssd-kcm is activated by systemd. If socket in
systemd unit and sssd.conf is defined in different locations then print a
warning.
Verifies: https://github.com/SSSD/sssd/issues/5406
---
src/tests/multihost/alltests/pytest.ini | 1 +
src/tests/multihost/alltests/test_kcm.py | 32
2 files changed, 33 insertions(+)
diff --git a/src/tests/multihost/alltests/pytest.ini b/src/tests/multihost/alltests/pytest.ini
index 7f625aa262..66519e8003 100644
--- a/src/tests/multihost/alltests/pytest.ini
+++ b/src/tests/multihost/alltests/pytest.ini
@@ -23,6 +23,7 @@ markers =
fips: Tests related to fips when auth_provider is krb5
ssh: Tests related to ssh responder
ldaplibdebuglevel: Test ldap_library_debug_level option
+no_tier: test cases are not executed on any tier
tier1: tier1 test cases with run time of aproximately 60 minutes
tier1_2: tier1 test cases split to keep runtime upto 60 minutes
tier2: tier2 test cases
diff --git a/src/tests/multihost/alltests/test_kcm.py b/src/tests/multihost/alltests/test_kcm.py
index db08dbd8c4..4809beefc1 100644
--- a/src/tests/multihost/alltests/test_kcm.py
+++ b/src/tests/multihost/alltests/test_kcm.py
@@ -52,3 +52,35 @@ def test_client_timeout(self, multihost, backupsssdconf):
" /var/log/sssd/"
"sssd_kcm.log")
assert 'Terminated client' in grep_cmd.stdout_text
+
+@pytest.mark.no_tier
+def test_kcm_check_socket_path(self, multihost, backupsssdconf):
+"""
+:title: kcm: Test socket path when sssd-kcm is activated by systemd
+:id: 6425bf2c-d07e-4d65-b15d-946141422f96
+:ticket: #https://github.com/SSSD/sssd/issues/5406
+"""
+# Start from a known-good state after removing log file and adding a
+# new socket path
+domain_log = '/var/log/sssd/sssd_kcm.log'
+multihost.client[0].service_sssd('stop')
+multihost.client[0].run_command("systemctl stop sssd-kcm")
+client = sssdTools(multihost.client[0])
+client.remove_sss_cache('/var/log/sssd')
+socket_path = "/some_path/kcm.socket"
+domain_section = "kcm"
+sssd_params = {'socket_path': '%s' % (socket_path)}
+client.sssd_conf(domain_section, sssd_params)
+multihost.client[0].service_sssd('start')
+multihost.client[0].run_command("systemctl start sssd-kcm")
+# Give sssd some time to load
+time.sleep(2)
+
+# Check log file for the expected warning message
+log = multihost.client[0].get_file_contents(domain_log).decode('utf-8')
+msg = "Warning: socket path defined in systemd unit "\
+ "\([a-zA-Z0-9\/._-]) and sssd.conf \(%s\) don't "\
+ "match" % (socket_path)
+find = re.compile(r'%s' % msg)
+
+assert find.search(log)
___
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://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives:
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
Do not reply to spam on the list, report it:
https://pagure.io/fedora-infrastructure
[SSSD] [sssd PR#5513][-Changes requested] TESTS: test socket path when systemd activation
URL: https://github.com/SSSD/sssd/pull/5513 Title: #5513: TESTS: test socket path when systemd activation Label: -Changes requested ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[SSSD] [sssd PR#5513][+Waiting for review] TESTS: test socket path when systemd activation
URL: https://github.com/SSSD/sssd/pull/5513 Title: #5513: TESTS: test socket path when systemd activation Label: +Waiting for review ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[SSSD] [sssd PR#5513][synchronized] TESTS: test socket path when systemd activation
URL: https://github.com/SSSD/sssd/pull/5513
Author: ikerexxe
Title: #5513: TESTS: test socket path when systemd activation
Action: synchronized
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5513/head:pr5513
git checkout pr5513
From af29c2de94ac7e1af18053979590460a039a Mon Sep 17 00:00:00 2001
From: ikerexxe
Date: Tue, 26 Jan 2021 16:01:48 +0100
Subject: [PATCH] TESTS: test socket path when systemd activation
Test socket path when sssd-kcm is activated by systemd. If socket in
systemd unit and sssd.conf is defined in different locations then print a
warning.
Verifies: https://github.com/SSSD/sssd/issues/5406
---
src/tests/multihost/alltests/pytest.ini | 1 +
src/tests/multihost/alltests/test_kcm.py | 32
2 files changed, 33 insertions(+)
diff --git a/src/tests/multihost/alltests/pytest.ini b/src/tests/multihost/alltests/pytest.ini
index 7f625aa262..66519e8003 100644
--- a/src/tests/multihost/alltests/pytest.ini
+++ b/src/tests/multihost/alltests/pytest.ini
@@ -23,6 +23,7 @@ markers =
fips: Tests related to fips when auth_provider is krb5
ssh: Tests related to ssh responder
ldaplibdebuglevel: Test ldap_library_debug_level option
+no_tier: test cases are not executed on any tier
tier1: tier1 test cases with run time of aproximately 60 minutes
tier1_2: tier1 test cases split to keep runtime upto 60 minutes
tier2: tier2 test cases
diff --git a/src/tests/multihost/alltests/test_kcm.py b/src/tests/multihost/alltests/test_kcm.py
index db08dbd8c4..58ac1fa875 100644
--- a/src/tests/multihost/alltests/test_kcm.py
+++ b/src/tests/multihost/alltests/test_kcm.py
@@ -52,3 +52,35 @@ def test_client_timeout(self, multihost, backupsssdconf):
" /var/log/sssd/"
"sssd_kcm.log")
assert 'Terminated client' in grep_cmd.stdout_text
+
+@pytest.mark.no_tier
+def test_kcm_check_socket_path(self, multihost, backupsssdconf):
+"""
+:title: kcm: Test socket path when sssd-kcm is activated by systemd
+:id: 6425bf2c-d07e-4d65-b15d-946141422f96
+:ticket: https://github.com/SSSD/sssd/issues/5406
+"""
+# Start from a known-good state after removing log file and adding a
+# new socket path
+domain_log = '/var/log/sssd/sssd_kcm.log'
+multihost.client[0].service_sssd('stop')
+multihost.client[0].run_command("systemctl stop sssd-kcm")
+client = sssdTools(multihost.client[0])
+client.remove_sss_cache(domain_log)
+socket_path = "/some_path/kcm.socket"
+domain_section = "kcm"
+sssd_params = {'socket_path': '%s' % (socket_path)}
+client.sssd_conf(domain_section, sssd_params)
+multihost.client[0].service_sssd('start')
+multihost.client[0].run_command("systemctl start sssd-kcm")
+# Give sssd some time to load
+time.sleep(2)
+
+# Check log file for the expected warning message
+log = multihost.client[0].get_file_contents(domain_log).decode('utf-8')
+msg = "Warning: socket path defined in systemd unit "\
+ "\([a-zA-Z0-9\/._-]) and sssd.conf \(%s\) don't "\
+ "match" % (socket_path)
+find = re.compile(r'%s' % msg)
+
+assert find.search(log)
___
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://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives:
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
Do not reply to spam on the list, report it:
https://pagure.io/fedora-infrastructure
[SSSD] [sssd PR#5513][synchronized] TESTS: test socket path when systemd activation
URL: https://github.com/SSSD/sssd/pull/5513
Author: ikerexxe
Title: #5513: TESTS: test socket path when systemd activation
Action: synchronized
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5513/head:pr5513
git checkout pr5513
From c6860f9c43b17177736753ede4d2da97d6aa9c38 Mon Sep 17 00:00:00 2001
From: ikerexxe
Date: Tue, 26 Jan 2021 16:01:48 +0100
Subject: [PATCH] TESTS: test socket path when systemd activation
Test socket path when sssd-kcm is activated by systemd. If socket in
systemd unit and sssd.conf is defined in different locations then print a
warning.
Verifies: https://github.com/SSSD/sssd/issues/5406
---
src/tests/multihost/alltests/pytest.ini | 1 +
src/tests/multihost/alltests/test_kcm.py | 33
2 files changed, 34 insertions(+)
diff --git a/src/tests/multihost/alltests/pytest.ini b/src/tests/multihost/alltests/pytest.ini
index 7f625aa262..66519e8003 100644
--- a/src/tests/multihost/alltests/pytest.ini
+++ b/src/tests/multihost/alltests/pytest.ini
@@ -23,6 +23,7 @@ markers =
fips: Tests related to fips when auth_provider is krb5
ssh: Tests related to ssh responder
ldaplibdebuglevel: Test ldap_library_debug_level option
+no_tier: test cases are not executed on any tier
tier1: tier1 test cases with run time of aproximately 60 minutes
tier1_2: tier1 test cases split to keep runtime upto 60 minutes
tier2: tier2 test cases
diff --git a/src/tests/multihost/alltests/test_kcm.py b/src/tests/multihost/alltests/test_kcm.py
index a2bcce1e5e..6a3e10e907 100644
--- a/src/tests/multihost/alltests/test_kcm.py
+++ b/src/tests/multihost/alltests/test_kcm.py
@@ -88,3 +88,36 @@ def test_refresh_contain_timestamp(self,
f"ldap_search_ext with' "
f"{log_location}")
assert 'modifyTimestamp>=' not in grep_cmd.stdout_text
+
+@pytest.mark.no_tier
+def test_kcm_check_socket_path(self, multihost, backupsssdconf):
+"""
+:title: kcm: Test socket path when sssd-kcm is activated by systemd
+:id: 6425bf2c-d07e-4d65-b15d-946141422f96
+:ticket: https://github.com/SSSD/sssd/issues/5406
+"""
+# Start from a known-good state after removing log file and adding a
+# new socket path
+domain_log = '/var/log/sssd/sssd_kcm.log'
+multihost.client[0].service_sssd('stop')
+multihost.client[0].run_command("systemctl stop sssd-kcm")
+client = sssdTools(multihost.client[0])
+client.remove_sss_cache(domain_log)
+socket_path = "/some_path/kcm.socket"
+domain_section = "kcm"
+sssd_params = {'socket_path': '%s' % (socket_path)}
+client.sssd_conf(domain_section, sssd_params)
+multihost.client[0].service_sssd('start')
+multihost.client[0].run_command("systemctl start sssd-kcm")
+# Give sssd some time to load
+time.sleep(2)
+
+# Check log file for the expected warning message
+log = multihost.client[0].get_file_contents(domain_log).decode('utf-8')
+msg = "Warning: socket path defined in systemd unit "\
+ "\([a-zA-Z0-9\/._-]) and sssd.conf \(%s\) don't "\
+ "match" % (socket_path)
+find = re.compile(r'%s' % msg)
+
+assert find.search(log)
+
___
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://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives:
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
Do not reply to spam on the list, report it:
https://pagure.io/fedora-infrastructure
[SSSD] [sssd PR#5513][comment] TESTS: test socket path when systemd activation
URL: https://github.com/SSSD/sssd/pull/5513 Title: #5513: TESTS: test socket path when systemd activation ikerexxe commented: """ Rebased to fix merge conflict. """ See the full comment at https://github.com/SSSD/sssd/pull/5513#issuecomment-789664521 ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[SSSD] [sssd PR#5532][+Waiting for review] Handle ldap_install_tls() configuration and retrial
URL: https://github.com/SSSD/sssd/pull/5532 Title: #5532: Handle ldap_install_tls() configuration and retrial Label: +Waiting for review ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[SSSD] [sssd PR#5532][+Bugzilla] Handle ldap_install_tls() configuration and retrial
URL: https://github.com/SSSD/sssd/pull/5532 Title: #5532: Handle ldap_install_tls() configuration and retrial Label: +Bugzilla ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[SSSD] [sssd PR#5532][opened] Handle ldap_install_tls() configuration and retrial
URL: https://github.com/SSSD/sssd/pull/5532
Author: ikerexxe
Title: #5532: Handle ldap_install_tls() configuration and retrial
Action: opened
PR body:
"""
Configure socket options when calling ldap_install_tls() to avoid hitting
EINTR during connect. Set the communication to asynchronous. This
configuration can't be applied for the connection part, which has to be
always blocking. On top of that set the network timeout to
ldap_opt_timeout option, to decrease the possibility of triggering a
timeout error when polling.
If the call to ldap_install_tls() fails with EINTR, retry it again.
"""
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5532/head:pr5532
git checkout pr5532
From 32dacb14b14ddd8189512e3f12088970eeabf744 Mon Sep 17 00:00:00 2001
From: Iker Pedrosa
Date: Mon, 22 Feb 2021 11:29:53 +0100
Subject: [PATCH 1/2] util: set socket options for ldap_install_tls()
Configure socket options when calling ldap_install_tls() to avoid hitting
EINTR during connect. Set the communication to asynchronous. This
configuration can't be applied for the connection part, which has to be
always blocking. On top of that set the network timeout to
ldap_opt_timeout option, to decrease the possibility of triggering a
timeout error when polling.
Resolves: https://github.com/SSSD/sssd/issues/5531
---
src/util/sss_ldap.c | 23 +++
1 file changed, 23 insertions(+)
diff --git a/src/util/sss_ldap.c b/src/util/sss_ldap.c
index 976d6208ee..6103a9aea9 100644
--- a/src/util/sss_ldap.c
+++ b/src/util/sss_ldap.c
@@ -117,6 +117,7 @@ struct sss_ldap_init_state {
int sd;
const char *uri;
bool use_udp;
+int timeout;
};
static int sss_ldap_init_state_destructor(void *data)
@@ -173,6 +174,8 @@ struct tevent_req *sss_ldap_init_send(TALLOC_CTX *mem_ctx,
goto fail;
}
+state->timeout = timeout;
+
tevent_req_set_callback(subreq, sss_ldap_init_sys_connect_done, req);
return req;
@@ -279,7 +282,27 @@ static void sss_ldap_init_sys_connect_done(struct tevent_req *subreq)
}
if (ldap_is_ldaps_url(state->uri)) {
+/*
+ * Set communication options to reduce the possibility of hitting an
+ * otherwise avoidable problem when calling ldap_install_tls().
+ */
+struct timeval tv;
+tv.tv_sec = state->timeout;
+ldap_set_option(state->ldap, LDAP_OPT_NETWORK_TIMEOUT, &tv);
+if (ret != LDAP_OPT_SUCCESS) {
+DEBUG(SSSDBG_CRIT_FAILURE, "Failed to set network timeout to %d\n",
+ state->timeout);
+goto fail;
+}
+ldap_set_option(state->ldap, LDAP_OPT_CONNECT_ASYNC, LDAP_OPT_ON);
+if (ret != LDAP_OPT_SUCCESS) {
+DEBUG(SSSDBG_CRIT_FAILURE,
+ "Failed to set asynchronous connection option.\n");
+goto fail;
+}
+
lret = ldap_install_tls(state->ldap);
+
if (lret != LDAP_SUCCESS) {
if (lret == LDAP_LOCAL_ERROR) {
DEBUG(SSSDBG_FUNC_DATA, "TLS/SSL already in place.\n");
From 27e2332818ecf269ef732375cf264b7ddc71de68 Mon Sep 17 00:00:00 2001
From: Iker Pedrosa
Date: Wed, 3 Mar 2021 15:34:49 +0100
Subject: [PATCH 2/2] ldap: retry ldap_install_tls() when EINTR
When the call to ldap_install_tls() fails with EINTR, retry it again.
Resolves: https://github.com/SSSD/sssd/issues/5531
---
src/providers/backend.h| 6 +-
src/providers/data_provider_fo.c | 19 -
src/providers/krb5/krb5_auth.c | 15 +-
src/providers/ldap/sdap_async_connection.c | 24 ++
src/util/sss_ldap.c| 8
5 files changed, 57 insertions(+), 15 deletions(-)
diff --git a/src/providers/backend.h b/src/providers/backend.h
index f8e8af9aa0..c5280c8ffb 100644
--- a/src/providers/backend.h
+++ b/src/providers/backend.h
@@ -60,6 +60,9 @@ struct be_svc_data {
struct be_svc_callback *callbacks;
struct fo_server *first_resolved;
+
+struct fo_server *current_resolved;
+int current_resolved_attempts;
};
struct be_failover_ctx {
@@ -189,7 +192,8 @@ struct tevent_req *be_resolve_server_send(TALLOC_CTX *memctx,
struct tevent_context *ev,
struct be_ctx *ctx,
const char *service_name,
- bool first_try);
+ bool first_try,
+ bool retry_same_server);
int be_resolve_server_recv(struct tevent_req *req,
TALLOC_CTX *ref_ctx,
struct fo_server **srv);
diff --git a/src/providers/data_provider_fo.c b/src/providers/data_pro
[SSSD] [sssd PR#5532][comment] Handle ldap_install_tls() configuration and retrial
URL: https://github.com/SSSD/sssd/pull/5532 Title: #5532: Handle ldap_install_tls() configuration and retrial ikerexxe commented: """ > **Wrt 1st patch** > I don't understand it's description, specifically `Configure socket options > when calling ldap_install_tls() to avoid hitting EINTR during connect.` part. > IIUC as a result of this patch blocking `read()` is replaced with timed > `poll()` under the hood of openlap:openssl (`LDAP_OPT_CONNECT_ASYNC`). > As for `LDAP_OPT_NETWORK_TIMEOUT` - it is set in > [`sdap_sys_connect_done()`](https://github.com/SSSD/sssd/blob/master/src/providers/ldap/sdap_async_connection.c#L199). > Does this happen too late? Do we need to set it in both places? > Also take a note of `LDAP_OPT_RESTART` set a little bit above and a > corresponding comment. This comment looks interesting. > > In general, this clearly doesn't "avoid" the issue (both read() with our > socket options and poll() return EINTR being interrupted with a signal) and, > taking into account our default timeout 6 seconds, I think it won't even make > probability lower. > > So... does it make sense to make openldap switch to timed poll() instead of > blocking read()? Probably "yes" as a general improvement for a 2-x branch, > but at least I wouldn't propose this change for 1-16 branch without a better > reason. And anyway this requires a better explanation. I've run a battery of tests by setting only one of the communication flags per test. I've uploaded the most prominent strace logs to the bugzilla in the attachment called `strace battery of tests`. Setting `LDAP_OPT_CONNECT_ASYNC` uses the read() system call to initiate the SSL connection and it fails with EINTR. In my opinion, it doesn't make any sense to use this flag. Setting `LDAP_OPT_NETWORK_TIMEOUT` also uses the read() system call to initiate the SSL connection, but in this case it fails with EAGAIN. I'm not sure if openldap retries the process because I don't see sssd printing `ldap_install_tls failed` but I don't see any retrial either. I'm hesitant about setting this flag. Setting `LDAP_OPT_RESTART` uses the read() system call to initiate the SSL connection and it fails with EINTR. As pointed out in the bugzilla, checking the man page for SSL_get_error API (https://www.openssl.org/docs/man1.1.1/man3/SSL_get_error.html), it seems that the connection should be dropped when it returns `SSL_ERROR_SYSCALL`. Taking this into account I think that `LDAP_OPT_RESTART` shouldn't be used. > > **Wrt 2nd patch** > I think we shouldn't rely on undocumented usage of `errno`, i.e. it's better > to check watchdog's context. Do you mean watchdog_ctx structure? Would it be enough to compare the timestamp in that structure with the actual time and see if it's below a threshold? """ See the full comment at https://github.com/SSSD/sssd/pull/5532#issuecomment-800125286 ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[SSSD] [sssd PR#5532][comment] Handle ldap_install_tls() configuration and retrial
URL: https://github.com/SSSD/sssd/pull/5532 Title: #5532: Handle ldap_install_tls() configuration and retrial ikerexxe commented: """ > > > **Wrt 1st patch** > > Why did we see poll() previously? We saw and still see poll(), but not for the authentication handshake sequence. Without any of the options set poll() is used for some part of the communication. This part of the communication is the same one that uses poll() when any of the options is set. > Probably because it requires both ASYNC && (TIMEOUT != 0). No, I set both options and read() is still used for the authentication handshake. As said before I think we were looking to the wrong part of the logs and that's why we saw poll(). > Anyway, at this point I'm totally lost wrt rationale of the 1st patch. I'm having doubts with the usage of `LDAP_OPT_NETWORK_TIMEOUT` option, as in this case read() fails with EAGAIN instead of EINTR and ldap_install_tls() doesn't fail. Maybe openldap handles the retry in this case but I need to take a closer look. I'll remove the usage of `LDAP_OPT_CONNECT_ASYNC` from this patch because I don't see any benefit. > > > > **Wrt 2nd patch** > > > I think we shouldn't rely on undocumented usage of `errno`, i.e. it's > > > better to check watchdog's context. > > > > > > Do you mean watchdog_ctx structure? > > Yes. Take a note: it's private. Please avoid making it public entirely, > better introduce a helper to read required info. > > > Would it be enough to compare the timestamp in that structure with the > > actual time and see if it's below a threshold? > > Not sure I understand your idea. IMO, it's better to rely on `ticks`. I've understood your idea. """ See the full comment at https://github.com/SSSD/sssd/pull/5532#issuecomment-800937194 ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[SSSD] [sssd PR#5532][comment] Handle ldap_install_tls() configuration and retrial
URL: https://github.com/SSSD/sssd/pull/5532
Title: #5532: Handle ldap_install_tls() configuration and retrial
ikerexxe commented:
"""
> > > > > **Wrt 1st patch**
>
> ```
> 04:45:50.443947 fcntl(23, F_GETFL) = 0x2 (flags O_RDWR) <0.09>
> 04:45:50.443984 fcntl(23, F_SETFL, O_RDWR|O_NONBLOCK) = 0 <0.09>
> 04:45:50.444071 write(23, "\26\...", 289) = 289 <0.32>
> 04:45:50.444142 read(23, 0x55c5711ef360, 7) = -1 EAGAIN (Resource temporarily
> unavailable) <0.11>
> 04:45:50.444190 poll([{fd=23, events=POLLIN|POLLPRI|POLLERR|POLLHUP}], 1,
> 12000) = ? ERESTART_RESTARTBLOCK (Interrupted by signal) <1.771686>
> ```
>
> If this is not a part of a handshake then please tell me what is it?
It's part of the handshake, but I haven't seen this behaviour again even though
I've tried it another 10 times.
> But you wrote:
>
> > I'm having doubts with the usage of LDAP_OPT_NETWORK_TIMEOUT option, as in
> > this case read() fails with EAGAIN instead of EINTR and ldap_install_tls()
> > doesn't fail. Maybe openldap handles the retry in this case but I need to
> > take a closer look.
>
> This is interesting. Could you please quote corresponding strace or at least
> point to a specific log that exhibits this behavior?
https://bugzilla.redhat.com/attachment.cgi?id=1763588, file
strace_network_timeout.out.
>
> As I pointed out previously, SSSD already sets `LDAP_OPT_NETWORK_TIMEOUT`.
> Setting the same option in different places needs an explanation.
There isn't any log but it still fails and there isn't any retry procedure. So
I'll remove the first patch completely.
"""
See the full comment at
https://github.com/SSSD/sssd/pull/5532#issuecomment-800986678
___
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://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives:
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
Do not reply to spam on the list, report it:
https://pagure.io/fedora-infrastructure
[SSSD] [sssd PR#5532][synchronized] Handle ldap_install_tls() configuration and retrial
URL: https://github.com/SSSD/sssd/pull/5532
Author: ikerexxe
Title: #5532: Handle ldap_install_tls() configuration and retrial
Action: synchronized
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5532/head:pr5532
git checkout pr5532
From 5065db722c8b14d4e475469b996797018b7a94bb Mon Sep 17 00:00:00 2001
From: Iker Pedrosa
Date: Wed, 3 Mar 2021 15:34:49 +0100
Subject: [PATCH] ldap: retry ldap_install_tls() when watchdog interruption
When the call to ldap_install_tls() fails because the watchdog
interrupted it, retry it. The watchdog interruption is detected by
checking the value of the ticks before and after the call to
ldap_install_tls().
Resolves: https://github.com/SSSD/sssd/issues/5531
---
src/providers/backend.h| 6 +-
src/providers/data_provider_fo.c | 19 -
src/providers/krb5/krb5_auth.c | 15 +-
src/providers/ldap/sdap_async_connection.c | 24 ++
src/util/sss_ldap.c| 13
src/util/util.h| 1 +
src/util/util_errors.c | 2 ++
src/util/util_errors.h | 2 ++
src/util/util_watchdog.c | 5 +
9 files changed, 72 insertions(+), 15 deletions(-)
diff --git a/src/providers/backend.h b/src/providers/backend.h
index f8e8af9aa0..c5280c8ffb 100644
--- a/src/providers/backend.h
+++ b/src/providers/backend.h
@@ -60,6 +60,9 @@ struct be_svc_data {
struct be_svc_callback *callbacks;
struct fo_server *first_resolved;
+
+struct fo_server *current_resolved;
+int current_resolved_attempts;
};
struct be_failover_ctx {
@@ -189,7 +192,8 @@ struct tevent_req *be_resolve_server_send(TALLOC_CTX *memctx,
struct tevent_context *ev,
struct be_ctx *ctx,
const char *service_name,
- bool first_try);
+ bool first_try,
+ bool retry_same_server);
int be_resolve_server_recv(struct tevent_req *req,
TALLOC_CTX *ref_ctx,
struct fo_server **srv);
diff --git a/src/providers/data_provider_fo.c b/src/providers/data_provider_fo.c
index 0dfbb04b0f..7cf15fe496 100644
--- a/src/providers/data_provider_fo.c
+++ b/src/providers/data_provider_fo.c
@@ -24,6 +24,8 @@
#include "providers/backend.h"
#include "resolv/async_resolv.h"
+#define MAX_CURRENT_RESOLVED_ATTEMPTS 1
+
struct be_svc_callback {
struct be_svc_callback *prev;
struct be_svc_callback *next;
@@ -506,7 +508,8 @@ struct tevent_req *be_resolve_server_send(TALLOC_CTX *memctx,
struct tevent_context *ev,
struct be_ctx *ctx,
const char *service_name,
- bool first_try)
+ bool first_try,
+ bool retry_same_server)
{
struct tevent_req *req, *subreq;
struct be_resolve_server_state *state;
@@ -529,6 +532,17 @@ struct tevent_req *be_resolve_server_send(TALLOC_CTX *memctx,
state->attempts = 0;
state->first_try = first_try;
+if (retry_same_server == true &&
+state->svc->current_resolved_attempts <= MAX_CURRENT_RESOLVED_ATTEMPTS) {
+state->srv = state->svc->current_resolved;
+state->svc->current_resolved_attempts++;
+tevent_req_done(req);
+tevent_req_post(req, ev);
+return req;
+}
+
+state->svc->current_resolved_attempts = 0;
+
subreq = fo_resolve_service_send(state, ev,
ctx->be_fo->be_res->resolv,
ctx->be_fo->fo_ctx,
@@ -645,6 +659,9 @@ errno_t be_resolve_server_process(struct tevent_req *subreq,
return ENOENT;
}
+state->svc->current_resolved = state->srv;
+state->svc->current_resolved_attempts++;
+
if (DEBUG_IS_SET(SSSDBG_FUNC_DATA) && fo_get_server_name(state->srv)) {
struct resolv_hostent *srvaddr;
char ipaddr[128];
diff --git a/src/providers/krb5/krb5_auth.c b/src/providers/krb5/krb5_auth.c
index 699c2467b8..95336f68d0 100644
--- a/src/providers/krb5/krb5_auth.c
+++ b/src/providers/krb5/krb5_auth.c
@@ -688,7 +688,8 @@ struct tevent_req *krb5_auth_send(TALLOC_CTX *mem_ctx,
state->search_kpasswd = false;
subreq = be_resolve_server_send(state, state->ev, state->be_ctx,
state->krb5_ctx->service->name,
-
[SSSD] [sssd PR#5532][edited] ldap: retry ldap_install_tls() when watchdog interruption
URL: https://github.com/SSSD/sssd/pull/5532 Author: ikerexxe Title: #5532: ldap: retry ldap_install_tls() when watchdog interruption Action: edited Changed field: title Original value: """ Handle ldap_install_tls() configuration and retrial """ ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[SSSD] [sssd PR#5532][comment] ldap: retry ldap_install_tls() when watchdog interruption
URL: https://github.com/SSSD/sssd/pull/5532 Title: #5532: ldap: retry ldap_install_tls() when watchdog interruption ikerexxe commented: """ I've updated the PR by dropping the first patch and changing the second one to detect if ldap_install_tls() failed because the watchdog interrupted it. """ See the full comment at https://github.com/SSSD/sssd/pull/5532#issuecomment-801041154 ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[SSSD] [sssd PR#5532][synchronized] ldap: retry ldap_install_tls() when watchdog interruption
URL: https://github.com/SSSD/sssd/pull/5532
Author: ikerexxe
Title: #5532: ldap: retry ldap_install_tls() when watchdog interruption
Action: synchronized
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5532/head:pr5532
git checkout pr5532
From 9673ff10cda335c1c3f2ed63ea8295d370813d55 Mon Sep 17 00:00:00 2001
From: Iker Pedrosa
Date: Wed, 3 Mar 2021 15:34:49 +0100
Subject: [PATCH] ldap: retry ldap_install_tls() when watchdog interruption
When the call to ldap_install_tls() fails because the watchdog
interrupted it, retry it. The watchdog interruption is detected by
checking the value of the ticks before and after the call to
ldap_install_tls().
Resolves: https://github.com/SSSD/sssd/issues/5531
---
src/providers/backend.h| 6 +-
src/providers/data_provider_fo.c | 19 -
src/providers/krb5/krb5_auth.c | 15 +-
src/providers/ldap/sdap_async_connection.c | 24 ++
src/util/sss_ldap.c| 13
src/util/util.h| 1 +
src/util/util_errors.c | 2 ++
src/util/util_errors.h | 2 ++
src/util/util_watchdog.c | 5 +
9 files changed, 72 insertions(+), 15 deletions(-)
diff --git a/src/providers/backend.h b/src/providers/backend.h
index f8e8af9aa0..c5280c8ffb 100644
--- a/src/providers/backend.h
+++ b/src/providers/backend.h
@@ -60,6 +60,9 @@ struct be_svc_data {
struct be_svc_callback *callbacks;
struct fo_server *first_resolved;
+
+struct fo_server *current_resolved;
+int current_resolved_attempts;
};
struct be_failover_ctx {
@@ -189,7 +192,8 @@ struct tevent_req *be_resolve_server_send(TALLOC_CTX *memctx,
struct tevent_context *ev,
struct be_ctx *ctx,
const char *service_name,
- bool first_try);
+ bool first_try,
+ bool retry_same_server);
int be_resolve_server_recv(struct tevent_req *req,
TALLOC_CTX *ref_ctx,
struct fo_server **srv);
diff --git a/src/providers/data_provider_fo.c b/src/providers/data_provider_fo.c
index 0dfbb04b0f..7cf15fe496 100644
--- a/src/providers/data_provider_fo.c
+++ b/src/providers/data_provider_fo.c
@@ -24,6 +24,8 @@
#include "providers/backend.h"
#include "resolv/async_resolv.h"
+#define MAX_CURRENT_RESOLVED_ATTEMPTS 1
+
struct be_svc_callback {
struct be_svc_callback *prev;
struct be_svc_callback *next;
@@ -506,7 +508,8 @@ struct tevent_req *be_resolve_server_send(TALLOC_CTX *memctx,
struct tevent_context *ev,
struct be_ctx *ctx,
const char *service_name,
- bool first_try)
+ bool first_try,
+ bool retry_same_server)
{
struct tevent_req *req, *subreq;
struct be_resolve_server_state *state;
@@ -529,6 +532,17 @@ struct tevent_req *be_resolve_server_send(TALLOC_CTX *memctx,
state->attempts = 0;
state->first_try = first_try;
+if (retry_same_server == true &&
+state->svc->current_resolved_attempts <= MAX_CURRENT_RESOLVED_ATTEMPTS) {
+state->srv = state->svc->current_resolved;
+state->svc->current_resolved_attempts++;
+tevent_req_done(req);
+tevent_req_post(req, ev);
+return req;
+}
+
+state->svc->current_resolved_attempts = 0;
+
subreq = fo_resolve_service_send(state, ev,
ctx->be_fo->be_res->resolv,
ctx->be_fo->fo_ctx,
@@ -645,6 +659,9 @@ errno_t be_resolve_server_process(struct tevent_req *subreq,
return ENOENT;
}
+state->svc->current_resolved = state->srv;
+state->svc->current_resolved_attempts++;
+
if (DEBUG_IS_SET(SSSDBG_FUNC_DATA) && fo_get_server_name(state->srv)) {
struct resolv_hostent *srvaddr;
char ipaddr[128];
diff --git a/src/providers/krb5/krb5_auth.c b/src/providers/krb5/krb5_auth.c
index 699c2467b8..95336f68d0 100644
--- a/src/providers/krb5/krb5_auth.c
+++ b/src/providers/krb5/krb5_auth.c
@@ -688,7 +688,8 @@ struct tevent_req *krb5_auth_send(TALLOC_CTX *mem_ctx,
state->search_kpasswd = false;
subreq = be_resolve_server_send(state, state->ev, state->be_ctx,
state->krb5_ctx->service->name,
-
[SSSD] [sssd PR#5532][synchronized] ldap: retry ldap_install_tls() when watchdog interruption
URL: https://github.com/SSSD/sssd/pull/5532
Author: ikerexxe
Title: #5532: ldap: retry ldap_install_tls() when watchdog interruption
Action: synchronized
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5532/head:pr5532
git checkout pr5532
From 7b8a0ae99044dc885f22f68a7f4b528cc06f6896 Mon Sep 17 00:00:00 2001
From: Iker Pedrosa
Date: Wed, 3 Mar 2021 15:34:49 +0100
Subject: [PATCH] ldap: retry ldap_install_tls() when watchdog interruption
When the call to ldap_install_tls() fails because the watchdog
interrupted it, retry it. The watchdog interruption is detected by
checking the value of the ticks before and after the call to
ldap_install_tls().
Resolves: https://github.com/SSSD/sssd/issues/5531
---
src/providers/backend.h| 6 -
src/providers/data_provider_fo.c | 19 +++-
src/providers/krb5/krb5_auth.c | 15 -
src/providers/ldap/sdap_async_connection.c | 26 +++---
src/util/sss_ldap.c| 15 +
src/util/util.h| 1 +
src/util/util_errors.c | 2 ++
src/util/util_errors.h | 2 ++
src/util/util_watchdog.c | 5 +
9 files changed, 76 insertions(+), 15 deletions(-)
diff --git a/src/providers/backend.h b/src/providers/backend.h
index f8e8af9aa0..c5280c8ffb 100644
--- a/src/providers/backend.h
+++ b/src/providers/backend.h
@@ -60,6 +60,9 @@ struct be_svc_data {
struct be_svc_callback *callbacks;
struct fo_server *first_resolved;
+
+struct fo_server *current_resolved;
+int current_resolved_attempts;
};
struct be_failover_ctx {
@@ -189,7 +192,8 @@ struct tevent_req *be_resolve_server_send(TALLOC_CTX *memctx,
struct tevent_context *ev,
struct be_ctx *ctx,
const char *service_name,
- bool first_try);
+ bool first_try,
+ bool retry_same_server);
int be_resolve_server_recv(struct tevent_req *req,
TALLOC_CTX *ref_ctx,
struct fo_server **srv);
diff --git a/src/providers/data_provider_fo.c b/src/providers/data_provider_fo.c
index 0dfbb04b0f..7cf15fe496 100644
--- a/src/providers/data_provider_fo.c
+++ b/src/providers/data_provider_fo.c
@@ -24,6 +24,8 @@
#include "providers/backend.h"
#include "resolv/async_resolv.h"
+#define MAX_CURRENT_RESOLVED_ATTEMPTS 1
+
struct be_svc_callback {
struct be_svc_callback *prev;
struct be_svc_callback *next;
@@ -506,7 +508,8 @@ struct tevent_req *be_resolve_server_send(TALLOC_CTX *memctx,
struct tevent_context *ev,
struct be_ctx *ctx,
const char *service_name,
- bool first_try)
+ bool first_try,
+ bool retry_same_server)
{
struct tevent_req *req, *subreq;
struct be_resolve_server_state *state;
@@ -529,6 +532,17 @@ struct tevent_req *be_resolve_server_send(TALLOC_CTX *memctx,
state->attempts = 0;
state->first_try = first_try;
+if (retry_same_server == true &&
+state->svc->current_resolved_attempts <= MAX_CURRENT_RESOLVED_ATTEMPTS) {
+state->srv = state->svc->current_resolved;
+state->svc->current_resolved_attempts++;
+tevent_req_done(req);
+tevent_req_post(req, ev);
+return req;
+}
+
+state->svc->current_resolved_attempts = 0;
+
subreq = fo_resolve_service_send(state, ev,
ctx->be_fo->be_res->resolv,
ctx->be_fo->fo_ctx,
@@ -645,6 +659,9 @@ errno_t be_resolve_server_process(struct tevent_req *subreq,
return ENOENT;
}
+state->svc->current_resolved = state->srv;
+state->svc->current_resolved_attempts++;
+
if (DEBUG_IS_SET(SSSDBG_FUNC_DATA) && fo_get_server_name(state->srv)) {
struct resolv_hostent *srvaddr;
char ipaddr[128];
diff --git a/src/providers/krb5/krb5_auth.c b/src/providers/krb5/krb5_auth.c
index 699c2467b8..95336f68d0 100644
--- a/src/providers/krb5/krb5_auth.c
+++ b/src/providers/krb5/krb5_auth.c
@@ -688,7 +688,8 @@ struct tevent_req *krb5_auth_send(TALLOC_CTX *mem_ctx,
state->search_kpasswd = false;
subreq = be_resolve_server_send(state, state->ev, state->be_ctx,
state->krb5_ctx->service->name,
-
[SSSD] [sssd PR#5532][comment] ldap: retry ldap_install_tls() when watchdog interruption
URL: https://github.com/SSSD/sssd/pull/5532 Title: #5532: ldap: retry ldap_install_tls() when watchdog interruption ikerexxe commented: """ Updated with Alexey's improvements. """ See the full comment at https://github.com/SSSD/sssd/pull/5532#issuecomment-801112078 ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[SSSD] [sssd PR#5532][comment] ldap: retry ldap_install_tls() when watchdog interruption
URL: https://github.com/SSSD/sssd/pull/5532 Title: #5532: ldap: retry ldap_install_tls() when watchdog interruption ikerexxe commented: """ > Thanks for the updates. > Did you try your latest version with your reproducer? Yes, when the process fails it is retried. """ See the full comment at https://github.com/SSSD/sssd/pull/5532#issuecomment-801756933 ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[SSSD] [sssd PR#5532][synchronized] ldap: retry ldap_install_tls() when watchdog interruption
URL: https://github.com/SSSD/sssd/pull/5532
Author: ikerexxe
Title: #5532: ldap: retry ldap_install_tls() when watchdog interruption
Action: synchronized
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5532/head:pr5532
git checkout pr5532
From 59989754ae1f3f55837c9f5b9a83aa749b708c50 Mon Sep 17 00:00:00 2001
From: Iker Pedrosa
Date: Wed, 3 Mar 2021 15:34:49 +0100
Subject: [PATCH] ldap: retry ldap_install_tls() when watchdog interruption
When the call to ldap_install_tls() fails because the watchdog
interrupted it, retry it. The watchdog interruption is detected by
checking the value of the ticks before and after the call to
ldap_install_tls().
Resolves: https://github.com/SSSD/sssd/issues/5531
---
src/providers/backend.h| 6 +++-
src/providers/data_provider_fo.c | 20 +-
src/providers/krb5/krb5_auth.c | 15 ++
src/providers/ldap/sdap_async_connection.c | 32 ++
src/util/sss_ldap.c| 15 ++
src/util/util.h| 1 +
src/util/util_errors.c | 2 ++
src/util/util_errors.h | 2 ++
src/util/util_watchdog.c | 5
9 files changed, 80 insertions(+), 18 deletions(-)
diff --git a/src/providers/backend.h b/src/providers/backend.h
index f8e8af9aa0..c5280c8ffb 100644
--- a/src/providers/backend.h
+++ b/src/providers/backend.h
@@ -60,6 +60,9 @@ struct be_svc_data {
struct be_svc_callback *callbacks;
struct fo_server *first_resolved;
+
+struct fo_server *current_resolved;
+int current_resolved_attempts;
};
struct be_failover_ctx {
@@ -189,7 +192,8 @@ struct tevent_req *be_resolve_server_send(TALLOC_CTX *memctx,
struct tevent_context *ev,
struct be_ctx *ctx,
const char *service_name,
- bool first_try);
+ bool first_try,
+ bool retry_same_server);
int be_resolve_server_recv(struct tevent_req *req,
TALLOC_CTX *ref_ctx,
struct fo_server **srv);
diff --git a/src/providers/data_provider_fo.c b/src/providers/data_provider_fo.c
index 0dfbb04b0f..c01f117d5c 100644
--- a/src/providers/data_provider_fo.c
+++ b/src/providers/data_provider_fo.c
@@ -24,6 +24,8 @@
#include "providers/backend.h"
#include "resolv/async_resolv.h"
+#define MAX_CURRENT_RESOLVED_ATTEMPTS 1
+
struct be_svc_callback {
struct be_svc_callback *prev;
struct be_svc_callback *next;
@@ -506,7 +508,8 @@ struct tevent_req *be_resolve_server_send(TALLOC_CTX *memctx,
struct tevent_context *ev,
struct be_ctx *ctx,
const char *service_name,
- bool first_try)
+ bool first_try,
+ bool retry_same_server)
{
struct tevent_req *req, *subreq;
struct be_resolve_server_state *state;
@@ -529,6 +532,18 @@ struct tevent_req *be_resolve_server_send(TALLOC_CTX *memctx,
state->attempts = 0;
state->first_try = first_try;
+if (retry_same_server == true &&
+state->svc->current_resolved_attempts <= MAX_CURRENT_RESOLVED_ATTEMPTS) {
+DEBUG(SSSDBG_OP_FAILURE, "Performing retry\n");
+state->srv = state->svc->current_resolved;
+state->svc->current_resolved_attempts++;
+tevent_req_done(req);
+tevent_req_post(req, ev);
+return req;
+}
+
+state->svc->current_resolved_attempts = 0;
+
subreq = fo_resolve_service_send(state, ev,
ctx->be_fo->be_res->resolv,
ctx->be_fo->fo_ctx,
@@ -645,6 +660,9 @@ errno_t be_resolve_server_process(struct tevent_req *subreq,
return ENOENT;
}
+state->svc->current_resolved = state->srv;
+state->svc->current_resolved_attempts++;
+
if (DEBUG_IS_SET(SSSDBG_FUNC_DATA) && fo_get_server_name(state->srv)) {
struct resolv_hostent *srvaddr;
char ipaddr[128];
diff --git a/src/providers/krb5/krb5_auth.c b/src/providers/krb5/krb5_auth.c
index 699c2467b8..95336f68d0 100644
--- a/src/providers/krb5/krb5_auth.c
+++ b/src/providers/krb5/krb5_auth.c
@@ -688,7 +688,8 @@ struct tevent_req *krb5_auth_send(TALLOC_CTX *mem_ctx,
state->search_kpasswd = false;
subreq = be_resolve_server_send(state, state->ev, state->be_ctx,
state->krb5_ctx->service->n
[SSSD] [sssd PR#5532][comment] ldap: retry ldap_install_tls() when watchdog interruption
URL: https://github.com/SSSD/sssd/pull/5532 Title: #5532: ldap: retry ldap_install_tls() when watchdog interruption ikerexxe commented: """ > -- why `Marking port ... as 'not working'`? IIUC, this is exactly ip:port > that is being retried (and succeeds). I've also taken this into account in the new version of the patch. """ See the full comment at https://github.com/SSSD/sssd/pull/5532#issuecomment-801986134 ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[SSSD] [sssd PR#5549][+Changes requested] data_provider: Configure backend probing interval
URL: https://github.com/SSSD/sssd/pull/5549 Title: #5549: data_provider: Configure backend probing interval Label: +Changes requested ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[SSSD] [sssd PR#5532][synchronized] ldap: retry ldap_install_tls() when watchdog interruption
URL: https://github.com/SSSD/sssd/pull/5532
Author: ikerexxe
Title: #5532: ldap: retry ldap_install_tls() when watchdog interruption
Action: synchronized
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5532/head:pr5532
git checkout pr5532
From e14a200e401a9ec3c14ee48d87ed0b42a701dca6 Mon Sep 17 00:00:00 2001
From: Iker Pedrosa
Date: Wed, 3 Mar 2021 15:34:49 +0100
Subject: [PATCH] ldap: retry ldap_install_tls() when watchdog interruption
When the call to ldap_install_tls() fails because the watchdog
interrupted it, retry it. The watchdog interruption is detected by
checking the value of the ticks before and after the call to
ldap_install_tls().
Resolves: https://github.com/SSSD/sssd/issues/5531
---
src/providers/backend.h| 6 +++-
src/providers/data_provider_fo.c | 29 +++-
src/providers/krb5/krb5_auth.c | 15 ++
src/providers/ldap/sdap_async_connection.c | 32 ++
src/util/sss_ldap.c| 12
src/util/util.h| 1 +
src/util/util_errors.c | 2 ++
src/util/util_errors.h | 2 ++
src/util/util_watchdog.c | 5
9 files changed, 86 insertions(+), 18 deletions(-)
diff --git a/src/providers/backend.h b/src/providers/backend.h
index f8e8af9aa0..c5280c8ffb 100644
--- a/src/providers/backend.h
+++ b/src/providers/backend.h
@@ -60,6 +60,9 @@ struct be_svc_data {
struct be_svc_callback *callbacks;
struct fo_server *first_resolved;
+
+struct fo_server *current_resolved;
+int current_resolved_attempts;
};
struct be_failover_ctx {
@@ -189,7 +192,8 @@ struct tevent_req *be_resolve_server_send(TALLOC_CTX *memctx,
struct tevent_context *ev,
struct be_ctx *ctx,
const char *service_name,
- bool first_try);
+ bool first_try,
+ bool retry_same_server);
int be_resolve_server_recv(struct tevent_req *req,
TALLOC_CTX *ref_ctx,
struct fo_server **srv);
diff --git a/src/providers/data_provider_fo.c b/src/providers/data_provider_fo.c
index 0dfbb04b0f..89dbcaabd2 100644
--- a/src/providers/data_provider_fo.c
+++ b/src/providers/data_provider_fo.c
@@ -24,6 +24,8 @@
#include "providers/backend.h"
#include "resolv/async_resolv.h"
+#define MAX_RESOLVED_ATTEMPTS 2
+
struct be_svc_callback {
struct be_svc_callback *prev;
struct be_svc_callback *next;
@@ -506,7 +508,8 @@ struct tevent_req *be_resolve_server_send(TALLOC_CTX *memctx,
struct tevent_context *ev,
struct be_ctx *ctx,
const char *service_name,
- bool first_try)
+ bool first_try,
+ bool retry_same_server)
{
struct tevent_req *req, *subreq;
struct be_resolve_server_state *state;
@@ -529,6 +532,27 @@ struct tevent_req *be_resolve_server_send(TALLOC_CTX *memctx,
state->attempts = 0;
state->first_try = first_try;
+if (retry_same_server &&
+state->svc->current_resolved_attempts < MAX_RESOLVED_ATTEMPTS) {
+state->srv = state->svc->current_resolved;
+state->svc->current_resolved_attempts++;
+DEBUG(SSSDBG_IMPORTANT_INFO, "Performing retry to %s:%d\n",
+ fo_get_server_str_name(state->srv),
+ fo_get_server_port(state->srv));
+tevent_req_done(req);
+tevent_req_post(req, ev);
+return req;
+} else if (retry_same_server &&
+ state->svc->current_resolved_attempts >= MAX_RESOLVED_ATTEMPTS) {
+DEBUG(SSSDBG_IMPORTANT_INFO,
+ "Will not retry %s:%d, maximum number of attempts (%d) reached\n",
+ fo_get_server_str_name(state->srv),
+ fo_get_server_port(state->srv),
+ MAX_RESOLVED_ATTEMPTS);
+}
+
+state->svc->current_resolved_attempts = 0;
+
subreq = fo_resolve_service_send(state, ev,
ctx->be_fo->be_res->resolv,
ctx->be_fo->fo_ctx,
@@ -645,6 +669,9 @@ errno_t be_resolve_server_process(struct tevent_req *subreq,
return ENOENT;
}
+state->svc->current_resolved = state->srv;
+state->svc->current_resolved_attempts++;
+
if (DEBUG_IS_SET(SSSDBG_FUNC_DATA) && fo_get_server_name(state->srv)) {
[SSSD] [sssd PR#5532][comment] ldap: retry ldap_install_tls() when watchdog interruption
URL: https://github.com/SSSD/sssd/pull/5532 Title: #5532: ldap: retry ldap_install_tls() when watchdog interruption ikerexxe commented: """ I've changed the PR taking into account your improvements. """ See the full comment at https://github.com/SSSD/sssd/pull/5532#issuecomment-810005543 ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[SSSD] [sssd PR#5495][comment] Fix python headers detection with recent autoconf (Fixes #5336)
URL: https://github.com/SSSD/sssd/pull/5495 Title: #5495: Fix python headers detection with recent autoconf (Fixes #5336) ikerexxe commented: """ By the way, do you mind changing your commit messages? Something in the following format would be nice: `configure: Fix python headers detection with recent autoconf` `Resolves: https://github.com/SSSD/sssd/issues/5336` """ See the full comment at https://github.com/SSSD/sssd/pull/5495#issuecomment-810284993 ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[SSSD] [sssd PR#5532][synchronized] ldap: retry ldap_install_tls() when watchdog interruption
URL: https://github.com/SSSD/sssd/pull/5532
Author: ikerexxe
Title: #5532: ldap: retry ldap_install_tls() when watchdog interruption
Action: synchronized
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5532/head:pr5532
git checkout pr5532
From 41f59e1b90b331869b56d74414a347a8204c7f2b Mon Sep 17 00:00:00 2001
From: Iker Pedrosa
Date: Wed, 3 Mar 2021 15:34:49 +0100
Subject: [PATCH] ldap: retry ldap_install_tls() when watchdog interruption
When the call to ldap_install_tls() fails because the watchdog
interrupted it, retry it. The watchdog interruption is detected by
checking the value of the ticks before and after the call to
ldap_install_tls().
Resolves: https://github.com/SSSD/sssd/issues/5531
---
src/providers/backend.h| 6 +++-
src/providers/data_provider_fo.c | 32 +-
src/providers/krb5/krb5_auth.c | 15 ++
src/providers/ldap/sdap_async_connection.c | 32 ++
src/util/sss_ldap.c| 12
src/util/util.h| 1 +
src/util/util_errors.c | 2 ++
src/util/util_errors.h | 2 ++
src/util/util_watchdog.c | 5
9 files changed, 89 insertions(+), 18 deletions(-)
diff --git a/src/providers/backend.h b/src/providers/backend.h
index f8e8af9aa0..c5280c8ffb 100644
--- a/src/providers/backend.h
+++ b/src/providers/backend.h
@@ -60,6 +60,9 @@ struct be_svc_data {
struct be_svc_callback *callbacks;
struct fo_server *first_resolved;
+
+struct fo_server *current_resolved;
+int current_resolved_attempts;
};
struct be_failover_ctx {
@@ -189,7 +192,8 @@ struct tevent_req *be_resolve_server_send(TALLOC_CTX *memctx,
struct tevent_context *ev,
struct be_ctx *ctx,
const char *service_name,
- bool first_try);
+ bool first_try,
+ bool retry_same_server);
int be_resolve_server_recv(struct tevent_req *req,
TALLOC_CTX *ref_ctx,
struct fo_server **srv);
diff --git a/src/providers/data_provider_fo.c b/src/providers/data_provider_fo.c
index 0dfbb04b0f..91ea7e68c2 100644
--- a/src/providers/data_provider_fo.c
+++ b/src/providers/data_provider_fo.c
@@ -24,6 +24,8 @@
#include "providers/backend.h"
#include "resolv/async_resolv.h"
+#define MAX_RESOLVED_ATTEMPTS 2
+
struct be_svc_callback {
struct be_svc_callback *prev;
struct be_svc_callback *next;
@@ -506,7 +508,8 @@ struct tevent_req *be_resolve_server_send(TALLOC_CTX *memctx,
struct tevent_context *ev,
struct be_ctx *ctx,
const char *service_name,
- bool first_try)
+ bool first_try,
+ bool retry_same_server)
{
struct tevent_req *req, *subreq;
struct be_resolve_server_state *state;
@@ -529,6 +532,30 @@ struct tevent_req *be_resolve_server_send(TALLOC_CTX *memctx,
state->attempts = 0;
state->first_try = first_try;
+if (retry_same_server &&
+state->svc->current_resolved_attempts < MAX_RESOLVED_ATTEMPTS) {
+state->srv = state->svc->current_resolved;
+state->svc->current_resolved_attempts++;
+DEBUG(SSSDBG_IMPORTANT_INFO, "Performing retry to %s:%d\n",
+ fo_get_server_str_name(state->srv),
+ fo_get_server_port(state->srv));
+tevent_req_done(req);
+tevent_req_post(req, ev);
+return req;
+} else if (retry_same_server &&
+ state->svc->current_resolved_attempts >= MAX_RESOLVED_ATTEMPTS) {
+DEBUG(SSSDBG_IMPORTANT_INFO,
+ "Will not retry %s:%d, maximum number of attempts (%d) reached\n",
+ fo_get_server_str_name(state->svc->current_resolved),
+ fo_get_server_port(state->svc->current_resolved),
+ MAX_RESOLVED_ATTEMPTS);
+be_fo_set_port_status(state->ctx, state->svc->name,
+ state->svc->current_resolved,
+ PORT_NOT_WORKING);
+}
+
+state->svc->current_resolved_attempts = 0;
+
subreq = fo_resolve_service_send(state, ev,
ctx->be_fo->be_res->resolv,
ctx->be_fo->fo_ctx,
@@ -645,6 +672,9 @@ errno_t be_resolve_server_process(struct tevent_req *subreq,
[SSSD] [sssd PR#5532][+Waiting for review] ldap: retry ldap_install_tls() when watchdog interruption
URL: https://github.com/SSSD/sssd/pull/5532 Title: #5532: ldap: retry ldap_install_tls() when watchdog interruption Label: +Waiting for review ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[SSSD] [sssd PR#5532][comment] ldap: retry ldap_install_tls() when watchdog interruption
URL: https://github.com/SSSD/sssd/pull/5532 Title: #5532: ldap: retry ldap_install_tls() when watchdog interruption ikerexxe commented: """ > -- `status ... is 'working'` despite `Will not retry` Pushed a new revision that changes this behaviour. """ See the full comment at https://github.com/SSSD/sssd/pull/5532#issuecomment-810875437 ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[SSSD] [sssd PR#5532][comment] ldap: retry ldap_install_tls() when watchdog interruption
URL: https://github.com/SSSD/sssd/pull/5532 Title: #5532: ldap: retry ldap_install_tls() when watchdog interruption ikerexxe commented: """ > Thanks. From functional point of view this now looks good. > > I'm really not sure about implementation of FO code changes though. > Do we actually need to go to FO code? Can't we handle `retry_same_server` > right inside `sdap_cli_resolve_next()`? It's possible but I am not sure if that's better than the actual implementation. @sumit-bose what do you think? Should I move the retry logic from `be_resolve_server_send()` to `sdap_cli_resolve_next()` as Alexey suggests? """ See the full comment at https://github.com/SSSD/sssd/pull/5532#issuecomment-810947952 ___ 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://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
