function_clause when loading a virtual host-specific definitions file at boot time

[lukebakken]

Describe the bug

RabbitMQ version - 3.12.10
Originally reported here - https://groups.google.com/g/rabbitmq-users/c/oLnrjOJAxM0

Loading a provided definitions.json file results in the following crash at boot:

RabbitMQ Boot Output

2023-12-07 12:57:12.714000-08:00 [info] <0.526.0> Applying definitions from regular file at C:/Users/lbakken/development/lukebakken/rabbitmq-users-oLnrjOJAxM0/definitions.json
2023-12-07 12:57:12.715000-08:00 [info] <0.526.0> Applying definitions from file at 'C:/Users/lbakken/development/lukebakken/rabbitmq-users-oLnrjOJAxM0/definitions.json'
2023-12-07 12:57:12.715000-08:00 [info] <0.526.0> Asked to import definitions. Acting user: rmq-internal
1>
BOOT FAILED
===========
2023-12-07 12:57:12.724000-08:00 [error] <0.526.0>
2023-12-07 12:57:12.724000-08:00 [error] <0.526.0> BOOT FAILED
2023-12-07 12:57:12.724000-08:00 [error] <0.526.0> ===========
2023-12-07 12:57:12.724000-08:00 [error] <0.526.0> Exception during startup:
2023-12-07 12:57:12.724000-08:00 [error] <0.526.0>
2023-12-07 12:57:12.724000-08:00 [error] <0.526.0> error:function_clause
2023-12-07 12:57:12.724000-08:00 [error] <0.526.0>
2023-12-07 12:57:12.724000-08:00 [error] <0.526.0>     rabbit_db_rtparams:get_all/2, line 178
2023-12-07 12:57:12.724000-08:00 [error] <0.526.0>         args: [undefined,<<"vhost-limits">>]
2023-12-07 12:57:12.724000-08:00 [error] <0.526.0>     rabbit_runtime_parameters:list/2, line 261
2023-12-07 12:57:12.724000-08:00 [error] <0.526.0>     rabbit_vhost_limit:get_limit/2, line 192
2023-12-07 12:57:12.724000-08:00 [error] <0.526.0>     rabbit_vhost_limit:would_exceed_queue_limit/2, line 111
2023-12-07 12:57:12.724000-08:00 [error] <0.526.0>     rabbit_definitions:validate_vhost_limit/3, line 867
2023-12-07 12:57:12.724000-08:00 [error] <0.526.0>     maps:fold_1/4, line 416
2023-12-07 12:57:12.724000-08:00 [error] <0.526.0>     rabbit_definitions:apply_defs/3, line 433
2023-12-07 12:57:12.724000-08:00 [error] <0.526.0>     rabbit_definitions:maybe_load_definitions/0, line 100
2023-12-07 12:57:12.724000-08:00 [error] <0.526.0>
1> Exception during startup:

error:function_clause

    rabbit_db_rtparams:get_all/2, line 178
        args: [undefined,<<"vhost-limits">>]
    rabbit_runtime_parameters:list/2, line 261
    rabbit_vhost_limit:get_limit/2, line 192
    rabbit_vhost_limit:would_exceed_queue_limit/2, line 111
    rabbit_definitions:validate_vhost_limit/3, line 867
    maps:fold_1/4, line 416
    rabbit_definitions:apply_defs/3, line 433
    rabbit_definitions:maybe_load_definitions/0, line 100

Reproduction steps

Configure RabbitMQ to load the defititions.json file from here:

https://github.com/lukebakken/rabbitmq-users-oLnrjOJAxM0

Note that the path in rabbitmq.conf will have to be adjusted.

Expected behavior

No crash.

Additional context

No response

[michaelklishin]

get_all(VHostName, Comp)
  when (is_binary(VHostName) orelse VHostName =:= '_') andalso
       (is_binary(Comp) orelse Comp =:= '_') ->
    rabbit_db:run(
      #{mnesia => fun() -> get_all_in_mnesia(VHostName, Comp) end}).

So the function assumes that virtual host name is always present or is set to _ (a special value for Khepri?), while in this file it is undefined because there are no runtime parameters.

[michaelklishin]

diff --git a/deps/rabbit/src/rabbit_db_rtparams.erl b/deps/rabbit/src/rabbit_db_rtparams.erl
index c4d86f9f25..be52cd3f88 100644
--- a/deps/rabbit/src/rabbit_db_rtparams.erl
+++ b/deps/rabbit/src/rabbit_db_rtparams.erl
@@ -175,6 +175,8 @@ get_all_in_mnesia() ->
 %%
 %% @private

+get_all(undefined = _VHostName, Comp) ->
+      get_all('_', Comp);
 get_all(VHostName, Comp)
   when (is_binary(VHostName) orelse VHostName =:= '_') andalso
        (is_binary(Comp) orelse Comp =:= '_') ->

may be all that's necessary.

[michaelklishin]

The queue definitions in this file do not have a virtual host, so you cannot import this file at boot time (you might be able to via the HTTP API endpoint that imports them for a specific) virtual host.

Compare this definition file produced by a 3.12.10 node:

  "queues": [
    {
      "name": "sq.1",
      "vhost": "/",
      "durable": true,
      "auto_delete": false,
      "arguments": {
        "x-queue-type": "stream"
      }
    },
    {
      "name": "cq.1",
      "vhost": "/",
      "durable": true,
      "auto_delete": false,
      "arguments": {}
    },
    {
      "name": "qq.1",
      "vhost": "/",
      "durable": true,
      "auto_delete": false,
      "arguments": {
        "x-queue-type": "quorum"
      }
    }
  ],

and one of the queues in this issue:

      {
          "source": "ote_messages",
          "destination": "ote_measurements",
          "destination_type": "exchange",
          "routing_key": "otemsg.121",
          "arguments": {}
      },

This means that in rabbit_definitions, the following line produces a map where keys are undefined:

{ok, VHostMap} = filter_out_existing_queues(Queues0),

[michaelklishin]

I have verified that if you export definitions using management UI (the form with a button at the bottom) while you have All virtual host selected, you get back all definitions and all resources have the virtual host field.
If you only export when just one virtual host is selected, the resources do not have a virtual host set.

There are multiple different ways to avoid this confusion. On the export side:

• Set virtual host even when a single virtual host's definitions are imported (I have no idea why it is not currently the case, could be unintentional)
• Name resulting files in a way that would hint at the fact that these are definitions of a single virtual hosts
  On the importing code path:
• Validate queues specifically and report a more specific error during boot (and abort definition import)
• OR use the default virtual host (a setting in RabbitMQ) but I suspect this can end up being more confusing, not less
  Or we can always export all definitions because export of an individual virtual host is an rarely used feature. This is an option for 3.13.

8ba0e07 was the commit that introduced virtual host field stripping. I don't remember any details on why the virtual host was removed but it is a trivial thing to undo.