From 997a262c96d91c538a82786a80ebb3514b4dcac3 Mon Sep 17 00:00:00 2001
From: Thales Macedo Garitezi <thalesmg@gmail.com>
Date: Fri, 21 Oct 2022 15:57:30 -0300
Subject: [PATCH] fix(mgmt_api): return 503 when emqx is not running in
 `/status` (5.0)

---
 .../src/emqx_mgmt_api_status.erl              |  7 +-
 .../test/emqx_mgmt_api_status_SUITE.erl       | 88 ++++++++++++++++++-
 changes/v5.0.10-en.md                         |  2 +
 changes/v5.0.10-zh.md                         |  2 +
 4 files changed, 94 insertions(+), 5 deletions(-)

diff --git a/apps/emqx_management/src/emqx_mgmt_api_status.erl b/apps/emqx_management/src/emqx_mgmt_api_status.erl
index 68bad03e4..37a8274f8 100644
--- a/apps/emqx_management/src/emqx_mgmt_api_status.erl
+++ b/apps/emqx_management/src/emqx_mgmt_api_status.erl
@@ -45,7 +45,12 @@ 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,
+            {StatusCode, #{<<"content-type">> => <<"text/plain">>}, list_to_binary(Body)};
         false ->
             {503, #{<<"retry-after">> => <<"15">>}, <<>>}
     end.
diff --git a/apps/emqx_management/test/emqx_mgmt_api_status_SUITE.erl b/apps/emqx_management/test/emqx_mgmt_api_status_SUITE.erl
index b725e37b2..4051bd62c 100644
--- a/apps/emqx_management/test/emqx_mgmt_api_status_SUITE.erl
+++ b/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,82 @@ 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 := Path,
+        method := Method,
+        headers := Headers,
+        body := Body0
+    } = Opts,
+    URL = ?HOST ++ filename:join(Path),
+    Request =
+        case Body0 of
+            no_body -> {URL, Headers};
+            {Encoding, Body} -> {URL, Headers, Encoding, Body}
+        end,
+    ct:pal("Method: ~p, Request: ~p", [Method, Request]),
+    case httpc:request(Method, Request, [], []) of
+        {error, socket_closed_remotely} ->
+            {error, socket_closed_remotely};
+        {ok, {{_, StatusCode, _}, Headers1, Body1}} ->
+            Body2 =
+                case emqx_json:safe_decode(Body1, [return_maps]) of
+                    {ok, Json} -> Json;
+                    {error, _} -> Body1
+                end,
+            {ok, #{status_code => StatusCode, headers => Headers1, body => Body2}}
+    end.
+
+%%---------------------------------------------------------------------------------------
+%% Test cases
+%%---------------------------------------------------------------------------------------
+
+t_status_ok(_Config) ->
+    {ok, #{
+        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) ->
+    {ok, #{
+        body := Resp,
+        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$">>)
+    ),
     ok.
diff --git a/changes/v5.0.10-en.md b/changes/v5.0.10-en.md
index a822817d4..52400b5e6 100644
--- a/changes/v5.0.10-en.md
+++ b/changes/v5.0.10-en.md
@@ -9,3 +9,5 @@
 ## Bug fixes
 
 - Fix error log message when `mechanism` is missing in authentication config [#8924](https://github.com/emqx/emqx/pull/8924).
+
+- Fixed the response status code for the `/status` endpoint. Before the fix, it always returned 200 even if the EMQX application was not running. Now it returns 503 in that case.
diff --git a/changes/v5.0.10-zh.md b/changes/v5.0.10-zh.md
index 0368a6f60..14bb94d1d 100644
--- a/changes/v5.0.10-zh.md
+++ b/changes/v5.0.10-zh.md
@@ -9,3 +9,5 @@
 ## Bug fixes
 
 - 优化认认证配置中 `mechanism` 字段缺失情况下的错误日志 [#8924](https://github.com/emqx/emqx/pull/8924)。
+
+- 修正了`/status`端点的响应状态代码。 在修复之前，它总是返回200，即使EMQX应用程序没有运行。 现在它在这种情况下返回503。