diff --git a/apps/emqx/test/emqx_common_test_helpers.erl b/apps/emqx/test/emqx_common_test_helpers.erl index c8ef40925..d8e0690f2 100644 --- a/apps/emqx/test/emqx_common_test_helpers.erl +++ b/apps/emqx/test/emqx_common_test_helpers.erl @@ -237,6 +237,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), @@ -520,6 +522,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.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 26a22f73b..5f06f757c 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() ++ @@ -495,7 +506,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 } )} @@ -507,3 +518,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 5aa45d9ad..b59a5f819 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) -> @@ -133,6 +134,7 @@ 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}), @@ -199,6 +201,7 @@ log_test() -> %% erlfmt-ignore log_rotation_count_limit_test() -> + ensure_acl_conf(), Format = """ log.file { @@ -271,6 +274,7 @@ log_rotation_count_limit_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">>]), @@ -328,6 +332,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 = <>, @@ -402,6 +407,7 @@ authentication_headers(Conf) -> Headers. doc_gen_test() -> + ensure_acl_conf(), %% the json file too large to encode. { timeout, @@ -424,3 +430,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.