From e737f18548100b71b8856405b35ca49c26f147ea Mon Sep 17 00:00:00 2001 From: JimMoen Date: Thu, 23 Sep 2021 09:16:33 +0800 Subject: [PATCH] fix(mgmt_api): Friendly HTTP Status Code for Listeners. --- apps/emqx/src/emqx_listeners.erl | 10 ++-- apps/emqx_management/src/emqx_mgmt.erl | 16 +++-- .../src/emqx_mgmt_api_listeners.erl | 60 ++++++++++++++----- 3 files changed, 61 insertions(+), 25 deletions(-) diff --git a/apps/emqx/src/emqx_listeners.erl b/apps/emqx/src/emqx_listeners.erl index 06d89c86d..5c4776207 100644 --- a/apps/emqx/src/emqx_listeners.erl +++ b/apps/emqx/src/emqx_listeners.erl @@ -46,6 +46,7 @@ -export([post_config_update/4]). -define(CONF_KEY_PATH, [listeners]). +-define(TYPES_STRING, ["tcp","ssl","ws","wss","quic"]). %% @doc List configured listeners. -spec(list() -> [{ListenerId :: atom(), ListenerConf :: map()}]). @@ -349,11 +350,10 @@ listener_id(Type, ListenerName) -> list_to_atom(lists:append([str(Type), ":", str(ListenerName)])). parse_listener_id(Id) -> - try - [Type, Name] = string:split(str(Id), ":", leading), - {list_to_existing_atom(Type), list_to_atom(Name)} - catch - _ : _ -> error({invalid_listener_id, Id}) + [Type, Name] = string:split(str(Id), ":", leading), + case lists:member(Type, ?TYPES_STRING) of + true -> {list_to_existing_atom(Type), list_to_atom(Name)}; + false -> {error, {invalid_listener_id, Id}} end. zone(Opts) -> diff --git a/apps/emqx_management/src/emqx_mgmt.erl b/apps/emqx_management/src/emqx_mgmt.erl index 3cc31b47f..3b1fd5903 100644 --- a/apps/emqx_management/src/emqx_mgmt.erl +++ b/apps/emqx_management/src/emqx_mgmt.erl @@ -508,12 +508,16 @@ update_listener(Id, Config) -> [update_listener(Node, Id, Config) || Node <- ekka_mnesia:running_nodes()]. update_listener(Node, Id, Config) when Node =:= node() -> - {Type, Name} = emqx_listeners:parse_listener_id(Id), - case emqx:update_config([listeners, Type, Name], Config, #{}) of - {ok, #{raw_config := RawConf}} -> - RawConf#{node => Node, id => Id, running => true}; - {error, Reason} -> - error(Reason) + case emqx_listeners:parse_listener_id(Id) of + {error, {invalid_listener_id, Id}} -> + {error, {invalid_listener_id, Id}}; + {Type, Name} -> + case emqx:update_config([listeners, Type, Name], Config, #{}) of + {ok, #{raw_config := RawConf}} -> + RawConf#{node => Node, id => Id, running => true}; + {error, Reason} -> + {error, Reason} + end end; update_listener(Node, Id, Config) -> rpc_call(Node, update_listener, [Node, Id, Config]). diff --git a/apps/emqx_management/src/emqx_mgmt_api_listeners.erl b/apps/emqx_management/src/emqx_mgmt_api_listeners.erl index 375bd258f..9e12cbe3a 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_listeners.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_listeners.erl @@ -35,6 +35,11 @@ -define(NODE_LISTENER_NOT_FOUND, <<"Node name or listener id not found">>). -define(NODE_NOT_FOUND_OR_DOWN, <<"Node not found or Down">>). -define(LISTENER_NOT_FOUND, <<"Listener id not found">>). +-define(ADDR_PORT_INUSE, <<"Addr port in use">>). +-define(CONFIG_SCHEMA_ERROR, <<"Config schema error">>). +-define(INVALID_LISTENER_PROTOCOL, <<"Invalid listener type">>). +-define(UPDATE_CONFIG_FAILED, <<"Update configuration failed">>). +-define(OPERATION_FAILED, <<"Operation failed">>). api_spec() -> { @@ -49,11 +54,11 @@ api_spec() -> [] }. --define(TYPES, [tcp, ssl, ws, wss, quic]). +-define(TYPES_ATOM, [tcp, ssl, ws, wss, quic]). req_schema() -> Schema = [emqx_mgmt_api_configs:gen_schema( emqx:get_raw_config([listeners, T, default], #{})) - || T <- ?TYPES], + || T <- ?TYPES_ATOM], #{oneOf => Schema}. resp_schema() -> @@ -91,8 +96,12 @@ api_list_update_listeners_by_id() -> parameters => [param_path_id()], requestBody => emqx_mgmt_util:schema(req_schema(), <<"Listener Config">>), responses => #{ + <<"400">> => + emqx_mgmt_util:error_schema(?UPDATE_CONFIG_FAILED, ['BAD_LISTENER_ID', 'BAD_CONFIG_SCHEMA']), <<"404">> => emqx_mgmt_util:error_schema(?LISTENER_NOT_FOUND, ['BAD_LISTENER_ID']), + <<"500">> => + emqx_mgmt_util:error_schema(?OPERATION_FAILED, ['INTERNAL_ERROR']), <<"200">> => emqx_mgmt_util:array_schema(resp_schema(), <<"Create or update listener successfully">>)}}, delete => #{ @@ -115,7 +124,7 @@ api_list_listeners_on_node() -> <<"404">> => emqx_mgmt_util:error_schema(?NODE_NOT_FOUND_OR_DOWN, ['RESOURCE_NOT_FOUND']), <<"500">> => - emqx_mgmt_util:error_schema(<<"Operation Failed">>, ['INTERNAL_ERROR']), + emqx_mgmt_util:error_schema(?OPERATION_FAILED, ['INTERNAL_ERROR']), <<"200">> => emqx_mgmt_util:schema(resp_schema(), <<"List listeners successfully">>)}}}, {"/nodes/:node/listeners", Metadata, list_listeners_on_node}. @@ -136,9 +145,13 @@ api_get_update_listener_by_id_on_node() -> parameters => [param_path_node(), param_path_id()], requestBody => emqx_mgmt_util:schema(req_schema(), <<"Listener Config">>), responses => #{ + <<"400">> => + emqx_mgmt_util:error_schema(?UPDATE_CONFIG_FAILED, ['BAD_LISTENER_ID', 'BAD_CONFIG_SCHEMA']), <<"404">> => emqx_mgmt_util:error_schema(?NODE_LISTENER_NOT_FOUND, ['BAD_NODE_NAME', 'BAD_LISTENER_ID']), + <<"500">> => + emqx_mgmt_util:error_schema(?OPERATION_FAILED, ['INTERNAL_ERROR']), <<"200">> => emqx_mgmt_util:schema(resp_schema(), <<"Get listener successfully">>)}}, delete => #{ @@ -160,7 +173,7 @@ api_manage_listeners() -> param_path_id(), param_path_operation()], responses => #{ - <<"500">> => emqx_mgmt_util:error_schema(<<"Operation Failed">>, ['INTERNAL_ERROR']), + <<"500">> => emqx_mgmt_util:error_schema(?OPERATION_FAILED, ['INTERNAL_ERROR']), <<"200">> => emqx_mgmt_util:schema(<<"Operation success">>)}}}, {"/listeners/:id/operation/:operation", Metadata, manage_listeners}. @@ -173,7 +186,7 @@ api_manage_listeners_on_node() -> param_path_id(), param_path_operation()], responses => #{ - <<"500">> => emqx_mgmt_util:error_schema(<<"Operation Failed">>, ['INTERNAL_ERROR']), + <<"500">> => emqx_mgmt_util:error_schema(?OPERATION_FAILED, ['INTERNAL_ERROR']), <<"200">> => emqx_mgmt_util:schema(<<"Operation success">>)}}}, {"/nodes/:node/listeners/:id/operation/:operation", Metadata, manage_listeners}. @@ -221,10 +234,23 @@ crud_listeners_by_id(get, #{bindings := #{id := Id}}) -> {200, format(Listeners)} end; crud_listeners_by_id(put, #{bindings := #{id := Id}, body := Conf}) -> - return_listeners(emqx_mgmt:update_listener(Id, Conf)); + Results = format(emqx_mgmt:update_listener(Id, Conf)), + case lists:filter(fun filter_errors/1, Results) of + [{error, {invalid_listener_id, Id}} | _] -> + {400, #{code => 'BAD_REQUEST', message => ?INVALID_LISTENER_PROTOCOL}}; + [{error, {emqx_machine_schema, _}} | _] -> + {400, #{code => 'BAD_REQUEST', message => ?CONFIG_SCHEMA_ERROR}}; + [{error, {eaddrinuse, _}} | _] -> + {400, #{code => 'BAD_REQUEST', message => ?ADDR_PORT_INUSE}}; + [{error, Reason} | _] -> + {500, #{code => 'UNKNOWN_ERROR', message => err_msg(Reason)}}; + [] -> + {200, Results} + end; + crud_listeners_by_id(delete, #{bindings := #{id := Id}}) -> Results = emqx_mgmt:remove_listener(Id), - case lists:filter(fun({error, _}) -> true; (_) -> false end, Results) of + case lists:filter(fun filter_errors/1, Results) of [] -> {200}; Errors -> {500, #{code => 'UNKNOW_ERROR', message => err_msg(Errors)}} end. @@ -250,6 +276,14 @@ crud_listener_by_id_on_node(get, #{bindings := #{id := Id, node := Node}}) -> end; crud_listener_by_id_on_node(put, #{bindings := #{id := Id, node := Node}, body := Conf}) -> case emqx_mgmt:update_listener(atom(Node), Id, Conf) of + {error, nodedown} -> + {404, #{code => 'RESOURCE_NOT_FOUND', message => ?NODE_NOT_FOUND_OR_DOWN}}; + {error, {invalid_listener_id, _}} -> + {400, #{code => 'BAD_REQUEST', message => ?INVALID_LISTENER_PROTOCOL}}; + {error, {emqx_machine_schema, _}} -> + {400, #{code => 'BAD_REQUEST', message => ?CONFIG_SCHEMA_ERROR}}; + {error, {eaddrinuse, _}} -> + {400, #{code => 'BAD_REQUEST', message => ?ADDR_PORT_INUSE}}; {error, Reason} -> {500, #{code => 'UNKNOW_ERROR', message => err_msg(Reason)}}; Listener -> @@ -301,13 +335,6 @@ do_manage_listeners2(<<"restart">>, Param) -> {500, #{code => 'UNKNOW_ERROR', message => err_msg(Reason)}} end. -return_listeners(Listeners) -> - Results = format(Listeners), - case lists:filter(fun({error, _}) -> true; (_) -> false end, Results) of - [] -> {200, Results}; - Errors -> {500, #{code => 'UNKNOW_ERROR', message => manage_listeners_err(Errors)}} - end. - manage_listeners_err(Errors) -> list_to_binary(lists:foldl(fun({Node, Err}, Str) -> err_msg_str(#{node => Node, error => Err}) ++ "; " ++ Str @@ -332,6 +359,11 @@ trans_running(Conf) -> Running end. +filter_errors({error, _}) -> + true; +filter_errors(_) -> + false. + jsonable_resp(bind, Port) when is_integer(Port) -> {bind, Port}; jsonable_resp(bind, {Addr, Port}) when is_tuple(Addr); is_integer(Port)->