Bug 49608 - Fix MAC address and ip address in computer model
Fix MAC address and ip address in computer model
Status: CLOSED FIXED
Product: UCS@school
Classification: Unclassified
Component: Ucsschool-lib
UCS@school 4.4
Other Linux
: P5 normal (vote)
: UCS@school 4.4 v2-errata
Assigned To: Ole Schwiegert
Jürn Brodersen
:
Depends on:
Blocks: 48080 50323
  Show dependency treegraph
 
Reported: 2019-06-07 10:30 CEST by Ole Schwiegert
Modified: 2019-10-07 11:55 CEST (History)
3 users (show)

See Also:
What kind of report is it?: Development Internal
What type of bug is this?: 3: Simply Wrong: The implementation doesn't match the docu
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

Note You need to log in before you can comment on or make changes to this bug.
Description Ole Schwiegert univentionstaff 2019-06-07 10:30:39 CEST
In the ucsschool lib mac address and ip address are handled as single values but are indeed multi values in UDM. Via UMC the problem does not exist, but you cannot work with computers via python since they throw Validation errors as such:

In [26]: c1.modify(lo)

---------------------------------------------------------------------------

TypeError                                 Traceback (most recent call last)

<ipython-input-26-37d852b82f23> in <module>()

----> 1 c1.modify(lo)


/usr/lib/pymodules/python2.7/ucsschool/lib/models/base.pyc in modify(self, lo, validate, move_if_necessary)

    531 		'''

    532                 self.call_hooks('pre', 'modify')

--> 533                 success = self.modify_without_hooks(lo, validate, move_if_necessary)

    534                 if success:

    535                         self.call_hooks('post', 'modify')


/usr/lib/pymodules/python2.7/ucsschool/lib/models/computer.pyc in modify_without_hooks(self, lo, validate, move_if_necessary)

    231         def modify_without_hooks(self, lo, validate=True, move_if_necessary=None):

    232                 self.create_network(lo)

--> 233                 return super(SchoolComputer, self).modify_without_hooks(lo, validate, move_if_necessary)

    234 

    235         def get_ipv4_network(self):


/usr/lib/pymodules/python2.7/ucsschool/lib/models/base.pyc in modify_without_hooks(self, lo, validate, move_if_necessary)

    545 

    546                 if validate:

--> 547                         self.validate(lo, validate_unlikely_changes=True)

    548                         if self.errors:

    549                                 raise ValidationError(self.errors.copy())


/usr/lib/pymodules/python2.7/ucsschool/lib/models/computer.pyc in validate(self, lo, validate_unlikely_changes)

    264 

    265         def validate(self, lo, validate_unlikely_changes=False):

--> 266                 super(SchoolComputer, self).validate(lo, validate_unlikely_changes)

    267                 if self.ip_address:

    268                         name, ip_address = escape_filter_chars(self.name), escape_filter_chars(self.ip_address)


/usr/lib/pymodules/python2.7/ucsschool/lib/models/base.pyc in validate(self, lo, validate_unlikely_changes)

    335                         value = getattr(self, name)

    336                         try:

--> 337                                 attr.validate(value)

    338                         except ValueError as e:

    339                                 self.add_error(name, str(e))


/usr/lib/pymodules/python2.7/ucsschool/lib/models/attributes.pyc in validate(self, value)

     80                                 raise ValueError(_('"%(label)s" needs to be a %(type)s') % {'type': self.value_type.__name__, 'label': self.label})

     81                         values = value if self.value_type else [value]

---> 82                         self._validate_syntax(values)

     83                 else:

     84                         if self.required:


/usr/lib/pymodules/python2.7/ucsschool/lib/models/attributes.pyc in _validate_syntax(self, values, syntax)

     71                         for val in values:

     72                                 try:

---> 73                                         syntax.parse(val)

     74                                 except valueError as e:

     75                                         raise ValueError(str(e))


/usr/lib/pymodules/python2.7/univention/admin/syntax.pyc in parse(self, text)

   2157         @classmethod

   2158         def parse(self, text):

-> 2159                 if self.regexLinuxFormat.match(text) is not None:

   2160                         return text.lower()

   2161                 elif self.regexWindowsFormat.match(text) is not None:


TypeError: expected string or buffer



This has to be fixed

! Be aware of possible problems in the UMC after fixing
Comment 1 Ole Schwiegert univentionstaff 2019-06-07 12:24:12 CEST
Package: ucs-school-umc-wizards
Version: 11.0.0-6A~4.4.0.201906071212
Branch: ucs_4.4-0
Scope: ucs-school-4.4

Package: ucs-school-lib
Version: 12.1.1-8A~4.4.0.201906071216
Branch: ucs_4.4-0
Scope: ucs-school-4.4


Editing and creating computers in the UMC should work as usual
Opening and saving(modify) a computer in python code should work.
Comment 2 Jürn Brodersen univentionstaff 2019-06-12 14:36:15 CEST
I think this breaks the computerroom tests:
https://jenkins.knut.univention.de:8181/job/UCSschool-4.4/job/Install%20Singleserver/lastCompletedBuild/testReport/



Traceback (most recent call last):
  File "21_computerroom_module_base_checks", line 55, in <module>
    main()
  File "21_computerroom_module_base_checks", line 24, in main
    created_computers = computers.create()
  File "/usr/lib/pymodules/python2.7/univention/testing/ucsschool/computerroom.py", line 734, in create
    computer_import.run_import(self.open_ldap_co)
  File "/usr/lib/pymodules/python2.7/univention/testing/ucsschool/computerroom.py", line 81, in run_import
    WindowsComputerLib(**kwargs).create(open_ldap_co)
  File "/usr/lib/pymodules/python2.7/ucsschool/lib/models/computer.py", line 212, in create
    return super(SchoolComputer, self).create(lo, validate)
  File "/usr/lib/pymodules/python2.7/ucsschool/lib/models/base.py", line 470, in create
    success = self.create_without_hooks(lo, validate)
  File "/usr/lib/pymodules/python2.7/ucsschool/lib/models/computer.py", line 216, in create_without_hooks
    return super(SchoolComputer, self).create_without_hooks(lo, validate)
  File "/usr/lib/pymodules/python2.7/ucsschool/lib/models/base.py", line 485, in create_without_hooks
    raise ValidationError(self.errors.copy())
ucsschool.lib.models.attributes.ValidationError: {'ip_address': ['"IP address" needs to be a list'], 'mac_address': ['"MAC address" needs to be a list']}
Comment 3 Ole Schwiegert univentionstaff 2019-06-13 09:52:09 CEST
Package: ucs-school-lib
Version: 12.1.1-9A~4.4.0.201906130949
Branch: ucs_4.4-0
Scope: ucs-school-4.4

Package: ucs-test-ucsschool
Version: 6.0.8A~4.4.0.201906130951
Branch: ucs_4.4-0
Scope: ucs-school-4.4

This should fix the tests and also fix another error in the get_ipv4_network function of the ucsschool lib
Comment 4 Daniel Tröder univentionstaff 2019-06-17 09:13:58 CEST
http://jenkins.knut.univention.de:8080/job/UCSschool-4.4/job/Install%20Multiserver/133/Config=s4-all-components,TestGroup=base1/testReport/junit/90_ucsschool/22_computerroom_test_printmode_ucr_variables/slave2032/

(2019-06-17 03:31:01.252910) Traceback (most recent call last):
(2019-06-17 03:31:01.252939)   File "22_computerroom_test_printmode_ucr_variables", line 131, in <module>
(2019-06-17 03:31:01.252988)     main()
(2019-06-17 03:31:01.253013)   File "22_computerroom_test_printmode_ucr_variables", line 110, in main
(2019-06-17 03:31:01.253063)     if not ucr_check_both_values(settingslist):
(2019-06-17 03:31:01.253090)   File "22_computerroom_test_printmode_ucr_variables", line 93, in ucr_check_both_values
(2019-06-17 03:31:01.253115)     elif setting.printmode == 'default' and (setting.ip in ucr.get('samba/printmode/hosts/all', '') or setting.ip in ucr.get('samba/printmode/hosts/none', '')):
(2019-06-17 03:31:01.253140) TypeError: 'in <string>' requires string as left operand, not list
Comment 5 Ole Schwiegert univentionstaff 2019-06-17 10:24:53 CEST
Package: ucs-test-ucsschool
Version: 6.0.11A~4.4.0.201906171024
Branch: ucs_4.4-0
Scope: ucs-school-4.4

fixed.
Comment 6 Daniel Tröder univentionstaff 2019-06-18 11:03:06 CEST
Fix 90_ucsschool.35_import-computers_via_* (`TypeError: cannot concatenate 'str' and 'list' objects`):

[4.4] fa458df76 Bug #49608: fix test not extracting mac/ip from list

ucs-test-ucsschool (6.0.12)
Comment 7 Jürn Brodersen univentionstaff 2019-07-01 16:57:12 CEST
Should this actually work with multiple IPs/macs?

I added a second IP to a schoolcomputer (through udm). On the umc both IPs are shown comma separated, but it's not possible to save any changes due to validation errors. I guess something like:
'''
values['ip_address'] = values['ip_address'].split(',')
'''
would work, but read on ;)

I tested the python api, changes to a SchoolComputer object remove any IP but the first:

'''
In [11]: x = computer.SchoolComputer.from_dn(c1_dn, 'DEMOSCHOOL', lo)

In [12]: x.ip_address
Out[12]: ['10.200.41.231', '10.200.41.232']

In [13]: x.inventory_number
Out[13]: '123'

In [14]: x.inventory_number = '456'

In [15]: x.modify(lo)
Out[15]: True

In [16]: x = computer.SchoolComputer.from_dn(c1_dn, 'DEMOSCHOOL', lo)

In [17]: x.ip_address
Out[17]: ['10.200.41.231']
'''
Comment 8 Ole Schwiegert univentionstaff 2019-07-02 07:54:21 CEST
The scope of this bug was to fix the broken usage (modification) of SchoolComputer in python due to the different data handling between the schoollib and udm. This was done by adapting the ip and mac address to lists.

I agree that the current state is not very fullfilling -- the data is stored as a list but the lib only supports one value. This should be changed in my opinion as well. But since before the change both data fields were single values, SchoolComputers were not supposed to have multiple network cards.

IMHO a new bug should be created addressing the missing handling of ip_address and mac_address in the school lib. This one should be closed though to free the way for Bug #48080.
Comment 9 Florian Best univentionstaff 2019-07-02 10:59:40 CEST
For me this looks like a regression which causes data loss.
Modifying an existing computer object via the UCS@school library causes that all IP addresses except the first one gets removed from the object. That's even happening when you don't change the IP adress at all.

I disagree to fix that in a further bug if that wasn't a bug before.
Comment 10 Ole Schwiegert univentionstaff 2019-07-09 09:38:53 CEST
Package: ucs-school-lib
Version: 12.1.2-1A~4.4.0.201907090931
Branch: ucs_4.4-0
Scope: ucs-school-4.4

The logic causing this to happen is a piece of code integrated into the lib to trigger the automatic assignment of the next free ip functionality in udm. I altered it, so that it is only triggered if the computer object has no ip yet or only one.

This should fix the issue and retain the functionality for the cases it was intended to (import computers, create computer in UMC wizard)
Comment 11 Jürn Brodersen univentionstaff 2019-07-10 11:07:00 CEST
What I tested:
Added and modified a computer through the umc school computer module -> OK

Opening a computer with multiple IPs works on the umc -> saving throws an validation error, but nothing is changed -> No regression -> OK

Opening a computer with multiple IPs works with the python api -> Ip addresses aren't changed. (bug 49828, but no regression) -> OK
Comment 12 Jürn Brodersen univentionstaff 2019-07-26 13:57:38 CEST
4.4 v3 released