diff --git a/apps/emqx_bridge/src/emqx_bridge_api.erl b/apps/emqx_bridge/src/emqx_bridge_api.erl index ff55976d0..30671b2bb 100644 --- a/apps/emqx_bridge/src/emqx_bridge_api.erl +++ b/apps/emqx_bridge/src/emqx_bridge_api.erl @@ -46,12 +46,18 @@ -export([lookup_from_local_node/2]). +-define(BAD_REQUEST(Reason), {400, error_msg('BAD_REQUEST', Reason)}). + +-define(BRIDGE_NOT_ENABLED, + ?BAD_REQUEST(<<"Forbidden operation, bridge not enabled">>) +). + -define(NOT_FOUND(Reason), {404, error_msg('NOT_FOUND', Reason)}). --define(BRIDGE_NOT_FOUND(Type, Name), +-define(BRIDGE_NOT_FOUND(BridgeType, BridgeName), ?NOT_FOUND( - <<"Bridge lookup failed: bridge named '", Name/binary, "' of type ", - (atom_to_binary(Type))/binary, " does not exist.">> + <<"Bridge lookup failed: bridge named '", BridgeName/binary, "' of type ", + (atom_to_binary(BridgeType))/binary, " does not exist.">> ) ). @@ -98,11 +104,11 @@ get_response_body_schema() -> param_path_operation_cluster() -> {operation, mk( - enum([stop, restart]), + enum([start, stop, restart]), #{ in => path, required => true, - example => <<"restart">>, + example => <<"start">>, desc => ?DESC("desc_param_path_operation_cluster") } )}. @@ -110,11 +116,11 @@ param_path_operation_cluster() -> param_path_operation_on_node() -> {operation, mk( - enum([stop, restart]), + enum([start, stop, restart]), #{ in => path, required => true, - example => <<"stop">>, + example => <<"start">>, desc => ?DESC("desc_param_path_operation_on_node") } )}. @@ -355,7 +361,7 @@ schema("/bridges/:id") -> 204 => <<"Bridge deleted">>, 400 => error_schema( 'BAD_REQUEST', - "Can not delete bridge while active rules defined for this bridge" + "Cannot delete bridge while active rules are defined for this bridge" ), 404 => error_schema('NOT_FOUND', "Bridge not found"), 503 => error_schema('SERVICE_UNAVAILABLE', "Service unavailable") @@ -447,7 +453,7 @@ schema("/nodes/:node/bridges/:id/:operation") -> 'BAD_REQUEST', "Problem with configuration of external service or bridge not enabled" ), - 404 => error_schema('NOT_FOUND', "Bridge not found or invalid operation"), + 404 => error_schema('NOT_FOUND', "Bridge or node not found or invalid operation"), 501 => error_schema('NOT_IMPLEMENTED', "Not Implemented"), 503 => error_schema('SERVICE_UNAVAILABLE', "Service unavailable") } @@ -472,15 +478,13 @@ schema("/bridges_probe") -> }. '/bridges'(post, #{body := #{<<"type">> := BridgeType, <<"name">> := BridgeName} = Conf0}) -> - Conf = filter_out_request_body(Conf0), case emqx_bridge:lookup(BridgeType, BridgeName) of {ok, _} -> {400, error_msg('ALREADY_EXISTS', <<"bridge already exists">>)}; {error, not_found} -> - case ensure_bridge_created(BridgeType, BridgeName, Conf) of - ok -> lookup_from_all_nodes(BridgeType, BridgeName, 201); - {error, Error} -> {400, Error} - end + Conf = filter_out_request_body(Conf0), + {ok, _} = emqx_bridge:create(BridgeType, BridgeName, Conf), + lookup_from_all_nodes(BridgeType, BridgeName, 201) end; '/bridges'(get, _Params) -> {200, @@ -499,12 +503,8 @@ schema("/bridges_probe") -> {ok, _} -> RawConf = emqx:get_raw_config([bridges, BridgeType, BridgeName], #{}), Conf = deobfuscate(Conf1, RawConf), - case ensure_bridge_created(BridgeType, BridgeName, Conf) of - ok -> - lookup_from_all_nodes(BridgeType, BridgeName, 200); - {error, Error} -> - {400, Error} - end; + {ok, _} = emqx_bridge:create(BridgeType, BridgeName, Conf), + lookup_from_all_nodes(BridgeType, BridgeName, 200); {error, not_found} -> ?BRIDGE_NOT_FOUND(BridgeType, BridgeName) end @@ -524,12 +524,10 @@ schema("/bridges_probe") -> {ok, _} -> 204; {error, {rules_deps_on_this_bridge, RuleIds}} -> - {400, - error_msg( - 'BAD_REQUEST', - {<<"Can not delete bridge while active rules defined for this bridge">>, - RuleIds} - )}; + ?BAD_REQUEST( + {<<"Cannot delete bridge while active rules are defined for this bridge">>, + RuleIds} + ); {error, timeout} -> {503, error_msg('SERVICE_UNAVAILABLE', <<"request timeout">>)}; {error, Reason} -> @@ -561,7 +559,7 @@ schema("/bridges_probe") -> Params1 = maybe_deobfuscate_bridge_probe(Params), case emqx_bridge_resource:create_dry_run(ConnType, maps:remove(<<"type">>, Params1)) of ok -> - {204}; + 204; {error, Reason} when not is_tuple(Reason); element(1, Reason) =/= 'exit' -> {400, error_msg('TEST_FAILED', to_hr_reason(Reason))} end; @@ -615,7 +613,7 @@ lookup_from_local_node(BridgeType, BridgeName) -> OperFunc -> case emqx_bridge:disable_enable(OperFunc, BridgeType, BridgeName) of {ok, _} -> - {204}; + 204; {error, {pre_config_update, _, bridge_not_found}} -> ?BRIDGE_NOT_FOUND(BridgeType, BridgeName); {error, {_, _, timeout}} -> @@ -638,8 +636,16 @@ lookup_from_local_node(BridgeType, BridgeName) -> invalid -> ?NOT_FOUND(<<"Invalid operation: ", Op/binary>>); OperFunc -> - Nodes = mria:running_nodes(), - call_operation(all, OperFunc, [Nodes, BridgeType, BridgeName]) + try is_enabled_bridge(BridgeType, BridgeName) of + false -> + ?BRIDGE_NOT_ENABLED; + true -> + Nodes = mria:running_nodes(), + call_operation(all, OperFunc, [Nodes, BridgeType, BridgeName]) + catch + throw:not_found -> + ?BRIDGE_NOT_FOUND(BridgeType, BridgeName) + end end ). @@ -653,14 +659,9 @@ lookup_from_local_node(BridgeType, BridgeName) -> invalid -> ?NOT_FOUND(<<"Invalid operation: ", Op/binary>>); OperFunc -> - ConfMap = emqx:get_config([bridges, BridgeType, BridgeName]), - case maps:get(enable, ConfMap, false) of + try is_enabled_bridge(BridgeType, BridgeName) of false -> - {400, - error_msg( - 'BAD_REQUEST', - <<"Forbidden operation, bridge not enabled">> - )}; + ?BRIDGE_NOT_ENABLED; true -> case emqx_misc:safe_to_existing_atom(Node, utf8) of {ok, TargetNode} -> @@ -670,10 +671,22 @@ lookup_from_local_node(BridgeType, BridgeName) -> {error, _} -> ?NOT_FOUND(<<"Invalid node name: ", Node/binary>>) end + catch + throw:not_found -> + ?BRIDGE_NOT_FOUND(BridgeType, BridgeName) end end ). +is_enabled_bridge(BridgeType, BridgeName) -> + try emqx:get_config([bridges, BridgeType, BridgeName]) of + ConfMap -> + maps:get(enable, ConfMap, false) + catch + error:{config_not_found, _} -> + throw(not_found) + end. + node_operation_func(<<"restart">>) -> restart_bridge_to_node; node_operation_func(<<"start">>) -> start_bridge_to_node; node_operation_func(<<"stop">>) -> stop_bridge_to_node; @@ -688,12 +701,6 @@ enable_func(<<"true">>) -> enable; enable_func(<<"false">>) -> disable; enable_func(_) -> invalid. -ensure_bridge_created(BridgeType, BridgeName, Conf) -> - case emqx_bridge:create(BridgeType, BridgeName, Conf) of - {ok, _} -> ok; - {error, Reason} -> {error, error_msg('BAD_REQUEST', Reason)} - end. - zip_bridges([BridgesFirstNode | _] = BridgesAllNodes) -> lists:foldl( fun(#{type := Type, name := Name}, Acc) -> @@ -930,12 +937,10 @@ bin(S) when is_atom(S) -> bin(S) when is_binary(S) -> S. -call_operation(NodeOrAll, OperFunc, Args) -> +call_operation(NodeOrAll, OperFunc, Args = [_Nodes, BridgeType, BridgeName]) -> case is_ok(do_bpapi_call(NodeOrAll, OperFunc, Args)) of - ok -> - {204}; - {ok, _} -> - {204}; + Ok when Ok =:= ok; is_tuple(Ok), element(1, Ok) =:= ok -> + 204; {error, not_implemented} -> %% Should only happen if we call `start` on a node that is %% still on an older bpapi version that doesn't support it. @@ -953,8 +958,12 @@ call_operation(NodeOrAll, OperFunc, Args) -> ) ) )}; + {error, not_found} -> + ?BRIDGE_NOT_FOUND(BridgeType, BridgeName); + {error, {node_not_found, Node}} -> + ?NOT_FOUND(<<"Node not found: ", (atom_to_binary(Node))/binary>>); {error, Reason} when not is_tuple(Reason); element(1, Reason) =/= 'exit' -> - {400, error_msg('BAD_REQUEST', to_hr_reason(Reason))} + ?BAD_REQUEST(to_hr_reason(Reason)) end. maybe_try_restart(all, start_bridges_to_all_nodes, Args) -> @@ -962,14 +971,19 @@ maybe_try_restart(all, start_bridges_to_all_nodes, Args) -> maybe_try_restart(Node, start_bridge_to_node, Args) -> call_operation(Node, restart_bridge_to_node, Args); maybe_try_restart(_, _, _) -> - {501}. + 501. do_bpapi_call(all, Call, Args) -> maybe_unwrap( do_bpapi_call_vsn(emqx_bpapi:supported_version(emqx_bridge), Call, Args) ); do_bpapi_call(Node, Call, Args) -> - do_bpapi_call_vsn(emqx_bpapi:supported_version(Node, emqx_bridge), Call, Args). + case lists:member(Node, mria:running_nodes()) of + true -> + do_bpapi_call_vsn(emqx_bpapi:supported_version(Node, emqx_bridge), Call, Args); + false -> + {error, {node_not_found, Node}} + end. do_bpapi_call_vsn(SupportedVersion, Call, Args) -> case lists:member(SupportedVersion, supported_versions(Call)) of diff --git a/apps/emqx_bridge/test/emqx_bridge_api_SUITE.erl b/apps/emqx_bridge/test/emqx_bridge_api_SUITE.erl index d30a9bff8..8b388a771 100644 --- a/apps/emqx_bridge/test/emqx_bridge_api_SUITE.erl +++ b/apps/emqx_bridge/test/emqx_bridge_api_SUITE.erl @@ -196,6 +196,9 @@ t_http_crud_apis(Config) -> %% assert we there's no bridges at first {ok, 200, <<"[]">>} = request(get, uri(["bridges"]), []), + {ok, 404, _} = request(get, uri(["bridges", "foo"]), []), + {ok, 404, _} = request(get, uri(["bridges", "webhook:foo"]), []), + %% then we add a webhook bridge, using POST %% POST /bridges/ will create a bridge URL1 = ?URL(Port, "path1"), @@ -214,7 +217,7 @@ t_http_crud_apis(Config) -> <<"status">> := _, <<"node_status">> := [_ | _], <<"url">> := URL1 - } = jsx:decode(Bridge), + } = emqx_json:decode(Bridge, [return_maps]), BridgeID = emqx_bridge_resource:bridge_id(?BRIDGE_TYPE, Name), %% send an message to emqx and the message should be forwarded to the HTTP server @@ -251,7 +254,7 @@ t_http_crud_apis(Config) -> <<"node_status">> := [_ | _], <<"url">> := URL2 }, - jsx:decode(Bridge2) + emqx_json:decode(Bridge2, [return_maps]) ), %% list all bridges again, assert Bridge2 is in it @@ -269,7 +272,7 @@ t_http_crud_apis(Config) -> <<"url">> := URL2 } ], - jsx:decode(Bridge2Str) + emqx_json:decode(Bridge2Str, [return_maps]) ), %% get the bridge by id @@ -283,7 +286,7 @@ t_http_crud_apis(Config) -> <<"node_status">> := [_ | _], <<"url">> := URL2 }, - jsx:decode(Bridge3Str) + emqx_json:decode(Bridge3Str, [return_maps]) ), %% send an message to emqx again, check the path has been changed @@ -315,8 +318,19 @@ t_http_crud_apis(Config) -> <<"code">> := <<"NOT_FOUND">>, <<"message">> := _ }, - jsx:decode(ErrMsg2) + emqx_json:decode(ErrMsg2, [return_maps]) ), + + %% try delete bad bridge id + {ok, 404, BadId} = request(delete, uri(["bridges", "foo"]), []), + ?assertMatch( + #{ + <<"code">> := <<"NOT_FOUND">>, + <<"message">> := <<"Invalid bridge ID", _/binary>> + }, + emqx_json:decode(BadId, [return_maps]) + ), + %% Deleting a non-existing bridge should result in an error {ok, 404, ErrMsg3} = request(delete, uri(["bridges", BridgeID]), []), ?assertMatch( @@ -324,7 +338,7 @@ t_http_crud_apis(Config) -> <<"code">> := <<"NOT_FOUND">>, <<"message">> := _ }, - jsx:decode(ErrMsg3) + emqx_json:decode(ErrMsg3, [return_maps]) ), ok. @@ -402,14 +416,17 @@ t_check_dependent_actions_on_delete(Config) -> <<"sql">> => <<"SELECT * from \"t\"">> } ), - #{<<"id">> := RuleId} = jsx:decode(Rule), - %% delete the bridge should fail because there is a rule depenents on it - {ok, 400, _} = request(delete, uri(["bridges", BridgeID]), []), + #{<<"id">> := RuleId} = emqx_json:decode(Rule, [return_maps]), + %% deleting the bridge should fail because there is a rule that depends on it + {ok, 400, _} = request( + delete, uri(["bridges", BridgeID]) ++ "?also_delete_dep_actions=false", [] + ), %% delete the rule first {ok, 204, <<>>} = request(delete, uri(["rules", RuleId]), []), %% then delete the bridge is OK {ok, 204, <<>>} = request(delete, uri(["bridges", BridgeID]), []), {ok, 200, <<"[]">>} = request(get, uri(["bridges"]), []), + ok. t_cascade_delete_actions(Config) -> @@ -437,18 +454,39 @@ t_cascade_delete_actions(Config) -> <<"sql">> => <<"SELECT * from \"t\"">> } ), - #{<<"id">> := RuleId} = jsx:decode(Rule), + #{<<"id">> := RuleId} = emqx_json:decode(Rule, [return_maps]), %% delete the bridge will also delete the actions from the rules - {ok, 204, _} = request(delete, uri(["bridges", BridgeID]) ++ "?also_delete_dep_actions", []), + {ok, 204, _} = request( + delete, uri(["bridges", BridgeID]) ++ "?also_delete_dep_actions=true", [] + ), {ok, 200, <<"[]">>} = request(get, uri(["bridges"]), []), {ok, 200, Rule1} = request(get, uri(["rules", RuleId]), []), ?assertMatch( #{ <<"actions">> := [] }, - jsx:decode(Rule1) + emqx_json:decode(Rule1, [return_maps]) ), {ok, 204, <<>>} = request(delete, uri(["rules", RuleId]), []), + + {ok, 201, _} = request( + post, + uri(["bridges"]), + ?HTTP_BRIDGE(URL1, ?BRIDGE_TYPE, Name) + ), + {ok, 201, _} = request( + post, + uri(["rules"]), + #{ + <<"name">> => <<"t_http_crud_apis">>, + <<"enable">> => true, + <<"actions">> => [BridgeID], + <<"sql">> => <<"SELECT * from \"t\"">> + } + ), + + {ok, 204, _} = request(delete, uri(["bridges", BridgeID]) ++ "?also_delete_dep_actions", []), + {ok, 200, <<"[]">>} = request(get, uri(["bridges"]), []), ok. t_broken_bpapi_vsn(Config) -> @@ -486,6 +524,18 @@ t_old_bpapi_vsn(Config) -> ok. t_start_stop_bridges_node(Config) -> + {ok, 404, _} = + request( + post, + uri(["nodes", "thisbetterbenotanatomyet", "bridges", "webhook:foo", start]), + <<"">> + ), + {ok, 404, _} = + request( + post, + uri(["nodes", "undefined", "bridges", "webhook:foo", start]), + <<"">> + ), do_start_stop_bridges(node, Config). t_start_stop_bridges_cluster(Config) -> @@ -511,34 +561,42 @@ do_start_stop_bridges(Type, Config) -> <<"status">> := <<"connected">>, <<"node_status">> := [_ | _], <<"url">> := URL1 - } = jsx:decode(Bridge), + } = emqx_json:decode(Bridge, [return_maps]), BridgeID = emqx_bridge_resource:bridge_id(?BRIDGE_TYPE, Name), %% stop it {ok, 204, <<>>} = request(post, operation_path(Type, stop, BridgeID), <<"">>), {ok, 200, Bridge2} = request(get, uri(["bridges", BridgeID]), []), - ?assertMatch(#{<<"status">> := <<"stopped">>}, jsx:decode(Bridge2)), + ?assertMatch(#{<<"status">> := <<"stopped">>}, emqx_json:decode(Bridge2, [return_maps])), %% start again {ok, 204, <<>>} = request(post, operation_path(Type, start, BridgeID), <<"">>), {ok, 200, Bridge3} = request(get, uri(["bridges", BridgeID]), []), - ?assertMatch(#{<<"status">> := <<"connected">>}, jsx:decode(Bridge3)), + ?assertMatch(#{<<"status">> := <<"connected">>}, emqx_json:decode(Bridge3, [return_maps])), %% start a started bridge {ok, 204, <<>>} = request(post, operation_path(Type, start, BridgeID), <<"">>), {ok, 200, Bridge3_1} = request(get, uri(["bridges", BridgeID]), []), - ?assertMatch(#{<<"status">> := <<"connected">>}, jsx:decode(Bridge3_1)), + ?assertMatch(#{<<"status">> := <<"connected">>}, emqx_json:decode(Bridge3_1, [return_maps])), %% restart an already started bridge {ok, 204, <<>>} = request(post, operation_path(Type, restart, BridgeID), <<"">>), {ok, 200, Bridge3} = request(get, uri(["bridges", BridgeID]), []), - ?assertMatch(#{<<"status">> := <<"connected">>}, jsx:decode(Bridge3)), + ?assertMatch(#{<<"status">> := <<"connected">>}, emqx_json:decode(Bridge3, [return_maps])), %% stop it again {ok, 204, <<>>} = request(post, operation_path(Type, stop, BridgeID), <<"">>), %% restart a stopped bridge {ok, 204, <<>>} = request(post, operation_path(Type, restart, BridgeID), <<"">>), {ok, 200, Bridge4} = request(get, uri(["bridges", BridgeID]), []), - ?assertMatch(#{<<"status">> := <<"connected">>}, jsx:decode(Bridge4)), + ?assertMatch(#{<<"status">> := <<"connected">>}, emqx_json:decode(Bridge4, [return_maps])), + + {ok, 404, _} = request(post, operation_path(Type, invalidop, BridgeID), <<"">>), + %% delete the bridge {ok, 204, <<>>} = request(delete, uri(["bridges", BridgeID]), []), {ok, 200, <<"[]">>} = request(get, uri(["bridges"]), []), + %% Fail parse-id check + {ok, 404, _} = request(post, operation_path(Type, start, <<"wreckbook_fugazi">>), <<"">>), + %% Looks ok but doesn't exist + {ok, 404, _} = request(post, operation_path(Type, start, <<"webhook:cptn_hook">>), <<"">>), + %% Create broken bridge {ListenPort, Sock} = listen_on_random_port(), %% Connecting to this endpoint should always timeout @@ -556,7 +614,7 @@ do_start_stop_bridges(Type, Config) -> <<"server">> := BadServer, <<"status">> := <<"connecting">>, <<"node_status">> := [_ | _] - } = jsx:decode(BadBridge1), + } = emqx_json:decode(BadBridge1, [return_maps]), BadBridgeID = emqx_bridge_resource:bridge_id(?BRIDGE_TYPE_MQTT, BadName), ?assertMatch( {ok, SC, _} when SC == 500 orelse SC == 503, @@ -585,33 +643,39 @@ t_enable_disable_bridges(Config) -> <<"status">> := <<"connected">>, <<"node_status">> := [_ | _], <<"url">> := URL1 - } = jsx:decode(Bridge), + } = emqx_json:decode(Bridge, [return_maps]), BridgeID = emqx_bridge_resource:bridge_id(?BRIDGE_TYPE, Name), %% disable it {ok, 204, <<>>} = request(put, enable_path(false, BridgeID), <<"">>), {ok, 200, Bridge2} = request(get, uri(["bridges", BridgeID]), []), - ?assertMatch(#{<<"status">> := <<"stopped">>}, jsx:decode(Bridge2)), + ?assertMatch(#{<<"status">> := <<"stopped">>}, emqx_json:decode(Bridge2, [return_maps])), %% enable again {ok, 204, <<>>} = request(put, enable_path(true, BridgeID), <<"">>), {ok, 200, Bridge3} = request(get, uri(["bridges", BridgeID]), []), - ?assertMatch(#{<<"status">> := <<"connected">>}, jsx:decode(Bridge3)), + ?assertMatch(#{<<"status">> := <<"connected">>}, emqx_json:decode(Bridge3, [return_maps])), %% enable an already started bridge {ok, 204, <<>>} = request(put, enable_path(true, BridgeID), <<"">>), {ok, 200, Bridge3} = request(get, uri(["bridges", BridgeID]), []), - ?assertMatch(#{<<"status">> := <<"connected">>}, jsx:decode(Bridge3)), + ?assertMatch(#{<<"status">> := <<"connected">>}, emqx_json:decode(Bridge3, [return_maps])), %% disable it again {ok, 204, <<>>} = request(put, enable_path(false, BridgeID), <<"">>), - {ok, 400, Res} = request(post, operation_path(node, restart, BridgeID), <<"">>), + %% bad param + {ok, 404, _} = request(put, enable_path(foo, BridgeID), <<"">>), + {ok, 404, _} = request(put, enable_path(true, "foo"), <<"">>), + {ok, 404, _} = request(put, enable_path(true, "webhook:foo"), <<"">>), + + {ok, 400, Res} = request(post, operation_path(node, start, BridgeID), <<"">>), ?assertEqual( <<"{\"code\":\"BAD_REQUEST\",\"message\":\"Forbidden operation, bridge not enabled\"}">>, Res ), + {ok, 400, Res} = request(post, operation_path(cluster, start, BridgeID), <<"">>), %% enable a stopped bridge {ok, 204, <<>>} = request(put, enable_path(true, BridgeID), <<"">>), {ok, 200, Bridge4} = request(get, uri(["bridges", BridgeID]), []), - ?assertMatch(#{<<"status">> := <<"connected">>}, jsx:decode(Bridge4)), + ?assertMatch(#{<<"status">> := <<"connected">>}, emqx_json:decode(Bridge4, [return_maps])), %% delete the bridge {ok, 204, <<>>} = request(delete, uri(["bridges", BridgeID]), []), {ok, 200, <<"[]">>} = request(get, uri(["bridges"]), []). @@ -636,7 +700,7 @@ t_reset_bridges(Config) -> <<"status">> := <<"connected">>, <<"node_status">> := [_ | _], <<"url">> := URL1 - } = jsx:decode(Bridge), + } = emqx_json:decode(Bridge, [return_maps]), BridgeID = emqx_bridge_resource:bridge_id(?BRIDGE_TYPE, Name), {ok, 204, <<>>} = request(put, uri(["bridges", BridgeID, "metrics/reset"]), []), @@ -704,7 +768,7 @@ t_bridges_probe(Config) -> <<"code">> := <<"TEST_FAILED">>, <<"message">> := _ }, - jsx:decode(NxDomain) + emqx_json:decode(NxDomain, [return_maps]) ), {ok, 204, _} = request( @@ -723,7 +787,7 @@ t_bridges_probe(Config) -> <<"code">> := <<"TEST_FAILED">>, <<"message">> := <<"Connection refused">> }, - jsx:decode(ConnRefused) + emqx_json:decode(ConnRefused, [return_maps]) ), {ok, 400, HostNotFound} = request( @@ -736,7 +800,7 @@ t_bridges_probe(Config) -> <<"code">> := <<"TEST_FAILED">>, <<"message">> := <<"Host not found">> }, - jsx:decode(HostNotFound) + emqx_json:decode(HostNotFound, [return_maps]) ), AuthnConfig = #{ @@ -767,7 +831,7 @@ t_bridges_probe(Config) -> <<"code">> := <<"TEST_FAILED">>, <<"message">> := <<"Unauthorized client">> }, - jsx:decode(Unauthorized) + emqx_json:decode(Unauthorized, [return_maps]) ), {ok, 400, Malformed} = request( @@ -782,7 +846,7 @@ t_bridges_probe(Config) -> <<"code">> := <<"TEST_FAILED">>, <<"message">> := <<"Malformed username or password">> }, - jsx:decode(Malformed) + emqx_json:decode(Malformed, [return_maps]) ), {ok, 400, NotAuthorized} = request( @@ -795,7 +859,7 @@ t_bridges_probe(Config) -> <<"code">> := <<"TEST_FAILED">>, <<"message">> := <<"Not authorized">> }, - jsx:decode(NotAuthorized) + emqx_json:decode(NotAuthorized, [return_maps]) ), {ok, 400, BadReq} = request( @@ -803,7 +867,7 @@ t_bridges_probe(Config) -> uri(["bridges_probe"]), ?BRIDGE(<<"bad_bridge">>, <<"unknown_type">>) ), - ?assertMatch(#{<<"code">> := <<"BAD_REQUEST">>}, jsx:decode(BadReq)), + ?assertMatch(#{<<"code">> := <<"BAD_REQUEST">>}, emqx_json:decode(BadReq, [return_maps])), ok. t_metrics(Config) -> @@ -829,7 +893,7 @@ t_metrics(Config) -> <<"status">> := _, <<"node_status">> := [_ | _], <<"url">> := URL1 - } = jsx:decode(Bridge), + } = emqx_json:decode(Bridge, [return_maps]), BridgeID = emqx_bridge_resource:bridge_id(?BRIDGE_TYPE, Name), @@ -840,12 +904,12 @@ t_metrics(Config) -> <<"metrics">> := #{<<"success">> := 0}, <<"node_metrics">> := [_ | _] }, - jsx:decode(Bridge1Str) + emqx_json:decode(Bridge1Str, [return_maps]) ), %% check that the bridge doesn't contain metrics anymore {ok, 200, Bridge2Str} = request(get, uri(["bridges", BridgeID]), []), - Decoded = jsx:decode(Bridge2Str), + Decoded = emqx_json:decode(Bridge2Str, [return_maps]), ?assertNot(maps:is_key(<<"metrics">>, Decoded)), ?assertNot(maps:is_key(<<"node_metrics">>, Decoded)), @@ -875,7 +939,7 @@ t_metrics(Config) -> <<"metrics">> := #{<<"success">> := _}, <<"node_metrics">> := [_ | _] }, - jsx:decode(Bridge3Str) + emqx_json:decode(Bridge3Str, [return_maps]) ), %% check for non-empty metrics when listing all bridges @@ -887,7 +951,7 @@ t_metrics(Config) -> <<"node_metrics">> := [_ | _] } ], - jsx:decode(BridgesStr) + emqx_json:decode(BridgesStr, [return_maps]) ), ok. diff --git a/changes/ce/fix-10107.en.md b/changes/ce/fix-10107.en.md new file mode 100644 index 000000000..1bcbbad60 --- /dev/null +++ b/changes/ce/fix-10107.en.md @@ -0,0 +1,9 @@ +For operations on `bridges API` if `bridge-id` is unknown we now return `404` +instead of `400`. Also a bug was fixed that caused a crash if that was a node +operation. Additionally we now also check if the given bridge is enabled when +doing the cluster operation `start` . Affected endpoints: + * [cluster] `/bridges/:id/:operation`, + * [node] `/nodes/:node/bridges/:id/:operation`, where `operation` is one of +`[start|stop|restart]`. +Moreover, for a node operation, EMQX checks if node name is in our cluster and +return `404` instead of `501`. diff --git a/changes/ce/fix-10107.zh.md b/changes/ce/fix-10107.zh.md new file mode 100644 index 000000000..e541a834f --- /dev/null +++ b/changes/ce/fix-10107.zh.md @@ -0,0 +1,8 @@ +现在对桥接的 API 进行调用时，如果 `bridge-id` 不存在，将会返回 `404`，而不再是`400`。 +然后，还修复了这种情况下，在节点级别上进行 API 调用时，可能导致崩溃的问题。 +另外，在启动某个桥接时，会先检查指定桥接是否已启用。 +受影响的接口有： + * [cluster] `/bridges/:id/:operation`, + * [node] `/nodes/:node/bridges/:id/:operation`, +其中 `operation` 是 `[start|stop|restart]` 之一。 +此外，对于节点操作，EMQX 将检查节点是否存在于集群中，如果不在，则会返回`404`，而不再是`501`。