rabbitmq / rabbitmq-management

RabbitMQ Management UI and HTTP API
https://www.rabbitmq.com/management.html
Other
370 stars 165 forks source link

Importing global runtime parameters via definitions inserts values as Erlang maps instead of proplists #528

Closed dumbbell closed 6 years ago

dumbbell commented 6 years ago

mertz___ on #rabbitmq reports that when he imports the mqtt_port_to_vhost_mapping global parameter's definitions for the MQTT plugin through the HTTP API, the plugin fails to get its value with the following exception:

{badarg,[
  {lists,keyfind,[<<"1883">>,1,#{<<"1883">> => <<"device">>,<<"1884">> => <<"admin">>}
],[]}

He is using RabbitMQ 3.7.2 on Erlang 20.1.7.

Indeed, when the parameter is set from the command line:

rabbitmqctl set_global_parameter \
  mqtt_port_to_vhost_mapping  \
  '{"1883":"device", "1884":"admin"}'

The value is stored as an Erlang proplist:

(rabbit@magellan)2> ets:i(rabbit_runtime_parameters).
<1   > {runtime_parameters,mqtt_port_to_vhost_mapping,
                     ...
EOT  (q)uit (p)Digits (k)ill /Regexp -->p1
{runtime_parameters,mqtt_port_to_vhost_mapping,
                    [{<<"1883">>,<<"device">>},{<<"1884">>,<<"admin">>}]}

However, when the same parameter is imported using the web UI with the following input:

{
  "vhosts":[{"name":"device"},{"name":"admin"}],
  "global_parameters":[
    {
      "name":"mqtt_port_to_vhost_mapping",
      "value":{"1883":"device","1884":"admin"}
    }
  ]
}

The value is stored as an Erlang map:

(rabbit@magellan)1> ets:i(rabbit_runtime_parameters).
                     ...
<2   > {runtime_parameters,mqtt_port_to_vhost_mapping,
                     ...
EOT  (q)uit (p)Digits (k)ill /Regexp -->p2                               
{runtime_parameters,mqtt_port_to_vhost_mapping,
                    #{<<"1883">> => <<"device">>,<<"1884">> => <<"admin">>}}

rabbitmqctl(8) calls rabbit_runtime_parameters:parse_set_global() parses the JSON input, converts it to a proplist if it's a map, then calls set_global() to store the result. The management plugin parses the JSON and calls set_global() directly, skipping the possible conversion to a proplist.

This explains the inconsistency and thus the crash of the plugin which expects a proplist.

Mert-Z commented 6 years ago

The problem is that I need those global parameters configured with the startup of RabbitMQ which is running inside a docker container. Definitions json file was a neat way for me to set this configuration up as part of docker build. Now as a workaround, it seems like, I now need to deploy a background script that will wait for RabbitMQ startup and then execute rabbitmqctl to set the parameters. Any other suggestions for a workaround?

michaelklishin commented 6 years ago

There are two options:

michaelklishin commented 6 years ago

While we should add the conversion step to the HTTP API handler, the MQTT plugin in 3.7.x should support both proplsits and maps. We won't be using proplists forever now that we require 19.3+.

michaelklishin commented 6 years ago

The easiest solution here is to make rabbit_mqtt_processor:get_vhost_from_port_mapping/2 accept both types of values. Runtime parameters in general do not dictate what data type should be used.

I will file a new issue for the MQTT plugin.

michaelklishin commented 6 years ago

Actually it should be easy to special case certain parameters in add_global_parameter/2. However, a map value can already stored by an older version, so https://github.com/rabbitmq/rabbitmq-mqtt/issues/156 must be resolved anyway.

michaelklishin commented 6 years ago

Oh, there's workaround number 3: the good ol' vhost in username convention we have. The plugin will extract vhost from username if you follow this format: {vhost}:{username} (unless ignore_colons_in_username is set to true for the plugin).

Mert-Z commented 6 years ago

Thanks for the workaround#3 @michaelklishin but unfortunately this workaround does not seem backwards compatible such that I don't have any control on the existing active MQTT producers which are configured to send username in plain format.

michaelklishin commented 6 years ago

OK, then in the meantime global parameters have to be set up via rabbitmqctl. This was addressed in v3.7.x and will ship in 3.7.3 in a few weeks or so.

dumbbell commented 6 years ago

The latest snapshot contains the patches and looks fine to me.

@Mert-Z: Could you please give it a try?

Mert-Z commented 6 years ago

@dumbbell: Patch works perfectly. :thumbsup: 2018-01-08 10:12:11.492 [info] <0.712.0> MQTT vhost picked using MQTT port to vhost mapping 2018-01-08 10:12:11.495 [info] <0.712.0> accepting MQTT connection <0.712.0> (192.168.33.1:62410 -> 192.168.33.10:1884) 2018-01-08 10:12:56.615 [info] <0.763.0> MQTT vhost picked using MQTT port to vhost mapping 2018-01-08 10:12:56.619 [info] <0.763.0> accepting MQTT connection <0.763.0> (192.168.33.1:62415 -> 192.168.33.10:1883)

michaelklishin commented 6 years ago

Thank you, we hope to ship 3.7.3 some time this month.