clarify config data types and structures

fdroidserver/common.py

@@ -25,8 +25,32 @@
 # You should have received a copy of the GNU Affero General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-# common.py is imported by all modules, so do not import third-party
+
-# libraries here as they will become a requirement for all commands.
+"""Collection of functions shared by subcommands.
+
+This is basically the "shared library" for all the fdroid subcommands.
+The contains core functionality and a number of utility functions.
+This is imported by all modules, so do not import third-party
+libraries here as they will become a requirement for all commands.
+
+Config
+------
+
+Parsing and using the configuration settings from config.yml is
+handled here. The data format is YAML 1.2. The config has its own
+supported data types:
+
+* Boolean (e.g. deploy_process_logs:)
+* Integer (e.g. archive_older:, repo_maxage:)
+* String-only (e.g. repo_name:, sdk_path:)
+* Multi-String (string, list of strings, or list of dicts with
+  strings, e.g. serverwebroot:, mirrors:)
+
+String-only fields can also use a special value {env: varname}, which
+is a dict with a single key 'env' and a value that is the name of the
+environment variable to include.
+
+"""
 
 import copy
 import difflib
@@ -586,12 +610,15 @@ def read_config():
     fill_config_defaults(config)
 
     if 'serverwebroot' in config:
-        roots = parse_mirrors_config(config['serverwebroot'])
+        roots = parse_list_of_dicts(config['serverwebroot'])
         rootlist = []
         for d in roots:
             # since this is used with rsync, where trailing slashes have
             # meaning, ensure there is always a trailing slash
-            rootstr = d['url']
+            rootstr = d.get('url')
+            if not rootstr:
+                logging.error('serverwebroot: has blank value!')
+                continue
             if rootstr[-1] != '/':
                 rootstr += '/'
             d['url'] = rootstr.replace('//', '/')
@@ -599,7 +626,7 @@ def read_config():
         config['serverwebroot'] = rootlist
 
     if 'servergitmirrors' in config:
-        config['servergitmirrors'] = parse_mirrors_config(config['servergitmirrors'])
+        config['servergitmirrors'] = parse_list_of_dicts(config['servergitmirrors'])
 
         limit = config['git_mirror_size_limit']
         config['git_mirror_size_limit'] = parse_human_readable_size(limit)
@@ -666,18 +693,23 @@ def expand_env_dict(s):
     return os.path.expanduser(s)
 
 
-def parse_mirrors_config(mirrors):
+def parse_list_of_dicts(l_of_d):
-    """Mirrors can be specified as a string, list of strings, or dictionary map."""
+    """Parse config data structure that is a list of dicts of strings.
-    if isinstance(mirrors, str):
+
-        return [{"url": expand_env_dict(mirrors)}]
+    The value can be specified as a string, list of strings, or list of dictionary maps
-    if isinstance(mirrors, dict):
+    where the values are strings.
-        return [{"url": expand_env_dict(mirrors)}]
+
-    if all(isinstance(item, str) for item in mirrors):
+    """
-        return [{'url': expand_env_dict(i)} for i in mirrors]
+    if isinstance(l_of_d, str):
-    if all(isinstance(item, dict) for item in mirrors):
+        return [{"url": expand_env_dict(l_of_d)}]
-        for item in mirrors:
+    if isinstance(l_of_d, dict):
+        return [{"url": expand_env_dict(l_of_d)}]
+    if all(isinstance(item, str) for item in l_of_d):
+        return [{'url': expand_env_dict(i)} for i in l_of_d]
+    if all(isinstance(item, dict) for item in l_of_d):
+        for item in l_of_d:
             item['url'] = expand_env_dict(item['url'])
-        return mirrors
+        return l_of_d
     raise TypeError(_('only accepts strings, lists, and tuples'))
 
 
@@ -690,7 +722,7 @@ def get_mirrors(url, filename=None):
     if url.netloc == 'f-droid.org':
         mirrors = FDROIDORG_MIRRORS
     else:
-        mirrors = parse_mirrors_config(url.geturl())
+        mirrors = parse_list_of_dicts(url.geturl())
 
     if filename:
         return append_filename_to_mirrors(filename, mirrors)

fdroidserver/lint.py

@@ -899,7 +899,7 @@ def lint_config(arg):
 
             show_error = False
             if t is str:
-                if type(data[key]) not in (str, dict):
+                if type(data[key]) not in (str, list, dict):
                     passed = False
                     show_error = True
             elif type(data[key]) != t:

fdroidserver/net.py

@@ -92,7 +92,7 @@ def download_using_mirrors(mirrors, local_filename=None):
     logic will try it twice: first without SNI, then again with SNI.
 
     """
-    mirrors = common.parse_mirrors_config(mirrors)
+    mirrors = common.parse_list_of_dicts(mirrors)
     mirror_configs_to_try = []
     for mirror in mirrors:
         mirror_configs_to_try.append(mirror)

tests/test_common.py

@@ -2903,46 +2903,46 @@ class CommonTest(unittest.TestCase):
         with self.assertRaises(TypeError):
             fdroidserver.common.expand_env_dict({'env': 'foo', 'foo': 'bar'})
 
-    def test_parse_mirrors_config_str(self):
+    def test_parse_list_of_dicts_str(self):
         s = 'foo@example.com:/var/www'
         mirrors = yaml.load("""'%s'""" % s)
         self.assertEqual(
-            [{'url': s}], fdroidserver.common.parse_mirrors_config(mirrors)
+            [{'url': s}], fdroidserver.common.parse_list_of_dicts(mirrors)
         )
 
-    def test_parse_mirrors_config_list(self):
+    def test_parse_list_of_dicts_list(self):
         s = 'foo@example.com:/var/www'
         mirrors = yaml.load("""- '%s'""" % s)
         self.assertEqual(
-            [{'url': s}], fdroidserver.common.parse_mirrors_config(mirrors)
+            [{'url': s}], fdroidserver.common.parse_list_of_dicts(mirrors)
         )
 
-    def test_parse_mirrors_config_dict(self):
+    def test_parse_list_of_dicts_dict(self):
         s = 'foo@example.com:/var/www'
         mirrors = yaml.load("""- url: '%s'""" % s)
         self.assertEqual(
-            [{'url': s}], fdroidserver.common.parse_mirrors_config(mirrors)
+            [{'url': s}], fdroidserver.common.parse_list_of_dicts(mirrors)
         )
 
     @mock.patch.dict(os.environ, {'PATH': os.getenv('PATH'), 'foo': 'bar'}, clear=True)
-    def test_parse_mirrors_config_env_str(self):
+    def test_parse_list_of_dicts_env_str(self):
         mirrors = yaml.load('{env: foo}')
         self.assertEqual(
-            [{'url': 'bar'}], fdroidserver.common.parse_mirrors_config(mirrors)
+            [{'url': 'bar'}], fdroidserver.common.parse_list_of_dicts(mirrors)
         )
 
-    def test_parse_mirrors_config_env_list(self):
+    def test_parse_list_of_dicts_env_list(self):
         s = 'foo@example.com:/var/www'
         mirrors = yaml.load("""- '%s'""" % s)
         self.assertEqual(
-            [{'url': s}], fdroidserver.common.parse_mirrors_config(mirrors)
+            [{'url': s}], fdroidserver.common.parse_list_of_dicts(mirrors)
         )
 
-    def test_parse_mirrors_config_env_dict(self):
+    def test_parse_list_of_dicts_env_dict(self):
         s = 'foo@example.com:/var/www'
         mirrors = yaml.load("""- url: '%s'""" % s)
         self.assertEqual(
-            [{'url': s}], fdroidserver.common.parse_mirrors_config(mirrors)
+            [{'url': s}], fdroidserver.common.parse_list_of_dicts(mirrors)
         )
 
     def test_KnownApks_recordapk(self):

tests/test_lint.py

@@ -4,6 +4,7 @@ import logging
 import os
 import shutil
 import tempfile
+import textwrap
 import unittest
 from pathlib import Path
 
@@ -534,6 +535,13 @@ class LintAntiFeaturesTest(unittest.TestCase):
 
 
 class ConfigYmlTest(LintTest):
+    """Test data formats used in config.yml.
+
+    lint.py uses print() and not logging so hacks are used to control
+    the output when running in the test runner.
+
+    """
+
     def setUp(self):
         super().setUp()
         self.config_yml = Path(self.testdir) / fdroidserver.common.CONFIG_FILE
@@ -550,6 +558,22 @@ class ConfigYmlTest(LintTest):
         self.config_yml.write_text('sdk_path: /opt/android-sdk\n')
         self.assertTrue(fdroidserver.lint.lint_config(self.config_yml))
 
+    def test_config_yml_str_list(self):
+        self.config_yml.write_text('serverwebroot: [server1, server2]\n')
+        self.assertTrue(fdroidserver.lint.lint_config(self.config_yml))
+
+    def test_config_yml_str_list_of_dicts(self):
+        self.config_yml.write_text(
+            textwrap.dedent(
+                """\
+                serverwebroot:
+                  - url: 'me@b.az:/srv/fdroid'
+                    index_only: true
+                """
+            )
+        )
+        self.assertTrue(fdroidserver.lint.lint_config(self.config_yml))
+
     def test_config_yml_str_list_of_dicts_env(self):
         """serverwebroot can be str, list of str, or list of dicts."""
         self.config_yml.write_text('serverwebroot: {env: ANDROID_HOME}\n')
@@ -595,3 +619,32 @@ class ConfigYmlTest(LintTest):
                 fdroidserver.lint.lint_config(self.config_yml),
                 f'{key} should fail on value of "{value}"',
             )
+
+    def test_config_yml_keyaliases(self):
+        self.config_yml.write_text(
+            textwrap.dedent(
+                """\
+                keyaliases:
+                  com.example: myalias
+                  com.foo: '@com.example'
+                """
+            )
+        )
+        self.assertTrue(fdroidserver.lint.lint_config(self.config_yml))
+
+    def test_config_yml_keyaliases_bad_str(self):
+        """The keyaliases: value is a dict not a str."""
+        self.config_yml.write_text("keyaliases: '@com.example'\n")
+        self.assertFalse(fdroidserver.lint.lint_config(self.config_yml))
+
+    def test_config_yml_keyaliases_bad_list(self):
+        """The keyaliases: value is a dict not a list."""
+        self.config_yml.write_text(
+            textwrap.dedent(
+                """\
+                keyaliases:
+                  - com.example: myalias
+                """
+            )
+        )
+        self.assertFalse(fdroidserver.lint.lint_config(self.config_yml))