track config file and command line locations for user settings

Keep track of which configuration file a group of settings is in to
try to report that as a detail for user settings.

Track settings from the command line separately from settings in the
configuration files.

It is possible that the configuration file details can be wrong if a
group appears in more than one file. We can be more accurate with this
after we have the backend driver feature implemented, but since most
deployments use a single file so this should be relatively useful.

Change-Id: I04ce72dbc5f676296acfeafc13169177ad815350
Signed-off-by: Doug Hellmann <doug@doughellmann.com>

oslo_config/cfg.py

@@ -512,6 +512,7 @@ class Locations(enum.Enum):
     set_default = (2, False)
     set_override = (3, False)
     user = (4, True)
+    command_line = (5, True)
 
     def __init__(self, num, is_user_controlled):
         self.num = num
@@ -1052,7 +1053,7 @@ class Opt(object):
                 names.append((dgroup if dgroup else group_name,
                               dname if dname else self.dest))
 
-        value = namespace._get_value(
+        value, loc = namespace._get_value(
             names, multi=self.multi,
             positional=self.positional, current_name=current_name)
         # The previous line will raise a KeyError if no value is set in the
@@ -1070,7 +1071,7 @@ class Opt(object):
                         {'option': self.dest,
                          'group': pretty_group,
                          'reason': pretty_reason})
-        return value
+        return (value, loc)
 
     def _add_to_cli(self, parser, group=None):
         """Makes the option available in the command line interface.
@@ -2003,8 +2004,9 @@ class ConfigParser(iniparser.BaseParser):
                 return
             raise
 
-        namespace._add_parsed_config_file(sections, normalized)
-        namespace._parse_cli_opts_from_config_file(sections, normalized)
+        namespace._add_parsed_config_file(config_file, sections, normalized)
+        namespace._parse_cli_opts_from_config_file(
+            config_file, sections, normalized)
 
 
 @removals.remove(version='3.4', removal_version='4.0')
@@ -2042,12 +2044,12 @@ class MultiConfigParser(object):
                 parser.parse()
             except IOError:
                 continue
-            self._add_parsed_config_file(sections, normalized)
+            self._add_parsed_config_file(filename, sections, normalized)
             read_ok.append(filename)
 
         return read_ok
 
-    def _add_parsed_config_file(self, sections, normalized):
+    def _add_parsed_config_file(self, filename, sections, normalized):
         """Add a parsed config file to the list of parsed files.
 
         :param sections: a mapping of section name to dicts of config values
@@ -2143,8 +2145,10 @@ class _Namespace(argparse.Namespace):
         self._files_not_found = []
         self._files_permission_denied = []
         self._config_dirs = []
+        self._sections_to_file = {}
 
-    def _parse_cli_opts_from_config_file(self, sections, normalized):
+    def _parse_cli_opts_from_config_file(self, config_file, sections,
+                                         normalized):
         """Parse CLI options from a config file.
 
         CLI options are special - we require they be registered before the
@@ -2167,12 +2171,12 @@ class _Namespace(argparse.Namespace):
         override values found in this file.
         """
         namespace = _Namespace(self._conf)
-        namespace._add_parsed_config_file(sections, normalized)
+        namespace._add_parsed_config_file(config_file, sections, normalized)
 
         for opt, group in self._conf._all_cli_opts():
             group_name = group.name if group is not None else None
             try:
-                value = opt._get_from_namespace(namespace, group_name)
+                value, loc = opt._get_from_namespace(namespace, group_name)
             except KeyError:
                 continue
             except ValueError as ve:
@@ -2193,13 +2197,16 @@ class _Namespace(argparse.Namespace):
             else:
                 setattr(self, dest, value)
 
-    def _add_parsed_config_file(self, sections, normalized):
+    def _add_parsed_config_file(self, filename, sections, normalized):
         """Add a parsed config file to the list of parsed files.
 
+        :param filename: the full name of the file that was parsed
         :param sections: a mapping of section name to dicts of config values
         :param normalized: sections mapping with section names normalized
         :raises: ConfigFileValueError
         """
+        for s in sections:
+            self._sections_to_file[s] = filename
         self._parsed.insert(0, sections)
         self._normalized.insert(0, normalized)
 
@@ -2258,6 +2265,7 @@ class _Namespace(argparse.Namespace):
 
         names = [(normalize(section), name) for section, name in names]
 
+        loc = None
         for sections in (self._normalized if normalized else self._parsed):
             for section, name in names:
                 if section not in sections:
@@ -2267,12 +2275,17 @@ class _Namespace(argparse.Namespace):
                     self._check_deprecated((section, name), current_name,
                                            names[1:])
                     val = sections[section][name]
+                    if loc is None:
+                        loc = LocationInfo(
+                            Locations.user,
+                            self._sections_to_file.get(section, ''),
+                        )
                     if multi:
                         rvalue = val + rvalue
                     else:
-                        return val
+                        return (val, loc)
         if multi and rvalue != []:
-            return rvalue
+            return (rvalue, loc)
         raise KeyError
 
     def _check_deprecated(self, name, current, deprecated):
@@ -2310,14 +2323,43 @@ class _Namespace(argparse.Namespace):
         :param multi: a boolean indicating whether to return multiple values
         :param normalized: whether to normalize group names to lowercase
         """
+        # NOTE(dhellmann): We don't have a way to track which options
+        # that are registered as command line values show up on the
+        # command line or in the configuration files. So we look up
+        # the value in the file first to get the location, and then
+        # try looking it up as a CLI value in case it was set there.
+
+        # Set a default location indicating that the value came from
+        # the command line. This will be overridden if we find a value
+        # in a file.
+        loc = LocationInfo(Locations.command_line, '')
+
         try:
-            return self._get_cli_value(names, positional)
-        except KeyError:
-            names = [(g if g is not None else 'DEFAULT', n) for g, n in names]
-            values = self._get_file_value(
-                names, multi=multi, normalized=normalized,
+            file_names = [(g if g is not None else 'DEFAULT', n)
+                          for g, n in names]
+            values, loc = self._get_file_value(
+                file_names, multi=multi, normalized=normalized,
                 current_name=current_name)
-            return values if multi else values[-1]
+        except KeyError:
+            # If we receive a KeyError when looking for the CLI, just
+            # go ahead and throw it because we know we don't have a
+            # value.
+            raise_later = True
+        else:
+            raise_later = False
+
+        # Now try the CLI
+        try:
+            value = self._get_cli_value(names, positional)
+            return (value, loc)
+        except KeyError:
+            if raise_later:
+                # Re-raise to indicate that we haven't found the value
+                # anywhere.
+                raise
+
+        # Return the value we found in the file.
+        return (values if multi else values[-1], loc)
 
     def _sections(self):
         for sections in self._parsed:
@@ -3037,10 +3079,8 @@ class ConfigOpts(collections.Mapping):
         if namespace is not None:
             group_name = group.name if group else None
             try:
-                return (
-                    convert(opt._get_from_namespace(namespace, group_name)),
-                    loc,
-                )
+                val, alt_loc = opt._get_from_namespace(namespace, group_name)
+                return (convert(val), alt_loc)
             except KeyError:  # nosec: Valid control flow instruction
                 pass
             except ValueError as ve:
@@ -3272,7 +3312,7 @@ class ConfigOpts(collections.Mapping):
                                  key=lambda x: x[0].name):
             group_name = group.name if group else None
             try:
-                value = opt._get_from_namespace(namespace, group_name)
+                value, loc = opt._get_from_namespace(namespace, group_name)
             except KeyError:
                 continue
 
@@ -3373,11 +3413,11 @@ class ConfigOpts(collections.Mapping):
                 continue
             groupname = group.name if group else 'DEFAULT'
             try:
-                old = opt._get_from_namespace(self._namespace, groupname)
+                old, _ = opt._get_from_namespace(self._namespace, groupname)
             except KeyError:
                 old = None
             try:
-                new = opt._get_from_namespace(self._mutable_ns, groupname)
+                new, _ = opt._get_from_namespace(self._mutable_ns, groupname)
             except KeyError:
                 new = None
             if old != new:
@@ -3399,11 +3439,11 @@ class ConfigOpts(collections.Mapping):
                 continue
             groupname = group.name if group else None
             try:
-                old = opt._get_from_namespace(old_ns, groupname)
+                old, _ = opt._get_from_namespace(old_ns, groupname)
             except KeyError:
                 old = None
             try:
-                new = opt._get_from_namespace(new_ns, groupname)
+                new, _ = opt._get_from_namespace(new_ns, groupname)
             except KeyError:
                 new = None
             if old != new:

oslo_config/fixture.py

@@ -174,7 +174,8 @@ class Config(fixtures.Fixture):
             # Parsed values are an array of raw strings.
             raw_config[group][key] = [str(value)]
 
-        self.conf._namespace._add_parsed_config_file(raw_config, raw_config)
+        self.conf._namespace._add_parsed_config_file(
+            '<memory>', raw_config, raw_config)
 
     def set_config_files(self, config_files):
         """Specify a list of config files to read.

oslo_config/tests/test_cfg.py

@@ -4057,7 +4057,8 @@ class NamespaceTestCase(BaseTestCase):
                           normalized=normalized)
 
     def assertValue(self, key, expect, multi=False, normalized=False):
-        actual = self.ns._get_value([key], multi=multi, normalized=normalized)
+        actual, _ = self.ns._get_value([key], multi=multi,
+                                       normalized=normalized)
         self.assertEqual(actual, expect)
 
     def test_cli(self):

oslo_config/tests/test_get_location.py

@@ -10,6 +10,10 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import io
+import tempfile
+import textwrap
+
 from oslotest import base
 
 from oslo_config import cfg
@@ -35,6 +39,7 @@ class LocationTestCase(base.BaseTestCase):
 
     def test_user_controlled(self):
         self.assertTrue(cfg.Locations.user.is_user_controlled)
+        self.assertTrue(cfg.Locations.command_line.is_user_controlled)
 
     def test_not_user_controlled(self):
         self.assertFalse(cfg.Locations.opt_default.is_user_controlled)
@@ -96,3 +101,65 @@ class GetLocationTestCase(base.BaseTestCase):
             loc.location,
         )
         self.assertIn('test_get_location.py', loc.detail)
+
+    def test_user_cli(self):
+        filename = self._write_opt_to_tmp_file(
+            'DEFAULT', 'unknown_opt', self.id())
+        self.conf(['--config-file', filename,
+                   '--cli_opt', 'blah'])
+        loc = self.conf.get_location('cli_opt')
+        self.assertEqual(
+            cfg.Locations.command_line,
+            loc.location,
+        )
+
+    def test_default_cli(self):
+        filename = self._write_opt_to_tmp_file(
+            'DEFAULT', 'unknown_opt', self.id())
+        self.conf(['--config-file', filename])
+        loc = self.conf.get_location('cli_opt')
+        self.assertEqual(
+            cfg.Locations.opt_default,
+            loc.location,
+        )
+
+    def _write_opt_to_tmp_file(self, group, option, value):
+        filename = tempfile.mktemp()
+        with io.open(filename, 'w', encoding='utf-8') as f:
+            f.write(textwrap.dedent(u'''
+            [{group}]
+            {option} = {value}
+            ''').format(
+                group=group,
+                option=option,
+                value=value,
+            ))
+        return filename
+
+    def test_user_cli_opt_in_file(self):
+        filename = self._write_opt_to_tmp_file(
+            'DEFAULT', 'cli_opt', self.id())
+        self.conf(['--config-file', filename])
+        loc = self.conf.get_location('cli_opt')
+        self.assertEqual(
+            cfg.Locations.user,
+            loc.location,
+        )
+        self.assertEqual(
+            filename,
+            loc.detail,
+        )
+
+    def test_user_file(self):
+        filename = self._write_opt_to_tmp_file(
+            'DEFAULT', 'normal_opt', self.id())
+        self.conf(['--config-file', filename])
+        loc = self.conf.get_location('normal_opt')
+        self.assertEqual(
+            cfg.Locations.user,
+            loc.location,
+        )
+        self.assertEqual(
+            filename,
+            loc.detail,
+        )