Bug 48410 - S4 Connector: make mapping configurable
S4 Connector: make mapping configurable
Status: CLOSED FIXED
Product: UCS
Classification: Unclassified
Component: S4 Connector
UCS 4.3
Other Linux
: P5 enhancement with 4 votes (vote)
: UCS 4.4-1-errata
Assigned To: Florian Best
Arvid Requate
:
: 32211 45220 (view as bug list)
Depends on:
Blocks: 51280 49981
  Show dependency treegraph
 
Reported: 2019-01-03 09:12 CET by Stephan Hendl
Modified: 2020-05-12 12:29 CEST (History)
11 users (show)

See Also:
What kind of report is it?: Bug Report
What type of bug is this?: 4: Minor Usability: Impairs usability in secondary scenarios
Who will be affected by this bug?: 2: Will only affect a few 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.091
Enterprise Customer affected?: Yes
School Customer affected?: Yes
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number: 2019072621000249
Bug group (optional): Forked for project, Roadmap discussion (moved)
Max CVSS v3 score:


Attachments
example.patch (1.78 KB, patch)
2019-04-29 17:29 CEST, Arvid Requate
Details | Diff
patch alternative (git:fbest/48410-s4connector-mapping-include) (897 bytes, patch)
2019-05-07 09:58 CEST, Florian Best
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Stephan Hendl 2019-01-03 09:12:30 CET
+++ This bug was initially created as a clone of Bug #35589 +++

We should consider making the synchronisation of additional attributes between OpenLDAP and Samba 4/Microsoft AD more comfortable. 

Right now, when one wants to synchronize e.g. departmentNumber or pagerTelephoneNumber, one needs to modify the mapping.py and maintain an own version of it.
Making this configurable through UCR would make maintaining these customizations much easier.
Comment 1 Michel Smidt 2019-04-23 09:49:45 CEST
Feedback from a customer:
"Lassen sich diese Einstellungen nicht per include oder ähnlicher
Mechanik persistenter, bzw. weniger intrusiv gestalten?"
Comment 2 Arvid Requate univentionstaff 2019-04-29 17:29:00 CEST
Created attachment 9995 [details]
example.patch

For example like this: Make main.py include a localmapping.py which defines a function adjustmapping that can modify the passed mapping.s4_mapping dictionary.
Comment 3 Florian Best univentionstaff 2019-05-07 09:58:15 CEST
Created attachment 10007 [details]
patch alternative (git:fbest/48410-s4connector-mapping-include)

I am suggesting this alternative patch. It does the mapping in mapping.py itself because mapping is also included in some other scripts (list-rejected, ...). What do you think, Arvid?
Comment 4 Arvid Requate univentionstaff 2019-05-07 20:22:23 CEST
Yeah, I was hoping for you to make a better suggestion :-) Looks good to me.
Comment 5 Christina Scheinig univentionstaff 2019-07-26 10:14:34 CEST
Requested again
Comment 6 Florian Best univentionstaff 2019-08-08 15:46:26 CEST
You can now create /etc/univention/connector/s4/localmapping.py with the following content:

def hook_mapping(s4_mapping):
    # Do something
    # e.g.: s4_mapping['user'].con_default_dn = 'cn=userz,DC=SCHOOL,DC=DEV'
    return s4_mapping

univention-s4-connector (13.0.2-32)
50a40df1d069 | Bug #48410: Include /etc/univention/connector/s4/localmapping.py in mapping.py

univention-s4-connector.yaml
50a40df1d069 | Bug #48410: Include /etc/univention/connector/s4/localmapping.py in mapping.py
Comment 7 Arvid Requate univentionstaff 2019-08-08 23:18:12 CEST
> def hook_mapping(s4_mapping):

actually it's  "mapping_hook" and that brings me to the point: It would be nice to have a debug message here. Something like this pseudo-code:

==================
try:                                                                             
    localmapping = imp.load_source('localmapping', os.path.join(os.path.dirname(__file__), 'localmapping.py'))
except IOError:                                                
    pass

try:                                                                             
    mapping_hook = localmapping.mapping_hook
except AttributeError:                                                
    ud.debug(... WARN, "No mapping_hook defined in localmapping.py")  ## Or print to stderr / connector-status.log or whatever.
==================

But I see that the univention.debug stuff get's initialized much later, so that might be difficult.
What do you think? We can also postpone this log message to another improvement bug at leave it like this for now.
Comment 8 Florian Best univentionstaff 2019-08-08 23:22:39 CEST
(In reply to Arvid Requate from comment #7)
> What do you think? We can also postpone this log message to another
> improvement bug at leave it like this for now.
Jeah, maybe later with more error handling.
Comment 9 Arvid Requate univentionstaff 2019-08-09 10:07:21 CEST
6f1210fc6f | Minor advisory wording change

Verified:
* Code review
* Simple functional test (mapping_hook get's executed)
* Advisory
Comment 10 Valentin Heidelberger univentionstaff 2019-08-14 12:23:32 CEST
*** Bug 32211 has been marked as a duplicate of this bug. ***
Comment 11 Florian Best univentionstaff 2019-08-15 19:23:41 CEST
I added a further change which converts the whole mapping.py @!@ UCR things into a regular python file.
It still contains some @%@ to evaluate some UCR variables but no @!@ python evaluations.
The file is still written prior to starting the S4-Connector. This is necessary to not break any customer which has a overwritten configuration file in /etc/….
But we should move away from /etc/ to /usr/lib/python2.7/… somehwen, which is now possible with the local-config.
This change is the base for importing the mapping without the need to pipe it through UCR filter.

The change has a dramatical performance increase. Prior to the change it took ~12 seconds to start the S4-Connector which now happens in 1 second.

Additionally, to make debugging in support cases easier, the whole evaluated s4_mapping in now logged at every start of the s4-connector with debug/level=4 (ALL). This is ~75 KB per start!

univention-s4-connector (13.0.2-38)
4795bb08ed44 | Bug #48410: Cleanup mapping.py to be pythonic

Prior:
# time cat /etc/univention/connector/s4/mapping | univention-config-registry filter --encode-utf8 >/etc/univention/connector/s4/mapping.py
real    0m12,648s
user    0m10,508s
sys     0m1,484s

Afterwards:
# time cat /etc/univention/connector/s4/mapping | univention-config-registry filter --encode-utf8 >/etc/univention/connector/s4/mapping.py                                                                                                                       
real    0m0,050s
user    0m0,040s
sys     0m0,004s
# time python /etc/univention/connector/s4/mapping.py
real    0m1,264s
user    0m1,124s
sys     0m0,124s
Comment 12 Florian Best univentionstaff 2019-08-16 09:49:08 CEST
Fixed tracebacks like these:

Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/univention/s4connector/__init__.py", line 897, in __sync_file_from_ucs
    if ((old_dn and not self.sync_from_ucs(key, mapped_object, pre_mapped_ucs_dn, unicode(old_dn, 'utf8'), old, new)) or (not old_dn and not self.sync_from_ucs(key, mapped_object, pre_mapped_ucs_dn, old_dn, old, new))):
  File "/usr/lib/python2.7/dist-packages/univention/s4connector/s4/__init__.py", line 2460, in sync_from_ucs
    f(self, property_type, object)
TypeError: 'NoneType' object is not callable

univention-s4-connector (13.0.2-39)
f61cbad53b42 | Bug #48410: fix filtering None from lists
Comment 13 Arvid Requate univentionstaff 2019-08-22 13:00:45 CEST
Ok, works.
Advisory Ok.
Comment 14 Arvid Requate univentionstaff 2019-08-22 15:29:57 CEST
<http://errata.software-univention.de/ucs/4.4/239.html>
Comment 15 Florian Best univentionstaff 2019-10-23 13:20:34 CEST
*** Bug 45220 has been marked as a duplicate of this bug. ***