Merge pull request #13015 from rabbitmq/mk-virtual-host-protection-from-accidental-deletion

4.1.0: Introduce a way to protect a virtual host from deletion

Author: michaelklishin

deps/rabbit/src/rabbit_db_vhost.erl

@@ -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,
@@ -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().
 %% -------------------------------------------------------------------

deps/rabbit/src/rabbit_definitions.erl

@@ -786,8 +786,17 @@ add_vhost(VHost, ActingUser) ->
     case rabbit_vhost:put_vhost(Name, Description, Tags, DefaultQueueType, IsTracingEnabled, ActingUser) of
         ok ->
             ok;
-        {error, _} = Err ->
-            throw(Err)
+        {error, _} = Err1 ->
+            throw(Err1)
+    end,
+
+    %% The newly created virtual host won't have all the metadata keys. Rather than
+    %% changing the functions above, simply update the metadata as a separate step.
+    case rabbit_vhost:update_metadata(Name, Metadata, ActingUser) of
+        ok ->
+            ok;
+        {error, _} = Err2 ->
+            throw(Err2)
     end.
 
 add_permission(Permission, ActingUser) ->

deps/rabbit/src/rabbit_vhost.erl

@@ -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]).
@@ -253,20 +253,37 @@ declare_default_exchanges(VHostName, ActingUser) ->
       end, DefaultExchanges).
 
 -spec update_metadata(vhost:name(), vhost:metadata(), rabbit_types:username()) -> rabbit_types:ok_or_error(any()).
+update_metadata(Name, undefined, _ActingUser) ->
+    case rabbit_db_vhost:exists(Name) of
+        true ->
+            ok;
+        false ->
+            {error, {no_such_vhost, Name}}
+    end;
+update_metadata(Name, Metadata0, _ActingUser) when is_map(Metadata0) andalso map_size(Metadata0) =:= 0 ->
+    case rabbit_db_vhost:exists(Name) of
+        true ->
+            ok;
+        false ->
+            {error, {no_such_vhost, Name}}
+    end;
 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
@@ -278,45 +295,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(),
@@ -530,6 +563,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
@@ -624,6 +665,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

deps/rabbit/src/vhost.erl

@@ -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,
 
@@ -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}.
@@ -123,6 +130,7 @@ info_keys() ->
      description,
      tags,
      default_queue_type,
+     protected_from_deletion,
      metadata,
      tracing,
      cluster_state].
@@ -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,

deps/rabbit/test/definition_import_SUITE.erl

@@ -53,7 +53,8 @@ groups() ->
                                import_case18,
                                import_case19,
                                import_case20,
-                               import_case21
+                               import_case21,
+                               import_case22
                               ]},
 
         {boot_time_import_using_classic_source, [], [
@@ -273,7 +274,7 @@ import_case16(Config) ->
         Val when is_tuple(Val) ->
             ?assertEqual(<<"A case16 description">>, vhost:get_description(VHostRec)),
             ?assertEqual(<<"quorum">>, vhost:get_default_queue_type(VHostRec)),
-            ?assertEqual([multi_dc_replication,ab,cde], vhost:get_tags(VHostRec))
+            ?assertEqual([<<"multi_dc_replication">>,<<"ab">>,<<"cde">>], vhost:get_tags(VHostRec))
     end,
 
     ok.
@@ -315,6 +316,25 @@ import_case20(Config) ->
 
 import_case21(Config) -> import_invalid_file_case(Config, "failing_case21").
 
+import_case22(Config) ->
+    import_file_case(Config, "case22"),
+    Name = <<"protected">>,
+    VirtualHostIsImported =
+        fun () ->
+            case vhost_lookup(Config, Name) of
+                {error, not_found} -> false;
+                _       -> true
+            end
+        end,
+    rabbit_ct_helpers:await_condition(VirtualHostIsImported, 20000),
+    VHost = vhost_lookup(Config, Name),
+    ?assertEqual(true, vhost:is_protected_from_deletion(VHost)),
+
+    VHost2 = vhost_lookup(Config, <<"non-protected">>),
+    ?assertEqual(false, vhost:is_protected_from_deletion(VHost2)),
+
+    ok.
+
 export_import_round_trip_case1(Config) ->
     case rabbit_ct_helpers:is_mixed_versions() of
       false ->

deps/rabbit/test/definition_import_SUITE_data/case22.json

@@ -0,0 +1,66 @@
+{
+  "bindings": [],
+  "exchanges": [],
+  "global_parameters": [
+    {
+      "name": "cluster_name",
+      "value": "rabbitmq@localhost"
+    }
+  ],
+  "parameters": [],
+  "permissions": [
+    {
+      "configure": ".*",
+      "read": ".*",
+      "user": "guest",
+      "vhost": "/",
+      "write": ".*"
+    }
+  ],
+  "policies": [],
+  "queues": [],
+  "rabbit_version": "4.0.5",
+  "rabbitmq_version": "4.0.5",
+  "topic_permissions": [],
+  "users": [
+    {
+      "hashing_algorithm": "rabbit_password_hashing_sha256",
+      "limits": {"max-connections" : 2},
+      "name": "limited_guest",
+      "password_hash": "wS4AT3B4Z5RpWlFn1FA30osf2C75D7WA3gem591ACDZ6saO6",
+      "tags": [
+        "administrator"
+      ]
+    }
+  ],
+  "vhosts": [
+    {
+      "limits": [],
+      "name": "non-protected",
+      "description": "",
+      "metadata": {
+        "description": "",
+        "tags": [],
+        "default_queue_type": "classic"
+      },
+      "tags": [],
+      "default_queue_type": "classic"
+    },
+    {
+      "name": "protected",
+      "description": "protected, DQT = quorum",
+      "metadata": {
+        "description": "DQT = quorum",
+        "tags": [],
+        "default_queue_type": "quorum",
+        "protected_from_deletion": true
+      },
+      "tags": [],
+      "default_queue_type": "quorum"
+    },
+    {
+      "limits": [],
+      "name": "tagged"
+    }
+  ]
+}