Merge pull request #5435 from rabbitmq/mergify/bp/v3.11.x/pr-5236

Remove pre-virtual_host_metadata feature flag compatibility code (backport #5236)

9 files changed, 71 insertions, 274 deletions

diff --git a/deps/rabbit/include/vhost.hrl b/deps/rabbit/include/vhost.hrl
index d3abc0dd2a..4f9b4aeae7 100644
--- a/deps/rabbit/include/vhost.hrl
+++ b/deps/rabbit/include/vhost.hrl
@@ -1,6 +1,4 @@
--include("vhost_v1.hrl").
 -include("vhost_v2.hrl").
 
 -define(is_vhost(V),
-        (?is_vhost_v2(V) orelse
-         ?is_vhost_v1(V))).
+        (?is_vhost_v2(V))).
diff --git a/deps/rabbit/include/vhost_v1.hrl b/deps/rabbit/include/vhost_v1.hrl
deleted file mode 100644
index 185739c6be..0000000000
--- a/deps/rabbit/include/vhost_v1.hrl
+++ /dev/null
@@ -1,4 +0,0 @@
--define(is_vhost_v1(V), is_record(V, vhost, 3)).
-
--define(vhost_v1_field_name(V), element(2, V)).
--define(vhost_v1_field_limits(V), element(3, V)).
diff --git a/deps/rabbit/src/vhost.erl b/deps/rabbit/src/vhost.erl
index d6d7e8aac7..cdd3ba3ce5 100644
--- a/deps/rabbit/src/vhost.erl
+++ b/deps/rabbit/src/vhost.erl
@@ -42,7 +42,7 @@
                       tags => [atom()],
                       metadata_key() => any()} | undefined).
 
--type vhost() :: vhost_v1:vhost_v1() | vhost_v2().
+-type vhost() :: vhost_v2().
 
 -record(vhost, {
     %% name as a binary
@@ -58,8 +58,7 @@
                           metadata :: metadata()
                          }.
 
--type vhost_pattern() :: vhost_v1:vhost_v1_pattern() |
-                         vhost_v2_pattern().
+-type vhost_pattern() :: vhost_v2_pattern().
 -type vhost_v2_pattern() :: #vhost{
                                   virtual_host :: name() | '_',
                                   limits :: '_',
@@ -76,104 +75,63 @@
 
 -spec new(name(), list()) -> vhost().
 new(Name, Limits) ->
-    case record_version_to_use() of
-        ?record_version ->
-          #vhost{virtual_host = Name, limits = Limits};
-        _ ->
-          vhost_v1:new(Name, Limits)
-    end.
+    #vhost{virtual_host = Name, limits = Limits}.
 
 -spec new(name(), list(), map()) -> vhost().
 new(Name, Limits, Metadata) ->
-    case record_version_to_use() of
-        ?record_version ->
-          #vhost{virtual_host = Name, limits = Limits, metadata = Metadata};
-        _ ->
-          vhost_v1:new(Name, Limits)
-    end.
+    #vhost{virtual_host = Name, limits = Limits, metadata = Metadata}.
 
--spec record_version_to_use() -> vhost_v1 | vhost_v2.
+-spec record_version_to_use() -> vhost_v2.
 
 record_version_to_use() ->
-    case rabbit_feature_flags:is_enabled(virtual_host_metadata) of
-        true  -> ?record_version;
-        false -> vhost_v1:record_version_to_use()
-    end.
+    ?record_version.
 
 -spec upgrade(vhost()) -> vhost().
 
-upgrade(#vhost{} = VHost) -> VHost;
-upgrade(OldVHost)         -> upgrade_to(record_version_to_use(), OldVHost).
+upgrade(#vhost{} = VHost) -> VHost.
 
--spec upgrade_to
-(vhost_v2, vhost()) -> vhost_v2();
-(vhost_v1, vhost_v1:vhost_v1()) -> vhost_v1:vhost_v1().
+-spec upgrade_to(vhost_v2, vhost()) -> vhost_v2().
 
 upgrade_to(?record_version, #vhost{} = VHost) ->
-    VHost;
-upgrade_to(?record_version, OldVHost) ->
-    Fields = erlang:tuple_to_list(OldVHost) ++ [#{description => <<"">>, tags => []}],
-    #vhost{} = erlang:list_to_tuple(Fields);
-upgrade_to(Version, OldVHost) ->
-    vhost_v1:upgrade_to(Version, OldVHost).
-
+    VHost.
 
 fields() ->
-    case record_version_to_use() of
-        ?record_version -> fields(?record_version);
-        _               -> vhost_v1:fields()
-    end.
+    fields(?record_version).
 
-fields(?record_version) -> record_info(fields, vhost);
-fields(Version)         -> vhost_v1:fields(Version).
+fields(?record_version) -> record_info(fields, vhost).
 
 info_keys() ->
-    case record_version_to_use() of
-        %% note: this reports description and tags separately even though
-        %% they are stored in the metadata map. MK.
-        ?record_version ->
-            [name,
-             description,
-             tags,
-             default_queue_type,
-             metadata,
-             tracing,
-             cluster_state];
-        _ ->
-            vhost_v1:info_keys()
-    end.
+    %% note: this reports description and tags separately even though
+    %% they are stored in the metadata map. MK.
+    [name,
+     description,
+     tags,
+     default_queue_type,
+     metadata,
+     tracing,
+     cluster_state].
 
 -spec pattern_match_all() -> vhost_pattern().
 
 pattern_match_all() ->
-    case record_version_to_use() of
-        ?record_version -> #vhost{_ = '_'};
-        _               -> vhost_v1:pattern_match_all()
-    end.
+    #vhost{_ = '_'}.
 
 -spec get_name(vhost()) -> name().
-get_name(#vhost{virtual_host = Value}) -> Value;
-get_name(VHost) -> vhost_v1:get_name(VHost).
+get_name(#vhost{virtual_host = Value}) -> Value.
 
 -spec get_limits(vhost()) -> list().
-get_limits(#vhost{limits = Value}) -> Value;
-get_limits(VHost) -> vhost_v1:get_limits(VHost).
+get_limits(#vhost{limits = Value}) -> Value.
 
 -spec get_metadata(vhost()) -> metadata().
-get_metadata(#vhost{metadata = Value}) -> Value;
-get_metadata(VHost) -> vhost_v1:get_metadata(VHost).
+get_metadata(#vhost{metadata = Value}) -> Value.
 
 -spec get_description(vhost()) -> binary().
 get_description(#vhost{} = VHost) ->
-    maps:get(description, get_metadata(VHost), undefined);
-get_description(VHost) ->
-    vhost_v1:get_description(VHost).
+    maps:get(description, get_metadata(VHost), undefined).
 
 -spec get_tags(vhost()) -> [atom()].
 get_tags(#vhost{} = VHost) ->
-    maps:get(tags, get_metadata(VHost), undefined);
-get_tags(VHost) ->
-    vhost_v1:get_tags(VHost).
+    maps:get(tags, get_metadata(VHost), undefined).
 
 -spec get_default_queue_type(vhost()) -> binary() | undefined.
 get_default_queue_type(#vhost{} = VHost) ->
@@ -182,34 +140,17 @@ get_default_queue_type(_VHost) ->
     undefined.
 
 set_limits(VHost, Value) ->
-    case record_version_to_use() of
-      ?record_version ->
-        VHost#vhost{limits = Value};
-      _ ->
-        vhost_v1:set_limits(VHost, Value)
-    end.
+    VHost#vhost{limits = Value}.
 
 -spec set_metadata(vhost(), metadata()) -> vhost().
 set_metadata(VHost, Value) ->
-    case record_version_to_use() of
-      ?record_version ->
-        VHost#vhost{metadata = Value};
-      _ ->
-        %% the field is not available, so this is a no-op
-        VHost
-    end.
+    VHost#vhost{metadata = Value}.
 
 -spec merge_metadata(vhost(), metadata()) -> vhost().
 merge_metadata(VHost, Value) ->
-    case record_version_to_use() of
-      ?record_version ->
-        Meta0 = get_metadata(VHost),
-        NewMeta = maps:merge(Meta0, Value),
-        VHost#vhost{metadata = NewMeta};
-      _ ->
-        %% the field is not available, so this is a no-op
-        VHost
-    end.
+    Meta0 = get_metadata(VHost),
+    NewMeta = maps:merge(Meta0, Value),
+    VHost#vhost{metadata = NewMeta}.
 
 -spec is_tagged_with(vhost:vhost(), atom()) -> boolean().
 is_tagged_with(VHost, Tag) ->
diff --git a/deps/rabbit/src/vhost_v1.erl b/deps/rabbit/src/vhost_v1.erl
deleted file mode 100644
index 3d94b0cb4b..0000000000
--- a/deps/rabbit/src/vhost_v1.erl
+++ /dev/null
@@ -1,106 +0,0 @@
-%% This Source Code Form is subject to the terms of the Mozilla Public
-%% 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) 2018-2022 VMware, Inc. or its affiliates.  All rights reserved.
-%%
-
--module(vhost_v1).
-
--include("vhost.hrl").
-
--export([new/2,
-         new/3,
-         upgrade/1,
-         upgrade_to/2,
-         fields/0,
-         fields/1,
-         info_keys/0,
-         field_name/0,
-         record_version_to_use/0,
-         pattern_match_all/0,
-         get_name/1,
-         get_limits/1,
-         get_metadata/1,
-         get_description/1,
-         get_tags/1,
-         set_limits/2
-]).
-
--define(record_version, ?MODULE).
-
-%% Represents a vhost.
-%%
-%% Historically this record had 2 arguments although the 2nd
-%% was never used (`dummy`, always undefined). This is because
-%% single field records were/are illegal in OTP.
-%%
-%% As of 3.6.x, the second argument is vhost limits,
-%% which is actually used and has the same default.
-%% Nonetheless, this required a migration, see rabbit_upgrade_functions.
-
--record(vhost, {
-  %% name as a binary
-  virtual_host :: vhost:name() | '_',
-  %% proplist of limits configured, if any
-  limits :: list() | '_'}).
-
--type vhost() :: vhost_v1().
--type vhost_v1() :: #vhost{
-                          virtual_host :: vhost:name(),
-                          limits :: list()
-                         }.
-
--export_type([vhost/0,
-              vhost_v1/0,
-              vhost_pattern/0,
-              vhost_v1_pattern/0]).
-
-
--spec new(vhost:name(), list()) -> vhost().
-new(Name, Limits) ->
-  #vhost{virtual_host = Name, limits = Limits}.
-
--spec new(vhost:name(), list(), map()) -> vhost().
-new(Name, Limits, _Metadata) ->
-  #vhost{virtual_host = Name, limits = Limits}.
-
-
--spec record_version_to_use() -> vhost_v1.
-record_version_to_use() ->
-    ?record_version.
-
--spec upgrade(vhost()) -> vhost().
-upgrade(#vhost{} = VHost) -> VHost.
-
--spec upgrade_to(vhost_v1, vhost()) -> vhost().
-upgrade_to(?record_version, #vhost{} = VHost) ->
-    VHost.
-
-fields() -> fields(?record_version).
-
-fields(?record_version) -> record_info(fields, vhost).
-
-field_name() -> #vhost.virtual_host.
-
-info_keys() -> [name, tracing, cluster_state].
-
--type vhost_pattern() :: vhost_v1_pattern().
--type vhost_v1_pattern() :: #vhost{
-                                  virtual_host :: vhost:name() | '_',
-                                  limits :: '_'
-                                 }.
-
--spec pattern_match_all() -> vhost_pattern().
-
-pattern_match_all() -> #vhost{_ = '_'}.
-
-get_name(#vhost{virtual_host = Value}) -> Value.
-get_limits(#vhost{limits = Value}) -> Value.
-
-get_metadata(_VHost) -> undefined.
-get_description(_VHost) -> undefined.
-get_tags(_VHost) -> undefined.
-
-set_limits(VHost, Value) ->
-    VHost#vhost{limits = Value}.
diff --git a/deps/rabbit/test/cluster_SUITE.erl b/deps/rabbit/test/cluster_SUITE.erl
index 97373e7672..5530350024 100644
--- a/deps/rabbit/test/cluster_SUITE.erl
+++ b/deps/rabbit/test/cluster_SUITE.erl
@@ -313,34 +313,24 @@ queue_name(Name) ->
     rabbit_misc:r(<<"/">>, queue, Name).
 
 credentials_obfuscation(Config) ->
-    case rabbit_ct_helpers:is_mixed_versions() of
-        false ->
-          case rabbit_ct_broker_helpers:enable_feature_flag(Config, virtual_host_metadata) of
-              ok ->
-                    Value = <<"amqp://something">>,
-                    Obfuscated0 = obfuscate_secret(Config, 0, Value),
-                    Obfuscated1 = obfuscate_secret(Config, 1, Value),
-
-                    ok = rabbit_ct_broker_helpers:restart_broker(Config, 1),
-
-                    ?assertEqual(Value, deobfuscate_secret(Config, 0, Obfuscated0)),
-                    ?assertEqual(Value, deobfuscate_secret(Config, 1, Obfuscated1)),
-                    ?assertEqual(Value, deobfuscate_secret(Config, 0, Obfuscated1)),
-                    ?assertEqual(Value, deobfuscate_secret(Config, 1, Obfuscated1)),
-
-                    Obfuscated2 = obfuscate_secret(Config, 1, Value),
-
-                    ok = rabbit_ct_broker_helpers:restart_broker(Config, 0),
-
-                    ?assertEqual(Value, deobfuscate_secret(Config, 0, Obfuscated2)),
-                    ok;
-              Skip ->
-                  Skip
-          end;
-        _ ->
-          %% skip the test in mixed version mode
-          {skip, "Should not run in mixed version environments"}
-      end.
+    Value = <<"amqp://something">>,
+    Obfuscated0 = obfuscate_secret(Config, 0, Value),
+    Obfuscated1 = obfuscate_secret(Config, 1, Value),
+
+    ok = rabbit_ct_broker_helpers:restart_broker(Config, 1),
+
+    ?assertEqual(Value, deobfuscate_secret(Config, 0, Obfuscated0)),
+    ?assertEqual(Value, deobfuscate_secret(Config, 1, Obfuscated1)),
+    ?assertEqual(Value, deobfuscate_secret(Config, 0, Obfuscated1)),
+    ?assertEqual(Value, deobfuscate_secret(Config, 1, Obfuscated1)),
+
+    Obfuscated2 = obfuscate_secret(Config, 1, Value),
+
+    ok = rabbit_ct_broker_helpers:restart_broker(Config, 0),
+
+    ?assertEqual(Value, deobfuscate_secret(Config, 0, Obfuscated2)),
+    ok.
+
 obfuscate_secret(Config, Node, Value) -> 
     {encrypted, _} = Result = rabbit_ct_broker_helpers:rpc(Config, Node, 
         credentials_obfuscation, encrypt, [Value]),
diff --git a/deps/rabbit/test/definition_import_SUITE.erl b/deps/rabbit/test/definition_import_SUITE.erl
index df1963e5a4..3e526a30fc 100644
--- a/deps/rabbit/test/definition_import_SUITE.erl
+++ b/deps/rabbit/test/definition_import_SUITE.erl
@@ -261,33 +261,22 @@ import_case14(Config) -> import_file_case(Config, "case14").
 import_case15(Config) -> import_file_case(Config, "case15").
 %% contains a virtual host with tags
 import_case16(Config) ->
-    case rabbit_ct_helpers:is_mixed_versions() of
-      false ->
-        case rabbit_ct_broker_helpers:enable_feature_flag(Config, virtual_host_metadata) of
-            ok ->
-                import_file_case(Config, "case16"),
-                VHost = <<"tagged">>,
-                VHostIsImported =
-                fun () ->
-                        case vhost_lookup(Config, VHost) of
-                            {error, {no_such_vhosts, _}} -> false;
-                            _       -> true
-                        end
-                end,
-                rabbit_ct_helpers:await_condition(VHostIsImported, 20000),
-                VHostRec = vhost_lookup(Config, VHost),
-                ?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)),
-
-                ok;
-            Skip ->
-                Skip
-        end;
-      _ ->
-        %% skip the test in mixed version mode
-        {skip, "Should not run in mixed version environments"}
-    end.
+    import_file_case(Config, "case16"),
+    VHost = <<"tagged">>,
+    VHostIsImported =
+    fun () ->
+            case vhost_lookup(Config, VHost) of
+                {error, {no_such_vhosts, _}} -> false;
+                _       -> true
+            end
+    end,
+    rabbit_ct_helpers:await_condition(VHostIsImported, 20000),
+    VHostRec = vhost_lookup(Config, VHost),
+    ?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)),
+
+    ok.
 
 import_case17(Config) -> import_invalid_file_case(Config, "failing_case17").
 
diff --git a/deps/rabbit/test/quorum_queue_SUITE.erl b/deps/rabbit/test/quorum_queue_SUITE.erl
index 5fcbe320e6..d22aa735c7 100644
--- a/deps/rabbit/test/quorum_queue_SUITE.erl
+++ b/deps/rabbit/test/quorum_queue_SUITE.erl
@@ -222,8 +222,6 @@ init_per_group(Group, Config) ->
                             %% more time after clustering before running the
                             %% tests.
                             timer:sleep(ClusterSize * 1000),
-                            ok = rabbit_ct_broker_helpers:enable_feature_flag(
-                                   Config2, virtual_host_metadata),
                             Config2;
                         Skip ->
                             end_per_group(Group, Config2),
diff --git a/deps/rabbitmq_management/test/rabbit_mgmt_http_SUITE.erl b/deps/rabbitmq_management/test/rabbit_mgmt_http_SUITE.erl
index 323128a1b1..9b975988a1 100644
--- a/deps/rabbitmq_management/test/rabbit_mgmt_http_SUITE.erl
+++ b/deps/rabbitmq_management/test/rabbit_mgmt_http_SUITE.erl
@@ -462,21 +462,13 @@ vhosts_test(Config) ->
     passed.
 
 vhosts_description_test(Config) ->
-    Ret = rabbit_ct_broker_helpers:enable_feature_flag(
-            Config, virtual_host_metadata),
-
     http_put(Config, "/vhosts/myvhost", [{description, <<"vhost description">>},
                                          {tags, <<"tag1,tag2">>}], {group, '2xx'}),
-    Expected = case Ret of
-                   {skip, _} ->
-                       #{name => <<"myvhost">>};
-                   _ ->
-                       #{name => <<"myvhost">>,
-                         metadata => #{
-                           description => <<"vhost description">>,
-                           tags => [<<"tag1">>, <<"tag2">>]
-                          }}
-               end,
+    Expected = #{name => <<"myvhost">>,
+                 metadata => #{
+                               description => <<"vhost description">>,
+                               tags => [<<"tag1">>, <<"tag2">>]
+                              }},
     assert_item(Expected, http_get(Config, "/vhosts/myvhost")),
 
     %% Delete it
diff --git a/deps/rabbitmq_stream/test/rabbit_stream_SUITE.erl b/deps/rabbitmq_stream/test/rabbit_stream_SUITE.erl
index 78abc4fb48..7749c1bca1 100644
--- a/deps/rabbitmq_stream/test/rabbit_stream_SUITE.erl
+++ b/deps/rabbitmq_stream/test/rabbit_stream_SUITE.erl
@@ -83,7 +83,6 @@ init_per_group(Group, Config)
                                                      [{forced_feature_flags_on_init,
                                                        [classic_mirrored_queue_version,
                                                         implicit_default_bindings,
-                                                        virtual_host_metadata,
                                                         quorum_queue,
                                                         stream_queue]}]})
                  end];