Bug 49638 - Add servercontrols to lookup() to support pagination
Add servercontrols to lookup() to support pagination
Status: CLOSED FIXED
Product: UCS
Classification: Unclassified
Component: UDM (Generic)
UCS 4.4
Other Linux
: P5 normal (vote)
: UCS 4.4-0-errata
Assigned To: Florian Best
Jürn Brodersen
:
Depends on:
Blocks: 49666
  Show dependency treegraph
 
Reported: 2019-06-13 10:21 CEST by Florian Best
Modified: 2019-06-26 17:42 CEST (History)
3 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:
best: Patch_Available+


Attachments
patch (8.89 KB, patch)
2019-06-13 10:22 CEST, Florian Best
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Florian Best univentionstaff 2019-06-13 10:21:44 CEST
The UDM handlers lookup() method should support additional optional parameters to allow passing ldap-controls.
This allows e.g. to use the page control in a lookup() call.
Comment 1 Florian Best univentionstaff 2019-06-13 10:22:57 CEST
Created attachment 10059 [details]
patch
Comment 2 Florian Best univentionstaff 2019-06-13 10:33:43 CEST
Further patches in branch: fbest/49638-lookup-pagination
But it won't work for virtual modules (e.g. computers/computer) as they are doing multiple searches.
Comment 3 Florian Best univentionstaff 2019-06-13 12:37:38 CEST
If we enable the ldap overlay "sssvlv" we could also sort results:
moduleload sssvlv.so
overlay sssvlv
sssvlv-max 1000
sssvlv-maxkeys 1000
sssvlv-maxperconn 1000
(http://www.openldap.org/software/man.cgi?query=slapo-sssvlv&apropos=0&sektion=0&manpath=OpenLDAP+2.4-Release&format=html)
Comment 4 Florian Best univentionstaff 2019-06-13 13:03:16 CEST
Applied patch with some enhancements. Added a ucs-test case.

univention-directory-manager-modules (14.0.12-38)
6b7fca1a1cb2 | Bug #49638: Implement servercontrols for search()

ucs-test (9.0.2-69)
47f199fcefe1 | Bug #49638: test serverctrls in lookup()

univention-python.yaml
6b7fca1a1cb2 | Bug #49638: Implement servercontrols for search()

univention-python (12.0.0-12)
6b7fca1a1cb2 | Bug #49638: Implement servercontrols for search()

univention-directory-manager-modules.yaml
6b7fca1a1cb2 | Bug #49638: Implement servercontrols for search()
Comment 5 Jürn Brodersen univentionstaff 2019-06-13 19:15:13 CEST
Ok looks good

Will be verified if the test a green tomorrow
Comment 6 Daniel Tröder univentionstaff 2019-06-14 11:30:29 CEST
REOPEN: Docstrings have not been updated.

Please also supply example code how to use this new feature.
Comment 7 Florian Best univentionstaff 2019-06-14 12:00:02 CEST
(In reply to Daniel Tröder from comment #6)
> REOPEN: Docstrings have not been updated.
OK: adjusted in:
342210ec26a3 | Bug #49638: adjust docstrings

> Please also supply example code how to use this new feature.
The commit message contains example code and the ucs-test case does.
Comment 8 Daniel Tröder univentionstaff 2019-06-17 09:12:52 CEST
(In reply to Florian Best from comment #7)
> > Please also supply example code how to use this new feature.
> The commit message contains example code and the ucs-test case does.
* The example code should be somewhere a developer can find it with reasonable effort. That means in the docstring (or a hint in the docstring pointing to a readme file).
* It should include comments that explain what's going on.

Neither the commit message nor the ucs-test can easily be found or explain in the least what's happening.
Comment 9 Felix Botner univentionstaff 2019-06-17 10:45:06 CEST
This breaks the ucs -ldap-test (or 49422)

as soons as i install 


Holen:1 http://192.168.0.10/build2 ucs_4.4-0-errata4.4-0/all/ python-univention 12.0.0-12A~4.4.0.201906131636 [22,8 kB]
Holen:2 http://192.168.0.10/build2 ucs_4.4-0-errata4.4-0/all/ univention-directory-manager-tools 14.0.12-41A~4.4.0.201906131812 [106 kB]
Holen:3 http://192.168.0.10/build2 ucs_4.4-0-errata4.4-0/all/ python-univention-directory-manager 14.0.12-41A~4.4.0.201906131812 [323 kB]
Holen:4 http://192.168.0.10/build2 ucs_4.4-0-errata4.4-0/all/ python-univention-directory-manager-cli 14.0.12-41A~4.4.0.201906131812 [92,9 kB]


the tests fail, please repair asap
Comment 10 Florian Best univentionstaff 2019-06-17 12:05:48 CEST
(In reply to Daniel Tröder from comment #8)
> (In reply to Florian Best from comment #7)
> > > Please also supply example code how to use this new feature.
> > The commit message contains example code and the ucs-test case does.
> * The example code should be somewhere a developer can find it with
> reasonable effort. That means in the docstring (or a hint in the docstring
> pointing to a readme file).
> * It should include comments that explain what's going on.
> 
> Neither the commit message nor the ucs-test can easily be found or explain
> in the least what's happening.
Here is the example code:

from ldap.controls import SimplePagedResultsControl
res = {'ctrls': []}  # dict holding the server control response
pctrl = SimplePagedResultsControl(True, size=2, cookie='')

users = univention.admin.modules.get('users/user')
yield users.lookup(None, lo, '', serverctrls=[pctrl], response=res)
while True:
    for control in res['ctrls']:
        if control.controlType == SimplePagedResultsControl.controlType:
            pctrl.cookie = control.cookie
    if not pctrl.cookie:
        break

    yield users.lookup(None, lo, '', serverctrls=[pctrl], response=res)

python-ldap documentation:
https://www.python-ldap.org/en/latest/reference/ldap-controls.html
https://www.python-ldap.org/en/latest/reference/ldap-controls.html#ldap.controls.libldap.SimplePagedResultsControl

The explanation why/how it works is:
https://tools.ietf.org/html/rfc2696
Comment 11 Florian Best univentionstaff 2019-06-17 12:13:35 CEST
Reverted the changed search() method in univention.uldap() because ldap.ldapobject.ReconnectLDAPObject does no reconnect for search_ext().

Therefore passing serverctrls to lookup() still works, but one is unable to receive the response.
Let's fix that next BSD.

ucs-test (9.0.2-71)
63ba30a20400 | Bug #49638: partially revert because search_ext() does no reconnect-handling

univention-python (12.0.0-13)
63ba30a20400 | Bug #49638: partially revert because search_ext() does no reconnect-handling
Comment 12 Jürn Brodersen univentionstaff 2019-06-26 10:36:46 CEST
Revert -> "lo.search_ext_s(" is used directly again -> OK
Tests -> OK
YAML -> small typo (fixed) -> OK

[4.4-0 2875a57543] Bug #49638: yaml

-> Verified