Merge pull request #9211 from thalesmg/fix-mgmt-endpoint-status-code-v50

fix(mgmt_api): return 503 when emqx is not running in `/status` (5.0)

apps/emqx_management/src/emqx_mgmt_api_status.erl

@@ -45,7 +45,16 @@ running_status() ->
             BrokerStatus = broker_status(),
             AppStatus = application_status(),
             Body = io_lib:format("Node ~ts is ~ts~nemqx is ~ts", [node(), BrokerStatus, AppStatus]),
-            {200, #{<<"content-type">> => <<"text/plain">>}, list_to_binary(Body)};
+            StatusCode =
+                case AppStatus of
+                    running -> 200;
+                    not_running -> 503
+                end,
+            Headers = #{
+                <<"content-type">> => <<"text/plain">>,
+                <<"retry-after">> => <<"15">>
+            },
+            {StatusCode, Headers, list_to_binary(Body)};
         false ->
             {503, #{<<"retry-after">> => <<"15">>}, <<>>}
     end.

apps/emqx_management/test/emqx_mgmt_api_status_SUITE.erl

@@ -20,6 +20,12 @@
 
 -include_lib("eunit/include/eunit.hrl").
 
+-define(HOST, "http://127.0.0.1:18083/").
+
+%%---------------------------------------------------------------------------------------
+%% CT boilerplate
+%%---------------------------------------------------------------------------------------
+
 all() ->
     emqx_common_test_helpers:all(?MODULE).
 
@@ -30,8 +36,102 @@ init_per_suite(Config) ->
 end_per_suite(_) ->
     emqx_mgmt_api_test_util:end_suite().
 
-t_status(_Config) ->
-    Path = emqx_mgmt_api_test_util:api_path_without_base_path(["/status"]),
-    Status = io_lib:format("Node ~ts is ~ts~nemqx is ~ts", [node(), started, running]),
-    {ok, Status} = emqx_mgmt_api_test_util:request_api(get, Path),
+init_per_testcase(t_status_not_ok, Config) ->
+    ok = application:stop(emqx),
+    Config;
+init_per_testcase(_TestCase, Config) ->
+    Config.
+
+end_per_testcase(t_status_not_ok, _Config) ->
+    {ok, _} = application:ensure_all_started(emqx),
+    ok;
+end_per_testcase(_TestCase, _Config) ->
+    ok.
+
+%%---------------------------------------------------------------------------------------
+%% Helper fns
+%%---------------------------------------------------------------------------------------
+
+do_request(Opts) ->
+    #{
+        path := Path0,
+        method := Method,
+        headers := Headers,
+        body := Body0
+    } = Opts,
+    URL = ?HOST ++ filename:join(Path0),
+    {ok, #{host := Host, port := Port, path := Path}} = emqx_http_lib:uri_parse(URL),
+    %% we must not use `httpc' here, because it keeps retrying when it
+    %% receives a 503 with `retry-after' header, and there's no option
+    %% to stop that behavior...
+    {ok, Gun} = gun:open(Host, Port, #{retry => 0}),
+    {ok, http} = gun:await_up(Gun),
+    Request =
+        fun() ->
+            case Body0 of
+                no_body -> gun:Method(Gun, Path, Headers);
+                {_Encoding, Body} -> gun:Method(Gun, Path, Headers, Body)
+            end
+        end,
+    Ref = Request(),
+    receive
+        {gun_response, Gun, Ref, nofin, StatusCode, Headers1} ->
+            Data = data_loop(Gun, Ref, _Acc = <<>>),
+            #{status_code => StatusCode, headers => maps:from_list(Headers1), body => Data}
+    after 5_000 ->
+        error({timeout, Opts, process_info(self(), messages)})
+    end.
+
+data_loop(Gun, Ref, Acc) ->
+    receive
+        {gun_data, Gun, Ref, nofin, Data} ->
+            data_loop(Gun, Ref, <<Acc/binary, Data/binary>>);
+        {gun_data, Gun, Ref, fin, Data} ->
+            gun:shutdown(Gun),
+            <<Acc/binary, Data/binary>>
+    after 5000 ->
+        error(timeout)
+    end.
+
+%%---------------------------------------------------------------------------------------
+%% Test cases
+%%---------------------------------------------------------------------------------------
+
+t_status_ok(_Config) ->
+    #{
+        body := Resp,
+        status_code := StatusCode
+    } = do_request(#{
+        method => get,
+        path => ["status"],
+        headers => [],
+        body => no_body
+    }),
+    ?assertEqual(200, StatusCode),
+    ?assertMatch(
+        {match, _},
+        re:run(Resp, <<"emqx is running$">>)
+    ),
+    ok.
+
+t_status_not_ok(_Config) ->
+    #{
+        body := Resp,
+        headers := Headers,
+        status_code := StatusCode
+    } = do_request(#{
+        method => get,
+        path => ["status"],
+        headers => [],
+        body => no_body
+    }),
+    ?assertEqual(503, StatusCode),
+    ?assertMatch(
+        {match, _},
+        re:run(Resp, <<"emqx is not_running$">>)
+    ),
+    ?assertMatch(
+        #{<<"retry-after">> := <<"15">>},
+        Headers
+    ),
     ok.

changes/v5.0.10-en.md

@@ -16,3 +16,6 @@
 ## Bug fixes
 
 - Fix error log message when `mechanism` is missing in authentication config [#8924](https://github.com/emqx/emqx/pull/8924).
+
+- Fixed the HTTP response status code for the `/status` endpoint [#9211](https://github.com/emqx/emqx/pull/9211).
+  Before the fix, it always returned `200` even if the EMQX application was not running. Now it returns `503` in that case.

changes/v5.0.10-zh.md

@@ -15,3 +15,6 @@
 ## Bug fixes
 
 - 优化认认证配置中 `mechanism` 字段缺失情况下的错误日志 [#8924](https://github.com/emqx/emqx/pull/8924)。
+
+- 修正了 `/status` 端点的响应状态代码 [#9211](https://github.com/emqx/emqx/pull/9211)。
+  在此修复前，它总是返回 HTTP 状态码 `200`，即使 EMQX 没有完成启动或正在重启。 现在它在这些情况下会返回状态码 `503`。