Merge pull request #10226 from sstrigler/EMQX-9273-e-5-0-2-creating-a-data-bridge-the-name-verification-returned-an-incorrect-error-code-of-500

fix(emqx_bridge): don't crash on validation error

apps/emqx_bridge/src/emqx_bridge_api.erl

@@ -481,8 +481,7 @@ schema("/bridges_probe") ->
             ?BAD_REQUEST('ALREADY_EXISTS', <<"bridge already exists">>);
         {error, not_found} ->
             Conf = filter_out_request_body(Conf0),
-            {ok, _} = emqx_bridge:create(BridgeType, BridgeName, Conf),
-            lookup_from_all_nodes(BridgeType, BridgeName, 201)
+            create_bridge(BridgeType, BridgeName, Conf)
     end;
 '/bridges'(get, _Params) ->
     Nodes = mria:running_nodes(),
@@ -508,8 +507,7 @@ schema("/bridges_probe") ->
             {ok, _} ->
                 RawConf = emqx:get_raw_config([bridges, BridgeType, BridgeName], #{}),
                 Conf = deobfuscate(Conf1, RawConf),
-                {ok, _} = emqx_bridge:create(BridgeType, BridgeName, Conf),
-                lookup_from_all_nodes(BridgeType, BridgeName, 200);
+                update_bridge(BridgeType, BridgeName, Conf);
             {error, not_found} ->
                 ?BRIDGE_NOT_FOUND(BridgeType, BridgeName)
         end
@@ -609,6 +607,20 @@ lookup_from_local_node(BridgeType, BridgeName) ->
         Error -> Error
     end.
 
+create_bridge(BridgeType, BridgeName, Conf) ->
+    create_or_update_bridge(BridgeType, BridgeName, Conf, 201).
+
+update_bridge(BridgeType, BridgeName, Conf) ->
+    create_or_update_bridge(BridgeType, BridgeName, Conf, 200).
+
+create_or_update_bridge(BridgeType, BridgeName, Conf, HttpStatusCode) ->
+    case emqx_bridge:create(BridgeType, BridgeName, Conf) of
+        {ok, _} ->
+            lookup_from_all_nodes(BridgeType, BridgeName, HttpStatusCode);
+        {error, #{kind := validation_error} = Reason} ->
+            ?BAD_REQUEST(map_to_json(Reason))
+    end.
+
 '/bridges/:id/enable/:enable'(put, #{bindings := #{id := Id, enable := Enable}}) ->
     ?TRY_PARSE_ID(
         Id,
@@ -929,7 +941,7 @@ filter_out_request_body(Conf) ->
         <<"type">>,
         <<"name">>,
         <<"status">>,
-        <<"error">>,
+        <<"status_reason">>,
         <<"node_status">>,
         <<"node_metrics">>,
         <<"metrics">>,
@@ -1033,3 +1045,8 @@ deobfuscate(NewConf, OldConf) ->
         #{},
         NewConf
     ).
+
+map_to_json(M) ->
+    emqx_json:encode(
+        emqx_map_lib:jsonable_map(M, fun(K, V) -> {K, emqx_map_lib:binary_string(V)} end)
+    ).

apps/emqx_bridge/test/emqx_bridge_api_SUITE.erl

@@ -322,6 +322,33 @@ t_http_crud_apis(Config) ->
         end
     ),
 
+    %% Test bad updates
+    {ok, 400, PutFail1} = request(
+        put,
+        uri(["bridges", BridgeID]),
+        maps:remove(<<"url">>, ?HTTP_BRIDGE(URL2, Name))
+    ),
+    ?assertMatch(
+        #{<<"reason">> := <<"required_field">>},
+        emqx_json:decode(maps:get(<<"message">>, emqx_json:decode(PutFail1, [return_maps])), [
+            return_maps
+        ])
+    ),
+    {ok, 400, PutFail2} = request(
+        put,
+        uri(["bridges", BridgeID]),
+        maps:put(<<"curl">>, URL2, maps:remove(<<"url">>, ?HTTP_BRIDGE(URL2, Name)))
+    ),
+    ?assertMatch(
+        #{
+            <<"reason">> := <<"unknown_fields">>,
+            <<"unknown">> := <<"curl">>
+        },
+        emqx_json:decode(maps:get(<<"message">>, emqx_json:decode(PutFail2, [return_maps])), [
+            return_maps
+        ])
+    ),
+
     %% delete the bridge
     {ok, 204, <<>>} = request(delete, uri(["bridges", BridgeID]), []),
     {ok, 200, <<"[]">>} = request(get, uri(["bridges"]), []),
@@ -387,6 +414,9 @@ t_http_crud_apis(Config) ->
     ?assert(not maps:is_key(<<"status_reason">>, FixedBridge)),
     ?assert(not maps:is_key(<<"status_reason">>, FixedNodeStatus)),
     {ok, 204, <<>>} = request(delete, uri(["bridges", BridgeID]), []),
+
+    %% Try create bridge with bad characters as name
+    {ok, 400, _} = request(post, uri(["bridges"]), ?HTTP_BRIDGE(URL1, <<"隋达"/utf8>>)),
     ok.
 
 t_http_bridges_local_topic(Config) ->

changes/ce/fix-10226.en.md

@@ -0,0 +1 @@
+Don't crash on validation error in `/bridges` API, return `400` instead.