From b350e34bc175a56eb8b941ae7e3c89b2d876ea48 Mon Sep 17 00:00:00 2001
From: Jake Hunsaker <jhunsake@redhat.com>
Date: Fri, 26 Jun 2020 12:29:09 -0400
Subject: [global] Update config file location and layout

Moves the default config file we look for to /etc/sos/sos.conf instead
of /etc/sos.conf. Extends the config file to look for a section matching
the name of the component being used. Renames the "general" section to
"global" and the "tunables" section to "plugin_options".

Updates the default sos.conf to this style and adds some comments to the
file. Update the man page for sos.conf.

Note that this commit does NOT update sos.spec to drop the default
configuration file in the new location, as that will be handled by a
later commit to update the specfile wholesale.

Closes: #2125
Resolves: #2136

Signed-off-by: Jake Hunsaker <jhunsake@redhat.com>
---
 man/en/sos.conf.5      | 44 ++++++++++++-----------
 sos.conf               | 33 ++++++++++++++---
 sos/__init__.py        |  4 ++-
 sos/component.py       | 48 +++++++++++++++++++++----
 sos/options.py         | 97 ++++++++++++++++++++++++++------------------------
 sos/report/__init__.py | 42 +++++++++++-----------
 6 files changed, 169 insertions(+), 99 deletions(-)

diff --git a/man/en/sos.conf.5 b/man/en/sos.conf.5
index 2d2386f7..248af60c 100644
--- a/man/en/sos.conf.5
+++ b/man/en/sos.conf.5
@@ -3,11 +3,11 @@
 sos.conf \- sosreport configuration
 .SH DESCRIPTION
 .sp
-sosreport uses a configuration file at /etc/sos.conf.
+sosreport uses a configuration file at /etc/sos/sos.conf.
 .SH PARAMETERS
 .sp
-There are three sections in the sosreport configuration file:
-general, plugins and tunables. Options are set using 'ini'-style
+There are sections for each sos component, as well as global values and
+those for plugin options. Options are set using 'ini'-style
 \fBname = value\fP pairs. Disabling/enabling a boolean option
 is done the same way like on command line (e.g. process.lsof=off).
 
@@ -18,30 +18,30 @@ will result in enabling those options, regardless of value set.
 
 Sections are parsed in the ordering:
 .br
-- \fB[general]\fP
+- \fB[global]\fP
 .br
-- \fB[plugins]\fP (disable)
+- \fB[component]\fP
 .br
-- \fB[plugins]\fP (enable)
-.br
-- \fB[tunables]\fP
+- \fB[plugin_options]\fP
 
 .TP
-\fB[general]\fP
+\fB[global]\fP
 <option>      Sets (long) option value. Short options (i.e. z=auto)
               are not supported.
 .TP
-\fB[plugins]\fP
-disable       Comma separated list of plugins to disable.
-.br
-enable        Comma separated list of plugins to enable.
+\fB[component]\fP
+Each component will have a separate section, and it will support the options
+that particular component provides. These are readily identifiable in the
+\fB--help\fP output for each component, E.G. \fBsos report --help\fP.
 .TP
-\fB[tunables]\fP
-plugin.option Alter available options for defined plugin.
+\fB[plugin_options]\fP
+Alter available options for defined (and loaded) plugins.
+
+Takes the form plugin.option = value, for example \fBrpm.rpmva = true\fP.
 .SH EXAMPLES
 To use quiet and batch mode with 10 threads:
 .LP
-[general]
+[global]
 .br
 batch=yes
 .br
@@ -51,19 +51,21 @@ threads=10
 .sp
 To disable the 'host' and 'filesys' plugins:
 .LP
-[plugins]
+[report]
 .br
-disable = host, filesys
+noplugins = host,filesys
 .sp
 To disable rpm package verification in the RPM plugin:
 .LP
-[tunables]
+[plugin_options]
 .br
 rpm.rpmva = off
 .br
 .SH FILES
 .sp
-/etc/sos.conf
+/etc/sos/sos.conf
 .SH SEE ALSO
 .sp
-sosreport(1)
+sos-report(1)
+sos-collect(1)
+sos-clean(1)
diff --git a/sos.conf b/sos.conf
index ad15af86..4c2c383a 100644
--- a/sos.conf
+++ b/sos.conf
@@ -1,11 +1,36 @@
-[general]
+[global]
+# Set global options here that are not component specific
+# If you would like one global default value to be specifically overridden for
+# just one component, but not others, you may override that value in the
+# component specific section below
 #verbose = 3
 #verify = yes
 #batch = yes
 #log-size = 15
 
-[plugins]
-#disable = rpm, selinux, dovecot
+[report]
+# Options that will apply to any `sos report` run should be listed here.
+# Note that the option names *must* be the long-form name as seen in --help
+# output. Use a comma for list delimitations.
+#skip-plugins = rpm, selinux, dovecot
+#enable-plugins = host,logs
 
-[tunables]
+[collect]
+# Options that will apply to any `sos collect` run should be listed here.
+# Note that the option names *must* be the long-form name as seen in --help
+# output. Use a comma for list delimitations
+#master = myhost.example.com
+#ssh-key = /home/user/.ssh/mykey
+#password = true
+
+[clean]
+# Options that will apply to any `sos clean|mask` run should be listed here.
+# Note that the option names *must* be the long-form name as seen in --help
+# output. Use a comma for list delimitations
+#domains = mydomain.com
+#no-update = true
+
+[plugin_options]
+# Specify any plugin options and their values here. These options take the form
+# plugin_name.option_name = value
 #rpm.rpmva = off
diff --git a/sos/__init__.py b/sos/__init__.py
index a5c69ca0..f9de58cc 100644
--- a/sos/__init__.py
+++ b/sos/__init__.py
@@ -127,7 +127,8 @@ class SoS():
         global_grp.add_argument("--batch", default=False, action="store_true",
                                 help="Do not prompt interactively")
         global_grp.add_argument("--config-file", type=str, action="store",
-                                dest="config_file", default="/etc/sos.conf",
+                                dest="config_file",
+                                default="/etc/sos/sos.conf",
                                 help="specify alternate configuration file")
         global_grp.add_argument("--debug", action="store_true", dest="debug",
                                 help="enable interactive debugging using the "
@@ -173,6 +174,7 @@ class SoS():
             if _to_load.root_required and not os.getuid() == 0:
                 raise Exception("Component must be run with root privileges")
             self._component = _to_load(self.parser, self.args, self.cmdline)
+
         except Exception as err:
             print("Could not initialize '%s': %s" % (_com, err))
             if self.args.debug:
diff --git a/sos/component.py b/sos/component.py
index 0b0b7914..462e3f33 100644
--- a/sos/component.py
+++ b/sos/component.py
@@ -53,7 +53,7 @@ class SoSComponent():
     _arg_defaults = {
         "batch": False,
         "compression_type": 'auto',
-        "config_file": '/etc/sos.conf',
+        "config_file": '/etc/sos/sos.conf',
         "debug": False,
         "encrypt_key": None,
         "encrypt_pass": None,
@@ -158,6 +158,41 @@ class SoSComponent():
         """
         pass
 
+    def apply_options_from_cmdline(self, opts):
+        """(Re-)apply options specified via the cmdline to an options instance
+
+        There are several cases where we may need to re-apply the options from
+        the cmdline over previously loaded options - for instance when an
+        option is specified in both a config file and cmdline, or a preset and
+        the cmdline, or all three.
+
+        Use this to re-apply cmdline option overrides to anything that may
+        change the default values of options
+
+        Positional arguments:
+
+            :param opts:        SoSOptions object to update
+
+        """
+        # override the values from config files with the values from the
+        # cmdline iff that value was explicitly specified, and compare it to
+        # the _current_ set of opts from the config files as a default
+        cmdopts = SoSOptions().from_args(
+            self.parser.parse_args(self.cmdline),
+            arg_defaults=opts.dict(preset_filter=False)
+        )
+        # we can't use merge() here, as argparse will pass default values for
+        # unset options which would effectively negate config file settings and
+        # set all values back to their normal default
+        codict = cmdopts.dict(preset_filter=False)
+        for opt, val in codict.items():
+            if opt not in cmdopts.arg_defaults.keys():
+                continue
+            if val and val != opts.arg_defaults[opt]:
+                setattr(opts, opt, val)
+
+        return opts
+
     def load_options(self):
         """Compile arguments loaded from defaults, config files, and the
         command line into a usable set of options
@@ -169,12 +204,13 @@ class SoSComponent():
             if option.default != SUPPRESS:
                 option.default = None
 
-        # load values from cmdline
-        cmdopts = SoSOptions().from_args(self.parser.parse_args(self.cmdline))
-        opts.merge(cmdopts)
+        opts.update_from_conf(self.args.config_file, self.args.component)
+        if os.getuid() != 0:
+            userconf = os.path.join(Path.home(), '.config/sos/sos.conf')
+            if os.path.exists(userconf):
+                opts.update_from_conf(userconf, self.args.component)
 
-        # load values from config file
-        opts.update_from_conf(opts.config_file)
+        opts = self.apply_options_from_cmdline(opts)
 
         return opts
 
diff --git a/sos/options.py b/sos/options.py
index 3b2b5085..c398dfae 100644
--- a/sos/options.py
+++ b/sos/options.py
@@ -7,7 +7,8 @@
 # See the LICENSE file in the source distribution for further information.
 
 from argparse import Action
-from configparser import ConfigParser, ParsingError, Error
+from configparser import (ConfigParser, ParsingError, Error,
+                          DuplicateOptionError)
 
 
 def _is_seq(val):
@@ -106,7 +107,7 @@ class SoSOptions():
             setattr(self, arg, kwargs[arg])
 
     @classmethod
-    def from_args(cls, args):
+    def from_args(cls, args, arg_defaults={}):
         """Initialise a new SoSOptions object from a ``Namespace``
             obtained by parsing command line arguments.
 
@@ -114,7 +115,7 @@ class SoSOptions():
             :returns: an initialised SoSOptions object
             :returntype: SoSOptions
         """
-        opts = SoSOptions(**vars(args))
+        opts = SoSOptions(**vars(args), arg_defaults=arg_defaults)
         opts._merge_opts(args, True)
         return opts
 
@@ -169,58 +170,59 @@ class SoSOptions():
                                 % (key, conf))
         return val
 
-    def update_from_conf(self, config_file):
+    def update_from_conf(self, config_file, component):
+        """Read the provided config_file and update options from that.
+
+        Positional arguments:
+
+            :param config_file:             Filepath to the config file
+            :param component:               Which component (section) to load
+        """
+
+        def _update_from_section(section, config):
+            if config.has_section(section):
+                odict = dict(config.items(section))
+                # handle verbose explicitly
+                if 'verbose' in odict.keys():
+                    odict['verbosity'] = int(odict.pop('verbose'))
+                # convert options names
+                for key in odict.keys():
+                    if '-' in key:
+                        odict[key.replace('-', '_')] = odict.pop(key)
+                # set the values according to the config file
+                for key, val in odict.items():
+                    if isinstance(val, str):
+                        val = val.replace(' ', '')
+                    if key not in self.arg_defaults:
+                        # read an option that is not loaded by the current
+                        # SoSComponent
+                        print("Unknown option '%s' in section '%s'"
+                              % (key, section))
+                        continue
+                    val = self._convert_to_type(key, val, config_file)
+                    setattr(self, key, val)
+
         config = ConfigParser()
         try:
             try:
                 with open(config_file) as f:
                     config.readfp(f)
-            except (ParsingError, Error) as e:
+            except DuplicateOptionError as err:
+                raise exit("Duplicate option '%s' in section '%s' in file %s"
+                           % (err.option, err.section, config_file))
+            except (ParsingError, Error):
                 raise exit('Failed to parse configuration file %s'
                            % config_file)
         except (OSError, IOError) as e:
             raise exit('Unable to read configuration file %s : %s'
                        % (config_file, e.args[1]))
 
-        if config.has_section("general"):
-            odict = dict(config.items("general"))
-            # handle verbose explicitly
-            if 'verbose' in odict.keys():
-                odict['verbosity'] = int(odict.pop('verbose'))
-            # convert options names
-            for key in odict.keys():
-                if '-' in key:
-                    odict[key.replace('-', '_')] = odict.pop(key)
-            # set the values according to the config file
-            for key, val in odict.items():
-                if key not in self.arg_defaults:
-                    # read an option that is not loaded by the current
-                    # SoSComponent
-                    continue
-                val = self._convert_to_type(key, val, config_file)
-                setattr(self, key, val)
-
-        # report specific checks
-
-        if hasattr(self, 'noplugins'):
-            if config.has_option("plugins", "disable"):
-                self.noplugins.extend([
-                    plugin.strip() for plugin in
-                    config.get("plugins", "disable").split(',')
-                ])
-
-        if hasattr(self, 'enableplugins'):
-            if config.has_option("plugins", "enable"):
-                self.enableplugins.extend([
-                    plugin.strip() for plugin in
-                    config.get("plugins", "enable").split(',')
-                ])
-
-        if hasattr(self, 'plugopts'):
-            if config.has_section("tunables"):
-                for opt, val in config.items("tunables"):
-                    if not opt.split('.')[0] in opts.noplugins:
-                        self.plugopts.append(opt + "=" + val)
+        _update_from_section("global", config)
+        _update_from_section(component, config)
+        if config.has_section("plugin_options") and hasattr(self, 'plugopts'):
+            for key, val in config.items("plugin_options"):
+                if not key.split('.')[0] in self.skip_plugins:
+                    self.plugopts.append(key + '=' + val)
 
     def merge(self, src, skip_default=True):
         """Merge another set of ``SoSOptions`` into this object.
@@ -237,7 +239,7 @@ class SoSOptions():
             if getattr(src, arg) is not None or not skip_default:
                 self._merge_opt(arg, src, False)
 
-    def dict(self):
+    def dict(self, preset_filter=True):
         """Return this ``SoSOptions`` option values as a dictionary of
             argument name to value mappings.
 
@@ -247,8 +249,9 @@ class SoSOptions():
         for arg in self.arg_names:
             value = getattr(self, arg)
             # Do not attempt to store preset option values in presets
-            if arg in ('add_preset', 'del_preset', 'desc', 'note'):
-                value = None
+            if preset_filter:
+                if arg in ('add_preset', 'del_preset', 'desc', 'note'):
+                    value = None
             odict[arg] = value
         return odict
 
diff --git a/sos/report/__init__.py b/sos/report/__init__.py
index 16f7f2aa..74454806 100644
--- a/sos/report/__init__.py
+++ b/sos/report/__init__.py
@@ -85,7 +85,7 @@ class SoSReport(SoSComponent):
         'domains': [],
         'dry_run': False,
         'experimental': False,
-        'enableplugins': [],
+        'enable_plugins': [],
         'keywords': [],
         'plugopts': [],
         'label': '',
@@ -94,13 +94,13 @@ class SoSReport(SoSComponent):
         'list_profiles': False,
         'log_size': 25,
         'map_file': '/etc/sos/cleaner/default_mapping',
-        'noplugins': [],
+        'skip_plugins': [],
         'noreport': False,
         'no_env_vars': False,
         'no_postproc': False,
         'no_update': False,
         'note': '',
-        'onlyplugins': [],
+        'only_plugins': [],
         'preset': 'auto',
         'plugin_timeout': 300,
         'profiles': [],
@@ -148,6 +148,8 @@ class SoSReport(SoSComponent):
             self.preset = self.policy.probe_preset()
         # now merge preset options to self.opts
         self.opts.merge(self.preset.opts)
+        # re-apply any cmdline overrides to the preset
+        self.opts = self.apply_options_from_cmdline(self.opts)
 
         self._set_directories()
 
@@ -207,7 +209,7 @@ class SoSReport(SoSComponent):
                                 dest="experimental", default=False,
                                 help="enable experimental plugins")
         report_grp.add_argument("-e", "--enable-plugins", action="extend",
-                                dest="enableplugins", type=str,
+                                dest="enable_plugins", type=str,
                                 help="enable these plugins", default=[])
         report_grp.add_argument("-k", "--plugin-option", action="extend",
                                 dest="plugopts", type=str,
@@ -231,7 +233,7 @@ class SoSReport(SoSComponent):
                                 help="limit the size of collected logs "
                                      "(in MiB)")
         report_grp.add_argument("-n", "--skip-plugins", action="extend",
-                                dest="noplugins", type=str,
+                                dest="skip_plugins", type=str,
                                 help="disable these plugins", default=[])
         report_grp.add_argument("--no-report", action="store_true",
                                 dest="noreport", default=False,
@@ -245,7 +247,7 @@ class SoSReport(SoSComponent):
         report_grp.add_argument("--note", type=str, action="store", default="",
                                 help="Behaviour notes for new preset")
         report_grp.add_argument("-o", "--only-plugins", action="extend",
-                                dest="onlyplugins", type=str,
+                                dest="only_plugins", type=str,
                                 help="enable these plugins only", default=[])
         report_grp.add_argument("--preset", action="store", type=str,
                                 help="A preset identifier", default="auto")
@@ -421,31 +423,31 @@ class SoSReport(SoSComponent):
                                   dest=os.path.join('sos_logs', 'ui.log'))
 
     def _is_in_profile(self, plugin_class):
-        onlyplugins = self.opts.onlyplugins
+        only_plugins = self.opts.only_plugins
         if not len(self.opts.profiles):
             return True
         if not hasattr(plugin_class, "profiles"):
             return False
-        if onlyplugins and not self._is_not_specified(plugin_class.name()):
+        if only_plugins and not self._is_not_specified(plugin_class.name()):
             return True
         return any([p in self.opts.profiles for p in plugin_class.profiles])
 
     def _is_skipped(self, plugin_name):
-        return (plugin_name in self.opts.noplugins)
+        return (plugin_name in self.opts.skip_plugins)
 
     def _is_inactive(self, plugin_name, pluginClass):
         return (not pluginClass(self.get_commons()).check_enabled() and
-                plugin_name not in self.opts.enableplugins and
-                plugin_name not in self.opts.onlyplugins)
+                plugin_name not in self.opts.enable_plugins and
+                plugin_name not in self.opts.only_plugins)
 
     def _is_not_default(self, plugin_name, pluginClass):
         return (not pluginClass(self.get_commons()).default_enabled() and
-                plugin_name not in self.opts.enableplugins and
-                plugin_name not in self.opts.onlyplugins)
+                plugin_name not in self.opts.enable_plugins and
+                plugin_name not in self.opts.only_plugins)
 
     def _is_not_specified(self, plugin_name):
-        return (self.opts.onlyplugins and
-                plugin_name not in self.opts.onlyplugins)
+        return (self.opts.only_plugins and
+                plugin_name not in self.opts.only_plugins)
 
     def _skip(self, plugin_class, reason="unknown"):
         self.skipped_plugins.append((
@@ -596,9 +598,9 @@ class SoSReport(SoSComponent):
 
     def _check_for_unknown_plugins(self):
         import itertools
-        for plugin in itertools.chain(self.opts.onlyplugins,
-                                      self.opts.noplugins,
-                                      self.opts.enableplugins):
+        for plugin in itertools.chain(self.opts.only_plugins,
+                                      self.opts.skip_plugins,
+                                      self.opts.enable_plugins):
             plugin_name = plugin.split(".")[0]
             if plugin_name not in self.plugin_names:
                 self.soslog.fatal('a non-existing plugin (%s) was specified '
@@ -1294,8 +1296,8 @@ class SoSReport(SoSComponent):
         self.report_md.add_section('devices')
         for key, value in self.devices.items():
             self.report_md.devices.add_list(key, value)
-        self.report_md.add_list('enabled_plugins', self.opts.enableplugins)
-        self.report_md.add_list('disabled_plugins', self.opts.noplugins)
+        self.report_md.add_list('enabled_plugins', self.opts.enable_plugins)
+        self.report_md.add_list('disabled_plugins', self.opts.skip_plugins)
         self.report_md.add_section('plugins')
 
     def execute(self):
-- 
cgit