root
/
rabbitmq-server
mirror of https://github.com/rabbitmq/rabbitmq-server.git
1
0
Fork 0
Code Issues Actions 1 Packages Projects Releases Wiki Activity

Fix default queue type handling for single vhost imports 

Default queue type logic didn't apply when import was done for a
single vhost. Exported definitions for a single vhost don't contain
`vhost` key, and anyway it's better to use the actual vhost name that
will be used for queue declaration.

And just in case, `vhost` is now stripped from all exported items for
a single vhost (it was still present in `parameters` and `policies`) -
on import it was ignored, but it could have been a source of the bug
similar to the one with the default queue type.
This commit is contained in:
Alexey Lebedeff 2022-12-08 16:07:04 +01:00
parent 31dffe78a7
commit 4bfe352168
3 changed files with 52 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
deps/rabbit/src/rabbit_definitions.erl vendored
View File

@ -729,12 +729,11 @@ add_queue_int(_Queue, R = #resource{kind = queue,
Name = R#resource.name,
rabbit_log:warning("Skipping import of a queue whose name begins with 'amq.', "
"name: ~ts, acting user: ~ts", [Name, ActingUser]);
add_queue_int(Queue, Name, ActingUser) ->
add_queue_int(Queue, Name = #resource{virtual_host = VHostName}, ActingUser) ->
case rabbit_amqqueue:exists(Name) of
true ->
ok;
false ->
VHostName = maps:get(vhost, Queue, rabbit_vhost:default_name()),
AutoDelete = maps:get(auto_delete, Queue, false),
DurableDeclare = maps:get(durable, Queue, true),
ExclusiveDeclare = maps:get(exclusive, Queue, false),

7
deps/rabbitmq_management/src/rabbit_mgmt_wm_definitions.erl vendored
View File

@ -99,14 +99,15 @@ vhost_definitions(ReqData, VHost, Context) ->
Bs = [strip_vhost(B) || B <- rabbit_mgmt_wm_bindings:basic(ReqData),
export_binding(B, QNames)],
{ok, Vsn} = application:get_key(rabbit, vsn),
Parameters = [rabbit_mgmt_format:parameter(
rabbit_mgmt_wm_parameters:fix_shovel_publish_properties(P))
Parameters = [strip_vhost(
rabbit_mgmt_format:parameter(
rabbit_mgmt_wm_parameters:fix_shovel_publish_properties(P)))
|| P <- rabbit_runtime_parameters:list(VHost)],
rabbit_mgmt_util:reply(
[{rabbit_version, rabbit_data_coercion:to_binary(Vsn)}] ++
filter(
[{parameters, Parameters},
{policies, rabbit_mgmt_wm_policies:basic(ReqData)},
{policies, [strip_vhost(P) || P <- rabbit_mgmt_wm_policies:basic(ReqData)]},
{queues, Qs},
{exchanges, Xs},
{bindings, Bs}]),

64
deps/rabbitmq_management/test/rabbit_mgmt_http_SUITE.erl vendored
View File

@ -98,6 +98,7 @@ all_tests() -> [
definitions_remove_things_test,
definitions_server_named_queue_test,
definitions_with_charset_test,
definitions_default_queue_type_test,
long_definitions_test,
long_definitions_multipart_test,
aliveness_test,
@ -1667,10 +1668,33 @@ long_definitions_vhosts(long_definitions_multipart_test) ->
[#{name => <<"long_definitions_test-", Bin/binary, (integer_to_binary(N))/binary>>} ||
N <- lists:seq(1, 16)].
defs_default_queue_type_vhost(Config, QueueType) ->
register_parameters_and_policy_validator(Config),
%% Create test vhost
http_put(Config, "/vhosts/test-vhost", #{defaultqueuetype => QueueType}, {group, '2xx'}),
PermArgs = [{configure, <<".*">>}, {write, <<".*">>}, {read, <<".*">>}],
http_put(Config, "/permissions/test-vhost/guest", PermArgs, {group, '2xx'}),
%% Import queue definition without an explicit queue type
http_post(Config, "/definitions/test-vhost",
#{queues => [#{name => <<"test-queue">>, durable => true}]},
{group, '2xx'}),
%% And check whether it was indeed created with the default type
Q = http_get(Config, "/queues/test-vhost/test-queue", ?OK),
QueueType = maps:get(type, Q),
%% Remove test vhost
http_delete(Config, "/vhosts/test-vhost", {group, '2xx'}),
ok.
definitions_default_queue_type_test(Config) ->
defs_default_queue_type_vhost(Config, <<"classic">>),
defs_default_queue_type_vhost(Config, <<"quorum">>).
defs_vhost(Config, Key, URI, CreateMethod, Args) ->
Rep1 = fun (S, S2) -> re:replace(S, "<vhost>", S2, [{return, list}]) end,
ReplaceVHostInArgs = fun(M, V2) -> maps:map(fun(vhost, _) -> V2;
(_, V1) -> V1 end, M) end,
%% Create test vhost
http_put(Config, "/vhosts/test", none, {group, '2xx'}),
@ -1678,41 +1702,49 @@ defs_vhost(Config, Key, URI, CreateMethod, Args) ->
http_put(Config, "/permissions/test/guest", PermArgs, {group, '2xx'}),
%% Test against default vhost
defs_vhost(Config, Key, URI, Rep1, "%2F", "test", CreateMethod,
ReplaceVHostInArgs(Args, <<"/">>), ReplaceVHostInArgs(Args, <<"test">>),
defs_vhost(Config, Key, URI, Rep1, "%2F", "test", CreateMethod, Args,
fun(URI2) -> http_delete(Config, URI2, {group, '2xx'}) end),
%% Test against test vhost
defs_vhost(Config, Key, URI, Rep1, "test", "%2F", CreateMethod,
ReplaceVHostInArgs(Args, <<"test">>), ReplaceVHostInArgs(Args, <<"/">>),
defs_vhost(Config, Key, URI, Rep1, "test", "%2F", CreateMethod, Args,
fun(URI2) -> http_delete(Config, URI2, {group, '2xx'}) end),
%% Remove test vhost
http_delete(Config, "/vhosts/test", {group, '2xx'}).
defs_vhost(Config, Key, URI0, Rep1, VHost1, VHost2, CreateMethod, Args1, Args2,
defs_vhost(Config, Key, URI0, Rep1, VHost1, VHost2, CreateMethod, Args,
DeleteFun) ->
%% Create the item
URI2 = create(Config, CreateMethod, Rep1(URI0, VHost1), Args1),
URI2 = create(Config, CreateMethod, Rep1(URI0, VHost1), Args),
%% Make sure it ends up in definitions
Definitions = http_get(Config, "/definitions/" ++ VHost1, ?OK),
true = lists:any(fun(I) -> test_item(Args1, I) end, maps:get(Key, Definitions)),
true = lists:any(fun(I) -> test_item(Args, I) end, maps:get(Key, Definitions)),
%% `vhost` is implied when importing/exporting for a single
%% virtual host, let's make sure that it doesn't accidentally
%% appear in the exported definitions. This can (and did) cause a
%% confusion about which part of the request to use as the source
%% for the vhost name.
case [ I || #{vhost := _} = I <- maps:get(Key, Definitions)] of
[] -> ok;
WithVHost -> error({vhost_included_in, Key, WithVHost})
end,
%% Make sure it is not in the other vhost
Definitions0 = http_get(Config, "/definitions/" ++ VHost2, ?OK),
false = lists:any(fun(I) -> test_item(Args2, I) end, maps:get(Key, Definitions0)),
false = lists:any(fun(I) -> test_item(Args, I) end, maps:get(Key, Definitions0)),
%% Post the definitions back
http_post(Config, "/definitions/" ++ VHost2, Definitions, {group, '2xx'}),
%% Make sure it is now in the other vhost
Definitions1 = http_get(Config, "/definitions/" ++ VHost2, ?OK),
true = lists:any(fun(I) -> test_item(Args2, I) end, maps:get(Key, Definitions1)),
true = lists:any(fun(I) -> test_item(Args, I) end, maps:get(Key, Definitions1)),
%% Delete it
DeleteFun(URI2),
URI3 = create(Config, CreateMethod, Rep1(URI0, VHost2), Args2),
URI3 = create(Config, CreateMethod, Rep1(URI0, VHost2), Args),
DeleteFun(URI3),
passed.
@ -1731,15 +1763,13 @@ definitions_vhost_test(Config) ->
defs_vhost(Config, bindings, "/bindings/<vhost>/e/amq.direct/e/amq.fanout", post,
#{routing_key => <<"routing">>, arguments => #{}}),
defs_vhost(Config, policies, "/policies/<vhost>/my-policy", put,
#{vhost => vhost,
name => <<"my-policy">>,
#{name => <<"my-policy">>,
pattern => <<".*">>,
definition => #{testpos => [1, 2, 3]},
priority => 1}),
defs_vhost(Config, parameters, "/parameters/vhost-limits/<vhost>/limits", put,
#{vhost => vhost,
name => <<"limits">>,
#{name => <<"limits">>,
component => <<"vhost-limits">>,
value => #{ 'max-connections' => 100 }}),
Upload =