Bug 52874 - 55_adconnector/263read_ad_remove_capital_user_from_group shows timing dependent memberUid spelling
55_adconnector/263read_ad_remove_capital_user_from_group shows timing depende...
Status: CLOSED FIXED
Product: UCS
Classification: Unclassified
Component: AD Connector
UCS 5.0
Other Linux
: P5 normal (vote)
: UCS 5.0
Assigned To: Arvid Requate
Florian Best
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2021-03-08 21:36 CET by Arvid Requate
Modified: 2021-05-25 16:02 CEST (History)
2 users (show)

See Also:
What kind of report is it?: Development Internal
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
263read_ad_remove_capital_user_from_group-bad-timing.sh (2.81 KB, application/x-shellscript)
2021-03-08 22:03 CET, Arvid Requate
Details
263read_ad_remove_capital_user_from_group-good-timing.sh (2.79 KB, application/x-shellscript)
2021-03-08 22:05 CET, Arvid Requate
Details
bug52874-workaround.patch (1.59 KB, patch)
2021-03-08 22:34 CET, Arvid Requate
Details | Diff
bug52874-fix-proposal.diff (779 bytes, patch)
2021-03-09 12:19 CET, Arvid Requate
Details | Diff
bug52874-fix-proposal2.diff (1.28 KB, patch)
2021-03-09 14:26 CET, Arvid Requate
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Arvid Requate univentionstaff 2021-03-08 21:36:04 CET
The test case 55_adconnector/263read_ad_remove_capital_user_from_group shows that the synchronization of mixed case AD group members to UCS/OpenLDAP is timing dependent. In UCS 4 the case is preserved (in most cases?) in the memberUid and uniqueMember values. IN UCS 5 the memberUid and uniqueMember are (apparantly more often) lowercase.

The test does this:

1. create a group in AD
2. create a user in AD
3. add the user to the group in AD

The situation is a bit hard to debug, but my impression is that it depends if object_memberships_sync_to_ucs or group_members_sync_to_ucs are used for synchronization. The former possibly preserves the case of members, the latter uses lowercase values. I think object_memberships_sync_to_ucs is used if the AD-Connector sees the changes 2+3 together and the group_members_sync_to_ucs is called when the ADC sees change 3 separately.

Additionally, in UCS 5.0 the sort order of 1 and 2 seem to have changed (if the ADC sees both in one cycle). In UCS 4 the changes reported by self.__search_ad_changes seem to be ordered by ascending uSNCreated while the code in UCS 5 seems to order according to uSNChanged.

I'm not quite sure if this analysis is accurate though.
Comment 1 Arvid Requate univentionstaff 2021-03-08 22:03:58 CET
Created attachment 10633 [details]
263read_ad_remove_capital_user_from_group-bad-timing.sh

This version of the test triggers the problem also under UCS-4.

It also shows that uniqueMember is lowercase, but memberUid seems to be Ok.
Comment 2 Arvid Requate univentionstaff 2021-03-08 22:05:09 CET
Created attachment 10634 [details]
263read_ad_remove_capital_user_from_group-good-timing.sh

This version of the test artificially adjusts the timing so that the problem doesn't occur.
Comment 3 Arvid Requate univentionstaff 2021-03-08 22:34:27 CET
Created attachment 10635 [details]
bug52874-workaround.patch

This workaround seemed to fix the issue in my local tests.
Comment 4 Arvid Requate univentionstaff 2021-03-09 12:19:48 CET
Created attachment 10637 [details]
bug52874-fix-proposal.diff

Adjusting the AD change sort order back (closer) to the UCS 4.4-7 sort order seems to fix this issue. This is a refinement of the change Bug 52358#c15 .
Comment 5 Arvid Requate univentionstaff 2021-03-09 14:26:26 CET
Created attachment 10638 [details]
bug52874-fix-proposal2.diff

This version also works in Python > 3.7 and looks cleaner to me.
Comment 6 Arvid Requate univentionstaff 2021-03-09 16:30:14 CET
841d9afc80 | Fix uSNCreated/uSNChanged order of AD changes
44f98ae0a4 | Use local loop variable instead of changing lastUSN
16d8201103 | Fix sorting of AD changes in case paged search does not work

Interim version, no UCS-5 Changelog entry required.

Package: univention-ad-connector
Version: 14.0.5-5A~5.0.0.202103091626
Branch: ucs_5.0-0
Comment 9 Florian Best univentionstaff 2021-03-10 14:04:05 CET
(In reply to Arvid Requate from comment #8)
> I don't understand, Run #41 shows it as repaired:
> 
> https://jenkins.knut.univention.de:8181/job/UCS-5.0/job/UCS-5.0-0/view/
> Default/job/ADConnectorMultiEnv/lastCompletedBuild/Version=w2k16-german/
> testReport/55_adconnector/263read_ad_remove_capital_user_from_group/
> adsync234/history/#
> 
> https://jenkins.knut.univention.de:8181/job/UCS-5.0/job/UCS-5.0-0/view/
> Default/job/ADConnectorMultiEnv/Version=w2k16-german/41/testReport/

Oh, the job from yesterday is still running. That's why I didn't get the correct results.
Comment 10 Florian Best univentionstaff 2021-03-16 11:22:51 CET
OK: changes
OK: no changelog entry necessary - interim version. (We should probably have reopened the Py3-migration bug instead)
OK: Jenkins Tests

I merged the changed to the S4-Connector. All tests are passing there as well.

univention-s4-connector (14.0.6-5)
66511ba17890 | Bug #52043: Fix uSNCreated/uSNChanged order of AD changes
Comment 11 Florian Best univentionstaff 2021-05-25 16:02:43 CEST
UCS 5.0 has been released:
 https://docs.software-univention.de/release-notes-5.0-0-en.html
 https://docs.software-univention.de/release-notes-5.0-0-de.html

If this error occurs again, please use "Clone This Bug".