diff --git a/knack/config.py b/knack/config.py index 9f19243..2c5e580 100644 --- a/knack/config.py +++ b/knack/config.py @@ -109,9 +109,20 @@ def sections(self): def items(self, section): import re - pattern = self.env_var_name(section, '.+') - candidates = [(k.split('_')[-1], os.environ[k], k) for k in os.environ if re.match(pattern, k)] - result = {c[0]: c for c in candidates} + # Only allow valid env vars, in all caps: CLI_SECTION_TEST_OPTION, CLI_SECTION__TEST_OPTION + pattern = self.env_var_name(section, '([0-9A-Z_]+)') + env_entries = [] + for k in os.environ: + # Must be a full match, otherwise CLI_SECTION_T part in CLI_MYSECTION_Test_Option will match + matched = re.fullmatch(pattern, k) + if matched: + # (name, value, ENV_VAR_NAME) + item = (matched.group(1).lower(), os.environ[k], k) + env_entries.append(item) + + # Prepare result with env entries first + result = {c[0]: c for c in env_entries} + # Add entries from config files if they do not exist yet for config in self._config_file_chain if self.use_local_config else self._config_file_chain[-1:]: try: entries = config.items(section) diff --git a/tests/test_config.py b/tests/test_config.py index 95dd840..ce1b274 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -169,6 +169,64 @@ def test_getboolean_error(self): with self.assertRaises(ValueError): self.cli_config.getboolean(section, option) + def test_items(self): + file_section = "MySection" + file_value = 'file_value' + env_value = 'env_value' + + # Test file-only options are listed + file_only_option = 'file_only_option' + self.cli_config.set_value(file_section, file_only_option, file_value) + items_result = self.cli_config.items(file_section) + self.assertEqual(len(items_result), 1) + self.assertEqual(items_result[0]['name'], file_only_option) + self.assertEqual(items_result[0]['value'], file_value) + self.cli_config.remove_option(file_section, file_only_option) + + # Test env-only options are listed + with mock.patch.dict('os.environ', {'CLI_MYSECTION_ENV_ONLY_OPTION': env_value}): + items_result = self.cli_config.items(file_section) + self.assertEqual(len(items_result), 1) + self.assertEqual(items_result[0]['name'], 'env_only_option') + self.assertEqual(items_result[0]['value'], env_value) + + # Test file options are overridden by env options, for both single-word and multi-word options + test_options = [ + # file_option, file_value, env_name, env_value + # Test single-word option + ('optionsingle', 'file_value_single', 'CLI_MYSECTION_OPTIONSINGLE', 'env_value_single'), + # Test multi-word option + ('option_multiple', 'file_value_multiple', 'CLI_MYSECTION_OPTION_MULTIPLE', 'env_value_multiple') + ] + for file_option, file_value, env_name, env_value in test_options: + self.cli_config.set_value(file_section, file_option, file_value) + items_result = self.cli_config.items(file_section) + self.assertEqual(len(items_result), 1) + self.assertEqual(items_result[0]['value'], file_value) + + with mock.patch.dict('os.environ', {env_name: env_value}): + items_result = self.cli_config.items(file_section) + self.assertEqual(len(items_result), 1) + self.assertEqual(items_result[0]['value'], env_value) + + self.cli_config.remove_option(file_section, file_option) + + # Test Invalid_Env_Var is not accepted on Linux + # Windows' env var is case-insensitive, so skip + import platform + if platform.system() != 'Windows': + # Not shown + with mock.patch.dict('os.environ', {'CLI_MYSECTION_Test_Option': env_value}): + items_result = self.cli_config.items(file_section) + self.assertEqual(len(items_result), 0) + + # No overriding + self.cli_config.set_value(file_section, 'test_option', file_value) + with mock.patch.dict('os.environ', {'CLI_MYSECTION_Test_Option': env_value}): + items_result = self.cli_config.items(file_section) + self.assertEqual(len(items_result), 1) + self.assertEqual(items_result[0]['value'], file_value) + def test_set_config_value(self): self.cli_config.set_value('test_section', 'test_option', 'a_value') config = configparser.ConfigParser()