Bug 45972 - global school-import config prevents undefining scheme
global school-import config prevents undefining scheme
Status: CLOSED FIXED
Product: UCS@school
Classification: Unclassified
Component: Import scripts
UCS@school 4.2
Other Linux
: P5 normal (vote)
: UCS@school 4.2 v9
Assigned To: Daniel Tröder
Jürn Brodersen
:
: 45997 (view as bug list)
Depends on:
Blocks: 46923
  Show dependency treegraph
 
Reported: 2018-01-03 12:10 CET by Daniel Tröder
Modified: 2018-06-04 15:34 CEST (History)
3 users (show)

See Also:
What kind of report is it?: Bug Report
What type of bug is this?: 3: Simply Wrong: The implementation doesn't match the docu
Who will be affected by this bug?: 1: Will affect a very 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.034
Enterprise Customer affected?:
School Customer affected?: Yes
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 Daniel Tröder univentionstaff 2018-01-03 12:10:17 CET
The schemes for email and username in the default legacy import configuration don't make sense (self referencing) and now lead to a failing test:
http://jenkins.knut.univention.de:8080/job/UCSschool%204.2/job/UCSschool%204.2%20Singleserver/lastSuccessfulBuild/ImportTests=NoImportTests,SambaVersion=s4-all-components/testReport/junit/90_ucsschool/102_rename_class/test/

Fix user_import_legacy_defaults.json.
Comment 1 Daniel Tröder univentionstaff 2018-01-03 12:16:42 CET
[4.2 323e8f2b] Bug #45972: fix self reference in schemes of legacy import

ucs-school-import 15.0.3-21A~4.2.0.201801031214

90_ucsschool/34_import-users-legacy still works.
Comment 3 Daniel Tröder univentionstaff 2018-01-08 10:46:59 CET
line 773, in run_checks
    raise InvalidEmail("Email address '{}' has invalid format.".format(self.email), entry_count=self.entry_count, import_user=self)
InvalidEmail: Email address '@' has invalid format.


This happens when the email address in the CSV input is empty, and thus the calculation from scheme is tried, but the scheme for the "email" property is "<email>" (which makes no sense).
Comment 4 Daniel Tröder univentionstaff 2018-01-08 10:48:58 CET
The new scheme in the default legacy configuration is the same now as in the other (non-legacy) default/example configurations.
Comment 5 Daniel Tröder univentionstaff 2018-01-08 10:58:52 CET
Remove email und username from scheme, as in the legacy import:
* email should be empty if not set
* username is required in the input
Comment 6 Daniel Tröder univentionstaff 2018-01-08 13:41:24 CET
[4.2 6ff44fc0] Bug #45972: legacy import requires the username in the input and should produce an empty email address when the input email field is empty

ucs-school-import 15.0.3-22A~4.2.0.201801081338
Comment 7 Sönke Schwardt-Krummrich univentionstaff 2018-01-10 13:38:23 CET
At a customer, I had to remove "email" from user_import_defaults.json: 

--- user_import_defaults.json	2018-01-10 09:21:11.424875917 +0100
+++ user_import_defaults.json.ORIG	2018-01-10 09:20:17.836784954 +0100
@@ -20,6 +20,7 @@
 		"deletion": 0
 	},
 	"scheme": {
+		"email": "<firstname>[0].<lastname>@<maildomain>",
 		"recordUID": "<email>",
 		"username": {
 			"default": "<:umlauts><firstname>[0].<lastname><:lower>[COUNTER2]"
Comment 9 Daniel Tröder univentionstaff 2018-01-11 10:29:04 CET
*** Bug 45997 has been marked as a duplicate of this bug. ***
Comment 11 Daniel Tröder univentionstaff 2018-01-12 13:23:40 CET
The global user import configuration (user_import_defaults.json) does not contain any scheme anymore.

A test was added (203_import-users_empty_email_scheme) that runs 2*2 imports: in legacy and non-legacy mode each an import with and without email addresses in the input CSV data.

[dtroeder/45972_global_config 65cf32c6] Bug #45972: do not set any scheme in global defaults
[dtroeder/45972_global_config b016277a] Bug #45972: test importing users with empty email scheme
[dtroeder/45972_global_config c6a44db0] Bug #45972: changelog
[4.2 ece39949] Bug #45972: advisory

ucs-school-import 15.0.3-23
ucs-test-ucsschool 4.0.4-54
Comment 12 Jürn Brodersen univentionstaff 2018-01-18 10:46:32 CET
ucs-test-ucsschool depends on python-pytest which depends on python-py (>=.4.29)

But only python-py 1.4.25 is in our repository.
Comment 13 Daniel Tröder univentionstaff 2018-01-18 13:25:29 CET
There was a leftover from Bug #45998 - sorry.

I removed it. Please downgrade python-pytest to 2.6.3-2.
Comment 14 Jürn Brodersen univentionstaff 2018-01-19 14:50:30 CET
Looks good.

What I tested:
./203_import-users_email_scheme -f -> OK
./203_import-users_empty_email_scheme -f -> OK
Adding users with mail, without mail and mail schemes -> OK

I think it should be (better) documented that the configurations are updated recursively. Meaning that config keys can not be removed only added or modified. Like in this case the mail scheme.

I will add a separate bug for that.

-> Verified
Comment 15 Sönke Schwardt-Krummrich univentionstaff 2018-02-06 15:34:48 CET
Some wishes/questions for the YAML advisory:
- Please mention the full path to the affected json file
- Are there any scenario where this change will have negative side effects?
- Does the user have to alter its own config 
  (e.g. /var/lib/ucs-school-import/configs/user_import.json) to compensate the 
  modification of this bug?
Comment 16 Daniel Tröder univentionstaff 2018-02-13 13:38:44 CET
[4.2 403e1a9d] Bug #45972: enhance advisory
Comment 17 Jürn Brodersen univentionstaff 2018-02-19 14:36:03 CET
befc4e7f, 342529f7: YAML formating

The changelog does not build:
http://jenkins.knut.univention.de:8080/job/UCSschool%204.2/job/ReleaseNotesPreview/2/console

See also bug 46337 for how to use that job.

I see two options:
Edit the changelog directly.
Add the json part to the import documentation and link to it.
Comment 18 Daniel Tröder univentionstaff 2018-02-19 16:43:45 CET
I replaced <simpara> by <para>, then it built.

--- create_app_changelog.ori	2018-02-19 15:33:34.673311354 +0100
+++ create_app_changelog	2018-02-19 15:33:46.317452191 +0100
@@ -213,7 +213,7 @@
 						if entry not in ('This update addresses the following issue(s):',):
 							log.error('SKIP: cannot find bug number in %r in headline %r', entry, section_title)
 					else:
-						itemlist.append(u'<listitem><simpara>%s</simpara></listitem>\n' % (entry,))
+						itemlist.append(u'<listitem><para>%s</para></listitem>\n' % (entry,))
 			if itemlist:
 				data = {
 					'sectionid': create_section_id(section_title),

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

But I had to edit the resulting changelog-ucsschool-4.2v8-de.xml manually anyway, because the create_app_changelog script does not yet handle <programlisting> entries correctly: it removes the indentation.
Of cause < and > had to be replaced by &lt; and &gt; :)

I think the resulting changelog in build #8 is OK.

We'll have to either teach create_app_changelog how to handle <programlisting> or manually fix it's output in the rare cases we need it.
Comment 19 Jürn Brodersen univentionstaff 2018-02-26 10:46:24 CET
OK,
currently the changelog has a page break in the code section. But until everything else is checked in for the next release that might change anyways.

-> Verified
Comment 20 Sönke Schwardt-Krummrich univentionstaff 2018-05-02 17:53:00 CEST
UCS@school 4.2 v9 has been released.

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

If this error occurs again, please clone this bug.