From 7b2a65d449112ab32c437d29e0b3d74d191cba42 Mon Sep 17 00:00:00 2001 From: Sunil Mohan Adapa Date: Tue, 13 Feb 2024 11:36:14 -0800 Subject: [PATCH 01/18] container: Fix issue with missing make command on stable image Fixes: #2402. Signed-off-by: Sunil Mohan Adapa Reviewed-by: James Valleroy --- container | 1 + 1 file changed, 1 insertion(+) diff --git a/container b/container index 08873e84f..f1cc8379d 100755 --- a/container +++ b/container @@ -159,6 +159,7 @@ set -xe pipefail cd /freedombox/ +sudo apt-get -y install make sudo make provision-dev echo 'alias freedombox-develop="cd /freedombox; sudo -u plinth /freedombox/run --develop"' \ From 29d48e86b77894b7c1ae62dfb6f07ea9e2449907 Mon Sep 17 00:00:00 2001 From: James Valleroy Date: Sat, 10 Feb 2024 16:44:28 -0500 Subject: [PATCH 02/18] setup: Try force upgrade before running app setup Avoid running setup if it would bypass a needed force upgrade. Fixes: #2397 Tests: - Rerun setup on an app and see that there are no errors. - Install modified freedombox on bookworm and perform dist-upgrade to testing. Then rerun setup on Firewall app. It fails with the message "App firewall failed to force upgrade." firewalld package is not upgraded. - Modify Firewall app to allow force upgrade to latest version. Then rerun setup on Firewall app. firewalld is successfully force upgraded. NOTE: In this case, Firewall setup is run twice, once by force upgrade, and again by rerun setup. Signed-off-by: James Valleroy Reviewed-by: Sunil Mohan Adapa --- plinth/setup.py | 53 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/plinth/setup.py b/plinth/setup.py index 04c0dfd3f..fb4b1ba45 100644 --- a/plinth/setup.py +++ b/plinth/setup.py @@ -54,10 +54,19 @@ def run_setup_on_app(app_id, allow_install=True, rerun=False): def _run_setup_on_app(app, current_version): """Execute the setup process.""" + global _force_upgrader logger.info('Setup run: %s', app.app_id) exception_to_update = None message = None try: + if not _force_upgrader: + _force_upgrader = ForceUpgrader() + + # Check if this app needs force_upgrade. If it is needed, but not yet + # supported for the new version of the package, then an exception will + # be raised, so that we do not run setup. + _force_upgrader.attempt_upgrade_for_app(app.app_id) + current_version = app.get_setup_version() app.setup(old_version=current_version) app.set_setup_version(app.info.version) @@ -448,6 +457,50 @@ class ForceUpgrader(): if need_retry: raise self.TemporaryFailure('Some apps failed to force upgrade.') + def attempt_upgrade_for_app(self, app_id): + """Attempt to perform an upgrade for specified app. + + Raise TemporaryFailure if upgrade can't be performed now. + + Raise PermanentFailure if upgrade can't be performed until something + with the system state changes. We don't want to try again until + notified of further package cache changes. + + Return True if upgrade was performed successfully. + + Return False if upgrade is not needed. + + """ + if _is_shutting_down: + raise self.PermanentFailure('Service is shutting down') + + if packages_privileged.is_package_manager_busy(): + raise self.TemporaryFailure('Package manager is busy') + + apps = self._get_list_of_apps_to_force_upgrade() + if app_id not in apps: + logger.info('App %s does not need force upgrade', app_id) + return False + + packages = apps[app_id] + app = app_module.App.get(app_id) + try: + logger.info('Force upgrading app: %s', app.info.name) + if app.force_upgrade(packages): + logger.info('Successfully force upgraded app: %s', + app.info.name) + return True + else: + logger.warning('Ignored force upgrade for app: %s', + app.info.name) + raise self.TemporaryFailure( + 'Force upgrade is needed, but not yet implemented for new ' + f'version of app: {app_id}') + except Exception as exception: + logger.exception('Error running force upgrade: %s', exception) + raise self.TemporaryFailure( + f'App {app_id} failed to force upgrade.') + def _run_force_upgrade_as_operation(self, app, packages): """Start an operation for force upgrading.""" name = gettext_noop('Updating app packages') From 56b58174b3e3ff96c27e90b2f1b92735a89e5faa Mon Sep 17 00:00:00 2001 From: Sunil Mohan Adapa Date: Wed, 14 Feb 2024 15:23:38 -0800 Subject: [PATCH 03/18] setup: Minor refactoring of force upgrader class instantiation - It is okay to instantiate before shutdown to mark shutdown initiation. - Still keep the instantiating lazy. Tests: - Tested as part of patch series. Signed-off-by: Sunil Mohan Adapa Reviewed-by: James Valleroy --- plinth/setup.py | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/plinth/setup.py b/plinth/setup.py index fb4b1ba45..97cbfd89e 100644 --- a/plinth/setup.py +++ b/plinth/setup.py @@ -26,8 +26,6 @@ _is_first_setup = False is_first_setup_running = False _is_shutting_down = False -_force_upgrader = None - def run_setup_on_app(app_id, allow_install=True, rerun=False): """Execute the setup process in a thread.""" @@ -54,18 +52,16 @@ def run_setup_on_app(app_id, allow_install=True, rerun=False): def _run_setup_on_app(app, current_version): """Execute the setup process.""" - global _force_upgrader logger.info('Setup run: %s', app.app_id) exception_to_update = None message = None try: - if not _force_upgrader: - _force_upgrader = ForceUpgrader() + force_upgrader = ForceUpgrader.get_instance() # Check if this app needs force_upgrade. If it is needed, but not yet # supported for the new version of the package, then an exception will # be raised, so that we do not run setup. - _force_upgrader.attempt_upgrade_for_app(app.app_id) + force_upgrader.attempt_upgrade_for_app(app.app_id) current_version = app.get_setup_version() app.setup(old_version=current_version) @@ -152,8 +148,8 @@ def stop(): global _is_shutting_down _is_shutting_down = True - if _force_upgrader: - _force_upgrader.shutdown() + force_upgrader = ForceUpgrader.get_instance() + force_upgrader.shutdown() def setup_apps(app_ids=None, essential=False, allow_install=True): @@ -238,8 +234,8 @@ def _get_apps_for_regular_setup(): 1. essential apps that are not up-to-date 2. non-essential app that are installed and need updates """ - if (app.info.is_essential and - app.get_setup_state() != app_module.App.SetupState.UP_TO_DATE): + if (app.info.is_essential and app.get_setup_state() + != app_module.App.SetupState.UP_TO_DATE): return True if app.get_setup_state() == app_module.App.SetupState.NEEDS_UPDATE: @@ -344,6 +340,7 @@ class ForceUpgrader(): """ + _instance = None _run_lock = threading.Lock() _wait_event = threading.Event() @@ -358,6 +355,14 @@ class ForceUpgrader(): """Raised when upgrade fails and there is nothing more we wish to do. """ + @classmethod + def get_instance(cls): + """Return a single instance of a the class.""" + if not cls._instance: + cls._instance = ForceUpgrader() + + return cls._instance + def __init__(self): """Initialize the force upgrader.""" if plinth.cfg.develop: @@ -410,7 +415,7 @@ class ForceUpgrader(): def shutdown(self): """If we are sleeping for next attempt, cancel it. - If we are actually upgrading packages, don nothing. + If we are actually upgrading packages, do nothing. """ self._wait_event.set() @@ -576,8 +581,5 @@ class ForceUpgrader(): def on_package_cache_updated(): """Called by D-Bus service when apt package cache is updated.""" - global _force_upgrader - if not _force_upgrader: - _force_upgrader = ForceUpgrader() - - _force_upgrader.on_package_cache_updated() + force_upgrader = ForceUpgrader.get_instance() + force_upgrader.on_package_cache_updated() From e208bdbbcafc7f6e1268bd153efb38567adab124 Mon Sep 17 00:00:00 2001 From: Sunil Mohan Adapa Date: Wed, 14 Feb 2024 15:47:41 -0800 Subject: [PATCH 04/18] setup: Ensure that force upgrade won't run when app is not installed - This keep the initial setup of the app simple and efficient. Setup will be less prone to any issues in force upgrade code. There are also fewer chances for immediate regressions. Tests: - Tested as part of the patch series. Signed-off-by: Sunil Mohan Adapa Reviewed-by: James Valleroy --- plinth/setup.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/plinth/setup.py b/plinth/setup.py index 97cbfd89e..4c3581786 100644 --- a/plinth/setup.py +++ b/plinth/setup.py @@ -56,14 +56,15 @@ def _run_setup_on_app(app, current_version): exception_to_update = None message = None try: - force_upgrader = ForceUpgrader.get_instance() - - # Check if this app needs force_upgrade. If it is needed, but not yet - # supported for the new version of the package, then an exception will - # be raised, so that we do not run setup. - force_upgrader.attempt_upgrade_for_app(app.app_id) - current_version = app.get_setup_version() + + if current_version != 0: + # Check if this app needs force_upgrade. If it is needed, but not + # yet supported for the new version of the package, then an + # exception will be raised, so that we do not run setup. + force_upgrader = ForceUpgrader.get_instance() + force_upgrader.attempt_upgrade_for_app(app.app_id) + app.setup(old_version=current_version) app.set_setup_version(app.info.version) post_setup.send_robust(sender=app.__class__, module_name=app.app_id) From bd4ddcf1584352a4550f0114181422c2deed37b0 Mon Sep 17 00:00:00 2001 From: Sunil Mohan Adapa Date: Wed, 14 Feb 2024 16:46:43 -0800 Subject: [PATCH 05/18] setup: Ensure that apt is updated before checking force upgrade If the package cache is outdated and a check is performed for whether force upgrade is needed, it might return negative. After the operation proceeds to run setup, we perform 'apt update' before 'apt install' at this point the configuration file prompt will cause failure. Tests: - Re-run firewalld upgrade from 1.3.x to 2.1.x with a re-run setup. Signed-off-by: Sunil Mohan Adapa Reviewed-by: James Valleroy --- plinth/setup.py | 1 + 1 file changed, 1 insertion(+) diff --git a/plinth/setup.py b/plinth/setup.py index 4c3581786..edf9e54f9 100644 --- a/plinth/setup.py +++ b/plinth/setup.py @@ -62,6 +62,7 @@ def _run_setup_on_app(app, current_version): # Check if this app needs force_upgrade. If it is needed, but not # yet supported for the new version of the package, then an # exception will be raised, so that we do not run setup. + package.refresh_package_lists() force_upgrader = ForceUpgrader.get_instance() force_upgrader.attempt_upgrade_for_app(app.app_id) From 5e10b2d4ae62966680f67e811a9bdc289a98d57a Mon Sep 17 00:00:00 2001 From: Sunil Mohan Adapa Date: Wed, 14 Feb 2024 16:50:09 -0800 Subject: [PATCH 06/18] firewalld: Implement force upgrading to any 2.x versions Closes: #2396. New configuration options were introduced from 1.3.x to 2.1.x. This cause configuration file prompt due to our existing changes to the configuration file. Changes to the configuration file were investigated in #2396. Tests: - Install firewalld 1.3.x. Ensure that firewalld configuration changes are present as intended by FreedomBox. Change priority of the .deb package to allow installing 2.1.x. Run apt update and notice that force upgrade has been performed to 2.1.x. - firewalld upgrade has also been tested as part of this patch series. Signed-off-by: Sunil Mohan Adapa Reviewed-by: James Valleroy --- plinth/modules/firewall/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plinth/modules/firewall/__init__.py b/plinth/modules/firewall/__init__.py index df49e8876..3cd45ea7c 100644 --- a/plinth/modules/firewall/__init__.py +++ b/plinth/modules/firewall/__init__.py @@ -87,9 +87,9 @@ class FirewallApp(app_module.App): if 'firewalld' not in packages: return False - # Allow upgrade from any version to any version below 2.0 + # Allow upgrade from any version to any version below 3.0 package = packages['firewalld'] - if Version(package['new_version']) > Version('2~'): + if Version(package['new_version']) > Version('3~'): return False install(['firewalld'], force_configuration='new') From 6c5fbf652319215e4cd76e2bc7d7dd262df7404a Mon Sep 17 00:00:00 2001 From: James Valleroy Date: Fri, 16 Feb 2024 17:37:08 -0500 Subject: [PATCH 07/18] tests: Patch apps_init for enable/disable daemon test Otherwise, apps_init calls _sort_apps, which randomly fails in functional tests CI job. The failure may be due to the tests running in parallel, since I could not reproduce it locally. Signed-off-by: James Valleroy Reviewed-by: Sunil Mohan Adapa --- plinth/tests/test_daemon.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/plinth/tests/test_daemon.py b/plinth/tests/test_daemon.py index fd745e8ca..7051d6fe3 100644 --- a/plinth/tests/test_daemon.py +++ b/plinth/tests/test_daemon.py @@ -77,10 +77,11 @@ def test_is_enabled(service_is_enabled, daemon): service_is_enabled.assert_has_calls([call('test-unit', strict_check=True)]) +@patch('plinth.app.apps_init') @patch('subprocess.run') @patch('subprocess.call') -def test_enable(subprocess_call, subprocess_run, app_list, mock_privileged, - daemon): +def test_enable(subprocess_call, subprocess_run, apps_init, app_list, + mock_privileged, daemon): """Test that enabling the daemon works.""" daemon.enable() subprocess_call.assert_has_calls( @@ -101,9 +102,11 @@ def test_enable(subprocess_call, subprocess_run, app_list, mock_privileged, stdout=subprocess.DEVNULL, check=False) +@patch('plinth.app.apps_init') @patch('subprocess.run') @patch('subprocess.call') -def test_disable(subprocess_call, subprocess_run, mock_privileged, daemon): +def test_disable(subprocess_call, subprocess_run, apps_init, app_list, + mock_privileged, daemon): """Test that disabling the daemon works.""" daemon.disable() subprocess_call.assert_has_calls( From 71a10bfd31d96c40db9d9c949753ff1cbcc8e6ef Mon Sep 17 00:00:00 2001 From: Sunil Mohan Adapa Date: Thu, 15 Feb 2024 10:27:11 -0800 Subject: [PATCH 08/18] backups: tests: Don't use pytest marks on fixtures - It removes this warning. plinth/modules/backups/tests/test_ssh_remotes.py:62: PytestRemovedIn9Warning: Marks applied to fixtures have no effect. Signed-off-by: Sunil Mohan Adapa Reviewed-by: James Valleroy --- plinth/modules/backups/tests/test_ssh_remotes.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/plinth/modules/backups/tests/test_ssh_remotes.py b/plinth/modules/backups/tests/test_ssh_remotes.py index 083ec5a47..cae0c2f36 100644 --- a/plinth/modules/backups/tests/test_ssh_remotes.py +++ b/plinth/modules/backups/tests/test_ssh_remotes.py @@ -59,9 +59,8 @@ def fixture_create_temp_user(temp_home, password, needs_root): subprocess.check_call(['sudo', 'userdel', username]) -@pytest.mark.usefixtures('needs_sudo') @pytest.fixture(name='has_ssh_key', scope='module', autouse=True) -def fixture_ssh_key(temp_home, temp_user, password, needs_root): +def fixture_ssh_key(temp_home, temp_user, password, needs_root, needs_sudo): subprocess.check_call([ 'sudo', '-n', '-u', temp_user, 'ssh-keygen', '-t', 'rsa', '-b', '1024', '-N', '', '-f', f'{temp_home}/.ssh/id_rsa', '-q' From 37b9e21e3058732c12bdf447cc1db2e74ea2495f Mon Sep 17 00:00:00 2001 From: Sunil Mohan Adapa Date: Thu, 15 Feb 2024 10:28:50 -0800 Subject: [PATCH 09/18] tor: tests: Fix issue with pytest 8.x versions Closes: https://bugs.debian.org/1063968. - Due to a changed behavior in pytest 8.x, any imports with 'setup_module' name will be treated as a method to setup the module in the style of unittest/nose. pytest tries to call this as a method and will fail. - Rename the import to 'setup_module_' instead of 'setup_module' to fix this issue. Signed-off-by: Sunil Mohan Adapa Reviewed-by: James Valleroy --- plinth/modules/tor/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plinth/modules/tor/__init__.py b/plinth/modules/tor/__init__.py index dfc8c48de..541ff9e92 100644 --- a/plinth/modules/tor/__init__.py +++ b/plinth/modules/tor/__init__.py @@ -10,7 +10,7 @@ from django.utils.translation import gettext_noop from plinth import action_utils from plinth import app as app_module from plinth import cfg, kvstore, menu -from plinth import setup as setup_module +from plinth import setup as setup_module_ # Not setup_module, for pytest from plinth.daemon import (Daemon, app_is_running, diagnose_netcat, diagnose_port_listening) from plinth.modules import torproxy @@ -209,7 +209,7 @@ class TorApp(app_module.App): kvstore.set(torproxy.PREINSTALL_CONFIG_KEY, json.dumps(config)) # This creates the operation, which will run after the current # operation (Tor setup) is completed. - setup_module.run_setup_on_app('torproxy') + setup_module_.run_setup_on_app('torproxy') if not old_version: logger.info('Enabling Tor app') From 3aae4b39d6a6d0263c8cc001ee644f16ff5f4730 Mon Sep 17 00:00:00 2001 From: Sunil Mohan Adapa Date: Thu, 15 Feb 2024 10:33:45 -0800 Subject: [PATCH 10/18] tor: tests: Convert to pytest style tests from class based tests Signed-off-by: Sunil Mohan Adapa Reviewed-by: James Valleroy --- plinth/modules/tor/tests/test_tor.py | 104 +++++++++++++-------------- 1 file changed, 48 insertions(+), 56 deletions(-) diff --git a/plinth/modules/tor/tests/test_tor.py b/plinth/modules/tor/tests/test_tor.py index 3c61a57b6..260672728 100644 --- a/plinth/modules/tor/tests/test_tor.py +++ b/plinth/modules/tor/tests/test_tor.py @@ -11,72 +11,64 @@ from django.core.exceptions import ValidationError from plinth.modules.tor import forms, utils -class TestTor: - """Test cases for testing the Tor module.""" +@patch('plinth.app.App.get') +@pytest.mark.usefixtures('needs_root', 'load_cfg') +def test_get_status(_app_get): + """Test that get_status does not raise any unhandled exceptions. - @staticmethod - @patch('plinth.app.App.get') - @pytest.mark.usefixtures('needs_root', 'load_cfg') - def test_get_status(_app_get): - """Test that get_status does not raise any unhandled exceptions. - - This should work regardless of whether tor is installed, or - /etc/tor/instances/plinth/torrc exists. - """ - utils.get_status() + This should work regardless of whether tor is installed, or + /etc/tor/instances/plinth/torrc exists. + """ + utils.get_status() -class TestTorForm: - """Test whether Tor configration form works.""" +def test_bridge_validator(): + """Test upstream bridges' form field validator.""" + validator = forms.bridges_validator - @staticmethod - def test_bridge_validator(): - """Test upstream bridges' form field validator.""" - validator = forms.bridges_validator + # Just IP:port + validator('73.237.165.184:9001') + validator('73.237.165.184') + validator('[2001:db8:85a3:8d3:1319:8a2e:370:7348]:443') + validator('[2001:db8:85a3:8d3:1319:8a2e:370:7348]') - # Just IP:port - validator('73.237.165.184:9001') - validator('73.237.165.184') - validator('[2001:db8:85a3:8d3:1319:8a2e:370:7348]:443') - validator('[2001:db8:85a3:8d3:1319:8a2e:370:7348]') + # With fingerprint + validator('73.237.165.184:9001 ' + '0D04F10F497E68D2AF32375BB763EC3458A908C8') - # With fingerprint - validator('73.237.165.184:9001 ' - '0D04F10F497E68D2AF32375BB763EC3458A908C8') + # With transport type + validator('obfs4 73.237.165.184:9001 ' + '0D04F10F497E68D2AF32375BB763EC3458A908C8') - # With transport type - validator('obfs4 73.237.165.184:9001 ' - '0D04F10F497E68D2AF32375BB763EC3458A908C8') + # With transport type and extra options + validator('obfs4 10.1.1.1:30000 ' + '0123456789ABCDEF0123456789ABCDEF01234567 ' + 'cert=A/b+1 iat-mode=0') - # With transport type and extra options - validator('obfs4 10.1.1.1:30000 ' - '0123456789ABCDEF0123456789ABCDEF01234567 ' - 'cert=A/b+1 iat-mode=0') + # Leading, trailing spaces and empty lines + validator('\n' + ' \n' + '73.237.165.184:9001 ' + '0D04F10F497E68D2AF32375BB763EC3458A908C8' + ' \n' + '73.237.165.184:9001 ' + '0D04F10F497E68D2AF32375BB763EC3458A908C8' + ' \n' + '\n') - # Leading, trailing spaces and empty lines - validator('\n' - ' \n' - '73.237.165.184:9001 ' - '0D04F10F497E68D2AF32375BB763EC3458A908C8' - ' \n' - '73.237.165.184:9001 ' - '0D04F10F497E68D2AF32375BB763EC3458A908C8' - ' \n' - '\n') + # Invalid number for parts + with pytest.raises(ValidationError): + validator(' ') - # Invalid number for parts - with pytest.raises(ValidationError): - validator(' ') + # Invalid IP address/port + with pytest.raises(ValidationError): + validator('73.237.165.384:9001') - # Invalid IP address/port - with pytest.raises(ValidationError): - validator('73.237.165.384:9001') + with pytest.raises(ValidationError): + validator('73.237.165.184:90001') - with pytest.raises(ValidationError): - validator('73.237.165.184:90001') + with pytest.raises(ValidationError): + validator('[a2001:db8:85a3:8d3:1319:8a2e:370:7348]:443') - with pytest.raises(ValidationError): - validator('[a2001:db8:85a3:8d3:1319:8a2e:370:7348]:443') - - with pytest.raises(ValidationError): - validator('[2001:db8:85a3:8d3:1319:8a2e:370:7348]:90443') + with pytest.raises(ValidationError): + validator('[2001:db8:85a3:8d3:1319:8a2e:370:7348]:90443') From 5399efde733a30a4e636ac9abcb9ecdfb77e3dab Mon Sep 17 00:00:00 2001 From: Sunil Mohan Adapa Date: Tue, 13 Feb 2024 16:24:36 -0800 Subject: [PATCH 11/18] pyproject.toml: Exclude the build directory from mypy checks - build/ directory is generated after the 'make build install' step and contains files that are duplicates of the main source leading to a failure. Signed-off-by: Sunil Mohan Adapa Reviewed-by: James Valleroy --- pyproject.toml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pyproject.toml b/pyproject.toml index 322a0dfff..2a035450f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -201,6 +201,9 @@ disable = [ "too-many-ancestors", # Easy to hit when using Django ] +[tool.mypy] +exclude = "build/" + # Ignore missing type stubs for some libraries. Try to keep this list minimal # and use type annotations where available. [[tool.mypy.overrides]] From d32d02ecb5140e4ba834a52ff8e3e64fd0c6ad35 Mon Sep 17 00:00:00 2001 From: Sunil Mohan Adapa Date: Tue, 13 Feb 2024 16:26:29 -0800 Subject: [PATCH 12/18] gitweb, users: Minor fixes for newer pycodestyle Signed-off-by: Sunil Mohan Adapa Reviewed-by: James Valleroy --- plinth/modules/gitweb/tests/test_views.py | 2 +- plinth/modules/users/tests/test_views.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/plinth/modules/gitweb/tests/test_views.py b/plinth/modules/gitweb/tests/test_views.py index 1bf20ffec..eae5da05f 100644 --- a/plinth/modules/gitweb/tests/test_views.py +++ b/plinth/modules/gitweb/tests/test_views.py @@ -53,7 +53,7 @@ def gitweb_patch(): with patch('plinth.modules.gitweb.get_repo_list') as get_repo_list, \ patch('plinth.app.App.get') as app_get, \ patch(f'{privileged}.create_repo'), \ - patch(f'{privileged}.repo_exists') as repo_exists,\ + patch(f'{privileged}.repo_exists') as repo_exists, \ patch(f'{privileged}.repo_info') as repo_info, \ patch(f'{privileged}.rename_repo'), \ patch(f'{privileged}.set_repo_description'), \ diff --git a/plinth/modules/users/tests/test_views.py b/plinth/modules/users/tests/test_views.py index badb05c59..ec4751beb 100644 --- a/plinth/modules/users/tests/test_views.py +++ b/plinth/modules/users/tests/test_views.py @@ -72,9 +72,9 @@ def make_request(request, view, as_admin=True, **kwargs): request.user = admin_user if as_admin else user with patch('plinth.modules.users.forms.is_user_admin', - return_value=as_admin),\ + return_value=as_admin), \ patch('plinth.modules.users.views.is_user_admin', - return_value=as_admin),\ + return_value=as_admin), \ patch('plinth.modules.users.views.update_session_auth_hash'): response = view(request, **kwargs) From 02e409a3a1584d59e7f9f03276398b49a721752d Mon Sep 17 00:00:00 2001 From: Sunil Mohan Adapa Date: Tue, 13 Feb 2024 16:27:39 -0800 Subject: [PATCH 13/18] daemon: Add new component for daemons shared across apps - This is useful for managing redis service needed by the upcoming Nextcloud app. - Disable the daemon only if all the apps using it are disabled. Enable it when even one of the them is enabled. - The component is not a 'leader' component as it does not decide the enabled/disabled status of the app. Tests: - Unit tests pass. - Install zoph and wordpress with full patch series. If one of the apps is disabled, mysql service is still enabled and running. If both apps are disabled, then mysql service is disabled and not running. Enabled/disabled status of apps are accurate after they are enabled/disabled. Signed-off-by: Sunil Mohan Adapa Reviewed-by: James Valleroy --- doc/dev/reference/components/daemon.rst | 3 ++ plinth/daemon.py | 35 ++++++++++++++++ plinth/tests/test_daemon.py | 55 ++++++++++++++++++++++++- 3 files changed, 92 insertions(+), 1 deletion(-) diff --git a/doc/dev/reference/components/daemon.rst b/doc/dev/reference/components/daemon.rst index ff6dd4664..69ef72786 100644 --- a/doc/dev/reference/components/daemon.rst +++ b/doc/dev/reference/components/daemon.rst @@ -8,3 +8,6 @@ Daemon .. autoclass:: plinth.daemon.RelatedDaemon :members: + +.. autoclass:: plinth.daemon.SharedDaemon + :members: diff --git a/plinth/daemon.py b/plinth/daemon.py index 88750ece0..24844f1be 100644 --- a/plinth/daemon.py +++ b/plinth/daemon.py @@ -135,6 +135,41 @@ class RelatedDaemon(app.FollowerComponent): self.unit = unit +class SharedDaemon(Daemon): + """Component to manage a daemon that is used by multiple apps. + + Daemons such as a database server are a hard requirement for an app. + However, there may be multiple apps using that server. This component + ensures that server is enabled and running when app is enabled. It runs + diagnostics on the daemon when app is diagnosed. The primary difference + from the Daemon component is that when the app is disabled the daemon must + only be disabled if there is no other app using this daemon. + """ + + # A shared daemon may be running even when an app is disabled because + # another app might be using the daemon. Hence, the enabled/disabled state + # of this component can't be used to determine the enabled/disabled state + # of the app. + is_leader = False + + def set_enabled(self, enabled): + """Do nothing. Enabled state is still determined by unit status.""" + + def disable(self): + """Disable the daemon iff this is the last app using the daemon.""" + other_apps_enabled = False + for other_app in app.App.list(): + if other_app.app_id == self.app_id: + continue + + for component in other_app.get_components_of_type(SharedDaemon): + if component.unit == self.unit and other_app.is_enabled(): + other_apps_enabled = True + + if not other_apps_enabled: + super().disable() + + def app_is_running(app_): """Return whether all the daemons in the app are running.""" for component in app_.components.values(): diff --git a/plinth/tests/test_daemon.py b/plinth/tests/test_daemon.py index 7051d6fe3..9dabdf092 100644 --- a/plinth/tests/test_daemon.py +++ b/plinth/tests/test_daemon.py @@ -10,7 +10,7 @@ from unittest.mock import Mock, call, patch import pytest from plinth.app import App, FollowerComponent, Info -from plinth.daemon import (Daemon, RelatedDaemon, app_is_running, +from plinth.daemon import (Daemon, RelatedDaemon, SharedDaemon, app_is_running, diagnose_netcat, diagnose_port_listening) from plinth.modules.diagnostics.check import DiagnosticCheck, Result @@ -332,3 +332,56 @@ def test_related_daemon_initialization(): with pytest.raises(ValueError): RelatedDaemon(None, 'test-daemon') + + +def test_shared_daemon_leader(): + """Test that shared daemon is not a leader component.""" + component1 = SharedDaemon('test-component1', 'test-daemon') + assert not component1.is_leader + + +@patch('plinth.action_utils.service_is_enabled') +def test_shared_daemon_set_enabled(service_is_enabled): + """Test that enabled status is determined by unit status.""" + component = SharedDaemon('test-component', 'test-daemon') + + service_is_enabled.return_value = False + component.set_enabled(False) + assert not component.is_enabled() + component.set_enabled(True) + assert not component.is_enabled() + + service_is_enabled.return_value = True + component.set_enabled(False) + assert component.is_enabled() + component.set_enabled(True) + assert component.is_enabled() + + +@patch('plinth.privileged.service.disable') +def test_shared_daemon_disable(disable_method): + """Test that shared daemon disables service correctly.""" + + class AppTest2(App): + """Test application class.""" + app_id = 'test-app-2' + + component1 = SharedDaemon('test-component1', 'test-daemon') + app1 = AppTest() + app1.add(component1) + app1.is_enabled = Mock() + + component2 = SharedDaemon('test-component2', 'test-daemon') + app2 = AppTest2() + app2.add(component2) + + # When another app is enabled, service should not be disabled + app1.is_enabled.return_value = True + app2.disable() + assert disable_method.mock_calls == [] + + # When all other apps are disabled, service should be disabled + disable_method.reset_mock() + app1.is_enabled.return_value = False + app2.disable() + assert disable_method.mock_calls == [call('test-daemon')] From 2fc354ea7febf0d33352385ac579e5170d1a9c79 Mon Sep 17 00:00:00 2001 From: Sunil Mohan Adapa Date: Tue, 13 Feb 2024 16:32:07 -0800 Subject: [PATCH 14/18] wordpress: Add shared daemon component for mariadb/mysql Signed-off-by: Sunil Mohan Adapa Reviewed-by: James Valleroy --- plinth/modules/wordpress/__init__.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/plinth/modules/wordpress/__init__.py b/plinth/modules/wordpress/__init__.py index f584e21b3..668e2c9c9 100644 --- a/plinth/modules/wordpress/__init__.py +++ b/plinth/modules/wordpress/__init__.py @@ -6,7 +6,7 @@ from django.utils.translation import gettext_lazy as _ from plinth import app as app_module from plinth import cfg, frontpage, menu from plinth.config import DropinConfigs -from plinth.daemon import Daemon +from plinth.daemon import Daemon, SharedDaemon from plinth.modules.apache.components import Webserver from plinth.modules.backups.components import BackupRestore from plinth.modules.firewall.components import Firewall @@ -106,6 +106,9 @@ class WordPressApp(app_module.App): daemon = Daemon('daemon-wordpress', 'wordpress-freedombox.timer') self.add(daemon) + daemon = SharedDaemon('shared-daemon-wordpress-mysql', 'mysql') + self.add(daemon) + backup_restore = WordPressBackupRestore('backup-restore-wordpress', **manifest.backup) self.add(backup_restore) From dbdac3b001ade5c84b55163cb0e5ab95590db9ce Mon Sep 17 00:00:00 2001 From: Sunil Mohan Adapa Date: Tue, 13 Feb 2024 16:32:39 -0800 Subject: [PATCH 15/18] zoph: Add shared daemon component for mariadb/mysql Signed-off-by: Sunil Mohan Adapa Reviewed-by: James Valleroy --- plinth/modules/zoph/__init__.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/plinth/modules/zoph/__init__.py b/plinth/modules/zoph/__init__.py index 85aec2b00..3f6297c19 100644 --- a/plinth/modules/zoph/__init__.py +++ b/plinth/modules/zoph/__init__.py @@ -8,6 +8,7 @@ from django.utils.translation import gettext_lazy as _ from plinth import app as app_module from plinth import cfg, frontpage, menu from plinth.config import DropinConfigs +from plinth.daemon import SharedDaemon from plinth.modules.apache.components import Webserver from plinth.modules.backups.components import BackupRestore from plinth.modules.firewall.components import Firewall @@ -89,6 +90,9 @@ class ZophApp(app_module.App): webserver = Webserver('webserver-zoph-freedombox', 'zoph-freedombox') self.add(webserver) + daemon = SharedDaemon('shared-daemon-zoph-mysql', 'mysql') + self.add(daemon) + backup_restore = ZophBackupRestore('backup-restore-zoph', **manifest.backup) self.add(backup_restore) From a9b77bbec813b2a9d74ff0313f7a5293671a4ca2 Mon Sep 17 00:00:00 2001 From: Olaf Schaf Date: Wed, 21 Feb 2024 13:59:59 +0000 Subject: [PATCH 16/18] Translated using Weblate (German) Currently translated at 100.0% (1557 of 1557 strings) --- plinth/locale/de/LC_MESSAGES/django.po | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/plinth/locale/de/LC_MESSAGES/django.po b/plinth/locale/de/LC_MESSAGES/django.po index b86aa9832..aae99e23c 100644 --- a/plinth/locale/de/LC_MESSAGES/django.po +++ b/plinth/locale/de/LC_MESSAGES/django.po @@ -10,8 +10,8 @@ msgstr "" "Project-Id-Version: FreedomBox UI\n" "Report-Msgid-Bugs-To: \n" "POT-Creation-Date: 2024-02-12 20:35-0500\n" -"PO-Revision-Date: 2024-02-11 20:14+0000\n" -"Last-Translator: Dietmar \n" +"PO-Revision-Date: 2024-02-22 14:02+0000\n" +"Last-Translator: Olaf Schaf \n" "Language-Team: German \n" "Language: de\n" @@ -19,7 +19,7 @@ msgstr "" "Content-Type: text/plain; charset=UTF-8\n" "Content-Transfer-Encoding: 8bit\n" "Plural-Forms: nplurals=2; plural=n != 1;\n" -"X-Generator: Weblate 5.4-dev\n" +"X-Generator: Weblate 5.5-dev\n" #: config.py:120 #, python-brace-format @@ -7042,10 +7042,8 @@ msgid "Read-only root filesystem" msgstr "Schreibgeschütztes Root-Dateisystem" #: modules/storage/__init__.py:391 -#, fuzzy -#| msgid "Go to Networks" msgid "Go to Power" -msgstr "Zur Stromversorgung" +msgstr "Springe zur Stromversorgung" #: modules/storage/forms.py:63 msgid "Invalid directory name." From 169eb9854f6baf073a4d45e92610d085b372afc6 Mon Sep 17 00:00:00 2001 From: James Valleroy Date: Mon, 26 Feb 2024 20:58:28 -0500 Subject: [PATCH 17/18] doc: Fetch latest manual Signed-off-by: James Valleroy --- doc/manual/en/ReleaseNotes.raw.wiki | 19 +++++++++++++++++++ doc/manual/es/ReleaseNotes.raw.wiki | 19 +++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/doc/manual/en/ReleaseNotes.raw.wiki b/doc/manual/en/ReleaseNotes.raw.wiki index 117947c7b..696ddcd00 100644 --- a/doc/manual/en/ReleaseNotes.raw.wiki +++ b/doc/manual/en/ReleaseNotes.raw.wiki @@ -8,6 +8,25 @@ For more technical details, see the [[https://salsa.debian.org/freedombox-team/f The following are the release notes for each !FreedomBox version. +== FreedomBox 24.5 (2024-02-26) == + + * backups: tests: Don't use pytest marks on fixtures + * container: Fix issue with missing make command on stable image + * daemon: Add new component for daemons shared across apps + * firewalld: Implement force upgrading to any 2.x versions + * gitweb, users: Minor fixes for newer pycodestyle + * locale: Update translations for German + * pyproject.toml: Exclude the build directory from mypy checks + * setup: Ensure that apt is updated before checking force upgrade + * setup: Ensure that force upgrade won't run when app is not installed + * setup: Minor refactoring of force upgrader class instantiation + * setup: Try force upgrade before running app setup + * tests: Patch apps_init for enable/disable daemon test + * tor: tests: Convert to pytest style tests from class based tests + * tor: tests: Fix issue with pytest 8.x versions + * wordpress: Add shared daemon component for mariadb/mysql + * zoph: Add shared daemon component for mariadb/mysql + == FreedomBox 24.4 (2024-02-12) == === Highlights === diff --git a/doc/manual/es/ReleaseNotes.raw.wiki b/doc/manual/es/ReleaseNotes.raw.wiki index 117947c7b..696ddcd00 100644 --- a/doc/manual/es/ReleaseNotes.raw.wiki +++ b/doc/manual/es/ReleaseNotes.raw.wiki @@ -8,6 +8,25 @@ For more technical details, see the [[https://salsa.debian.org/freedombox-team/f The following are the release notes for each !FreedomBox version. +== FreedomBox 24.5 (2024-02-26) == + + * backups: tests: Don't use pytest marks on fixtures + * container: Fix issue with missing make command on stable image + * daemon: Add new component for daemons shared across apps + * firewalld: Implement force upgrading to any 2.x versions + * gitweb, users: Minor fixes for newer pycodestyle + * locale: Update translations for German + * pyproject.toml: Exclude the build directory from mypy checks + * setup: Ensure that apt is updated before checking force upgrade + * setup: Ensure that force upgrade won't run when app is not installed + * setup: Minor refactoring of force upgrader class instantiation + * setup: Try force upgrade before running app setup + * tests: Patch apps_init for enable/disable daemon test + * tor: tests: Convert to pytest style tests from class based tests + * tor: tests: Fix issue with pytest 8.x versions + * wordpress: Add shared daemon component for mariadb/mysql + * zoph: Add shared daemon component for mariadb/mysql + == FreedomBox 24.4 (2024-02-12) == === Highlights === From c217fdb5c643b6a567529f875a24ae95253fe0db Mon Sep 17 00:00:00 2001 From: James Valleroy Date: Mon, 26 Feb 2024 20:59:13 -0500 Subject: [PATCH 18/18] Release v24.5 to unstable Signed-off-by: James Valleroy --- debian/changelog | 27 +++++++++++++++++++++++++++ plinth/__init__.py | 2 +- 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/debian/changelog b/debian/changelog index 5d5a31903..a7c8fc9c4 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,30 @@ +freedombox (24.5) unstable; urgency=medium + + [ Sunil Mohan Adapa ] + * container: Fix issue with missing make command on stable image + * setup: Minor refactoring of force upgrader class instantiation + * setup: Ensure that force upgrade won't run when app is not installed + * setup: Ensure that apt is updated before checking force upgrade + * firewalld: Implement force upgrading to any 2.x versions + * backups: tests: Don't use pytest marks on fixtures + * tor: tests: Fix issue with pytest 8.x versions + * tor: tests: Convert to pytest style tests from class based tests + * pyproject.toml: Exclude the build directory from mypy checks + * gitweb, users: Minor fixes for newer pycodestyle + * daemon: Add new component for daemons shared across apps + * wordpress: Add shared daemon component for mariadb/mysql + * zoph: Add shared daemon component for mariadb/mysql + + [ James Valleroy ] + * setup: Try force upgrade before running app setup + * tests: Patch apps_init for enable/disable daemon test + * doc: Fetch latest manual + + [ Olaf Schaf ] + * Translated using Weblate (German) + + -- James Valleroy Mon, 26 Feb 2024 20:58:45 -0500 + freedombox (24.4) unstable; urgency=medium [ Johannes Keyser ] diff --git a/plinth/__init__.py b/plinth/__init__.py index 43be27668..5ccd68a0b 100644 --- a/plinth/__init__.py +++ b/plinth/__init__.py @@ -3,4 +3,4 @@ Package init file. """ -__version__ = '24.4' +__version__ = '24.5'