config: Add new component for managing drop-in /etc/ config files

- Use two separate strategies: one to symlink from /etc to files in /usr/ and
two to copy files from /usr to /etc. The latter is needed for some rare cases
where symlinks don't work because the daemon runs in a chrooted environment.

- Update tutorial and reference sections in developer documentation.

Tests:

- Unit tests works and are comprehensive.

- Using the component in apps works.

Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>

doc/dev/reference/components/config.rst

@@ -0,0 +1,7 @@
+.. SPDX-License-Identifier: CC-BY-SA-4.0
+
+Configuration
+^^^^^^^^^^^^^
+
+.. autoclass:: plinth.config.DropinConfigs
+   :members:

doc/dev/reference/components/index.rst

@@ -10,6 +10,7 @@ Components
    enablestate
    menu
    packages
+   config
    daemon
    firewall
    webserver

doc/dev/tutorial/code.rst

@@ -48,10 +48,10 @@ plinth/modules/transmission/data/usr/share/freedombox/modules-enabled/transmissi
 .. literalinclude:: ../../../plinth/modules/transmission/data/usr/share/freedombox/modules-enabled/transmission
    :language: text
 
-plinth/modules/transmission/data/etc/apache2/conf-available/transmission-plinth.conf
+plinth/modules/transmission/data/usr/share/freedombox/etc/apache2/conf-available/transmission-plinth.conf
-^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-.. literalinclude:: ../../../plinth/modules/transmission/data/etc/apache2/conf-available/transmission-plinth.conf
+.. literalinclude:: ../../../plinth/modules/transmission/data/usr/share/freedombox/etc/apache2/conf-available/transmission-plinth.conf
    :language: apache
 
 plinth/modules/transmission/tests/__init__.py

doc/dev/tutorial/components.rst

@@ -170,19 +170,56 @@ The first argument to instantiate the
 :class:`~plinth.modules.apache.components.Webserver` class is a unique ID. The
 second is the name of the Apache2 web server configuration snippet that contains
 the directives to proxy Transmission web interface via Apache2. We then need to
-create the configuration file itself in ``tranmission-freedombox.conf``. The
+create the configuration file itself in ``transmission-plinth.conf``. The
 final argument is the list of URLs that the app exposes to the users of the app.
 This information is used to check if the URLs are accessible as expected when
 the user requests diagnostic tests on the app.
 
+Simply creating and shipping a configuration file into ``/etc`` folder creates
+some hassles. Consider the following scenario: a debian package, either
+freedombox or the app's separate debian package ships a file in ``/etc``. Then
+the user deliberately or accidentally modifies the configuration file. Then the
+debian package provides a newer version of the configuration file with, say,
+more tweaks. As a result, a configuration file prompt is shown to the user
+during package upgrade process. In case of unattended upgrades, the package is
+not upgraded at all. To avoid such problems, FreedomBox provides the
+:class:`~plinth.config.DropinConfigs` component. Let us add it in our app's
+class.
+
+.. code-block:: python3
+  :caption: ``__init__.py``
+
+  from plinth.config import DropinConfigs
+
+  class TransmissionApp(app_module.App):
+      ...
+
+      def __init__(self):
+        ...
+
+        dropin_configs = DropinConfigs('dropin-configs-transmission', [
+            '/etc/apache2/conf-available/transmission-plinth.conf',
+        ])
+        self.add(dropin_configs)
+
+The first argument to instantiate the :class:`~plinth.config.DropinConfigs`
+class is the unique ID. The second argument is the list of configuration files
+as paths is ``/etc/``. The :class:`~plinth.config.DropinConfigs` component
+requires that a file be shipped into ``/usr/share/freedombox/etc`` instead of
+``/etc``. The component will handle the creation of a symlink from ``/usr`` path
+to ``/etc`` path. To ship the file, we can simply create file in the ``data/``
+directory of the app and let the FreedomBox setup script handling the
+installation and shipping.
+
 .. code-block:: apache
-  :caption: ``data/etc/apache2/conf-available/transmission-freedombox.conf``
+  :caption: ``data/usr/share/freedombox/etc/apache2/conf-available/transmission-plinth.conf``
 
   ## On all sites, provide Transmission on a default path: /transmission
   <Location /transmission>
       ProxyPass        http://localhost:9091/transmission
   </Location>
 
+
 Managing the firewall
 ^^^^^^^^^^^^^^^^^^^^^
 
@@ -249,10 +286,10 @@ with the FreedomBox framework in ``__init.py__``.
 
 Then in the Apache configuration snippet, we can mandate that only users of this
 group (and, of course, admin users) should be allowed to access our app. In the
-file ``tranmission-freedombox.conf``, add the following.
+file ``transmission-plinth.conf``, add the following.
 
 .. code-block:: apache
-  :caption: ``data/etc/apache2/conf-available/transmission-freedombox.conf``
+  :caption: ``data/etc/apache2/conf-available/transmission-plinth.conf``
 
   <Location /transmission>
       ...

doc/dev/tutorial/skeleton.rst

@@ -22,10 +22,13 @@ in a step-by-step manner::
           ├─ urls.py
           ├─ views.py
           ├─┬ data/
-          │ ├─┬ etc/
+          │ ├─┬ usr/
-          │ │ └─┬ apache2/
+          │ │ └─┬ share/
-          │ │   └─┬ conf-available/
+          │ │   └─┬ freedombox/
-          │ │     └─ transmission-freedombox.conf
+          │ │     └─┬ etc/
+          │ │       └─┬ apache2/
+          │ │         └─┬ conf-available/
+          │ │           └─ transmission-plinth.conf
           │ └─┬ usr/
           │   └─┬ share/
           │     └─┬ freedombox/

plinth/config.py

@@ -0,0 +1,133 @@
+"""Components for managing configuration files."""
+
+import logging
+import pathlib
+
+from django.utils.text import format_lazy
+from django.utils.translation import gettext_lazy as _
+
+from plinth.privileged import config as privileged
+
+from . import app as app_module
+
+logger = logging.getLogger(__name__)
+
+
+class DropinConfigs(app_module.FollowerComponent):
+    """Component to manage config files dropped into /etc.
+
+    When configuring a daemon, it is often simpler to ship a configuration file
+    into the daemon's configuration directory. However, if the user modifies
+    this configuration file and freedombox ships a new version of this
+    configuration file, then a conflict arises between user's changes and
+    changes in the new version of configuration file shipped by freedombox.
+    This leads to freedombox package getting marked by unattended-upgrades as
+    not automatically upgradable. Dpkg's solution of resolving the conflicts is
+    to present the option to the user which is also not acceptable.
+
+    Further, if a package is purged from the system, sometimes the
+    configuration directories are fully removed by deb's scripts. This removes
+    files installed by freedombox package. Dpkg treats these files as if user
+    has explictly removed them and may lead to a configuration conflict
+    described above.
+
+    The approach freedombox takes to address these issues is using this
+    component. Files are shipped into /usr/share/freedombox/etc/ instead of
+    /etc/ (keeping the subpath unchanged). Then when an app is enabled, a
+    symlink or copy is created from the /usr/share/freedombox/etc/ into /etc/.
+    This way, user's understand the configuration file is not meant to be
+    edited. Even if they do, next upgrade of freedombox package will silently
+    overwrite those changes without causing merge conflicts. Also when purging
+    a package removes entire configuration directory, only symlinks/copies are
+    lost. They will recreated when the app is reinstalled/enabled.
+    """
+
+    ROOT = '/'  # To make writing tests easier
+    DROPIN_CONFIG_ROOT = '/usr/share/freedombox/'
+
+    def __init__(self, component_id, etc_paths=None, copy_only=False):
+        """Initialize the drop-in configuration component.
+
+        component_id should be a unique ID across all components of an app and
+        across all components.
+
+        etc_paths is a list of all drop-in configuration files as absolute
+        paths in /etc/ which need to managed by this component. For each of the
+        paths, it is expected that the actual configuration file exists in
+        /usr/share/freedombox/etc/. A link to the file or copy of the file is
+        created in /etc/ when app is enabled and the link or file is removed
+        when app is disabled. For example, if etc_paths contains
+        /etc/apache/conf-enabled/myapp.conf then
+        /usr/share/freedombox/etc/apache/conf-enabled/myapp.conf must be
+        shipped and former path will be link to or be a copy of the latter when
+        app is enabled.
+        """
+        super().__init__(component_id)
+        self.etc_paths = etc_paths or []
+        self.copy_only = copy_only
+
+    def setup(self, old_version):
+        """Create symlinks or copies of files during app update.
+
+        During the transition from shipped configs to the symlink/copy
+        approach, files in /etc will be removed during .deb upgrade. This
+        method ensures that symlinks or copies are properly recreated.
+        """
+        if self.app_id and self.app.is_enabled():
+            self.enable()
+
+    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)
+
+    def disable(self):
+        """Remove the links/copies in /etc/ of the configuration files."""
+        for path in self.etc_paths:
+            privileged.dropin_unlink(self.app_id, path, missing_ok=True)
+
+    def diagnose(self):
+        """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_string = 'passed' if result else 'failed'
+            template = _('Static configuration {etc_path} is setup properly')
+            test_name = format_lazy(template, etc_path=str(etc_path))
+            results.append([test_name, result_string])
+
+        return results
+
+    @staticmethod
+    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):
+        """Return Path object for etc path."""
+        return pathlib.Path(DropinConfigs.ROOT) / path.lstrip('/')

plinth/privileged/__init__.py

@@ -1,6 +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 .packages import (filter_conffile_packages, install,
                        is_package_manager_busy, remove, update)
 from .service import (disable, enable, is_enabled, is_running, mask, reload,
@@ -9,5 +10,6 @@ from .service import (disable, enable, is_enabled, is_running, mask, reload,
 __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'
+    'reload', 'restart', 'start', 'stop', 'try_restart', 'unmask',
+    'dropin_link', 'dropin_unlink'
 ]

plinth/privileged/config.py

@@ -0,0 +1,53 @@
+# SPDX-License-Identifier: AGPL-3.0-or-later
+"""Symlink and unlink configuration files into /etc."""
+
+import importlib
+import inspect
+import shutil
+
+from plinth import app as app_module
+from plinth import module_loader
+from plinth.actions import privileged
+
+
+def _assert_managed_dropin_config(app_id: str, path: str):
+    """Check that this is a path managed by the specified app."""
+    module_path = module_loader.get_module_import_path(app_id)
+    module = importlib.import_module(module_path)
+    module_classes = inspect.getmembers(module, inspect.isclass)
+    app_classes = [
+        cls[1] for cls in module_classes if issubclass(cls[1], app_module.App)
+    ]
+
+    for cls in app_classes:
+        app = cls()
+        from plinth.config import DropinConfigs
+        components = app.get_components_of_type(DropinConfigs)
+        for component in components:
+            if path in component.etc_paths:
+                return
+
+    raise AssertionError('Not a managed drop-in config')
+
+
+@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)
+    etc_path.parent.mkdir(parents=True, exist_ok=True)
+    if copy_only:
+        shutil.copyfile(target, etc_path)
+    else:
+        etc_path.symlink_to(target)
+
+
+@privileged
+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.unlink(missing_ok=missing_ok)

plinth/tests/test_config.py

@@ -0,0 +1,222 @@
+# SPDX-License-Identifier: AGPL-3.0-or-later
+"""
+Test the component that manages drop-in configuration.
+"""
+
+from unittest.mock import Mock, patch
+
+import pytest
+
+from plinth.app import App
+from plinth.config import DropinConfigs
+
+pytestmark = pytest.mark.usefixtures('mock_privileged')
+privileged_modules_to_mock = ['plinth.privileged.config']
+
+
+@pytest.fixture(name='dropin_configs')
+def fixture_dropin_configs():
+    """Fixture to create a basic drop-in configs component."""
+
+    class AppTest(App):
+        app_id = 'test-app'
+
+    app = AppTest()
+    component = DropinConfigs('test-component',
+                              ['/etc/test/path1', '/etc/path2'])
+    app.add(component)
+
+    return component
+
+
+@pytest.fixture(autouse=True)
+def fixture_assert_dropin_config():
+    """Mock asserting dropin config path."""
+    with patch('plinth.privileged.config._assert_managed_dropin_config'):
+        yield
+
+
+def test_dropin_configs_init(dropin_configs):
+    """Test initialization of drop-in configs component."""
+    assert dropin_configs.component_id == 'test-component'
+    assert dropin_configs.etc_paths[0] == '/etc/test/path1'
+    assert dropin_configs.etc_paths[1] == '/etc/path2'
+    assert not dropin_configs.copy_only
+
+    component = DropinConfigs('test-component', [], copy_only=False)
+    assert not component.copy_only
+
+    component = DropinConfigs('test-component', [], copy_only=True)
+    assert component.copy_only
+
+
+def _assert_symlinks(component, tmp_path, should_exist, copy_only=False):
+    """Assert that symlinks exists and they point correctly."""
+    for path in component.etc_paths:
+        full_path = tmp_path / path.lstrip('/')
+        if should_exist:
+            target = tmp_path / 'usr/share/freedombox' / path.lstrip('/')
+            if copy_only:
+                assert full_path.is_file()
+                assert full_path.read_text() == target.read_text()
+            else:
+                assert full_path.is_symlink()
+                assert full_path.resolve() == target
+        else:
+            assert not full_path.exists()
+
+
+def test_dropin_configs_setup(dropin_configs, tmp_path):
+    """Test setup for dropin configs component."""
+    with patch('plinth.config.DropinConfigs.ROOT', new=tmp_path):
+        is_enabled = Mock()
+        App.get('test-app').is_enabled = is_enabled
+
+        is_enabled.return_value = False
+        dropin_configs.setup(old_version=0)
+        _assert_symlinks(dropin_configs, tmp_path, should_exist=False)
+
+        is_enabled.return_value = True
+        dropin_configs.setup(old_version=0)
+        _assert_symlinks(dropin_configs, tmp_path, should_exist=True)
+
+
+def test_dropin_configs_enable_disable_symlinks(dropin_configs, tmp_path):
+    """Test enable/disable for dropin configs component for symlinks."""
+    with patch('plinth.config.DropinConfigs.ROOT', new=tmp_path):
+        # Enable when nothing exists
+        dropin_configs.enable()
+        _assert_symlinks(dropin_configs, tmp_path, should_exist=True)
+
+        # Disable
+        dropin_configs.disable()
+        _assert_symlinks(dropin_configs, tmp_path, should_exist=False)
+
+        # Enable when a file already exists
+        dropin_configs.disable()
+        etc_path = DropinConfigs._get_etc_path('/etc/test/path1')
+        etc_path.touch()
+        dropin_configs.enable()
+        _assert_symlinks(dropin_configs, tmp_path, should_exist=True)
+
+        # When symlink already exists to wrong location
+        dropin_configs.disable()
+        etc_path.symlink_to('/blah')
+        dropin_configs.enable()
+        _assert_symlinks(dropin_configs, tmp_path, should_exist=True)
+
+        # When symlink already exists to correct location
+        dropin_configs.disable()
+        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)
+
+
+def test_dropin_configs_enable_disable_copy_only(dropin_configs, tmp_path):
+    """Test enable/disable for dropin configs component for copying."""
+    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.parent.mkdir(parents=True, exist_ok=True)
+            target.write_text('test-config-content')
+
+        # Enable when nothing exists
+        dropin_configs.enable()
+        _assert_symlinks(dropin_configs, tmp_path, should_exist=True,
+                         copy_only=True)
+
+        # Disable
+        dropin_configs.disable()
+        _assert_symlinks(dropin_configs, tmp_path, should_exist=False,
+                         copy_only=True)
+
+        # Enable when a file already exists with wrong content
+        dropin_configs.disable()
+        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,
+                         copy_only=True)
+
+        # When the file is a symlink
+        dropin_configs.disable()
+        etc_path.symlink_to('/blah')
+        dropin_configs.enable()
+        _assert_symlinks(dropin_configs, tmp_path, should_exist=True,
+                         copy_only=True)
+
+        # When copy already exists with correct content
+        dropin_configs.disable()
+        etc_path.write_text('test-config-content')
+        dropin_configs.enable()
+        _assert_symlinks(dropin_configs, tmp_path, should_exist=True,
+                         copy_only=True)
+
+
+def test_dropin_config_diagnose_symlinks(dropin_configs, tmp_path):
+    """Test diagnosing dropin configs for symlinks."""
+    with patch('plinth.config.DropinConfigs.ROOT', new=tmp_path):
+        # Nothing exists
+        results = dropin_configs.diagnose()
+        assert results[0][1] == 'failed'
+        assert results[1][1] == 'failed'
+
+        # Proper symlinks exist
+        dropin_configs.enable()
+        results = dropin_configs.diagnose()
+        assert results[0][1] == 'passed'
+        assert results[1][1] == 'passed'
+
+        # A file exists instead of symlink
+        dropin_configs.disable()
+        etc_path = DropinConfigs._get_etc_path('/etc/test/path1')
+        etc_path.touch()
+        results = dropin_configs.diagnose()
+        assert results[0][1] == 'failed'
+
+        # Symlink points to wrong location
+        dropin_configs.disable()
+        etc_path.symlink_to('/blah')
+        results = dropin_configs.diagnose()
+        assert results[0][1] == 'failed'
+
+        # Symlink is recreated
+        dropin_configs.enable()
+        results = dropin_configs.diagnose()
+        assert results[0][1] == 'passed'
+
+
+def test_dropin_config_diagnose_copy_only(dropin_configs, tmp_path):
+    """Test diagnosing dropin configs."""
+    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.parent.mkdir(parents=True, exist_ok=True)
+            target.write_text('test-config-content')
+
+        # Nothing exists
+        results = dropin_configs.diagnose()
+        assert results[0][1] == 'failed'
+        assert results[1][1] == 'failed'
+
+        # Proper copies exist
+        dropin_configs.enable()
+        results = dropin_configs.diagnose()
+        assert results[0][1] == 'passed'
+        assert results[1][1] == 'passed'
+
+        # A symlink exists instead of a copied file
+        dropin_configs.disable()
+        etc_path = DropinConfigs._get_etc_path('/etc/test/path1')
+        etc_path.symlink_to('/blah')
+        results = dropin_configs.diagnose()
+        assert results[0][1] == 'failed'
+
+        # Copied file contains wrong contents
+        dropin_configs.disable()
+        etc_path.write_text('x-invalid-contents')
+        results = dropin_configs.diagnose()
+        assert results[0][1] == 'failed'