Fix reload ACL failed (#2344)

The original `reload_acl` function only parse the ACL file, not compile and rehook to 'client.check_acl'

src/emqx_access_control.erl

@@ -57,12 +57,14 @@ do_check_acl(#{zone := Zone} = Credentials, PubSub, Topic) ->
         _ -> deny
     end.
 
--spec(reload_acl() -> list(ok | {error, term()})).
+-spec(reload_acl() -> ok | {error, term()}).
 reload_acl() ->
+    emqx_acl_cache:is_enabled() andalso
+        emqx_acl_cache:empty_acl_cache(),
     emqx_mod_acl_internal:reload_acl().
 
 init_auth_result(Credentials) ->
     case emqx_zone:get_env(maps:get(zone, Credentials, undefined), allow_anonymous, false) of
         true -> success;
         false -> not_authorized
-    end.
+    end.

src/emqx_acl_cache.erl

@@ -16,19 +16,19 @@
 
 -include("emqx.hrl").
 
--export([  get_acl_cache/2
-         , put_acl_cache/3
-         , cleanup_acl_cache/0
-         , empty_acl_cache/0
-         , dump_acl_cache/0
-         , get_cache_size/0
-         , get_cache_max_size/0
-         , get_newest_key/0
-         , get_oldest_key/0
-         , cache_k/2
-         , cache_v/1
-         , is_enabled/0
-         ]).
+-export([ get_acl_cache/2
+        , put_acl_cache/3
+        , cleanup_acl_cache/0
+        , empty_acl_cache/0
+        , dump_acl_cache/0
+        , get_cache_size/0
+        , get_cache_max_size/0
+        , get_newest_key/0
+        , get_oldest_key/0
+        , cache_k/2
+        , cache_v/1
+        , is_enabled/0
+        ]).
 
 -type(acl_result() :: allow | deny).
 

src/emqx_mod_acl_internal.erl

@@ -25,11 +25,9 @@
 
 -export([check_acl/5, reload_acl/0]).
 
--define(ACL_RULE_TAB, emqx_acl_rule).
+-define(MFA(M, F, A), {M, F, A}).
 
--define(FUNC(M, F, A), {M, F, A}).
-
--type(acl_rules() :: #{publish => [emqx_access_rule:rule()],
+-type(acl_rules() :: #{publish   => [emqx_access_rule:rule()],
                        subscribe => [emqx_access_rule:rule()]}).
 
 %%------------------------------------------------------------------------------
@@ -37,28 +35,64 @@
 %%------------------------------------------------------------------------------
 
 load(_Env) ->
-    Rules = load_rules_from_file(acl_file()),
-    emqx_hooks:add('client.check_acl', ?FUNC(?MODULE, check_acl, [Rules]),  -1).
+    Rules = rules_from_file(acl_file()),
+    emqx_hooks:add('client.check_acl', ?MFA(?MODULE, check_acl, [Rules]),  -1).
 
 unload(_Env) ->
-    Rules = load_rules_from_file(acl_file()),
-    emqx_hooks:del('client.check_acl', ?FUNC(?MODULE, check_acl, [Rules])).
+    Rules = rules_from_file(acl_file()),
+    emqx_hooks:del('client.check_acl', ?MFA(?MODULE, check_acl, [Rules])).
 
 %% @doc Read all rules
 -spec(all_rules() -> list(emqx_access_rule:rule())).
 all_rules() ->
-    load_rules_from_file(acl_file()).
+    rules_from_file(acl_file()).
 
 %%------------------------------------------------------------------------------
 %% ACL callbacks
 %%------------------------------------------------------------------------------
 
-load_rules_from_file(AclFile) ->
+%% @doc Check ACL
+-spec(check_acl(emqx_types:credentials(), emqx_types:pubsub(), emqx_topic:topic(),
+                emqx_access_rule:acl_result(), acl_rules())
+      -> {ok, allow} | {ok, deny} | ok).
+check_acl(Credentials, PubSub, Topic, _AclResult, Rules) ->
+    case match(Credentials, Topic, lookup(PubSub, Rules)) of
+        {matched, allow} -> {ok, allow};
+        {matched, deny}  -> {ok, deny};
+        nomatch          -> ok
+    end.
+
+-spec(reload_acl() -> ok | {error, term()}).
+reload_acl() ->
+    unload([]), load([]).
+
+%%------------------------------------------------------------------------------
+%% Internal Functions
+%%------------------------------------------------------------------------------
+
+acl_file() ->
+    emqx_config:get_env(acl_file).
+
+lookup(PubSub, Rules) ->
+    maps:get(PubSub, Rules, []).
+
+match(_Credentials, _Topic, []) ->
+    nomatch;
+match(Credentials, Topic, [Rule|Rules]) ->
+    case emqx_access_rule:match(Credentials, Topic, Rule) of
+        nomatch ->
+            match(Credentials, Topic, Rules);
+        {matched, AllowDeny} ->
+            {matched, AllowDeny}
+    end.
+
+-spec(rules_from_file(file:filename()) -> map()).
+rules_from_file(AclFile) ->
     case file:consult(AclFile) of
         {ok, Terms} ->
             Rules = [emqx_access_rule:compile(Term) || Term <- Terms],
-            #{publish => lists:filter(fun(Rule) -> filter(publish, Rule) end, Rules),
-              subscribe => lists:filter(fun(Rule) -> filter(subscribe, Rule) end, Rules)};
+            #{publish   => [Rule || Rule <- Rules, filter(publish, Rule)],
+              subscribe => [Rule || Rule <- Rules, filter(subscribe, Rule)]};
         {error, Reason} ->
             ?LOG(error, "[ACL_INTERNAL] Failed to read ~s: ~p", [AclFile, Reason]),
             #{}
@@ -77,43 +111,3 @@ filter(subscribe, {_AllowDeny, _Who, subscribe, _Topics}) ->
 filter(_PubSub, {_AllowDeny, _Who, _, _Topics}) ->
     false.
 
-%% @doc Check ACL
--spec(check_acl(emqx_types:credentials(), emqx_types:pubsub(), emqx_topic:topic(),
-                emqx_access_rule:acl_result(), acl_rules())
-      -> {ok, allow} | {ok, deny} | ok).
-check_acl(Credentials, PubSub, Topic, _AclResult, Rules) ->
-    case match(Credentials, Topic, lookup(PubSub, Rules)) of
-        {matched, allow} -> {ok, allow};
-        {matched, deny}  -> {ok, deny};
-        nomatch          -> ok
-    end.
-
-lookup(PubSub, Rules) ->
-    maps:get(PubSub, Rules, []).
-
-match(_Credentials, _Topic, []) ->
-    nomatch;
-match(Credentials, Topic, [Rule|Rules]) ->
-    case emqx_access_rule:match(Credentials, Topic, Rule) of
-        nomatch ->
-            match(Credentials, Topic, Rules);
-        {matched, AllowDeny} ->
-            {matched, AllowDeny}
-    end.
-
--spec(reload_acl() -> ok | {error, term()}).
-reload_acl() ->
-    try load_rules_from_file(acl_file()) of
-        _ ->
-            emqx_logger:info("Reload acl_file ~s successfully", [acl_file()]),
-            ok;
-        {error, Error} ->
-            {error, Error}
-    catch
-        error:Reason:StackTrace ->
-            ?LOG(error, "Reload acl failed. StackTrace: ~p", [StackTrace]),
-            {error, Reason}
-    end.
-
-acl_file() ->
-    emqx_config:get_env(acl_file).