diff --git a/plinth/config.py b/plinth/config.py index d274afd08..cf15821e1 100644 --- a/plinth/config.py +++ b/plinth/config.py @@ -80,21 +80,9 @@ class DropinConfigs(app_module.FollowerComponent): def enable(self): """Create a symlink or copy in /etc/ of the configuration file.""" for path in self.etc_paths: - etc_path = self._get_etc_path(path) - target = self._get_target_path(path) - if etc_path.exists() or etc_path.is_symlink(): - if (not self.copy_only and etc_path.is_symlink() - and etc_path.readlink() == target): - continue - - if (self.copy_only and etc_path.is_file() - and etc_path.read_text() == target.read_text()): - continue - - logger.warning('Removing dropin configuration: %s', path) - privileged.dropin_unlink(self.app_id, path) - - privileged.dropin_link(self.app_id, path, self.copy_only) + if not privileged.dropin_is_valid( + self.app_id, path, self.copy_only, unlink_invalid=True): + privileged.dropin_link(self.app_id, path, self.copy_only) def disable(self): """Remove the links/copies in /etc/ of the configuration files.""" @@ -105,15 +93,10 @@ class DropinConfigs(app_module.FollowerComponent): """Check all links/copies and return generate diagnostic results.""" results = [] for path in self.etc_paths: - etc_path = self._get_etc_path(path) - target = self._get_target_path(path) - if self.copy_only: - result = (etc_path.is_file() - and etc_path.read_text() == target.read_text()) - else: - result = (etc_path.is_symlink() - and etc_path.readlink() == target) + result = privileged.dropin_is_valid(self.app_id, path, + self.copy_only) + etc_path = self.get_etc_path(path) check_id = f'dropin-config-{etc_path}' result_string = Result.PASSED if result else Result.FAILED description = gettext_noop( @@ -126,13 +109,13 @@ class DropinConfigs(app_module.FollowerComponent): return results @staticmethod - def _get_target_path(path): + def get_target_path(path): """Return Path object for a target path.""" target = pathlib.Path(DropinConfigs.ROOT) target /= DropinConfigs.DROPIN_CONFIG_ROOT.lstrip('/') return target / path.lstrip('/') @staticmethod - def _get_etc_path(path): + def get_etc_path(path): """Return Path object for etc path.""" return pathlib.Path(DropinConfigs.ROOT) / path.lstrip('/') diff --git a/plinth/privileged/__init__.py b/plinth/privileged/__init__.py index b1c51ad2b..3b4a81fc2 100644 --- a/plinth/privileged/__init__.py +++ b/plinth/privileged/__init__.py @@ -1,7 +1,7 @@ # SPDX-License-Identifier: AGPL-3.0-or-later """Package holding all the privileged actions outside of apps.""" -from .config import dropin_link, dropin_unlink +from .config import dropin_is_valid, dropin_link, dropin_unlink from .packages import (filter_conffile_packages, install, is_package_manager_busy, remove, update) from .service import (disable, enable, is_enabled, is_running, mask, reload, @@ -11,5 +11,5 @@ __all__ = [ 'filter_conffile_packages', 'install', 'is_package_manager_busy', 'remove', 'update', 'disable', 'enable', 'is_enabled', 'is_running', 'mask', 'reload', 'restart', 'start', 'stop', 'try_restart', 'unmask', - 'dropin_link', 'dropin_unlink' + 'dropin_is_valid', 'dropin_link', 'dropin_unlink' ] diff --git a/plinth/privileged/config.py b/plinth/privileged/config.py index 850568485..57bd922a6 100644 --- a/plinth/privileged/config.py +++ b/plinth/privileged/config.py @@ -30,13 +30,39 @@ def _assert_managed_dropin_config(app_id: str, path: str): raise AssertionError('Not a managed drop-in config') +@privileged +def dropin_is_valid(app_id: str, path: str, copy_only: bool, + unlink_invalid: bool = False) -> bool: + """Check if symlink from /etc/ to /usr/share/freedombox/etc is valid. + + Optionally, drop the link if it is invalid. + """ + _assert_managed_dropin_config(app_id, path) + from plinth.config import DropinConfigs + etc_path = DropinConfigs.get_etc_path(path) + target = DropinConfigs.get_target_path(path) + if etc_path.exists() or etc_path.is_symlink(): + if (not copy_only and etc_path.is_symlink() + and etc_path.readlink() == target): + return True + + if (copy_only and etc_path.is_file() + and etc_path.read_text() == target.read_text()): + return True + + if unlink_invalid: + etc_path.unlink(missing_ok=True) + + return False + + @privileged def dropin_link(app_id: str, path: str, copy_only: bool): """Create a symlink from /etc/ to /usr/share/freedombox/etc.""" _assert_managed_dropin_config(app_id, path) from plinth.config import DropinConfigs - target = DropinConfigs._get_target_path(path) - etc_path = DropinConfigs._get_etc_path(path) + target = DropinConfigs.get_target_path(path) + etc_path = DropinConfigs.get_etc_path(path) etc_path.parent.mkdir(parents=True, exist_ok=True) if copy_only: shutil.copyfile(target, etc_path) @@ -49,5 +75,5 @@ def dropin_unlink(app_id: str, path: str, missing_ok: bool = False): """Remove a symlink in /etc/.""" _assert_managed_dropin_config(app_id, path) from plinth.config import DropinConfigs - etc_path = DropinConfigs._get_etc_path(path) + etc_path = DropinConfigs.get_etc_path(path) etc_path.unlink(missing_ok=missing_ok) diff --git a/plinth/tests/test_config.py b/plinth/tests/test_config.py index 6f82b8172..bbab5a343 100644 --- a/plinth/tests/test_config.py +++ b/plinth/tests/test_config.py @@ -95,7 +95,7 @@ def test_dropin_configs_enable_disable_symlinks(dropin_configs, tmp_path): # Enable when a file already exists dropin_configs.disable() - etc_path = DropinConfigs._get_etc_path('/etc/test/path1') + etc_path = DropinConfigs.get_etc_path('/etc/test/path1') etc_path.touch() dropin_configs.enable() _assert_symlinks(dropin_configs, tmp_path, should_exist=True) @@ -108,7 +108,7 @@ def test_dropin_configs_enable_disable_symlinks(dropin_configs, tmp_path): # When symlink already exists to correct location dropin_configs.disable() - target_path = DropinConfigs._get_target_path('/etc/test/path1') + target_path = DropinConfigs.get_target_path('/etc/test/path1') etc_path.symlink_to(target_path) dropin_configs.enable() _assert_symlinks(dropin_configs, tmp_path, should_exist=True) @@ -119,7 +119,7 @@ def test_dropin_configs_enable_disable_copy_only(dropin_configs, tmp_path): with patch('plinth.config.DropinConfigs.ROOT', new=tmp_path): dropin_configs.copy_only = True for path in ['/etc/test/path1', '/etc/path2']: - target = DropinConfigs._get_target_path(path) + target = DropinConfigs.get_target_path(path) target.parent.mkdir(parents=True, exist_ok=True) target.write_text('test-config-content') @@ -135,7 +135,7 @@ def test_dropin_configs_enable_disable_copy_only(dropin_configs, tmp_path): # Enable when a file already exists with wrong content dropin_configs.disable() - etc_path = DropinConfigs._get_etc_path('/etc/test/path1') + etc_path = DropinConfigs.get_etc_path('/etc/test/path1') etc_path.write_text('x-invalid-content') dropin_configs.enable() _assert_symlinks(dropin_configs, tmp_path, should_exist=True, @@ -180,7 +180,7 @@ def test_dropin_config_diagnose_symlinks(dropin_configs, tmp_path): # A file exists instead of symlink dropin_configs.disable() - etc_path = DropinConfigs._get_etc_path('/etc/test/path1') + etc_path = DropinConfigs.get_etc_path('/etc/test/path1') etc_path.touch() results = dropin_configs.diagnose() assert results[0].result == 'failed' @@ -202,7 +202,7 @@ def test_dropin_config_diagnose_copy_only(dropin_configs, tmp_path): with patch('plinth.config.DropinConfigs.ROOT', new=tmp_path): dropin_configs.copy_only = True for path in ['/etc/test/path1', '/etc/path2']: - target = DropinConfigs._get_target_path(path) + target = DropinConfigs.get_target_path(path) target.parent.mkdir(parents=True, exist_ok=True) target.write_text('test-config-content') @@ -219,7 +219,7 @@ def test_dropin_config_diagnose_copy_only(dropin_configs, tmp_path): # A symlink exists instead of a copied file dropin_configs.disable() - etc_path = DropinConfigs._get_etc_path('/etc/test/path1') + etc_path = DropinConfigs.get_etc_path('/etc/test/path1') etc_path.symlink_to('/blah') results = dropin_configs.diagnose() assert results[0].result == 'failed'