Skip to content

Commit

Permalink
Introduce a way to protect a virtual host from deletion
Browse files Browse the repository at this point in the history
Accidental "fat finger" virtual deletion accidents
would be easier to avoid if there was a protection mechanism
that would apply equally even to CLI tools and external
applications that do not use confirmations for deletion
operations.

This introduce the following changes:

 * Virtual host metadata now supports a new queue,
   'protected_from_deletion', which, when set,
   will be considered by key virtual host deletion function(s)
 * DELETE /api/vhosts/{name} was adapted to handle
   such blocked deletion attempts to respond with
   a 412 Precondition Failed status
 * 'rabbitmqctl list_vhosts' and 'rabbitmqctl delete_vhost'
   were adapted accordingly
 * DELETE /api/vhosts/{name}/deletion/protection
   is a new endpoint that can be used to remove
   the protective seal (the metadata key)
 * POST /api/vhosts/{name}/deletion/protection
   marks the virtual host as protected

In the case of the HTTP API, all operations on
virtual host metadata require administrative
privileges from the target user.

Other considerations:

 * When a virtual host does not exist, the behavior
  remains the same: the original, protection-unaware
  code path is used to preserve backwards compatibility

References #12772.

(cherry picked from commit f62d46c)
  • Loading branch information
michaelklishin authored and mergify[bot] committed Jan 4, 2025
1 parent baf3e31 commit 97183e8
Show file tree
Hide file tree
Showing 17 changed files with 400 additions and 43 deletions.
32 changes: 32 additions & 0 deletions deps/rabbit/src/rabbit_db_vhost.erl
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
count_all/0,
list/0,
update/2,
enable_protection_from_deletion/1,
disable_protection_from_deletion/1,
with_fun_in_mnesia_tx/2,
with_fun_in_khepri_tx/2,
delete/1,
Expand Down Expand Up @@ -224,6 +226,36 @@ set_tags_in_khepri(VHostName, Tags) ->
UpdateFun = fun(VHost) -> do_set_tags(VHost, Tags) end,
update_in_khepri(VHostName, UpdateFun).

%% -------------------------------------------------------------------
%% Deletion protection
%% -------------------------------------------------------------------

-spec enable_protection_from_deletion(VHostName) -> Ret when
VHostName :: vhost:name(),
Ret :: {ok, VHost} |
{error, {no_such_vhost, VHostName}} |
rabbit_khepri:timeout_error(),
VHost :: vhost:vhost().
enable_protection_from_deletion(VHostName) ->
MetadataPatch = #{
protected_from_deletion => true
},
rabbit_log:info("Enabling deletion protection for virtual host '~ts'", [VHostName]),
merge_metadata(VHostName, MetadataPatch).

-spec disable_protection_from_deletion(VHostName) -> Ret when
VHostName :: vhost:name(),
Ret :: {ok, VHost} |
{error, {no_such_vhost, VHostName}} |
rabbit_khepri:timeout_error(),
VHost :: vhost:vhost().
disable_protection_from_deletion(VHostName) ->
MetadataPatch = #{
protected_from_deletion => false
},
rabbit_log:info("Disabling deletion protection for virtual host '~ts'", [VHostName]),
merge_metadata(VHostName, MetadataPatch).

%% -------------------------------------------------------------------
%% exists().
%% -------------------------------------------------------------------
Expand Down
70 changes: 49 additions & 21 deletions deps/rabbit/src/rabbit_vhost.erl
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@
-include("vhost.hrl").

-export([recover/0, recover/1, read_config/1]).
-export([add/2, add/3, add/4, delete/2, exists/1, assert/1,
-export([add/2, add/3, add/4, delete/2, delete_ignoring_protection/2, exists/1, assert/1,
set_limits/2, vhost_cluster_state/1, is_running_on_all_nodes/1, await_running_on_all_nodes/2,
list/0, count/0, list_names/0, all/0, all_tagged_with/1]).
-export([parse_tags/1, update_tags/3]).
-export([update_metadata/3]).
-export([update_metadata/3, enable_protection_from_deletion/1, disable_protection_from_deletion/1]).
-export([lookup/1, default_name/0]).
-export([info/1, info/2, info_all/0, info_all/1, info_all/2, info_all/3]).
-export([dir/1, msg_store_dir_path/1, msg_store_dir_wildcard/0, msg_store_dir_base/0, config_file_path/1, ensure_config_file/1]).
Expand Down Expand Up @@ -254,19 +254,22 @@ declare_default_exchanges(VHostName, ActingUser) ->

-spec update_metadata(vhost:name(), vhost:metadata(), rabbit_types:username()) -> rabbit_types:ok_or_error(any()).
update_metadata(Name, Metadata0, ActingUser) ->
Metadata = maps:with([description, tags, default_queue_type], Metadata0),
KnownKeys = [description, tags, default_queue_type, protected_from_deletion],
Metadata = maps:with(KnownKeys, Metadata0),

case rabbit_db_vhost:merge_metadata(Name, Metadata) of
{ok, VHost} ->
Description = vhost:get_description(VHost),
Tags = vhost:get_tags(VHost),
DefaultQueueType = vhost:get_default_queue_type(VHost),
IsProtected = vhost:is_protected_from_deletion(VHost),
rabbit_event:notify(
vhost_updated,
info(VHost) ++ [{user_who_performed_action, ActingUser},
{description, Description},
{tags, Tags},
{default_queue_type, DefaultQueueType}]),
{default_queue_type, DefaultQueueType},
{deletion_protection, IsProtected}]),
ok;
{error, _} = Error ->
Error
Expand All @@ -278,45 +281,61 @@ update(Name, Description, Tags, DefaultQueueType, ActingUser) ->
update_metadata(Name, Metadata, ActingUser).

-spec delete(vhost:name(), rabbit_types:username()) -> rabbit_types:ok_or_error(any()).
delete(VHost, ActingUser) ->
delete(Name, ActingUser) ->
case rabbit_db_vhost:get(Name) of
%% preserve the original behavior for backwards compatibility
undefined -> delete_ignoring_protection(Name, ActingUser);
VHost ->
case vhost:is_protected_from_deletion(VHost) of
true ->
Msg = "Refusing to delete virtual host '~ts' because it is protected from deletion",
rabbit_log:debug(Msg, [Name]),
{error, protected_from_deletion};
false ->
delete_ignoring_protection(Name, ActingUser)
end
end.

-spec delete_ignoring_protection(vhost:name(), rabbit_types:username()) -> rabbit_types:ok_or_error(any()).
delete_ignoring_protection(Name, ActingUser) ->
%% FIXME: We are forced to delete the queues and exchanges outside
%% the TX below. Queue deletion involves sending messages to the queue
%% process, which in turn results in further database actions and
%% eventually the termination of that process. Exchange deletion causes
%% notifications which must be sent outside the TX
rabbit_log:info("Deleting vhost '~ts'", [VHost]),
rabbit_log:info("Deleting vhost '~ts'", [Name]),
%% TODO: This code does a lot of "list resources, walk through the list to
%% delete each resource". This feature should be provided by each called
%% modules, like `rabbit_amqqueue:delete_all_for_vhost(VHost)'. These new
%% 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]),
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]),
rabbit_log:info("Clearing permissions in vhost '~ts' because it's being deleted", [Name]),
ok = rabbit_auth_backend_internal:clear_all_permissions_for_vhost(Name, ActingUser),
rabbit_log:info("Deleting queues in vhost '~ts' because it's being deleted", [Name]),
QDelFun = fun (Q) -> rabbit_amqqueue:delete(Q, false, false, ActingUser) end,
[begin
Name = amqqueue:get_name(Q),
assert_benign(rabbit_amqqueue:with(Name, QDelFun), ActingUser)
end || Q <- rabbit_amqqueue:list(VHost)],
rabbit_log:info("Deleting exchanges in vhost '~ts' because it's being deleted", [VHost]),
ok = rabbit_exchange:delete_all(VHost, ActingUser),
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_log:debug("Removing vhost '~ts' from the metadata storage because it's being deleted", [VHost]),
Ret = case rabbit_db_vhost:delete(VHost) of
QName = amqqueue:get_name(Q),
assert_benign(rabbit_amqqueue:with(QName, QDelFun), ActingUser)
end || Q <- rabbit_amqqueue:list(Name)],
rabbit_log:info("Deleting exchanges in vhost '~ts' because it's being deleted", [Name]),
ok = rabbit_exchange:delete_all(Name, ActingUser),
rabbit_log:info("Clearing policies and runtime parameters in vhost '~ts' because it's being deleted", [Name]),
_ = rabbit_runtime_parameters:clear_vhost(Name, ActingUser),
rabbit_log:debug("Removing vhost '~ts' from the metadata storage because it's being deleted", [Name]),
Ret = case rabbit_db_vhost:delete(Name) of
true ->
ok = rabbit_event:notify(
vhost_deleted,
[{name, VHost},
[{name, Name},
{user_who_performed_action, ActingUser}]);
false ->
{error, {no_such_vhost, VHost}};
{error, {no_such_vhost, Name}};
{error, _} = Err ->
Err
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),
rabbit_vhost_sup_sup:delete_on_all_nodes(Name),
Ret.

-spec put_vhost(vhost:name(),
Expand Down Expand Up @@ -530,6 +549,14 @@ lookup(VHostName) ->
VHost -> VHost
end.

-spec enable_protection_from_deletion(vhost:name()) -> vhost:vhost() | rabbit_types:ok_or_error(any()).
enable_protection_from_deletion(VHostName) ->
rabbit_db_vhost:enable_protection_from_deletion(VHostName).

-spec disable_protection_from_deletion(vhost:name()) -> vhost:vhost() | rabbit_types:ok_or_error(any()).
disable_protection_from_deletion(VHostName) ->
rabbit_db_vhost:disable_protection_from_deletion(VHostName).

-spec assert(vhost:name()) -> 'ok'.
assert(VHostName) ->
case exists(VHostName) of
Expand Down Expand Up @@ -624,6 +651,7 @@ i(cluster_state, VHost) -> vhost_cluster_state(vhost:get_name(VHost));
i(description, VHost) -> vhost:get_description(VHost);
i(tags, VHost) -> vhost:get_tags(VHost);
i(default_queue_type, VHost) -> rabbit_queue_type:short_alias_of(default_queue_type(vhost:get_name(VHost)));
i(protected_from_deletion, VHost) -> vhost:is_protected_from_deletion(VHost);
i(metadata, VHost) ->
DQT = rabbit_queue_type:short_alias_of(default_queue_type(vhost:get_name(VHost))),
case vhost:get_metadata(VHost) of
Expand Down
25 changes: 25 additions & 0 deletions deps/rabbit/src/vhost.erl
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,11 @@
set_limits/2,
set_metadata/2,
merge_metadata/2,

is_protected_from_deletion/1,
enable_protection_from_deletion/1,
disable_protection_from_deletion/1,

new_metadata/3,
is_tagged_with/2,

Expand Down Expand Up @@ -89,6 +94,8 @@
vhost_pattern/0,
vhost_v2_pattern/0]).

-define(DELETION_PROTECTION_KEY, protected_from_deletion).

-spec new(name(), limits()) -> vhost().
new(Name, Limits) ->
#vhost{virtual_host = Name, limits = Limits}.
Expand Down Expand Up @@ -123,6 +130,7 @@ info_keys() ->
description,
tags,
default_queue_type,
protected_from_deletion,
metadata,
tracing,
cluster_state].
Expand Down Expand Up @@ -187,6 +195,23 @@ metadata_merger(default_queue_type, _, NewVHostDefaultQueueType) ->
metadata_merger(_, _, NewMetadataValue) ->
NewMetadataValue.

-spec is_protected_from_deletion(vhost()) -> boolean().
is_protected_from_deletion(VHost) ->
case get_metadata(VHost) of
Map when map_size(Map) =:= 0 -> false;
#{?DELETION_PROTECTION_KEY := true} -> true;
#{?DELETION_PROTECTION_KEY := false} -> false;
_ -> false
end.

-spec enable_protection_from_deletion(vhost()) -> vhost().
enable_protection_from_deletion(VHost) ->
merge_metadata(VHost, #{?DELETION_PROTECTION_KEY => true}).

-spec disable_protection_from_deletion(vhost()) -> vhost().
disable_protection_from_deletion(VHost) ->
merge_metadata(VHost, #{?DELETION_PROTECTION_KEY => false}).

-spec new_metadata(binary(), [atom()], rabbit_queue_type:queue_type() | 'undefined') -> metadata().
new_metadata(Description, Tags, undefined) ->
#{description => Description,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@
## Copyright (c) 2007-2023 Broadcom. All Rights Reserved. The term “Broadcom” refers to Broadcom Inc. and/or its subsidiaries. All rights reserved.

defmodule RabbitMQ.CLI.Ctl.Commands.DeleteVhostCommand do
alias RabbitMQ.CLI.Core.{DocGuide, Helpers}
alias RabbitMQ.CLI.Core.{DocGuide, Helpers, ExitCodes}

@behaviour RabbitMQ.CLI.CommandBehaviour

@protected_from_deletion_err "Cannot delete this virtual host: it is protected from deletion. To lift the protection, inspect and update its metadata"

use RabbitMQ.CLI.Core.MergesNoDefaults
use RabbitMQ.CLI.Core.AcceptsOnePositionalArgument
use RabbitMQ.CLI.Core.RequiresRabbitAppRunning
Expand All @@ -17,6 +19,12 @@ defmodule RabbitMQ.CLI.Ctl.Commands.DeleteVhostCommand do
:rabbit_misc.rpc_call(node_name, :rabbit_vhost, :delete, [arg, Helpers.cli_acting_user()])
end

def output({:error, :protected_from_deletion}, %{formatter: "json", node: node_name}) do
{:error, %{"result" => "error", "node" => node_name, "reason" => @protected_from_deletion_err}}
end
def output({:error, :protected_from_deletion}, _opts) do
{:error, ExitCodes.exit_dataerr(), @protected_from_deletion_err}
end
use RabbitMQ.CLI.DefaultOutput

def usage, do: "delete_vhost <vhost>"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ defmodule RabbitMQ.CLI.Ctl.Commands.ListVhostsCommand do
@behaviour RabbitMQ.CLI.CommandBehaviour
use RabbitMQ.CLI.DefaultOutput

@info_keys ~w(name description tags default_queue_type tracing cluster_state)a
@info_keys ~w(name description tags default_queue_type protected_from_deletion tracing cluster_state metadata)a

def info_keys(), do: @info_keys

Expand Down
28 changes: 19 additions & 9 deletions deps/rabbitmq_cli/test/ctl/update_vhost_metadata_command_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
## License, v. 2.0. If a copy of the MPL was not distributed with this
## file, You can obtain one at https://mozilla.org/MPL/2.0/.
##
## Copyright (c) 2007-2023 Broadcom. All Rights Reserved. The term “Broadcom” refers to Broadcom Inc. and/or its subsidiaries. All rights reserved.
## Copyright (c) 2007-2024 Broadcom. All Rights Reserved. The term “Broadcom” refers to Broadcom Inc. and/or its subsidiaries. All rights reserved.

defmodule UpdateVhostMetadataCommandTest do
use ExUnit.Case, async: false
Expand Down Expand Up @@ -81,6 +81,24 @@ defmodule UpdateVhostMetadataCommandTest do
assert vh[:tags] == [:a1, :b2, :c3]
end

test "run: enabling and disabling deletion protection succeeds", context do
add_vhost(@vhost)

opts = Map.merge(context[:opts], %{description: "Protected from deletion", protected_from_deletion: true})
assert @command.run([@vhost], opts) == :ok
vh = find_vhost(@vhost)
assert vh[:tags] == [:my_tag]
end

test "run: vhost tags are coerced to a list", context do
add_vhost(@vhost)

opts = Map.merge(context[:opts], %{description: "My vhost", tags: "my_tag"})
assert @command.run([@vhost], opts) == :ok
vh = find_vhost(@vhost)
assert vh[:tags] == [:my_tag]
end

test "run: attempt to use a non-existent virtual host fails", context do
vh = "a-non-existent-3882-vhost"

Expand All @@ -95,14 +113,6 @@ defmodule UpdateVhostMetadataCommandTest do
assert match?({:badrpc, _}, @command.run(["na"], opts))
end

test "run: vhost tags are coerced to a list", context do
add_vhost(@vhost)

opts = Map.merge(context[:opts], %{description: "My vhost", tags: "my_tag"})
assert @command.run([@vhost], opts) == :ok
vh = find_vhost(@vhost)
assert vh[:tags] == [:my_tag]
end

test "banner", context do
assert @command.banner([@vhost], context[:opts]) =~
Expand Down
14 changes: 14 additions & 0 deletions deps/rabbitmq_ct_helpers/src/rabbit_ct_broker_helpers.erl
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,8 @@
add_vhost/3,
add_vhost/4,
update_vhost_metadata/3,
enable_vhost_protection_from_deletion/2,
disable_vhost_protection_from_deletion/2,
delete_vhost/2,
delete_vhost/3,
delete_vhost/4,
Expand Down Expand Up @@ -1606,6 +1608,18 @@ add_vhost(Config, Node, VHost, Username) ->
update_vhost_metadata(Config, VHost, Meta) ->
catch rpc(Config, 0, rabbit_vhost, update_metadata, [VHost, Meta, <<"acting-user">>]).

enable_vhost_protection_from_deletion(Config, VHost) ->
MetadataPatch = #{
protected_from_deletion => true
},
update_vhost_metadata(Config, VHost, MetadataPatch).

disable_vhost_protection_from_deletion(Config, VHost) ->
MetadataPatch = #{
protected_from_deletion => false
},
update_vhost_metadata(Config, VHost, MetadataPatch).

delete_vhost(Config, VHost) ->
delete_vhost(Config, 0, VHost).

Expand Down
4 changes: 4 additions & 0 deletions deps/rabbitmq_ct_helpers/src/rabbit_mgmt_test_util.erl
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,10 @@ http_post(Config, Path, List, CodeExp) ->
http_post(Config, Path, List, User, Pass, CodeExp) ->
http_post_raw(Config, Path, format_for_upload(List), User, Pass, CodeExp).

http_post_json(Config, Path, Body, Assertion) ->
http_upload_raw(Config, post, Path, Body, "guest", "guest",
Assertion, [{"content-type", "application/json"}]).

http_post_accept_json(Config, Path, List, CodeExp) ->
http_post_accept_json(Config, Path, List, "guest", "guest", CodeExp).

Expand Down
8 changes: 8 additions & 0 deletions deps/rabbitmq_management/priv/www/api/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -723,6 +723,14 @@ <h2>Reference</h2>
<td class="path">/api/vhosts/<i>name</i>/topic-permissions</td>
<td>A list of all topic permissions for a given virtual host.</td>
</tr>
<tr>
<td></td>
<td></td>
<td>X</td>
<td>X</td>
<td class="path">/api/vhosts/<i>name</i>/deletion/protection</td>
<td>Enables (when used with POST) or disabled (with DELETE) deletion protection for the virtual host.</td>
</tr>
<tr>
<td></td>
<td></td>
Expand Down
1 change: 1 addition & 0 deletions deps/rabbitmq_management/src/rabbit_mgmt_dispatcher.erl
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ dispatcher() ->
{"/bindings/:vhost/e/:source/:dtype/:destination/:props", rabbit_mgmt_wm_binding, []},
{"/vhosts", rabbit_mgmt_wm_vhosts, []},
{"/vhosts/:vhost", rabbit_mgmt_wm_vhost, []},
{"/vhosts/:vhost/deletion/protection", rabbit_mgmt_wm_vhost_deletion_protection, []},
{"/vhosts/:vhost/start/:node", rabbit_mgmt_wm_vhost_restart, []},
{"/vhosts/:vhost/permissions", rabbit_mgmt_wm_permissions_vhost, []},
{"/vhosts/:vhost/topic-permissions", rabbit_mgmt_wm_topic_permissions_vhost, []},
Expand Down
Loading

0 comments on commit 97183e8

Please sign in to comment.