PR#51149: Ticket 49859 A distinguished value can be missing in an entry - 389-ds-base

389-ds-base

#51149 Ticket 49859 A distinguished value can be missing in an entry

Closed 3 years ago by spichugi. Opened 3 years ago by tbordaz.

tbordaz/389-ds-base ticket_49859 into master

Ticket 49859 - A distinguished value can be missing in an entry

Thierry Bordaz • 3 years ago

2ccd0be

dirsrvtests/tests/suites/replication/conflict_resolve_test.py

file modified

+173 -1

		`@@ -10,10 +10,11 @@`
		`import logging`
		`import ldap`
		`import pytest`
		`+ import re`
		`from itertools import permutations`
		`from lib389._constants import *`
		`from lib389.idm.nscontainer import nsContainers`
		`- from lib389.idm.user import UserAccounts`
		`+ from lib389.idm.user import UserAccounts, UserAccount`
		`from lib389.idm.group import Groups`
		`from lib389.idm.organizationalunit import OrganizationalUnits`
		`from lib389.replica import ReplicationManager`
		`@@ -763,6 +764,177 @@`
		`user_dns_m2 = [user.dn for user in test_users_m2.list()]`
		`assert set(user_dns_m1) == set(user_dns_m2)`

		`+ def test_conflict_attribute_multi_valued(self, topology_m2, base_m2):`
		`+ """A RDN attribute being multi-valued, checks that after several operations`
		`+ MODRDN and MOD_REPL its RDN values are the same on both servers`
		`+`
		`+ :id: 225b3522-8ed7-4256-96f9-5fab9b7044a5`
		`+ :setup: Two master replication,`
		`+ audit log, error log for replica and access log for internal`
		`+ :steps:`
		`+ 1. Create a test entry uid=user_test_1000,...`
		`+ 2. Pause all replication agreements`
		`+ 3. On M1 rename it into uid=foo1,...`
		`+ 4. On M2 rename it into uid=foo2,...`
		`+ 5. On M1 MOD_REPL uid:foo1`
		`+ 6. Resume all replication agreements`
		`+ 7. Check that entry on M1 has uid=foo1, foo2`
		`+ 8. Check that entry on M2 has uid=foo1, foo2`
		`+ 9. Check that entry on M1 and M2 has the same uid values`
		`+ :expectedresults:`
		`+ 1. It should pass`
		`+ 2. It should pass`
		`+ 3. It should pass`
		`+ 4. It should pass`
		`+ 5. It should pass`
		`+ 6. It should pass`
		`+ 7. It should pass`
		`+ 8. It should pass`
		`+ 9. It should pass`
		`+ """`
		`+`
		`+ M1 = topology_m2.ms["master1"]`
		`+ M2 = topology_m2.ms["master2"]`
		`+`
		`+ # add a test user`
		`+ test_users_m1 = UserAccounts(M1, base_m2.dn, rdn=None)`
		`+ user_1 = test_users_m1.create_test_user(uid=1000)`
		`+ test_users_m2 = UserAccount(M2, user_1.dn)`
		`+ # Waiting fo the user to be replicated`
		`+ for i in range(0,4):`
		`+ time.sleep(1)`
		`+ if test_users_m2.exists():`
		`+ break`
		`+ assert(test_users_m2.exists())`
		`+`
		`+ # Stop replication agreements`
		`+ topology_m2.pause_all_replicas()`
		`+`
		`+ # On M1 rename test entry in uid=foo1`
		`+ original_dn = user_1.dn`
		`+ user_1.rename('uid=foo1')`
		`+ time.sleep(1)`
		`+`
		`+ # On M2 rename test entry in uid=foo2`
		`+ M2.rename_s(original_dn, 'uid=foo2')`
		`+ time.sleep(2)`
		`+`
		`+ # on M1 MOD_REPL uid into foo1`
		`+ user_1.replace('uid', 'foo1')`
		`+`
		`+ # resume replication agreements`
		`+ topology_m2.resume_all_replicas()`
		`+ time.sleep(5)`
		`+`
		`+ # check that on M1, the entry 'uid' has two values 'foo1' and 'foo2'`
		`+ final_dn = re.sub('^.*1000,', 'uid=foo2,', original_dn)`
		`+ final_user_m1 = UserAccount(M1, final_dn)`
		`+ for val in final_user_m1.get_attr_vals_utf8('uid'):`
		`+ log.info("Check %s is on M1" % val)`
		`+ assert(val in ['foo1', 'foo2'])`
		`+`
		`+ # check that on M2, the entry 'uid' has two values 'foo1' and 'foo2'`
		`+ final_user_m2 = UserAccount(M2, final_dn)`
		`+ for val in final_user_m2.get_attr_vals_utf8('uid'):`
		`+ log.info("Check %s is on M1" % val)`
		`+ assert(val in ['foo1', 'foo2'])`
		`+`
		`+ # check that the entry have the same uid values`
		`+ for val in final_user_m1.get_attr_vals_utf8('uid'):`
		`+ log.info("Check M1.uid %s is also on M2" % val)`
		`+ assert(val in final_user_m2.get_attr_vals_utf8('uid'))`
		`+`
		`+ for val in final_user_m2.get_attr_vals_utf8('uid'):`
		`+ log.info("Check M2.uid %s is also on M1" % val)`
		`+ assert(val in final_user_m1.get_attr_vals_utf8('uid'))`
		`+`
		`+ def test_conflict_attribute_single_valued(self, topology_m2, base_m2):`
		`+ """A RDN attribute being signle-valued, checks that after several operations`
		`+ MODRDN and MOD_REPL its RDN values are the same on both servers`
		`+`
		`+ :id: c38ae613-5d1e-47cf-b051-c7284e64b817`
		`+ :setup: Two master replication, test container for entries, enable plugin logging,`
		`+ audit log, error log for replica and access log for internal`
		`+ :steps:`
		`+ 1. Create a test entry uid=user_test_1000,...`
		`+ 2. Pause all replication agreements`
		`+ 3. On M1 rename it into employeenumber=foo1,...`
		`+ 4. On M2 rename it into employeenumber=foo2,...`
		`+ 5. On M1 MOD_REPL employeenumber:foo1`
		`+ 6. Resume all replication agreements`
		`+ 7. Check that entry on M1 has employeenumber=foo1`
		`+ 8. Check that entry on M2 has employeenumber=foo1`
		`+ 9. Check that entry on M1 and M2 has the same employeenumber values`
		`+ :expectedresults:`
		`+ 1. It should pass`
		`+ 2. It should pass`
		`+ 3. It should pass`
		`+ 4. It should pass`
		`+ 5. It should pass`
		`+ 6. It should pass`
		`+ 7. It should pass`
		`+ 8. It should pass`
		`+ 9. It should pass`
		`+ """`
		`+`
		`+ M1 = topology_m2.ms["master1"]`
		`+ M2 = topology_m2.ms["master2"]`
		`+`
		`+ # add a test user with a dummy 'uid' extra value because modrdn removes`
		`+ # uid that conflict with 'account' objectclass`
		`+ test_users_m1 = UserAccounts(M1, base_m2.dn, rdn=None)`
		`+ user_1 = test_users_m1.create_test_user(uid=1000)`
		`+ user_1.add('objectclass', 'extensibleobject')`
		`+ user_1.add('uid', 'dummy')`
		`+ test_users_m2 = UserAccount(M2, user_1.dn)`
		`+`
		`+ # Waiting fo the user to be replicated`
		`+ for i in range(0,4):`
		`+ time.sleep(1)`
		`+ if test_users_m2.exists():`
		`+ break`
		`+ assert(test_users_m2.exists())`
		`+`
		`+ # Stop replication agreements`
		`+ topology_m2.pause_all_replicas()`
		`+`
		`+ # On M1 rename test entry in employeenumber=foo1`
		`+ original_dn = user_1.dn`
		`+ user_1.rename('employeenumber=foo1')`
		`+ time.sleep(1)`
		`+`
		`+ # On M2 rename test entry in employeenumber=foo2`
		`+ M2.rename_s(original_dn, 'employeenumber=foo2')`
		`+ time.sleep(2)`
		`+`
		`+ # on M1 MOD_REPL uid into foo1`
		`+ user_1.replace('employeenumber', 'foo1')`
		`+`
		`+ # resume replication agreements`
		`+ topology_m2.resume_all_replicas()`
		`+ time.sleep(5)`
		`+`
		`+ # check that on M1, the entry 'employeenumber' has value 'foo1'`
		`+ final_dn = re.sub('^.*1000,', 'employeenumber=foo2,', original_dn)`
		`+ final_user_m1 = UserAccount(M1, final_dn)`
		`+ for val in final_user_m1.get_attr_vals_utf8('employeenumber'):`
		`+ log.info("Check %s is on M1" % val)`
		`+ assert(val in ['foo1'])`
		`+`
		`+ # check that on M2, the entry 'employeenumber' has values 'foo1'`
		`+ final_user_m2 = UserAccount(M2, final_dn)`
		`+ for val in final_user_m2.get_attr_vals_utf8('employeenumber'):`
		`+ log.info("Check %s is on M2" % val)`
		`+ assert(val in ['foo1'])`
		`+`
		`+ # check that the entry have the same uid values`
		`+ for val in final_user_m1.get_attr_vals_utf8('employeenumber'):`
		`+ log.info("Check M1.uid %s is also on M2" % val)`
		`+ assert(val in final_user_m2.get_attr_vals_utf8('employeenumber'))`
		`+`
		`+ for val in final_user_m2.get_attr_vals_utf8('employeenumber'):`
		`+ log.info("Check M2.uid %s is also on M1" % val)`
		`+ assert(val in final_user_m1.get_attr_vals_utf8('employeenumber'))`

		`class TestThreeMasters:`
		`def test_nested_entries(self, topology_m3, base_m3):`

ldap/servers/slapd/back-ldbm/ldbm_modify.c

file modified

+136

		`@@ -213,6 +213,112 @@`
		`return retval;`
		`}`

		`+ int32_t`
		`+ entry_get_rdn_mods(Slapi_PBlock pb, Slapi_Entry entry, CSN csn, int repl_op, Slapi_Mods *smods_ret)`
		`+ {`
		`+ unsigned long op_type = SLAPI_OPERATION_NONE;`
		`+ char *new_rdn = NULL;`
		`+ char **dns = NULL;`
		`+ char **rdns = NULL;`
		`+ Slapi_Mods *smods = NULL;`
		`+ char *type = NULL;`
		`+ struct berval *bvp[2] = {0};`
		`+ struct berval bv;`
		`+ Slapi_Attr *attr = NULL;`
		`+ const char *entry_dn = NULL;`
		`+`
		`+ *smods_ret = NULL;`
		`+ entry_dn = slapi_entry_get_dn_const(entry);`
		`+ /* Do not bother to check that RDN is present, no one rename RUV or change its nsuniqueid */`
		`+ if (strcasestr(entry_dn, RUV_STORAGE_ENTRY_UNIQUEID)) {`
		`+ return 0;`
		`+ }`
		`+`
		`+ /* First get the RDNs of the operation */`
		`+ slapi_pblock_get(pb, SLAPI_OPERATION_TYPE, &op_type);`
		`+ switch (op_type) {`
		`+ case SLAPI_OPERATION_MODIFY:`
		`+ dns = slapi_ldap_explode_dn(entry_dn, 0);`
		`+ if (dns == NULL) {`
		`+ slapi_log_err(SLAPI_LOG_ERR, "entry_get_rdn_mods",`
		`+ "Fails to split DN \"%s\" into components\n", entry_dn);`
		`+ return -1;`
		`+ }`
		`+ rdns = slapi_ldap_explode_rdn(dns[0], 0);`
		`+ slapi_ldap_value_free(dns);`
		`+`
		`+ break;`
		`+ case SLAPI_OPERATION_MODRDN:`
		`+ slapi_pblock_get(pb, SLAPI_MODRDN_NEWRDN, &new_rdn);`
		`+ rdns = slapi_ldap_explode_rdn(new_rdn, 0);`
		`+ break;`
		`+ default:`
		`+ break;`
		`+ }`
		`+ if (rdns == NULL \|\| rdns[0] == NULL) {`
		`+ slapi_log_err(SLAPI_LOG_ERR, "entry_get_rdn_mods",`
		`+ "Fails to split RDN \"%s\" into components\n", slapi_entry_get_dn_const(entry));`
		`+ return -1;`
		`+ }`
		`+`
		`+ /* Update the entry to add RDNs values if they are missing */`
		`+ smods = slapi_mods_new();`
		`+`
		`+ bvp[0] = &bv;`
		`+ bvp[1] = NULL;`
		`+ for (size_t rdns_count = 0; rdns[rdns_count]; rdns_count++) {`
		`+ Slapi_Value *value;`
		`+ attr = NULL;`
		`+ slapi_rdn2typeval(rdns[rdns_count], &type, &bv);`
		`+`
		`+ /* Check if the RDN value exists */`
		`+ if ((slapi_entry_attr_find(entry, type, &attr) != 0) \|\|`
		`+ (slapi_attr_value_find(attr, &bv))) {`
		`+ const CSN *csn_rdn_add;`
		`+ const CSN *adcsn = attr_get_deletion_csn(attr);`
		`+`
		`+ /* It is missing => adds it */`
		`+ if (slapi_attr_flag_is_set(attr, SLAPI_ATTR_FLAG_SINGLE)) {`
		`+ if (csn_compare(adcsn, csn) >= 0) {`
		`+ /* this is a single valued attribute and the current value`
		`+ * (that is different from RDN value) is more recent than`
		`+ * the RDN value we want to apply.`
		`+ * Keep the current value and add a conflict flag`
		`+ */`
		`+`
		`+ type = ATTR_NSDS5_REPLCONFLICT;`
		`+ bv.bv_val = "RDN value may be missing because it is single-valued";`
		`+ bv.bv_len = strlen(bv.bv_val);`
		`+ slapi_entry_add_string(entry, type, bv.bv_val);`
		`+ slapi_mods_add_modbvps(smods, LDAP_MOD_ADD, type, bvp);`
		`+ continue;`
		`+ }`
		`+ }`
		`+ /* if a RDN value needs to be forced, make sure it csn is ahead */`
		`+ slapi_mods_add_modbvps(smods, LDAP_MOD_ADD, type, bvp);`
		`+ csn_rdn_add = csn_max(adcsn, csn);`
		`+`
		`+ if (entry_apply_mods_wsi(entry, smods, csn_rdn_add, repl_op)) {`
		`+ slapi_log_err(SLAPI_LOG_ERR, "entry_get_rdn_mods",`
		`+ "Fails to set \"%s\" in \"%s\"\n", type, slapi_entry_get_dn_const(entry));`
		`+ slapi_ldap_value_free(rdns);`
		`+ slapi_mods_free(&smods);`
		`+ return -1;`
		`+ }`
		`+ /* Make the RDN value a distinguished value */`
		`+ attr_value_find_wsi(attr, &bv, &value);`
		`+ value_update_csn(value, CSN_TYPE_VALUE_DISTINGUISHED, csn_rdn_add);`
		`+ }`
		`+ }`
		`+ slapi_ldap_value_free(rdns);`
		`+ if (smods->num_mods == 0) {`
		`+ /* smods_ret already NULL, just free the useless smods */`
		`+ slapi_mods_free(&smods);`
		`+ } else {`
		`+ *smods_ret = smods;`
		`+ }`
		`+ return 0;`
		`+ }`
		`/**`
		`Apply the mods to the ec entry. Check for syntax, schema problems.`
		`Check for abandon.`
		`@@ -269,6 +375,8 @@`
		`goto done;`
		`}`

		`+`
		`+`
		`/*`
		`* If the objectClass attribute type was modified in any way, expand`
		`* the objectClass values to reflect the inheritance hierarchy.`
		`@@ -414,6 +522,7 @@`
		`int result_sent = 0;`
		`int32_t parent_op = 0;`
		`struct timespec parent_time;`
		`+ Slapi_Mods *smods_add_rdn = NULL;`

		`slapi_pblock_get(pb, SLAPI_BACKEND, &be);`
		`slapi_pblock_get(pb, SLAPI_PLUGIN_PRIVATE, &li);`
		`@@ -731,6 +840,15 @@`
		`}`
		`} /* else if new_mod_count == mod_count then betxnpremod plugin did nothing */`

		`+ /* time to check if applying a replicated operation removed`
		`+ * the RDN value from the entry. Assuming that only replicated update`
		`+ * can lead to that bad result`
		`+ */`
		`+ if (entry_get_rdn_mods(pb, ec->ep_entry, opcsn, repl_op, &smods_add_rdn)) {`
		`+ goto error_return;`
		`+ }`
		`+`
		`+`
		`/*`
		`* Update the ID to Entry index.`
		`* Note that id2entry_add replaces the entry, so the Entry ID`
		`@@ -764,6 +882,23 @@`
		`MOD_SET_ERROR(ldap_result_code, LDAP_OPERATIONS_ERROR, retry_count);`
		`goto error_return;`
		`}`
		`+`
		`+ if (smods_add_rdn && slapi_mods_get_num_mods(smods_add_rdn) > 0) {`
		`+ retval = index_add_mods(be, (LDAPMod **) slapi_mods_get_ldapmods_byref(smods_add_rdn), e, ec, &txn);`
		`+ if (DB_LOCK_DEADLOCK == retval) {`
		`+ /* Abort and re-try */`
		`+ slapi_mods_free(&smods_add_rdn);`
		`+ continue;`
		`+ }`
		`+ if (retval != 0) {`
		`+ slapi_log_err(SLAPI_LOG_ERR, "ldbm_back_modify",`
		`+ "index_add_mods (rdn) failed, err=%d %s\n",`
		`+ retval, (msg = dblayer_strerror(retval)) ? msg : "");`
		`+ MOD_SET_ERROR(ldap_result_code, LDAP_OPERATIONS_ERROR, retry_count);`
		`+ slapi_mods_free(&smods_add_rdn);`
		`+ goto error_return;`
		`+ }`
		`+ }`
		`/*`
		`* Remove the old entry from the Virtual List View indexes.`
		`* Add the new entry to the Virtual List View indexes.`
		`@@ -978,6 +1113,7 @@`

		`common_return:`
		`slapi_mods_done(&smods);`
		`+ slapi_mods_free(&smods_add_rdn);`

		`if (inst) {`
		`if (ec_locked \|\| cache_is_in_cache(&inst->inst_cache, ec)) {`

ldap/servers/slapd/back-ldbm/ldbm_modrdn.c

file modified

+33 -4

		`@@ -21,7 +21,7 @@`
		`static int moddn_newrdn_mods(Slapi_PBlock pb, const char olddn, struct backentry ec, Slapi_Mods smods_wsi, int is_repl_op);`
		`static IDList moddn_get_children(back_txn ptxn, Slapi_PBlock pb, backend be, struct backentry parententry, Slapi_DN parentdn, struct backentry *child_entries, struct backdn *child_dns, int is_resurect_operation);`
		`static int moddn_rename_children(back_txn ptxn, Slapi_PBlock pb, backend be, IDList children, Slapi_DN dn_parentdn, Slapi_DN dn_newsuperiordn, struct backentry *child_entries[]);`
		`- static int modrdn_rename_entry_update_indexes(back_txn ptxn, Slapi_PBlock pb, struct ldbminfo li, struct backentry e, struct backentry *ec, Slapi_Mods smods1, Slapi_Mods smods2, Slapi_Mods smods3);`
		`+ static int modrdn_rename_entry_update_indexes(back_txn ptxn, Slapi_PBlock pb, struct ldbminfo li, struct backentry e, struct backentry *ec, Slapi_Mods smods1, Slapi_Mods smods2, Slapi_Mods smods3, Slapi_Mods *smods4);`
		`static void mods_remove_nsuniqueid(Slapi_Mods *smods);`

		`#define MOD_SET_ERROR(rc, error, count) \`
		`@@ -100,6 +100,7 @@`
		`Connection *pb_conn = NULL;`
		`int32_t parent_op = 0;`
		`struct timespec parent_time;`
		`+ Slapi_Mods *smods_add_rdn = NULL;`

		`if (slapi_pblock_get(pb, SLAPI_CONN_ID, &conn_id) < 0) {`
		`conn_id = 0; /* connection is NULL */`
		`@@ -842,6 +843,15 @@`
		`goto error_return;`
		`}`
		`}`
		`+`
		`+ /* time to check if applying a replicated operation removed`
		`+ * the RDN value from the entry. Assuming that only replicated update`
		`+ * can lead to that bad result`
		`+ */`
		`+ if (entry_get_rdn_mods(pb, ec->ep_entry, opcsn, is_replicated_operation, &smods_add_rdn)) {`
		`+ goto error_return;`
		`+ }`
		`+`
		`/* check that the entry still obeys the schema */`
		`if (slapi_entry_schema_check(pb, ec->ep_entry) != 0) {`
		`ldap_result_code = LDAP_OBJECT_CLASS_VIOLATION;`
		`@@ -1003,7 +1013,7 @@`
		`/*`
		`* Update the indexes for the entry.`
		`*/`
		`- retval = modrdn_rename_entry_update_indexes(&txn, pb, li, e, &ec, &smods_generated, &smods_generated_wsi, &smods_operation_wsi);`
		`+ retval = modrdn_rename_entry_update_indexes(&txn, pb, li, e, &ec, &smods_generated, &smods_generated_wsi, &smods_operation_wsi, smods_add_rdn);`
		`if (DB_LOCK_DEADLOCK == retval) {`
		`/* Retry txn */`
		`continue;`
		`@@ -1497,6 +1507,7 @@`
		`slapi_mods_done(&smods_operation_wsi);`
		`slapi_mods_done(&smods_generated);`
		`slapi_mods_done(&smods_generated_wsi);`
		`+ slapi_mods_free(&smods_add_rdn);`
		`slapi_ch_free((void **)&child_entries);`
		`slapi_ch_free((void **)&child_dns);`
		`if (ldap_result_matcheddn && 0 != strcmp(ldap_result_matcheddn, "NULL"))`
		`@@ -1778,7 +1789,7 @@`
		`* mods contains the list of attribute change made.`
		`*/`
		`static int`
		`- modrdn_rename_entry_update_indexes(back_txn ptxn, Slapi_PBlock pb, struct ldbminfo li __attribute__((unused)), struct backentry e, struct backentry *ec, Slapi_Mods smods1, Slapi_Mods smods2, Slapi_Mods smods3)`
		`+ modrdn_rename_entry_update_indexes(back_txn ptxn, Slapi_PBlock pb, struct ldbminfo li __attribute__((unused)), struct backentry e, struct backentry *ec, Slapi_Mods smods1, Slapi_Mods smods2, Slapi_Mods smods3, Slapi_Mods *smods4)`
		`{`
		`backend *be;`
		`ldbm_instance *inst;`
		`@@ -1874,6 +1885,24 @@`
		`goto error_return;`
		`}`
		`}`
		`+ if (smods4 != NULL && slapi_mods_get_num_mods(smods4) > 0) {`
		`+ /*`
		`+ * update the indexes: lastmod, rdn, etc.`
		`+ */`
		`+ retval = index_add_mods(be, slapi_mods_get_ldapmods_byref(smods4), e, *ec, ptxn);`
		`+ if (DB_LOCK_DEADLOCK == retval) {`
		`+ /* Retry txn */`
		`+ slapi_log_err(SLAPI_LOG_BACKLDBM, "modrdn_rename_entry_update_indexes",`
		`+ "index_add_mods4 deadlock\n");`
		`+ goto error_return;`
		`+ }`
		`+ if (retval != 0) {`
		`+ slapi_log_err(SLAPI_LOG_TRACE, "modrdn_rename_entry_update_indexes",`
		`+ "index_add_mods 4 failed, err=%d %s\n",`
		`+ retval, (msg = dblayer_strerror(retval)) ? msg : "");`
		`+ goto error_return;`
		`+ }`
		`+ }`
		`/*`
		`* Remove the old entry from the Virtual List View indexes.`
		`* Add the new entry to the Virtual List View indexes.`
		`@@ -1991,7 +2020,7 @@`
		`* Update all the indexes.`
		`*/`
		`retval = modrdn_rename_entry_update_indexes(ptxn, pb, li, e, ec,`
		`- smodsp, NULL, NULL);`
		`+ smodsp, NULL, NULL, NULL);`
		`/* JCMREPL - Should the children get updated modifiersname and lastmodifiedtime? */`
		`slapi_mods_done(&smods);`
		`}`

ldap/servers/slapd/back-ldbm/proto-back-ldbm.h

file modified

		`@@ -323,6 +323,7 @@`
		`/*`
		`* modify.c`
		`*/`
		`+ int32_t entry_get_rdn_mods(Slapi_PBlock pb, Slapi_Entry entry, CSN csn, int repl_op, Slapi_Mods *smods_ret);`
		`int modify_update_all(backend be, Slapi_PBlock pb, modify_context mc, back_txn txn);`
		`void modify_init(modify_context mc, struct backentry old_entry);`
		`int modify_apply_mods(modify_context mc, Slapi_Mods smods);`

tbordaz commented 3 years ago

Bug description:
According to RFC 4511 (see ticket), the values of the RDN attributes
should be present in an entry.
With a set of replicated operations, it is possible that those values
would be missing

Fix description:
MOD and MODRDN update checks that the RDN values are presents.
If they are missing they are added to the resulting entry. In addition
the set of modifications to add those values are also indexed.
The specific case of single-valued attributes, where the final and unique value
can not be the RDN value, the attribute nsds5ReplConflict is added.

https://pagure.io/389-ds-base/issue/49859

Reviewed by: ?

Platforms tested: F31

mreynolds commented 3 years ago

I noticed that entry_get_rdn_mods() returns a result code, but you never check it. Is that intentional?

mreynolds commented on line 46 of dirsrvtests/tests/suites/replication/conflict_resolve_test.py 3 years ago

You should not be using python-ldap "_s" functions any more. You just need to create a second UserAccount for M2 with the same DN, then you don't need to call search_s() or rename_s()

firstyear commented on line 81 of dirsrvtests/tests/suites/replication/conflict_resolve_test.py 3 years ago

Similar here, you should be able to do a UserAccounts.get(...) and check the attrs rather than ent.getValues.

firstyear commented on line 8 of ldap/servers/slapd/back-ldbm/ldbm_modify.c 3 years ago

good hygiene to always 0 ints, NULL pointers, and for the struct bv, to do {0} which is c99 syntax to zero the struct on the stack.

firstyear commented on line 59 of ldap/servers/slapd/back-ldbm/ldbm_modify.c 3 years ago

rdns_count should be size_t since it offsets into an array. You can also declare it in the for loop (again, c99).

rebased onto b985ae7e3664fc6f77b6ab35ff137e6a98faa2b2

3 years ago

tbordaz commented 3 years ago

Thanks for the reviews. I updated the patch.

mreynolds commented 3 years ago

Thanks Thierry, LGTM!

rebased onto 2ccd0be

3 years ago

Pull-Request has been merged by tbordaz

3 years ago

spichugi commented 3 years ago

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This pull request has been cloned to Github as issue and is available here:
- https://github.com/389ds/389-ds-base/issues/4202

If you want to continue to work on the PR, please navigate to the github issue,
download the patch from the attachments and file a new pull request.

Thank you for understanding. We apologize for all inconvenience.

Pull-Request has been closed by spichugi

3 years ago

Metadata

Assignee

None

Tags

No Tags

Changes Summary 4

+173 -1

file changed

dirsrvtests/tests/suites/replication/conflict_resolve_test.py

+136 -0

file changed

ldap/servers/slapd/back-ldbm/ldbm_modify.c

+33 -4

file changed

ldap/servers/slapd/back-ldbm/ldbm_modrdn.c

+1 -0

file changed

ldap/servers/slapd/back-ldbm/proto-back-ldbm.h

389-ds-base

Source Code

#51149 Ticket 49859 A distinguished value can be missing in an entry Closed 3 years ago by spichugi. Opened 3 years ago by tbordaz. tbordaz/389-ds-base ticket_49859 into master

Metadata

Changes Summary 4

#51149 Ticket 49859 A distinguished value can be missing in an entry

Closed 3 years ago by spichugi. Opened 3 years ago by tbordaz.

tbordaz/389-ds-base ticket_49859 into master