URL: https://github.com/SSSD/sssd/pull/537 Author: fidencio Title: #537: confdb_expand_application_domains() related issues (round 1) Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/537/head:pr537 git checkout pr537
From a1c5652430627758339128a8e87598d900a3940c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com> Date: Wed, 14 Mar 2018 22:55:21 +0100 Subject: [PATCH 1/3] CONFDB: Start a ldb transaction from sss_ldb_modify_permissive() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The reason why confdb_expand_app_domains() always fails is because we try to do a ldb_request() without starting a ldb transaction. When we're dealing with ldb_modify(), ldb_add(), ldb_delete() kind of messages, those call ldb_autotransaction_request() which will start a new transaction and treat it properly when doing the ldb_request(). In our case that we're calling ldb_request() by our own, we must ensure that the transaction is started and properly deal with it._ It's never been noticed because in the only place the function is used its errors are ignored. Resolves: https://pagure.io/SSSD/sssd/issue/3660 Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com> --- src/db/sysdb_ops.c | 44 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c index 15915101e..6fc1d5430 100644 --- a/src/db/sysdb_ops.c +++ b/src/db/sysdb_ops.c @@ -66,7 +66,9 @@ int sss_ldb_modify_permissive(struct ldb_context *ldb, struct ldb_message *msg) { struct ldb_request *req; - int ret = EOK; + int ret; + int sysdb_ret; + bool in_transaction = false; ret = ldb_build_mod_req(&req, ldb, ldb, msg, @@ -84,9 +86,49 @@ int sss_ldb_modify_permissive(struct ldb_context *ldb, return ret; } + ret = ldb_transaction_start(ldb); + if (ret != LDB_SUCCESS) { + sysdb_ret = sysdb_error_to_errno(ret); + DEBUG(SSSDBG_CRIT_FAILURE, + "Failed to start ldb transaction [%d]: %s\n", + sysdb_ret, sss_strerror(sysdb_ret)); + goto done; + } + + in_transaction = true; + ret = ldb_request(ldb, req); if (ret == LDB_SUCCESS) { ret = ldb_wait(req->handle, LDB_WAIT_ALL); + if (ret != LDB_SUCCESS) { + goto done; + } + } + + ret = ldb_transaction_commit(ldb); + if (ret != LDB_SUCCESS) { + sysdb_ret = sysdb_error_to_errno(ret); + DEBUG(SSSDBG_CRIT_FAILURE, + "Failed to commit ldb transaction [%d]: %s\n", + sysdb_ret, sss_strerror(sysdb_ret)); + goto done; + } + + in_transaction = false; + + ret = LDB_SUCCESS; + +done: + if (in_transaction) { + /* Let's just reuse sysdb_ret here in order to not override the + * original ret withe the return of the ldb_transaction_cancel() */ + sysdb_ret = ldb_transaction_cancel(ldb); + if (sysdb_ret != LDB_SUCCESS) { + sysdb_ret = sysdb_error_to_errno(sysdb_ret); + DEBUG(SSSDBG_CRIT_FAILURE, + "Failed to cancel ldb transaction [%d]: %s\n", + sysdb_ret, sss_strerror(sysdb_ret)); + } } talloc_free(req); From d7ec4b2b3c51d205d42d55641c9a75ad18a35245 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com> Date: Wed, 14 Mar 2018 23:01:39 +0100 Subject: [PATCH 2/3] TOOLS: Take into consideration app domains MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In order to properly show an app domain when listing domains using sssctl domain-list we have to expand the confdb, as already done in the monitor code. Resolves: https://pagure.io/SSSD/sssd/issue/3658 Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com> --- src/tools/common/sss_tools.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/tools/common/sss_tools.c b/src/tools/common/sss_tools.c index e491a1286..4832db5a0 100644 --- a/src/tools/common/sss_tools.c +++ b/src/tools/common/sss_tools.c @@ -117,6 +117,14 @@ static errno_t sss_tool_domains_init(TALLOC_CTX *mem_ctx, struct sss_domain_info *dom; errno_t ret; + ret = confdb_expand_app_domains(confdb); + if (ret != EOK) { + DEBUG(SSSDBG_CRIT_FAILURE, + "Unable to expand application domains [%d]: %s\n", + ret, sss_strerror(ret)); + return ret; + } + ret = confdb_get_domains(confdb, &domains); if (ret != EOK) { DEBUG(SSSDBG_CRIT_FAILURE, "Unable to setup domains [%d]: %s\n", From 65f7995faefeb9e4ecedc671af096b35c7cc220a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com> Date: Thu, 15 Mar 2018 16:28:41 +0100 Subject: [PATCH 3/3] TESTS: Add a basic test of `sssctl domain-list` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Let's just add a test for `sssctl domain-list` in order to avoid regressing https://pagure.io/SSSD/sssd/issue/3658. The test has been added as part of test_infopipe.py in order to take advantage of the machinery already provided there. Resolves: https://pagure.io/SSSD/sssd/issue/3658 Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com> --- src/tests/intg/test_infopipe.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/tests/intg/test_infopipe.py b/src/tests/intg/test_infopipe.py index 3a7961403..55e1f98c3 100644 --- a/src/tests/intg/test_infopipe.py +++ b/src/tests/intg/test_infopipe.py @@ -194,7 +194,7 @@ def format_basic_conf(ldap_conn, schema): return unindent("""\ [sssd] debug_level = 0xffff - domains = LDAP + domains = LDAP, app services = nss, ifp enable_files_domain = false @@ -212,6 +212,9 @@ def format_basic_conf(ldap_conn, schema): id_provider = ldap ldap_uri = {ldap_conn.ds_inst.ldap_url} ldap_search_base = {ldap_conn.ds_inst.base_dn} + + [application/app] + inherit_from = LDAP """).format(**locals()) @@ -532,3 +535,14 @@ def test_get_user_groups(dbus_system_bus, ldap_conn, sanity_rfc2307): assert len(res) == 2 assert sorted(res) == ['single_user_group', 'two_user_group'] + + +def test_sssctl_domain_list_app_domain(dbus_system_bus, + ldap_conn, + sanity_rfc2307): + cmd = ["sssctl", "domain-list"] + output = subprocess.check_output(cmd, stderr=subprocess.STDOUT) + + assert "Error" not in output + assert output.find("LDAP") != -1 + assert output.find("app") != -1
_______________________________________________ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
