Bug 42450 - Empty sources.list may result in unwanted package operations
Empty sources.list may result in unwanted package operations
Status: CLOSED FIXED
Product: UCS
Classification: Unclassified
Component: Update - univention-updater
UCS 4.1
Other Linux
: P5 normal (vote)
: UCS 4.1-3-errata
Assigned To: Philipp Hahn
Stefan Gohmann
:
Depends on:
Blocks: 42452 42481
  Show dependency treegraph
 
Reported: 2016-09-20 15:57 CEST by Dirk Wiesenthal
Modified: 2016-09-23 13:53 CEST (History)
3 users (show)

See Also:
What kind of report is it?: Bug Report
What type of bug is this?: 7: Crash: Bug causes crash or data loss
Who will be affected by this bug?: 4: Will affect most installed domains
How will those affected feel about the bug?: 4: A User would return the product
User Pain: 0.640
Enterprise Customer affected?:
School Customer affected?:
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number: 2016091921000326, 2016091921000317, 2016091821000131, 2016091721000179, 2016091521000182, 2016091321000579, 2016091321000524, 2016091321000257, 2016092021000172
Bug group (optional): Error handling, External feedback
Max CVSS v3 score:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Dirk Wiesenthal univentionstaff 2016-09-20 15:57:08 CEST
We found in /etc/apt/sources.list.d/15_ucs-online-version.list

# An error occurred during the repository check. The error message:
#   Traceback (most recent call last):
#     File "<stdin>", line 13, in <module>
#     File "/usr/lib/pymodules/python2.7/univention/updater/tools.py", line 579, in __init__
#       self.ucr_reinit()
#     File "/usr/lib/pymodules/python2.7/univention/updater/tools.py", line 660, in ucr_reinit
#       assert self.server.access(None, '')
#     File "/usr/lib/pymodules/python2.7/univention/updater/tools.py", line 488, in access
#       raise ConfigurationError(uri, reason)
#   ConfigurationError: Configuration error: host is unresolvable


Only 4.1-3-errata was included in 20_ucs-online-component.list

This resulted in the following apt-get operation set:

Statusinformationen werden eingelesen....
Die folgenden Pakete werden ENTFERNT:
  univention-management-console univention-management-console-module-setup
  univention-server-master
Die folgenden NEUEN Pakete werden installiert:
  univention-nagios-samba
Die folgenden Pakete sind zurückgehalten worden:
  python-univention-appcenter
Die folgenden Pakete werden aktualisiert (Upgrade):
  libidn11 libpython2.7 libxslt1.1 python-univention-connector-s4
  [...]

The problem came from added dependencies in univention-management-console-module-setup and python-univention-appcenter in the latest 4.1-3 errata updates. These dependencies could not be found and thus the packages were not installed / had to be removed.

The updater should not operate under these conditions. Also, the sources.list should rewrite itself. We got feedback that later on, the error messages disappeared.
Comment 1 Florian Best univentionstaff 2016-09-20 16:20:40 CEST
Some of 37146,26432,28853,38373,38810 might be duplicates.
Comment 2 Dirk Wiesenthal univentionstaff 2016-09-20 16:27:44 CEST
While the updater may stop operating when the file is (more or less) empty, one could also just write the sources.list without ever hitting the server.

Just iterate over ["4.0", "4.1"] and write it down. Would also greatly reduce the amount of time it takes to commit these files. Also applies to 20_ucs-online-component.list. The App Center sometimes struggles with this file as it takes very long.

There seems to be no benefit in hitting the server and telling that the host is unresolvable. Only if there was some kind of rollback to the last "correct" version of the file (or even a fallback), it would make sense to test the configuration.
Comment 3 Stefan Gohmann univentionstaff 2016-09-20 17:17:08 CEST
It seems to affect a lot of testers.
Comment 4 Stefan Gohmann univentionstaff 2016-09-20 17:18:51 CEST
(In reply to Stefan Gohmann from comment #3)
> It seems to affect a lot of testers.
Comment 5 Philipp Hahn univentionstaff 2016-09-20 21:15:32 CEST
(In reply to Dirk Wiesenthal from comment #2)
> While the updater may stop operating when the file is (more or less) empty,
> one could also just write the sources.list without ever hitting the server.

Wrong:
The updater (currently) has now knowledge, which previous releases exist, that is 4.1-3 needs
4.1-2
4.1-1
4.1-0
4.0-5 \
4.0-4 |
4.0-3 +  those are found only by iterating through all existing releases
4.0-2 |
4.0-1 /
4.0-0

I see two options:

1. we could hard-code the previous releases, as they are known at the point of releasing

2. we should create a single /index.json with all UCS release information (including expiry date of standard support). That way a single fetch would be sufficient and the UMC frontend could warn about expired releases.  (there alredy exists <http://updates.software-univention.de/download/ucs-maintenance/>, but 1) YAML is not browser friendly, 2) it's one file per patch-level
Comment 6 Stefan Gohmann univentionstaff 2016-09-20 21:41:48 CEST
The UMC warning about releases is planned, see the technical concept here:
 https://mail.univention.de/appsuite/ui#!!&app=io.ox/office/text&folder=1117&id=1117/1025

We have already implemented the repository restrictions in our updater lib. I guess we will lost some functionality if we add a static entry. And we need to change the version if we release a patchlevel release after the next minor release, see 4.0-5 and 4.1-0.

What about using the old sources.list entry if a traceback happened? Philipp, you said something like that today? I think that would avoid this issue.
Comment 7 Dirk Wiesenthal univentionstaff 2016-09-20 22:52:05 CEST
(In reply to Philipp Hahn from comment #5)
> (In reply to Dirk Wiesenthal from comment #2)
> > While the updater may stop operating when the file is (more or less) empty,
> > one could also just write the sources.list without ever hitting the server.
> 
> Wrong:
> The updater (currently) has now knowledge, which previous releases exist,
> that is 4.1-3 needs
> 4.1-2
> 4.1-1
> 4.1-0
> 4.0-5 \
> 4.0-4 |
> 4.0-3 +  those are found only by iterating through all existing releases
> 4.0-2 |
> 4.0-1 /
> 4.0-0
> 

Ah, right.

> 2. we should create a single /index.json with all UCS release information
> (including expiry date of standard support). That way a single fetch would
> be sufficient and the UMC frontend could warn about expired releases. 
> (there alredy exists
> <http://updates.software-univention.de/download/ucs-maintenance/>, but 1)
> YAML is not browser friendly, 2) it's one file per patch-level

Not necessarily. In fact, we could add one (extensive) file at http://updates.software-univention.de/index.json and put every release in there. 4.1-3 would know about 4.0-5 (and eventually even 4.2-0 - but skip it during the ucr commit).

One would ship one initial file in univention-updater locally, so that an internet connection is not necessary during the initial setup.

(In reply to Stefan Gohmann from comment #6)
> We have already implemented the repository restrictions in our updater lib.
> I guess we will lost some functionality if we add a static entry.

Restriction functionality? So it will result in a lot of error messages if 4.0-6 is added without permission? This would indeed be a problem. Depending on the complexity of the authentication / authorization mechanism, a simple "extended_support_only": true in the json could be enough.
Comment 8 Stefan Gohmann univentionstaff 2016-09-21 05:30:28 CEST
(In reply to Dirk Wiesenthal from comment #7)
> (In reply to Stefan Gohmann from comment #6)
> > We have already implemented the repository restrictions in our updater lib.
> > I guess we will lost some functionality if we add a static entry.
> 
> Restriction functionality? So it will result in a lot of error messages if
> 4.0-6 is added without permission? This would indeed be a problem. Depending
> on the complexity of the authentication / authorization mechanism, a simple
> "extended_support_only": true in the json could be enough.

No, the updater has to check every release separately if username and password are required.

I don't think that we talk about the root cause of the error. I guess the same issue would have been occurred if we use a json file. 

The 15sources.list is generated during the system setup. It looks like bind or the forwarder wasn't ready at that point. That would have been the same issue if we use a json file.

The 20sources.list is re-created every time the updater checks for an update. This is at least once a day but I guess it happens more often.

My suggestion is to re-create the 15sources.list also every time if it is empty or it contains only a traceback and an existing 15sources.list should not be overwritten with an empty file or a file which contains only a traceback.
Comment 9 Philipp Hahn univentionstaff 2016-09-22 16:31:06 CEST
r72760 | Bug #42450 up: Assert valid /etc/apt/sources.list.d/15_ucs-online-version.list
 Add Pre-Invoke checks

Package: univention-updater
Version: 11.0.11-9.1491.201609221623
Branch: ucs_4.1-0
Scope: errata4.1-3

r72761 | Bug #42450 up: Assert valid /etc/apt/sources.list.d/15_ucs-online-version.list YAML
 univention-updater.yaml

QA:
 repo=$(dig +short $(ucr get repository/online/server | sed -re 's,https?://,,;s,/.*,,')|tail -n1)
 ip route add prohibit "$repo"
 ucr commit /etc/apt/sources.list.d/15_ucs-online-version.list
 apt-get update # will fail
 ip route del "$repo"
 apt-get update # will succeed

 ip route add prohibit "$repo"
 ucr commit /etc/apt/sources.list.d/15_ucs-online-version.list
 apt-get remove vim # will fail
 ip route del "$repo"
 apt-get remove vim # will fail
 ucr commit /etc/apt/sources.list.d/15_ucs-online-version.list
 apt-get remove vim # will succeed
Comment 10 Stefan Gohmann univentionstaff 2016-09-23 07:19:48 CEST
Code review: OK

Merge to UCS 4.2: Fail, I've cloned it into Bug #42481

Tests: OK

YAML: OK

ucs-test: OK
Comment 11 Janek Walkenhorst univentionstaff 2016-09-23 13:53:18 CEST
<http://errata.software-univention.de/ucs/4.1/277.html>