fix(plugins): fix backward compatibility

apps/emqx/src/emqx_hookpoints.erl

@@ -16,6 +16,8 @@
 
 -module(emqx_hookpoints).
 
+-include("logger.hrl").
+
 -type callback_result() :: stop | any().
 -type fold_callback_result(Acc) :: {stop, Acc} | {ok, Acc} | stop | any().
 
@@ -62,12 +64,16 @@
     'delivery.dropped',
     'delivery.completed',
     'cm.channel.unregistered',
-    'tls_handshake.psk_lookup',
+    'tls_handshake.psk_lookup'
+]).
 
+%% Our template plugin used this hookpoints before its 5.1.0 version,
+%% so we keep them here
+-define(DEPRECATED_HOOKPOINTS, [
     %% This is a deprecated hookpoint renamed to 'client.authorize'
-    %% However, our template plugin used this hookpoint before its 5.1.0 version,
-    %% so we keep it here
-    'client.check_acl'
+    'client.check_acl',
+    %% Misspelled hookpoint
+    'session.takeovered'
 ]).
 
 %%-----------------------------------------------------------------------------
@@ -206,27 +212,42 @@ when
 %% API
 %%-----------------------------------------------------------------------------
 
+%% Binary hookpoint names are dynamic and used for bridges
+-type registered_hookpoint() :: atom().
+-type registered_hookpoint_status() :: valid | deprecated.
+
+-spec default_hookpoints() -> #{registered_hookpoint() => registered_hookpoint_status()}.
 default_hookpoints() ->
-    ?HOOKPOINTS.
+    maps:merge(
+        maps:from_keys(?HOOKPOINTS, valid),
+        maps:from_keys(?DEPRECATED_HOOKPOINTS, deprecated)
+    ).
 
+-spec register_hookpoints() -> ok.
 register_hookpoints() ->
     register_hookpoints(default_hookpoints()).
 
-register_hookpoints(HookPoints) ->
-    persistent_term:put(?MODULE, maps:from_keys(HookPoints, true)).
+-spec register_hookpoints(
+    [registered_hookpoint()] | #{registered_hookpoint() => registered_hookpoint_status()}
+) -> ok.
+register_hookpoints(HookPoints) when is_list(HookPoints) ->
+    register_hookpoints(maps:from_keys(HookPoints, valid));
+register_hookpoints(HookPoints) when is_map(HookPoints) ->
+    persistent_term:put(?MODULE, HookPoints).
 
+-spec verify_hookpoint(registered_hookpoint() | binary()) -> ok | no_return().
 verify_hookpoint(HookPoint) when is_binary(HookPoint) -> ok;
 verify_hookpoint(HookPoint) ->
-    case maps:is_key(HookPoint, registered_hookpoints()) of
-        true ->
-            ok;
-        false ->
-            error({invalid_hookpoint, HookPoint})
+    case maps:find(HookPoint, registered_hookpoints()) of
+        {ok, valid} -> ok;
+        {ok, deprecated} -> ?SLOG(warning, #{msg => deprecated_hookpoint, hookpoint => HookPoint});
+        error -> error({invalid_hookpoint, HookPoint})
     end.
 
 %%-----------------------------------------------------------------------------
 %% Internal API
 %%-----------------------------------------------------------------------------
 
+-spec registered_hookpoints() -> #{registered_hookpoint() => registered_hookpoint_status()}.
 registered_hookpoints() ->
     persistent_term:get(?MODULE, #{}).

apps/emqx_plugins/test/emqx_plugins_SUITE.erl

@@ -29,6 +29,16 @@
 ).
 -define(EMQX_PLUGIN_TEMPLATE_VSN, "5.1.0").
 -define(EMQX_PLUGIN_TEMPLATE_TAG, "5.1.0").
+
+-define(EMQX_PLUGIN_TEMPLATES_LEGACY, [
+    #{
+        vsn => "5.0.0",
+        tag => "5.0.0",
+        release_name => "emqx_plugin_template",
+        app_name => emqx_plugin_template
+    }
+]).
+
 -define(EMQX_ELIXIR_PLUGIN_TEMPLATE_RELEASE_NAME, "elixir_plugin_template").
 -define(EMQX_ELIXIR_PLUGIN_TEMPLATE_URL,
     "https://github.com/emqx/emqx-elixir-plugin/releases/download/"
@@ -290,6 +300,36 @@ t_start_restart_and_stop(Config) ->
     ?assertEqual([], emqx_plugins:list()),
     ok.
 
+t_legacy_plugins({init, Config}) ->
+    Config;
+t_legacy_plugins({'end', _Config}) ->
+    ok;
+t_legacy_plugins(Config) ->
+    lists:foreach(
+        fun(LegacyPlugin) ->
+            test_legacy_plugin(LegacyPlugin, Config)
+        end,
+        ?EMQX_PLUGIN_TEMPLATES_LEGACY
+    ).
+
+test_legacy_plugin(#{app_name := AppName} = LegacyPlugin, _Config) ->
+    #{package := Package} = get_demo_plugin_package(LegacyPlugin#{
+        shdir => emqx_plugins:install_dir(), git_url => ?EMQX_PLUGIN_TEMPLATE_URL
+    }),
+    NameVsn = filename:basename(Package, ?PACKAGE_SUFFIX),
+    ok = emqx_plugins:ensure_installed(NameVsn),
+    %% start
+    ok = emqx_plugins:ensure_started(NameVsn),
+    ok = assert_app_running(AppName, true),
+    ok = assert_app_running(map_sets, true),
+    %% stop
+    ok = emqx_plugins:ensure_stopped(NameVsn),
+    ok = assert_app_running(AppName, false),
+    ok = assert_app_running(map_sets, false),
+    ok = emqx_plugins:ensure_uninstalled(NameVsn),
+    ?assertEqual([], emqx_plugins:list()),
+    ok.
+
 t_enable_disable({init, Config}) ->
     #{package := Package} = get_demo_plugin_package(),
     NameVsn = filename:basename(Package, ?PACKAGE_SUFFIX),

changes/ce/fix-11886.en.md

@@ -0,0 +1,3 @@
+Fixed backward plugin compatibility.
+
+Currently, EMQX validates hookpoint names, so invalid hookspoints cannot be used for registering hooks. However, older versions of plugin templates used some misspelled hookpoints, and so could the real plugins. We allow the old hookpoints to be used for registering hooks, but issue a warning that they are deprecated. As before, these hooks are never called.