[Config] Make config items case-insensitive (#220)

microsoft · Dec 9, 2020 · 676477e · 676477e
1 parent dc7b7d8
commit 676477e
Show file tree

Hide file tree

Showing 2 changed files with 72 additions and 3 deletions.
diff --git 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
@@ -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()