Univention Bugzilla – Bug 33621
Concurrent changes in S4 and OpenLDAP
Last modified: 2020-05-19 15:14:08 CEST
We have some reports about concurrent changes, see Bug #18501 for the AD Connector issue. (In reply to Ingo Steuwer from Bug #18501 comment #12) > reported by: 2013081521023638 > > S4-connector deletes a group that was deleted and recreated by UDM A similar issue is Bug #32263. I think we are unable to solve all issues, for example if the connector is stopped and on both sides the description of a user is changed to different values. Only one side can win. But we should avoid the re-creation or re-deletion of objects.
If we will use a time stamp based sync we have to check the group member sync separately because we are using for the group members a diff based sync. And we can not change to a time stamp sync for members because this would break basic use cases.
I've added two test cases to demonstrate this issue: - 52_s4connector/020_concurrent_changes_in_ucs - 52_s4connector/021_concurrent_changes_in_ad
*** Bug 34205 has been marked as a duplicate of this bug. ***
During QA for Bug #34267 it happened multiple times, that after a ucs-test run the test-group still existed, as it got re-added by the S4 connector after it had already been cleaned-up at the end of the test. Same happened for modifying the user, when an added email-address was removed again by the S4 connector.
(In reply to Philipp Hahn from comment #4) > During QA for Bug #34267 it happened multiple times, that after a ucs-test > run the test-group still existed, as it got re-added by the S4 connector I've created two test cases for this issue: * 52_s4connector/022_concurrent_deletion_in_ucs * 52_s4connector/023_concurrent_deletion_in_ad
(In reply to Stefan Gohmann from comment #5) > (In reply to Philipp Hahn from comment #4) > > During QA for Bug #34267 it happened multiple times, that after a ucs-test > > run the test-group still existed, as it got re-added by the S4 connector > > I've created two test cases for this issue: > * 52_s4connector/022_concurrent_deletion_in_ucs > * 52_s4connector/023_concurrent_deletion_in_ad This is Bug #32263.
The defined attributes are now synced in diff mode. The password and the group membership is currently synced via separate sync functions. I've added a check in the password sync functions, the password are now only synchronized if one password attribute was changed. The membership function is currently untouched. Test cases: 52_s4connector/021_concurrent_changes_in_ad 52_s4connector/020_concurrent_changes_in_ucs 52_s4connector/120sync_create_and_modify_ucs_user 52_s4connector/124sync_modify_con_other_attributes Code: r48654, r48656, r49219, r49225, r49466, r49467, r49468 YAML: r49473
This bug will be moved to 3.2-2-errata.
Created attachment 5893 [details] bug_33621-32263.patch This patch was reverted in order to get the package ready for 3.2-1-errata.
Created attachment 5894 [details] bug_33621-32263_conffiles.patch Another patch
Package version 8.0.33-47.473.201405151521 seems to break objectSid replication from Samba4 to OpenLDAP. After successfully joining a Windows client (not pre-created) the situation is as follows. Manual modification of the description on the Samba4 object also doesn't cause the sid to be synchronized. Machine snapshot is available. # WIN7PRO231, computers, ar320i1.qa dn: cn=WIN7PRO231,cn=computers,dc=ar320i1,dc=qa sambaSID: S-1-4-2010 description: foo2 root@master40:~# univention-s4search cn=win7pro231 objectsid description # record 1 dn: CN=WIN7PRO231,CN=Computers,DC=ar320i1,DC=qa objectSid: S-1-5-21-2504708665-2173701359-1147132429-1602 description: foo2
(In reply to Arvid Requate from comment #11) > Package version 8.0.33-47.473.201405151521 seems to break objectSid > replication from Samba4 to OpenLDAP. After successfully joining a Windows > client (not pre-created) the situation is as follows. Manual modification of > the description on the Samba4 object also doesn't cause the sid to be > synchronized. Machine snapshot is available. Yes, the change of the description does no longer tigger the synchronization of the other attributes. I'm unable to reproduce the SID difference. Could you append the connector-s4.log with debug level 4?
That could be a similar case: http://jenkins.knut.univention.de:8080/view/Autotest/job/UCS%203.2-2%20Autotest%20MultiEnv/SambaVersion=s4,Systemrolle=backup/28/testReport/50_samba/30winbind/test/
The following things need to be done: * AD Takeover tests In a short test the user krbtgt was rejected. * Re-sync one object Previously it was possible to sync the whole object with the change of the description. Now it should be added a script which removes the object from the cache.
Another issue: * We don't check the values of the attributes. If I change the same attribute in a loop, the value is overwritten. The test cases 020_concurrent_changes_in_ucs_same_attribute and 021_concurrent_changes_in_ad_same_attribute demonstrate this case.
(In reply to Stefan Gohmann from comment #15) > Another issue: > > * We don't check the values of the attributes. If I change the same > attribute in a loop, the value is overwritten. The test cases > 020_concurrent_changes_in_ucs_same_attribute and > 021_concurrent_changes_in_ad_same_attribute demonstrate this case. This issue was separated to Bug #35336.
I've added some new helper tools for debugging this changes: - DN2base64Guid.py Converts the objectGUID of an S4 object to base64. This is helpful for debugging the /etc/univention/connector/s4cache.sqlite values. - base64Guid2DN.py Decodes the base64 based objectGUID and returns the S4 DN. This is helpful for debugging the /etc/univention/connector/s4cache.sqlite values. - resync_object_from_s4.py Removes all cache entries of an S4 object and resyncs this object from S4 to OpenLDAP. - resync_object_from_ucs.py Removes all cache entries of an UCS object and resyncs this object from OpenLDAP to UCS.
(In reply to Stefan Gohmann from comment #14) > The following things need to be done: > > * AD Takeover tests > In a short test the user krbtgt was rejected. I wasn't able to reproduce this. The rejected object was synced in the next turn. The traceback is already known: Bug #33263 > * Re-sync one object > Previously it was possible to sync the whole object with the change of the > description. Now it should be added a script which removes the object from > the cache. Done
As discussed we need one more thing. After the creation of an object in S4, the object should be searched in S4 and dumped to the cache.
(In reply to Stefan Gohmann from comment #19) > As discussed we need one more thing. After the creation of an object in S4, > the object should be searched in S4 and dumped to the cache. Fixed with r51885
(In reply to Stefan Gohmann from comment #20) > (In reply to Stefan Gohmann from comment #19) > > As discussed we need one more thing. After the creation of an object in S4, > > the object should be searched in S4 and dumped to the cache. > > Fixed with r51885 I've reverted the commit because the jenkins tests failed. I've created a new bug for this: Bug #35391
Created attachment 6011 [details] s4cache_EntryDiff.patch The attribute list needs to be sorted for comparison. Test case: run s4cache.py twice.
Created attachment 6012 [details] objectGUID.patch objectGUID is not unicode but a binary NDR format which should be converted to a string via ndr_unpack. I have no clue why the S4/AD Connector wraps all attributes to unicode (__object_from_element) when reading from S4, seems like legacy code, but we should not change that. I see that the "S4 GUID" table is written in base64, but using this patch we can avoid that for the s4cache database: improves readability and simplifies the s4cache code.
Created attachment 6013 [details] sync_to_ucs_call.patch Looks like one of the sync_to_ucs calls still passes only three arguments.
(In reply to Arvid Requate from comment #24) > Created attachment 6013 [details] > sync_to_ucs_call.patch > > Looks like one of the sync_to_ucs calls still passes only three arguments. Yes, applied: r52020 (In reply to Arvid Requate from comment #22) > Created attachment 6011 [details] > s4cache_EntryDiff.patch > > The attribute list needs to be sorted for comparison. > Test case: run s4cache.py twice. I use now a set() which should be faster. (In reply to Arvid Requate from comment #23) > Created attachment 6012 [details] > objectGUID.patch > > objectGUID is not unicode but a binary NDR format which should be converted > to a string via ndr_unpack. > > I have no clue why the S4/AD Connector wraps all attributes to unicode > (__object_from_element) when reading from S4, seems like legacy code, but we > should not change that. > > I see that the "S4 GUID" table is written in base64, but using this patch we > can avoid that for the s4cache database: improves readability and simplifies > the s4cache code. OK, applied with some changes: r52022
Ok, verified: * Code review * ucs-test * Jenkins seems to be ok too. * Advisory: Ok We still need to decide upon the release date. If it's released before the ad-member enhancements for shell-univention-lib then we need to revert the univention-s4-connector change made for version 8.0.33-63 and possibly later.
* change made for version 8.0.33-63 has been reverted now for release. * Built version updated in advisory. Just for future reference: There is a new sqlite database and a new table now: root@master40:~# sqlite3 /etc/univention/connector/s4cache.sqlite "select * from GUIDS;" 1|d1038bd5-b248-4bb5-b75d-8158ca731ac9 root@master40:~# sqlite3 /etc/univention/connector/s4internal.sqlite "select * from 'UCS Deleted';" 3cbb6838-0be2-1033-9a35-0d1639c0bdaf|e003cdd0-8023-4709-95d4-36be33390736
http://errata.univention.de/ucs/3.2/153.html