feat: add authz file rule validator

apps/emqx/test/emqx_common_test_helpers.erl

@@ -238,6 +238,8 @@ render_and_load_app_config(App, Opts) ->
     end.
 
 do_render_app_config(App, Schema, ConfigFile, Opts) ->
+    %% copy acl_conf must run before read_schema_configs
+    copy_acl_conf(),
     Vars = mustache_vars(App, Opts),
     RenderedConfigFile = render_config_file(ConfigFile, Vars),
     read_schema_configs(Schema, RenderedConfigFile),
@@ -497,6 +499,16 @@ copy_certs(emqx_conf, Dest0) ->
 copy_certs(_, _) ->
     ok.
 
+copy_acl_conf() ->
+    Dest = filename:join([code:lib_dir(emqx), "etc/acl.conf"]),
+    case code:lib_dir(emqx_authz) of
+        {error, bad_name} ->
+            (not filelib:is_regular(Dest)) andalso file:write_file(Dest, <<"">>);
+        _ ->
+            {ok, _} = file:copy(deps_path(emqx_authz, "etc/acl.conf"), Dest)
+    end,
+    ok.
+
 load_config(SchemaModule, Config) ->
     ConfigBin =
         case is_map(Config) of

apps/emqx_authz/src/emqx_authz.app.src

@@ -1,7 +1,7 @@
 %% -*- mode: erlang -*-
 {application, emqx_authz, [
     {description, "An OTP application"},
-    {vsn, "0.1.19"},
+    {vsn, "0.1.20"},
     {registered, []},
     {mod, {emqx_authz_app, []}},
     {applications, [

apps/emqx_authz/src/emqx_authz.erl

@@ -482,6 +482,8 @@ maybe_write_files(NewSource) ->
 
 write_acl_file(#{<<"rules">> := Rules} = Source0) ->
     AclPath = ?MODULE:acl_conf_file(),
+    %% Always check if the rules are valid before writing to the file
+    %% If the rules are invalid, the old file will be kept
     ok = check_acl_file_rules(AclPath, Rules),
     ok = write_file(AclPath, Rules),
     Source1 = maps:remove(<<"rules">>, Source0),
@@ -538,8 +540,7 @@ check_acl_file_rules(Path, Rules) ->
     TmpPath = Path ++ ".tmp",
     try
         ok = write_file(Path, Rules),
-        #{annotations := #{rules := _}} = emqx_authz_file:create(#{path => Path}),
-        ok
+        emqx_authz_schema:validate_file_rules(Path)
     catch
         throw:Reason -> throw(Reason)
     after

apps/emqx_authz/src/emqx_authz_schema.erl

@@ -42,7 +42,8 @@
 
 -export([
     headers_no_content_type/1,
-    headers/1
+    headers/1,
+    validate_file_rules/1
 ]).
 
 %%--------------------------------------------------------------------
@@ -78,7 +79,17 @@ fields("authorization") ->
     authz_fields();
 fields(file) ->
     authz_common_fields(file) ++
-        [{path, ?HOCON(string(), #{required => true, desc => ?DESC(path)})}];
+        [
+            {path,
+                ?HOCON(
+                    string(),
+                    #{
+                        required => true,
+                        validator => fun ?MODULE:validate_file_rules/1,
+                        desc => ?DESC(path)
+                    }
+                )}
+        ];
 fields(http_get) ->
     authz_common_fields(http) ++
         http_common_fields() ++
@@ -496,7 +507,7 @@ authz_fields() ->
                     %% doc_lift is force a root level reference instead of nesting sub-structs
                     extra => #{doc_lift => true},
                     %% it is recommended to configure authz sources from dashboard
-                    %% hance the importance level for config is low
+                    %% hence the importance level for config is low
                     importance => ?IMPORTANCE_LOW
                 }
             )}
@@ -508,3 +519,10 @@ 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_conf/test/emqx_conf_schema_tests.erl

@@ -23,6 +23,7 @@
     """).
 
 array_nodes_test() ->
+    ensure_acl_conf(),
     ExpectNodes = ['emqx1@127.0.0.1', 'emqx2@127.0.0.1'],
     lists:foreach(
         fun(Nodes) ->
@@ -47,6 +48,200 @@ 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,
     """
@@ -79,6 +274,7 @@ array_nodes_test() ->
 ).
 
 authn_validations_test() ->
+    ensure_acl_conf(),
     BaseConf = to_bin(?BASE_CONF, ["emqx1@127.0.0.1", "emqx1@127.0.0.1"]),
 
     OKHttps = to_bin(?BASE_AUTHN_ARRAY, [post, true, <<"https://127.0.0.1:8080">>]),
@@ -128,6 +324,7 @@ authn_validations_test() ->
 ).
 
 listeners_test() ->
+    ensure_acl_conf(),
     BaseConf = to_bin(?BASE_CONF, ["emqx1@127.0.0.1", "emqx1@127.0.0.1"]),
 
     Conf = <<BaseConf/binary, ?LISTENERS>>,
@@ -198,6 +395,7 @@ listeners_test() ->
     ok.
 
 doc_gen_test() ->
+    ensure_acl_conf(),
     %% the json file too large to encode.
     {
         timeout,
@@ -220,3 +418,11 @@ doc_gen_test() ->
 
 to_bin(Format, Args) ->
     iolist_to_binary(io_lib:format(Format, Args)).
+
+ensure_acl_conf() ->
+    File = emqx_schema:naive_env_interpolation(<<"${EMQX_ETC_DIR}/acl.conf">>),
+    ok = filelib:ensure_dir(filename:dirname(File)),
+    case filelib:is_regular(File) of
+        true -> ok;
+        false -> file:write_file(File, <<"">>)
+    end.