Bug 49102 - type and sanity check import configuration
type and sanity check import configuration
Status: CLOSED FIXED
Product: UCS@school
Classification: Unclassified
Component: Import scripts
UCS@school 4.4
Other Linux
: P5 normal (vote)
: UCS@school 4.4 v9-errata
Assigned To: Toni Röhmeyer
Tobias Wenzel
:
: 49580 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2019-03-26 12:46 CET by Daniel Tröder
Modified: 2021-05-26 11:28 CEST (History)
4 users (show)

See Also:
What kind of report is it?: Bug Report
What type of bug is this?: 2: Improvement: Would be a product improvement
Who will be affected by this bug?: 2: Will only affect a few installed domains
How will those affected feel about the bug?: 1: Nuisance – not a big deal but noticeable
User Pain: 0.023
Enterprise Customer affected?: Yes
School Customer affected?: Yes
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number: 2019031921001045
Bug group (optional):
Max CVSS v3 score:


Attachments
configuration schema patch (1.68 KB, patch)
2021-05-17 13:08 CEST, Toni Röhmeyer
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Tröder univentionstaff 2019-03-26 12:46:52 CET
A customer used "false" instead of false for a boolean. This was in Python interpreted as True. Those errors are hard to spot and create support tickets.

Run type and value-sanity checks for the complete import configuration.
Comment 1 Daniel Tröder univentionstaff 2020-02-19 11:56:03 CET
*** Bug 49580 has been marked as a duplicate of this bug. ***
Comment 2 Toni Röhmeyer univentionstaff 2020-02-25 16:21:25 CET
Added json schema file for user_import configurations.
User import will now perform a json schema validation with the added schema file.

Solution was pushed on branch troehmey/49102 with commits

commit 3d5886e30e6d142b28a3c11b9c92fd2128ca070a
Author: Toni Röhmeyer <roehmeyer@univention.de>
Date:   Tue Feb 25 16:15:48 2020 +0100

    Bug #49102: add validation with scheme

commit 59741f867d991ea3a5cc484a854ff7eb1f13c7d7
Author: Toni Röhmeyer <roehmeyer@univention.de>
Date:   Thu Feb 20 14:22:46 2020 +0100

    Bug #49102: added user_import schema


For testing /usr/share/ucs-school-import/scripts/ucs-school-user-import was performed with a test_users.csv


Waiting for QA...
Comment 3 Ole Schwiegert univentionstaff 2020-04-16 09:22:47 CEST
Great work, I am sure this will help a lot! Some things to improve upon:

** Using the command line the validation was performed, but using the Import UMC Modul for some reason I do not understand at this point validation was not performed.

** The global configuration options (https://docs.software-univention.de/ucsschool-import-handbuch-4.4.html#configuration:json_format:global) are missing in the schema and thus are not checked. They cannot only be used via command line (and are thus controlled by us), but also set in the config file. Even though this is unlikely it should be put in the schema anyway.

** If the schema file does not exists a rather confusing error message is generated if using the CommandLine:

Error setting up or checking the configuration: Error reading configuration file [Errno 2] No such file or directory: '/usr/share/ucs-school-import/schema/user_schema.json'.
Traceback (most recent call last):
  File "/usr/lib/pymodules/python2.7/ucsschool/importer/frontend/cmdline.py", line 114, in setup_config
    self.config = setup_configuration(configs, **self.args.settings)
  File "/usr/lib/pymodules/python2.7/ucsschool/importer/configuration.py", line 58, in setup_configuration
    config = Configuration(conffiles)
  File "/usr/lib/pymodules/python2.7/ucsschool/importer/configuration.py", line 209, in __new__
    cls._instance = cls.__SingleConf(filenames)
  File "/usr/lib/pymodules/python2.7/ucsschool/importer/configuration.py", line 200, in __init__
    raise InitialisationError("Error reading configuration file {}.".format(exc))
InitialisationError: Error reading configuration file [Errno 2] No such file or directory: '/usr/share/ucs-school-import/schema/user_schema.json'.
Used configuration files: ['/usr/share/ucs-school-import/configs/global_defaults.json', '/var/lib/ucs-school-import/configs/global.json', '/usr/share/ucs-school-import/configs/user_import_defaults.json', '/var/lib/ucs-school-import/configs/user_import.json', '/usr/share/ucs-school-import/configs/ucs-school-testuser-http-import.json'].
Using command line arguments: {'input': {'filename': 'test.csv'}, 'school': 'DEMOSCHOOL', 'dry_run': True, 'source_uid': 'demoschool-student', 'user_role': 'student'}
InitialisationError: Configuration not yet loaded.
Traceback (most recent call last):
  File "/usr/lib/pymodules/python2.7/ucsschool/importer/frontend/cmdline.py", line 231, in main
    self.prepare_import()
  File "/usr/lib/pymodules/python2.7/ucsschool/importer/frontend/cmdline.py", line 194, in prepare_import
    self.setup_config()
  File "/usr/lib/pymodules/python2.7/ucsschool/importer/frontend/cmdline.py", line 121, in setup_config
    config = Configuration()
  File "/usr/lib/pymodules/python2.7/ucsschool/importer/configuration.py", line 209, in __new__
    cls._instance = cls.__SingleConf(filenames)
  File "/usr/lib/pymodules/python2.7/ucsschool/importer/configuration.py", line 182, in __init__
    raise InitialisationError("Configuration not yet loaded.")
InitialisationError: Configuration not yet loaded.

This originates in the fact, that an IOError in Configuration.__init__ thrown by opening the non existing schema file is catched by the catch of non existing config files. This should be handled separately to generate a better exception message than "error reading configuration file"
Comment 4 Toni Röhmeyer univentionstaff 2020-04-17 14:57:32 CEST
Fixed two of the issues with commit

commit a3c070d9f64320773277fe0bdcafbdb3d3e9be9d
Bug #49102: added global conf options

on branch troehmey/bug49102


So far I could not fix the issue that the validation was not performed via the UMC Import module.

Work in progress...
Comment 5 Daniel Tröder univentionstaff 2020-05-05 18:09:02 CEST
(In reply to Ole Schwiegert from comment #3)
> ** Using the command line the validation was performed, but using the Import
> UMC Modul for some reason I do not understand at this point validation was
> not performed.
Validation is performed.
I added debug-level logging to the validation process:
===============================================================
------ UCS@school import tool starting ------
Reading configuration from '/usr/share/ucs-school-import/configs/global_defaults.json'...
Validating '/usr/share/ucs-school-import/configs/global_defaults.json'...
Reading configuration from '/var/lib/ucs-school-import/configs/global.json'...
Validating '/var/lib/ucs-school-import/configs/global.json'...
[..]
===============================================================
It can also be seen in /var/lib/ucs-school-import/jobs/2020/***/ucs-school-import.log

> ** The global configuration options
> (https://docs.software-univention.de/ucsschool-import-handbuch-4.4.
> html#configuration:json_format:global) are missing in the schema and thus
> are not checked. They cannot only be used via command line (and are thus
> controlled by us), but also set in the config file. Even though this is
> unlikely it should be put in the schema anyway.
Done by Toni:
[troehmey/bug49102] a3c070d9f Bug #49102: added global conf options


> ** If the schema file does not exists a rather confusing error message is
> generated if using the CommandLine:
> 
> Error setting up or checking the configuration: Error reading configuration
> file [Errno 2] No such file or directory:
> '/usr/share/ucs-school-import/schema/user_schema.json'.
> Traceback (most recent call last):
>   File
> "/usr/lib/pymodules/python2.7/ucsschool/importer/frontend/cmdline.py", line
> 114, in setup_config
>     self.config = setup_configuration(configs, **self.args.settings)
>   File "/usr/lib/pymodules/python2.7/ucsschool/importer/configuration.py",
> line 58, in setup_configuration
>     config = Configuration(conffiles)
>   File "/usr/lib/pymodules/python2.7/ucsschool/importer/configuration.py",
> line 209, in __new__
>     cls._instance = cls.__SingleConf(filenames)
>   File "/usr/lib/pymodules/python2.7/ucsschool/importer/configuration.py",
> line 200, in __init__
>     raise InitialisationError("Error reading configuration file
> {}.".format(exc))
> InitialisationError: Error reading configuration file [Errno 2] No such file
> or directory: '/usr/share/ucs-school-import/schema/user_schema.json'.
> Used configuration files:
> ['/usr/share/ucs-school-import/configs/global_defaults.json',
> '/var/lib/ucs-school-import/configs/global.json',
> '/usr/share/ucs-school-import/configs/user_import_defaults.json',
> '/var/lib/ucs-school-import/configs/user_import.json',
> '/usr/share/ucs-school-import/configs/ucs-school-testuser-http-import.json'].
> Using command line arguments: {'input': {'filename': 'test.csv'}, 'school':
> 'DEMOSCHOOL', 'dry_run': True, 'source_uid': 'demoschool-student',
> 'user_role': 'student'}
> InitialisationError: Configuration not yet loaded.
> Traceback (most recent call last):
>   File
> "/usr/lib/pymodules/python2.7/ucsschool/importer/frontend/cmdline.py", line
> 231, in main
>     self.prepare_import()
>   File
> "/usr/lib/pymodules/python2.7/ucsschool/importer/frontend/cmdline.py", line
> 194, in prepare_import
>     self.setup_config()
>   File
> "/usr/lib/pymodules/python2.7/ucsschool/importer/frontend/cmdline.py", line
> 121, in setup_config
>     config = Configuration()
>   File "/usr/lib/pymodules/python2.7/ucsschool/importer/configuration.py",
> line 209, in __new__
>     cls._instance = cls.__SingleConf(filenames)
>   File "/usr/lib/pymodules/python2.7/ucsschool/importer/configuration.py",
> line 182, in __init__
>     raise InitialisationError("Configuration not yet loaded.")
> InitialisationError: Configuration not yet loaded.
> 
> This originates in the fact, that an IOError in Configuration.__init__
> thrown by opening the non existing schema file is catched by the catch of
> non existing config files. This should be handled separately to generate a
> better exception message than "error reading configuration file"
1. Some InitialisationErrors are now displayed without a traceback.
2. The original Exception is retained, when retrying to acquire the configuration.

[troehmey/bug49102] 1d6930615 Bug #49102: improve error reporting during setup
[troehmey/bug49102 fd68b5ee7] Bug #49102: handle log_traceback=False also in Import-HTTP-API

The json schema file was renamed.

[troehmey/bug49102] 5f0cc7bb9 Bug #49102: rename json schema

Exception handling and the error generating code has been moved closer to each other.

[troehmey/bug49102] 37e34dd12 Bug #49102: refactor common code and move exception handling closer to exception cause
Comment 6 Tobias Wenzel univentionstaff 2021-05-10 14:39:47 CEST
QA → reopen

I agree: This looks very helpful.

univention-app info
UCS: 4.4-8 errata969
Installed: ucsschool=4.4 v9

ucr get server/role 
domaincontroller_master



please rebase the branch, frontend.py was changed
code → looks good.

functionality:

before fix

"verbose":"false",
→  u"verbose":u"false",

{'activate_new_users': {'default': 'false'}
→ {u'activate_new_users': {u'default': u'false'}


after fix:

Only one validation-error is displayed if multiple errors are present in config, this is OK
Every validation error is displayed twice. This might be fixed with Bug 49578, please check after rebase.

.....................

some samples of my qa:


Error setting up or checking the configuration: Schema validation failed for configuration file '/var/lib/ucs-school-import/configs/user_import.json': u'false' is not of type u'boolean'

Failed validating u'type' in schema[u'properties'][u'verbose']:
    {u'type': u'boolean'}

On instance[u'verbose']:
    u'false'.
Schema validation failed for configuration file '/var/lib/ucs-school-import/configs/user_import.json': u'false' is not of type u'boolean'

Failed validating u'type' in schema[u'properties'][u'verbose']:
    {u'type': u'boolean'}

On instance[u'verbose']:
    u'false'.


cf. Bug 49580 - verify structure and values of configuration file

Schema validation failed for configuration file '/var/lib/ucs-school-import/configs/user_import.json': {u'default': u'value'} is not of type u'string'

Failed validating u'type' in schema[u'properties'][u'scheme'][u'properties'][u'email']:
    {u'type': u'string'}

On instance[u'scheme'][u'email']:
    {u'default': u'value'}.


Error setting up or checking the configuration: Schema validation failed for configuration file '/var/lib/ucs-school-import/configs/user_import.json': u'false' is not of type u'boolean'

Failed validating u'type' in schema[u'properties'][u'activate_new_users'][u'properties'][u'default']:
    {u'type': u'boolean'}

On instance[u'activate_new_users'][u'default']:
    u'false'.


..........................

"properties": {
	"default": {"type": "string"}
}
→ student, staff, teacher, teacher_and_staff are missing

same with:
- username.max_length
- activate_new_users?

If the key is wrong, I also get no error. This is OK for me, but could be another improvement.

validation errors of imports, started via the uas user import web interface are logged to  /var/lib/ucs-school-import/jobs/ → OK


test missing validation file → OK

Error reading json schema '/usr/share/ucs-school-import/schema/user_import_configuration_schema.json': [Errno 2] No such file or directory: '/usr/share/ucs-school-import/schema/user_import_configuration_schema.json'.
Comment 7 Toni Röhmeyer univentionstaff 2021-05-12 16:42:24 CEST
I rebased and fixed the mentioned issues from comment #6 on a new feature branch "troehmey/bug49102_config_type_checks" with these commits:

71bae4c0a Bug #49102: improved exception handling
f8bb9dbe6 Bug #49102: added role dependent types
73b8556e4 Bug #49102: handle log_traceback=False also in Import-HTTP-API
44499f6a2 Bug #49102: improved error reporting during startup
00e80243f Bug #49102: refactor code and move exception handling
08bb36f01 Bug #49102: rename json schema
ac7adb7ad Bug #49102: add validation with scheme
fb4567577 Bug #49102: added user_import schema


The behavior was tested just like before.
Please look out for correct exception handling, because I had to make some changes on older commits mentioned in comment #5.
Simple rebasing or cherry-picking was not possible because most commits caused too many conflicts.
Comment 8 Tobias Wenzel univentionstaff 2021-05-14 12:16:42 CEST
QA: reopen for squash, merge & build

rebase → looks good
new values in schema → as expected
working? → as expected
exception handling → looks good
Comment 9 Toni Röhmeyer univentionstaff 2021-05-14 12:41:52 CEST
branch merged to 4.4 with the following commits:

commit f8161e7b8e33c65bf92abfe091dac98699a1fcb5 (HEAD -> 4.4)
Author: Toni Röhmeyer <roehmeyer@univention.de>
Date:   Fri May 14 12:39:42 2021 +0200

    Bug #49102: added advisory

commit 9ca6fe684df3b3226600a4fc5f26912b265b8727 (origin/HEAD, origin/4.4)
Author: Toni Röhmeyer <roehmeyer@univention.de>
Date:   Fri May 14 12:30:04 2021 +0200

    Bug #49102: added changelog entry

commit d0abd349f00aefbf3ae00de4a0f02c7d1212f2d0
Merge: 3657641a7 b9da7641f
Author: Toni Röhmeyer <roehmeyer@univention.de>
Date:   Fri May 14 12:28:21 2021 +0200

    Bug #49102: Merge branch 'troehmey/bug49102_config_type_checks' into 4.4

commit b9da7641f88a8a5d00a3168ef63ceeee9c0b5bc0 (troehmey/bug49102_config_type_checks)
Author: Toni Röhmeyer <roehmeyer@univention.de>
Date:   Thu Feb 20 14:22:46 2020 +0100

    Bug #49102: added user_import schema
    
    Bug #49102: add validation with scheme
    
    Bug #49102: refactor code and improved exception handling
    
    Bug #49102: handle log_traceback=False also in Import-HTTP-API
    
    Bug #49102: added role dependent types
    
    Bug #49102: improved exception handling


Successful build:

Package: ucs-school-import
Version: 17.0.58A~4.4.0.202105141231
Branch: ucs_4.4-0
Scope: ucs-school-4.4


I am glad, that this bug fix can finally be released :)
Comment 10 Tobias Wenzel univentionstaff 2021-05-14 13:18:25 CEST
QA

merge -> ok
changelog -> ok
yaml -> missing (not pushed)

I will verify this next week with jenkins & yaml
Comment 12 Toni Röhmeyer univentionstaff 2021-05-17 13:08:42 CEST
Created attachment 10729 [details]
configuration schema patch

added "null" as option to some fields according to manual https://docs.software-univention.de/ucsschool-import-handbuch-4.4.html#configuration:json_format:userimport
Comment 13 Tobias Wenzel univentionstaff 2021-05-17 14:33:47 CEST
Sorry for the miscommunication. 

The patch looks good: The import tests pass on my VM

[4.4] 3f5c13b84 Bug #49102: change advisory
[4.4] bde381abf Bug #49102: allow null for not set schema values


Package: ucs-school-import
Version: 17.0.59A~4.4.0.202105171429
Branch: ucs_4.4-0
Scope: ucs-school-4.4
Comment 14 Tobias Wenzel univentionstaff 2021-05-18 09:47:32 CEST
Jenkins looks happy :)
→ verify
Comment 15 Ole Schwiegert univentionstaff 2021-05-26 11:28:41 CEST
Errata updates for UCS@school 4.4 v9 have been released.

https://docs.software-univention.de/changelog-ucsschool-4.4v9-de.html

If this error occurs again, please clone this bug.