Merge pull request #11106 from rabbitmq/mergify/bp/v3.13.x/pr-11077

michaelklishin · web-flow · commit 78d9cc9c2451 · 2024-04-26T20:41:40.000-04:00
Notify of permission and parameter deletions when deleting a vhost (backport #11077)
diff --git a/deps/rabbit/src/rabbit_auth_backend_internal.erl b/deps/rabbit/src/rabbit_auth_backend_internal.erl
@@ -17,8 +17,9 @@
 -export([add_user/3, add_user/4, add_user/5, delete_user/2, lookup_user/1, exists/1,
          change_password/3, clear_password/2,
          hash_password/2, change_password_hash/2, change_password_hash/3,
-         set_tags/3, set_permissions/6, clear_permissions/3, clear_permissions_for_vhost/2, set_permissions_globally/5,
-         set_topic_permissions/6, clear_topic_permissions/3, clear_topic_permissions/4, clear_topic_permissions_for_vhost/2,
+         set_tags/3, set_permissions/6, clear_permissions/3, set_permissions_globally/5,
+         set_topic_permissions/6, clear_topic_permissions/3, clear_topic_permissions/4,
+         clear_all_permissions_for_vhost/2,
          add_user_sans_validation/3, put_user/2, put_user/3,
          update_user/5,
          update_user_with_hash/5,
@@ -540,8 +541,35 @@ clear_permissions(Username, VirtualHost, ActingUser) ->
             erlang:raise(Class, Error, Stacktrace)
     end.
 
-clear_permissions_for_vhost(VirtualHost, _ActingUser) ->
-    rabbit_db_user:clear_matching_user_permissions('_', VirtualHost).
+-spec clear_all_permissions_for_vhost(VirtualHost, ActingUser) -> Ret when
+      VirtualHost :: rabbit_types:vhost(),
+      ActingUser :: rabbit_types:username(),
+      Ret :: ok | {error, Reason :: any()}.
+
+clear_all_permissions_for_vhost(VirtualHost, ActingUser) ->
+    case rabbit_db_user:clear_all_permissions_for_vhost(VirtualHost) of
+        {ok, Deletions} ->
+            lists:foreach(
+              fun (#topic_permission{topic_permission_key =
+                      #topic_permission_key{user_vhost =
+                         #user_vhost{username = Username}}}) ->
+                      rabbit_event:notify(
+                        topic_permission_deleted,
+                        [{user, Username},
+                         {vhost, VirtualHost},
+                         {user_who_performed_action, ActingUser}]);
+                  (#user_permission{user_vhost =
+                                    #user_vhost{username = Username}}) ->
+                      rabbit_event:notify(
+                        permission_deleted,
+                        [{user, Username},
+                         {vhost, VirtualHost},
+                         {user_who_performed_action, ActingUser}])
+              end, Deletions),
+            ok;
+        {error, _} = Err ->
+            Err
+    end.
 
 set_permissions_globally(Username, ConfigurePerm, WritePerm, ReadPerm, ActingUser) ->
     VirtualHosts = rabbit_vhost:list_names(),
@@ -642,9 +670,6 @@ clear_topic_permissions(Username, VirtualHost, Exchange, ActingUser) ->
             erlang:raise(Class, Error, Stacktrace)
     end.
 
-clear_topic_permissions_for_vhost(VirtualHost, _ActingUser) ->
-    rabbit_db_user:clear_matching_topic_permissions('_', VirtualHost, '_').
-
 put_user(User, ActingUser) -> put_user(User, undefined, ActingUser).
 
 put_user(User, Version, ActingUser) ->
diff --git a/deps/rabbit/src/rabbit_db_rtparams.erl b/deps/rabbit/src/rabbit_db_rtparams.erl
@@ -14,7 +14,8 @@
          get/1,
          get_or_set/2,
          get_all/0, get_all/2,
-         delete/1, delete/3]).
+         delete/1, delete/3,
+         delete_vhost/1]).
 
 -export([khepri_vhost_rp_path/3,
          khepri_global_rp_path/1,
@@ -340,6 +341,55 @@ delete_matching_in_khepri(VHostName, Comp, Name) ->
     Key = {?any(VHostName), ?any(Comp), ?any(Name)},
     delete_in_khepri(Key).
 
+%% -------------------------------------------------------------------
+%% delete_vhost().
+%% -------------------------------------------------------------------
+
+-spec delete_vhost(VHostName) -> Ret when
+      VHostName :: vhost:name(),
+      Ret :: {ok, Deletions} | {error, Reason :: any()},
+      Deletions :: [#runtime_parameters{}].
+%% @doc Deletes all runtime parameters belonging to the given virtual host.
+%%
+%% @returns an OK tuple containing the deleted runtime parameters if
+%% successful, or an error tuple otherwise.
+%%
+%% @private
+
+delete_vhost(VHostName) when is_binary(VHostName) ->
+    rabbit_khepri:handle_fallback(
+      #{mnesia => fun() -> delete_vhost_in_mnesia(VHostName) end,
+        khepri => fun() -> delete_vhost_in_khepri(VHostName) end}).
+
+delete_vhost_in_mnesia(VHostName) ->
+    rabbit_mnesia:execute_mnesia_transaction(
+      fun() ->
+              Deletions = delete_vhost_in_mnesia_tx(VHostName),
+              {ok, Deletions}
+      end).
+
+delete_vhost_in_mnesia_tx(VHostName) ->
+    Match = #runtime_parameters{key = {VHostName, '_', '_'},
+                                _   = '_'},
+    [begin
+         mnesia:delete(?MNESIA_TABLE, Key, write),
+         Record
+     end
+     || #runtime_parameters{key = Key} = Record
+        <- mnesia:match_object(?MNESIA_TABLE, Match, read)].
+
+delete_vhost_in_khepri(VHostName) ->
+    Path = khepri_vhost_rp_path(
+             VHostName, ?KHEPRI_WILDCARD_STAR, ?KHEPRI_WILDCARD_STAR),
+    case rabbit_khepri:adv_delete_many(Path) of
+        {ok, Props} ->
+            {ok, rabbit_khepri:collect_payloads(Props)};
+        {error, _} = Err ->
+            Err
+    end.
+
+%% -------------------------------------------------------------------
+
 khepri_rp_path() ->
     [?MODULE].
 
diff --git a/deps/rabbit/src/rabbit_db_user.erl b/deps/rabbit/src/rabbit_db_user.erl
@@ -28,7 +28,8 @@
          set_topic_permissions/1,
          clear_topic_permissions/3,
          clear_matching_topic_permissions/3,
-         delete/1]).
+         delete/1,
+         clear_all_permissions_for_vhost/1]).
 
 -export([khepri_users_path/0,
          khepri_user_path/1,
@@ -548,6 +549,57 @@ clear_matching_user_permissions_in_khepri(Username, VHostName) ->
 any('_') -> ?KHEPRI_WILDCARD_STAR;
 any(Value) -> Value.
 
+%% -------------------------------------------------------------------
+%% clear_all_permissions_for_vhost().
+%% -------------------------------------------------------------------
+
+-spec clear_all_permissions_for_vhost(VHostName) -> Ret when
+      VHostName :: vhost:name(),
+      Ret :: {ok, DeletedPermissions} | {error, Reason :: any()},
+      DeletedPermissions :: [#topic_permission{} | #user_permission{}].
+%% @doc Transactionally deletes all user and topic permissions for a virtual
+%% host, returning any permissions that were deleted.
+%%
+%% @returns an OK-tuple with the deleted permissions or an error tuple if the
+%% operation could not be completed.
+%%
+%% @private
+
+clear_all_permissions_for_vhost(VHostName) when is_binary(VHostName) ->
+    rabbit_khepri:handle_fallback(
+      #{mnesia =>
+        fun() -> clear_all_permissions_for_vhost_in_mnesia(VHostName) end,
+        khepri =>
+        fun() -> clear_all_permissions_for_vhost_in_khepri(VHostName) end}).
+
+clear_all_permissions_for_vhost_in_mnesia(VHostName) ->
+    rabbit_mnesia:execute_mnesia_transaction(
+      fun() ->
+              Deletions =
+              clear_matching_topic_permissions_in_mnesia_tx(
+                '_', VHostName, '_') ++
+                  clear_matching_user_permissions_in_mnesia_tx(
+                    '_', VHostName),
+              {ok, Deletions}
+      end).
+
+clear_all_permissions_for_vhost_in_khepri(VHostName) ->
+    rabbit_khepri:transaction(
+      fun() ->
+              UserPermissionsPath = khepri_user_permission_path(
+                                      ?KHEPRI_WILDCARD_STAR, VHostName),
+              TopicPermissionsPath = khepri_topic_permission_path(
+                                       ?KHEPRI_WILDCARD_STAR, VHostName,
+                                       ?KHEPRI_WILDCARD_STAR),
+              {ok, UserProps} = khepri_tx_adv:delete_many(UserPermissionsPath),
+              {ok, TopicProps} = khepri_tx_adv:delete_many(
+                                   TopicPermissionsPath),
+              Deletions = rabbit_khepri:collect_payloads(
+                            TopicProps,
+                            rabbit_khepri:collect_payloads(UserProps)),
+              {ok, Deletions}
+      end, rw).
+
 %% -------------------------------------------------------------------
 %% get_topic_permissions().
 %% -------------------------------------------------------------------
diff --git a/deps/rabbit/src/rabbit_khepri.erl b/deps/rabbit/src/rabbit_khepri.erl
@@ -133,6 +133,7 @@
          clear_payload/1,
          delete/1, delete/2,
          delete_or_fail/1,
+         adv_delete_many/1,
 
          transaction/1,
          transaction/2,
@@ -170,6 +171,10 @@
 
 -export([force_shrink_member_to_current_member/0]).
 
+%% Helpers for working with the Khepri API / types.
+-export([collect_payloads/1,
+         collect_payloads/2]).
+
 -ifdef(TEST).
 -export([force_metadata_store/1,
          clear_forced_metadata_store/0]).
@@ -946,6 +951,9 @@ delete_or_fail(Path) ->
             Error
     end.
 
+adv_delete_many(Path) ->
+    khepri_adv:delete_many(?STORE_ID, Path, ?DEFAULT_COMMAND_OPTIONS).
+
 put(PathPattern, Data) ->
     khepri:put(
       ?STORE_ID, PathPattern, Data, ?DEFAULT_COMMAND_OPTIONS).
@@ -983,6 +991,48 @@ info() ->
 handle_async_ret(RaEvent) ->
     khepri:handle_async_ret(?STORE_ID, RaEvent).
 
+%% -------------------------------------------------------------------
+%% collect_payloads().
+%% -------------------------------------------------------------------
+
+-spec collect_payloads(Props) -> Ret when
+      Props :: khepri:node_props(),
+      Ret :: [Payload],
+      Payload :: term().
+
+%% @doc Collects all payloads from a node props map.
+%%
+%% This is the same as calling `collect_payloads(Props, [])'.
+%%
+%% @private
+
+collect_payloads(Props) when is_map(Props) ->
+    collect_payloads(Props, []).
+
+-spec collect_payloads(Props, Acc0) -> Ret when
+      Props :: khepri:node_props(),
+      Acc0 :: [Payload],
+      Ret :: [Payload],
+      Payload :: term().
+
+%% @doc Collects all payloads from a node props map into the accumulator list.
+%%
+%% This is meant to be used with the `khepri_adv' API to easily collect the
+%% payloads from the return value of `khepri_adv:delete_many/4' for example.
+%%
+%% @returns all payloads in the node props map collected into a list, with
+%% `Acc0' as the tail.
+%%
+%% @private
+
+collect_payloads(Props, Acc0) when is_map(Props) andalso is_list(Acc0) ->
+    maps:fold(
+      fun (_Path, #{data := Payload}, Acc) ->
+              [Payload | Acc];
+          (_Path, _NoPayload, Acc) ->
+              Acc
+      end, Acc0, Props).
+
 %% -------------------------------------------------------------------
 %% if_has_data_wildcard().
 %% -------------------------------------------------------------------
diff --git a/deps/rabbit/src/rabbit_runtime_parameters.erl b/deps/rabbit/src/rabbit_runtime_parameters.erl
@@ -203,8 +203,28 @@ clear_global(Key, ActingUser) ->
                           {user_who_performed_action, ActingUser}])
     end.
 
-clear_vhost(VHostName, _ActingUser) when is_binary(VHostName) ->
-    ok = rabbit_db_rtparams:delete(VHostName, '_', '_').
+clear_vhost(VHostName, ActingUser) when is_binary(VHostName) ->
+    case rabbit_db_rtparams:delete_vhost(VHostName) of
+        {ok, DeletedParams} ->
+            lists:foreach(
+              fun(#runtime_parameters{key = {_VHost, Component, Name}}) ->
+                      case lookup_component(Component) of
+                          {ok, Mod} ->
+                              event_notify(
+                                parameter_cleared, VHostName, Component,
+                                [{name, Name},
+                                 {user_who_performed_action, ActingUser}]),
+                              Mod:notify_clear(
+                                    VHostName, Component, Name, ActingUser),
+                              ok;
+                          _ ->
+                              ok
+                      end
+              end, DeletedParams),
+            ok;
+        {error, _} = Err ->
+            Err
+    end.
 
 clear_component(<<"policy">>, _) ->
     {error_string, "policies may not be cleared using this method"};
diff --git a/deps/rabbit/src/rabbit_vhost.erl b/deps/rabbit/src/rabbit_vhost.erl
@@ -272,8 +272,7 @@ delete(VHost, ActingUser) ->
     %% calls would be responsible for the atomicity, not this code.
     %% Clear the permissions first to prohibit new incoming connections when deleting a vhost
     rabbit_log:info("Clearing permissions in vhost '~ts' because it's being deleted", [VHost]),
-    _ = rabbit_auth_backend_internal:clear_permissions_for_vhost(VHost, ActingUser),
-    _ = rabbit_auth_backend_internal:clear_topic_permissions_for_vhost(VHost, ActingUser),
+    ok = rabbit_auth_backend_internal:clear_all_permissions_for_vhost(VHost, ActingUser),
     rabbit_log:info("Deleting queues in vhost '~ts' because it's being deleted", [VHost]),
     QDelFun = fun (Q) -> rabbit_amqqueue:delete(Q, false, false, ActingUser) end,
     [begin
@@ -285,22 +284,20 @@ delete(VHost, ActingUser) ->
         #exchange{name = Name} <- rabbit_exchange:list(VHost)],
     rabbit_log:info("Clearing policies and runtime parameters in vhost '~ts' because it's being deleted", [VHost]),
     _ = rabbit_runtime_parameters:clear_vhost(VHost, ActingUser),
-    _ = [rabbit_policy:delete(VHost, proplists:get_value(name, Info), ActingUser)
-         || Info <- rabbit_policy:list(VHost)],
     rabbit_log:debug("Removing vhost '~ts' from the metadata storage because it's being deleted", [VHost]),
-    case rabbit_db_vhost:delete(VHost) of
-        true ->
-            ok = rabbit_event:notify(
-                   vhost_deleted,
-                   [{name, VHost},
-                    {user_who_performed_action, ActingUser}]);
-        false ->
-            ok
-    end,
+    Ret = case rabbit_db_vhost:delete(VHost) of
+             true ->
+                 ok = rabbit_event:notify(
+                        vhost_deleted,
+                        [{name, VHost},
+                         {user_who_performed_action, ActingUser}]);
+             false ->
+                 {error, {no_such_vhost, VHost}}
+         end,
     %% After vhost was deleted from the database, we try to stop vhost
     %% supervisors on all the nodes.
     rabbit_vhost_sup_sup:delete_on_all_nodes(VHost),
-    ok.
+    Ret.
 
 -spec put_vhost(vhost:name(),
     binary(),
diff --git a/deps/rabbit/src/rabbit_vhost_limit.erl b/deps/rabbit/src/rabbit_vhost_limit.erl
@@ -39,12 +39,14 @@ validate(_VHost, <<"vhost-limits">>, Name, Term, _User) ->
 
 notify(VHost, <<"vhost-limits">>, <<"limits">>, Limits, ActingUser) ->
     rabbit_event:notify(vhost_limits_set, [{name, <<"limits">>},
+                                           {vhost, VHost},
                                            {user_who_performed_action, ActingUser}
                                            | Limits]),
     update_vhost(VHost, Limits).
 
 notify_clear(VHost, <<"vhost-limits">>, <<"limits">>, ActingUser) ->
     rabbit_event:notify(vhost_limits_cleared, [{name, <<"limits">>},
+                                               {vhost, VHost},
                                                {user_who_performed_action, ActingUser}]),
     %% If the function is called as a part of vhost deletion, the vhost can
     %% be already deleted.
diff --git a/deps/rabbit/test/vhost_SUITE.erl b/deps/rabbit/test/vhost_SUITE.erl
diff --git a/deps/rabbitmq_management/src/rabbit_mgmt_wm_vhost.erl b/deps/rabbitmq_management/src/rabbit_mgmt_wm_vhost.erl
