https://fedorahosted.org/sssd/ticket/1758
The autofs entries do not have the key as an unique identifier, but rather the full (key, value) tuple as some keys have a special meaning, such as the direct mount key (/-) and may be present in a single map multiple times. Comparing the full DN that contains both the key and the value will allow for working updates if either key or value changes.
>From fcd1889485bb0cc7115da4bc6f14212ef6d97aed Mon Sep 17 00:00:00 2001 From: Jakub Hrozek <[email protected]> Date: Sat, 19 Jan 2013 15:17:42 +0100 Subject: [PATCH] LDAP: Compare lists of DNs when saving autofs entries https://fedorahosted.org/sssd/ticket/1758 The autofs entries do not have the key as an unique identifier, but rather the full (key, value) tuple as some keys have a special meaning, such as the direct mount key (/-) and may be present in a single map multiple times. Comparing the full DN that contains both the key and the value will allow for working updates if either key or value changes. --- src/db/sysdb_autofs.c | 27 ++- src/db/sysdb_autofs.h | 13 +- src/providers/ldap/sdap_async_autofs.c | 303 +++++++++++++++++---------------- 3 files changed, 189 insertions(+), 154 deletions(-) diff --git a/src/db/sysdb_autofs.c b/src/db/sysdb_autofs.c index f8fe3925b49d936c8c40986ca5087a82bda42d8e..cc44c350b744fc01f3a081c4a3de57b879a2693a 100644 --- a/src/db/sysdb_autofs.c +++ b/src/db/sysdb_autofs.c @@ -78,6 +78,26 @@ done: return dn; } +char * +sysdb_autofsentry_strdn(TALLOC_CTX *mem_ctx, + struct sysdb_ctx *sysdb, + struct sss_domain_info *domain, + const char *map_name, + const char *entry_name, + const char *entry_value) +{ + struct ldb_dn *dn; + char *strdn; + + dn = sysdb_autofsentry_dn(mem_ctx, sysdb, domain, + map_name, entry_name, entry_value); + if (!dn) return NULL; + + strdn = talloc_strdup(mem_ctx, ldb_dn_get_linearized(dn)); + talloc_free(dn); + return strdn; +} + errno_t sysdb_save_autofsmap(struct sysdb_ctx *sysdb_ctx, struct sss_domain_info *domain, @@ -319,15 +339,12 @@ done: errno_t sysdb_del_autofsentry(struct sysdb_ctx *sysdb_ctx, - struct sss_domain_info *domain, - const char *map, - const char *key, - const char *value) + const char *entry_dn) { struct ldb_dn *dn; errno_t ret; - dn = sysdb_autofsentry_dn(sysdb_ctx, sysdb_ctx, domain, map, key, value); + dn = ldb_dn_new(NULL, sysdb_ctx_get_ldb(sysdb_ctx), entry_dn); if (!dn) { return ENOMEM; } diff --git a/src/db/sysdb_autofs.h b/src/db/sysdb_autofs.h index 0d130946250d56c01966ff1ea9ceb7a871e56ba3..e3528ce4e6bd2c8b594c80828cfb02a9a7ce7923 100644 --- a/src/db/sysdb_autofs.h +++ b/src/db/sysdb_autofs.h @@ -65,10 +65,7 @@ sysdb_save_autofsentry(struct sysdb_ctx *sysdb_ctx, struct sysdb_attrs *attrs); errno_t sysdb_del_autofsentry(struct sysdb_ctx *sysdb_ctx, - struct sss_domain_info *domain, - const char *map, - const char *key, - const char *value); + const char *entry_dn); errno_t sysdb_autofs_entries_by_map(TALLOC_CTX *mem_ctx, @@ -89,4 +86,12 @@ errno_t sysdb_invalidate_autofs_maps(struct sysdb_ctx *sysdb, struct sss_domain_info *domain); +char * +sysdb_autofsentry_strdn(TALLOC_CTX *mem_ctx, + struct sysdb_ctx *sysdb, + struct sss_domain_info *domain, + const char *map_name, + const char *entry_name, + const char *entry_value); + #endif /* _SYSDB_AUTOFS_H_ */ diff --git a/src/providers/ldap/sdap_async_autofs.c b/src/providers/ldap/sdap_async_autofs.c index 6306991e22fc3326bea32b7e04ec83ceeedcf775..8e874d64b5eb047fae3f36816e3ff4d342fb8575 100644 --- a/src/providers/ldap/sdap_async_autofs.c +++ b/src/providers/ldap/sdap_async_autofs.c @@ -50,127 +50,60 @@ get_autofs_map_name(struct sysdb_attrs *map, struct sdap_options *opts) } static const char * +get_autofs_entry_attr(struct sysdb_attrs *entry, struct sdap_options *opts, + enum sdap_autofs_entry_attrs attr) +{ + errno_t ret; + struct ldb_message_element *el; + + ret = sysdb_attrs_get_el(entry, + opts->autofs_entry_map[attr].sys_name, + &el); + if (ret) return NULL; + if (el->num_values != 1) { + DEBUG(SSSDBG_CRIT_FAILURE, + ("Expected one entry got %d\n", el->num_values)); + return NULL; + } + + return (const char *)el->values[0].data; +} + +static const char * get_autofs_entry_key(struct sysdb_attrs *entry, struct sdap_options *opts) { - errno_t ret; - struct ldb_message_element *el; + return get_autofs_entry_attr(entry, opts, SDAP_AT_AUTOFS_ENTRY_KEY); +} - ret = sysdb_attrs_get_el(entry, - opts->autofs_entry_map[SDAP_AT_AUTOFS_ENTRY_KEY].sys_name, - &el); - if (ret) return NULL; - if (el->num_values == 0) return NULL; - - return (const char *)el->values[0].data; +static const char * +get_autofs_entry_value(struct sysdb_attrs *entry, struct sdap_options *opts) +{ + return get_autofs_entry_attr(entry, opts, SDAP_AT_AUTOFS_ENTRY_VALUE); } static errno_t -mod_autofs_entry(struct sysdb_ctx *sysdb, +add_autofs_entry(struct sysdb_ctx *sysdb, struct sss_domain_info *domain, const char *map, struct sdap_options *opts, - struct sysdb_attrs *entry, - enum autofs_map_op mod_op) + struct sysdb_attrs *entry) { const char *key; const char *value; - struct ldb_message_element *el; - errno_t ret; key = get_autofs_entry_key(entry, opts); - if (!key) return EINVAL; - - ret = sysdb_attrs_get_el(entry, - opts->autofs_entry_map[SDAP_AT_AUTOFS_ENTRY_VALUE].sys_name, - &el); - if (ret) return ret; - if (el->num_values == 0) return EINVAL; - else value = (const char *)el->values[0].data; - - switch (mod_op) { - case AUTOFS_MAP_OP_ADD: - ret = sysdb_save_autofsentry(sysdb, domain, map, key, value, NULL); - break; - case AUTOFS_MAP_OP_DEL: - ret = sysdb_del_autofsentry(sysdb, domain, map, key, value); - break; - } - - return ret; -} - -static errno_t -mod_autofs_entries(struct sysdb_ctx *sysdb, - struct sss_domain_info *domain, - struct sdap_options *opts, - const char *map, - char **mod_entries, - struct sysdb_attrs **entries, - size_t num_entries, - enum autofs_map_op mod_op) - -{ - errno_t ret, tret; - const char *key; - bool in_transaction = false; - int i, j; - - ret = sysdb_transaction_start(sysdb); - if (ret != EOK) { - DEBUG(SSSDBG_CRIT_FAILURE, - ("Cannot start sysdb transaction [%d]: %s\n", - ret, strerror(ret))); - goto done; - } - in_transaction = true; - - /* Loop through entry names.. */ - for (i=0; mod_entries[i]; i++) { - for (j=0; j < num_entries; j++) { - /* get a pointer to sysdb_attrs of an entry that is not - * cached, skip names that are not in **entries - */ - key = get_autofs_entry_key(entries[j], opts); - if (!key) { - DEBUG(SSSDBG_MINOR_FAILURE, - ("An entry without name? Skipping\n")); - return EINVAL; - } - - if (strcmp(mod_entries[i], key)) { - continue; - } - - ret = mod_autofs_entry(sysdb, domain, map, opts, entries[j], mod_op); - if (ret != EOK) { - DEBUG(SSSDBG_OP_FAILURE, - ("Cannot modify autofs entry [%d]: %s. Ignoring.\n", - ret, strerror(ret))); - continue; - } - } + if (!key) { + DEBUG(SSSDBG_OP_FAILURE, ("Could not get autofs entry key\n")); + return EINVAL; } - ret = sysdb_transaction_commit(sysdb); - if (ret != EOK) { - DEBUG(SSSDBG_CRIT_FAILURE, - ("Cannot commit sysdb transaction [%d]: %s\n", - ret, strerror(ret))); - goto done; + value = get_autofs_entry_value(entry, opts); + if (!value) { + DEBUG(SSSDBG_OP_FAILURE, ("Could not get autofs entry value\n")); + return EINVAL; } - in_transaction = false; - ret = EOK; -done: - if (in_transaction) { - tret = sysdb_transaction_cancel(sysdb); - if (tret != EOK) { - DEBUG(SSSDBG_CRIT_FAILURE, - ("Cannot cancel sysdb transaction [%d]: %s\n", - ret, strerror(ret))); - } - } - return ret; + return sysdb_save_autofsentry(sysdb, domain, map, key, value, NULL); } static errno_t @@ -178,25 +111,77 @@ save_autofs_entries(struct sysdb_ctx *sysdb, struct sss_domain_info *domain, struct sdap_options *opts, const char *map, - char **add_entries, - struct sysdb_attrs **entries, - size_t num_entries) + char **add_dn_list, + hash_table_t *entry_hash) { - return mod_autofs_entries(sysdb, domain, opts, map, add_entries, - entries, num_entries, AUTOFS_MAP_OP_ADD); + hash_key_t key; + hash_value_t value; + size_t i; + int hret; + errno_t ret; + struct sysdb_attrs *entry; + + if (!add_dn_list) { + return EOK; + } + + for (i=0; add_dn_list[i]; i++) { + key.type = HASH_KEY_STRING; + key.str = (char *) add_dn_list[i]; + + hret = hash_lookup(entry_hash, &key, &value); + if (hret != HASH_SUCCESS) { + DEBUG(SSSDBG_MINOR_FAILURE, + ("Cannot retrieve entry [%s] from hash\n", add_dn_list[i])); + continue; + } + + entry = talloc_get_type(value.ptr, struct sysdb_attrs); + if (!entry) { + DEBUG(SSSDBG_MINOR_FAILURE, + ("Cannot retrieve entry [%s] from ptr\n", add_dn_list[i])); + continue; + } + + DEBUG(SSSDBG_TRACE_FUNC, + ("Saving autofs entry [%s]\n", add_dn_list[i])); + ret = add_autofs_entry(sysdb, domain, map, opts, entry); + if (ret) { + DEBUG(SSSDBG_MINOR_FAILURE, + ("Cannot save entry [%s] to cache\n", add_dn_list[i])); + continue; + } + + DEBUG(SSSDBG_TRACE_FUNC, ("Saved entry [%s]\n", add_dn_list[i])); + } + + DEBUG(SSSDBG_TRACE_INTERNAL, ("All entries saved\n")); + return EOK; } static errno_t del_autofs_entries(struct sysdb_ctx *sysdb, - struct sss_domain_info *domain, struct sdap_options *opts, const char *map, - char **add_entries, - struct sysdb_attrs **entries, - size_t num_entries) + char **del_dn_list) { - return mod_autofs_entries(sysdb, domain, opts, map, add_entries, - entries, num_entries, AUTOFS_MAP_OP_DEL); + size_t i; + errno_t ret; + + for (i=0; del_dn_list[i]; i++) { + DEBUG(SSSDBG_TRACE_FUNC, + ("Removing autofs entry [%s]\n", del_dn_list[i])); + + ret = sysdb_del_autofsentry(sysdb, del_dn_list[i]); + if (ret) { + DEBUG(SSSDBG_MINOR_FAILURE, + ("Cannot delete entry %s\n", del_dn_list[i])); + continue; + } + } + + DEBUG(SSSDBG_TRACE_INTERNAL, ("All entries removed\n")); + return EOK; } static errno_t @@ -798,18 +783,25 @@ sdap_autofs_setautomntent_save(struct tevent_req *req) bool in_transaction = false; TALLOC_CTX *tmp_ctx; struct ldb_message **entries = NULL; - struct sysdb_attrs **entries_attrs; size_t count; + const char *key; const char *val; char **sysdb_entrylist; char **ldap_entrylist; char **add_entries; char **del_entries; - size_t i; + size_t i, j; + + hash_table_t *entry_hash; + hash_key_t hkey; + hash_value_t value; + int hret; tmp_ctx = talloc_new(NULL); if (!tmp_ctx) return ENOMEM; + DEBUG(SSSDBG_TRACE_LIBS, + ("Got %d map entries from LDAP\n", state->entries_count)); if (state->entries_count == 0) { /* No entries for this map in LDAP. * We need to ensure that there are no entries @@ -817,17 +809,51 @@ sdap_autofs_setautomntent_save(struct tevent_req *req) */ ldap_entrylist = NULL; } else { - ret = sysdb_attrs_to_list( - tmp_ctx, state->entries, - state->entries_count, - state->opts->autofs_entry_map[SDAP_AT_AUTOFS_ENTRY_KEY].sys_name, - &ldap_entrylist); - if (ret != EOK) { - DEBUG(SSSDBG_OP_FAILURE, - ("sysdb_attrs_primary_name_list failed [%d]: %s\n", - ret, strerror(ret))); + ldap_entrylist = talloc_array(tmp_ctx, char *, + state->entries_count+1); + if (!ldap_entrylist) { + ret = ENOMEM; goto done; } + + ret = sss_hash_create(state, 32, &entry_hash); + if (ret) { + goto done; + } + + /* Get a list of the map members by DN */ + for (i=0, j=0; i < state->entries_count; i++) { + key = get_autofs_entry_key(state->entries[i], state->opts); + val = get_autofs_entry_value(state->entries[i], state->opts); + if (!key || !val) { + DEBUG(SSSDBG_MINOR_FAILURE, ("Malformed entry, skipping\n")); + continue; + } + + ldap_entrylist[j] = sysdb_autofsentry_strdn(ldap_entrylist, + state->sysdb, + state->dom, + state->mapname, + key, val); + if (!ldap_entrylist[j]) { + ret = ENOMEM; + goto done; + } + + hkey.type = HASH_KEY_STRING; + hkey.str = ldap_entrylist[j]; + value.type = HASH_VALUE_PTR; + value.ptr = state->entries[i]; + + hret = hash_enter(entry_hash, &hkey, &value); + if (hret != HASH_SUCCESS) { + ret = EIO; + goto done; + } + + j++; + } + ldap_entrylist[state->entries_count] = NULL; } ret = sysdb_autofs_entries_by_map(tmp_ctx, state->sysdb, @@ -840,6 +866,7 @@ sdap_autofs_setautomntent_save(struct tevent_req *req) goto done; } + DEBUG(SSSDBG_TRACE_LIBS, ("Got %d map entries from sysdb\n", count)); if (count == 0) { /* No map members for this map in sysdb currently */ sysdb_entrylist = NULL; @@ -850,16 +877,10 @@ sdap_autofs_setautomntent_save(struct tevent_req *req) goto done; } - /* Get a list of the map members by name only */ + /* Get a list of the map members by DN */ for (i=0; i < count; i++) { - val = ldb_msg_find_attr_as_string(entries[i], - SYSDB_AUTOFS_ENTRY_KEY, NULL); - if (!val) { - DEBUG(SSSDBG_MINOR_FAILURE, ("An entry with no value?\n")); - continue; - } - - sysdb_entrylist[i] = talloc_strdup(sysdb_entrylist, val); + sysdb_entrylist[i] = talloc_strdup(sysdb_entrylist, + ldb_dn_get_linearized(entries[i]->dn)); if (!sysdb_entrylist[i]) { ret = ENOMEM; goto done; @@ -897,7 +918,7 @@ sdap_autofs_setautomntent_save(struct tevent_req *req) if (add_entries && add_entries[0]) { ret = save_autofs_entries(state->sysdb, state->dom, state->opts, state->mapname, add_entries, - state->entries, state->entries_count); + entry_hash); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, ("Cannot save autofs entries [%d]: %s\n", @@ -908,16 +929,8 @@ sdap_autofs_setautomntent_save(struct tevent_req *req) /* Delete entries that don't exist anymore */ if (del_entries && del_entries[0]) { - ret = sysdb_msg2attrs(tmp_ctx, count, entries, &entries_attrs); - if (ret != EOK) { - DEBUG(SSSDBG_OP_FAILURE, - ("sysdb_msg2attrs failed: [%d]: %s\n", ret, strerror(ret))); - goto done; - } - - ret = del_autofs_entries(state->sysdb, state->dom, state->opts, - state->mapname, del_entries, - entries_attrs, count); + ret = del_autofs_entries(state->sysdb, state->opts, + state->mapname, del_entries); if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, ("Cannot delete autofs entries [%d]: %s\n", -- 1.8.0.2
_______________________________________________ sssd-devel mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
