URL: https://github.com/SSSD/sssd/pull/936 Author: pbrezina Title: #936: autofs: delete possible duplicate of an autofs entry Action: opened
PR body: """ Steps to reproduce: 1. Create the following autofs objects ```ldif dn: ou=auto.master,ou=autofs,dc=ldap,dc=vm objectClass: automountMap objectClass: top ou: auto.master dn: automountKey=/home,ou=auto.master,ou=autofs,dc=ldap,dc=vm objectClass: automount objectClass: top automountInformation: auto.home automountKey: /home dn: ou=auto.home,ou=autofs,dc=ldap,dc=vm objectClass: automountMap objectClass: top ou: auto.home dn: automountKey=/home1,ou=auto.home,ou=autofs,dc=ldap,dc=vm objectClass: automount objectClass: top automountInformation: home1 automountKey: /home1 ``` 2. Use e.g. the test tool to fetch the maps: ``` ./autofs_test_client auto.master ./autofs_test_client auto.home -n /home1 ``` 3. Change automountInformation of /home1 ``` dn: automountKey=/home1,ou=auto.home,ou=autofs,dc=ldap,dc=vm objectClass: automount objectClass: top automountInformation: home1_1 automountKey: /home1 ``` 4. Run the test tool again: ``` ./autofs_test_client auto.master ./autofs_test_client auto.home -n /home1 > error happens ``` It is important the `get entry by name is called` thus the `-n` parameter. Resolves: https://pagure.io/SSSD/sssd/issue/4116 """ To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/936/head:pr936 git checkout pr936
From 819a6300ad94ad5b360634bbd4a79688b90e4d91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrez...@redhat.com> Date: Mon, 11 Nov 2019 13:38:29 +0100 Subject: [PATCH 1/2] autofs: remove unused enum --- src/providers/ldap/sdap_async_autofs.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/providers/ldap/sdap_async_autofs.c b/src/providers/ldap/sdap_async_autofs.c index c31df2f59f..232d0c34a1 100644 --- a/src/providers/ldap/sdap_async_autofs.c +++ b/src/providers/ldap/sdap_async_autofs.c @@ -30,11 +30,6 @@ #include "providers/ldap/sdap_autofs.h" #include "providers/ldap/sdap_ops.h" -enum autofs_map_op { - AUTOFS_MAP_OP_ADD, - AUTOFS_MAP_OP_DEL -}; - /* ====== Utility functions ====== */ static const char * get_autofs_map_name(struct sysdb_attrs *map, struct sdap_options *opts) From 6f3dea6b255b70f7c2c50cf205dd7d62010bfd97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrez...@redhat.com> Date: Mon, 11 Nov 2019 13:56:34 +0100 Subject: [PATCH 2/2] autofs: delete possible duplicate of an autofs entry Steps to reproduce: 1. Create the following autofs objects ```ldif dn: ou=auto.master,ou=autofs,dc=ldap,dc=vm objectClass: automountMap objectClass: top ou: auto.master dn: automountKey=/home,ou=auto.master,ou=autofs,dc=ldap,dc=vm objectClass: automount objectClass: top automountInformation: auto.home automountKey: /home dn: ou=auto.home,ou=autofs,dc=ldap,dc=vm objectClass: automountMap objectClass: top ou: auto.home dn: automountKey=/home1,ou=auto.home,ou=autofs,dc=ldap,dc=vm objectClass: automount objectClass: top automountInformation: home1 automountKey: /home1 ``` 2. Use e.g. the test tool to fetch the maps: ``` ./autofs_test_client auto.master ./autofs_test_client auto.home -n /home1 ``` 3. Change automountInformation of /home1 ``` dn: automountKey=/home1,ou=auto.home,ou=autofs,dc=ldap,dc=vm objectClass: automount objectClass: top automountInformation: home1_1 automountKey: /home1 ``` 4. Run the test tool again: ``` ./autofs_test_client auto.master ./autofs_test_client auto.home -n /home1 > error happens ``` It is important the `get entry by name is called` thus the `-n` parameter. Resolves: https://pagure.io/SSSD/sssd/issue/4116 --- src/providers/ldap/sdap_async_autofs.c | 93 ++++++++++++++++++++------ 1 file changed, 74 insertions(+), 19 deletions(-) diff --git a/src/providers/ldap/sdap_async_autofs.c b/src/providers/ldap/sdap_async_autofs.c index 232d0c34a1..6f37b1c843 100644 --- a/src/providers/ldap/sdap_async_autofs.c +++ b/src/providers/ldap/sdap_async_autofs.c @@ -1337,6 +1337,12 @@ static void sdap_autofs_get_entry_connect_done(struct tevent_req *subreq) tevent_req_set_callback(subreq, sdap_autofs_get_entry_done, req); } +static errno_t sdap_autofs_save_entry(struct sss_domain_info *domain, + struct sdap_options *opts, + struct sysdb_attrs *newentry, + const char *mapname, + const char *entryname); + static void sdap_autofs_get_entry_done(struct tevent_req *subreq) { struct tevent_req *req; @@ -1365,31 +1371,18 @@ static void sdap_autofs_get_entry_done(struct tevent_req *subreq) return; } - if (reply_count == 0) { - ret = sysdb_del_autofsentry_by_key(state->id_ctx->be->domain, - state->mapname, state->entryname); - if (ret != EOK) { - DEBUG(SSSDBG_MINOR_FAILURE, "Cannot delete entry %s:%s\n", - state->mapname, state->entryname); - tevent_req_error(req, ret); - return; - } - - tevent_req_done(req); - return; - } - - ret = add_autofs_entry(state->id_ctx->be->domain, state->mapname, - state->opts, reply[0], time(NULL)); + ret = sdap_autofs_save_entry(state->id_ctx->be->domain, + state->opts, + reply_count != 0 ? reply[0] : NULL, + state->mapname, + state->entryname); if (ret != EOK) { - DEBUG(SSSDBG_OP_FAILURE, - "Cannot save autofs entry %s:%s [%d]: %s\n", - state->mapname, state->entryname, ret, strerror(ret)); tevent_req_error(req, ret); return; } tevent_req_done(req); + return; } errno_t sdap_autofs_get_entry_recv(struct tevent_req *req, @@ -1405,3 +1398,65 @@ errno_t sdap_autofs_get_entry_recv(struct tevent_req *req, return EOK; } + +static errno_t sdap_autofs_save_entry(struct sss_domain_info *domain, + struct sdap_options *opts, + struct sysdb_attrs *newentry, + const char *mapname, + const char *entryname) +{ + bool in_transaction = false; + errno_t ret; + int tret; + + ret = sysdb_transaction_start(domain->sysdb); + if (ret != EOK) { + DEBUG(SSSDBG_CRIT_FAILURE, "Cannot start sysdb transaction [%d]: %s\n", + ret, sss_strerror(ret)); + goto done; + } + in_transaction = true; + + /* Delete existing entry to cover case where new entry has the same key + * but different automountInformation. Because the dn is created from the + * combination of key and information it would be possible to end up with + * two entries with same key but different information otherwise. + */ + ret = sysdb_del_autofsentry_by_key(domain, mapname, entryname); + if (ret != EOK) { + DEBUG(SSSDBG_MINOR_FAILURE, "Cannot delete entry %s:%s\n", + mapname, entryname); + goto done; + } + + if (newentry != NULL) { + ret = add_autofs_entry(domain, mapname, opts, newentry, time(NULL)); + if (ret != EOK) { + DEBUG(SSSDBG_OP_FAILURE, + "Cannot save autofs entry %s:%s [%d]: %s\n", + mapname, entryname, ret, sss_strerror(ret)); + goto done; + } + } + + ret = sysdb_transaction_commit(domain->sysdb); + if (ret != EOK) { + DEBUG(SSSDBG_CRIT_FAILURE, "Cannot commit sysdb transaction [%d]: %s\n", + ret, sss_strerror(ret)); + goto done; + } + in_transaction = false; + + ret = EOK; + +done: + if (in_transaction) { + tret = sysdb_transaction_cancel(domain->sysdb); + if (tret != EOK) { + DEBUG(SSSDBG_CRIT_FAILURE, "Cannot cancel sysdb transaction " + "[%d]: %s\n", ret, sss_strerror(ret)); + } + } + + return ret; +}
_______________________________________________ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org Fedora Code of Conduct: https://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