Bug 33223 - DHCP-Optionen broken, attach to all DHCP levels
DHCP-Optionen broken, attach to all DHCP levels
Status: CLOSED FIXED
Product: UCS
Classification: Unclassified
Component: UMC - DHCP
UCS 3.2
All Linux
: P5 enhancement (vote)
: UCS 4.2
Assigned To: Florian Best
Philipp Hahn
: interim-1
Depends on: 26838
Blocks: 31383 32557 43748
  Show dependency treegraph
 
Reported: 2013-11-11 08:51 CET by Philipp Hahn
Modified: 2017-04-04 18:28 CEST (History)
8 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): Cleanup
Max CVSS v3 score:
klaeser: Patch_Available+


Attachments
Fix DHCP options + cleanup (79.83 KB, patch)
2013-11-11 08:51 CET, Philipp Hahn
Details | Diff
Fix DHCP options + cleanup (splitup) (176.19 KB, patch)
2013-11-20 22:44 CET, Philipp Hahn
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Philipp Hahn univentionstaff 2013-11-11 08:51:44 CET
Created attachment 5596 [details]
Fix DHCP options + cleanup

+++ This bug was initially created as a clone of Bug #26838 +++
The mentioned bug added the ability to configure custom "DHCP option" statements for subnets and services. It's incomplete and partly broken:

1. add_dhcp_objectclass() is only called for service and (shared)subnet, but options can only be set for hosts, pools (Bug #3374), shared networks and services.

# grep -n add_dhcp_options\( [^_]*.py
server.py:78:add_dhcp_options( property_descriptions, mapping, layout )
service.py:76:add_dhcp_options( property_descriptions, mapping, layout )
sharedsubnet.py:123:add_dhcp_options( property_descriptions, mapping, layout )
subnet.py:123:add_dhcp_options( property_descriptions, mapping, layout )

# grep -n add_dhcp_options\( [^_]*.py
server.py:78:add_dhcp_options( property_descriptions, mapping, layout )
service.py:76:add_dhcp_options( property_descriptions, mapping, layout )
sharedsubnet.py:123:add_dhcp_options( property_descriptions, mapping, layout )
subnet.py:123:add_dhcp_options( property_descriptions, mapping, layout )


2. Doing it for dhcp/server is pointless, since the LDAP object is only used to search for the dhcp/service that server is supposed to serve.


3. The mapping is wrong: the attribute is multi-valued:
 _properties = {
        'option': univention.admin.property(
...
                multivalue=True,
...
 _mappings = (
        ( 'option', 'dhcpOption', None, univention.admin.mapping.ListToString ),


The attached patch fixes that and
3. allow enabling/disabling in GUI (Bug #32557, Bug #29034, Bug #15841)
4. adds the ability to declare DHCP options on the advanced tab (Bug #32557)
5. implements lookup_filter to speed-up lookups
6. improve the short descriptions (Bug #33222 needs to be adapted then)
7. removes double definition of options={}
8. removes useless global declarations in __init__
9. merges several implementations of rangeMap() and rangeUnmap()
10. removes code duplication by using loops instead of loop-unrolling
11. re-facture code into common super-class
12. converts 0/1 to False/True
13. more pep8 compliance
Comment 1 Philipp Hahn univentionstaff 2013-11-20 22:44:44 CET
Created attachment 5666 [details]
Fix DHCP options + cleanup (splitup)

As Alex requested here's the patch split up into logical chunks.
Comment 2 Stefan Gohmann univentionstaff 2013-11-22 06:46:07 CET
To get an idea for the priority:

(In reply to Philipp Hahn from comment #0)
> 1. add_dhcp_objectclass() is only called for service and (shared)subnet, but
> options can only be set for hosts, pools (Bug #3374), shared networks and
> services.

This is an enhancement request.

> 2. Doing it for dhcp/server is pointless, since the LDAP object is only used
> to search for the dhcp/service that server is supposed to serve.

This breaks nothing but as you said it does not help and would be a cleanup task.
 
> 
> 3. The mapping is wrong: the attribute is multi-valued:
>  _properties = {
>         'option': univention.admin.property(
> ...
>                 multivalue=True,
> ...
>  _mappings = (
>         ( 'option', 'dhcpOption', None,
> univention.admin.mapping.ListToString ),

This is a bug and results that only one option can be set.
Comment 3 Stefan Gohmann univentionstaff 2013-11-29 07:59:04 CET
(In reply to Stefan Gohmann from comment #2)
> > 
> > 3. The mapping is wrong: the attribute is multi-valued:
> >  _properties = {
> >         'option': univention.admin.property(
> > ...
> >                 multivalue=True,
> > ...
> >  _mappings = (
> >         ( 'option', 'dhcpOption', None,
> > univention.admin.mapping.ListToString ),
> 
> This is a bug and results that only one option can be set.

This has been split to: Bug #33614
Comment 4 Florian Best univentionstaff 2016-11-02 20:55:43 CET
Most of the patch has already been applied during the current errata updates. The rest is now also considered in:

univention-directory-manager-modules (12.0.5-1):
r74041 | Bug #33223: get the module through sys.module[] instead of passing in all the option/properties/mapping/layout instances of the module.
r74040 | Bug #33223: Implement searching as a common class-method reusing the speed-optimized lookup_filter() function introduced for UCS-3.2.
r74039 | Bug #33223: Define function to map and unmap ranges once and import them in all other users.
r74038 | Bug #33223: Use loop instead of unrolled implementation to handle all allow/deny variants the same.


DHCP options get fixed in Bug #32557.
Comment 5 Philipp Hahn univentionstaff 2017-01-19 13:51:38 CET
OK: r74041 r74040 r74039 r74038
OK: Bug #33614
OK: Bug #32557

1. add_dhcp_objectclass → Bug #32557
2. dhcp/server → Bug #32557 @ r75613 
3. multivalued → Bug #33614
3. allow enabling/disabling in GUI → Bug #32557
4. advanced options → Bug #32557
5. lookup_filter → r74040
6. short descriptions → Bug #42177 @ r73506
7. double options={} → Bug #27123 @ r73160
8. global → Bug #27123 @ r73160
9. merges rangeMap() → r74039
10. loop-unrolling → r74038
11. super-class -> r74040
12. converts bool → Bug #31771 @ r73805
13. pep8 → Bug #31771 @ r73869

OK: No changelog
Comment 6 Stefan Gohmann univentionstaff 2017-04-04 18:28:25 CEST
UCS 4.2 has been released:
 https://docs.software-univention.de/release-notes-4.2-0-en.html
 https://docs.software-univention.de/release-notes-4.2-0-de.html

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