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 35be49f7fd2f9bc50d24cf988cd53e45173e860f 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/4] CONFDB: Start a ldb transaction from confdb_merge_parent_domain() 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/confdb/confdb.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/confdb/confdb.c b/src/confdb/confdb.c index 5b4cbec8e..505255ec0 100644 --- a/src/confdb/confdb.c +++ b/src/confdb/confdb.c @@ -2012,6 +2012,7 @@ static int confdb_merge_parent_domain(const char *name, struct ldb_result *app_section) { int ret; + int cancel_ret; int ldb_flag; struct ldb_result *parent_domain = NULL; struct ldb_message *replace_msg = NULL; @@ -2116,6 +2117,15 @@ static int confdb_merge_parent_domain(const char *name, } } + ret = ldb_transaction_start(cdb->ldb); + if (ret != LDB_SUCCESS) { + ret = sysdb_error_to_errno(ret); + DEBUG(SSSDBG_CRIT_FAILURE, + "Failed to start ldb transaction [%d]: %s\n", + ret, sss_strerror(ret)); + goto done; + } + /* We use permissive modification here because adding cn or * distinguishedName from the app_section to the application * message would throw EEXIST @@ -2126,6 +2136,24 @@ static int confdb_merge_parent_domain(const char *name, DEBUG(SSSDBG_OP_FAILURE, "Adding app-specific options failed [%d]: %s\n", ret, sss_strerror(ret)); + + cancel_ret = ldb_transaction_cancel(cdb->ldb); + if (cancel_ret != LDB_SUCCESS) { + cancel_ret = sysdb_error_to_errno(cancel_ret); + DEBUG(SSSDBG_CRIT_FAILURE, + "Failed to cancel ldb transaction [%d]: %s\n", + cancel_ret, sss_strerror(cancel_ret)); + } + + goto done; + } + + ret = ldb_transaction_commit(cdb->ldb); + if (ret != LDB_SUCCESS) { + ret = sysdb_error_to_errno(ret); + DEBUG(SSSDBG_CRIT_FAILURE, + "Failed to commit ldb transaction [%d]: %s\n", + ret, sss_strerror(ret)); goto done; } From 95b2a9f9ff7b5d17ccf0201583ca8b5ee21da69f 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/4] 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 e627780c39299c88db4a47d051fa6e706c426863 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fiden...@redhat.com> Date: Thu, 15 Mar 2018 16:26:03 +0100 Subject: [PATCH 3/4] TESTS: Move get_call_output() to utils.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Although this function is helpful only for `sssctl` related tests, it'll be reused from the tests_infopipe code, from where some new `sssctl` tests will be added (the ones depending on infopipe). Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com> --- src/tests/intg/test_sssctl.py | 9 +-------- src/tests/intg/util.py | 7 +++++++ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/tests/intg/test_sssctl.py b/src/tests/intg/test_sssctl.py index 0df5d0bc1..e8861dd86 100644 --- a/src/tests/intg/test_sssctl.py +++ b/src/tests/intg/test_sssctl.py @@ -28,7 +28,7 @@ import ds_openldap import ldap_ent import config -from util import unindent +from util import unindent, get_call_output import sssd_netgroup LDAP_BASE_DN = "dc=example,dc=com" @@ -203,13 +203,6 @@ def fqname_case_insensitive_rfc2307(request, ldap_conn): return None -def get_call_output(cmd): - process = subprocess.Popen(cmd, stdout=subprocess.PIPE, - stderr=subprocess.PIPE) - output, ret = process.communicate() - return output.decode('utf-8') - - def test_user_show_basic_sanity(ldap_conn, sanity_rfc2307, portable_LC_ALL): # Fill the cache first ent.assert_passwd_by_name( diff --git a/src/tests/intg/util.py b/src/tests/intg/util.py index 2b40311bd..a1c439648 100644 --- a/src/tests/intg/util.py +++ b/src/tests/intg/util.py @@ -78,3 +78,10 @@ def restore_envvar_file(name): path = os.environ[name] backup_path = path + ".bak" os.rename(backup_path, path) + + +def get_call_output(cmd): + process = subprocess.Popen(cmd, stdout=subprocess.PIPE, + stderr=subprocess.PIPE) + output, ret = process.communicate() + return output.decode('utf-8') From d84c7b1b3867d747b2ac491b9003e05c8aa10c56 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 4/4] 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 | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/tests/intg/test_infopipe.py b/src/tests/intg/test_infopipe.py index 3a7961403..89b9f9399 100644 --- a/src/tests/intg/test_infopipe.py +++ b/src/tests/intg/test_infopipe.py @@ -34,7 +34,7 @@ import config import ds_openldap import ldap_ent -from util import unindent +from util import unindent, get_call_output LDAP_BASE_DN = "dc=example,dc=com" INTERACTIVE_TIMEOUT = 4 @@ -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,10 @@ 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): + output = get_call_output(["sssctl", "domain-list"]) + + 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
