Merge pull request #97 from PUNCH-Cyber/plugin_opts_config

Cleanup defining plugin configuration

Author: mlaferrera

CHANGELOG.md

@@ -15,6 +15,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
+- Improve handling of plugin configuration options. Plugin options can now also be in stoq.cfg. (Thanks for feedback @chemberger!)
+- Set default precendence for plugin configuration options to be 1) `plugin_opts` when instantiating `Stoq`, 2) `stoq.cfg`, 3) Plugin config file (Thanks for feedback @chemberger!)
 - Make formatted exceptions more legible in results
 
 ## [2.0.2] - 2019-01-14

docs/dev/plugin_overview.rst

@@ -19,7 +19,62 @@ For a full listing of all publicly available plugins, check out the `stoQ public
 Configuration
 *************
 
-Each plugin must have an ``.stoq`` configuration file. The configuration file resides in
+Plugins may be provided configuration options in one of four ways. In order of precendece:
+
+    - From the command line
+    - Upon instantiation of `Stoq()`
+    - Defined in `stoq.cfg`
+    - Defined in the plugin's `.stoq` configuration file
+
+.. _pluginconfigcmdline:
+
+Command Line
+------------
+
+When running ``stoq`` from the command line, simply add ``--plugin-opts`` to your arguments
+followed by the desired plugin options. The syntax for plugin options is::
+
+    plugin_name:option=value
+
+For example, if we want to tell the plugin ``dirmon`` to monitor the directory ``/tmp/monitor``
+for new files by setting the option ``source_dir``, the syntax would be::
+
+    dirmon:source_dir=/tmp/monitor
+
+
+.. _pluginconfiginstantiation:
+
+Instantiation
+-------------
+
+When using stoQ as a framework, plugin options may be defined when instantiating ``Stoq`` using the ``plugin_opts``
+argument::
+
+    >>> from stoq import Stoq
+    >>> plugin_options = {'dirmon': {'source_dir': '/tmp/monitor'}}
+    >>> s = Stoq(plugin_opts=plugin_options)
+
+
+.. _pluginconfigstoqcfg:
+
+stoq.cfg
+--------
+
+The recommended location for storing static plugin configuration options is in `stoq.cfg`.  The reason for this
+if all plugin options defined in the plugin's `.stoq` file will be overwritten when the plugin is upgraded.
+
+To define plugin options in `stoq.cfg` simply add a section header of the plugin name, then define the plugin options::
+
+    [dirmon]
+    source_dir = /tmp/monitor
+
+
+.. _pluginconfigpluginstoq:
+
+Plugin .stoq configuration file
+--------------------------------
+
+Each plugin must have a ``.stoq`` configuration file. The configuration file resides in
 the same directory as the plugin module. The plugin's configuration file allows for
 configuring a plugin with default or static settings. The configuration file is a standard
 YAML file and is parsed using the ``configparser`` module. The following is an example
@@ -55,37 +110,14 @@ Additionally, some optional settings may be defined::
 * **options**
     - **min_stoq_version**: Minimum version of stoQ required to work properly. If the version of `stoQ` is less than the version defined, a warning will be raised.
 
-Custom settings may be added as required for plugins, but the plugins must be configured to
-load and set them. For example, our configuration file may be::
-
-    [Core]
-    Name = example_plugin
-    Module = example_plugin
-
-    [Documentation]
-    Author = PUNCH Cyber
-    Version = 0.1
-    Website = https://github.com/PUNCH-Cyber/stoq-plugins-public
-    Description = Example stoQ Plugin
-
-    [worker]
-    source = /tmp
-
-
-Now, in the ``__init__`` method of our plugin class, we can ensure we define the ``source``
-setting under the ``worker`` section of the configuration file::
-
-
-        if plugin_opts and 'source' in plugin_opts:
-            self.source = plugin_opts['source']
-        elif config.has_option('worker', 'source'):
-            self.source = config.get('worker', 'source')
-
+.. note::
+    Plugin options *must* be under the `[options]` section header to be accessible via the other plugin configuration options.J
 
-First, we are checking for any plugin options were provided to ``Stoq`` at instantiation or at the
-:ref:`command line <pluginoptions>`. If not, it will check the plugin's configuration file for
-the ``source`` setting under the ``worker`` section. If ``source`` is defined in either, the
-setting will be made available to the plugin by defining ``self.source``.
+.. warning::
+    Plugin configuration options may be overwritten when a plugin is upgraded. Upgrading plugins is a destructive
+    operation. This will overwrite/remove all data within the plugins directory, to include the plugin configuration
+    file. It is highly recommended that the plugin directory be backed up regularly to ensure important information
+    is not lost, or plugin configuration options be defined in `stoq.cfg`.
 
 
 .. _multiclass:

docs/gettingstarted.rst

@@ -34,6 +34,8 @@ look for ``stoq.cfg`` in ``$STOQ_HOME`` if running from the command line, or ``$
 used as a library.
 
 
+Plugin options may also be defined in `stoq.cfg`. For more information on how
+
 .. _stoqhome:
 
 $STOQ_HOME
@@ -281,32 +283,10 @@ to what stoQ can do. For more command line options in `run` mode, simply run::
 
     $ stoq run -h
 
-.. _pluginoptions:
-
-Plugin Options
---------------
-
-Plugin options allows for configuration settings of plugins to be modified upon instantiation.
-This is extremely useful when you need to change a configuration options on the fly, such as
-our `run` mode example above.
-
-When running ``stoq`` from the command line, simply add ``--plugin-opts`` to your arguments
-followed by the desired plugin options. The syntax for plugin options is::
-
-    plugin_name:option=value
-
-For example, if we want to tell the plugin ``dirmon`` to monitor the directory ``/tmp/monitor``
-for new files by setting the option ``source_dir``, the syntax would be::
-
-    dirmon:source_dir=/tmp/monitor
-
-Additionally, plugin options may be defined when instantiating ``Stoq`` using the ``plugin_opts``
-argument::
-
-    >>> from stoq import Stoq
-    >>> plugin_options = {'dirmon': {'source_dir': '/tmp/monitor'}}
-    >>> s = Stoq(plugin_opts=plugin_options)
+Plugin configuration
+--------------------
 
+Plugin configurations may be defined in several ways, see :ref:`plugin configuration <pluginconfig>`.
 
 RequestMeta Options
 -------------------

docs/installation.rst

@@ -115,7 +115,7 @@ Plugins may be upgraded (or downgraded) by adding the `--upgrade` command line o
 .. warning::
     Upgrading plugins is a destructive operation. This will overwrite/remove all data within the plugins directory,
     to include the plugin configuration file. It is highly recommended that the plugin directory be backed up
-    regularly to ensure important information is not lost.
+    regularly to ensure important information is not lost, or plugin configuration options be defined in `stoq.cfg`.
 
 .. _devenv:
 

stoq/core.py

@@ -392,7 +392,7 @@ def __init__(
             )
             plugin_dir_list = [d.strip() for d in plugin_dir_str.split(',')]
 
-        super().__init__(plugin_dir_list, plugin_opts)
+        super().__init__(plugin_dir_list, plugin_opts, config)
 
         if not providers:
             providers_str = config.get('core', 'providers', fallback='')
@@ -650,7 +650,7 @@ def _single_scan(
             # Normal dispatches are the "1st round" of scanning
             payload.plugins_run['workers'][0].append(plugin_name)
             try:
-                worker_response = plugin.scan(payload, request_meta) # pyre-ignore[16]
+                worker_response = plugin.scan(payload, request_meta)  # pyre-ignore[16]
             except Exception as e:
                 msg = 'worker:failed to scan'
                 self.log.exception(msg)

stoq/plugin_manager.py

@@ -20,7 +20,7 @@
 import configparser
 import importlib.util
 from pkg_resources import parse_version
-from typing import Dict, List, Optional, Set, Tuple, Any
+from typing import Dict, List, Optional, Tuple, Any
 
 from .exceptions import StoqException
 from stoq.plugins import (
@@ -37,8 +37,12 @@
 
 class StoqPluginManager:
     def __init__(
-        self, plugin_dir_list: List[str], plugin_opts: Optional[Dict[str, Dict]] = None
+        self,
+        plugin_dir_list: List[str],
+        plugin_opts: Optional[Dict[str, Dict]] = None,
+        stoq_config: Optional[configparser.ConfigParser] = None,
     ) -> None:
+        self._stoq_config = stoq_config
         self._plugin_opts = {} if plugin_opts is None else plugin_opts
         self._plugin_name_to_info: Dict[str, Tuple[str, configparser.ConfigParser]] = {}
         self._loaded_plugins: Dict[str, BasePlugin] = {}
@@ -68,11 +72,11 @@ def _collect_plugins(self, plugin_dir_list: List[str]) -> None:
                     if not file.endswith('.stoq'):
                         continue
                     plugin_conf_path = os.path.join(root_path, file)
-                    config = configparser.ConfigParser()
+                    plugin_config = configparser.ConfigParser()
                     try:
-                        config.read(plugin_conf_path)
-                        name = config.get('Core', 'Name')
-                        module_name = config.get('Core', 'Module')
+                        plugin_config.read(plugin_conf_path)
+                        plugin_name = plugin_config.get('Core', 'Name')
+                        module_name = plugin_config.get('Core', 'Module')
                     except Exception:
                         self.log.warning(
                             f'Error parsing config file: {plugin_conf_path}',
@@ -82,34 +86,40 @@ def _collect_plugins(self, plugin_dir_list: List[str]) -> None:
                     module_path_pyc = os.path.join(root_path, module_name) + '.pyc'
                     module_path_py = os.path.join(root_path, module_name) + '.py'
                     if os.path.isfile(module_path_pyc):
-                        self._plugin_name_to_info[name] = (module_path_pyc, config)
+                        self._plugin_name_to_info[plugin_name] = (
+                            module_path_pyc,
+                            plugin_config,
+                        )
                     elif os.path.isfile(module_path_py):
-                        self._plugin_name_to_info[name] = (module_path_py, config)
+                        self._plugin_name_to_info[plugin_name] = (
+                            module_path_py,
+                            plugin_config,
+                        )
                     else:
                         self.log.warning(
                             f'Unable to find module at: {module_path_pyc} or {module_path_py}',
                             exc_info=True,
                         )
                         continue
 
-    def load_plugin(self, name: str) -> BasePlugin:
-        name = name.strip()
-        if name in self._loaded_plugins:
-            return self._loaded_plugins[name]
-        module_path, config = self._plugin_name_to_info[name]
-        if config.has_option('options', 'min_stoq_version'):
-            min_stoq_version = config.get('options', 'min_stoq_version')
+    def load_plugin(self, plugin_name: str) -> BasePlugin:
+        plugin_name = plugin_name.strip()
+        if plugin_name in self._loaded_plugins:
+            return self._loaded_plugins[plugin_name]
+        module_path, plugin_config = self._plugin_name_to_info[plugin_name]
+        if plugin_config.has_option('options', 'min_stoq_version'):
+            min_stoq_version = plugin_config.get('options', 'min_stoq_version')
             # Placing this import at the top of this file causes a circular
             # import chain that causes stoq to crash on initialization
             from stoq import __version__
 
             if parse_version(__version__) < parse_version(min_stoq_version):
                 self.log.warning(
-                    f'Plugin {name} not compatible with this version of '
+                    f'Plugin {plugin_name} not compatible with this version of '
                     'stoQ. Unpredictable results may occur!'
                 )
         spec = importlib.util.spec_from_file_location(
-            config.get('Core', 'Module'), module_path
+            plugin_config.get('Core', 'Module'), module_path
         )
         module = importlib.util.module_from_spec(spec)
         spec.loader.exec_module(module)  # pyre-ignore
@@ -132,16 +142,29 @@ def load_plugin(self, name: str) -> BasePlugin:
         )
         if len(plugin_classes) == 0:
             raise StoqException(
-                f'No valid plugin classes found in the module for {name}'
+                f'No valid plugin classes found in the module for {plugin_name}'
             )
         if len(plugin_classes) > 1:
             raise StoqException(
-                f'Multiple possible plugin classes found in the module for {name},'
+                f'Multiple possible plugin classes found in the module for {plugin_name},'
                 ' unable to distinguish which to use.'
             )
         _, plugin_class = plugin_classes[0]
-        plugin = plugin_class(config, self._plugin_opts.get(name))
-        self._loaded_plugins[name] = plugin
+        # Plugin configuration order of precendence:
+        # 1) plugin options provided at instantiation of `Stoq()`
+        # 2) plugin configuration in `stoq.cfg`
+        # 3) `plugin_name.stoq`
+        if isinstance(
+            self._stoq_config, configparser.ConfigParser
+        ) and self._stoq_config.has_section(plugin_name):
+            if not plugin_config.has_section('options'):
+                plugin_config.add_section('options')
+            for opt in self._stoq_config.options(plugin_name):
+                plugin_config['options'][opt] = self._stoq_config.get(plugin_name, opt)
+        if self._plugin_opts.get(plugin_name):
+            plugin_config.read_dict({'options': self._plugin_opts.get(plugin_name)})
+        plugin = plugin_class(plugin_config, self._plugin_opts.get(plugin_name))
+        self._loaded_plugins[plugin_name] = plugin
         return plugin
 
     def list_plugins(self) -> Dict[str, Dict[str, Any]]:

stoq/tests/data/stoq.cfg

@@ -22,3 +22,13 @@ log_dir:
 # What is the maximum size of the provider queue?
 max_queue: 919
 max_recursion: 4
+
+[configurable_worker]
+worker_test_option_bool = True
+worker_test_option_str = Worker Testy McTest Face
+worker_test_option_int = 10
+
+[dummy_connector]
+connector_test_option_bool = False
+connector_test_option_str = Connector Testy McTest Face
+connector_test_option_int = 5

stoq/tests/test_plugin_manager.py

@@ -20,7 +20,7 @@
 import unittest
 
 
-from stoq import StoqException
+from stoq import Stoq, StoqException
 from stoq.data_classes import Payload, WorkerResponse, RequestMeta
 from stoq.plugin_manager import StoqPluginManager
 from stoq.plugins import WorkerPlugin
@@ -110,6 +110,49 @@ def test_plugin_opts(self):
         plugin = pm.load_plugin('configurable_worker')
         self.assertEqual(plugin.get_crazy_runtime_option(), 16)
 
+    def test_plugin_opts_from_stoq_cfg(self):
+        s = Stoq(base_dir=utils.get_data_dir())
+        plugin = s.load_plugin('configurable_worker')
+        self.assertEqual(
+            plugin.config.getboolean('options', 'worker_test_option_bool'), True
+        )
+        self.assertEqual(
+            plugin.config.get('options', 'worker_test_option_str'),
+            'Worker Testy McTest Face',
+        )
+        self.assertEqual(plugin.config.getint('options', 'worker_test_option_int'), 10)
+        plugin = s.load_plugin('dummy_connector')
+        self.assertEqual(
+            plugin.config.getboolean('options', 'connector_test_option_bool'), False
+        )
+        self.assertEqual(
+            plugin.config.get('options', 'Connector_test_option_str'),
+            'Connector Testy McTest Face',
+        )
+        self.assertEqual(
+            plugin.config.getint('options', 'connector_test_option_int'), 5
+        )
+
+    def test_plugin_opts_precedence(self):
+        s = Stoq(
+            base_dir=utils.get_data_dir(),
+            plugin_opts={
+                'configurable_worker': {
+                    'worker_test_option_bool': False,
+                    'worker_test_option_str': 'Test string',
+                    'worker_test_option_int': 20,
+                }
+            },
+        )
+        plugin = s.load_plugin('configurable_worker')
+        self.assertEqual(
+            plugin.config.getboolean('options', 'worker_test_option_bool'), False
+        )
+        self.assertEqual(
+            plugin.config.get('options', 'worker_test_option_str'), 'Test string'
+        )
+        self.assertEqual(plugin.config.getint('options', 'worker_test_option_int'), 20)
+
     def test_min_stoq_version(self):
         pm = StoqPluginManager([utils.get_invalid_plugins_dir()])
         # We have to override the fact that all log calls are disabled in setUp()
@@ -135,7 +178,6 @@ def test_plugin_override(self):
             worker.PLUGINS2_DUP_MARKER
 
 
-
 class ExampleExternalPlugin(WorkerPlugin):
     # Intentionally override this method to not require the config argument
     def __init__(self):