Skip to content

Commit f62d46c

Browse files
Introduce a way to protect a virtual host from deletion
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.
1 parent 5981ecf commit f62d46c

17 files changed

+400
-43
lines changed

deps/rabbit/src/rabbit_db_vhost.erl

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
count_all/0,
2424
list/0,
2525
update/2,
26+
enable_protection_from_deletion/1,
27+
disable_protection_from_deletion/1,
2628
with_fun_in_mnesia_tx/2,
2729
with_fun_in_khepri_tx/2,
2830
delete/1,
@@ -224,6 +226,36 @@ set_tags_in_khepri(VHostName, Tags) ->
224226
UpdateFun = fun(VHost) -> do_set_tags(VHost, Tags) end,
225227
update_in_khepri(VHostName, UpdateFun).
226228

229+
%% -------------------------------------------------------------------
230+
%% Deletion protection
231+
%% -------------------------------------------------------------------
232+
233+
-spec enable_protection_from_deletion(VHostName) -> Ret when
234+
VHostName :: vhost:name(),
235+
Ret :: {ok, VHost} |
236+
{error, {no_such_vhost, VHostName}} |
237+
rabbit_khepri:timeout_error(),
238+
VHost :: vhost:vhost().
239+
enable_protection_from_deletion(VHostName) ->
240+
MetadataPatch = #{
241+
protected_from_deletion => true
242+
},
243+
rabbit_log:info("Enabling deletion protection for virtual host '~ts'", [VHostName]),
244+
merge_metadata(VHostName, MetadataPatch).
245+
246+
-spec disable_protection_from_deletion(VHostName) -> Ret when
247+
VHostName :: vhost:name(),
248+
Ret :: {ok, VHost} |
249+
{error, {no_such_vhost, VHostName}} |
250+
rabbit_khepri:timeout_error(),
251+
VHost :: vhost:vhost().
252+
disable_protection_from_deletion(VHostName) ->
253+
MetadataPatch = #{
254+
protected_from_deletion => false
255+
},
256+
rabbit_log:info("Disabling deletion protection for virtual host '~ts'", [VHostName]),
257+
merge_metadata(VHostName, MetadataPatch).
258+
227259
%% -------------------------------------------------------------------
228260
%% exists().
229261
%% -------------------------------------------------------------------

deps/rabbit/src/rabbit_vhost.erl

Lines changed: 49 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,11 @@
1111
-include("vhost.hrl").
1212

1313
-export([recover/0, recover/1, read_config/1]).
14-
-export([add/2, add/3, add/4, delete/2, exists/1, assert/1,
14+
-export([add/2, add/3, add/4, delete/2, delete_ignoring_protection/2, exists/1, assert/1,
1515
set_limits/2, vhost_cluster_state/1, is_running_on_all_nodes/1, await_running_on_all_nodes/2,
1616
list/0, count/0, list_names/0, all/0, all_tagged_with/1]).
1717
-export([parse_tags/1, update_tags/3]).
18-
-export([update_metadata/3]).
18+
-export([update_metadata/3, enable_protection_from_deletion/1, disable_protection_from_deletion/1]).
1919
-export([lookup/1, default_name/0]).
2020
-export([info/1, info/2, info_all/0, info_all/1, info_all/2, info_all/3]).
2121
-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]).
@@ -254,19 +254,22 @@ declare_default_exchanges(VHostName, ActingUser) ->
254254

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

259260
case rabbit_db_vhost:merge_metadata(Name, Metadata) of
260261
{ok, VHost} ->
261262
Description = vhost:get_description(VHost),
262263
Tags = vhost:get_tags(VHost),
263264
DefaultQueueType = vhost:get_default_queue_type(VHost),
265+
IsProtected = vhost:is_protected_from_deletion(VHost),
264266
rabbit_event:notify(
265267
vhost_updated,
266268
info(VHost) ++ [{user_who_performed_action, ActingUser},
267269
{description, Description},
268270
{tags, Tags},
269-
{default_queue_type, DefaultQueueType}]),
271+
{default_queue_type, DefaultQueueType},
272+
{deletion_protection, IsProtected}]),
270273
ok;
271274
{error, _} = Error ->
272275
Error
@@ -278,45 +281,61 @@ update(Name, Description, Tags, DefaultQueueType, ActingUser) ->
278281
update_metadata(Name, Metadata, ActingUser).
279282

280283
-spec delete(vhost:name(), rabbit_types:username()) -> rabbit_types:ok_or_error(any()).
281-
delete(VHost, ActingUser) ->
284+
delete(Name, ActingUser) ->
285+
case rabbit_db_vhost:get(Name) of
286+
%% preserve the original behavior for backwards compatibility
287+
undefined -> delete_ignoring_protection(Name, ActingUser);
288+
VHost ->
289+
case vhost:is_protected_from_deletion(VHost) of
290+
true ->
291+
Msg = "Refusing to delete virtual host '~ts' because it is protected from deletion",
292+
rabbit_log:debug(Msg, [Name]),
293+
{error, protected_from_deletion};
294+
false ->
295+
delete_ignoring_protection(Name, ActingUser)
296+
end
297+
end.
298+
299+
-spec delete_ignoring_protection(vhost:name(), rabbit_types:username()) -> rabbit_types:ok_or_error(any()).
300+
delete_ignoring_protection(Name, ActingUser) ->
282301
%% FIXME: We are forced to delete the queues and exchanges outside
283302
%% the TX below. Queue deletion involves sending messages to the queue
284303
%% process, which in turn results in further database actions and
285304
%% eventually the termination of that process. Exchange deletion causes
286305
%% notifications which must be sent outside the TX
287-
rabbit_log:info("Deleting vhost '~ts'", [VHost]),
306+
rabbit_log:info("Deleting vhost '~ts'", [Name]),
288307
%% TODO: This code does a lot of "list resources, walk through the list to
289308
%% delete each resource". This feature should be provided by each called
290309
%% modules, like `rabbit_amqqueue:delete_all_for_vhost(VHost)'. These new
291310
%% calls would be responsible for the atomicity, not this code.
292311
%% Clear the permissions first to prohibit new incoming connections when deleting a vhost
293-
rabbit_log:info("Clearing permissions in vhost '~ts' because it's being deleted", [VHost]),
294-
ok = rabbit_auth_backend_internal:clear_all_permissions_for_vhost(VHost, ActingUser),
295-
rabbit_log:info("Deleting queues in vhost '~ts' because it's being deleted", [VHost]),
312+
rabbit_log:info("Clearing permissions in vhost '~ts' because it's being deleted", [Name]),
313+
ok = rabbit_auth_backend_internal:clear_all_permissions_for_vhost(Name, ActingUser),
314+
rabbit_log:info("Deleting queues in vhost '~ts' because it's being deleted", [Name]),
296315
QDelFun = fun (Q) -> rabbit_amqqueue:delete(Q, false, false, ActingUser) end,
297316
[begin
298-
Name = amqqueue:get_name(Q),
299-
assert_benign(rabbit_amqqueue:with(Name, QDelFun), ActingUser)
300-
end || Q <- rabbit_amqqueue:list(VHost)],
301-
rabbit_log:info("Deleting exchanges in vhost '~ts' because it's being deleted", [VHost]),
302-
ok = rabbit_exchange:delete_all(VHost, ActingUser),
303-
rabbit_log:info("Clearing policies and runtime parameters in vhost '~ts' because it's being deleted", [VHost]),
304-
_ = rabbit_runtime_parameters:clear_vhost(VHost, ActingUser),
305-
rabbit_log:debug("Removing vhost '~ts' from the metadata storage because it's being deleted", [VHost]),
306-
Ret = case rabbit_db_vhost:delete(VHost) of
317+
QName = amqqueue:get_name(Q),
318+
assert_benign(rabbit_amqqueue:with(QName, QDelFun), ActingUser)
319+
end || Q <- rabbit_amqqueue:list(Name)],
320+
rabbit_log:info("Deleting exchanges in vhost '~ts' because it's being deleted", [Name]),
321+
ok = rabbit_exchange:delete_all(Name, ActingUser),
322+
rabbit_log:info("Clearing policies and runtime parameters in vhost '~ts' because it's being deleted", [Name]),
323+
_ = rabbit_runtime_parameters:clear_vhost(Name, ActingUser),
324+
rabbit_log:debug("Removing vhost '~ts' from the metadata storage because it's being deleted", [Name]),
325+
Ret = case rabbit_db_vhost:delete(Name) of
307326
true ->
308327
ok = rabbit_event:notify(
309328
vhost_deleted,
310-
[{name, VHost},
329+
[{name, Name},
311330
{user_who_performed_action, ActingUser}]);
312331
false ->
313-
{error, {no_such_vhost, VHost}};
332+
{error, {no_such_vhost, Name}};
314333
{error, _} = Err ->
315334
Err
316335
end,
317336
%% After vhost was deleted from the database, we try to stop vhost
318337
%% supervisors on all the nodes.
319-
rabbit_vhost_sup_sup:delete_on_all_nodes(VHost),
338+
rabbit_vhost_sup_sup:delete_on_all_nodes(Name),
320339
Ret.
321340

322341
-spec put_vhost(vhost:name(),
@@ -530,6 +549,14 @@ lookup(VHostName) ->
530549
VHost -> VHost
531550
end.
532551

552+
-spec enable_protection_from_deletion(vhost:name()) -> vhost:vhost() | rabbit_types:ok_or_error(any()).
553+
enable_protection_from_deletion(VHostName) ->
554+
rabbit_db_vhost:enable_protection_from_deletion(VHostName).
555+
556+
-spec disable_protection_from_deletion(vhost:name()) -> vhost:vhost() | rabbit_types:ok_or_error(any()).
557+
disable_protection_from_deletion(VHostName) ->
558+
rabbit_db_vhost:disable_protection_from_deletion(VHostName).
559+
533560
-spec assert(vhost:name()) -> 'ok'.
534561
assert(VHostName) ->
535562
case exists(VHostName) of
@@ -624,6 +651,7 @@ i(cluster_state, VHost) -> vhost_cluster_state(vhost:get_name(VHost));
624651
i(description, VHost) -> vhost:get_description(VHost);
625652
i(tags, VHost) -> vhost:get_tags(VHost);
626653
i(default_queue_type, VHost) -> rabbit_queue_type:short_alias_of(default_queue_type(vhost:get_name(VHost)));
654+
i(protected_from_deletion, VHost) -> vhost:is_protected_from_deletion(VHost);
627655
i(metadata, VHost) ->
628656
DQT = rabbit_queue_type:short_alias_of(default_queue_type(vhost:get_name(VHost))),
629657
case vhost:get_metadata(VHost) of

deps/rabbit/src/vhost.erl

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,11 @@
3030
set_limits/2,
3131
set_metadata/2,
3232
merge_metadata/2,
33+
34+
is_protected_from_deletion/1,
35+
enable_protection_from_deletion/1,
36+
disable_protection_from_deletion/1,
37+
3338
new_metadata/3,
3439
is_tagged_with/2,
3540

@@ -89,6 +94,8 @@
8994
vhost_pattern/0,
9095
vhost_v2_pattern/0]).
9196

97+
-define(DELETION_PROTECTION_KEY, protected_from_deletion).
98+
9299
-spec new(name(), limits()) -> vhost().
93100
new(Name, Limits) ->
94101
#vhost{virtual_host = Name, limits = Limits}.
@@ -123,6 +130,7 @@ info_keys() ->
123130
description,
124131
tags,
125132
default_queue_type,
133+
protected_from_deletion,
126134
metadata,
127135
tracing,
128136
cluster_state].
@@ -187,6 +195,23 @@ metadata_merger(default_queue_type, _, NewVHostDefaultQueueType) ->
187195
metadata_merger(_, _, NewMetadataValue) ->
188196
NewMetadataValue.
189197

198+
-spec is_protected_from_deletion(vhost()) -> boolean().
199+
is_protected_from_deletion(VHost) ->
200+
case get_metadata(VHost) of
201+
Map when map_size(Map) =:= 0 -> false;
202+
#{?DELETION_PROTECTION_KEY := true} -> true;
203+
#{?DELETION_PROTECTION_KEY := false} -> false;
204+
_ -> false
205+
end.
206+
207+
-spec enable_protection_from_deletion(vhost()) -> vhost().
208+
enable_protection_from_deletion(VHost) ->
209+
merge_metadata(VHost, #{?DELETION_PROTECTION_KEY => true}).
210+
211+
-spec disable_protection_from_deletion(vhost()) -> vhost().
212+
disable_protection_from_deletion(VHost) ->
213+
merge_metadata(VHost, #{?DELETION_PROTECTION_KEY => false}).
214+
190215
-spec new_metadata(binary(), [atom()], rabbit_queue_type:queue_type() | 'undefined') -> metadata().
191216
new_metadata(Description, Tags, undefined) ->
192217
#{description => Description,

deps/rabbitmq_cli/lib/rabbitmq/cli/ctl/commands/delete_vhost_command.ex

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,12 @@
55
## Copyright (c) 2007-2023 Broadcom. All Rights Reserved. The term “Broadcom” refers to Broadcom Inc. and/or its subsidiaries. All rights reserved.
66

77
defmodule RabbitMQ.CLI.Ctl.Commands.DeleteVhostCommand do
8-
alias RabbitMQ.CLI.Core.{DocGuide, Helpers}
8+
alias RabbitMQ.CLI.Core.{DocGuide, Helpers, ExitCodes}
99

1010
@behaviour RabbitMQ.CLI.CommandBehaviour
1111

12+
@protected_from_deletion_err "Cannot delete this virtual host: it is protected from deletion. To lift the protection, inspect and update its metadata"
13+
1214
use RabbitMQ.CLI.Core.MergesNoDefaults
1315
use RabbitMQ.CLI.Core.AcceptsOnePositionalArgument
1416
use RabbitMQ.CLI.Core.RequiresRabbitAppRunning
@@ -17,6 +19,12 @@ defmodule RabbitMQ.CLI.Ctl.Commands.DeleteVhostCommand do
1719
:rabbit_misc.rpc_call(node_name, :rabbit_vhost, :delete, [arg, Helpers.cli_acting_user()])
1820
end
1921

22+
def output({:error, :protected_from_deletion}, %{formatter: "json", node: node_name}) do
23+
{:error, %{"result" => "error", "node" => node_name, "reason" => @protected_from_deletion_err}}
24+
end
25+
def output({:error, :protected_from_deletion}, _opts) do
26+
{:error, ExitCodes.exit_dataerr(), @protected_from_deletion_err}
27+
end
2028
use RabbitMQ.CLI.DefaultOutput
2129

2230
def usage, do: "delete_vhost <vhost>"

deps/rabbitmq_cli/lib/rabbitmq/cli/ctl/commands/list_vhosts_command.ex

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ defmodule RabbitMQ.CLI.Ctl.Commands.ListVhostsCommand do
1111
@behaviour RabbitMQ.CLI.CommandBehaviour
1212
use RabbitMQ.CLI.DefaultOutput
1313

14-
@info_keys ~w(name description tags default_queue_type tracing cluster_state)a
14+
@info_keys ~w(name description tags default_queue_type protected_from_deletion tracing cluster_state metadata)a
1515

1616
def info_keys(), do: @info_keys
1717

deps/rabbitmq_cli/test/ctl/update_vhost_metadata_command_test.exs

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
## License, v. 2.0. If a copy of the MPL was not distributed with this
33
## file, You can obtain one at https://mozilla.org/MPL/2.0/.
44
##
5-
## Copyright (c) 2007-2023 Broadcom. All Rights Reserved. The term “Broadcom” refers to Broadcom Inc. and/or its subsidiaries. All rights reserved.
5+
## Copyright (c) 2007-2024 Broadcom. All Rights Reserved. The term “Broadcom” refers to Broadcom Inc. and/or its subsidiaries. All rights reserved.
66

77
defmodule UpdateVhostMetadataCommandTest do
88
use ExUnit.Case, async: false
@@ -81,6 +81,24 @@ defmodule UpdateVhostMetadataCommandTest do
8181
assert vh[:tags] == [:a1, :b2, :c3]
8282
end
8383

84+
test "run: enabling and disabling deletion protection succeeds", context do
85+
add_vhost(@vhost)
86+
87+
opts = Map.merge(context[:opts], %{description: "Protected from deletion", protected_from_deletion: true})
88+
assert @command.run([@vhost], opts) == :ok
89+
vh = find_vhost(@vhost)
90+
assert vh[:tags] == [:my_tag]
91+
end
92+
93+
test "run: vhost tags are coerced to a list", context do
94+
add_vhost(@vhost)
95+
96+
opts = Map.merge(context[:opts], %{description: "My vhost", tags: "my_tag"})
97+
assert @command.run([@vhost], opts) == :ok
98+
vh = find_vhost(@vhost)
99+
assert vh[:tags] == [:my_tag]
100+
end
101+
84102
test "run: attempt to use a non-existent virtual host fails", context do
85103
vh = "a-non-existent-3882-vhost"
86104

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

98-
test "run: vhost tags are coerced to a list", context do
99-
add_vhost(@vhost)
100-
101-
opts = Map.merge(context[:opts], %{description: "My vhost", tags: "my_tag"})
102-
assert @command.run([@vhost], opts) == :ok
103-
vh = find_vhost(@vhost)
104-
assert vh[:tags] == [:my_tag]
105-
end
106116

107117
test "banner", context do
108118
assert @command.banner([@vhost], context[:opts]) =~

deps/rabbitmq_ct_helpers/src/rabbit_ct_broker_helpers.erl

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,8 @@
112112
add_vhost/3,
113113
add_vhost/4,
114114
update_vhost_metadata/3,
115+
enable_vhost_protection_from_deletion/2,
116+
disable_vhost_protection_from_deletion/2,
115117
delete_vhost/2,
116118
delete_vhost/3,
117119
delete_vhost/4,
@@ -1606,6 +1608,18 @@ add_vhost(Config, Node, VHost, Username) ->
16061608
update_vhost_metadata(Config, VHost, Meta) ->
16071609
catch rpc(Config, 0, rabbit_vhost, update_metadata, [VHost, Meta, <<"acting-user">>]).
16081610

1611+
enable_vhost_protection_from_deletion(Config, VHost) ->
1612+
MetadataPatch = #{
1613+
protected_from_deletion => true
1614+
},
1615+
update_vhost_metadata(Config, VHost, MetadataPatch).
1616+
1617+
disable_vhost_protection_from_deletion(Config, VHost) ->
1618+
MetadataPatch = #{
1619+
protected_from_deletion => false
1620+
},
1621+
update_vhost_metadata(Config, VHost, MetadataPatch).
1622+
16091623
delete_vhost(Config, VHost) ->
16101624
delete_vhost(Config, 0, VHost).
16111625

deps/rabbitmq_ct_helpers/src/rabbit_mgmt_test_util.erl

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,10 @@ http_post(Config, Path, List, CodeExp) ->
6868
http_post(Config, Path, List, User, Pass, CodeExp) ->
6969
http_post_raw(Config, Path, format_for_upload(List), User, Pass, CodeExp).
7070

71+
http_post_json(Config, Path, Body, Assertion) ->
72+
http_upload_raw(Config, post, Path, Body, "guest", "guest",
73+
Assertion, [{"content-type", "application/json"}]).
74+
7175
http_post_accept_json(Config, Path, List, CodeExp) ->
7276
http_post_accept_json(Config, Path, List, "guest", "guest", CodeExp).
7377

deps/rabbitmq_management/priv/www/api/index.html

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -723,6 +723,14 @@ <h2>Reference</h2>
723723
<td class="path">/api/vhosts/<i>name</i>/topic-permissions</td>
724724
<td>A list of all topic permissions for a given virtual host.</td>
725725
</tr>
726+
<tr>
727+
<td></td>
728+
<td></td>
729+
<td>X</td>
730+
<td>X</td>
731+
<td class="path">/api/vhosts/<i>name</i>/deletion/protection</td>
732+
<td>Enables (when used with POST) or disabled (with DELETE) deletion protection for the virtual host.</td>
733+
</tr>
726734
<tr>
727735
<td></td>
728736
<td></td>

deps/rabbitmq_management/src/rabbit_mgmt_dispatcher.erl

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,7 @@ dispatcher() ->
162162
{"/bindings/:vhost/e/:source/:dtype/:destination/:props", rabbit_mgmt_wm_binding, []},
163163
{"/vhosts", rabbit_mgmt_wm_vhosts, []},
164164
{"/vhosts/:vhost", rabbit_mgmt_wm_vhost, []},
165+
{"/vhosts/:vhost/deletion/protection", rabbit_mgmt_wm_vhost_deletion_protection, []},
165166
{"/vhosts/:vhost/start/:node", rabbit_mgmt_wm_vhost_restart, []},
166167
{"/vhosts/:vhost/permissions", rabbit_mgmt_wm_permissions_vhost, []},
167168
{"/vhosts/:vhost/topic-permissions", rabbit_mgmt_wm_topic_permissions_vhost, []},

0 commit comments

Comments
 (0)