Revert "refactor(emqx_modules): Move modules config to app dir"

This reverts commit 9cae8784c0.
Because the reverted change broke compatibility with 4.2.x
configuration.

apps/emqx_web_hook/test/props/prop_webhook_confs.erl

@@ -62,6 +62,7 @@ do_teardown(_) ->
 
 set_special_cfgs(_) ->
     application:set_env(emqx, plugins_loaded_file, undefined),
+    application:set_env(emqx, modules_loaded_file, undefined),
     ok.
 
 assert_confs([{"web.hook.api.url", Url}|More], Envs) ->

etc/emqx.conf

@@ -2027,6 +2027,64 @@ listener.wss.external.allow_origin_absence = true
 ## Value: http://url eg. https://localhost:8084, https://127.0.0.1:8084
 listener.wss.external.check_origins = https://localhost:8084, https://127.0.0.1:8084
 
+##--------------------------------------------------------------------
+## Modules
+##--------------------------------------------------------------------
+## The file to store loaded module names.
+##
+## Value: File
+modules.loaded_file = {{ platform_data_dir }}/loaded_modules
+
+##--------------------------------------------------------------------
+## Presence Module
+
+## Sets the QoS for presence MQTT message.
+##
+## Value: 0 | 1 | 2
+module.presence.qos = 1
+
+##--------------------------------------------------------------------
+## Subscription Module
+
+## Subscribe the Topics automatically when client connected.
+##
+## Value: String
+## module.subscription.1.topic = connected/%c/%u
+
+## Qos of the proxy subscription.
+##
+## Value: 0 | 1 | 2
+## Default: 0
+## module.subscription.1.qos = 0
+
+## No Local of the proxy subscription options.
+## This configuration only takes effect in the MQTT V5 protocol.
+##
+## Value: 0 | 1
+## Default: 0
+## module.subscription.1.nl = 0
+
+## Retain As Published of the proxy subscription options.
+## This configuration only takes effect in the MQTT V5 protocol.
+##
+## Value: 0 | 1
+## Default: 0
+## module.subscription.1.rap = 0
+
+## Retain Handling of the proxy subscription options.
+## This configuration only takes effect in the MQTT V5 protocol.
+##
+## Value: 0 | 1 | 2
+## Default: 0
+## module.subscription.1.rh = 0
+
+##--------------------------------------------------------------------
+## Rewrite Module
+
+## {rewrite, Topic, Re, Dest}
+## module.rewrite.pub.rule.1 = x/# ^x/y/(.+)$ z/y/$1
+## module.rewrite.sub.rule.1 = y/+/z/# ^y/(.+)/z/(.+)$ y/z/$2
+
 ##-------------------------------------------------------------------
 ## Plugins
 ##-------------------------------------------------------------------

lib-opensource/emqx_modules/etc/emqx_modules.conf

@@ -1,57 +1 @@
-##--------------------------------------------------------------------
+# empty
-## Modules
-##--------------------------------------------------------------------
-## The file to store loaded module names.
-##
-## Value: File
-modules.loaded_file = {{ platform_data_dir }}/loaded_modules
-
-##--------------------------------------------------------------------
-## Presence Module
-
-## Sets the QoS for presence MQTT message.
-##
-## Value: 0 | 1 | 2
-module.presence.qos = 1
-
-##--------------------------------------------------------------------
-## Subscription Module
-
-## Subscribe the Topics automatically when client connected.
-##
-## Value: String
-## module.subscription.1.topic = connected/%c/%u
-
-## Qos of the proxy subscription.
-##
-## Value: 0 | 1 | 2
-## Default: 0
-## module.subscription.1.qos = 0
-
-## No Local of the proxy subscription options.
-## This configuration only takes effect in the MQTT V5 protocol.
-##
-## Value: 0 | 1
-## Default: 0
-## module.subscription.1.nl = 0
-
-## Retain As Published of the proxy subscription options.
-## This configuration only takes effect in the MQTT V5 protocol.
-##
-## Value: 0 | 1
-## Default: 0
-## module.subscription.1.rap = 0
-
-## Retain Handling of the proxy subscription options.
-## This configuration only takes effect in the MQTT V5 protocol.
-##
-## Value: 0 | 1 | 2
-## Default: 0
-## module.subscription.1.rh = 0
-
-##--------------------------------------------------------------------
-## Rewrite Module
-
-## {rewrite, Topic, Re, Dest}
-## module.rewrite.pub.rule.1 = x/# ^x/y/(.+)$ z/y/$1
-## module.rewrite.sub.rule.1 = y/+/z/# ^y/(.+)/z/(.+)$ y/z/$2

lib-opensource/emqx_modules/priv/emqx_modules.schema

@@ -1,89 +1 @@
-%%--------------------------------------------------------------------
+% empty
-%% Modules
-%%--------------------------------------------------------------------
-
-{mapping, "modules.loaded_file", "emqx_modules.modules_loaded_file", [
-  {datatype, string}
-]}.
-
-{mapping, "module.presence.qos", "emqx_modules.modules", [
-  {default, 1},
-  {datatype, integer},
-  {validators, ["range:0-2"]}
-]}.
-
-{mapping, "module.subscription.$id.topic", "emqx_modules.modules", [
-  {datatype, string}
-]}.
-
-{mapping, "module.subscription.$id.qos", "emqx_modules.modules", [
-  {default, 1},
-  {datatype, integer},
-  {validators, ["range:0-2"]}
-]}.
-
-{mapping, "module.subscription.$id.nl", "emqx_modules.modules", [
-  {default, 0},
-  {datatype, integer},
-  {validators, ["range:0-1"]}
-]}.
-
-{mapping, "module.subscription.$id.rap", "emqx_modules.modules", [
-  {default, 0},
-  {datatype, integer},
-  {validators, ["range:0-1"]}
-]}.
-
-{mapping, "module.subscription.$id.rh", "emqx_modules.modules", [
-  {default, 0},
-  {datatype, integer},
-  {validators, ["range:0-2"]}
-]}.
-
-{mapping, "module.rewrite.rule.$id", "emqx_modules.modules", [
-  {datatype, string}
-]}.
-
-{mapping, "module.rewrite.pub.rule.$id", "emqx_modules.modules", [
-  {datatype, string}
-]}.
-
-{mapping, "module.rewrite.sub.rule.$id", "emqx_modules.modules", [
-  {datatype, string}
-]}.
-
-{translation, "emqx_modules.modules", fun(Conf, _, Conf1) ->
-  Subscriptions = fun() ->
-      List = cuttlefish_variable:filter_by_prefix("module.subscription", Conf),
-      TopicList = [{N, Topic}|| {[_,"subscription",N,"topic"], Topic} <- List],
-      [{iolist_to_binary(T), #{ qos => cuttlefish:conf_get("module.subscription." ++ N ++ ".qos", Conf, 0),
-                                nl  => cuttlefish:conf_get("module.subscription." ++ N ++ ".nl", Conf, 0),
-                                rap => cuttlefish:conf_get("module.subscription." ++ N ++ ".rap", Conf, 0),
-                                rh  => cuttlefish:conf_get("module.subscription." ++ N ++ ".rh", Conf, 0)
-                                }} || {N, T} <- TopicList]
-  end,
-  Rewrites = fun() ->
-      Rules = cuttlefish_variable:filter_by_prefix("module.rewrite.rule", Conf),
-      PubRules = cuttlefish_variable:filter_by_prefix("module.rewrite.pub.rule", Conf),
-      SubRules = cuttlefish_variable:filter_by_prefix("module.rewrite.sub.rule", Conf),
-      TotalRules = lists:append(
-                     [ {["module", "rewrite", "pub", "rule", I], Rule} || {["module", "rewrite", "rule", I], Rule} <- Rules] ++ PubRules,
-                     [ {["module", "rewrite", "sub", "rule", I], Rule} || {["module", "rewrite", "rule", I], Rule} <- Rules] ++ SubRules
-                    ),
-      lists:map(fun({[_, "rewrite", PubOrSub, "rule", I], Rule}) ->
-                    [Topic, Re, Dest] = string:tokens(Rule, " "),
-                    {rewrite, list_to_atom(PubOrSub), list_to_binary(Topic), list_to_binary(Re), list_to_binary(Dest)}
-                end, TotalRules)
-  end,
-  lists:append([
-    [{emqx_mod_presence, [{qos, cuttlefish:conf_get("module.presence.qos", Conf, 1)}]}],
-    [{emqx_mod_subscription, Subscriptions()}],
-    [{emqx_mod_rewrite, Rewrites()}],
-    [{emqx_mod_topic_metrics, []}],
-    [{emqx_mod_delayed, []}],
-    %% TODO: acl_file config should be moved to emqx_modules.conf
-    %% when all the plubin tests stops using it in the old way.
-    [{emqx_mod_acl_internal, [{acl_file, {emqx, get_env, [acl_file]}}]}]
-    %[{emqx_mod_acl_internal, [{acl_file, cuttlefish:conf_get("acl_file", Conf1)}]}]
-  ])
-end}.

lib-opensource/emqx_modules/src/emqx_mod_acl_internal.erl

@@ -43,13 +43,7 @@
 %%--------------------------------------------------------------------
 
 load(Env) ->
-    %% TODO: acl_file config should be moved to emqx_modules.conf
+    Rules = rules_from_file(proplists:get_value(acl_file, Env)),
-    %% when all the plubin tests stops using it in the old way.
-    File = case proplists:get_value(acl_file, Env) of
-               {emqx, get_env, _} -> emqx:get_env(acl_file);
-               F -> F
-           end,
-    Rules = rules_from_file(File),
     emqx_hooks:add('client.check_acl', {?MODULE, check_acl, [Rules]},  -1).
 
 unload(_Env) ->

lib-opensource/emqx_modules/src/emqx_modules.erl

@@ -30,8 +30,6 @@
         , load_module/2
         ]).
 
--define(APP, ?MODULE).
-
 %% @doc List all available plugins
 -spec(list() -> [{atom(), boolean()}]).
 list() ->
@@ -40,7 +38,7 @@ list() ->
 %% @doc Load all the extended modules.
 -spec(load() -> ok).
 load() ->
-    case get_env(modules_loaded_file) of
+    case emqx:get_env(modules_loaded_file) of
         undefined -> ok;
         File ->
             load_modules(File)
@@ -61,7 +59,7 @@ load(ModuleName) ->
 %% @doc Unload all the extended modules.
 -spec(unload() -> ok).
 unload() ->
-    case get_env(modules_loaded_file) of
+    case emqx:get_env(modules_loaded_file) of
         undefined -> ignore;
         File ->
             unload_modules(File)
@@ -81,7 +79,7 @@ unload(ModuleName) ->
 
 -spec(reload(module()) -> ok | ignore | {error, any()}).
 reload(emqx_mod_acl_internal) ->
-    Modules = get_env(modules, []),
+    Modules = emqx:get_env(modules, []),
     Env = proplists:get_value(emqx_mod_acl_internal, Modules, undefined),
     case emqx_mod_acl_internal:reload(Env) of
         ok ->
@@ -98,7 +96,7 @@ find_module(ModuleName) ->
     ets:lookup(?MODULE, ModuleName).
 
 filter_module(ModuleNames) ->
-    filter_module(ModuleNames, get_env(modules, [])).
+    filter_module(ModuleNames, emqx:get_env(modules, [])).
 filter_module([], Acc) ->
     Acc;
 filter_module([{ModuleName, true} | ModuleNames], Acc) ->
@@ -125,7 +123,7 @@ load_module(ModuleName) ->
     load_module({ModuleName, true}).
 
 load_module(ModuleName, Persistent) ->
-    Modules = get_env(modules, []),
+    Modules = emqx:get_env(modules, []),
     Env = proplists:get_value(ModuleName, Modules, undefined),
     case ModuleName:load(Env) of
         ok ->
@@ -152,7 +150,7 @@ unload_module(ModuleName) ->
     unload_module({ModuleName, true}).
 
 unload_module(ModuleName, Persistent) ->
-    Modules = get_env(modules, []),
+    Modules = emqx:get_env(modules, []),
     Env = proplists:get_value(ModuleName, Modules, undefined),
     case ModuleName:unload(Env) of
         ok ->
@@ -164,7 +162,7 @@ unload_module(ModuleName, Persistent) ->
     end.
 
 write_loaded(true) ->
-    FilePath = get_env(modules_loaded_file),
+    FilePath = emqx:get_env(modules_loaded_file),
     case file:write_file(FilePath, [io_lib:format("~p.~n", [Name]) || Name <- list()]) of
         ok -> ok;
         {error, Error} ->
@@ -172,7 +170,3 @@ write_loaded(true) ->
             ok
     end;
 write_loaded(false) -> ok.
-
-get_env(Key) -> get_env(Key, undefined).
-
-get_env(Key, Default) -> application:get_env(?APP, Key, Default).

lib-opensource/emqx_modules/src/emqx_modules_app.erl

@@ -24,28 +24,13 @@
 
 -export([stop/1]).
 
--define(APP, emqx_modules).
-
 start(_Type, _Args) ->
     % the configs for emqx_modules is so far still in emqx application
     % Ensure it's loaded
     application:load(emqx),
-    ok = load_app_env(),
     {ok, Pid} = emqx_mod_sup:start_link(),
     ok = emqx_modules:load(),
     {ok, Pid}.
 
 stop(_State) ->
     emqx_modules:unload().
-
-load_app_env() ->
-    Schema = filename:join([code:priv_dir(?APP), "emqx_modules.schema"]),
-    Conf1 = filename:join([code:lib_dir(?APP), "etc", "emqx_modules.conf"]),
-    Conf2 = filename:join([emqx:get_env(plugins_etc_dir), "emqx_modules.conf"]),
-    [ConfFile | _] = lists:filter(fun filelib:is_regular/1, [Conf1, Conf2]),
-    Conf = cuttlefish_conf:file(ConfFile),
-    AppEnv = cuttlefish_generator:map(cuttlefish_schema:files([Schema]), Conf),
-    lists:foreach(fun({AppName, Envs}) ->
-        [application:set_env(AppName, Par, Val) || {Par, Val} <- Envs]
-    end, AppEnv).
-

lib-opensource/emqx_modules/test/emqx_modules_SUITE.erl

@@ -24,20 +24,20 @@
 all() -> emqx_ct:all(?MODULE).
 
 init_per_suite(Config) ->
-    emqx_ct_helpers:start_apps([emqx_modules]),
+    emqx_ct_helpers:start_apps([emqx_modules], fun set_sepecial_cfg/1),
-    File = emqx_ct_helpers:deps_path(emqx_modules, "test/emqx_modules_SUITE_data/loaded_modules"),
-    application:set_env(emqx_modules, modules_loaded_file, File),
-    ok = emqx_modules:unload(),
-    ok = emqx_modules:load(),
     Config.
 
+set_sepecial_cfg(_) ->
+    application:set_env(emqx, modules_loaded_file, emqx_ct_helpers:deps_path(emqx, "test/emqx_SUITE_data/loaded_modules")),
+    ok.
+
 end_per_suite(_Config) ->
     emqx_ct_helpers:stop_apps([emqx_modules]).
 
 t_load(_) ->
     ?assertEqual(ok, emqx_modules:unload()),
     ?assertEqual(ok, emqx_modules:load()),
-    ?assertEqual({error, not_found}, emqx_modules:load(foo)),
+    ?assertEqual({error, not_found}, emqx_modules:load(not_existed_module)),
     ?assertEqual({error, not_started}, emqx_modules:unload(emqx_mod_rewrite)),
     ?assertEqual(ignore, emqx_modules:reload(emqx_mod_rewrite)),
     ?assertEqual(ok, emqx_modules:reload(emqx_mod_acl_internal)).

priv/emqx.schema

@@ -2020,6 +2020,93 @@ end}.
                                                ++ cuttlefish_variable:filter_by_prefix("listener.wss", Conf)])
 end}.
 
+%%--------------------------------------------------------------------
+%% Modules
+%%--------------------------------------------------------------------
+
+{mapping, "modules.loaded_file", "emqx.modules_loaded_file", [
+  {datatype, string}
+]}.
+
+{mapping, "module.presence.qos", "emqx.modules", [
+  {default, 1},
+  {datatype, integer},
+  {validators, ["range:0-2"]}
+]}.
+
+{mapping, "module.subscription.$id.topic", "emqx.modules", [
+  {datatype, string}
+]}.
+
+{mapping, "module.subscription.$id.qos", "emqx.modules", [
+  {default, 1},
+  {datatype, integer},
+  {validators, ["range:0-2"]}
+]}.
+
+{mapping, "module.subscription.$id.nl", "emqx.modules", [
+  {default, 0},
+  {datatype, integer},
+  {validators, ["range:0-1"]}
+]}.
+
+{mapping, "module.subscription.$id.rap", "emqx.modules", [
+  {default, 0},
+  {datatype, integer},
+  {validators, ["range:0-1"]}
+]}.
+
+{mapping, "module.subscription.$id.rh", "emqx.modules", [
+  {default, 0},
+  {datatype, integer},
+  {validators, ["range:0-2"]}
+]}.
+
+{mapping, "module.rewrite.rule.$id", "emqx.modules", [
+  {datatype, string}
+]}.
+
+{mapping, "module.rewrite.pub.rule.$id", "emqx.modules", [
+  {datatype, string}
+]}.
+
+{mapping, "module.rewrite.sub.rule.$id", "emqx.modules", [
+  {datatype, string}
+]}.
+
+{translation, "emqx.modules", fun(Conf, _, Conf1) ->
+  Subscriptions = fun() ->
+      List = cuttlefish_variable:filter_by_prefix("module.subscription", Conf),
+      TopicList = [{N, Topic}|| {[_,"subscription",N,"topic"], Topic} <- List],
+      [{iolist_to_binary(T), #{ qos => cuttlefish:conf_get("module.subscription." ++ N ++ ".qos", Conf, 0),
+                                nl  => cuttlefish:conf_get("module.subscription." ++ N ++ ".nl", Conf, 0),
+                                rap => cuttlefish:conf_get("module.subscription." ++ N ++ ".rap", Conf, 0),
+                                rh  => cuttlefish:conf_get("module.subscription." ++ N ++ ".rh", Conf, 0)
+                                }} || {N, T} <- TopicList]
+  end,
+  Rewrites = fun() ->
+      Rules = cuttlefish_variable:filter_by_prefix("module.rewrite.rule", Conf),
+      PubRules = cuttlefish_variable:filter_by_prefix("module.rewrite.pub.rule", Conf),
+      SubRules = cuttlefish_variable:filter_by_prefix("module.rewrite.sub.rule", Conf),
+      TotalRules = lists:append(
+                     [ {["module", "rewrite", "pub", "rule", I], Rule} || {["module", "rewrite", "rule", I], Rule} <- Rules] ++ PubRules,
+                     [ {["module", "rewrite", "sub", "rule", I], Rule} || {["module", "rewrite", "rule", I], Rule} <- Rules] ++ SubRules
+                    ),
+      lists:map(fun({[_, "rewrite", PubOrSub, "rule", I], Rule}) ->
+                    [Topic, Re, Dest] = string:tokens(Rule, " "),
+                    {rewrite, list_to_atom(PubOrSub), list_to_binary(Topic), list_to_binary(Re), list_to_binary(Dest)}
+                end, TotalRules)
+  end,
+  lists:append([
+    [{emqx_mod_presence, [{qos, cuttlefish:conf_get("module.presence.qos", Conf, 1)}]}],
+    [{emqx_mod_subscription, Subscriptions()}],
+    [{emqx_mod_rewrite, Rewrites()}],
+    [{emqx_mod_topic_metrics, []}],
+    [{emqx_mod_delayed, []}],
+    [{emqx_mod_acl_internal, [{acl_file, cuttlefish:conf_get("acl_file", Conf1)}]}]
+  ])
+end}.
+
 %%-------------------------------------------------------------------
 %% Plugins
 %%-------------------------------------------------------------------

test/emqx_acl_cache_SUITE.erl

@@ -56,7 +56,7 @@ t_clean_acl_cache(_) ->
     emqtt:stop(Client).
 
 % optimize??
-t_reload_aclfile_and_cleanall(_Config) ->
+t_reload_aclfile_and_cleanall(Config) ->
 
     RasieMsg = fun() -> Self = self(), #{puback => fun(Msg) -> Self ! {puback, Msg} end,
                                          disconnected => fun(_) ->  ok end,
@@ -79,6 +79,27 @@ t_reload_aclfile_and_cleanall(_Config) ->
     %% Check acl cache list
     [ClientPid] = emqx_cm:lookup_channels(<<"emqx_c">>),
     ?assert(length(gen_server:call(ClientPid, list_acl_cache)) > 0),
+
+    %% Update acl file and reload mod_acl_internal
+    Path = filename:join([testdir(proplists:get_value(data_dir, Config)), "acl2.conf"]),
+    ok = file:write_file(Path, <<"{deny, all}.">>),
+    OldPath = emqx:get_env(acl_file),
+    % application:set_env(emqx, acl_file, Path),
+    emqx_mod_acl_internal:reload([{acl_file, Path}]),
+
+    ?assert(length(gen_server:call(ClientPid, list_acl_cache)) == 0),
+    {ok, PktId2} = emqtt:publish(Client, <<"t1">>, <<"{\"x\":1}">>, qos1),
+
+    receive
+        {puback, #{packet_id := PktId2, reason_code := Rc2}} ->
+            %% Not authorized
+            ?assertEqual(16#87, Rc2);
+        _ ->
+            ?assert(false)
+    end,
+    application:set_env(emqx, acl_file, OldPath),
+    file:delete(Path),
+    emqx_mod_acl_internal:reload([{acl_file, OldPath}]),
     emqtt:stop(Client).
 
 %% @private

test/mqtt_protocol_v5_SUITE.erl

@@ -183,7 +183,11 @@ t_batch_subscribe(_) ->
     {ok, Client} = emqtt:start_link([{proto_ver, v5}, {clientid, <<"batch_test">>}]),
     {ok, _} = emqtt:connect(Client),
     application:set_env(emqx, enable_acl_cache, false),
-    application:set_env(emqx, acl_nomatch, deny),
+    TempAcl = emqx_ct_helpers:deps_path(emqx, "test/emqx_access_SUITE_data/acl_temp.conf"),
+    file:write_file(TempAcl, "{deny, {client, \"batch_test\"}, subscribe,
+                    [\"t1\", \"t2\", \"t3\"]}.\n"),
+    timer:sleep(10),
+    emqx_mod_acl_internal:reload([{acl_file, TempAcl}]),
     {ok, _, [?RC_NOT_AUTHORIZED,
              ?RC_NOT_AUTHORIZED,
              ?RC_NOT_AUTHORIZED]} = emqtt:subscribe(Client, [{<<"t1">>, qos1},
@@ -194,7 +198,7 @@ t_batch_subscribe(_) ->
              ?RC_NO_SUBSCRIPTION_EXISTED]} = emqtt:unsubscribe(Client, [<<"t1">>,
                                                                         <<"t2">>,
                                                                         <<"t3">>]),
-    application:set_env(emqx, acl_nomatch, allow),
+    file:delete(TempAcl),
     emqtt:disconnect(Client).
 
 t_connect_will_retain(_) ->