From 30a72f557fe7d19ca9de5bf4c42ff025c77d6b77 Mon Sep 17 00:00:00 2001 From: firest Date: Wed, 1 Nov 2023 17:47:56 +0800 Subject: [PATCH] fix(gateway): improve gateway schema modules 1. enhances the gateway name as an enum 2. make the schema more flexible and extensible without some hardcode --- apps/emqx_gateway/src/emqx_gateway_api.erl | 65 +++++++------------ .../src/emqx_gateway_api_authn.erl | 2 +- .../emqx_gateway_api_authn_user_import.erl | 4 +- .../src/emqx_gateway_api_clients.erl | 2 +- .../src/emqx_gateway_api_listeners.erl | 2 +- apps/emqx_gateway/src/emqx_gateway_http.erl | 14 ++-- apps/emqx_gateway/src/emqx_gateway_schema.erl | 23 +++++-- apps/emqx_gateway/src/emqx_gateway_utils.erl | 45 ++++++++++++- .../test/emqx_gateway_api_SUITE.erl | 18 +++-- .../src/emqx_gateway_gbt32960.erl | 3 +- rel/i18n/emqx_gateway_api.hocon | 3 +- 11 files changed, 107 insertions(+), 74 deletions(-) diff --git a/apps/emqx_gateway/src/emqx_gateway_api.erl b/apps/emqx_gateway/src/emqx_gateway_api.erl index b628b47e3..bb80eac73 100644 --- a/apps/emqx_gateway/src/emqx_gateway_api.erl +++ b/apps/emqx_gateway/src/emqx_gateway_api.erl @@ -93,10 +93,9 @@ gateways(get, Request) -> gateway(get, #{bindings := #{name := Name}}) -> try - GwName = gw_name(Name), - case emqx_gateway:lookup(GwName) of + case emqx_gateway:lookup(Name) of undefined -> - {200, #{name => GwName, status => unloaded}}; + {200, #{name => Name, status => unloaded}}; Gateway -> GwConf = emqx_gateway_conf:gateway(Name), GwInfo0 = emqx_gateway_utils:unix_ts_to_rfc3339( @@ -125,15 +124,14 @@ gateway(put, #{ }) -> GwConf = maps:without([<<"name">>], GwConf0), try - GwName = gw_name(Name), LoadOrUpdateF = - case emqx_gateway:lookup(GwName) of + case emqx_gateway:lookup(Name) of undefined -> fun emqx_gateway_conf:load_gateway/2; _ -> fun emqx_gateway_conf:update_gateway/2 end, - case LoadOrUpdateF(GwName, GwConf) of + case LoadOrUpdateF(Name, GwConf) of {ok, _} -> {204}; {error, Reason} -> @@ -148,12 +146,11 @@ gateway(put, #{ gateway_enable(put, #{bindings := #{name := Name, enable := Enable}}) -> try - GwName = gw_name(Name), - case emqx_gateway:lookup(GwName) of + case emqx_gateway:lookup(Name) of undefined -> return_http_error(404, <<"NOT FOUND">>); _Gateway -> - {ok, _} = emqx_gateway_conf:update_gateway(GwName, #{<<"enable">> => Enable}), + {ok, _} = emqx_gateway_conf:update_gateway(Name, #{<<"enable">> => Enable}), {204} end catch @@ -161,15 +158,6 @@ gateway_enable(put, #{bindings := #{name := Name, enable := Enable}}) -> return_http_error(404, <<"NOT FOUND">>) end. --spec gw_name(binary()) -> stomp | coap | lwm2m | mqttsn | exproto | gbt32960 | no_return(). -gw_name(<<"stomp">>) -> stomp; -gw_name(<<"coap">>) -> coap; -gw_name(<<"lwm2m">>) -> lwm2m; -gw_name(<<"mqttsn">>) -> mqttsn; -gw_name(<<"exproto">>) -> exproto; -gw_name(<<"gbt32960">>) -> gbt32960; -gw_name(_Else) -> throw(not_found). - %%-------------------------------------------------------------------- %% Swagger defines %%-------------------------------------------------------------------- @@ -250,7 +238,7 @@ params_gateway_name_in_path() -> [ {name, mk( - binary(), + hoconsc:enum(emqx_gateway_schema:gateway_names()), #{ in => path, desc => ?DESC(gateway_name_in_qs), @@ -450,33 +438,30 @@ fields(gateway_stats) -> [{key, mk(binary(), #{})}]. schema_load_or_update_gateways_conf() -> + Names = emqx_gateway_schema:gateway_names(), emqx_dashboard_swagger:schema_with_examples( - hoconsc:union([ - ref(?MODULE, stomp), - ref(?MODULE, mqttsn), - ref(?MODULE, coap), - ref(?MODULE, lwm2m), - ref(?MODULE, exproto), - ref(?MODULE, update_stomp), - ref(?MODULE, update_mqttsn), - ref(?MODULE, update_coap), - ref(?MODULE, update_lwm2m), - ref(?MODULE, update_exproto), - ref(?MODULE, update_gbt32960) - ]), + hoconsc:union( + [ + ref(?MODULE, Name) + || Name <- + Names ++ + [ + erlang:list_to_existing_atom("update_" ++ erlang:atom_to_list(Name)) + || Name <- Names + ] + ] + ), examples_update_gateway_confs() ). schema_gateways_conf() -> emqx_dashboard_swagger:schema_with_examples( - hoconsc:union([ - ref(?MODULE, stomp), - ref(?MODULE, mqttsn), - ref(?MODULE, coap), - ref(?MODULE, lwm2m), - ref(?MODULE, exproto), - ref(?MODULE, gbt32960) - ]), + hoconsc:union( + [ + ref(?MODULE, Name) + || Name <- emqx_gateway_schema:gateway_names() + ] + ), examples_gateway_confs() ). diff --git a/apps/emqx_gateway/src/emqx_gateway_api_authn.erl b/apps/emqx_gateway/src/emqx_gateway_api_authn.erl index 539d65112..55672318a 100644 --- a/apps/emqx_gateway/src/emqx_gateway_api_authn.erl +++ b/apps/emqx_gateway/src/emqx_gateway_api_authn.erl @@ -327,7 +327,7 @@ params_gateway_name_in_path() -> [ {name, mk( - binary(), + hoconsc:enum(emqx_gateway_schema:gateway_names()), #{ in => path, desc => ?DESC(emqx_gateway_api, gateway_name_in_qs), diff --git a/apps/emqx_gateway/src/emqx_gateway_api_authn_user_import.erl b/apps/emqx_gateway/src/emqx_gateway_api_authn_user_import.erl index 30aeaf8fe..321b145ac 100644 --- a/apps/emqx_gateway/src/emqx_gateway_api_authn_user_import.erl +++ b/apps/emqx_gateway/src/emqx_gateway_api_authn_user_import.erl @@ -52,7 +52,7 @@ %%-------------------------------------------------------------------- api_spec() -> - emqx_dashboard_swagger:spec(?MODULE, #{check_schema => false}). + emqx_dashboard_swagger:spec(?MODULE, #{check_schema => true, translate_body => true}). paths() -> [ @@ -157,7 +157,7 @@ params_gateway_name_in_path() -> [ {name, mk( - binary(), + hoconsc:enum(emqx_gateway_schema:gateway_names()), #{ in => path, desc => ?DESC(emqx_gateway_api, gateway_name_in_qs), diff --git a/apps/emqx_gateway/src/emqx_gateway_api_clients.erl b/apps/emqx_gateway/src/emqx_gateway_api_clients.erl index 380ccfa6d..aedb4b0fa 100644 --- a/apps/emqx_gateway/src/emqx_gateway_api_clients.erl +++ b/apps/emqx_gateway/src/emqx_gateway_api_clients.erl @@ -700,7 +700,7 @@ params_gateway_name_in_path() -> [ {name, mk( - binary(), + hoconsc:enum(emqx_gateway_schema:gateway_names()), #{ in => path, desc => ?DESC(emqx_gateway_api, gateway_name) diff --git a/apps/emqx_gateway/src/emqx_gateway_api_listeners.erl b/apps/emqx_gateway/src/emqx_gateway_api_listeners.erl index 046e23300..284576983 100644 --- a/apps/emqx_gateway/src/emqx_gateway_api_listeners.erl +++ b/apps/emqx_gateway/src/emqx_gateway_api_listeners.erl @@ -609,7 +609,7 @@ params_gateway_name_in_path() -> [ {name, mk( - binary(), + hoconsc:enum(emqx_gateway_schema:gateway_names()), #{ in => path, desc => ?DESC(emqx_gateway_api, gateway_name_in_qs), diff --git a/apps/emqx_gateway/src/emqx_gateway_http.erl b/apps/emqx_gateway/src/emqx_gateway_http.erl index e8f0e034f..d1292c85b 100644 --- a/apps/emqx_gateway/src/emqx_gateway_http.erl +++ b/apps/emqx_gateway/src/emqx_gateway_http.erl @@ -513,29 +513,23 @@ codestr(501) -> 'NOT_IMPLEMENTED'. fmtstr(Fmt, Args) -> lists:flatten(io_lib:format(Fmt, Args)). --spec with_authn(binary(), function()) -> any(). +-spec with_authn(atom(), function()) -> any(). with_authn(GwName0, Fun) -> with_gateway(GwName0, fun(GwName, _GwConf) -> Authn = emqx_gateway_http:authn(GwName), Fun(GwName, Authn) end). --spec with_listener_authn(binary(), binary(), function()) -> any(). +-spec with_listener_authn(atom(), binary(), function()) -> any(). with_listener_authn(GwName0, Id, Fun) -> with_gateway(GwName0, fun(GwName, _GwConf) -> Authn = emqx_gateway_http:authn(GwName, Id), Fun(GwName, Authn) end). --spec with_gateway(binary(), function()) -> any(). -with_gateway(GwName0, Fun) -> +-spec with_gateway(atom(), function()) -> any(). +with_gateway(GwName, Fun) -> try - GwName = - try - binary_to_existing_atom(GwName0) - catch - _:_ -> error(badname) - end, case emqx_gateway:lookup(GwName) of undefined -> return_http_error(404, "Gateway not loaded"); diff --git a/apps/emqx_gateway/src/emqx_gateway_schema.erl b/apps/emqx_gateway/src/emqx_gateway_schema.erl index 2107d3133..c7c5dfd90 100644 --- a/apps/emqx_gateway/src/emqx_gateway_schema.erl +++ b/apps/emqx_gateway/src/emqx_gateway_schema.erl @@ -48,12 +48,13 @@ ip_port/0 ]). -elvis([{elvis_style, dont_repeat_yourself, disable}]). +-elvis([{elvis_style, invalid_dynamic_call, disable}]). -export([namespace/0, roots/0, fields/1, desc/1, tags/0]). -export([proxy_protocol_opts/0]). --export([mountpoint/0, mountpoint/1, gateway_common_options/0, gateway_schema/1]). +-export([mountpoint/0, mountpoint/1, gateway_common_options/0, gateway_schema/1, gateway_names/0]). namespace() -> gateway. @@ -337,13 +338,21 @@ proxy_protocol_opts() -> %% dynamic schemas %% FIXME: don't hardcode the gateway names -gateway_schema(stomp) -> emqx_stomp_schema:fields(stomp); -gateway_schema(mqttsn) -> emqx_mqttsn_schema:fields(mqttsn); -gateway_schema(coap) -> emqx_coap_schema:fields(coap); -gateway_schema(lwm2m) -> emqx_lwm2m_schema:fields(lwm2m); -gateway_schema(exproto) -> emqx_exproto_schema:fields(exproto); -gateway_schema(gbt32960) -> emqx_gbt32960_schema:fields(gbt32960). +gateway_schema(Name) -> + case emqx_gateway_utils:find_gateway_definition(Name) of + {ok, #{config_schema_module := SchemaMod}} -> + SchemaMod:fields(Name); + {error, _} = Error -> + throw(Error) + end. +gateway_names() -> + Definations = emqx_gateway_utils:find_gateway_definitions(), + [ + Name + || #{name := Name} = Defination <- Definations, + emqx_gateway_utils:check_gateway_edition(Defination) + ]. %%-------------------------------------------------------------------- %% helpers diff --git a/apps/emqx_gateway/src/emqx_gateway_utils.erl b/apps/emqx_gateway/src/emqx_gateway_utils.erl index 72751297b..8cc1396b4 100644 --- a/apps/emqx_gateway/src/emqx_gateway_utils.erl +++ b/apps/emqx_gateway/src/emqx_gateway_utils.erl @@ -45,8 +45,10 @@ global_chain/1, listener_chain/3, find_gateway_definitions/0, + find_gateway_definition/1, plus_max_connections/2, - random_clientid/1 + random_clientid/1, + check_gateway_edition/1 ]). -export([stringfy/1]). @@ -538,6 +540,32 @@ find_gateway_definitions() -> ) ). +-spec find_gateway_definition(atom()) -> {ok, map()} | {error, term()}. +find_gateway_definition(Name) -> + ensure_gateway_loaded(), + find_gateway_definition(Name, ignore_lib_apps(application:loaded_applications())). + +-dialyzer({no_match, [find_gateway_definition/2]}). +find_gateway_definition(Name, [App | T]) -> + Attrs = find_attrs(App, gateway), + SearchFun = fun({_App, _Mod, #{name := GwName}}) -> + GwName =:= Name + end, + case lists:search(SearchFun, Attrs) of + {value, {_App, _Mod, Defination}} -> + case check_gateway_edition(Defination) of + true -> + {ok, Defination}; + _ -> + {error, invalid_edition} + end; + false -> + find_gateway_definition(Name, T) + end; +find_gateway_definition(_Name, []) -> + {error, not_found}. + +-dialyzer({no_match, [gateways/1]}). gateways([]) -> []; gateways([ @@ -550,7 +578,20 @@ gateways([ }} | More ]) when is_atom(Name), is_atom(CbMod), is_atom(SchemaMod) -> - [Defination | gateways(More)]. + case check_gateway_edition(Defination) of + true -> + [Defination | gateways(More)]; + _ -> + gateways(More) + end. + +-if(?EMQX_RELEASE_EDITION == ee). +check_gateway_edition(_Defination) -> + true. +-else. +check_gateway_edition(Defination) -> + ce == maps:get(edition, Defination, ce). +-endif. find_attrs(App, Def) -> [ diff --git a/apps/emqx_gateway/test/emqx_gateway_api_SUITE.erl b/apps/emqx_gateway/test/emqx_gateway_api_SUITE.erl index 9cda5bc23..0b562e851 100644 --- a/apps/emqx_gateway/test/emqx_gateway_api_SUITE.erl +++ b/apps/emqx_gateway/test/emqx_gateway_api_SUITE.erl @@ -96,10 +96,8 @@ t_gateways(_) -> ok. t_gateway(_) -> - {404, GwNotFoundReq1} = request(get, "/gateways/not_a_known_atom"), - assert_not_found(GwNotFoundReq1), - {404, GwNotFoundReq2} = request(get, "/gateways/undefined"), - assert_not_found(GwNotFoundReq2), + ?assertMatch({400, #{code := <<"BAD_REQUEST">>}}, request(get, "/gateways/not_a_known_atom")), + ?assertMatch({400, #{code := <<"BAD_REQUEST">>}}, request(get, "/gateways/undefined")), {204, _} = request(put, "/gateways/stomp", #{}), {200, StompGw} = request(get, "/gateways/stomp"), assert_fields_exist( @@ -110,7 +108,7 @@ t_gateway(_) -> {200, #{enable := true}} = request(get, "/gateways/stomp"), {204, _} = request(put, "/gateways/stomp", #{enable => false}), {200, #{enable := false}} = request(get, "/gateways/stomp"), - {404, _} = request(put, "/gateways/undefined", #{}), + ?assertMatch({400, #{code := <<"BAD_REQUEST">>}}, request(put, "/gateways/undefined", #{})), {400, _} = request(put, "/gateways/stomp", #{bad_key => "foo"}), ok. @@ -129,8 +127,14 @@ t_gateway_enable(_) -> {200, #{enable := NotEnable}} = request(get, "/gateways/stomp"), {204, _} = request(put, "/gateways/stomp/enable/" ++ atom_to_list(Enable), undefined), {200, #{enable := Enable}} = request(get, "/gateways/stomp"), - {404, _} = request(put, "/gateways/undefined/enable/true", undefined), - {404, _} = request(put, "/gateways/not_a_known_atom/enable/true", undefined), + ?assertMatch( + {400, #{code := <<"BAD_REQUEST">>}}, + request(put, "/gateways/undefined/enable/true", undefined) + ), + ?assertMatch( + {400, #{code := <<"BAD_REQUEST">>}}, + request(put, "/gateways/not_a_known_atom/enable/true", undefined) + ), {404, _} = request(put, "/gateways/coap/enable/true", undefined), ok. diff --git a/apps/emqx_gateway_gbt32960/src/emqx_gateway_gbt32960.erl b/apps/emqx_gateway_gbt32960/src/emqx_gateway_gbt32960.erl index dde54026d..e4bcd4969 100644 --- a/apps/emqx_gateway_gbt32960/src/emqx_gateway_gbt32960.erl +++ b/apps/emqx_gateway_gbt32960/src/emqx_gateway_gbt32960.erl @@ -12,7 +12,8 @@ -gateway(#{ name => gbt32960, callback_module => ?MODULE, - config_schema_module => emqx_gbt32960_schema + config_schema_module => emqx_gbt32960_schema, + edition => ee }). %% callback_module must implement the emqx_gateway_impl behaviour diff --git a/rel/i18n/emqx_gateway_api.hocon b/rel/i18n/emqx_gateway_api.hocon index d712a054d..c8017e110 100644 --- a/rel/i18n/emqx_gateway_api.hocon +++ b/rel/i18n/emqx_gateway_api.hocon @@ -37,8 +37,7 @@ gateway_name.desc: """Gateway Name""" gateway_name_in_qs.desc: -"""Gateway Name.
-It's enum with `stomp`, `mqttsn`, `coap`, `lwm2m`, `exproto`, `gbt32960`""" +"""Gateway Name""" gateway_node_status.desc: """The status of the gateway on each node in the cluster"""