refactor(gw): simplify code structure

1. Add the management API for listener's authn
2. Clarify responsed error messages

apps/emqx_gateway/src/emqx_gateway_api_authn.erl

@@ -34,6 +34,9 @@
 %% http handlers
 -export([authn/2]).
 
+%% internal export for emqx_gateway_api_listeners module
+-export([schema_authn/0]).
+
 %%--------------------------------------------------------------------
 %% minirest behaviour callbacks
 %%--------------------------------------------------------------------
@@ -50,42 +53,27 @@ apis() ->
 
 authn(get, #{bindings := #{name := Name0}}) ->
     with_gateway(Name0, fun(GwName, _) ->
-        case emqx_gateway_http:authn(GwName) of
+        {200, emqx_gateway_http:authn(GwName)}
-            undefined ->
-                return_http_error(404, "No Authentication");
-            Auth ->
-                {200, Auth}
-        end
     end);
 
 authn(put, #{bindings := #{name := Name0},
              body := Body}) ->
     with_gateway(Name0, fun(GwName, _) ->
-        case emqx_gateway_http:update_authn(GwName, Body) of
+        ok = emqx_gateway_http:update_authn(GwName, Body),
-            ok ->
+        {204}
-                {204};
-            {error, Reason} ->
-                return_http_error(500, Reason)
-        end
     end);
 
 authn(post, #{bindings := #{name := Name0},
               body := Body}) ->
     with_gateway(Name0, fun(GwName, _) ->
-        case emqx_gateway_http:add_authn(GwName, Body) of
+        ok = emqx_gateway_http:add_authn(GwName, Body),
-            ok -> {204};
+        {204}
-            {error, Reason} ->
-                return_http_error(500, Reason)
-        end
     end);
 
 authn(delete, #{bindings := #{name := Name0}}) ->
     with_gateway(Name0, fun(GwName, _) ->
-        case emqx_gateway_http:remove_authn(GwName) of
+        ok = emqx_gateway_http:remove_authn(GwName),
-            ok -> {204};
+        {204}
-            {error, Reason} ->
-                return_http_error(500, Reason)
-        end
     end).
 
 %%--------------------------------------------------------------------

apps/emqx_gateway/src/emqx_gateway_api_listeners.erl

@@ -28,12 +28,15 @@
         , checks/2
         ]).
 
+-import(emqx_gateway_api_authn, [schema_authn/0]).
+
 %% minirest behaviour callbacks
 -export([api_spec/0]).
 
 %% http handlers
 -export([ listeners/2
         , listeners_insta/2
+        , listeners_insta_authn/2
         ]).
 
 %%--------------------------------------------------------------------
@@ -46,6 +49,7 @@ api_spec() ->
 apis() ->
     [ {"/gateway/:name/listeners", listeners}
     , {"/gateway/:name/listeners/:id", listeners_insta}
+    , {"/gateway/:name/listeners/:id/authentication", listeners_insta_authn}
     ].
 
 %%--------------------------------------------------------------------
@@ -70,27 +74,18 @@ listeners(post, #{bindings := #{name := Name0}, body := LConf}) ->
             undefined ->
                 ListenerId = emqx_gateway_utils:listener_id(
                                GwName, Type, LName),
-                case emqx_gateway_http:add_listener(ListenerId, LConf) of
+                ok = emqx_gateway_http:add_listener(ListenerId, LConf),
-                    ok ->
+                {204};
-                       {204};
-                    {error, Reason} ->
-                        return_http_error(500, Reason)
-                end;
             _ ->
                 return_http_error(400, "Listener name has occupied")
         end
     end).
 
-%% FIXME: not working
 listeners_insta(delete, #{bindings := #{name := Name0, id := ListenerId0}}) ->
     ListenerId = emqx_mgmt_util:urldecode(ListenerId0),
     with_gateway(Name0, fun(_GwName, _) ->
-        case emqx_gateway_http:remove_listener(ListenerId) of
+        ok = emqx_gateway_http:remove_listener(ListenerId),
-            ok -> {204};
+        {204}
-            {error, not_found} -> {204};
-            {error, Reason} ->
-                return_http_error(500, Reason)
-        end
     end);
 listeners_insta(get, #{bindings := #{name := Name0, id := ListenerId0}}) ->
     ListenerId = emqx_mgmt_util:urldecode(ListenerId0),
@@ -109,12 +104,38 @@ listeners_insta(put, #{body := LConf,
                       }) ->
     ListenerId = emqx_mgmt_util:urldecode(ListenerId0),
     with_gateway(Name0, fun(_GwName, _) ->
-        case emqx_gateway_http:update_listener(ListenerId, LConf) of
+        ok = emqx_gateway_http:update_listener(ListenerId, LConf),
-            ok ->
+        {204}
-                {204};
+    end).
-            {error, Reason} ->
+
-                return_http_error(500, Reason)
+listeners_insta_authn(get, #{bindings := #{name := Name0,
-        end
+                                           id := ListenerId0}}) ->
+    ListenerId = emqx_mgmt_util:urldecode(ListenerId0),
+    with_gateway(Name0, fun(GwName, _) ->
+        {200, emqx_gateway_http:authn(GwName, ListenerId)}
+    end);
+listeners_insta_authn(post, #{body := Conf,
+                              bindings := #{name := Name0,
+                                            id := ListenerId0}}) ->
+    ListenerId = emqx_mgmt_util:urldecode(ListenerId0),
+    with_gateway(Name0, fun(GwName, _) ->
+        ok = emqx_gateway_http:add_authn(GwName, ListenerId, Conf),
+        {204}
+    end);
+listeners_insta_authn(put, #{body := Conf,
+                             bindings := #{name := Name0,
+                                           id := ListenerId0}}) ->
+    ListenerId = emqx_mgmt_util:urldecode(ListenerId0),
+    with_gateway(Name0, fun(GwName, _) ->
+        ok = emqx_gateway_http:update_authn(GwName, ListenerId, Conf),
+        {204}
+    end);
+listeners_insta_authn(delete, #{bindings := #{name := Name0,
+                                              id := ListenerId0}}) ->
+    ListenerId = emqx_mgmt_util:urldecode(ListenerId0),
+    with_gateway(Name0, fun(GwName, _) ->
+        ok = emqx_gateway_http:remove_authn(GwName, ListenerId),
+        {204}
     end).
 
 %%--------------------------------------------------------------------
@@ -191,6 +212,52 @@ swagger("/gateway/:name/listeners/:id", put) ->
          , <<"500">> => schema_internal_error()
          , <<"200">> => schema_no_content()
          }
+     };
+swagger("/gateway/:name/listeners/:id/authentication", get) ->
+    #{ description => <<"Get the listener's authentication info">>
+     , parameters => params_gateway_name_in_path()
+                     ++ params_listener_id_in_path()
+     , responses =>
+        #{ <<"400">> => schema_bad_request()
+         , <<"404">> => schema_not_found()
+         , <<"500">> => schema_internal_error()
+         , <<"200">> => schema_authn()
+         }
+     };
+swagger("/gateway/:name/listeners/:id/authentication", post) ->
+    #{ description => <<"Add authentication for the listener">>
+     , parameters => params_gateway_name_in_path()
+                     ++ params_listener_id_in_path()
+     , requestBody => schema_authn()
+     , responses =>
+        #{ <<"400">> => schema_bad_request()
+         , <<"404">> => schema_not_found()
+         , <<"500">> => schema_internal_error()
+         , <<"204">> => schema_no_content()
+         }
+     };
+swagger("/gateway/:name/listeners/:id/authentication", put) ->
+    #{ description => <<"Update authentication for the listener">>
+     , parameters => params_gateway_name_in_path()
+                     ++ params_listener_id_in_path()
+     , requestBody => schema_authn()
+     , responses =>
+        #{ <<"400">> => schema_bad_request()
+         , <<"404">> => schema_not_found()
+         , <<"500">> => schema_internal_error()
+         , <<"204">> => schema_no_content()
+         }
+     };
+swagger("/gateway/:name/listeners/:id/authentication", delete) ->
+    #{ description => <<"Remove authentication for the listener">>
+     , parameters => params_gateway_name_in_path()
+                     ++ params_listener_id_in_path()
+     , responses =>
+        #{ <<"400">> => schema_bad_request()
+         , <<"404">> => schema_not_found()
+         , <<"500">> => schema_internal_error()
+         , <<"204">> => schema_no_content()
+         }
      }.
 
 %%--------------------------------------------------------------------

apps/emqx_gateway/src/emqx_gateway_http.erl

@@ -34,9 +34,13 @@
         ]).
 
 -export([ authn/1
+        , authn/2
         , add_authn/2
+        , add_authn/3
         , update_authn/2
+        , update_authn/3
         , remove_authn/1
+        , remove_authn/2
         ]).
 
 %% Mgmt APIs - clients
@@ -205,47 +209,69 @@ bind2str(LConf = #{bind := Bind}) when is_binary(Bind) ->
 bind2str(LConf = #{<<"bind">> := Bind}) when is_binary(Bind) ->
     LConf.
 
--spec add_listener(atom() | binary(), map()) -> ok | {error, any()}.
+-spec add_listener(atom() | binary(), map()) -> ok.
 add_listener(ListenerId, NewConf0) ->
     {GwName, Type, Name} = emqx_gateway_utils:parse_listener_id(ListenerId),
     NewConf = maps:without([<<"id">>, <<"name">>,
                             <<"type">>, <<"running">>], NewConf0),
-    emqx_gateway_conf:add_listener(GwName, {Type, Name}, NewConf).
+    confexp(emqx_gateway_conf:add_listener(GwName, {Type, Name}, NewConf)).
 
--spec update_listener(atom() | binary(), map()) -> ok | {error, any()}.
+-spec update_listener(atom() | binary(), map()) -> ok.
 update_listener(ListenerId, NewConf0) ->
     {GwName, Type, Name} = emqx_gateway_utils:parse_listener_id(ListenerId),
 
     NewConf = maps:without([<<"id">>, <<"name">>,
                             <<"type">>, <<"running">>], NewConf0),
-    emqx_gateway_conf:update_listener(GwName, {Type, Name}, NewConf).
+    confexp(emqx_gateway_conf:update_listener(GwName, {Type, Name}, NewConf)).
 
--spec remove_listener(binary()) -> ok | {error, not_found} | {error, any()}.
+-spec remove_listener(binary()) -> ok.
 remove_listener(ListenerId) ->
     {GwName, Type, Name} = emqx_gateway_utils:parse_listener_id(ListenerId),
-    emqx_gateway_conf:remove_listener(GwName, {Type, Name}).
+    confexp(emqx_gateway_conf:remove_listener(GwName, {Type, Name})).
 
--spec authn(gateway_name()) -> map() | undefined.
+-spec authn(gateway_name()) -> map().
 authn(GwName) ->
-    case emqx_map_lib:deep_get(
+    Path = [gateway, GwName, authentication],
-           [authentication],
+    emqx_map_lib:jsonable_map(emqx:get_config(Path)).
-           emqx:get_config([gateway, GwName]),
-           undefined)  of
-        undefined -> undefined;
-        AuthConf -> emqx_map_lib:jsonable_map(AuthConf)
-    end.
 
--spec add_authn(gateway_name(), map()) -> ok | {error, any()}.
+-spec authn(gateway_name(), binary()) -> map().
+authn(GwName, ListenerId) ->
+    {_, Type, Name} = emqx_gateway_utils:parse_listener_id(ListenerId),
+    Path = [gateway, GwName, listeners, Type, Name, authentication],
+    emqx_map_lib:jsonable_map(emqx:get_config(Path)).
+
+-spec add_authn(gateway_name(), map()) -> ok.
 add_authn(GwName, AuthConf) ->
-    emqx_gateway_conf:add_authn(GwName, AuthConf).
+    confexp(emqx_gateway_conf:add_authn(GwName, AuthConf)).
 
--spec update_authn(gateway_name(), map()) -> ok | {error, any()}.
+-spec add_authn(gateway_name(), binary(), map()) -> ok.
+add_authn(GwName, ListenerId, AuthConf) ->
+    {_, Type, Name} = emqx_gateway_utils:parse_listener_id(ListenerId),
+    confexp(emqx_gateway_conf:add_authn(GwName, {Type, Name}, AuthConf)).
+
+-spec update_authn(gateway_name(), map()) -> ok.
 update_authn(GwName, AuthConf) ->
-    emqx_gateway_conf:update_authn(GwName, AuthConf).
+    confexp(emqx_gateway_conf:update_authn(GwName, AuthConf)).
 
--spec remove_authn(gateway_name()) -> ok | {error, any()}.
+-spec update_authn(gateway_name(), binary(), map()) -> ok.
+update_authn(GwName, ListenerId, AuthConf) ->
+    {_, Type, Name} = emqx_gateway_utils:parse_listener_id(ListenerId),
+    confexp(emqx_gateway_conf:update_authn(GwName, {Type, Name}, AuthConf)).
+
+-spec remove_authn(gateway_name()) -> ok.
 remove_authn(GwName) ->
-    emqx_gateway_conf:remove_authn(GwName).
+    confexp(emqx_gateway_conf:remove_authn(GwName)).
+
+-spec remove_authn(gateway_name(), binary()) -> ok.
+remove_authn(GwName, ListenerId) ->
+    {_, Type, Name} = emqx_gateway_utils:parse_listener_id(ListenerId),
+    confexp(emqx_gateway_conf:remove_authn(GwName, {Type, Name})).
+
+confexp(ok) -> ok;
+confexp({error, not_found}) ->
+    error({update_conf_error, not_found});
+confexp({error, already_exist}) ->
+    error({update_conf_error, already_exist}).
 
 %%--------------------------------------------------------------------
 %% Mgmt APIs - clients
@@ -365,10 +391,22 @@ with_gateway(GwName0, Fun) ->
     catch
         error : badname ->
             return_http_error(404, "Bad gateway name");
+        %% Exceptions from: checks/2
         error : {miss_param, K} ->
             return_http_error(400, [K, " is required"]);
+        %% Exceptions from emqx_gateway_utils:parse_listener_id/1
         error : {invalid_listener_id, Id} ->
             return_http_error(400, ["invalid listener id: ", Id]);
+        %% Exceptions from: emqx:get_config/1
+        error : {config_not_found, Path0} ->
+            Path = lists:concat(
+                     lists:join(".", lists:map(fun to_list/1, Path0))),
+            return_http_error(404, "Resource not found. path: " ++ Path);
+        %% Exceptions from: confexp/1
+        error : {update_conf_error, not_found} ->
+            return_http_error(404, "Resource not found");
+        error : {update_conf_error, already_exist} ->
+            return_http_error(400, "Resource already exist");
         Class : Reason : Stk ->
             ?LOG(error, "Uncatched error: {~p, ~p}, stacktrace: ~0p",
                         [Class, Reason, Stk]),
@@ -385,6 +423,11 @@ checks([K|Ks], Map) ->
             error({miss_param, K})
     end.
 
+to_list(A) when is_atom(A) ->
+    atom_to_list(A);
+to_list(B) when is_binary(B) ->
+    binary_to_list(B).
+
 %%--------------------------------------------------------------------
 %% common schemas