Univention Bugzilla – Bug 45715
[4.3] school-import should check most common UDM/ucsschool.lib.models exceptions in dry-run
Last modified: 2019-02-27 12:33:18 CET
Some errors do not show up during a dry-run but when doing the real import. They are caught by either ucsschool.lib.models.*.modify() or udm-object.modify(), which is not done in the dry-run. Test for the 5 most common errors, that show up in the real import, but not in dry-run.
*** Bug 46235 has been marked as a duplicate of this bug. ***
Bug #46235: add test for existing username from import with different (or no) sourceUID.
Exceptions/Errors thrown by precreate/premodify/preremove hooks (I know, that the hooks may perform LDAP changes on their own, so it's no real try run, if they are run. But given the case, that they only modify input data, they should be run also in dry-run).
(In reply to Sönke Schwardt-Krummrich from comment #3) > Exceptions/Errors thrown by precreate/premodify/preremove hooks > (I know, that the hooks may perform LDAP changes on their own, so it's no > real try run, if they are run. But given the case, that they only modify > input data, they should be run also in dry-run). Hmm... I think we should do this only after adding a "dry-run" option to pyhooks. This way users can implement a dry-run mode and they will benefit from it by being able to easily test their hooks. Without that I fear to many side effects in currently used hooks.
(In reply to Daniel Tröder from comment #4) > (In reply to Sönke Schwardt-Krummrich from comment #3) > Hmm... I think we should do this only after adding a "dry-run" option to > pyhooks. This way users can implement a dry-run mode and they will benefit > from it by being able to easily test their hooks. Without that I fear to > many side effects in currently used hooks. +1
dry-run should check if the school(s), that a user should be added to, actually exist(s).
add a dry-run mode for pyhooks
add a warning if a class name begins with "$OU-".
Support for running PyHooks during a dry-run was added. The complete import code, including the hooks, now use a machine connection during dry-runs. The dry-run mode for hooks has been documented in the hook example and in the import manual. [dtroeder/45715_4.3_checks_in_dry-run] 798dccf3 Bug #45715: consistent wording: dry-run with dash [dtroeder/45715_4.3_checks_in_dry-run] 852bad00 Bug #45715: use admin connection in a real run, use machine connection during a dry-run [dtroeder/45715_4.3_checks_in_dry-run] 832ee201 Bug #45715: add support for a dry-run mode in pyhooks [dtroeder/45715_4.3_checks_in_dry-run] 1e828a06 Bug #45715: add support for a filter function to pyhooks-loader [dtroeder/45715_4.3_checks_in_dry-run] 4f129463 Bug #45715: during dry-run use only PyHook classes that support it [dtroeder/45715_4.3_checks_in_dry-run] 400094f1 Bug #45715: don't reload user from LDAP after a dry-run create [dtroeder/45715_4.3_checks_in_dry-run] 6d792566 Bug #45715: run hooks in dry-run [dtroeder/45715_4.3_checks_in_dry-run] 8f43047a Bug #45715: don't run legacy hooks in dry-run [dtroeder/45715_4.3_checks_in_dry-run] d0d21c46 Bug #45715: make import tests always verbose [dtroeder/45715_4.3_checks_in_dry-run] 70b0e1b5 Bug #45715: lower logging serverity for running hook announcement [dtroeder/45715_4.3_checks_in_dry-run] eaae624a Bug #45715: fix section links Commit 852bad00 contains a signature change (of modify()) that was intended for Bug #47183.
All check code is now in the method ImportUser.validate() which is called by the ucsschool.lib before create/modify/remove. The import code runs it additionally in dry-run. Future checks (for dry-run as well as real-run) should be added to validate(). * The check for taken usernames has been implemented in commit 6f6f7b2e6. * The warning about school name in class name has been implemented in commit 463c1b5c5. Two bugs have been accidentally been discovered and fixed: * Because of a change for the HTTP-API, that strictly prevents a role change, it had the side effect, that now it was not detected any more. Instead a new user was created. This has been fixed in commit 020daebec. * A birthday = '' lead to a UDM error. It is only happy with birthday = None. Fixed in commit b80950b91. The machine connection usage introduced in commit 852bad00 has been changed to be a read-only cn=admin connection. This is tracked in Bug #47203. A ucs-test has been added, that installs a PyHook with dry-run support and one without. It then starts 8 imports that lead to running create, modify, move and remove hooks in both dry-run and real-run. [dtroeder/45715_4.3_checks_in_dry-run] 865b5ac58 Bug #45715: remove unused exception [dtroeder/45715_4.3_checks_in_dry-run] 302a14bdd Bug #45715: remove useless try-except block, leftover from commit 1906c8c (Bug #41882: lower memory footprint of import) [dtroeder/45715_4.3_checks_in_dry-run] 8a1e4b1f7 Bug #45715: use ucsschool.lib exception instead of uldap exception [dtroeder/45715_4.3_checks_in_dry-run] 237c62f53 Bug #45715: add missing configuration object for when classmethod is used before initialization [dtroeder/45715_4.3_checks_in_dry-run] b80950b91 Bug #45715: handle empty birthday column [dtroeder/45715_4.3_checks_in_dry-run] 020daebec Bug #45715: detect role change and prevent creation of additional user [dtroeder/45715_4.3_checks_in_dry-run] 6f6f7b2e6 Bug #45715: check username not taken [dtroeder/45715_4.3_checks_in_dry-run] 463c1b5c5 Bug #45715: warn if school name in class name [dtroeder/45715_4.3_checks_in_dry-run] 55453cdce Bug #45715: use ucsschool.lib supported validate() instead of run_checks(), sort checks by how expensive they are and if in_hook [dtroeder/45715_4.3_checks_in_dry-run] f31420bd5 Bug #45715: add test for dry-run support in pyhooks
Code was merged to 4.3 and built. [4.3] 476880c71 Bug #45715 Bug #47203: Merge branch 'dtroeder/45715_4.3_checks_in_dry-run' into 4.3 [4.3] 87b3e8cb3 Bug #45715 Bug #47203: changelog [4.3] 96b537756 Bug #45715 Bug #47203: advisory ucs-school-import (16.0.2-29) ucs-school-lib (11.0.1-17) ucs-test-ucsschool (5.0.2-74)
Advisory & Changelog: OK Package Install: OK school in class name warning: OK Tests run: OK Hooks with and without dry run support: OK role change: OK birthday='': OK username taken: REOP (*) (*) In a dry run the import did not realize that the username was already taken. In the real import the import did realize but I could not find the message introduced in commit dcdf5fcf2e95739f283b1e3a7e344ff846521b70 but 2: Error adding 1/1 ImportStudent(name=u'ole.schwiegert', school='AE', dn=u'uid=ole.schwiegert,cn=schueler,cn=users,ou=AE,dc=realm2,dc=intranet', old_dn=None) (source_uid:TESTID record_uid: Ole.Schwiegert), does probably already exist. It is the same message I got while testing role change. Does that fix actually catch the more generic 'same username taken' error already?
* maybe add check for Bug #47681
There were actually 2 problems with the username check code: 1) Having moved to using validate() as in the ucsschool.lib, it didn't raise an exception, but added to self.errors. But self.errors is only checked when create()/modify()/move() actually run - and that doesn't happen in dry-run. The required code has been added (even though it is currently unused). 2) The uniqueness test checked that the username matched a DN. But that doesn't take into account that the User "dn" attribute is actually a property that calculates the DN from username+role+OU. So An existing user and a new/renamed user of the same role and in the same OU would automatically have the same username and DN - and the check would not fail. The new check now looks at source_uid+recourd_uid as identifying properties. If a triplett username+source_uid+recourd_uid matches, then the check is OK -> it must be a modify/move of the same user, otherwise it would mean a new user and it would fail. [4.3] 964148d64 Bug #45715: add test for "username not in use" [4.3] ae00d309c Bug #45715: check for ValidationErrors after running validate() in dry-run [4.3] b0c606483 Bug #45715: use string_types instead of basestring [4.3] 7127e1432 Bug #45715: fix username uniqueness test [4.3] 3d9161317 Bug #45715: add mypy annotations [4.3] 5bd1f4fee Bug #45715: handle ImportError while ucs-test from 4.3-1errata is not in 4.3-2 [4.3] 71ae5a8bc Bug #45715: advisory update ucs-school-import (16.0.2-41) ucs-test-ucsschool (5.0.2-88)
Tested with ucs-school-import 16.0.2-41A~4.3.0.201808311701 ucs-test-ucsschool 5.0.2-88A~4.3.0.201808311703 Changelog&Advisory: OK username taken: OK I got the following error in the import log (dry-run) Traceback (most recent call last): File "/usr/lib/pymodules/python2.7/ucsschool/importer/mass_import/user_import.py", line 193, in create_and_modify_users user.validate(self.connection, validate_unlikely_changes=True, check_username=True) File "/usr/lib/pymodules/python2.7/ucsschool/importer/models/import_user.py", line 1065, in validate self.name, un.dn, un.source_uid, un.record_uid)) UniqueIdError: Username 'ole.schwiegert' is already in use by 'uid=ole.schwiegert,cn=schueler,cn=users,ou=AE,dc=realm2,dc=intranet' (source_uid: 'ae-student', record_uid: 'Ole.Schwiegert'). Tests run: REOP I run ./231_import-users_checks_in_dry-run_username -f and get following error: 2018-09-03 07:57:33 ERROR cmdline.main:140 Outer Exception catcher: TypeError('expected string or buffer',) Traceback (most recent call last): File "/usr/lib/pymodules/python2.7/ucsschool/importer/frontend/cmdline.py", line 120, in main self.do_import() File "/usr/lib/pymodules/python2.7/ucsschool/importer/frontend/cmdline.py", line 97, in do_import importer.mass_import() File "/usr/lib/pymodules/python2.7/ucsschool/importer/mass_import/mass_import.py", line 76, in mass_import self.import_users() File "/usr/lib/pymodules/python2.7/ucsschool/importer/mass_import/mass_import.py", line 106, in import_users user_import.create_and_modify_users(imported_users) # 90% - 100% File "/usr/lib/pymodules/python2.7/ucsschool/importer/mass_import/user_import.py", line 201, in create_and_modify_users success = user.create(lo=self.connection) File "/usr/lib/pymodules/python2.7/ucsschool/importer/models/import_user.py", line 330, in create res = super(ImportUser, self).create(lo, validate) File "/usr/lib/pymodules/python2.7/ucsschool/lib/models/base.py", line 435, in create success = self.create_without_hooks(lo, validate) File "/usr/lib/pymodules/python2.7/ucsschool/lib/models/base.py", line 463, in create_without_hooks self.do_create(udm_obj, lo) File "/usr/lib/pymodules/python2.7/ucsschool/lib/models/user.py", line 235, in do_create udm_obj['groups'] = self.groups_used(lo) File "/usr/lib/pymodules/python2.7/ucsschool/lib/models/user.py", line 477, in groups_used self.get_or_create_group_udm_object(group_dn, lo) File "/usr/lib/pymodules/python2.7/ucsschool/lib/models/user.py", line 484, in get_or_create_group_udm_object school = cls.get_school_from_dn(group_dn) File "/usr/lib/pymodules/python2.7/ucsschool/lib/models/base.py", line 642, in get_school_from_dn return SchoolSearchBase.getOU(dn) File "/usr/lib/pymodules/python2.7/ucsschool/lib/schoolldap.py", line 203, in getOU school_dn = cls.getOUDN(dn) File "/usr/lib/pymodules/python2.7/ucsschool/lib/schoolldap.py", line 216, in getOUDN match = cls._RE_OUDN.search(dn) TypeError: expected string or buffer 2018-09-03 07:57:33 INFO: run_import:480: Import process exited with exit code 1 2018-09-03 07:57:33 ERROR: run_import:482: As requested raising an exception due to non-zero exit code *** Cleanup after exception: <class 'essential.importusers_cli_v2.ImportException'> Non-zero exit code 1 Performing UCSTestSchool cleanup... Performing UCSTestUDM cleanup... removing DN: uid=d64vceyo0m,cn=lehrer,cn=users,ou=px33klk,dc=realm2,dc=intranet removing DN: cn=realm2.intranet,cn=domain,cn=mail,dc=realm2,dc=intranet trying to restart UDM CLI server sending signal 15 to process 2237 (['/usr/bin/python2.7', '/usr/share/univention-directory-manager-tools/univention-cli-server']) process already terminated UCSTestUDM cleanup done Waiting for replication... . Done: replication complete. UCSTestSchool cleanup done 2018-09-03 07:57:36 INFO: cleanup:405: Performing CLI_Import_v2_Tester cleanup... 2018-09-03 07:57:36 INFO: cleanup:406: Purging '/tmp/34_import-users_via_cli_v2.ZllmLx' 2018-09-03 07:57:36 INFO: cleanup:194: Performing ImportTestbase cleanup... Performing UCSTestUDM cleanup... trying to restart UDM CLI server UCSTestUDM cleanup done Unsetting mail/hosteddomains 2018-09-03 07:57:37 INFO: cleanup:197: ImportTestbase cleanup done 2018-09-03 07:57:37 INFO: cleanup:414: CLI_Import_v2_Tester cleanup done Traceback (most recent call last): File "231_import-users_checks_in_dry-run_username", line 77, in <module> Test().run() File "/usr/share/ucs-test/90_ucsschool/essential/importusers_cli_v2.py", line 294, in run self.test() File "231_import-users_checks_in_dry-run_username", line 59, in test self.run_import(['-c', fn_config, '-i', fn_csv]) File "/usr/share/ucs-test/90_ucsschool/essential/importusers_cli_v2.py", line 483, in run_import raise ImportException('Non-zero exit code %r' % (exitcode,)) essential.importusers_cli_v2.ImportException: Non-zero exit code 1 Starting 1 ucs-test at 2018-09-03 07:57:37 to /dev/null UCS 4.3-1-e229 ucs-test 8.0.28-133A~4.3.0.201806112134 Verify that checks on existing usernames are executed in dry-run.......................................................................................... Test failed My ucs-school-lib version is 11.0.1-20A~4.3.0.201808301052. Did I miss or forget something to successfully run the test?
I cannot recreate this. There must be something wrong with the test-OUs on the system. Run 999_delete_test_ous to remove them. The test is successful in Jenkins on both "Install 4.3 Singleserver" and "Upgrade 4.2 to 4.3 Singleserver": http://jenkins.knut.univention.de:8080/job/UCSschool-4.3/job/Install%20Singleserver/lastBuild/Config=s4,TestGroup=import1/testReport/90_ucsschool/231_import-users_checks_in_dry-run_username/history/ http://jenkins.knut.univention.de:8080/job/UCSschool-4.3/job/Upgrade%20Singleserver/lastBuild/Config=s4,TestGroup=import1/testReport/90_ucsschool/231_import-users_checks_in_dry-run_username/history/ I fixed some mypy annotation syntax errors. No rebuild necessary. [4.3] 0b75c7ade Bug #45715: fix mypy annotations
And within hours I had the same Problem as Ole in comment 15 :) At least for me the Problem was that I had used the http api previously... """ cp /usr/share/ucs-school-import/configs/ucs-school-testuser-http-import.json /var/lib/ucs-school-import/configs/user_import.json ./ucs-school-testuser-import --students 1 --classes 1 $YOUR_SCHOOL_OU """ See also bug 47712
tested with ucs-test-ucsschool 5.0.2-90A~4.3.0.201809031312 After using a fresh ucs school install (where I did not use the httpapi before), the test passes for me as well. The problem seems to be somewhere else and not at this bug. Test runs: OK
90_ucsschool.34_import-users_legacy_migration.test is failing with this update. That has to be fixed before verifying.
The legacy import configuration of the import framework (aka "new-legacy") must handle users created with the old import script (aka "old-legacy"). This creates a update path for customers, because the new-legacy import adds source_uid and record_uid attributes to created and modified users. [4.3] cb9e21e3f Bug #45715: new-legacy import must handle users from old-legacy import that don't have record_uid and source_uid [4.3] 332f1ed32 Bug #45715: add static type annotations [4.3] 7afae16e7 Bug #45715: advisory update ucs-school-import (16.0.2-43)
(In reply to Daniel Tröder from comment #20) > [4.3] cb9e21e3f Bug #45715: new-legacy import must handle users from > old-legacy import that don't have record_uid and source_uid > [4.3] 332f1ed32 Bug #45715: add static type annotations > [4.3] 7afae16e7 Bug #45715: advisory update OK: code change (comparison now again via DN and not via sourceUID/recordUID) OK: ucs-test
UCS@school 4.3 v5 has been released. https://docs.software-univention.de/changelog-ucsschool-4.3v5-de.html If this error occurs again, please clone this bug.