diff --git a/apps/emqx/include/emqx_authentication.hrl b/apps/emqx/include/emqx_authentication.hrl index 5f0959094..dc501d1d8 100644 --- a/apps/emqx/include/emqx_authentication.hrl +++ b/apps/emqx/include/emqx_authentication.hrl @@ -28,4 +28,10 @@ %% and emqx_conf_schema for an examples -define(EMQX_AUTHENTICATION_SCHEMA_MODULE_PT_KEY, emqx_authentication_schema_module). +%% authentication move cmd +-define(CMD_MOVE_FRONT, front). +-define(CMD_MOVE_REAR, rear). +-define(CMD_MOVE_BEFORE(Before), {before, Before}). +-define(CMD_MOVE_AFTER(After), {'after', After}). + -endif. diff --git a/apps/emqx/src/emqx_authentication.erl b/apps/emqx/src/emqx_authentication.erl index 8312b312f..5df50a427 100644 --- a/apps/emqx/src/emqx_authentication.erl +++ b/apps/emqx/src/emqx_authentication.erl @@ -100,7 +100,7 @@ -type chain_name() :: atom(). -type authenticator_id() :: binary(). --type position() :: top | bottom | {before, authenticator_id()}. +-type position() :: front | rear | {before, authenticator_id()} | {'after', authenticator_id()}. -type authn_type() :: atom() | {atom(), atom()}. -type provider() :: module(). @@ -695,21 +695,29 @@ do_move_authenticator(ID, Authenticators, Position) -> {error, {not_found, {authenticator, ID}}}; {value, Authenticator, NAuthenticators} -> case Position of - top -> + ?CMD_MOVE_FRONT -> {ok, [Authenticator | NAuthenticators]}; - bottom -> + ?CMD_MOVE_REAR -> {ok, NAuthenticators ++ [Authenticator]}; - {before, ID0} -> - insert(Authenticator, NAuthenticators, ID0, []) + ?CMD_MOVE_BEFORE(RelatedID) -> + insert(Authenticator, NAuthenticators, ?CMD_MOVE_BEFORE(RelatedID), []); + ?CMD_MOVE_AFTER(RelatedID) -> + insert(Authenticator, NAuthenticators, ?CMD_MOVE_AFTER(RelatedID), []) end end. -insert(_, [], ID, _) -> - {error, {not_found, {authenticator, ID}}}; -insert(Authenticator, [#authenticator{id = ID} | _] = Authenticators, ID, Acc) -> - {ok, lists:reverse(Acc) ++ [Authenticator | Authenticators]}; -insert(Authenticator, [Authenticator0 | More], ID, Acc) -> - insert(Authenticator, More, ID, [Authenticator0 | Acc]). +insert(_, [], {_, RelatedID}, _) -> + {error, {not_found, {authenticator, RelatedID}}}; +insert(Authenticator, [#authenticator{id = RelatedID} = Related | Rest], + {Relative, RelatedID}, Acc) -> + case Relative of + before -> + {ok, lists:reverse(Acc) ++ [Authenticator, Related | Rest]}; + 'after' -> + {ok, lists:reverse(Acc) ++ [Related, Authenticator | Rest]} + end; +insert(Authenticator, [Authenticator0 | More], {Relative, RelatedID}, Acc) -> + insert(Authenticator, More, {Relative, RelatedID}, [Authenticator0 | Acc]). update_chain(ChainName, UpdateFun) -> case ets:lookup(?CHAINS_TAB, ChainName) of diff --git a/apps/emqx/src/emqx_authentication_config.erl b/apps/emqx/src/emqx_authentication_config.erl index 31c6ad8cd..66f96fc76 100644 --- a/apps/emqx/src/emqx_authentication_config.erl +++ b/apps/emqx/src/emqx_authentication_config.erl @@ -87,24 +87,36 @@ do_pre_config_update({update_authenticator, ChainName, AuthenticatorID, Config}, do_pre_config_update({move_authenticator, _ChainName, AuthenticatorID, Position}, OldConfig) -> case split_by_id(AuthenticatorID, OldConfig) of {error, Reason} -> {error, Reason}; - {ok, Part1, [Found | Part2]} -> + {ok, BeforeFound, [Found | AfterFound]} -> case Position of - top -> - {ok, [Found | Part1] ++ Part2}; - bottom -> - {ok, Part1 ++ Part2 ++ [Found]}; - {before, Before} -> - case split_by_id(Before, Part1 ++ Part2) of + ?CMD_MOVE_FRONT -> + {ok, [Found | BeforeFound] ++ AfterFound}; + ?CMD_MOVE_REAR -> + {ok, BeforeFound ++ AfterFound ++ [Found]}; + ?CMD_MOVE_BEFORE(BeforeRelatedID) -> + case split_by_id(BeforeRelatedID, BeforeFound ++ AfterFound) of {error, Reason} -> {error, Reason}; - {ok, NPart1, [NFound | NPart2]} -> - {ok, NPart1 ++ [Found, NFound | NPart2]} + {ok, BeforeNFound, [FoundRelated | AfterNFound]} -> + {ok, BeforeNFound ++ [Found, FoundRelated | AfterNFound]} + end; + ?CMD_MOVE_AFTER(AfterRelatedID) -> + case split_by_id(AfterRelatedID, BeforeFound ++ AfterFound) of + {error, Reason} -> + {error, Reason}; + {ok, BeforeNFound, [FoundRelated | AfterNFound]} -> + {ok, BeforeNFound ++ [FoundRelated, Found | AfterNFound]} end end end. --spec post_config_update(list(atom()), update_request(), map() | list(), emqx_config:raw_config(), emqx_config:app_envs()) - -> ok | {ok, map()} | {error, term()}. +-spec post_config_update(list(atom()), + update_request(), + map() | list(), + emqx_config:raw_config(), + emqx_config:app_envs() + ) + -> ok | {ok, map()} | {error, term()}. post_config_update(_, UpdateReq, NewConfig, OldConfig, AppEnvs) -> do_post_config_update(UpdateReq, check_configs(to_list(NewConfig)), OldConfig, AppEnvs). @@ -112,7 +124,8 @@ do_post_config_update({create_authenticator, ChainName, Config}, NewConfig, _Old NConfig = get_authenticator_config(authenticator_id(Config), NewConfig), _ = emqx_authentication:create_chain(ChainName), emqx_authentication:create_authenticator(ChainName, NConfig); -do_post_config_update({delete_authenticator, ChainName, AuthenticatorID}, _NewConfig, OldConfig, _AppEnvs) -> +do_post_config_update({delete_authenticator, ChainName, AuthenticatorID}, + _NewConfig, OldConfig, _AppEnvs) -> case emqx_authentication:delete_authenticator(ChainName, AuthenticatorID) of ok -> Config = get_authenticator_config(AuthenticatorID, to_list(OldConfig)), @@ -121,14 +134,16 @@ do_post_config_update({delete_authenticator, ChainName, AuthenticatorID}, _NewCo {error, Reason} -> {error, Reason} end; -do_post_config_update({update_authenticator, ChainName, AuthenticatorID, Config}, NewConfig, _OldConfig, _AppEnvs) -> +do_post_config_update({update_authenticator, ChainName, AuthenticatorID, Config}, + NewConfig, _OldConfig, _AppEnvs) -> case get_authenticator_config(authenticator_id(Config), NewConfig) of {error, not_found} -> {error, {not_found, {authenticator, AuthenticatorID}}}; NConfig -> emqx_authentication:update_authenticator(ChainName, AuthenticatorID, NConfig) end; -do_post_config_update({move_authenticator, ChainName, AuthenticatorID, Position}, _NewConfig, _OldConfig, _AppEnvs) -> +do_post_config_update({move_authenticator, ChainName, AuthenticatorID, Position}, + _NewConfig, _OldConfig, _AppEnvs) -> emqx_authentication:move_authenticator(ChainName, AuthenticatorID, Position). check_configs(Configs) -> diff --git a/apps/emqx/test/emqx_authentication_SUITE.erl b/apps/emqx/test/emqx_authentication_SUITE.erl index 7c3a7fecd..1a3ebb5e4 100644 --- a/apps/emqx/test/emqx_authentication_SUITE.erl +++ b/apps/emqx/test/emqx_authentication_SUITE.erl @@ -185,14 +185,17 @@ t_authenticator(Config) when is_list(Config) -> % Move authenticator ?assertMatch({ok, [#{id := ID1}, #{id := ID2}]}, ?AUTHN:list_authenticators(ChainName)), - ?assertEqual(ok, ?AUTHN:move_authenticator(ChainName, ID2, top)), + ?assertEqual(ok, ?AUTHN:move_authenticator(ChainName, ID2, ?CMD_MOVE_FRONT)), ?assertMatch({ok, [#{id := ID2}, #{id := ID1}]}, ?AUTHN:list_authenticators(ChainName)), - ?assertEqual(ok, ?AUTHN:move_authenticator(ChainName, ID2, bottom)), + ?assertEqual(ok, ?AUTHN:move_authenticator(ChainName, ID2, ?CMD_MOVE_REAR)), ?assertMatch({ok, [#{id := ID1}, #{id := ID2}]}, ?AUTHN:list_authenticators(ChainName)), - ?assertEqual(ok, ?AUTHN:move_authenticator(ChainName, ID2, {before, ID1})), - ?assertMatch({ok, [#{id := ID2}, #{id := ID1}]}, ?AUTHN:list_authenticators(ChainName)); + ?assertEqual(ok, ?AUTHN:move_authenticator(ChainName, ID2, ?CMD_MOVE_BEFORE(ID1))), + ?assertMatch({ok, [#{id := ID2}, #{id := ID1}]}, ?AUTHN:list_authenticators(ChainName)), + + ?assertEqual(ok, ?AUTHN:move_authenticator(ChainName, ID2, ?CMD_MOVE_AFTER(ID1))), + ?assertMatch({ok, [#{id := ID1}, #{id := ID2}]}, ?AUTHN:list_authenticators(ChainName)); t_authenticator({'end', Config}) -> ?AUTHN:delete_chain(test), @@ -211,7 +214,7 @@ t_authenticate(Config) when is_list(Config) -> listener => ListenerID, protocol => mqtt, username => <<"good">>, - password => <<"any">>}, + password => <<"any">>}, ?assertEqual({ok, #{is_superuser => false}}, emqx_access_control:authenticate(ClientInfo)), register_provider(AuthNType, ?MODULE), @@ -291,7 +294,7 @@ t_update_config(Config) when is_list(Config) -> ?assertMatch( {ok, _}, - update_config([?CONF_ROOT], {move_authenticator, Global, ID2, top})), + update_config([?CONF_ROOT], {move_authenticator, Global, ID2, ?CMD_MOVE_FRONT})), ?assertMatch({ok, [#{id := ID2}, #{id := ID1}]}, ?AUTHN:list_authenticators(Global)), @@ -344,7 +347,7 @@ t_update_config(Config) when is_list(Config) -> ?assertMatch( {ok, _}, - update_config(ConfKeyPath, {move_authenticator, ListenerID, ID2, top})), + update_config(ConfKeyPath, {move_authenticator, ListenerID, ID2, ?CMD_MOVE_FRONT})), ?assertMatch( {ok, [#{id := ID2}, #{id := ID1}]}, diff --git a/apps/emqx_authn/src/emqx_authn_api.erl b/apps/emqx_authn/src/emqx_authn_api.erl index 24de25dc8..2266abfb2 100644 --- a/apps/emqx_authn/src/emqx_authn_api.erl +++ b/apps/emqx_authn/src/emqx_authn_api.erl @@ -1059,12 +1059,18 @@ serialize_error(Reason) -> {400, #{code => <<"BAD_REQUEST">>, message => binfmt("~p", [Reason])}}. -parse_position(<<"top">>) -> - {ok, top}; -parse_position(<<"bottom">>) -> - {ok, bottom}; +parse_position(<<"front">>) -> + {ok, ?CMD_MOVE_FRONT}; +parse_position(<<"rear">>) -> + {ok, ?CMD_MOVE_REAR}; +parse_position(<<"before:">>) -> + {error, {invalid_parameter, position}}; +parse_position(<<"after:">>) -> + {error, {invalid_parameter, position}}; parse_position(<<"before:", Before/binary>>) -> - {ok, {before, Before}}; + {ok, ?CMD_MOVE_BEFORE(Before)}; +parse_position(<<"after:", After/binary>>) -> + {ok, ?CMD_MOVE_AFTER(After)}; parse_position(_) -> {error, {invalid_parameter, position}}. @@ -1199,16 +1205,16 @@ request_user_update_examples() -> request_move_examples() -> #{ - move_to_top => #{ + move_to_front => #{ summary => <<"Move authenticator to the beginning of the chain">>, value => #{ - position => <<"top">> + position => <<"front">> } }, - move_to_bottom => #{ + move_to_rear => #{ summary => <<"Move authenticator to the end of the chain">>, value => #{ - position => <<"bottom">> + position => <<"rear">> } }, 'move_before_password_based:built_in_database' => #{ diff --git a/apps/emqx_authn/test/emqx_authn_api_SUITE.erl b/apps/emqx_authn/test/emqx_authn_api_SUITE.erl index b0aced062..a707dfeb4 100644 --- a/apps/emqx_authn/test/emqx_authn_api_SUITE.erl +++ b/apps/emqx_authn/test/emqx_authn_api_SUITE.erl @@ -347,7 +347,7 @@ test_authenticator_move(PathPrefix) -> ], PathPrefix ++ [?CONF_NS]), - % Invalid moves + %% Invalid moves {ok, 400, _} = request( post, @@ -374,12 +374,13 @@ test_authenticator_move(PathPrefix) -> uri(PathPrefix ++ [?CONF_NS, "jwt", "move"]), #{position => <<"before:password_based:redis">>}), - % Valid moves + %% Valid moves + %% test front {ok, 204, _} = request( post, uri(PathPrefix ++ [?CONF_NS, "jwt", "move"]), - #{position => <<"top">>}), + #{position => <<"front">>}), ?assertAuthenticatorsMatch( [ @@ -389,10 +390,11 @@ test_authenticator_move(PathPrefix) -> ], PathPrefix ++ [?CONF_NS]), + %% test rear {ok, 204, _} = request( post, uri(PathPrefix ++ [?CONF_NS, "jwt", "move"]), - #{position => <<"bottom">>}), + #{position => <<"rear">>}), ?assertAuthenticatorsMatch( [ @@ -402,6 +404,7 @@ test_authenticator_move(PathPrefix) -> ], PathPrefix ++ [?CONF_NS]), + %% test before {ok, 204, _} = request( post, uri(PathPrefix ++ [?CONF_NS, "jwt", "move"]), @@ -413,6 +416,20 @@ test_authenticator_move(PathPrefix) -> #{<<"mechanism">> := <<"jwt">>}, #{<<"mechanism">> := <<"password_based">>, <<"backend">> := <<"built_in_database">>} ], + PathPrefix ++ [?CONF_NS]), + + %% test after + {ok, 204, _} = request( + post, + uri(PathPrefix ++ [?CONF_NS, "password_based%3Abuilt_in_database", "move"]), + #{position => <<"after:password_based:http">>}), + + ?assertAuthenticatorsMatch( + [ + #{<<"mechanism">> := <<"password_based">>, <<"backend">> := <<"http">>}, + #{<<"mechanism">> := <<"password_based">>, <<"backend">> := <<"built_in_database">>}, + #{<<"mechanism">> := <<"jwt">>} + ], PathPrefix ++ [?CONF_NS]). test_authenticator_import_users(PathPrefix) -> diff --git a/apps/emqx_authz/include/emqx_authz.hrl b/apps/emqx_authz/include/emqx_authz.hrl index 7b584b59b..13c100ba8 100644 --- a/apps/emqx_authz/include/emqx_authz.hrl +++ b/apps/emqx_authz/include/emqx_authz.hrl @@ -34,10 +34,10 @@ -define(CMD_APPEND, append). -define(CMD_MOVE, move). --define(CMD_MOVE_TOP, <<"top">>). --define(CMD_MOVE_BOTTOM, <<"bottom">>). --define(CMD_MOVE_BEFORE(Before), {<<"before">>, Before}). --define(CMD_MOVE_AFTER(After), {<<"after">>, After}). +-define(CMD_MOVE_FRONT, front). +-define(CMD_MOVE_REAR, rear). +-define(CMD_MOVE_BEFORE(Before), {before, Before}). +-define(CMD_MOVE_AFTER(After), {'after', After}). -define(CONF_KEY_PATH, [authorization, sources]). diff --git a/apps/emqx_authz/src/emqx_authz.erl b/apps/emqx_authz/src/emqx_authz.erl index a51e78719..b270176ec 100644 --- a/apps/emqx_authz/src/emqx_authz.erl +++ b/apps/emqx_authz/src/emqx_authz.erl @@ -110,10 +110,10 @@ lookup(Type) -> {Source, _Front, _Rear} = take(Type), Source. -move(Type, #{<<"before">> := Before}) -> +move(Type, {before, Before}) -> emqx_authz_utils:update_config( ?CONF_KEY_PATH, {?CMD_MOVE, type(Type), ?CMD_MOVE_BEFORE(type(Before))}); -move(Type, #{<<"after">> := After}) -> +move(Type, {'after', After}) -> emqx_authz_utils:update_config( ?CONF_KEY_PATH, {?CMD_MOVE, type(Type), ?CMD_MOVE_AFTER(type(After))}); move(Type, Position) -> @@ -127,10 +127,10 @@ update({?CMD_DELETE, Type}, Sources) -> update(Cmd, Sources) -> emqx_authz_utils:update_config(?CONF_KEY_PATH, {Cmd, Sources}). -do_update({?CMD_MOVE, Type, ?CMD_MOVE_TOP}, Conf) when is_list(Conf) -> +do_update({?CMD_MOVE, Type, ?CMD_MOVE_FRONT}, Conf) when is_list(Conf) -> {Source, Front, Rear} = take(Type, Conf), [Source | Front] ++ Rear; -do_update({?CMD_MOVE, Type, ?CMD_MOVE_BOTTOM}, Conf) when is_list(Conf) -> +do_update({?CMD_MOVE, Type, ?CMD_MOVE_REAR}, Conf) when is_list(Conf) -> {Source, Front, Rear} = take(Type, Conf), Front ++ Rear ++ [Source]; do_update({?CMD_MOVE, Type, ?CMD_MOVE_BEFORE(Before)}, Conf) when is_list(Conf) -> @@ -334,7 +334,7 @@ take(Type, Sources) -> {Front, Rear} = lists:splitwith(fun(T) -> type(T) =/= type(Type) end, Sources), case Rear =:= [] of true -> - error({authz_source_of_type_not_found, Type}); + error({not_found_source, Type}); _ -> {hd(Rear), Front, tl(Rear)} end. diff --git a/apps/emqx_authz/src/emqx_authz_api_schema.erl b/apps/emqx_authz/src/emqx_authz_api_schema.erl index 8cfcf3033..882d1581f 100644 --- a/apps/emqx_authz/src/emqx_authz_api_schema.erl +++ b/apps/emqx_authz/src/emqx_authz_api_schema.erl @@ -74,11 +74,10 @@ fields(file) -> , example => <<"acl.conf">>}}]; fields(position) -> [ { position - , mk( hoconsc:union([binary(), map()]) + , mk( string() , #{ desc => <<"Where to place the source">> , required => true - , in => body - , example => #{<<"before">> => <<"file">>}})}]. + , in => body})}]. %%------------------------------------------------------------------------------ %% http type funcs diff --git a/apps/emqx_authz/src/emqx_authz_api_sources.erl b/apps/emqx_authz/src/emqx_authz_api_sources.erl index 731c5d40f..58eed9982 100644 --- a/apps/emqx_authz/src/emqx_authz_api_sources.erl +++ b/apps/emqx_authz/src/emqx_authz_api_sources.erl @@ -262,14 +262,25 @@ move_source(Method, #{bindings := #{type := Type} = Bindings } = Req) when is_atom(Type) -> move_source(Method, Req#{bindings => Bindings#{type => atom_to_binary(Type, utf8)}}); move_source(post, #{bindings := #{type := Type}, body := #{<<"position">> := Position}}) -> - case emqx_authz:move(Type, Position) of - {ok, _} -> {204}; - {error, not_found_source} -> - {404, #{code => <<"NOT_FOUND">>, - message => <<"source ", Type/binary, " not found">>}}; - {error, {emqx_conf_schema, _}} -> - {400, #{code => <<"BAD_REQUEST">>, - message => <<"BAD_SCHEMA">>}}; + case parse_position(Position) of + {ok, NPosition} -> + try emqx_authz:move(Type, NPosition) of + {ok, _} -> {204}; + {error, {not_found_source, _Type}} -> + {404, #{code => <<"NOT_FOUND">>, + message => <<"source ", Type/binary, " not found">>}}; + {error, {emqx_conf_schema, _}} -> + {400, #{code => <<"BAD_REQUEST">>, + message => <<"BAD_SCHEMA">>}}; + {error, Reason} -> + {400, #{code => <<"BAD_REQUEST">>, + message => bin(Reason)}} + catch + error : {unknown_authz_source_type, Unknown} -> + NUnknown = bin(Unknown), + {400, #{code => <<"BAD_REQUEST">>, + message => <<"Unknown authz Source Type: ", NUnknown/binary>>}} + end; {error, Reason} -> {400, #{code => <<"BAD_REQUEST">>, message => bin(Reason)}} @@ -457,8 +468,6 @@ do_write_file(Filename, Bytes) -> error(Reason) end. -bin(Term) -> erlang:iolist_to_binary(io_lib:format("~p", [Term])). - acl_conf_file() -> emqx_authz:acl_conf_file(). @@ -468,8 +477,37 @@ parameters_field() -> } ]. +parse_position(<<"front">>) -> + {ok, ?CMD_MOVE_FRONT}; +parse_position(<<"rear">>) -> + {ok, ?CMD_MOVE_REAR}; +parse_position(<<"before:", Before/binary>>) -> + {ok, ?CMD_MOVE_BEFORE(Before)}; +parse_position(<<"after:", After/binary>>) -> + {ok, ?CMD_MOVE_AFTER(After)}; +parse_position(<<"before:">>) -> + {error, {invalid_parameter, position}}; +parse_position(<<"after:">>) -> + {error, {invalid_parameter, position}}; +parse_position(_) -> + {error, {invalid_parameter, position}}. + position_example() -> - #{<<"position">> => #{<<"before">> => <<"file">>}}. + #{ front => + #{ summary => <<"front example">> + , value => #{<<"position">> => <<"front">>}} + , rear => + #{ summary => <<"rear example">> + , value => #{<<"position">> => <<"rear">>}} + , relative_before => + #{ summary => <<"relative example">> + , value => #{<<"position">> => <<"before:file">>}} + , relative_after => + #{ summary => <<"relative example">> + , value => #{<<"position">> => <<"after:file">>}} + }. authz_sources_types(Type) -> emqx_authz_api_schema:authz_sources_types(Type). + +bin(Term) -> erlang:iolist_to_binary(io_lib:format("~p", [Term])). diff --git a/apps/emqx_authz/test/emqx_authz_SUITE.erl b/apps/emqx_authz/test/emqx_authz_SUITE.erl index e62be0aa2..0e73e44cf 100644 --- a/apps/emqx_authz/test/emqx_authz_SUITE.erl +++ b/apps/emqx_authz/test/emqx_authz_SUITE.erl @@ -186,7 +186,7 @@ t_move_source(_) -> , #{type := file} ], emqx_authz:lookup()), - {ok, _} = emqx_authz:move(postgresql, <<"top">>), + {ok, _} = emqx_authz:move(postgresql, ?CMD_MOVE_FRONT), ?assertMatch([ #{type := postgresql} , #{type := http} , #{type := mongodb} @@ -195,7 +195,7 @@ t_move_source(_) -> , #{type := file} ], emqx_authz:lookup()), - {ok, _} = emqx_authz:move(http, <<"bottom">>), + {ok, _} = emqx_authz:move(http, ?CMD_MOVE_REAR), ?assertMatch([ #{type := postgresql} , #{type := mongodb} , #{type := mysql} @@ -204,7 +204,7 @@ t_move_source(_) -> , #{type := http} ], emqx_authz:lookup()), - {ok, _} = emqx_authz:move(mysql, #{<<"before">> => postgresql}), + {ok, _} = emqx_authz:move(mysql, ?CMD_MOVE_BEFORE(postgresql)), ?assertMatch([ #{type := mysql} , #{type := postgresql} , #{type := mongodb} @@ -213,7 +213,7 @@ t_move_source(_) -> , #{type := http} ], emqx_authz:lookup()), - {ok, _} = emqx_authz:move(mongodb, #{<<"after">> => http}), + {ok, _} = emqx_authz:move(mongodb, ?CMD_MOVE_AFTER(http)), ?assertMatch([ #{type := mysql} , #{type := postgresql} , #{type := redis} diff --git a/apps/emqx_authz/test/emqx_authz_api_sources_SUITE.erl b/apps/emqx_authz/test/emqx_authz_api_sources_SUITE.erl index 608090640..23daf1454 100644 --- a/apps/emqx_authz/test/emqx_authz_api_sources_SUITE.erl +++ b/apps/emqx_authz/test/emqx_authz_api_sources_SUITE.erl @@ -318,7 +318,7 @@ t_move_source(_) -> ], emqx_authz:lookup()), {ok, 204, _} = request(post, uri(["authorization", "sources", "postgresql", "move"]), - #{<<"position">> => <<"top">>}), + #{<<"position">> => <<"front">>}), ?assertMatch([ #{type := postgresql} , #{type := http} , #{type := mongodb} @@ -327,7 +327,7 @@ t_move_source(_) -> ], emqx_authz:lookup()), {ok, 204, _} = request(post, uri(["authorization", "sources", "http", "move"]), - #{<<"position">> => <<"bottom">>}), + #{<<"position">> => <<"rear">>}), ?assertMatch([ #{type := postgresql} , #{type := mongodb} , #{type := mysql} @@ -336,7 +336,7 @@ t_move_source(_) -> ], emqx_authz:lookup()), {ok, 204, _} = request(post, uri(["authorization", "sources", "mysql", "move"]), - #{<<"position">> => #{<<"before">> => <<"postgresql">>}}), + #{<<"position">> => <<"before:postgresql">>}), ?assertMatch([ #{type := mysql} , #{type := postgresql} , #{type := mongodb} @@ -345,7 +345,7 @@ t_move_source(_) -> ], emqx_authz:lookup()), {ok, 204, _} = request(post, uri(["authorization", "sources", "mongodb", "move"]), - #{<<"position">> => #{<<"after">> => <<"http">>}}), + #{<<"position">> => <<"after:http">>}), ?assertMatch([ #{type := mysql} , #{type := postgresql} , #{type := redis} diff --git a/apps/emqx_exhook/include/emqx_exhook.hrl b/apps/emqx_exhook/include/emqx_exhook.hrl index e478c8cc7..466146212 100644 --- a/apps/emqx_exhook/include/emqx_exhook.hrl +++ b/apps/emqx_exhook/include/emqx_exhook.hrl @@ -45,3 +45,8 @@ ]). -endif. + +-define(CMD_MOVE_FRONT, front). +-define(CMD_MOVE_REAR, rear). +-define(CMD_MOVE_BEFORE(Before), {before, Before}). +-define(CMD_MOVE_AFTER(After), {'after', After}). diff --git a/apps/emqx_exhook/src/emqx_exhook_api.erl b/apps/emqx_exhook/src/emqx_exhook_api.erl index 52c16b39e..f2e51dc0f 100644 --- a/apps/emqx_exhook/src/emqx_exhook_api.erl +++ b/apps/emqx_exhook/src/emqx_exhook_api.erl @@ -18,6 +18,7 @@ -behaviour(minirest_api). +-include("emqx_exhook.hrl"). -include_lib("typerefl/include/types.hrl"). -include_lib("emqx/include/logger.hrl"). @@ -104,10 +105,14 @@ schema("/exhooks/:name/hooks") -> schema("/exhooks/:name/move") -> #{'operationId' => move, post => #{tags => ?TAGS, - description => <<"Move the server">>, + description => + <<"Move the server.\n", + "NOTE: The position should be \"front|rear|before:{name}|after:{name}\"\n">>, parameters => params_server_name_in_path(), - 'requestBody' => mk(ref(move_req), #{}), - responses => #{200 => <<>>, + 'requestBody' => emqx_dashboard_swagger:schema_with_examples( + ref(move_req), + position_example()), + responses => #{204 => <<"No Content">>, 400 => error_codes([?BAD_REQUEST], <<"Bad Request">>), 500 => error_codes([?BAD_RPC], <<"Bad RPC">>) } @@ -115,12 +120,8 @@ schema("/exhooks/:name/move") -> }. fields(move_req) -> - [ {position, mk(enum([top, bottom, before, 'after']), #{})} - , {related, mk(string(), #{desc => <<"Relative position of movement">>, - default => <<>>, - example => <<>> - })} - ]; + [{position, mk(string(), #{ desc => <<"The target position to be moved.">> + , example => <<"front">>})}]; fields(detail_server_info) -> [ {metrics, mk(ref(metrics), #{})} @@ -210,7 +211,8 @@ action_with_name(put, #{bindings := #{name := Name}, body := Body}) -> }}; {ok, {error, Reason}} -> {400, #{code => <<"BAD_REQUEST">>, - message => unicode:characters_to_binary(io_lib:format("Error Reason:~p~n", [Reason])) + message => unicode:characters_to_binary( + io_lib:format("Error Reason:~p~n", [Reason])) }}; {ok, _} -> {200}; @@ -231,20 +233,26 @@ action_with_name(delete, #{bindings := #{name := Name}}) -> }} end. -move(post, #{bindings := #{name := Name}, body := Body}) -> - #{<<"position">> := PositionT, <<"related">> := Related} = Body, - Position = erlang:binary_to_atom(PositionT), - case emqx_exhook_mgr:update_config([exhook, servers], - {move, Name, Position, Related}) of - {ok, ok} -> - {200}; - {ok, not_found} -> +move(post, #{bindings := #{name := Name}, body := #{<<"position">> := RawPosition}}) -> + case parse_position(RawPosition) of + {ok, Position} -> + case emqx_exhook_mgr:update_config([exhook, servers], + {move, Name, Position}) of + {ok, ok} -> + {204}; + {ok, not_found} -> + %% TODO: unify status code + {400, #{code => <<"BAD_REQUEST">>, + message => <<"Server not found">> + }}; + {error, Error} -> + {500, #{code => <<"BAD_RPC">>, + message => Error + }} + end; + {error, invalid_position} -> {400, #{code => <<"BAD_REQUEST">>, - message => <<"Server not found">> - }}; - {error, Error} -> - {500, #{code => <<"BAD_RPC">>, - message => Error + message => <<"Invalid Position">> }} end. @@ -297,11 +305,12 @@ fill_cluster_server_info([{Node, {error, _}} | T], StatusL, MetricsL, ServerName fill_cluster_server_info([{Node, Result} | T], StatusL, MetricsL, ServerName, Default) -> #{status := Status, metrics := Metrics} = Result, - fill_cluster_server_info(T, - [#{node => Node, status => maps:get(ServerName, Status, error)} | StatusL], - [#{node => Node, metrics => maps:get(ServerName, Metrics, Default)} | MetricsL], - ServerName, - Default); + fill_cluster_server_info( + T, + [#{node => Node, status => maps:get(ServerName, Status, error)} | StatusL], + [#{node => Node, metrics => maps:get(ServerName, Metrics, Default)} | MetricsL], + ServerName, + Default); fill_cluster_server_info([], StatusL, MetricsL, ServerName, _) -> Metrics = emqx_exhook_metrics:metrics_aggregate_by_key(metrics, MetricsL), @@ -350,7 +359,9 @@ get_nodes_server_hooks_info(Name) -> case emqx_exhook_mgr:hooks(Name) of [] -> []; Hooks -> - AllInfos = call_cluster(fun(Nodes) -> emqx_exhook_proto_v1:server_hooks_metrics(Nodes, Name) end), + AllInfos = call_cluster(fun(Nodes) -> + emqx_exhook_proto_v1:server_hooks_metrics(Nodes, Name) + end), Default = emqx_exhook_metrics:new_metrics_info(), get_nodes_server_hooks_info(Hooks, AllInfos, Default, []) end. @@ -385,3 +396,38 @@ call_cluster(Fun) -> Nodes = mria_mnesia:running_nodes(), Ret = Fun(Nodes), lists:zip(Nodes, lists:map(fun emqx_rpc:unwrap_erpc/1, Ret)). + + +%%-------------------------------------------------------------------- +%% Internal Funcs +%%-------------------------------------------------------------------- + +position_example() -> + #{ front => + #{ summary => <<"absolute position 'front'">> + , value => #{<<"position">> => <<"front">>}} + , rear => + #{ summary => <<"absolute position 'rear'">> + , value => #{<<"position">> => <<"rear">>}} + , related_before => + #{ summary => <<"relative position 'before'">> + , value => #{<<"position">> => <<"before:default">>}} + , related_after => + #{ summary => <<"relative position 'after'">> + , value => #{<<"position">> => <<"after:default">>}} + }. + +parse_position(<<"front">>) -> + {ok, ?CMD_MOVE_FRONT}; +parse_position(<<"rear">>) -> + {ok, ?CMD_MOVE_REAR}; +parse_position(<<"before:">>) -> + {error, invalid_position}; +parse_position(<<"after:">>) -> + {error, invalid_position}; +parse_position(<<"before:", Related/binary>>) -> + {ok, ?CMD_MOVE_BEFORE(Related)}; +parse_position(<<"after:", Related/binary>>) -> + {ok, ?CMD_MOVE_AFTER(Related)}; +parse_position(_) -> + {error, invalid_position}. diff --git a/apps/emqx_exhook/src/emqx_exhook_mgr.erl b/apps/emqx_exhook/src/emqx_exhook_mgr.erl index c24765d6b..e34a8c046 100644 --- a/apps/emqx_exhook/src/emqx_exhook_mgr.erl +++ b/apps/emqx_exhook/src/emqx_exhook_mgr.erl @@ -74,10 +74,10 @@ -type server() :: server_options(). -type server_options() :: map(). --type move_direct() :: top - | bottom - | before - | 'after'. +-type position() :: front + | rear + | {before, binary()} + | {'after', binary()}. -type orders() :: #{server_name() => integer()}. @@ -157,8 +157,8 @@ pre_config_update(_, {delete, ToDelete}, OldConf) -> {ok, lists:dropwhile(fun(#{<<"name">> := Name}) -> Name =:= ToDelete end, OldConf)}; -pre_config_update(_, {move, Name, Position, Relate}, OldConf) -> - case do_move(Name, Position, Relate, OldConf) of +pre_config_update(_, {move, Name, Position}, OldConf) -> + case do_move(Name, Position, OldConf) of not_found -> {error, not_found}; NewConf -> {ok, NewConf} end; @@ -226,7 +226,7 @@ handle_call(list, _From, State = #{running := Running, {reply, OrderServers, State}; -handle_call({update_config, {move, _Name, _Direct, _Related}, NewConfL}, +handle_call({update_config, {move, _Name, _Position}, NewConfL}, _From, State) -> Orders = reorder(NewConfL), @@ -461,39 +461,39 @@ clean_reload_timer(Name, State = #{trefs := TRefs}) -> State#{trefs := NTRefs} end. --spec do_move(binary(), move_direct(), binary(), list(server_options())) -> +-spec do_move(binary(), position(), list(server_options())) -> not_found | list(server_options()). -do_move(Name, Direct, ToName, ConfL) -> - move(ConfL, Name, Direct, ToName, []). +do_move(Name, Position, ConfL) -> + move(ConfL, Name, Position, []). -move([#{<<"name">> := Name} = Server | T], Name, Direct, ToName, HeadL) -> - move_to(Direct, ToName, Server, lists:reverse(HeadL) ++ T); +move([#{<<"name">> := Name} = Server | T], Name, Position, HeadL) -> + move_to(Position, Server, lists:reverse(HeadL) ++ T); -move([Server | T], Name, Direct, ToName, HeadL) -> - move(T, Name, Direct, ToName, [Server | HeadL]); +move([Server | T], Name, Position, HeadL) -> + move(T, Name, Position, [Server | HeadL]); -move([], _Name, _Direct, _ToName, _HeadL) -> +move([], _Name, _Position, _HeadL) -> not_found. -move_to(top, _, Server, ServerL) -> +move_to(?CMD_MOVE_FRONT, Server, ServerL) -> [Server | ServerL]; -move_to(bottom, _, Server, ServerL) -> +move_to(?CMD_MOVE_REAR, Server, ServerL) -> ServerL ++ [Server]; -move_to(Direct, ToName, Server, ServerL) -> - move_to(ServerL, Direct, ToName, Server, []). +move_to(Position, Server, ServerL) -> + move_to(ServerL, Position, Server, []). -move_to([#{<<"name">> := Name} | _] = T, before, Name, Server, HeadL) -> +move_to([#{<<"name">> := Name} | _] = T, ?CMD_MOVE_BEFORE(Name), Server, HeadL) -> lists:reverse(HeadL) ++ [Server | T]; -move_to([#{<<"name">> := Name} = H | T], 'after', Name, Server, HeadL) -> +move_to([#{<<"name">> := Name} = H | T], ?CMD_MOVE_AFTER(Name), Server, HeadL) -> lists:reverse(HeadL) ++ [H, Server | T]; -move_to([H | T], Direct, Name, Server, HeadL) -> - move_to(T, Direct, Name, Server, [H | HeadL]); +move_to([H | T], Position, Server, HeadL) -> + move_to(T, Position, Server, [H | HeadL]); -move_to([], _Direct, _Name, _Server, _HeadL) -> +move_to([], _Position, _Server, _HeadL) -> not_found. -spec reorder(list(server_options())) -> orders(). diff --git a/apps/emqx_exhook/test/emqx_exhook_api_SUITE.erl b/apps/emqx_exhook/test/emqx_exhook_api_SUITE.erl index 11b847250..47a453f1e 100644 --- a/apps/emqx_exhook/test/emqx_exhook_api_SUITE.erl +++ b/apps/emqx_exhook/test/emqx_exhook_api_SUITE.erl @@ -37,7 +37,7 @@ exhook { ">>). all() -> - [ t_list, t_get, t_add, t_move_top, t_move_bottom + [ t_list, t_get, t_add, t_move_front, t_move_rear , t_move_before, t_move_after, t_delete, t_hooks, t_update ]. @@ -133,18 +133,18 @@ t_add(Cfg) -> ?assertMatch([<<"default">>, <<"test1">>], emqx_exhook_mgr:running()). -t_move_top(_) -> +t_move_front(_) -> Result = request_api(post, api_path(["exhooks", "default", "move"]), "", auth_header_(), - #{position => top, related => <<>>}), + #{position => <<"front">>}), ?assertMatch({ok, <<>>}, Result), ?assertMatch([<<"default">>, <<"test1">>], emqx_exhook_mgr:running()). -t_move_bottom(_) -> +t_move_rear(_) -> Result = request_api(post, api_path(["exhooks", "default", "move"]), "", auth_header_(), - #{position => bottom, related => <<>>}), + #{position => <<"rear">>}), ?assertMatch({ok, <<>>}, Result), ?assertMatch([<<"test1">>, <<"default">>], emqx_exhook_mgr:running()). @@ -152,7 +152,7 @@ t_move_bottom(_) -> t_move_before(_) -> Result = request_api(post, api_path(["exhooks", "default", "move"]), "", auth_header_(), - #{position => before, related => <<"test1">>}), + #{position => <<"before:test1">>}), ?assertMatch({ok, <<>>}, Result), ?assertMatch([<<"default">>, <<"test1">>], emqx_exhook_mgr:running()). @@ -160,7 +160,7 @@ t_move_before(_) -> t_move_after(_) -> Result = request_api(post, api_path(["exhooks", "default", "move"]), "", auth_header_(), - #{position => 'after', related => <<"test1">>}), + #{position => <<"after:test1">>}), ?assertMatch({ok, <<>>}, Result), ?assertMatch([<<"test1">>, <<"default">>], emqx_exhook_mgr:running()). diff --git a/apps/emqx_management/src/emqx_mgmt_api_plugins.erl b/apps/emqx_management/src/emqx_mgmt_api_plugins.erl index c7c963626..d806c309e 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_plugins.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_plugins.erl @@ -199,11 +199,11 @@ fields(builder) -> {website, hoconsc:mk(string(), #{example => "www.emqx.com"})} ]; fields(position) -> - [{position, hoconsc:mk(hoconsc:union([top, bottom, binary()]), + [{position, hoconsc:mk(hoconsc:union([front, rear, binary()]), #{ desc => """ Enable auto-boot at position in the boot list, where Position could be - 'top', 'bottom', or 'before:other-vsn', 'after:other-vsn' + 'front', 'rear', or 'before:other-vsn', 'after:other-vsn' to specify a relative position. """, required => false @@ -221,13 +221,13 @@ fields(running_status) -> move_request_body() -> emqx_dashboard_swagger:schema_with_examples(hoconsc:ref(?MODULE, position), #{ - move_to_top => #{ - summary => <<"move plugin on the top">>, - value => #{position => <<"top">>} + move_to_front => #{ + summary => <<"move plugin on the front">>, + value => #{position => <<"front">>} }, - move_to_bottom => #{ - summary => <<"move plugin on the bottom">>, - value => #{position => <<"bottom">>} + move_to_rear => #{ + summary => <<"move plugin on the rear">>, + value => #{position => <<"rear">>} }, move_to_before => #{ summary => <<"move plugin before other plugins">>, @@ -350,8 +350,8 @@ return(_, {error, #{error := "bad_info_file", return := {enoent, _}, path := Pat return(_, {error, Reason}) -> {400, #{code => 'PARAM_ERROR', message => iolist_to_binary(io_lib:format("~p", [Reason]))}}. -parse_position(#{<<"position">> := <<"top">>}, _) -> front; -parse_position(#{<<"position">> := <<"bottom">>}, _) -> rear; +parse_position(#{<<"position">> := <<"front">>}, _) -> front; +parse_position(#{<<"position">> := <<"rear">>}, _) -> rear; parse_position(#{<<"position">> := <<"before:", Name/binary>>}, Name) -> {error, <<"Can't before:self">>}; parse_position(#{<<"position">> := <<"after:", Name/binary>>}, Name) ->