Bug 49728 - Renaming DNS host record does not delete old object in Samba/AD
Renaming DNS host record does not delete old object in Samba/AD
Status: CLOSED FIXED
Product: UCS
Classification: Unclassified
Component: S4 Connector
UCS 4.4
Other Linux
: P5 normal (vote)
: UCS 4.4-2-errata
Assigned To: Julia Bremer
Florian Best
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2019-06-26 10:31 CEST by Christian Völker
Modified: 2019-11-06 14:40 CET (History)
7 users (show)

See Also:
What kind of report is it?: Bug Report
What type of bug is this?: 5: Major Usability: Impairs usability in key scenarios
Who will be affected by this bug?: 3: Will affect average number of installed domains
How will those affected feel about the bug?: 2: A Pain – users won’t like this once they notice it
User Pain: 0.171
Enterprise Customer affected?: Yes
School Customer affected?:
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number: 2019062421000558, 2019092621000503, 2019100221000742, 2019102421000782
Bug group (optional):
Max CVSS v3 score:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Christian Völker univentionstaff 2019-06-26 10:31:48 CEST
Steps to reproduce:
Have an UCS server with dns/backend set to samba4.
Create an IP-managed client through UMC (Devices -> Computer). Save.
Open this object again and rename it to a different name.

The object still exists in Samba, but not in LDAP. It is only removeable with ldbdel.
Result:
# univention-ldapsearch| grep "old-name"
<NO RESULT>

# univention-s4search --cross-ncs "dc=old-name"
# record 1
dn: DC=old-name,DC=multi.de,CN=MicrosoftDNS,DC=DomainDnsZones,DC=multi,DC=de
objectClass: top
objectClass: dnsNode
instanceType: 4
whenCreated: 20190626082353.0Z
whenChanged: 20190626082353.0Z
uSNCreated: 7759
uSNChanged: 7759
showInAdvancedViewOnly: TRUE
name: old-name
objectGUID: 31fa6571-c0d1-49e7-a5b7-6d6e807b9cba
dnsRecord:: BAABAAXwAAABAAAAAAADhAAAAAAAAAAAwKgJDQ==
objectCategory: CN=Dns-Node,CN=Schema,CN=Configuration,DC=multi,DC=de
dc: old-name
distinguishedName: DC=old-name,DC=multi.de,CN=MicrosoftDNS,DC=DomainDnsZones,D
 C=multi,DC=de

# returned 1 records
# 1 entries
# 0 referrals
Comment 1 Arvid Requate univentionstaff 2019-06-26 13:29:37 CEST
This looks like a general issue of DNS host_record synchronization:

udm dns/host_record modify --dn relativeDomainName=foo,zoneName=ar41i1.qa,cn=dns,dc=ar41i1,dc=qa --set name=foo2
Comment 2 Christian Völker univentionstaff 2019-10-02 11:56:32 CEST
Another customer affected
Comment 3 Christian Völker univentionstaff 2019-10-02 15:15:12 CEST
Raising priority as the still existing entry obviously does not update the IP address in case the IP address of the renamed computer changes. This results in timeout errors when using the old name (which should instead immediately bring an error NXDOMAIN).
Comment 4 Benjamin Fels univentionstaff 2019-10-04 12:00:00 CEST
Key customer affected. Got the feedback that this probably affects further customers.
Comment 6 Florian Best univentionstaff 2019-10-22 14:39:00 CEST
@Julia: Is this related to Bug #49874? The patch seems at least to fix that as well.
Comment 7 Julia Bremer univentionstaff 2019-10-22 15:15:03 CEST
(In reply to Florian Best from comment #6)
> @Julia: Is this related to Bug #49874? The patch seems at least to fix that
> as well.

Not directly, the traceback from #49874 just broke my new test, which is why I fixed that too.
Comment 8 Florian Best univentionstaff 2019-10-22 22:23:42 CEST
(In reply to Julia Bremer from comment #7)
> (In reply to Florian Best from comment #6)
> > @Julia: Is this related to Bug #49874? The patch seems at least to fix that
> > as well.
> 
> Not directly, the traceback from #49874 just broke my new test, which is why
> I fixed that too.
So the changes would work without Bug #49874 but the test case would fail?!
Hm, I think in this case we should explicitly fix Bug #49874 instead.
Comment 9 Florian Best univentionstaff 2019-10-23 18:42:00 CEST
Julia and I further analyzed the code:
self.sync_from_ucs() is called where object is `mapped_object = self._object_mapping(key, object, 'ucs')`.
_object_mapping() already calls dn_mapping_function() (and self._subtree_replace()).
Therefore object['olddn'] contains a DN from the sqlite cache (get_dn_by_ucs()) which is the correctly mapped olddn of the S4-object.

So I guess we can just use this and remove the function calls for the detection of a move operation:

diff --git services/univention-s4-connector/modules/univention/s4connector/s4/__init__.py services/univention-s4-connector/modules/univention/s4connector/s4/__init__.py
index 1a78fa437f..e744e1fe43 100644
--- services/univention-s4-connector/modules/univention/s4connector/s4/__init__.py
+++ services/univention-s4-connector/modules/univention/s4connector/s4/__init__.py
@@ -2304,16 +2304,7 @@ class s4(univention.s4connector.ucs):
                # check for move, if old_object exists, set modtype move
                pre_mapped_ucs_old_dn = old_dn
                if old_dn:
-                       if hasattr(self.property[property_type], 'dn_mapping_function'):
-                               tmp_object = copy.deepcopy(object)
-                               tmp_object['dn'] = old_dn
-                               for function in self.property[property_type].dn_mapping_function:
-                                       tmp_object = function(self, tmp_object, [], isUCSobject=True)
-                               old_dn = tmp_object['dn']
-                       if hasattr(self.property[property_type], 'position_mapping'):
-                               for mapping in self.property[property_type].position_mapping:
-                                       old_dn = self._subtree_replace(old_dn.lower(), mapping[1].lower(), mapping[0].lower())
-                               old_dn = self._subtree_replace(old_dn, self.lo.base, self.lo_s4.base)
+                       old_dn = object['olddn']
                        # the old object was moved in UCS, but does this object exist in S4?
                        try:
                                old_object = self.s4_search_ext_s(compatible_modstring(old_dn), ldap.SCOPE_BASE, 'objectClass=*')

In a quick test this works. A branch test is running: https://jenkins.knut.univention.de:8181/job/UCS-4.4/job/UCS-4.4-2/view/Product%20Tests/job/product-test-samba-s4-connector/8/console
Unfortionately I cannot see the beyond the CVS history of why this second mapping was introduced.
Comment 10 Julia Bremer univentionstaff 2019-10-28 18:23:31 CET
8745a195a3 Bug #49728: yaml
1034645bbd Bug #49728: Version bump
940adbe79c Bug #49728: Added testcase move_dns_host_record
7eb78b60ed Bug #49728: Fix newdn instead of olddn is mapped in case of move

Package: univention-s4-connector
Version: 13.0.2-56A~4.4.0.201910281739
Branch: ucs_4.4-0
Scope: errata4.4-2
Comment 11 Florian Best univentionstaff 2019-10-30 12:50:21 CET
OK: dns/host_record
OK: dns/txt_record
OK: dns/srv_record
OK: dns/alias
OK: dns/ns_record
OK: dns/ptr_record (except if I set adress to something non-digital)
OK: ucs-test case (could include all types, could be written in python)
OK: works if I remove the object from the sqlite tables 'DN Mapping UCS', 'DN Mapping CON'
~OK: YAML (I adapted the wording a little bit)
Comment 12 Arvid Requate univentionstaff 2019-11-06 14:40:55 CET
<http://errata.software-univention.de/ucs/4.4/330.html>