From 1971af26a403c885a7ed106f90e487387451b619 Mon Sep 17 00:00:00 2001 From: Andrew Mirsky Date: Fri, 4 Jul 2025 16:05:55 -0400 Subject: [PATCH] match yaml 'plugins' format as dictionary with python dictionary format; allow for list as well as dictionary, in case that format slips in --- amqtt/plugins/manager.py | 39 ++++++------ amqtt/scripts/default_broker.yaml | 12 ++-- amqtt/scripts/default_client.yaml | 2 +- docs/custom_plugins.md | 6 +- docs/packaged_plugins.md | 57 +++++++++--------- tests/plugins/mocks.py | 1 + tests/plugins/test_config.py | 98 +++++++++++++++++++++++++++++++ 7 files changed, 159 insertions(+), 56 deletions(-) diff --git a/amqtt/plugins/manager.py b/amqtt/plugins/manager.py index 301d6b6..ced928e 100644 --- a/amqtt/plugins/manager.py +++ b/amqtt/plugins/manager.py @@ -88,8 +88,20 @@ class PluginManager(Generic[C]): if "topic-check" in self.app_context.config: self.logger.warning("Loading plugins from config will ignore 'topic-check' section of config") - plugin_list: list[Any] = self.app_context.config.get("plugins", []) - self._load_str_plugins(plugin_list) + plugins_config: list[Any] | dict[str, Any] = self.app_context.config.get("plugins", []) + if isinstance(plugins_config, list): + plugins_info: dict[str, Any] = {} + for plugin_config in plugins_config: + if isinstance(plugin_config, str): + plugins_info.update({plugin_config: {}}) + elif not isinstance(plugin_config, dict): + msg = "malformed 'plugins' configuration" + raise PluginLoadError(msg) + else: + plugins_info.update(plugin_config) + self._load_str_plugins(plugins_info) + if isinstance(plugins_config, dict): + self._load_str_plugins(plugins_config) else: if not namespace: msg = "Namespace needs to be provided for EntryPoint plugin definitions" @@ -163,35 +175,24 @@ class PluginManager(Generic[C]): self.logger.debug(f"Plugin init failed: {ep!r}", exc_info=True) raise PluginInitError(ep) from e - def _load_str_plugins(self, plugin_list: list[Any]) -> None: + def _load_str_plugins(self, plugins_info: dict[str, Any]) -> None: self.logger.info("Loading plugins from config") self._is_topic_filtering_enabled = True self._is_auth_filtering_enabled = True - for plugin_info in plugin_list: - - if isinstance(plugin_info, dict): - if len(plugin_info.keys()) > 1: - msg = f"config file should have only one key: {plugin_info.keys()}" - raise ValueError(msg) - plugin_path = next(iter(plugin_info.keys())) - plugin_cfg = plugin_info[plugin_path] - plugin = self._load_str_plugin(plugin_path, plugin_cfg) - elif isinstance(plugin_info, str): - plugin = self._load_str_plugin(plugin_info, {}) - else: - msg = "Unexpected entry in plugins config" - raise PluginLoadError(msg) + for plugin_path, plugin_config in plugins_info.items(): + plugin = self._load_str_plugin(plugin_path, plugin_config) self._plugins.append(plugin) + if isinstance(plugin, BaseAuthPlugin): if not iscoroutinefunction(plugin.authenticate): - msg = f"Auth plugin {plugin_info} has non-async authenticate method." + msg = f"Auth plugin {plugin_path} has non-async authenticate method." raise PluginCoroError(msg) self._auth_plugins.append(plugin) if isinstance(plugin, BaseTopicPlugin): if not iscoroutinefunction(plugin.topic_filtering): - msg = f"Topic plugin {plugin_info} has non-async topic_filtering method." + msg = f"Topic plugin {plugin_path} has non-async topic_filtering method." raise PluginCoroError(msg) self._topic_plugins.append(plugin) diff --git a/amqtt/scripts/default_broker.yaml b/amqtt/scripts/default_broker.yaml index d05b7f8..215412c 100644 --- a/amqtt/scripts/default_broker.yaml +++ b/amqtt/scripts/default_broker.yaml @@ -4,9 +4,9 @@ listeners: type: tcp bind: 0.0.0.0:1883 plugins: - - amqtt.plugins.logging_amqtt.EventLoggerPlugin: - - amqtt.plugins.logging_amqtt.PacketLoggerPlugin: - - amqtt.plugins.authentication.AnonymousAuthPlugin: - allow_anonymous: true - - amqtt.plugins.sys.broker.BrokerSysPlugin: - sys_interval: 20 + amqtt.plugins.logging_amqtt.EventLoggerPlugin: + amqtt.plugins.logging_amqtt.PacketLoggerPlugin: + amqtt.plugins.authentication.AnonymousAuthPlugin: + allow_anonymous: true + amqtt.plugins.sys.broker.BrokerSysPlugin: + sys_interval: 20 \ No newline at end of file diff --git a/amqtt/scripts/default_client.yaml b/amqtt/scripts/default_client.yaml index 1fd91be..b686f84 100644 --- a/amqtt/scripts/default_client.yaml +++ b/amqtt/scripts/default_client.yaml @@ -10,4 +10,4 @@ reconnect_retries: 2 broker: uri: "mqtt://127.0.0.1" plugins: - - amqtt.plugins.logging_amqtt.PacketLoggerPlugin: + amqtt.plugins.logging_amqtt.PacketLoggerPlugin: diff --git a/docs/custom_plugins.md b/docs/custom_plugins.md index 69c80ec..346babb 100644 --- a/docs/custom_plugins.md +++ b/docs/custom_plugins.md @@ -40,9 +40,9 @@ dictionary passed to the `Broker` or `MQTTClient`). ... ... plugins: - - module.submodule.file.OneClassName: - - module.submodule.file.TwoClassName: - option1: 123 + module.submodule.file.OneClassName: + module.submodule.file.TwoClassName: + option1: 123 ``` ??? warning "Deprecated: activating plugins using `EntryPoints`" diff --git a/docs/packaged_plugins.md b/docs/packaged_plugins.md index 5adf827..da2c6f2 100644 --- a/docs/packaged_plugins.md +++ b/docs/packaged_plugins.md @@ -47,11 +47,10 @@ By default, the `PacketLoggerPlugin` is activated and configured for the clien ```yaml plugins: - - ... - - amqtt.plugins.authentication.AnonymousAuthPlugin: - allow_anonymous: false - - ... - + . + . + amqtt.plugins.authentication.AnonymousAuthPlugin: + allow_anonymous: false ``` !!! danger @@ -78,10 +77,8 @@ clients are authorized by providing username and password, compared against file ```yaml plugins: - - ... - - amqtt.plugins.authentication.FileAuthPlugin: - password_file: /path/to/password_file - - ... + amqtt.plugins.authentication.FileAuthPlugin: + password_file: /path/to/password_file ``` ??? warning "EntryPoint-style configuration is deprecated" @@ -119,9 +116,7 @@ Prevents using topics named: `prohibited`, `top-secret`, and `data/classified` ```yaml plugins: - - ... - - amqtt.plugins.topic_checking.TopicTabooPlugin: - - ... + amqtt.plugins.topic_checking.TopicTabooPlugin: ``` ??? warning "EntryPoint-style configuration is deprecated" @@ -139,13 +134,13 @@ plugins: **Configuration** -- `acl` *(mapping)*: determines subscription access; if `publish-acl` is not specified, determine both publish and subscription access. +- `acl` *(mapping)*: determines subscription access The list should be a key-value pair, where: -`:[, , ...]` *(string, list[string])*: username of the client followed by a list of allowed topics (wildcards are supported: `#`, `+`). + `:[, , ...]` *(string, list[string])*: username of the client followed by a list of allowed topics (wildcards are supported: `#`, `+`). -- `publish-acl` *(mapping)*: determines publish access. This parameter defines the list of access control rules; each item is a key-value pair, where: -`:[, , ...]` *(string, list[string])*: username of the client followed by a list of allowed topics (wildcards are supported: `#`, `+`). +- `publish-acl` *(mapping)*: determines publish access. If absent, no restrictions are placed on client publishing. + `:[, , ...]` *(string, list[string])*: username of the client followed by a list of allowed topics (wildcards are supported: `#`, `+`). !!! info "Reserved usernames" @@ -154,13 +149,13 @@ plugins: ```yaml plugins: - - ... - - amqtt.plugins.topic_checking.TopicAccessControlListPlugin: - publish_acl: - - username: ["list", "of", "allowed", "topics", "for", "publishing"] - acl: - - username: ["list", "of", "allowed", "topics", "for", "subscribing"] - - ... + amqtt.plugins.topic_checking.TopicAccessControlListPlugin: + acl: + - username: ["list", "of", "allowed", "topics", "for", "subscribing"] + - . + publish_acl: + - username: ["list", "of", "allowed", "topics", "for", "publishing"] + - . ``` ??? warning "EntryPoint-style configuration is deprecated" @@ -186,12 +181,11 @@ Publishes, on a periodic basis, statistics about the broker **Configuration** - `sys_interval` - int, seconds between updates + ```yaml plugins: - - ... - - amqtt.plugins.sys.broker.BrokerSysPlugin: - sys_interval: 20 # int, seconds between updates - - ... + amqtt.plugins.sys.broker.BrokerSysPlugin: + sys_interval: 20 # int, seconds between updates ``` **Supported Topics** @@ -231,6 +225,11 @@ This plugin issues log messages when [broker and mqtt events](custom_plugins.md# - info level messages for `client connected` and `client disconnected` - debug level for all others +```yaml +plugins: + amqtt.plugins.logging_amqtt.EventLoggerPlugin: +``` + ### Packet Logger @@ -239,3 +238,7 @@ This plugin issues log messages when [broker and mqtt events](custom_plugins.md# This plugin issues debug-level messages for [mqtt events](custom_plugins.md#client-and-broker): `on_mqtt_packet_sent` and `on_mqtt_packet_received`. +```yaml +plugins: + amqtt.plugins.logging_amqtt.PacketLoggerPlugin: +``` diff --git a/tests/plugins/mocks.py b/tests/plugins/mocks.py index def9920..9166c64 100644 --- a/tests/plugins/mocks.py +++ b/tests/plugins/mocks.py @@ -25,6 +25,7 @@ class TestConfigPlugin(BasePlugin): class Config: option1: int option2: str + option3: int = 20 class TestCoroErrorPlugin(BaseAuthPlugin): diff --git a/tests/plugins/test_config.py b/tests/plugins/test_config.py index cd31c3d..3d7fb31 100644 --- a/tests/plugins/test_config.py +++ b/tests/plugins/test_config.py @@ -219,3 +219,101 @@ async def test_block_topic_plugin_load(): await client1.disconnect() await broker.shutdown() + +plugin_yaml_list_config_one = """--- +listeners: + default: + type: tcp + bind: 0.0.0.0:1883 +plugins: + - tests.plugins.mocks.TestSimplePlugin: + - tests.plugins.mocks.TestConfigPlugin: + option1: 1 + option2: bar + option3: 3 +""" + +plugin_yaml_list_config_two = """--- +listeners: + default: + type: tcp + bind: 0.0.0.0:1883 +plugins: + - tests.plugins.mocks.TestSimplePlugin: + - tests.plugins.mocks.TestConfigPlugin: + option1: 1 + option2: bar + option3: 3 +""" + +plugin_yaml_dict_config = """--- +listeners: + default: + type: tcp + bind: 0.0.0.0:1883 +plugins: + tests.plugins.mocks.TestSimplePlugin: + tests.plugins.mocks.TestConfigPlugin: + option1: 1 + option2: bar + option3: 3 +""" + +plugin_empty_dict_config = { + 'listeners': {'default': {'type': 'tcp', 'bind': '127.0.0.1'}}, + 'plugins': { + 'tests.plugins.mocks.TestSimplePlugin': {}, + } +} + +plugin_dict_option_config = { + 'listeners': {'default': {'type': 'tcp', 'bind': '127.0.0.1'}}, + 'plugins': { + 'tests.plugins.mocks.TestConfigPlugin': {'option1': 1, 'option2': 'bar', 'option3': 3} + } +} + +@pytest.mark.asyncio +async def test_plugin_yaml_list_config(): + cfg: dict[str, Any] = yaml.load(plugin_yaml_list_config_one, Loader=Loader) + broker = Broker(config=cfg) + + await asyncio.sleep(0.5) + plugin = broker.plugins_manager.get_plugin('TestConfigPlugin') + assert getattr(plugin.context.config, 'option1', None) == 1 + assert getattr(plugin.context.config, 'option3', None) == 3 + + cfg: dict[str, Any] = yaml.load(plugin_yaml_list_config_two, Loader=Loader) + broker = Broker(config=cfg) + + await asyncio.sleep(0.5) + plugin = broker.plugins_manager.get_plugin('TestConfigPlugin') + assert getattr(plugin.context.config, 'option1', None) == 1 + assert getattr(plugin.context.config, 'option3', None) == 3 + + +@pytest.mark.asyncio +async def test_plugin_yaml_dict_config(): + cfg: dict[str, Any] = yaml.load(plugin_yaml_dict_config, Loader=Loader) + broker = Broker(config=cfg) + + await asyncio.sleep(0.5) + assert broker.plugins_manager.get_plugin('TestSimplePlugin') is not None + + +@pytest.mark.asyncio +async def test_plugin_empty_dict_config(): + broker = Broker(config=plugin_empty_dict_config) + + await asyncio.sleep(0.5) + assert broker.plugins_manager.get_plugin('TestSimplePlugin') is not None + + +@pytest.mark.asyncio +async def test_plugin_option_dict_config(): + broker = Broker(config=plugin_dict_option_config) + + await asyncio.sleep(0.5) + plugin = broker.plugins_manager.get_plugin('TestConfigPlugin') + assert getattr(plugin.context.config, 'option1', None) == 1 + assert getattr(plugin.context.config, 'option3', None) == 3