Revert "Merge pull request #5409 from rabbitmq/mergify/bp/v3.11.x/pr-…

…5236"

Two reasons:
1. It was too early to backport.
2. There are at least two pull requests to backport first to avoid
   conflicts.

This reverts commit 871f1ff, reversing
changes made to adaf46b.

deps/rabbit/include/vhost.hrl

@@ -1,4 +1,6 @@
+-include("vhost_v1.hrl").
 -include("vhost_v2.hrl").
 
 -define(is_vhost(V),
-        (?is_vhost_v2(V))).
+        (?is_vhost_v2(V) orelse
+         ?is_vhost_v1(V))).

deps/rabbit/include/vhost_v1.hrl

@@ -0,0 +1,4 @@
+-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)).

deps/rabbit/src/vhost.erl

@@ -42,7 +42,7 @@
                       tags => [atom()],
                       metadata_key() => any()} | undefined).
 
--type vhost() :: vhost_v2().
+-type vhost() :: vhost_v1:vhost_v1() | vhost_v2().
 
 -record(vhost, {
     %% name as a binary
@@ -58,7 +58,8 @@
                           metadata :: metadata()
                          }.
 
--type vhost_pattern() :: vhost_v2_pattern().
+-type vhost_pattern() :: vhost_v1:vhost_v1_pattern() |
+                         vhost_v2_pattern().
 -type vhost_v2_pattern() :: #vhost{
                                   virtual_host :: name() | '_',
                                   limits :: '_',
@@ -75,63 +76,104 @@
 
 -spec new(name(), list()) -> vhost().
 new(Name, Limits) ->
-    #vhost{virtual_host = Name, limits = Limits}.
+    case record_version_to_use() of
+        ?record_version ->
+          #vhost{virtual_host = Name, limits = Limits};
+        _ ->
+          vhost_v1:new(Name, Limits)
+    end.
 
 -spec new(name(), list(), map()) -> vhost().
 new(Name, Limits, Metadata) ->
-    #vhost{virtual_host = Name, limits = Limits, metadata = Metadata}.
+    case record_version_to_use() of
+        ?record_version ->
+          #vhost{virtual_host = Name, limits = Limits, metadata = Metadata};
+        _ ->
+          vhost_v1:new(Name, Limits)
+    end.
 
--spec record_version_to_use() -> vhost_v2.
+-spec record_version_to_use() -> vhost_v1 | vhost_v2.
 
 record_version_to_use() ->
-    ?record_version.
+    case rabbit_feature_flags:is_enabled(virtual_host_metadata) of
+        true  -> ?record_version;
+        false -> vhost_v1:record_version_to_use()
+    end.
 
 -spec upgrade(vhost()) -> vhost().
 
-upgrade(#vhost{} = VHost) -> VHost.
+upgrade(#vhost{} = VHost) -> VHost;
+upgrade(OldVHost)         -> upgrade_to(record_version_to_use(), OldVHost).
 
--spec upgrade_to(vhost_v2, vhost()) -> vhost_v2().
+-spec upgrade_to
+(vhost_v2, vhost()) -> vhost_v2();
+(vhost_v1, vhost_v1:vhost_v1()) -> vhost_v1:vhost_v1().
 
 upgrade_to(?record_version, #vhost{} = 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).
+
 
 fields() ->
-    fields(?record_version).
+    case record_version_to_use() of
+        ?record_version -> fields(?record_version);
+        _               -> vhost_v1:fields()
+    end.
 
-fields(?record_version) -> record_info(fields, vhost).
+fields(?record_version) -> record_info(fields, vhost);
+fields(Version)         -> vhost_v1:fields(Version).
 
 info_keys() ->
-    %% 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].
+    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.
 
 -spec pattern_match_all() -> vhost_pattern().
 
 pattern_match_all() ->
-    #vhost{_ = '_'}.
+    case record_version_to_use() of
+        ?record_version -> #vhost{_ = '_'};
+        _               -> vhost_v1:pattern_match_all()
+    end.
 
 -spec get_name(vhost()) -> name().
-get_name(#vhost{virtual_host = Value}) -> Value.
+get_name(#vhost{virtual_host = Value}) -> Value;
+get_name(VHost) -> vhost_v1:get_name(VHost).
 
 -spec get_limits(vhost()) -> list().
-get_limits(#vhost{limits = Value}) -> Value.
+get_limits(#vhost{limits = Value}) -> Value;
+get_limits(VHost) -> vhost_v1:get_limits(VHost).
 
 -spec get_metadata(vhost()) -> metadata().
-get_metadata(#vhost{metadata = Value}) -> Value.
+get_metadata(#vhost{metadata = Value}) -> Value;
+get_metadata(VHost) -> vhost_v1:get_metadata(VHost).
 
 -spec get_description(vhost()) -> binary().
 get_description(#vhost{} = VHost) ->
-    maps:get(description, get_metadata(VHost), undefined).
+    maps:get(description, get_metadata(VHost), undefined);
+get_description(VHost) ->
+    vhost_v1:get_description(VHost).
 
 -spec get_tags(vhost()) -> [atom()].
 get_tags(#vhost{} = VHost) ->
-    maps:get(tags, get_metadata(VHost), undefined).
+    maps:get(tags, get_metadata(VHost), undefined);
+get_tags(VHost) ->
+    vhost_v1:get_tags(VHost).
 
 -spec get_default_queue_type(vhost()) -> binary() | undefined.
 get_default_queue_type(#vhost{} = VHost) ->
@@ -140,17 +182,34 @@ get_default_queue_type(_VHost) ->
     undefined.
 
 set_limits(VHost, Value) ->
-    VHost#vhost{limits = Value}.
+    case record_version_to_use() of
+      ?record_version ->
+        VHost#vhost{limits = Value};
+      _ ->
+        vhost_v1:set_limits(VHost, Value)
+    end.
 
 -spec set_metadata(vhost(), metadata()) -> vhost().
 set_metadata(VHost, Value) ->
-    VHost#vhost{metadata = 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.
 
 -spec merge_metadata(vhost(), metadata()) -> vhost().
 merge_metadata(VHost, Value) ->
-    Meta0 = get_metadata(VHost),
-    NewMeta = maps:merge(Meta0, Value),
-    VHost#vhost{metadata = NewMeta}.
+    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.
 
 -spec is_tagged_with(vhost:vhost(), atom()) -> boolean().
 is_tagged_with(VHost, Tag) ->

deps/rabbit/src/vhost_v1.erl

@@ -0,0 +1,106 @@
+%% 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}.

deps/rabbit/test/cluster_SUITE.erl

@@ -313,24 +313,34 @@ queue_name(Name) ->
     rabbit_misc:r(<<"/">>, queue, Name).
 
 credentials_obfuscation(Config) ->
-    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.
-
+    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.
 obfuscate_secret(Config, Node, Value) -> 
     {encrypted, _} = Result = rabbit_ct_broker_helpers:rpc(Config, Node, 
         credentials_obfuscation, encrypt, [Value]),