From ef19253e3831be8463b0e37f95dcc54e3680ffc8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20G=C3=B6m=C3=B6ri?= Date: Thu, 10 Jul 2025 00:37:26 +0200 Subject: [PATCH 1/2] Don't use Mnesia in rabbitmq_mqtt/test/processor_SUITE Soon Mnesia will be gone from RabbitMQ, so better make the test suite metadata store agnostic. (cherry picked from commit 10f1ea1bac513c44fa2f0d4c73b25808ee679087) --- deps/rabbitmq_mqtt/test/processor_SUITE.erl | 91 ++++++++++----------- 1 file changed, 42 insertions(+), 49 deletions(-) diff --git a/deps/rabbitmq_mqtt/test/processor_SUITE.erl b/deps/rabbitmq_mqtt/test/processor_SUITE.erl index 7e967325e0d2..8a679b764bff 100644 --- a/deps/rabbitmq_mqtt/test/processor_SUITE.erl +++ b/deps/rabbitmq_mqtt/test/processor_SUITE.erl @@ -31,30 +31,15 @@ suite() -> init_per_suite(Config) -> ok = application:load(rabbitmq_mqtt), + meck:new(rabbit_runtime_parameters, [passthrough, no_link]), Config. end_per_suite(Config) -> ok = application:unload(rabbitmq_mqtt), + meck:unload(rabbit_runtime_parameters), Config. init_per_group(_, Config) -> Config. end_per_group(_, Config) -> Config. -init_per_testcase(get_vhost, Config) -> - mnesia:start(), - mnesia:create_table(rabbit_runtime_parameters, [ - {attributes, record_info(fields, runtime_parameters)}, - {record_name, runtime_parameters}]), - meck:new(rabbit_feature_flags, [passthrough, no_link]), - meck:expect( - rabbit_feature_flags, is_enabled, - fun - (khepri_db, _) -> false; - (FeatureNames, _) -> meck:passthrough([FeatureNames]) - end), - Config; init_per_testcase(_, Config) -> Config. -end_per_testcase(get_vhost, Config) -> - meck:unload(rabbit_feature_flags), - mnesia:stop(), - Config; end_per_testcase(_, Config) -> Config. ignore_colons(B) -> application:set_env(rabbitmq_mqtt, ignore_colons_in_username, B). @@ -150,26 +135,32 @@ get_vhost(_Config) -> %% certificate user, port/vhost parameter but no mapping, cert/vhost mapping %% should use cert/vhost mapping - set_global_parameter(mqtt_default_vhosts, [ - {<<"O=client,CN=dummy">>, <<"somevhost">>}, - {<<"O=client,CN=otheruser">>, <<"othervhost">>} - ]), - set_global_parameter(mqtt_port_to_vhost_mapping, [ - {<<"1884">>, <<"othervhost">>} - ]), + set_global_parameters( + [{mqtt_default_vhosts, + [ + {<<"O=client,CN=dummy">>, <<"somevhost">>}, + {<<"O=client,CN=otheruser">>, <<"othervhost">>} + ]}, + {mqtt_port_to_vhost_mapping, + [ + {<<"1884">>, <<"othervhost">>} + ]}]), {_, {<<"somevhost">>, <<"guest">>}} = rabbit_mqtt_processor:get_vhost(<<"guest">>, <<"O=client,CN=dummy">>, 1883), clear_vhost_global_parameters(), %% certificate user, port/vhost parameter, cert/vhost parameter %% cert/vhost parameter takes precedence - set_global_parameter(mqtt_default_vhosts, [ - {<<"O=client,CN=dummy">>, <<"cert-somevhost">>}, - {<<"O=client,CN=otheruser">>, <<"othervhost">>} - ]), - set_global_parameter(mqtt_port_to_vhost_mapping, [ - {<<"1883">>, <<"port-vhost">>}, - {<<"1884">>, <<"othervhost">>} - ]), + set_global_parameters( + [{mqtt_default_vhosts, + [ + {<<"O=client,CN=dummy">>, <<"cert-somevhost">>}, + {<<"O=client,CN=otheruser">>, <<"othervhost">>} + ]}, + {mqtt_port_to_vhost_mapping, + [ + {<<"1883">>, <<"port-vhost">>}, + {<<"1884">>, <<"othervhost">>} + ]}]), {_, {<<"cert-somevhost">>, <<"guest">>}} = rabbit_mqtt_processor:get_vhost(<<"guest">>, <<"O=client,CN=dummy">>, 1883), clear_vhost_global_parameters(), @@ -179,28 +170,30 @@ get_vhost(_Config) -> %% not a certificate user, port/vhost parameter, cert/vhost parameter %% port/vhost mapping is used, as cert/vhost should not be used - set_global_parameter(mqtt_default_vhosts, [ - {<<"O=cert">>, <<"cert-somevhost">>}, - {<<"O=client,CN=otheruser">>, <<"othervhost">>} - ]), - set_global_parameter(mqtt_port_to_vhost_mapping, [ - {<<"1883">>, <<"port-vhost">>}, - {<<"1884">>, <<"othervhost">>} - ]), + set_global_parameters( + [{mqtt_default_vhosts, + [ + {<<"O=cert">>, <<"cert-somevhost">>}, + {<<"O=client,CN=otheruser">>, <<"othervhost">>} + ]}, + {mqtt_port_to_vhost_mapping, + [ + {<<"1883">>, <<"port-vhost">>}, + {<<"1884">>, <<"othervhost">>} + ]}]), {_, {<<"port-vhost">>, <<"guest">>}} = rabbit_mqtt_processor:get_vhost(<<"guest">>, none, 1883), clear_vhost_global_parameters(), ok. set_global_parameter(Key, Term) -> - InsertParameterFun = fun () -> - mnesia:write(rabbit_runtime_parameters, #runtime_parameters{key = Key, value = Term}, write) - end, + set_global_parameters([{Key, Term}]). - {atomic, ok} = mnesia:transaction(InsertParameterFun). +set_global_parameters(KVList) -> + meck:expect( + rabbit_runtime_parameters, value_global, + fun(Key) -> proplists:get_value(Key, KVList, not_found) end). clear_vhost_global_parameters() -> - DeleteParameterFun = fun () -> - ok = mnesia:delete(rabbit_runtime_parameters, mqtt_default_vhosts, write), - ok = mnesia:delete(rabbit_runtime_parameters, mqtt_port_to_vhost_mapping, write) - end, - {atomic, ok} = mnesia:transaction(DeleteParameterFun). + meck:expect( + rabbit_runtime_parameters, value_global, + fun(_) -> not_found end). From b30d1b1da0d9b57ea8c75c552510595622707f39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20G=C3=B6m=C3=B6ri?= Date: Fri, 18 Oct 2024 01:35:13 +0200 Subject: [PATCH 2/2] Refactor mqtt_processor get_vhost functions In order to clarify preference of different methods. This commit oes not change functionality. This highlights some inconsistences: - If both User/Password and SslLogin are provided, in `check_credentials` User/Password takes precedence while in `get_vhost` SslLogin - If SslLogin is provided (but no mapping is found) vhost from port mapping has precedence over vhost from UserName, while in case of no ssl it is the other way around. (cherry picked from commit 7d4ecb5e8223d8c3a5f1ef87769ee4c778186195) --- .../src/rabbit_mqtt_processor.erl | 71 +++++++++---------- deps/rabbitmq_mqtt/test/processor_SUITE.erl | 10 ++- 2 files changed, 41 insertions(+), 40 deletions(-) diff --git a/deps/rabbitmq_mqtt/src/rabbit_mqtt_processor.erl b/deps/rabbitmq_mqtt/src/rabbit_mqtt_processor.erl index 18ad6d9735bf..034e55e9dee4 100644 --- a/deps/rabbitmq_mqtt/src/rabbit_mqtt_processor.erl +++ b/deps/rabbitmq_mqtt/src/rabbit_mqtt_processor.erl @@ -1178,33 +1178,31 @@ get_vhost(UserBin, SslLogin, Port) -> get_vhost_ssl(UserBin, SslLogin, Port). get_vhost_no_ssl(UserBin, Port) -> - case vhost_in_username(UserBin) of - true -> - {vhost_in_username_or_default, get_vhost_username(UserBin)}; - false -> - PortVirtualHostMapping = rabbit_runtime_parameters:value_global( - mqtt_port_to_vhost_mapping - ), - case get_vhost_from_port_mapping(Port, PortVirtualHostMapping) of + case get_vhost_username(UserBin) of + undefined -> + case get_vhost_from_port_mapping(Port) of undefined -> - {plugin_configuration_or_default_vhost, {rabbit_mqtt_util:env(vhost), UserBin}}; - VHost -> - {port_to_vhost_mapping, {VHost, UserBin}} - end + VhostFromConfig = rabbit_mqtt_util:env(vhost), + {plugin_configuration_or_default_vhost, {VhostFromConfig, UserBin}}; + VHostFromPortMapping -> + {port_to_vhost_mapping, {VHostFromPortMapping, UserBin}} + end; + VHostUser -> + {vhost_in_username, VHostUser} end. get_vhost_ssl(UserBin, SslLoginName, Port) -> - UserVirtualHostMapping = rabbit_runtime_parameters:value_global( - mqtt_default_vhosts - ), - case get_vhost_from_user_mapping(SslLoginName, UserVirtualHostMapping) of + case get_vhost_from_user_mapping(SslLoginName) of undefined -> - PortVirtualHostMapping = rabbit_runtime_parameters:value_global( - mqtt_port_to_vhost_mapping - ), - case get_vhost_from_port_mapping(Port, PortVirtualHostMapping) of + case get_vhost_from_port_mapping(Port) of undefined -> - {vhost_in_username_or_default, get_vhost_username(UserBin)}; + case get_vhost_username(UserBin) of + undefined -> + VhostFromConfig = rabbit_mqtt_util:env(vhost), + {plugin_configuration_or_default_vhost, {VhostFromConfig, UserBin}}; + VHostUser -> + {vhost_in_username, VHostUser} + end; VHostFromPortMapping -> {port_to_vhost_mapping, {VHostFromPortMapping, UserBin}} end; @@ -1212,31 +1210,24 @@ get_vhost_ssl(UserBin, SslLoginName, Port) -> {client_cert_to_vhost_mapping, {VHostFromCertMapping, UserBin}} end. -vhost_in_username(UserBin) -> - case application:get_env(?APP_NAME, ignore_colons_in_username) of - {ok, true} -> false; - _ -> - %% split at the last colon, disallowing colons in username - case re:split(UserBin, ":(?!.*?:)") of - [_, _] -> true; - [UserBin] -> false; - [] -> false - end - end. - get_vhost_username(UserBin) -> - Default = {rabbit_mqtt_util:env(vhost), UserBin}, case application:get_env(?APP_NAME, ignore_colons_in_username) of - {ok, true} -> Default; + {ok, true} -> undefined; _ -> %% split at the last colon, disallowing colons in username case re:split(UserBin, ":(?!.*?:)") of [Vhost, UserName] -> {Vhost, UserName}; - [UserBin] -> Default; - [] -> Default + [UserBin] -> undefined; + [] -> undefined end end. +get_vhost_from_user_mapping(User) -> + UserVirtualHostMapping = rabbit_runtime_parameters:value_global( + mqtt_default_vhosts + ), + get_vhost_from_user_mapping(User, UserVirtualHostMapping). + get_vhost_from_user_mapping(_User, not_found) -> undefined; get_vhost_from_user_mapping(User, Mapping) -> @@ -1248,6 +1239,12 @@ get_vhost_from_user_mapping(User, Mapping) -> VHost end. +get_vhost_from_port_mapping(Port) -> + PortVirtualHostMapping = rabbit_runtime_parameters:value_global( + mqtt_port_to_vhost_mapping + ), + get_vhost_from_port_mapping(Port, PortVirtualHostMapping). + get_vhost_from_port_mapping(_Port, not_found) -> undefined; get_vhost_from_port_mapping(Port, Mapping) -> diff --git a/deps/rabbitmq_mqtt/test/processor_SUITE.erl b/deps/rabbitmq_mqtt/test/processor_SUITE.erl index 8a679b764bff..c7b38e57a719 100644 --- a/deps/rabbitmq_mqtt/test/processor_SUITE.erl +++ b/deps/rabbitmq_mqtt/test/processor_SUITE.erl @@ -45,9 +45,13 @@ end_per_testcase(_, Config) -> Config. ignore_colons(B) -> application:set_env(rabbitmq_mqtt, ignore_colons_in_username, B). ignores_colons_in_username_if_option_set(_Config) -> - ignore_colons(true), - ?assertEqual({rabbit_mqtt_util:env(vhost), <<"a:b:c">>}, - rabbit_mqtt_processor:get_vhost_username(<<"a:b:c">>)). + clear_vhost_global_parameters(), + ignore_colons(true), + ?assertEqual(undefined, + rabbit_mqtt_processor:get_vhost_username(<<"a:b:c">>)), + ?assertEqual({plugin_configuration_or_default_vhost, + {rabbit_mqtt_util:env(vhost), <<"a:b:c">>}}, + rabbit_mqtt_processor:get_vhost(<<"a:b:c">>, none, 1883)). interprets_colons_in_username_if_option_not_set(_Config) -> ignore_colons(false),