From d8be248a3dcb51c6ba100ccfeeace81c23424299 Mon Sep 17 00:00:00 2001 From: zhongwencool Date: Fri, 25 Aug 2023 17:20:02 +0800 Subject: [PATCH] fix: improve the suggest msg for update conf failed --- apps/emqx_conf/src/emqx_conf_cli.erl | 123 +++++++++++------- .../src/emqx_mgmt_api_configs.erl | 2 +- 2 files changed, 76 insertions(+), 49 deletions(-) diff --git a/apps/emqx_conf/src/emqx_conf_cli.erl b/apps/emqx_conf/src/emqx_conf_cli.erl index 8a4bb131f..e07963c09 100644 --- a/apps/emqx_conf/src/emqx_conf_cli.erl +++ b/apps/emqx_conf/src/emqx_conf_cli.erl @@ -50,17 +50,17 @@ conf(["show"]) -> conf(["show", Key]) -> print_hocon(get_config(list_to_binary(Key))); conf(["load", "--replace", Path]) -> - load_config(Path, replace); + load_config(Path, #{mode => replace}); conf(["load", "--merge", Path]) -> - load_config(Path, merge); + load_config(Path, #{mode => merge}); conf(["load", Path]) -> - load_config(Path, merge); + load_config(Path, #{mode => merge}); conf(["cluster_sync" | Args]) -> admins(Args); conf(["reload", "--merge"]) -> - reload_etc_conf_on_local_node(merge); + reload_etc_conf_on_local_node(#{mode => merge}); conf(["reload", "--replace"]) -> - reload_etc_conf_on_local_node(replace); + reload_etc_conf_on_local_node(#{mode => replace}); conf(["reload"]) -> conf(["reload", "--merge"]); conf(_) -> @@ -191,32 +191,32 @@ get_config(Key) -> end. -define(OPTIONS, #{rawconf_with_defaults => true, override_to => cluster}). -load_config(Path, ReplaceOrMerge) when is_list(Path) -> +load_config(Path, Opts) when is_list(Path) -> case hocon:files([Path]) of {ok, RawConf} when RawConf =:= #{} -> emqx_ctl:warning("load ~ts is empty~n", [Path]), {error, empty_hocon_file}; {ok, RawConf} -> - load_config_from_raw(RawConf, ReplaceOrMerge); + load_config_from_raw(RawConf, Opts); {error, Reason} -> emqx_ctl:warning("load ~ts failed~n~p~n", [Path, Reason]), {error, bad_hocon_file} end; -load_config(Bin, ReplaceOrMerge) when is_binary(Bin) -> +load_config(Bin, Opts) when is_binary(Bin) -> case hocon:binary(Bin) of {ok, RawConf} -> - load_config_from_raw(RawConf, ReplaceOrMerge); + load_config_from_raw(RawConf, Opts); {error, Reason} -> {error, Reason} end. -load_config_from_raw(RawConf, ReplaceOrMerge) -> +load_config_from_raw(RawConf, Opts) -> case check_config(RawConf) of ok -> Error = lists:filtermap( fun({K, V}) -> - case update_config_cluster(K, V, ReplaceOrMerge) of + case update_config_cluster(K, V, Opts) of ok -> false; {error, Msg} -> {true, Msg} end @@ -228,53 +228,71 @@ load_config_from_raw(RawConf, ReplaceOrMerge) -> ErrorBin -> {error, ErrorBin} end; {error, ?UPDATE_READONLY_KEYS_PROHIBITED = Reason} -> - emqx_ctl:warning("load config failed~n~ts~n", [Reason]), - emqx_ctl:warning( - "Maybe try `emqx_ctl conf reload` to reload etc/emqx.conf on local node~n" + warning(Opts, "load config failed~n~ts~n", [Reason]), + warning( + Opts, + "Maybe try `emqx_ctl conf reload` to reload etc/emqx.conf on local node~n", + [] ), {error, Reason}; {error, Errors} -> - emqx_ctl:warning("load schema check failed~n"), + warning(Opts, "load schema check failed~n", []), lists:foreach( fun({Key, Error}) -> - emqx_ctl:warning("~ts: ~p~n", [Key, Error]) + warning(Opts, "~ts: ~p~n", [Key, Error]) end, Errors ), {error, Errors} end. -update_config_cluster(?EMQX_AUTHORIZATION_CONFIG_ROOT_NAME_BINARY = Key, Conf, merge = Mode) -> - check_res(Key, emqx_authz:merge(Conf), Conf, Mode); -update_config_cluster(?EMQX_AUTHENTICATION_CONFIG_ROOT_NAME_BINARY = Key, Conf, merge = Mode) -> - check_res(Key, emqx_authn:merge_config(Conf), Conf, Mode); -update_config_cluster(Key, NewConf, merge = Mode) -> +update_config_cluster( + ?EMQX_AUTHORIZATION_CONFIG_ROOT_NAME_BINARY = Key, + Conf, + #{mode := merge} = Opts +) -> + check_res(Key, emqx_authz:merge(Conf), Conf, Opts); +update_config_cluster( + ?EMQX_AUTHENTICATION_CONFIG_ROOT_NAME_BINARY = Key, + Conf, + #{mode := merge} = Opts +) -> + check_res(Key, emqx_authn:merge_config(Conf), Conf, Opts); +update_config_cluster(Key, NewConf, #{mode := merge} = Opts) -> Merged = merge_conf(Key, NewConf), - check_res(Key, emqx_conf:update([Key], Merged, ?OPTIONS), NewConf, Mode); -update_config_cluster(Key, Value, replace = Mode) -> - check_res(Key, emqx_conf:update([Key], Value, ?OPTIONS), Value, Mode). + check_res(Key, emqx_conf:update([Key], Merged, ?OPTIONS), NewConf, Opts); +update_config_cluster(Key, Value, #{mode := replace} = Opts) -> + check_res(Key, emqx_conf:update([Key], Value, ?OPTIONS), Value, Opts). -define(LOCAL_OPTIONS, #{rawconf_with_defaults => true, persistent => false}). -update_config_local(?EMQX_AUTHORIZATION_CONFIG_ROOT_NAME_BINARY = Key, Conf, merge = Mode) -> - check_res(node(), Key, emqx_authz:merge_local(Conf, ?LOCAL_OPTIONS), Conf, Mode); -update_config_local(?EMQX_AUTHENTICATION_CONFIG_ROOT_NAME_BINARY = Key, Conf, merge = Mode) -> - check_res(node(), Key, emqx_authn:merge_config_local(Conf, ?LOCAL_OPTIONS), Conf, Mode); -update_config_local(Key, NewConf, merge = Mode) -> +update_config_local( + ?EMQX_AUTHORIZATION_CONFIG_ROOT_NAME_BINARY = Key, + Conf, + #{mode := merge} = Opts +) -> + check_res(node(), Key, emqx_authz:merge_local(Conf, ?LOCAL_OPTIONS), Conf, Opts); +update_config_local( + ?EMQX_AUTHENTICATION_CONFIG_ROOT_NAME_BINARY = Key, + Conf, + #{mode := merge} = Opts +) -> + check_res(node(), Key, emqx_authn:merge_config_local(Conf, ?LOCAL_OPTIONS), Conf, Opts); +update_config_local(Key, NewConf, #{mode := merge} = Opts) -> Merged = merge_conf(Key, NewConf), - check_res(node(), Key, emqx:update_config([Key], Merged, ?LOCAL_OPTIONS), NewConf, Mode); -update_config_local(Key, Value, replace = Mode) -> - check_res(node(), Key, emqx:update_config([Key], Value, ?LOCAL_OPTIONS), Value, Mode). + check_res(node(), Key, emqx:update_config([Key], Merged, ?LOCAL_OPTIONS), NewConf, Opts); +update_config_local(Key, Value, #{mode := replace} = Opts) -> + check_res(node(), Key, emqx:update_config([Key], Value, ?LOCAL_OPTIONS), Value, Opts). -check_res(Key, Res, Conf, Mode) -> check_res(cluster, Key, Res, Conf, Mode). -check_res(Node, Key, {ok, _}, _Conf, _Mode) -> - emqx_ctl:print("load ~ts on ~p ok~n", [Key, Node]), +check_res(Key, Res, Conf, Opts) -> check_res(cluster, Key, Res, Conf, Opts). +check_res(Node, Key, {ok, _}, _Conf, Opts) -> + print(Opts, "load ~ts on ~p ok~n", [Key, Node]), ok; -check_res(_Node, Key, {error, Reason}, Conf, Mode) -> +check_res(_Node, Key, {error, Reason}, Conf, Opts = #{mode := Mode}) -> Warning = "Can't ~ts the new configurations!~n" "Root key: ~ts~n" "Reason: ~p~n", - emqx_ctl:warning(Warning, [Mode, Key, Reason]), + warning(Opts, Warning, [Mode, Key, Reason]), ActiveMsg0 = "The effective configurations:~n" "```~n" @@ -285,12 +303,13 @@ check_res(_Node, Key, {error, Reason}, Conf, Mode) -> "```~n" "~ts```~n", FailedMsg = io_lib:format(FailedMsg0, [Mode, hocon_pp:do(#{Key => Conf}, #{})]), - SuggestMsg = suggest_msg(Mode), + SuggestMsg = suggest_msg(Reason, Mode), Msg = iolist_to_binary([ActiveMsg, FailedMsg, SuggestMsg]), - emqx_ctl:print("~ts", [Msg]), + print(Opts, "~ts", [Msg]), {error, iolist_to_binary([Warning, Msg])}. -suggest_msg(Mode) when Mode == merge orelse Mode == replace -> +%% The mix data failed validation, suggest the user to retry with another mode. +suggest_msg(#{kind := validation_error, reason := unknown_fields}, Mode) -> RetryMode = case Mode of merge -> "replace"; @@ -300,7 +319,9 @@ suggest_msg(Mode) when Mode == merge orelse Mode == replace -> "Tips: There may be some conflicts in the new configuration under `~ts` mode,~n" "Please retry with the `~ts` mode.~n", [Mode, RetryMode] - ). + ); +suggest_msg(_, _) -> + <<"">>. check_config(Conf) -> case check_keys_is_not_readonly(Conf) of @@ -327,19 +348,19 @@ check_config_schema(Conf) -> sorted_fold(Fold, Conf). %% @doc Reload etc/emqx.conf to runtime config except for the readonly config --spec reload_etc_conf_on_local_node(replace | merge) -> ok | {error, term()}. -reload_etc_conf_on_local_node(ReplaceOrMerge) -> +-spec reload_etc_conf_on_local_node(#{mode => replace | merge}) -> ok | {error, term()}. +reload_etc_conf_on_local_node(Opts) -> case load_etc_config_file() of {ok, RawConf} -> case filter_readonly_config(RawConf) of {ok, Reloaded} -> - reload_config(Reloaded, ReplaceOrMerge); + reload_config(Reloaded, Opts); {error, Error} -> - emqx_ctl:warning("check config failed~n~p~n", [Error]), + warning(Opts, "check config failed~n~p~n", [Error]), {error, Error} end; {error, Error} -> - emqx_ctl:warning("bad_hocon_file~n ~p~n", [Error]), + warning(Opts, "bad_hocon_file~n ~p~n", [Error]), {error, bad_hocon_file} end. @@ -385,9 +406,9 @@ filter_readonly_config(Raw) -> {error, Error} end. -reload_config(AllConf, ReplaceOrMerge) -> +reload_config(AllConf, Opts) -> Fold = fun({Key, Conf}, Acc) -> - case update_config_local(Key, Conf, ReplaceOrMerge) of + case update_config_local(Key, Conf, Opts) of ok -> Acc; Error -> @@ -441,3 +462,9 @@ check_config(SchemaMod, Key, Value) -> throw:Error -> {error, Error} end. + +warning(#{log := none}, _, _) -> ok; +warning(_, Format, Args) -> emqx_ctl:warning(Format, Args). + +print(#{log := none}, _, _) -> ok; +print(_, Format, Args) -> emqx_ctl:print(Format, Args). diff --git a/apps/emqx_management/src/emqx_mgmt_api_configs.erl b/apps/emqx_management/src/emqx_mgmt_api_configs.erl index 5edf8c564..5fe08a0e4 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_configs.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_configs.erl @@ -344,7 +344,7 @@ configs(get, #{query_string := QueryStr, headers := Headers}, _Req) -> {error, _} = Error -> {400, #{code => 'INVALID_ACCEPT', message => ?ERR_MSG(Error)}} end; configs(put, #{body := Conf, query_string := #{<<"mode">> := Mode}}, _Req) -> - case emqx_conf_cli:load_config(Conf, Mode) of + case emqx_conf_cli:load_config(Conf, #{mode => Mode, log => none}) of ok -> {200}; {error, Msg} -> {400, #{<<"content-type">> => <<"text/plain">>}, Msg} end.