Skip to content

Commit

Permalink
Avoid importing resources without specified virtual host
Browse files Browse the repository at this point in the history
On boot first and foremost. Log a more helpful message.

 See #10068 for the background.
  • Loading branch information
michaelklishin committed Dec 8, 2023
1 parent 62fffb6 commit 9d94048
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 6 deletions.
14 changes: 10 additions & 4 deletions deps/rabbit/src/rabbit_definitions.erl
Original file line number Diff line number Diff line change
Expand Up @@ -439,12 +439,14 @@ apply_defs(Map, ActingUser, SuccessFun) when is_function(SuccessFun) ->
HasExchangesWithoutVirtualHostField = any_orphaned_objects(maps:get(exchanges, Map, [])),
HasBindingsWithoutVirtualHostField = any_orphaned_objects(maps:get(bindings, Map, [])),

rabbit_log:debug("HasQueuesWithoutVirtualHostField = ~p, HasExchangesWithoutVirtualHostField = ~p, HasBindingsWithoutVirtualHostField = ~p",
[HasQueuesWithoutVirtualHostField, HasExchangesWithoutVirtualHostField, HasBindingsWithoutVirtualHostField]),

case (HasQueuesWithoutVirtualHostField and HasExchangesWithoutVirtualHostField and HasBindingsWithoutVirtualHostField) of
true ->
rabbit_log:error("Definitions import: some queues, exchanges or bindings in the definition file "
"are missing the virtual host field. Such files are produced when definitions of "
"a single virtual host are exported. They cannot be used to import definitions at boot time",
[]),
"a single virtual host are exported. They cannot be used to import definitions at boot time"),
throw({error, invalid_definitions_file});
false ->
ok
Expand Down Expand Up @@ -613,8 +615,8 @@ do_concurrent_for_all(List, WorkPoolFun) ->
fun() ->
_ = try
WorkPoolFun(M)
catch {error, E} -> gatherer:in(Gatherer, {error, E});
_:E -> gatherer:in(Gatherer, {error, E})
catch {error, E} -> gatherer:in(Gatherer, {error, E});
_:E:_Stacktrace -> gatherer:in(Gatherer, {error, E})
end,
gatherer:finish(Gatherer)
end)
Expand Down Expand Up @@ -761,6 +763,10 @@ 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, R = #resource{kind = queue, virtual_host = undefined}, ActingUser) ->
Name = R#resource.name,
rabbit_log:warning("Skipping import of a queue with an unset virtual host field, "
"name: ~ts, acting user: ~ts", [Name, ActingUser]);
add_queue_int(Queue, Name = #resource{virtual_host = VHostName}, ActingUser) ->
case rabbit_amqqueue:exists(Name) of
true ->
Expand Down
5 changes: 4 additions & 1 deletion deps/rabbit/test/definition_import_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,10 @@ import_file_case(Config, Subdirectory, CaseName) ->

import_invalid_file_case(Config, CaseName) ->
CasePath = filename:join(?config(data_dir, Config), CaseName ++ ".json"),
rabbit_ct_broker_helpers:rpc(Config, 0, ?MODULE, run_invalid_import_case, [CasePath]),
try
rabbit_ct_broker_helpers:rpc(Config, 0, ?MODULE, run_invalid_import_case, [CasePath])
catch _:_:_ -> ok
end,
ok.

import_invalid_file_case_in_khepri(Config, CaseName) ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ defmodule RabbitMQ.CLI.Ctl.Commands.ImportDefinitionsCommand do
false ->
{:ok,
"Successfully started definition import. " <>
"This process is asynchronous and can take some time.\n"}
"This process is asynchronous and can take some time. Watch target node logs for completion.\n"}
end
end

Expand Down

0 comments on commit 9d94048

Please sign in to comment.