Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Honor (don't ignore) /etc/kolibri/username when installing Kolibri — putting an end to 2 contradictory KOLIBRI_USER settings #117

Merged
merged 1 commit into from
Apr 18, 2023

Conversation

holta
Copy link
Contributor

@holta holta commented Mar 27, 2023

This PR fixes the worst of several problems described on Sept 9, 2022 here:

Thanks @jredrejo @benjaoming @radinamatic for reviewing when you have time.

Recap: This fixes the serious problem of Kolibri's Debian/Ubuntu (apt / .deb) installer ignoring /etc/kolibri/username

Which results in Kolibri's 2 permanently contradictory settings (i.e. 2 contradictory values) for KOLIBRI_USER — i.e. in these 2 places:

  • /var/cache/debconf/config.dat
  • /etc/kolibri/username

Not pretty! So a month or so later, typically the first time a school apt upgrades, Kolibri's new Debian package erroneously tries to force the school (and all https://internet-in-a-box.org schools, if they are online and run apt update etc) to try to change KOLIBRI_USER:

┌─────────────────────────────────────────────────────────┤ Kolibri configuration ├─────────────────────────────────────────────────────────┐
│ The default is to choose your preferred desktop user account, for instance to ensure access to importing data from external USB devices.  │
│                                                                                                                                           │
│ Entering a username that doesn't exist will create a new system user with home directory /var/<username>.                                 │
│                                                                                                                                           │
│ Which user account should own the Kolibri server?                                                                                         │
│                                                                                                                                           │
│ ubuntu   <<<< WRONG!!! (user "kolibri" is already specified at /etc/kolibri/username)   _________________________________________________ │
│                                                                                                                                           │
│                                                                  <Ok>                                                                     │
│                                                                                                                                           │
└───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┘

Why does this happen every single time a school first upgrades Kolibri? One can only assume that Ansible (Linux automation, that arranges all Internet-in-a-Box installs) runs something like:

export DEBIAN_FRONTEND=noninteractive
apt install kolibri

So what happens is that Kolibri's Ubuntu/Debian package (1) Encodes the very misleading (erroneous, desktop-like) KOLIBRI_USER into /var/cache/debconf/config.dat (2) Effectively deferring its 3-to-5 debconf screens asking for things like KOLIBRI_USER until 1 or 2 months later, into the hands of the wrong person, after everything's installed in a school (3) Leading to serious confusion + conflicting KOLIBRI_USER settings down the road, that nobody wants 😖

CONCLUSION: This simple PR eliminates the conflicting settings, and at least brings us a lot closer to what the doc portrays at https://kolibri.readthedocs.io/en/latest/install/ubuntu-debian.html#id2 — pasted in here:

image

STRETCH GOAL FOR LATER: Eliminating all screens during apt upgrade would be far-and-away better for most schools, who are simply trying to upgrade Kolibri, and do not want to change any settings, or read long-winded technical screens 🥲 (as we all strive to help schools focus on learning, not unnecessary low-level administration!)

IN SHORT, Internet-in-a-Box can easily force far cleaner behavior during Kolibri upgrades (i.e. what schools prefer) with a hacky script like the following: (but we'd rather work with Kolibri to solve the problem properly, thoughtfully and in due course, helping everyone)

#!/bin/bash

#CONFIGFILE=/var/cache/debconf/config.dat
set -e

. /usr/share/debconf/confmodule
db_fset kolibri/user seen true
db_fset kolibri/init seen true
db_fset kolibri/init-instructions seen true

@holta
Copy link
Contributor Author

holta commented Mar 27, 2023

BACKGROUND / CONTEXT: /etc/kolibri/README puts repeated and very strong emphasis on file /etc/kolibri/username which seems very positive. ✅

While @benjaoming did most of that writing more than 4 years ago of course, I can only assume it's still largely on target.

To that end, let's all hope that this still-very-useful (though at this point error-prone and stale!) top-level /etc/kolibri doc can be refined now in 2023, to fix its broken link and other small/diversionary errors 🏗️

e.g. Another small error in /etc/kolibri/README is the command it spells out to determine KOLIBRI_HOME ("this command will print it") doesn't work in most cases — with installs like Internet-in-a-Box that take security seriously — as KOLIBRI_USER has no real / runnable shell in /etc/passwd :

@@ -346,15 +346,21 @@ kolibri_debconf_set_defaults()
db_fset kolibri/pre-010-upgrade-system-user seen true
fi

if [ "$USER_SEEN" == "false" ]
if [ -s /etc/kolibri/username ] # Test that file exists with non-zero size
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can simplify this change quite a lot by adding the test to the list of tests in line 341 AFAICT

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw that earlier, yes.

But I chose to keep the change more localized & more legible.

i.e. to make clear this PR is basically a 2-line change wrapped in an if statement.

(@jredrejo is of course free to move things around as he sees fit!)

Copy link
Member

@jredrejo jredrejo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Despite I agree with @benjaoming comment, I'm approving this because the functionality is correct and I prefer not to delay it anymore

@jredrejo jredrejo merged commit ddf3e5b into learningequality:master Apr 18, 2023
@holta
Copy link
Contributor Author

holta commented Apr 18, 2023

I too agree with @benjaoming

(PR change was kept distinct/localized to illustrate the core idea...)

@benjaoming
Copy link
Contributor

Thanks for all the lovely mentions ❤️ I want to mention back that I do miss having time to work on this ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants