Bug 33594 - ldapmodrdn without "-r" leads to inconsistency between master and slave
ldapmodrdn without "-r" leads to inconsistency between master and slave
Status: CLOSED FIXED
Product: UCS
Classification: Unclassified
Component: Listener (univention-directory-listener)
UCS 3.2
Other Linux
: P5 normal (vote)
: UCS 4.1-2-errata
Assigned To: Philipp Hahn
Arvid Requate
:
Depends on:
Blocks: 34802 34971
  Show dependency treegraph
 
Reported: 2013-11-27 14:39 CET by Janek Walkenhorst
Modified: 2016-07-21 15:16 CEST (History)
2 users (show)

See Also:
What kind of report is it?: ---
What type of bug is this?: ---
Who will be affected by this bug?: ---
How will those affected feel about the bug?: ---
User Pain:
Enterprise Customer affected?:
School Customer affected?:
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number:
Bug group (optional):
Max CVSS v3 score:


Attachments
test-case: modrdn -> failed-LDIF (406 bytes, text/plain)
2014-04-16 18:36 CEST, Philipp Hahn
Details
modrdn_proposal.patch (2.38 KB, patch)
2016-07-14 21:46 CEST, Arvid Requate
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Janek Walkenhorst univentionstaff 2013-11-27 14:39:27 CET
Changing
  dn: uid=fred,cn=users,dc=schema,dc=test
  uid: fred
with ldapmodrdn (without "-r") uid=fred → uid=winifred leads to
  dn: uid=winifred,cn=users,dc=schema,dc=test
  uid: fred
  uid: winifred
on the master but it is replicated as
  dn: uid=winifred,cn=users,dc=schema,dc=test
  uid: winifred
thus creating inconsistency between the master
  master # id fred
  uid=2008(fred) gid=5001(Domain Users) Gruppen=5001(Domain Users)
  master # id winifred
  uid=2008(fred) gid=5001(Domain Users) Gruppen=5001(Domain Users)
and the slaves
  slave # id fred
  id: fred: No such user
  slave # id winifred
  uid=2008(winifred) gid=5001(Domain Users) Gruppen=5001(Domain Users)
.
Comment 1 Philipp Hahn univentionstaff 2014-04-10 18:36:31 CEST
This is a bug in replication.py:
1011 »···»···»···»···»···»···l.rename_s(old_dn, new_rdn, new_parent)

The code here assumes that during a modrdn operation only the dn is changed, but calling that function also causes the LDAP server to update the RDN.
As delold=1 is the default for <http://www.python-ldap.org/doc/html/ldap.html#ldap.LDAPObject.rename_s>, this causes the old RDN being removed, while on the Master "ldapmodrdn -r" does NOT remove the old RDN, but appends the new RDN to the attribute.

Using plain UDM commands this doesn't happen, as univention.uldap explicitly set oldrdn=0:
493 »···»···»···self.lo.rename_s(dn, rename, None, delold=0)

In both cases the listener only gets the two r/a notifications and fetches the new state from the remote server. What's missing here is that this new state is still applied after the rename_s() to get all other pending changes to be applied.

As an optimization the replication handler could also parse the RDN itself and compare it with the new state to decide, if delold was True or False, and do the same. (but probably isn't worth the code in comparison to just always doing a modify afterwards.)
The reverse issue arises in the Listener with Bug #34355 with a modrdn after an modify, where the modify will make the future modrdn visible to listener modules.
Comment 2 Philipp Hahn univentionstaff 2014-04-16 18:36:45 CEST
Created attachment 5876 [details]
test-case: modrdn -> failed-LDIF

The attached test case creates a "person" "cn=test" with MUST attributes (cn, sn). It's renamed to "sn=test". With the default oldrdn=1 as used by replication.py the LDAP C library tries to remove the required MUST attribute "cn", which fails:

updating cn=test,cn=visible,dc=phahn,dc=dev
delete handlers for cn=test,cn=visible,dc=phahn,dc=dev
replication: Running handler for: cn=test,cn=visible,dc=phahn,dc=dev
replication: rename phase I: cn=test,cn=visible,dc=phahn,dc=dev (entryUUID=eb5b3908-59c7-1033-8cc6-6d1c11cdc722)
handler: replication (successful)
deleted from cache: cn=test,cn=visible,dc=phahn,dc=dev
write to transaction file dn=[cn=test,cn=visible,dc=phahn,dc=dev], command=[r]
notifier returned = id: 24790  dn: sn=test,cn=visible,dc=phahn,dc=dev  cmd: a
updating sn=test,cn=visible,dc=phahn,dc=dev
...
replication: Running handler for: sn=test,cn=visible,dc=phahn,dc=dev
replication: rename phase II: sn=test,cn=visible,dc=phahn,dc=dev (entryUUID=eb5b3908-59c7-1033-8cc6-6d1c11cdc722)
replication: rename from cn=test,cn=visible,dc=phahn,dc=dev to sn=test,cn=visible,dc=phahn,dc=dev
dn=sn=test,cn=visible,dc=phahn,dc=dev: Object class violation
       additional info: object class 'person' requires attribute 'cn'
replication: Running handler for: sn=test,cn=visible,dc=phahn,dc=dev
replication: rename phase II: sn=test,cn=visible,dc=phahn,dc=dev (entryUUID=eb5b3908-59c7-1033-8cc6-6d1c11cdc722)
handler: replication (successful)
...
Could not write to transaction file /var/lib/univention-ldap/listener/listener. Check for /var/lib/univention-directory-replication/failed.ldif
Comment 3 Philipp Hahn univentionstaff 2016-06-06 11:50:03 CEST
r69836 | Bug #33594 repl: Fix modrdn.delold handling

Package: univention-directory-replication
Version: 9.0.1-3.104.201606061133
Branch: ucs_4.1-0
Scope: errata4.1-2

r69841 | Bug #33594,Bug #41347,Bug #28232,Bug #30489,Bug #31757: univention-directory-logger YAML
 univention-directory-replication.yaml
Comment 4 Arvid Requate univentionstaff 2016-07-14 21:46:07 CEST
The current patch doesn't fix Comment 2:

I tested with udl/doc.34355/test20_rename_rdn (without directly removing the test object):

=============================================================================
23.11.15 20:16:28.376  LISTENER    ( WARN    ) : replication: Can't contact LDAP server: retrying
23.11.15 20:16:28.376  LISTENER    ( WARN    ) :        machted dn: dc=ar41i1,dc=qa
23.11.15 20:17:37.268  LISTENER    ( PROCESS ) : replication: rename phase I: cn=test,cn=visible,dc=ar41i1,dc=qa (entryUUID=940de4d4-2662-1035-8200-c361c1f0c6e7)
23.11.15 20:17:37.269  LISTENER    ( PROCESS ) : replication: rename phase II: sn=test,cn=visible,dc=ar41i1,dc=qa (entryUUID=940de4d4-2662-1035-8200-c361c1f0c6e7)
23.11.15 20:17:37.270  LISTENER    ( PROCESS ) : replication: rename from cn=test,cn=visible,dc=ar41i1,dc=qa to sn=test,cn=visible,dc=ar41i1,dc=qa
23.11.15 20:17:37.270  LISTENER    ( ERROR   ) : replication: Object class violation; dn="sn=test,cn=visible,dc=ar41i1,dc=qa": Error
23.11.15 20:17:37.270  LISTENER    ( ERROR   ) :        additional info: object class 'person' requires attribute 'cn'
23.11.15 20:17:37.271  LISTENER    ( PROCESS ) : replication: rename phase II: sn=test,cn=visible,dc=ar41i1,dc=qa (entryUUID=940de4d4-2662-1035-8200-c361c1f0c6e7)
23.11.15 20:17:37.293  LISTENER    ( ERROR   ) : Could not write to transaction file /var/lib/univention-ldap/listener/listener. Check for /var/lib/univention-directory-replication/failed.ldif
23.11.15 20:17:37.293  LISTENER    ( ERROR   ) : listener: -1
=============================================================================

Also, after rename phase II failed, replication.py now removes the modrdn_cache and the link to it. So the "rename" information is lost and failed.ldif only contains the "add". I'm not sure, maybe it's important to keep it in that case?

I'll attach a patch proposal for both issues.
Comment 5 Arvid Requate univentionstaff 2016-07-14 21:46:33 CEST
Created attachment 7804 [details]
modrdn_proposal.patch
Comment 6 Philipp Hahn univentionstaff 2016-07-18 17:52:27 CEST
(In reply to Arvid Requate from comment #5)
> Created attachment 7804 [details]
> modrdn_proposal.patch

Thanks, applied without the last debug hunk:
r71074 | Bug #33594 repl: Handle ldap_rename(delold)

Package: univention-directory-replication
Version: 9.0.1-4.108.201607181744
Branch: ucs_4.1-0
Scope: errata4.1-2

r71076 | Bug #31757,Bug #33594 repl: YAML
 univention-directory-replication.yaml
Comment 7 Arvid Requate univentionstaff 2016-07-18 18:12:59 CEST
Code review: Ok
Functional tests: Ok
Advisory: Ok
Comment 8 Janek Walkenhorst univentionstaff 2016-07-21 15:16:01 CEST
<http://errata.software-univention.de/ucs/4.1/216.html>