Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

don't filter topics if its explicit disabled #93

Merged
merged 1 commit into from
Apr 14, 2022

Conversation

edenhaus
Copy link
Contributor

During the investigation of #92 I found out, that even when the topic check is disabled via config (topic-check": {"enabled": False}). All plugins will be used to check for topic filtering.

The reason is that in the method topic_filtering the value None is passed for filter_plugins to the method map_plugin_coro

amqtt/amqtt/broker.py

Lines 728 to 738 in 8961b8f

topic_plugins = None
topic_config = self.config.get("topic-check", None)
if topic_config and topic_config.get("enabled", False):
topic_plugins = topic_config.get("plugins", None)
returns = await self.plugins_manager.map_plugin_coro(
"topic_filtering",
session=session,
topic=topic,
action=action,
filter_plugins=topic_plugins,
)

In the manager then, if the value is None all available plugins will be used.

p_list = kwargs.pop("filter_plugins", None)
if p_list is None:
p_list = [p.name for p in self.plugins]

My fix will immediately return True (topic_result) if in the topic-check-configuration explicit enabled was set to False.
If the key enabled is missing all plugins will be used as it was done before (backward compatibility).

@edenhaus
Copy link
Contributor Author

As workaround until this PR is merged, you can add the following to your config:
"topic-check": {"enabled": True, "plugins": []}

@FlyingDiver
Copy link
Contributor

This bit me today as well.

@HerrMuellerluedenscheid
Copy link
Contributor

LGTM @FlorianLudwig

@edenhaus
Copy link
Contributor Author

edenhaus commented Apr 8, 2022

@FlorianLudwig Can this be merged or do I miss something?

@HerrMuellerluedenscheid HerrMuellerluedenscheid merged commit f71b99b into Yakifo:master Apr 14, 2022
@edenhaus edenhaus deleted the topic-check branch April 17, 2022 11:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants