Merge branch 'lazyconfig' into 'master'

Lazyload environment variables in config.yml

See merge request fdroid/fdroidserver!1645

fdroidserver/common.py

@@ -546,6 +546,60 @@ def config_type_check(path, data):
         )
 
 
+class _Config(dict):
+    def __init__(self, default={}):
+        super(_Config, self).__init__(default)
+        self.loaded = {}
+
+    def lazyget(self, key):
+        if key not in self.loaded:
+            value = super(_Config, self).__getitem__(key)
+
+            if key == 'serverwebroot':
+                roots = parse_list_of_dicts(value)
+                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.get('url')
+                    if not rootstr:
+                        logging.error('serverwebroot: has blank value!')
+                        continue
+                    if rootstr[-1] != '/':
+                        rootstr += '/'
+                    d['url'] = rootstr.replace('//', '/')
+                    rootlist.append(d)
+                self.loaded[key] = rootlist
+
+            elif key == 'servergitmirrors':
+                self.loaded[key] = parse_list_of_dicts(value)
+
+            elif isinstance(value, dict) and 'env' in value and len(value) == 1:
+                var = value['env']
+                if var in os.environ:
+                    self.loaded[key] = os.getenv(var)
+                else:
+                    logging.error(
+                        _(
+                            'Environment variable {var} from {configname} is not set!'
+                        ).format(var=value['env'], configname=key)
+                    )
+                    self.loaded[key] = None
+            else:
+                self.loaded[key] = value
+
+        return self.loaded[key]
+
+    def __getitem__(self, key):
+        return self.lazyget(key)
+
+    def get(self, key, default=None, /):
+        try:
+            return self.lazyget(key)
+        except KeyError:
+            return default
+
+
 def read_config():
     """Read the repository config.
 
@@ -601,25 +655,7 @@ def read_config():
 
     fill_config_defaults(config)
 
-    if 'serverwebroot' in config:
-        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.get('url')
-            if not rootstr:
-                logging.error('serverwebroot: has blank value!')
-                continue
-            if rootstr[-1] != '/':
-                rootstr += '/'
-            d['url'] = rootstr.replace('//', '/')
-            rootlist.append(d)
-        config['serverwebroot'] = rootlist
-
     if 'servergitmirrors' in config:
-        config['servergitmirrors'] = parse_list_of_dicts(config['servergitmirrors'])
-
         limit = config['git_mirror_size_limit']
         config['git_mirror_size_limit'] = parse_human_readable_size(limit)
 
@@ -642,15 +678,7 @@ def read_config():
             continue
         elif isinstance(dictvalue, dict):
             for k, v in dictvalue.items():
-                if k == 'env':
-                    env = os.getenv(v)
-                    if env:
-                        config[configname] = env
-                    else:
-                        confignames_to_delete.add(configname)
-                        logging.error(_('Environment variable {var} from {configname} is not set!')
-                                      .format(var=k, configname=configname))
-                else:
+                if k != 'env':
                     confignames_to_delete.add(configname)
                     logging.error(_('Unknown entry {key} in {configname}')
                                   .format(key=k, configname=configname))
@@ -670,6 +698,7 @@ def read_config():
                 )
             )
 
+    config = _Config(config)
     return config
 
 

tests/test_common.py

@@ -2029,6 +2029,49 @@ class CommonTest(SetUpTearDownMixin, unittest.TestCase):
         config = fdroidserver.common.read_config()
         self.assertEqual('/usr/lib/jvm/java-8-openjdk', config['java_paths']['8'])
 
+    @mock.patch.dict(os.environ, {'PATH': os.getenv('PATH')}, clear=True)
+    def test_config_lazy_load_env_vars(self):
+        """Test the environment variables in config.yml is lazy loaded.
+
+        It shouldn't throw errors when read the config if the environment variables are
+        not set. It should throw errors when the variables are get from the config.
+        """
+        os.chdir(self.testdir)
+        fdroidserver.common.write_config_file(
+            textwrap.dedent(
+                """
+                serverwebroot: {env: serverwebroot}
+                servergitmirrors:
+                  - url: {env: mirror1}
+                  - url: {env: mirror2}
+                keypass: {env: keypass}
+                keystorepass: {env: keystorepass}
+                """
+            )
+        )
+        with self.assertNoLogs(level=logging.ERROR):
+            config = fdroidserver.common.read_config()
+
+        # KeyError should be raised if a key is not in the config.yml
+        with self.assertRaises(KeyError):
+            config['gpghome']
+
+        self.assertEqual(config.get('gpghome', 'gpg'), 'gpg')
+        os.environ.update({key: f"{key}supersecret" for key in ["serverwebroot", "mirror1", "mirror2", "keystorepass"]})
+        self.assertEqual(config['keystorepass'], 'keystorepasssupersecret')
+        self.assertEqual(config['serverwebroot'], [{'url': 'serverwebrootsupersecret/'}])
+        self.assertEqual(config['servergitmirrors'], [{'url': 'mirror1supersecret'}, {'url': 'mirror2supersecret'}])
+
+    @mock.patch.dict(os.environ, {'PATH': os.getenv('PATH')}, clear=True)
+    def test_config_lazy_load_env_vars_not_set(self):
+        os.chdir(self.testdir)
+        fdroidserver.common.write_config_file('keypass: {env: keypass}')
+        fdroidserver.common.read_config()
+        with self.assertLogs(level=logging.ERROR) as lw:
+            fdroidserver.common.config['keypass']
+            self.assertTrue('is not set' in lw.output[0])
+            self.assertEqual(1, len(lw.output))
+
     @mock.patch.dict(os.environ, {'PATH': os.getenv('PATH')}, clear=True)
     def test_test_sdk_exists_fails_on_bad_sdk_path(self):
         config = {'sdk_path': 'nothinghere'}
@@ -3465,7 +3508,7 @@ class ConfigOptionsScopeTest(unittest.TestCase):
         self.assertIsNone(fdroidserver.common.config)
         config = fdroidserver.common.read_config()
         self.assertIsNotNone(fdroidserver.common.config)
-        self.assertEqual(dict, type(config))
+        self.assertTrue(isinstance(config, dict))
         self.assertEqual(config, fdroidserver.common.config)
 
     def test_get_config_global(self):
@@ -3475,7 +3518,7 @@ class ConfigOptionsScopeTest(unittest.TestCase):
         self.assertIsNone(fdroidserver.common.config)
         c = fdroidserver.common.read_config()
         self.assertIsNotNone(fdroidserver.common.config)
-        self.assertEqual(dict, type(c))
+        self.assertTrue(isinstance(c, dict))
         self.assertEqual(c, fdroidserver.common.config)
         self.assertTrue(
             'config' not in vars() and 'config' not in globals(),
@@ -3515,14 +3558,6 @@ class UnsafePermissionsTest(SetUpTearDownMixin, unittest.TestCase):
             self.assertTrue('unsafe' in lw.output[0])
             self.assertEqual(1, len(lw.output))
 
-    @mock.patch.dict(os.environ, {'PATH': os.getenv('PATH')}, clear=True)
-    def test_config_perm_unset_env_no_warning(self):
-        fdroidserver.common.write_config_file('keypass: {env: keypass}')
-        with self.assertLogs(level=logging.WARNING) as lw:
-            fdroidserver.common.read_config()
-            self.assertTrue('unsafe' not in lw.output[0])
-            self.assertEqual(1, len(lw.output))
-
 
 class GetHeadCommitIdTest(unittest.TestCase):
     """Test and compare two methods of getting the commit ID."""