From 8b10b78bce917e46d420944c813de70695a02cb0 Mon Sep 17 00:00:00 2001 From: Stefan Strigler Date: Thu, 3 Nov 2022 17:04:11 +0100 Subject: [PATCH 1/5] fix(emqx_mgmt_api_configs): use 'node' query paramter if given also fix result in error case - be compliant to schema and also return correct http status code --- apps/emqx_management/src/emqx_mgmt_api_configs.erl | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/apps/emqx_management/src/emqx_mgmt_api_configs.erl b/apps/emqx_management/src/emqx_mgmt_api_configs.erl index e38b6b729..db582c612 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_configs.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_configs.erl @@ -103,7 +103,9 @@ schema("/configs") -> )} ], responses => #{ - 200 => lists:map(fun({_, Schema}) -> Schema end, config_list()) + 200 => lists:map(fun({_, Schema}) -> Schema end, config_list()), + 404 => emqx_dashboard_swagger:error_codes(['NOT_FOUND']), + 500 => emqx_dashboard_swagger:error_codes(['BAD_NODE']) } } }; @@ -311,14 +313,15 @@ config_reset(post, _Params, Req) -> end. configs(get, Params, _Req) -> - Node = maps:get(node, Params, node()), + QS = maps:get(query_string, Params, #{}), + Node = maps:get(<<"node">>, QS, node()), case lists:member(Node, mria_mnesia:running_nodes()) andalso emqx_management_proto_v2:get_full_config(Node) of false -> Message = list_to_binary(io_lib:format("Bad node ~p, reason not found", [Node])), - {500, #{code => 'BAD_NODE', message => Message}}; + {404, #{code => 'NOT_FOUND', message => Message}}; {badrpc, R} -> Message = list_to_binary(io_lib:format("Bad node ~p, reason ~p", [Node, R])), {500, #{code => 'BAD_NODE', message => Message}}; From 104b1a63d99c0d49e76ca2e16cf46865c09e97b1 Mon Sep 17 00:00:00 2001 From: Stefan Strigler Date: Fri, 4 Nov 2022 17:13:54 +0100 Subject: [PATCH 2/5] test: pass down Opts in request_api/X --- .../test/emqx_mgmt_api_test_util.erl | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/apps/emqx_management/test/emqx_mgmt_api_test_util.erl b/apps/emqx_management/test/emqx_mgmt_api_test_util.erl index 1bc29dfee..aed28930b 100644 --- a/apps/emqx_management/test/emqx_mgmt_api_test_util.erl +++ b/apps/emqx_management/test/emqx_mgmt_api_test_util.erl @@ -44,15 +44,20 @@ set_special_configs(_App) -> ok. request_api(Method, Url) -> - request_api(Method, Url, [], auth_header_(), []). + request_api(Method, Url, [], [], [], #{}). request_api(Method, Url, AuthOrHeaders) -> - request_api(Method, Url, [], AuthOrHeaders, []). + request_api(Method, Url, [], AuthOrHeaders, [], #{}). request_api(Method, Url, QueryParams, AuthOrHeaders) -> - request_api(Method, Url, QueryParams, AuthOrHeaders, []). + request_api(Method, Url, QueryParams, AuthOrHeaders, [], #{}). -request_api(Method, Url, QueryParams, AuthOrHeaders, []) when +request_api(Method, Url, QueryParams, AuthOrHeaders, Body) -> + request_api(Method, Url, QueryParams, AuthOrHeaders, Body, #{}). + +request_api(Method, Url, QueryParams, [], Body, Opts) -> + request_api(Method, Url, QueryParams, auth_header_(), Body, Opts); +request_api(Method, Url, QueryParams, AuthOrHeaders, [], Opts) when (Method =:= options) orelse (Method =:= get) orelse (Method =:= put) orelse @@ -65,10 +70,7 @@ request_api(Method, Url, QueryParams, AuthOrHeaders, []) when "" -> Url; _ -> Url ++ "?" ++ QueryParams end, - do_request_api(Method, {NewUrl, build_http_header(AuthOrHeaders)}, #{}); -request_api(Method, Url, QueryParams, AuthOrHeaders, Body) -> - request_api(Method, Url, QueryParams, AuthOrHeaders, Body, #{}). - + do_request_api(Method, {NewUrl, build_http_header(AuthOrHeaders)}, Opts); request_api(Method, Url, QueryParams, AuthOrHeaders, Body, Opts) when (Method =:= post) orelse (Method =:= patch) orelse From 1fb441dd9e1977ddaada4704fc9117b1f1cbc04f Mon Sep 17 00:00:00 2001 From: Stefan Strigler Date: Fri, 4 Nov 2022 16:07:02 +0100 Subject: [PATCH 3/5] test: add tests for 'configs?node' --- .../test/emqx_mgmt_api_configs_SUITE.erl | 56 ++++++++++++++++++- 1 file changed, 54 insertions(+), 2 deletions(-) diff --git a/apps/emqx_management/test/emqx_mgmt_api_configs_SUITE.erl b/apps/emqx_management/test/emqx_mgmt_api_configs_SUITE.erl index 97939bbaf..83f68c5fe 100644 --- a/apps/emqx_management/test/emqx_mgmt_api_configs_SUITE.erl +++ b/apps/emqx_management/test/emqx_mgmt_api_configs_SUITE.erl @@ -30,6 +30,16 @@ init_per_suite(Config) -> end_per_suite(_) -> emqx_mgmt_api_test_util:end_suite([emqx_conf]). +init_per_testcase(TestCase = t_configs_node, Config) -> + ?MODULE:TestCase({'init', Config}); +init_per_testcase(_TestCase, Config) -> + Config. + +end_per_testcase(TestCase = t_configs_node, Config) -> + ?MODULE:TestCase({'end', Config}); +end_per_testcase(_TestCase, Config) -> + Config. + t_get(_Config) -> {ok, Configs} = get_configs(), maps:map( @@ -188,6 +198,37 @@ t_dashboard(_Config) -> timer:sleep(1000), ok. +t_configs_node({'init', Config}) -> + Node = node(), + meck:expect(mria_mnesia, running_nodes, fun() -> [Node, bad_node, other_node] end), + meck:expect( + emqx_management_proto_v2, + get_full_config, + fun + (Node0) when Node0 =:= Node -> <<"\"self\"">>; + (other_node) -> <<"\"other\"">>; + (bad_node) -> {badrpc, bad} + end + ), + Config; +t_configs_node({'end', _}) -> + meck:unload([mria_mnesia, emqx_management_proto_v2]); +t_configs_node(_) -> + Node = atom_to_list(node()), + + ?assertEqual({ok, <<"self">>}, get_configs(Node, #{return_body => true})), + ?assertEqual({ok, <<"other">>}, get_configs("other_node", #{return_body => true})), + + {ExpType, ExpRes} = get_configs("unknown_node", #{return_body => true}), + ?assertEqual(error, ExpType), + ?assertMatch({{_, 404, _}, _, _}, ExpRes), + {_, _, Body} = ExpRes, + ?assertMatch(#{<<"code">> := <<"NOT_FOUND">>}, emqx_json:decode(Body, [return_maps])), + + ?assertMatch({error, {_, 500, _}}, get_configs("bad_node")). + +%% Helpers + get_config(Name) -> Path = emqx_mgmt_api_test_util:api_path(["configs", Name]), case emqx_mgmt_api_test_util:request_api(get, Path) of @@ -198,8 +239,19 @@ get_config(Name) -> end. get_configs() -> - Path = emqx_mgmt_api_test_util:api_path(["configs"]), - case emqx_mgmt_api_test_util:request_api(get, Path) of + get_configs([], #{}). + +get_configs(Node) -> + get_configs(Node, #{}). + +get_configs(Node, Opts) -> + Path = + case Node of + [] -> ["configs"]; + _ -> ["configs?node=" ++ Node] + end, + URI = emqx_mgmt_api_test_util:api_path(Path), + case emqx_mgmt_api_test_util:request_api(get, URI, [], [], [], Opts) of {ok, Res} -> {ok, emqx_json:decode(Res, [return_maps])}; Error -> Error end. From ea5b2cba6a81a93b1a4a6d5b57e9daa3d7a78abc Mon Sep 17 00:00:00 2001 From: Stefan Strigler Date: Mon, 7 Nov 2022 15:02:26 +0100 Subject: [PATCH 4/5] chore: add changelog --- changes/v5.0.10-en.md | 2 ++ changes/v5.0.10-zh.md | 2 ++ 2 files changed, 4 insertions(+) diff --git a/changes/v5.0.10-en.md b/changes/v5.0.10-en.md index 9ec10d3a7..69fee3aae 100644 --- a/changes/v5.0.10-en.md +++ b/changes/v5.0.10-en.md @@ -35,3 +35,5 @@ - Fix incorrect topic authorize checking of delayed messages [#9290](https://github.com/emqx/emqx/pull/9290). Now will determine the actual topic of the delayed messages, e.g. `$delayed/1/t/foo` will be treated as `t/foo` in authorize checks. + +- Fix query string parameter 'node' to `/configs` resource being ignored, return 404 if node does not exist [9310](https://github.com/emqx/emqx/pull/9310/). diff --git a/changes/v5.0.10-zh.md b/changes/v5.0.10-zh.md index d7e758162..63a4a89c5 100644 --- a/changes/v5.0.10-zh.md +++ b/changes/v5.0.10-zh.md @@ -33,3 +33,5 @@ - 修复延迟消息的主题授权判断不正确的问题 [#9290](https://github.com/emqx/emqx/pull/9290)。 现在将会对延迟消息中的真实主题进行授权判断，比如，`$delayed/1/t/foo` 会被当作 `t/foo` 进行判断。 + +- 修复 `/configs` API 的 'node' 参数的问题，如果节点不存在，则返回 HTTP 状态码 404 From 62856a8b77a35ec6811019f79ae789c2fa5fd121 Mon Sep 17 00:00:00 2001 From: Stefan Strigler Date: Tue, 8 Nov 2022 16:10:18 +0100 Subject: [PATCH 5/5] style: fix typo in changelog Co-authored-by: Thales Macedo Garitezi --- changes/v5.0.10-en.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changes/v5.0.10-en.md b/changes/v5.0.10-en.md index 69fee3aae..a4e2097b2 100644 --- a/changes/v5.0.10-en.md +++ b/changes/v5.0.10-en.md @@ -36,4 +36,4 @@ - Fix incorrect topic authorize checking of delayed messages [#9290](https://github.com/emqx/emqx/pull/9290). Now will determine the actual topic of the delayed messages, e.g. `$delayed/1/t/foo` will be treated as `t/foo` in authorize checks. -- Fix query string parameter 'node' to `/configs` resource being ignored, return 404 if node does not exist [9310](https://github.com/emqx/emqx/pull/9310/). +- Fix query string parameter 'node' to `/configs` resource being ignored, return 404 if node does not exist [#9310](https://github.com/emqx/emqx/pull/9310/).