diff --git a/apps/emqx/test/emqx_common_test_helpers.erl b/apps/emqx/test/emqx_common_test_helpers.erl index 40e9ca5fc..15869fb36 100644 --- a/apps/emqx/test/emqx_common_test_helpers.erl +++ b/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 diff --git a/apps/emqx_authz/src/emqx_authz.app.src b/apps/emqx_authz/src/emqx_authz.app.src index dd0325694..4856008e6 100644 --- a/apps/emqx_authz/src/emqx_authz.app.src +++ b/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, [ diff --git a/apps/emqx_authz/src/emqx_authz.erl b/apps/emqx_authz/src/emqx_authz.erl index 5fffb61d2..e7f59cb71 100644 --- a/apps/emqx_authz/src/emqx_authz.erl +++ b/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 diff --git a/apps/emqx_authz/src/emqx_authz_schema.erl b/apps/emqx_authz/src/emqx_authz_schema.erl index a2a7c6b52..af4a04d96 100644 --- a/apps/emqx_authz/src/emqx_authz_schema.erl +++ b/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. diff --git a/apps/emqx_conf/test/emqx_conf_schema_tests.erl b/apps/emqx_conf/test/emqx_conf_schema_tests.erl index 79fe30293..d94975863 100644 --- a/apps/emqx_conf/test/emqx_conf_schema_tests.erl +++ b/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 = <>, + {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 = <>, + {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 = <>, + {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 = <>, @@ -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.