From e645fa48747e6e8408bd4734db1a3ef845e64116 Mon Sep 17 00:00:00 2001 From: zhongwencool Date: Wed, 27 Dec 2023 21:13:47 +0800 Subject: [PATCH] fix: ft config update api return raw config --- apps/emqx_ft/src/emqx_ft.app.src | 2 +- apps/emqx_ft/src/emqx_ft_api.erl | 30 ++------ apps/emqx_ft/src/emqx_ft_conf.erl | 5 -- apps/emqx_ft/test/emqx_ft_api_SUITE.erl | 68 +++++++++++++++++-- .../src/emqx_mgmt_api_configs.erl | 38 ++++------- 5 files changed, 83 insertions(+), 60 deletions(-) diff --git a/apps/emqx_ft/src/emqx_ft.app.src b/apps/emqx_ft/src/emqx_ft.app.src index 8f78bae35..9edc2fd89 100644 --- a/apps/emqx_ft/src/emqx_ft.app.src +++ b/apps/emqx_ft/src/emqx_ft.app.src @@ -1,6 +1,6 @@ {application, emqx_ft, [ {description, "EMQX file transfer over MQTT"}, - {vsn, "0.1.11"}, + {vsn, "0.1.12"}, {registered, []}, {mod, {emqx_ft_app, []}}, {applications, [ diff --git a/apps/emqx_ft/src/emqx_ft_api.erl b/apps/emqx_ft/src/emqx_ft_api.erl index ace8c7b83..cc0ae88a6 100644 --- a/apps/emqx_ft/src/emqx_ft_api.erl +++ b/apps/emqx_ft/src/emqx_ft_api.erl @@ -125,7 +125,8 @@ schema("/file_transfer") -> responses => #{ 200 => ?SCHEMA_CONFIG, 400 => emqx_dashboard_swagger:error_codes( - ['INVALID_CONFIG'], error_desc('INVALID_CONFIG') + ['UPDATE_FAILED', 'INVALID_CONFIG'], + error_desc('INVALID_CONFIG') ) } } @@ -175,19 +176,9 @@ check_ft_enabled(Params, _Meta) -> {503, error_msg('SERVICE_UNAVAILABLE')} end. -'/file_transfer'(get, _Meta) -> - {200, format_config(emqx_ft_conf:get())}; -'/file_transfer'(put, #{body := ConfigIn}) -> - OldConf = emqx_ft_conf:get_raw(), - UpdateConf = emqx_utils:deobfuscate(ConfigIn, OldConf), - case emqx_ft_conf:update(UpdateConf) of - {ok, #{config := Config}} -> - {200, format_config(Config)}; - {error, Error = #{kind := validation_error}} -> - {400, error_msg('INVALID_CONFIG', format_validation_error(Error))}; - {error, Error} -> - {400, error_msg('INVALID_CONFIG', emqx_utils:format(Error))} - end. +%% Forward /file_transfer to /configs/file_transfer +'/file_transfer'(Method, Data) -> + emqx_mgmt_api_configs:request_config([<<"file_transfer">>], Method, Data). format_page(#{items := Files, cursor := Cursor}) -> #{ @@ -199,17 +190,6 @@ format_page(#{items := Files}) -> <<"files">> => lists:map(fun format_file_info/1, Files) }. -format_config(Config) -> - Schema = emqx_hocon:make_schema(emqx_ft_schema:fields(file_transfer)), - hocon_tconf:make_serializable( - Schema, - emqx_utils_maps:binary_key_map(Config), - #{obfuscate_sensitive_values => true} - ). - -format_validation_error(Error) -> - emqx_logger_jsonfmt:best_effort_json(Error). - error_msg(Code) -> #{code => Code, message => error_desc(Code)}. diff --git a/apps/emqx_ft/src/emqx_ft_conf.erl b/apps/emqx_ft/src/emqx_ft_conf.erl index a507f840e..b67497aa8 100644 --- a/apps/emqx_ft/src/emqx_ft_conf.erl +++ b/apps/emqx_ft/src/emqx_ft_conf.erl @@ -36,7 +36,6 @@ -export([ load/0, unload/0, - get/0, update/1, get_raw/0 ]). @@ -109,10 +108,6 @@ unload() -> ok = emqx_conf:remove_handler([file_transfer]), maybe_stop(). --spec get() -> emqx_config:config(). -get() -> - emqx_config:get([file_transfer]). - get_raw() -> emqx:get_raw_config([file_transfer], #{}). diff --git a/apps/emqx_ft/test/emqx_ft_api_SUITE.erl b/apps/emqx_ft/test/emqx_ft_api_SUITE.erl index 66f770a22..250d28f56 100644 --- a/apps/emqx_ft/test/emqx_ft_api_SUITE.erl +++ b/apps/emqx_ft/test/emqx_ft_api_SUITE.erl @@ -277,17 +277,37 @@ t_ft_disabled(Config) -> ) ). -t_configure_1(Config) -> +t_configure_file_transfer(Config) -> Uri = uri(["file_transfer"]), test_configure(Uri, Config). -t_configure_2(Config) -> +t_configure_config_file_transfer(Config) -> Uri = uri(["configs/file_transfer"]), test_configure(Uri, Config). test_configure(Uri, Config) -> ?assertMatch( - {ok, 200, #{<<"enable">> := true, <<"storage">> := #{}}}, + {ok, 200, #{ + <<"enable">> := true, + <<"storage">> := + #{ + <<"local">> := + #{ + <<"enable">> := true, + <<"segments">> := + #{ + <<"gc">> := + #{ + %% Match keep the raw conf + %% 1h is not change to 3600000 + <<"interval">> := <<"1h">>, + <<"maximum_segments_ttl">> := <<"24h">>, + <<"minimum_segments_ttl">> := <<"5m">> + } + } + } + } + }}, request_json(get, Uri, Config) ), ?assertMatch( @@ -298,14 +318,43 @@ test_configure(Uri, Config) -> {ok, 200, #{<<"enable">> := false}}, request_json(get, Uri, Config) ), + Storage0 = emqx_ft_test_helpers:local_storage(Config), + Storage = emqx_utils_maps:deep_put( + [ + <<"local">>, + <<"segments">>, + <<"gc">>, + <<"maximum_segments_ttl">> + ], + Storage0, + <<"10m">> + ), ?assertMatch( - {ok, 200, #{}}, + {ok, 200, #{ + <<"storage">> := + #{ + <<"local">> := + #{ + <<"segments">> := + #{ + <<"gc">> := + #{ + <<"interval">> := <<"1h">>, + %% Match keep the raw conf + %% 10m is not change to 600,000 + <<"maximum_segments_ttl">> := <<"10m">>, + <<"minimum_segments_ttl">> := <<"5m">> + } + } + } + } + }}, request_json( put, Uri, #{ <<"enable">> => true, - <<"storage">> => emqx_ft_test_helpers:local_storage(Config) + <<"storage">> => Storage }, Config ) @@ -537,7 +586,14 @@ reset_ft_config(Config, Enable) -> <<"enable">> => Enable, <<"storage">> => #{ <<"local">> => #{ - <<"enable">> => true + <<"enable">> => true, + <<"segments">> => #{ + <<"gc">> => #{ + <<"interval">> => <<"1h">>, + <<"maximum_segments_ttl">> => "24h", + <<"minimum_segments_ttl">> => "5m" + } + } } } }, diff --git a/apps/emqx_management/src/emqx_mgmt_api_configs.erl b/apps/emqx_management/src/emqx_mgmt_api_configs.erl index ca5a95966..ca1a9fc0b 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_configs.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_configs.erl @@ -29,8 +29,10 @@ configs/3, get_full_config/0, global_zone_configs/3, - limiter/3 + limiter/3, + get_raw_config/1 ]). +-export([request_config/3]). -define(PREFIX, "/configs/"). -define(PREFIX_RESET, "/configs_reset/"). @@ -289,35 +291,22 @@ fields(Field) -> %% HTTP API Callbacks config(Method, Data, Req) -> Path = conf_path(Req), - do_config(Path, Method, Data). + request_config(Path, Method, Data). -do_config([<<"file_transfer">> | Path], Method, Data) -> - [] =/= Path andalso throw("file_transfer does no support deep config get/put"), - forward_file_transfer(Method, Data); -do_config([ConfigRoot | Path], get, _Params) -> +request_config([ConfigRoot | Path], get, _Params) -> [] =/= Path andalso throw("deep config get is not supported"), {200, get_raw_config(ConfigRoot)}; -do_config(Path, put, #{body := NewConf}) -> - case emqx_conf:update(Path, NewConf, ?OPTS) of +request_config(Path, put, #{body := NewConf}) -> + OldConf = emqx:get_raw_config(Path, #{}), + UpdateConf = emqx_utils:deobfuscate(NewConf, OldConf), + case emqx_conf:update(Path, UpdateConf, ?OPTS) of {ok, #{raw_config := RawConf}} -> - {200, RawConf}; + [ConfigRoot] = Path, + {200, obfuscate_raw_config(ConfigRoot, RawConf)}; {error, Reason} -> {400, #{code => 'UPDATE_FAILED', message => ?ERR_MSG(Reason)}} end. -%% @private file_transfer config update reside in this module -%% because it's needed to generate hotconf schema for dashboard UI rendering. -%% As a result of adding "file_transfer" root key to the root keys list -%% there is also a configs/file_transfer http path handler to be implemented. -%% Here we simply forward the call to the file_transfer API module. --if(?EMQX_RELEASE_EDITION == ee). -forward_file_transfer(Method, Data) -> - emqx_ft_api:'/file_transfer'(Method, Data). --else. -forward_file_transfer(_Method, _Data) -> - {400, #{code => 'BAD_REQUEST', message => <<"not supported">>}}. --endif. - global_zone_configs(get, _Params, _Req) -> {200, get_zones()}; global_zone_configs(put, #{body := Body}, _Req) -> @@ -474,9 +463,12 @@ get_full_config() -> ). get_raw_config(Path) -> + obfuscate_raw_config(Path, emqx:get_raw_config([Path])). + +obfuscate_raw_config(Path, Raw) -> #{Path := Conf} = emqx_config:fill_defaults( - #{Path => emqx:get_raw_config([Path])}, + #{Path => Raw}, #{obfuscate_sensitive_values => true} ), Conf.