fix(emqx_bridge): return 400 if operation not possible

This commit is contained in:
Stefan Strigler 2023-03-02 09:54:29 +01:00
parent 9fb74bfc87
commit 4c23ab097d
2 changed files with 15 additions and 15 deletions

View File

@ -353,8 +353,11 @@ schema("/bridges/:id") ->
parameters => [param_path_id()], parameters => [param_path_id()],
responses => #{ responses => #{
204 => <<"Bridge deleted">>, 204 => <<"Bridge deleted">>,
400 => error_schema(
'BAD_REQUEST',
"Can not delete bridge while active rules defined for this bridge"
),
404 => error_schema('NOT_FOUND', "Bridge not found"), 404 => error_schema('NOT_FOUND', "Bridge not found"),
403 => error_schema('FORBIDDEN_REQUEST', "Forbidden operation"),
503 => error_schema('SERVICE_UNAVAILABLE', "Service unavailable") 503 => error_schema('SERVICE_UNAVAILABLE', "Service unavailable")
} }
} }
@ -438,8 +441,8 @@ schema("/nodes/:node/bridges/:id/:operation") ->
], ],
responses => #{ responses => #{
204 => <<"Operation success">>, 204 => <<"Operation success">>,
400 => error_schema('BAD_REQUEST', "Forbidden operation, bridge not enabled"),
404 => error_schema('NOT_FOUND', "Bridge not found or invalid operation"), 404 => error_schema('NOT_FOUND', "Bridge not found or invalid operation"),
403 => error_schema('FORBIDDEN_REQUEST', "forbidden operation"),
501 => error_schema('NOT_IMPLEMENTED', "Not Implemented"), 501 => error_schema('NOT_IMPLEMENTED', "Not Implemented"),
503 => error_schema('SERVICE_UNAVAILABLE', "Service unavailable") 503 => error_schema('SERVICE_UNAVAILABLE', "Service unavailable")
} }
@ -516,12 +519,11 @@ schema("/bridges_probe") ->
{ok, _} -> {ok, _} ->
204; 204;
{error, {rules_deps_on_this_bridge, RuleIds}} -> {error, {rules_deps_on_this_bridge, RuleIds}} ->
%% [FIXME] this should be a 400 since '403' is about {400,
%% authorization and not application logic.
{403,
error_msg( error_msg(
'FORBIDDEN_REQUEST', 'BAD_REQUEST',
{<<"There're some rules dependent on this bridge">>, RuleIds} {<<"Can not delete bridge while active rules defined for this bridge">>,
RuleIds}
)}; )};
{error, timeout} -> {error, timeout} ->
{503, error_msg('SERVICE_UNAVAILABLE', <<"request timeout">>)}; {503, error_msg('SERVICE_UNAVAILABLE', <<"request timeout">>)};
@ -638,12 +640,10 @@ lookup_from_local_node(BridgeType, BridgeName) ->
ConfMap = emqx:get_config([bridges, BridgeType, BridgeName]), ConfMap = emqx:get_config([bridges, BridgeType, BridgeName]),
case maps:get(enable, ConfMap, false) of case maps:get(enable, ConfMap, false) of
false -> false ->
%% [FIXME] `403` is about authorization not application {400,
%% logic.
{403,
error_msg( error_msg(
'FORBIDDEN_REQUEST', 'BAD_REQUEST',
<<"forbidden operation: bridge disabled">> <<"Forbidden operation, bridge not enabled">>
)}; )};
true -> true ->
case emqx_misc:safe_to_existing_atom(Node, utf8) of case emqx_misc:safe_to_existing_atom(Node, utf8) of

View File

@ -403,7 +403,7 @@ t_check_dependent_actions_on_delete(Config) ->
), ),
#{<<"id">> := RuleId} = jsx:decode(Rule), #{<<"id">> := RuleId} = jsx:decode(Rule),
%% delete the bridge should fail because there is a rule depenents on it %% delete the bridge should fail because there is a rule depenents on it
{ok, 403, _} = request(delete, uri(["bridges", BridgeID]), []), {ok, 400, _} = request(delete, uri(["bridges", BridgeID]), []),
%% delete the rule first %% delete the rule first
{ok, 204, <<>>} = request(delete, uri(["rules", RuleId]), []), {ok, 204, <<>>} = request(delete, uri(["rules", RuleId]), []),
%% then delete the bridge is OK %% then delete the bridge is OK
@ -601,9 +601,9 @@ t_enable_disable_bridges(Config) ->
%% disable it again %% disable it again
{ok, 204, <<>>} = request(put, enable_path(false, BridgeID), <<"">>), {ok, 204, <<>>} = request(put, enable_path(false, BridgeID), <<"">>),
{ok, 403, Res} = request(post, operation_path(node, restart, BridgeID), <<"">>), {ok, 400, Res} = request(post, operation_path(node, restart, BridgeID), <<"">>),
?assertEqual( ?assertEqual(
<<"{\"code\":\"FORBIDDEN_REQUEST\",\"message\":\"forbidden operation: bridge disabled\"}">>, <<"{\"code\":\"BAD_REQUEST\",\"message\":\"Forbidden operation, bridge not enabled\"}">>,
Res Res
), ),