Merge pull request #9310 from sstrigler/fix-configs-api-for-bad-nodes

fix(emqx_mgmt_api_configs): use 'node' query paramter if given

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}};

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.

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

changes/v5.0.10-en.md

@@ -36,3 +36,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/).

changes/v5.0.10-zh.md

@@ -34,3 +34,5 @@
 
 - 修复延迟消息的主题授权判断不正确的问题 [#9290](https://github.com/emqx/emqx/pull/9290)。
   现在将会对延迟消息中的真实主题进行授权判断，比如，`$delayed/1/t/foo` 会被当作 `t/foo` 进行判断。
+
+- 修复 `/configs` API 的 'node' 参数的问题，如果节点不存在，则返回 HTTP 状态码 404