From cd1630d2f532b0cbd80b2ecac46cc452c2626ce6 Mon Sep 17 00:00:00 2001 From: linsui <2873532-linsui@users.noreply.gitlab.com> Date: Wed, 11 Jun 2025 13:02:09 +0000 Subject: [PATCH] Lazyload environment variables in config.yml --- fdroidserver/common.py | 83 ++++++++++++++++++++++++++++-------------- tests/test_common.py | 55 +++++++++++++++++++++++----- 2 files changed, 101 insertions(+), 37 deletions(-) diff --git a/fdroidserver/common.py b/fdroidserver/common.py index 05423fff..b131ce19 100644 --- a/fdroidserver/common.py +++ b/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 diff --git a/tests/test_common.py b/tests/test_common.py index c8d10045..1f6ff947 100755 --- a/tests/test_common.py +++ b/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."""