From 218fc4a83907fb92ca66d42208314ffa99add4b0 Mon Sep 17 00:00:00 2001 From: Zhongwen Deng Date: Mon, 22 May 2023 11:03:23 +0800 Subject: [PATCH] refactor: add emqx_authz_file validate function --- apps/emqx_authz/src/emqx_authz.erl | 5 +- apps/emqx_authz/src/emqx_authz_file.erl | 7 +- apps/emqx_authz/src/emqx_authz_schema.erl | 12 +- apps/emqx_authz/test/emqx_authz_SUITE.erl | 7 +- .../emqx_conf/test/emqx_conf_schema_tests.erl | 194 ------------------ 5 files changed, 17 insertions(+), 208 deletions(-) diff --git a/apps/emqx_authz/src/emqx_authz.erl b/apps/emqx_authz/src/emqx_authz.erl index e7f59cb71..c7db65992 100644 --- a/apps/emqx_authz/src/emqx_authz.erl +++ b/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 diff --git a/apps/emqx_authz/src/emqx_authz_file.erl b/apps/emqx_authz/src/emqx_authz_file.erl index dfd28f0c0..317395a45 100644 --- a/apps/emqx_authz/src/emqx_authz_file.erl +++ b/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) -> diff --git a/apps/emqx_authz/src/emqx_authz_schema.erl b/apps/emqx_authz/src/emqx_authz_schema.erl index af4a04d96..3b318c39f 100644 --- a/apps/emqx_authz/src/emqx_authz_schema.erl +++ b/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. diff --git a/apps/emqx_authz/test/emqx_authz_SUITE.erl b/apps/emqx_authz/test/emqx_authz_SUITE.erl index 2c57d807a..9c1b7fd51 100644 --- a/apps/emqx_authz/test/emqx_authz_SUITE.erl +++ b/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}, diff --git a/apps/emqx_conf/test/emqx_conf_schema_tests.erl b/apps/emqx_conf/test/emqx_conf_schema_tests.erl index d94975863..06784e32d 100644 --- a/apps/emqx_conf/test/emqx_conf_schema_tests.erl +++ b/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 = <>, - {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, """