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

Upcoming changes to Mosquitto plugin interface #96

Closed
ralight opened this issue Sep 24, 2020 · 8 comments · Fixed by #99 or #103
Closed

Upcoming changes to Mosquitto plugin interface #96

ralight opened this issue Sep 24, 2020 · 8 comments · Fixed by #99 or #103

Comments

@ralight
Copy link

ralight commented Sep 24, 2020

Hello,

Thanks for spending your time working on this, it's a valuable project! I thought you ought to be aware that the plugin interface is changing in the upcoming 2.0 release of Mosquitto. The older versions will remain supported, but there is a change that you should make. The documentation for mosquitto_auth_plugin_version() stated that you should return MOSQ_AUTH_PLUGIN_VERSION, which you have done. That means that a plugin compiled against the old headers will still work with a newer broker, but if you compile the plugin with the newer headers then the compilation will succeed, but the plugin will be reporting the the broker that it is the incorrect version. TLDR: Please change mosquitto_auth_plugin_version() so that it returns 4, or the version of the plugin interface that you support.

In case you're interested, the plugin support is changing so it is not just about authentication and access control, but covers a new $CONTROL topic mechanism, and message inspection and modification. There are just three functions you need to implement in your plugin (for version, init, and cleanup), then you register callbacks for different events. I hope it should be much simpler to develop for, and more extensible in the future without needing to change the interface.

@iegomez
Copy link
Owner

iegomez commented Sep 24, 2020

Hey, @ralight! Thank you very much for the heads up. I'll follow up the release to make the needed changes.
🙇‍♂️

@iegomez
Copy link
Owner

iegomez commented Oct 3, 2020

@ralight Do you happen to have an ETA for the 2.0 release? Also, I'd be very interested in checking out the upcoming changes you mention to start planning how to support them, are there any docs you can point me to for those?

I was meaning to ask you about both but had a super busy and exhausting week at work with a global production rollout, and will probably be focused on that for a few days to come, but will get to it as soon as I get the time.

Thanks again!

@ralight
Copy link
Author

ralight commented Oct 3, 2020

This month. I'm still working on the docs, but you can see the rough state of play in the develop branch and look at mosquitto_broker.h and mosquitto_plugin.h.

Honestly though, all you need to do for now is to change mosquitto_auth_plugin_version() so that it returns 4 and it will work with 1.6.12 and earlier, and with 2.0. I just don't want your plugin to break if someone compiles it against 2.0, which at the moment it will do.

@iegomez
Copy link
Owner

iegomez commented Oct 4, 2020

Gotcha! 🙇‍♂️

@iegomez
Copy link
Owner

iegomez commented Oct 22, 2020

@ralight Now that I've got some time to check, I saw a user couldn't compile against 1.4.x because I'm explicitly returning 4 instead of MOSQ_AUTH_PLUGIN_VERSION, which reasonably breaks for older interface, so he had to revert back to the previous version of the plugin. Would it be enough to check if MOSQ_AUTH_PLUGIN_VERSION exists and define it to 4 if not to continue supporting older version? I believe so, but since I'm curious why that wasn't your suggestion, I'm wondering if I'm missing something.

Thanks!

@ralight
Copy link
Author

ralight commented Oct 23, 2020

What a mess, sorry. I'm still surprised when people are running that old version.

How about something like this?

#ifdef MOSQ_AUTH_PLUGIN_VERSION
#  if MOSQ_AUTH_PLUGIN_VERSION == 5
     return 4; // This is v2.0, use the backwards compatibility
#  else
     return MOSQ_AUTH_PLUGIN_VERSION;
#  endif
#endif

@iegomez
Copy link
Owner

iegomez commented Oct 23, 2020

@ralight So v2.0 returns 5? Ha, much to figure out yet, but I'll follow your advice in the meantime. Thanks again!

@ralight
Copy link
Author

ralight commented Oct 23, 2020

Right, I've just changed it so that MOSQ_AUTH_PLUGIN_VERSION will only ever be 4 from now on, because the interface has changed to be more generic not just auth/acl. So MOSQ_PLUGIN_VERSION will be 5 onwards, and in fact I could have just not mentioned anything to you at all and it would just work. So sorry again for messing you about.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants