cfg: Drop the default configuration file

- The configuration module defaults to values in the production configuration
file.

- If the file is found, it is read and the read values overwrite the defaults.
If the file is not found, no error is raised. This allows us to not ship the
configuration file. User may create the configuration if they want to change the
defaults. This eases upgrades when configuration is edited. This also make
FreedomBox robust to deployments where /etc/ is not populated by default such as
OSTree. It is also a good practice for daemons as followed by the likes of
systemd.

- If the file partly populated only the values read override the defaults and
the remaining values don't change. This allows the user to write simpler
configuration file.

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

conftest.py

@@ -55,7 +55,7 @@ def fixture_load_cfg():
     cfg_file = test_data_dir / 'etc' / 'plinth' / 'plinth.config'
     cfg.read(str(cfg_file), str(root_dir))
     yield cfg
-    cfg.read()
+    cfg.read(str(cfg_file), str(root_dir))
 
 
 @pytest.fixture(name='develop_mode')

data/etc/plinth/plinth.config

@@ -1,36 +0,0 @@
-[Path]
-# directory locations
-file_root = /usr/share/plinth
-config_dir = /etc/plinth
-data_dir = /var/lib/plinth
-server_dir = /plinth
-actions_dir = /usr/share/plinth/actions
-doc_dir = /usr/share/freedombox
-custom_static_dir = /var/www/plinth/custom/static
-
-# file locations
-store_file = %(data_dir)s/plinth.sqlite3
-
-[Network]
-host = 127.0.0.1
-port = 8000
-
-# Enable the following only if Plinth is behind a proxy server.  The
-# proxy server should properly clean and the following HTTP headers:
-#   X-Forwarded-For
-#   X-Forwarded-Host
-#   X-Forwarded-Proto
-# If you enable these unnecessarily, this will lead to serious security
-# problems. For more information, see
-# https://docs.djangoproject.com/en/1.7/ref/settings/
-#
-# These are enabled by default in Plinth because the default
-# configuration allows only connections from localhost
-#
-# Leave the values blank to disable
-use_x_forwarded_for = True
-use_x_forwarded_host = True
-secure_proxy_ssl_header = HTTP_X_FORWARDED_PROTO
-
-[Misc]
-box_name = FreedomBox

debian/freedombox.maintscript

@@ -10,3 +10,4 @@ rm_conffile /etc/plinth/modules-enabled/udiskie 0.39.0~
 rm_conffile /etc/plinth/modules-enabled/restore 20.1~
 rm_conffile /etc/plinth/modules-enabled/repro 20.1~
 rm_conffile /etc/apt/preferences.d/50freedombox3.pref 20.5~
+rm_conffile /etc/plinth/plinth.config 20.12~

plinth/cfg.py

@@ -1,4 +1,7 @@
 # SPDX-License-Identifier: AGPL-3.0-or-later
+"""
+Configuration parser and default values for configuration options.
+"""
 
 import configparser
 import logging
@@ -6,28 +9,46 @@ import os
 
 logger = logging.getLogger(__name__)
 
-box_name = None
+# [Path] section
 root = None
-file_root = None
-config_dir = None
-data_dir = None
-custom_static_dir = None
-store_file = None
-actions_dir = None
-doc_dir = None
-host = None
-port = None
-use_x_forwarded_for = False
-use_x_forwarded_host = False
-secure_proxy_ssl_header = None
+file_root = '/usr/share/plinth'
+config_dir = '/etc/plinth'
+data_dir = '/var/lib/plinth'
+custom_static_dir = '/var/www/plinth/custom/static'
+store_file = data_dir + '/plinth.sqlite3'
+actions_dir = '/usr/share/plinth/actions'
+doc_dir = '/usr/share/freedombox'
+server_dir = '/plinth'
+
+# [Network] section
+host = '127.0.0.1'
+port = 8000
+
+# Enable the following only if Plinth is behind a proxy server.  The
+# proxy server should properly clean and the following HTTP headers:
+#   X-Forwarded-For
+#   X-Forwarded-Host
+#   X-Forwarded-Proto
+# If you enable these unnecessarily, this will lead to serious security
+# problems. For more information, see
+# https://docs.djangoproject.com/en/1.7/ref/settings/
+#
+# These are enabled by default in FreedomBox because the default
+# configuration allows only connections from localhost
+#
+# Leave the values blank to disable
+use_x_forwarded_for = True
+use_x_forwarded_host = True
+secure_proxy_ssl_header = 'HTTP_X_FORWARDED_PROTO'
+
+# [Misc] section
+box_name = 'FreedomBox'
+
+# Other globals
 develop = False
-server_dir = '/'
 
 config_file = None
 
-DEFAULT_CONFIG_FILE = '/etc/plinth/plinth.config'
-DEFAULT_ROOT = '/'
-
 
 def get_fallback_config_paths():
     """Get config paths of the current source code folder"""
@@ -54,8 +75,8 @@ def read(config_path=None, root_directory=None):
         config_path, root_directory = get_config_paths()
 
     if not os.path.isfile(config_path):
-        msg = 'No plinth.config file could be found on path: %s' % config_path
-        raise FileNotFoundError(msg)
+        # Ignore missing configuration files
+        return
 
     global config_file  # pylint: disable-msg=invalid-name,global-statement
     config_file = config_path
@@ -87,9 +108,8 @@ def read(config_path=None, root_directory=None):
         try:
             value = parser.get(section, name)
         except (configparser.NoSectionError, configparser.NoOptionError):
-            logger.error('Configuration does not contain option: %s.%s',
-                         section, name)
-            raise
+            # Use default values for any missing keys in configuration
+            continue
         else:
             if datatype == 'int':
                 value = int(value)

plinth/tests/data/plinth.config.with_missing_options

@@ -1,5 +1,5 @@
 [Misc]
-box_name = FreedomBox
+box_name = FreedomBoxTestMissingOptions
 
 [Path]
 

plinth/tests/data/plinth.config.with_missing_sections

@@ -1,2 +1,2 @@
 [Misc]
-box_name = FreedomBox
+box_name = FreedomBoxTestMissingSections

plinth/tests/test_cfg.py

@@ -6,6 +6,7 @@ Test module for configuration module.
 import configparser
 import logging
 import os
+from unittest.mock import patch
 
 import pytest
 
@@ -23,54 +24,33 @@ logging.disable(logging.CRITICAL)
 pytestmark = pytest.mark.usefixtures('load_cfg')
 
 
-@pytest.fixture(name='test_config_file')
-def fixture_test_config_file(load_cfg):
-    """Test fixture to return the configuration file path"""
-    return cfg.get_config_paths()[0]
-
-
-@pytest.fixture(name='test_config_dir')
-def fixture_test_config_dir(load_cfg):
-    """Test fixture to return the configuration file directory."""
-    return cfg.get_config_paths()[1]
-
-
-def test_read_default_config_file(test_config_dir, test_config_file):
+def test_read_default_config_file():
     """Verify that the default config file can be read correctly."""
+    config_file, config_dir = cfg.get_fallback_config_paths()
+
     # Read the plinth.config file directly
-    parser = configparser.ConfigParser(defaults={'root': test_config_dir})
-    parser.read(test_config_file)
+    parser = configparser.ConfigParser(defaults={'root': config_dir})
+    parser.read(config_file)
 
     # Read the plinth.config file via the cfg module
-    cfg.read(test_config_file, test_config_dir)
+    cfg.read(config_file, config_dir)
 
     # Compare the two results
     compare_configurations(parser)
 
 
-def test_read_primary_config_file():
+@patch('plinth.cfg.get_config_paths')
+def test_read_primary_config_file(get_config_paths):
     """Verify that the primary config file is used by default."""
-    original_config_path = cfg.DEFAULT_CONFIG_FILE
-    original_root_directory = cfg.DEFAULT_ROOT
-
     expected_config_path = CONFIG_FILE_WITH_MISSING_OPTIONS
     root_directory = 'x-default-root'
     expected_root_directory = os.path.realpath(root_directory)
+    get_config_paths.return_value = (expected_config_path, root_directory)
 
-    try:
-        cfg.DEFAULT_CONFIG_FILE = expected_config_path
-        cfg.DEFAULT_ROOT = root_directory
-        # reading the config file will fail, but still cfg.root and
-        # cfg.config_file will be set for parsing the config file
-        try:
-            cfg.read()
-        except configparser.NoOptionError:
-            pass
-        assert cfg.config_file == expected_config_path
-        assert cfg.root == expected_root_directory
-    finally:
-        cfg.DEFAULT_CONFIG_FILE = original_config_path
-        cfg.DEFAULT_ROOT = original_root_directory
+    cfg.read()
+
+    assert cfg.config_file == expected_config_path
+    assert cfg.root == expected_root_directory
 
 
 def test_read_fallback_config_file():
@@ -86,20 +66,19 @@ def test_read_fallback_config_file():
 
 def test_read_missing_config_file():
     """Verify that an exception is raised when there's no config file."""
-    with pytest.raises(FileNotFoundError):
-        cfg.read('x-non-existant-file', 'x-root-directory')
+    cfg.read('x-non-existant-file', 'x-root-directory')
 
 
-def test_read_config_file_with_missing_sections(test_config_dir):
+def test_read_config_file_with_missing_sections():
     """Verify that missing configuration sections can be detected."""
-    with pytest.raises(configparser.NoSectionError):
-        cfg.read(CONFIG_FILE_WITH_MISSING_SECTIONS, test_config_dir)
+    cfg.read(CONFIG_FILE_WITH_MISSING_SECTIONS, TEST_CONFIG_DIR)
+    assert cfg.box_name == 'FreedomBoxTestMissingSections'
 
 
-def test_read_config_file_with_missing_options(test_config_dir):
+def test_read_config_file_with_missing_options():
     """Verify that missing configuration options can be detected."""
-    with pytest.raises(configparser.NoOptionError):
-        cfg.read(CONFIG_FILE_WITH_MISSING_OPTIONS, test_config_dir)
+    cfg.read(CONFIG_FILE_WITH_MISSING_OPTIONS, TEST_CONFIG_DIR)
+    assert cfg.box_name == 'FreedomBoxTestMissingOptions'
 
 
 def compare_configurations(parser):