refactor: add emqx_authz_file validate function

apps/emqx_authz/src/emqx_authz.erl

@@ -539,8 +539,9 @@ update_authz_chain(Actions) ->
 check_acl_file_rules(Path, Rules) ->
     TmpPath = Path ++ ".tmp",
     try
-        ok = write_file(Path, Rules),
-        emqx_authz_schema:validate_file_rules(Path)
+        ok = write_file(TmpPath, Rules),
+        {ok, _} = emqx_authz_file:validate(TmpPath),
+        ok
     catch
         throw:Reason -> throw(Reason)
     after

apps/emqx_authz/src/emqx_authz_file.erl

@@ -33,13 +33,14 @@
     update/1,
     destroy/1,
     authorize/4,
+    validate/1,
     read_file/1
 ]).
 
 description() ->
     "AuthZ with static rules".
 
-create(#{path := Path0} = Source) ->
+validate(Path0) ->
     Path = filename(Path0),
     Rules =
         case file:consult(Path) of
@@ -56,6 +57,10 @@ create(#{path := Path0} = Source) ->
                 ?SLOG(alert, #{msg => bad_acl_file_content, path => Path, reason => Reason}),
                 throw({bad_acl_file_content, Reason})
         end,
+    {ok, Rules}.
+
+create(#{path := Path} = Source) ->
+    {ok, Rules} = validate(Path),
     Source#{annotations => #{rules => Rules}}.
 
 update(#{path := _Path} = Source) ->

apps/emqx_authz/src/emqx_authz_schema.erl

@@ -42,8 +42,7 @@
 
 -export([
     headers_no_content_type/1,
-    headers/1,
-    validate_file_rules/1
+    headers/1
 ]).
 
 %%--------------------------------------------------------------------
@@ -85,7 +84,7 @@ fields(file) ->
                     string(),
                     #{
                         required => true,
-                        validator => fun ?MODULE:validate_file_rules/1,
+                        validator => fun(Path) -> element(1, emqx_authz_file:validate(Path)) end,
                         desc => ?DESC(path)
                     }
                 )}
@@ -519,10 +518,3 @@ default_authz() ->
         <<"enable">> => true,
         <<"path">> => <<"${EMQX_ETC_DIR}/acl.conf">>
     }.
-
-validate_file_rules(Path) ->
-    %% Don't need assert the create result here, all error is thrown
-    %% some test mock the create function
-    %% #{annotations := #{rules := _}}
-    _ = emqx_authz_file:create(#{path => Path}),
-    ok.

apps/emqx_authz/test/emqx_authz_SUITE.erl

@@ -205,9 +205,14 @@ t_bad_file_source(_) ->
     BadActionErr = {invalid_authorization_action, pubsub},
     lists:foreach(
         fun({Source, Error}) ->
+            File = emqx_authz:acl_conf_file(),
+            {ok, Bin1} = file:read_file(File),
             ?assertEqual(?UPDATE_ERROR(Error), emqx_authz:update(?CMD_REPLACE, [Source])),
             ?assertEqual(?UPDATE_ERROR(Error), emqx_authz:update(?CMD_PREPEND, Source)),
-            ?assertEqual(?UPDATE_ERROR(Error), emqx_authz:update(?CMD_APPEND, Source))
+            ?assertEqual(?UPDATE_ERROR(Error), emqx_authz:update(?CMD_APPEND, Source)),
+            %% Check file content not changed if update failed
+            {ok, Bin2} = file:read_file(File),
+            ?assertEqual(Bin1, Bin2)
         end,
         [
             {BadContent, BadContentErr},

apps/emqx_conf/test/emqx_conf_schema_tests.erl

@@ -48,200 +48,6 @@ array_nodes_test() ->
     ),
     ok.
 
-%% erlfmt-ignore
--define(OUTDATED_LOG_CONF,
-    """
-log.console_handler {
-  burst_limit {
-    enable = true
-    max_count = 10000
-    window_time = 1000
-  }
-  chars_limit = unlimited
-  drop_mode_qlen = 3000
-  enable = true
-  flush_qlen = 8000
-  formatter = text
-  level = warning
-  max_depth = 100
-  overload_kill {
-    enable = true
-    mem_size = 31457280
-    qlen = 20000
-    restart_after = 5000
-  }
-  single_line = true
-  supervisor_reports = error
-  sync_mode_qlen = 100
-  time_offset = \"+02:00\"
-}
-log.file_handlers {
-  default {
-    burst_limit {
-      enable = true
-      max_count = 10000
-      window_time = 1000
-    }
-    chars_limit = unlimited
-    drop_mode_qlen = 3000
-    enable = true
-    file = \"log/my-emqx.log\"
-    flush_qlen = 8000
-    formatter = text
-    level = debug
-    max_depth = 100
-    max_size = \"1024MB\"
-    overload_kill {
-      enable = true
-      mem_size = 31457280
-      qlen = 20000
-      restart_after = 5000
-    }
-    rotation {count = 20, enable = true}
-    single_line = true
-    supervisor_reports = error
-    sync_mode_qlen = 100
-    time_offset = \"+01:00\"
-  }
-}
-    """
-).
--define(FORMATTER(TimeOffset),
-    {emqx_logger_textfmt, #{
-        chars_limit => unlimited,
-        depth => 100,
-        single_line => true,
-        template => [time, " [", level, "] ", msg, "\n"],
-        time_offset => TimeOffset
-    }}
-).
-
--define(FILTERS, [{drop_progress_reports, {fun logger_filters:progress/2, stop}}]).
--define(LOG_CONFIG, #{
-    burst_limit_enable => true,
-    burst_limit_max_count => 10000,
-    burst_limit_window_time => 1000,
-    drop_mode_qlen => 3000,
-    flush_qlen => 8000,
-    overload_kill_enable => true,
-    overload_kill_mem_size => 31457280,
-    overload_kill_qlen => 20000,
-    overload_kill_restart_after => 5000,
-    sync_mode_qlen => 100
-}).
-
-outdated_log_test() ->
-    validate_log(?OUTDATED_LOG_CONF).
-
-validate_log(Conf) ->
-    ensure_acl_conf(),
-    BaseConf = to_bin(?BASE_CONF, ["emqx1@127.0.0.1", "emqx1@127.0.0.1"]),
-    Conf0 = <<BaseConf/binary, (list_to_binary(Conf))/binary>>,
-    {ok, ConfMap0} = hocon:binary(Conf0, #{format => richmap}),
-    ConfList = hocon_tconf:generate(emqx_conf_schema, ConfMap0),
-    Kernel = proplists:get_value(kernel, ConfList),
-
-    ?assertEqual(silent, proplists:get_value(error_logger, Kernel)),
-    ?assertEqual(debug, proplists:get_value(logger_level, Kernel)),
-    Loggers = proplists:get_value(logger, Kernel),
-    FileHandler = lists:keyfind(logger_disk_log_h, 3, Loggers),
-    ?assertEqual(
-        {handler, default, logger_disk_log_h, #{
-            config => ?LOG_CONFIG#{
-                type => wrap,
-                file => "log/my-emqx.log",
-                max_no_bytes => 1073741824,
-                max_no_files => 20
-            },
-            filesync_repeat_interval => no_repeat,
-            filters => ?FILTERS,
-            formatter => ?FORMATTER("+01:00"),
-            level => debug
-        }},
-        FileHandler
-    ),
-    ConsoleHandler = lists:keyfind(logger_std_h, 3, Loggers),
-    ?assertEqual(
-        {handler, console, logger_std_h, #{
-            config => ?LOG_CONFIG#{type => standard_io},
-            filters => ?FILTERS,
-            formatter => ?FORMATTER("+02:00"),
-            level => warning
-        }},
-        ConsoleHandler
-    ).
-
-%% erlfmt-ignore
--define(KERNEL_LOG_CONF,
-    """
-    log.console {
-       enable = true
-       formatter = text
-       level = warning
-       time_offset = \"+02:00\"
-    }
-    log.file {
-        enable = false
-        file = \"log/xx-emqx.log\"
-        formatter = text
-        level = debug
-        rotation_count = 20
-        rotation_size = \"1024MB\"
-        time_offset = \"+01:00\"
-      }
-    log.file_handlers.default {
-        enable = true
-        file = \"log/my-emqx.log\"
-      }
-    """
-).
-
-log_test() ->
-    validate_log(?KERNEL_LOG_CONF).
-
-%% erlfmt-ignore
-log_rotation_count_limit_test() ->
-    ensure_acl_conf(),
-    Format =
-    """
-    log.file {
-    enable = true
-    to = \"log/emqx.log\"
-    formatter = text
-    level = debug
-    rotation = {count = ~w}
-    rotation_size = \"1024MB\"
-    }
-    """,
-    BaseConf = to_bin(?BASE_CONF, ["emqx1@127.0.0.1", "emqx1@127.0.0.1"]),
-    lists:foreach(fun({Conf, Count}) ->
-        Conf0 = <<BaseConf/binary, Conf/binary>>,
-        {ok, ConfMap0} = hocon:binary(Conf0, #{format => richmap}),
-        ConfList = hocon_tconf:generate(emqx_conf_schema, ConfMap0),
-        Kernel = proplists:get_value(kernel, ConfList),
-        Loggers = proplists:get_value(logger, Kernel),
-        ?assertMatch(
-            {handler, default, logger_disk_log_h, #{
-                config := #{max_no_files := Count}
-            }},
-            lists:keyfind(logger_disk_log_h, 3, Loggers)
-        )
-                  end,
-        [{to_bin(Format, [1]), 1}, {to_bin(Format, [128]), 128}]),
-    lists:foreach(fun({Conf, Count}) ->
-        Conf0 = <<BaseConf/binary, Conf/binary>>,
-        {ok, ConfMap0} = hocon:binary(Conf0, #{format => richmap}),
-        ?assertThrow({emqx_conf_schema,
-            [#{kind := validation_error,
-            mismatches := #{"handler_name" :=
-            #{kind := validation_error,
-                path := "log.file.default.rotation_count",
-                reason := #{expected_type := "1..128"},
-                value := Count}
-            }}]},
-            hocon_tconf:generate(emqx_conf_schema, ConfMap0))
-                  end, [{to_bin(Format, [0]), 0}, {to_bin(Format, [129]), 129}]).
-
 %% erlfmt-ignore
 -define(BASE_AUTHN_ARRAY,
     """