Browse Source

fix(bridge_v2 API): optional cascading delete operation

This commit makes the delete HTTP API operation for Bridge V2 behave in
the same way as in the Bridge V1 API.

Fixes:
https://emqx.atlassian.net/browse/EMQX-11293
Kjell Winblad 2 years atrás
parent
commit
1e935e9eb4

+ 24 - 1
apps/emqx_bridge/src/emqx_bridge_v2.erl

@@ -38,7 +38,11 @@
     list/0,
     lookup/2,
     create/3,
-    remove/2
+    remove/2,
+    %% The following is the remove function that is called by the HTTP API
+    %% It also checks for rule action dependencies and optionally removes
+    %% them
+    check_deps_and_remove/3
 ]).
 
 %% Operations
@@ -227,6 +231,25 @@ remove(BridgeType, BridgeName) ->
         {error, Reason} -> {error, Reason}
     end.
 
+check_deps_and_remove(BridgeType, BridgeName, AlsoDeleteActions) ->
+    AlsoDelete =
+        case AlsoDeleteActions of
+            true -> [rule_actions];
+            false -> []
+        end,
+    case
+        emqx_bridge_lib:maybe_withdraw_rule_action(
+            BridgeType,
+            BridgeName,
+            AlsoDelete
+        )
+    of
+        ok ->
+            remove(BridgeType, BridgeName);
+        {error, Reason} ->
+            {error, Reason}
+    end.
+
 %%--------------------------------------------------------------------
 %% Helpers for CRUD API
 %%--------------------------------------------------------------------

+ 28 - 3
apps/emqx_bridge/src/emqx_bridge_v2_api.erl

@@ -70,7 +70,11 @@
 namespace() -> "bridge_v2".
 
 api_spec() ->
-    emqx_dashboard_swagger:spec(?MODULE, #{check_schema => true}).
+    %% TODO
+    %% The check_schema option needs to be set to false so we get the
+    %% query_string to the delete operation. We can change this once
+    %% we have fixed the schmea for the delete operation.
+    emqx_dashboard_swagger:spec(?MODULE, #{check_schema => false}).
 
 paths() ->
     [
@@ -365,14 +369,35 @@ schema("/bridges_v2_probe") ->
                 ?BRIDGE_NOT_FOUND(BridgeType, BridgeName)
         end
     );
-'/bridges_v2/:id'(delete, #{bindings := #{id := Id}}) ->
+'/bridges_v2/:id'(delete, #{bindings := #{id := Id}, query_string := Qs} = All) ->
     ?TRY_PARSE_ID(
         Id,
         case emqx_bridge_v2:lookup(BridgeType, BridgeName) of
             {ok, _} ->
-                case emqx_bridge_v2:remove(BridgeType, BridgeName) of
+                AlsoDeleteActions =
+                    case maps:get(<<"also_delete_dep_actions">>, Qs, <<"false">>) of
+                        <<"true">> -> true;
+                        true -> true;
+                        _ -> false
+                    end,
+                case
+                    emqx_bridge_v2:check_deps_and_remove(BridgeType, BridgeName, AlsoDeleteActions)
+                of
                     ok ->
                         ?NO_CONTENT;
+                    {error, #{
+                        reason := rules_depending_on_this_bridge,
+                        rule_ids := RuleIds
+                    }} ->
+                        RuleIdLists = [binary_to_list(iolist_to_binary(X)) || X <- RuleIds],
+                        RulesStr = string:join(RuleIdLists, ", "),
+                        Msg = io_lib:format(
+                            "Cannot delete bridge while active rules are depending on it: ~s\n"
+                            "Append ?also_delete_dep_actions=true to the request URL to delete "
+                            "rule actions that depend on this bridge as well.",
+                            [RulesStr]
+                        ),
+                        ?BAD_REQUEST(iolist_to_binary(Msg));
                     {error, timeout} ->
                         ?SERVICE_UNAVAILABLE(<<"request timeout">>);
                     {error, Reason} ->

+ 69 - 1
apps/emqx_bridge/test/emqx_bridge_v2_api_SUITE.erl

@@ -147,7 +147,8 @@
     emqx,
     emqx_auth,
     emqx_management,
-    {emqx_bridge, "bridges_v2 {}"}
+    {emqx_bridge, "bridges_v2 {}"},
+    {emqx_rule_engine, "rule_engine { rules {} }"}
 ]).
 
 -define(APPSPEC_DASHBOARD,
@@ -667,6 +668,73 @@ t_bridges_probe(Config) ->
     ),
     ok.
 
+t_cascade_delete_actions(Config) ->
+    %% assert we there's no bridges at first
+    {ok, 200, []} = request_json(get, uri([?ROOT]), Config),
+    %% then we add a a bridge, using POST
+    %% POST /bridges_v2/ will create a bridge
+    BridgeID = emqx_bridge_resource:bridge_id(?BRIDGE_TYPE, ?BRIDGE_NAME),
+    {ok, 201, _} = request(
+        post,
+        uri([?ROOT]),
+        ?KAFKA_BRIDGE(?BRIDGE_NAME),
+        Config
+    ),
+    {ok, 201, #{<<"id">> := RuleId}} = request_json(
+        post,
+        uri(["rules"]),
+        #{
+            <<"name">> => <<"t_http_crud_apis">>,
+            <<"enable">> => true,
+            <<"actions">> => [BridgeID],
+            <<"sql">> => <<"SELECT * from \"t\"">>
+        },
+        Config
+    ),
+    %% delete the bridge will also delete the actions from the rules
+    {ok, 204, _} = request(
+        delete,
+        uri([?ROOT, BridgeID]) ++ "?also_delete_dep_actions=true",
+        Config
+    ),
+    {ok, 200, []} = request_json(get, uri([?ROOT]), Config),
+    ?assertMatch(
+        {ok, 200, #{<<"actions">> := []}},
+        request_json(get, uri(["rules", RuleId]), Config)
+    ),
+    {ok, 204, <<>>} = request(delete, uri(["rules", RuleId]), Config),
+
+    {ok, 201, _} = request(
+        post,
+        uri([?ROOT]),
+        ?KAFKA_BRIDGE(?BRIDGE_NAME),
+        Config
+    ),
+    {ok, 201, _} = request(
+        post,
+        uri(["rules"]),
+        #{
+            <<"name">> => <<"t_http_crud_apis">>,
+            <<"enable">> => true,
+            <<"actions">> => [BridgeID],
+            <<"sql">> => <<"SELECT * from \"t\"">>
+        },
+        Config
+    ),
+    {ok, 400, _} = request(
+        delete,
+        uri([?ROOT, BridgeID]),
+        Config
+    ),
+    {ok, 200, [_]} = request_json(get, uri([?ROOT]), Config),
+    %% Cleanup
+    {ok, 204, _} = request(
+        delete,
+        uri([?ROOT, BridgeID]) ++ "?also_delete_dep_actions=true",
+        Config
+    ),
+    {ok, 200, []} = request_json(get, uri([?ROOT]), Config).
+
 %%% helpers
 listen_on_random_port() ->
     SockOpts = [binary, {active, false}, {packet, raw}, {reuseaddr, true}, {backlog, 1000}],