From 5f4b08236c8806d3fd2d56f0f11ddc8309b6e506 Mon Sep 17 00:00:00 2001 From: Jean-Sebastien Pedron Date: Mon, 24 Nov 2014 20:40:21 +0100 Subject: Add plugin paths to the beginning of the code path The goal is to make sure the right application is picked, in case the same one is available as both a RabbitMQ plugin and an external Erlang application. For instance, this could be the case with the Cowboy application: a version is available in the plugins, but the user could add an incompatible version to Erlang/OTP libdir or set ERL_LIBS to point to it. There's one exception currently: eldap. This application used to be available as a 3rd party one. But since Erlang R15B01, it's included in the standard library. We trust this version to have a stable API. Therefore, if the node runs on R15B01 or later and eldap version is 1.0+, we use this one. In all other cases, we don't trust it and prefer the RabbitMQ plugin. --- src/rabbit_plugins.erl | 59 ++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 55 insertions(+), 4 deletions(-) diff --git a/src/rabbit_plugins.erl b/src/rabbit_plugins.erl index e290fb53..fe614b6a 100644 --- a/src/rabbit_plugins.erl +++ b/src/rabbit_plugins.erl @@ -90,7 +90,7 @@ list(PluginsDir) -> EZs = [{ez, EZ} || EZ <- filelib:wildcard("*.ez", PluginsDir)], FreeApps = [{app, App} || App <- filelib:wildcard("*/ebin/*.app", PluginsDir)], - {Plugins, Problems} = + {AvailablePlugins, Problems} = lists:foldl(fun ({error, EZ, Reason}, {Plugins1, Problems1}) -> {Plugins1, [{EZ, Reason} | Problems1]}; (Plugin = #plugin{}, {Plugins1, Problems1}) -> @@ -102,6 +102,7 @@ list(PluginsDir) -> _ -> rabbit_log:warning( "Problem reading some plugins: ~p~n", [Problems]) end, + Plugins = lists:filter(fun keep_plugin/1, AvailablePlugins), ensure_dependencies(Plugins). %% @doc Read the list of enabled plugins from the supplied term file. @@ -132,6 +133,55 @@ dependencies(Reverse, Sources, AllPlugins) -> true = digraph:delete(G), Dests. +%% For a few known cases, an externally provided plugin can be trusted. +%% In this special case, it overrides the plugin. +keep_plugin(#plugin{name = App} = Plugin) -> + case application:load(App) of + {error, {already_loaded, _}} -> + not plugin_provided_by_otp(Plugin); + ok -> + Ret = not plugin_provided_by_otp(Plugin), + application:unload(App), + Ret; + _ -> + true + end. + +plugin_provided_by_otp(#plugin{name = eldap, version = PluginVsn}) -> + %% eldap was added to Erlang/OTP R15B01. We prefer this version + %% to the plugin. Before, eldap always advertised version "1". In + %% R15B01, it got proper versionning starting from "1.0". If eldap's + %% version is "1", we keep using the plugin, otherwise we take the + %% OTP application. As an extra check, we look at ERTS version to be + %% sure we're on R15B01. + case application:get_key(eldap, vsn) of + {ok, PluginVsn} -> + %% The plugin itself; the plugin was previously added to the + %% code path. + false; + {ok, "1"} -> + %% The version available on GitHub, not part of OTP. + false; + {ok, Vsn} -> + try rabbit_misc:version_compare(Vsn, "1.0", gte) of + true -> + %% Probably part of OTP. Let's check ERTS version to + %% be sure. + rabbit_misc:version_compare( + erlang:system_info(version), "5.9.1", gte); + false -> + %% Probably not part of OTP. Use the plugin to be safe. + false + catch + _:_ -> + %% Couldn't parse the version. It's not the OTP + %% application. + false + end + end; +plugin_provided_by_otp(_) -> + false. + %% Make sure we don't list OTP apps in here, and also that we detect %% missing dependencies. ensure_dependencies(Plugins) -> @@ -158,7 +208,7 @@ is_loadable(App) -> ok -> application:unload(App), true; _ -> false - end. + end. %%---------------------------------------------------------------------------- @@ -197,8 +247,9 @@ clean_plugin(Plugin, ExpandDir) -> delete_recursively(rabbit_misc:format("~s/~s", [ExpandDir, Plugin])). prepare_dir_plugin(PluginAppDescPath) -> - code:add_path(filename:dirname(PluginAppDescPath)), - list_to_atom(filename:basename(PluginAppDescPath, ".app")). + rabbit_log:info("Plugins: adding \"~s\" to the beginning of the code path.~n", + [filename:dirname(PluginAppDescPath)]), + code:add_patha(filename:dirname(PluginAppDescPath)). %%---------------------------------------------------------------------------- -- cgit v1.2.1 From f8466d59675c57c89935c7b2466d7e3177440f40 Mon Sep 17 00:00:00 2001 From: Jean-Sebastien Pedron Date: Wed, 26 Nov 2014 18:21:44 +0100 Subject: Only check ERTS version to determine if we accept an external eldap --- src/rabbit_plugins.erl | 34 +++------------------------------- 1 file changed, 3 insertions(+), 31 deletions(-) diff --git a/src/rabbit_plugins.erl b/src/rabbit_plugins.erl index fe614b6a..fd6acb5c 100644 --- a/src/rabbit_plugins.erl +++ b/src/rabbit_plugins.erl @@ -148,37 +148,9 @@ keep_plugin(#plugin{name = App} = Plugin) -> end. plugin_provided_by_otp(#plugin{name = eldap, version = PluginVsn}) -> - %% eldap was added to Erlang/OTP R15B01. We prefer this version - %% to the plugin. Before, eldap always advertised version "1". In - %% R15B01, it got proper versionning starting from "1.0". If eldap's - %% version is "1", we keep using the plugin, otherwise we take the - %% OTP application. As an extra check, we look at ERTS version to be - %% sure we're on R15B01. - case application:get_key(eldap, vsn) of - {ok, PluginVsn} -> - %% The plugin itself; the plugin was previously added to the - %% code path. - false; - {ok, "1"} -> - %% The version available on GitHub, not part of OTP. - false; - {ok, Vsn} -> - try rabbit_misc:version_compare(Vsn, "1.0", gte) of - true -> - %% Probably part of OTP. Let's check ERTS version to - %% be sure. - rabbit_misc:version_compare( - erlang:system_info(version), "5.9.1", gte); - false -> - %% Probably not part of OTP. Use the plugin to be safe. - false - catch - _:_ -> - %% Couldn't parse the version. It's not the OTP - %% application. - false - end - end; + %% eldap was added to Erlang/OTP R15B01 (ERTS 5.9.1). In this case, + %% we prefer this version to the plugin. + rabbit_misc:version_compare(erlang:system_info(version), "5.9.1", gte); plugin_provided_by_otp(_) -> false. -- cgit v1.2.1 From 57d48705f822d41ca7f13b3c72c94f1b4dd7b91b Mon Sep 17 00:00:00 2001 From: Jean-Sebastien Pedron Date: Fri, 28 Nov 2014 12:33:18 +0100 Subject: Remove "Plugins: adding to code path" log message --- src/rabbit_plugins.erl | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/rabbit_plugins.erl b/src/rabbit_plugins.erl index fd6acb5c..3aad48d1 100644 --- a/src/rabbit_plugins.erl +++ b/src/rabbit_plugins.erl @@ -219,8 +219,6 @@ clean_plugin(Plugin, ExpandDir) -> delete_recursively(rabbit_misc:format("~s/~s", [ExpandDir, Plugin])). prepare_dir_plugin(PluginAppDescPath) -> - rabbit_log:info("Plugins: adding \"~s\" to the beginning of the code path.~n", - [filename:dirname(PluginAppDescPath)]), code:add_patha(filename:dirname(PluginAppDescPath)). %%---------------------------------------------------------------------------- -- cgit v1.2.1 From d6798de8bf063780787c87dc7c0d934d27452cab Mon Sep 17 00:00:00 2001 From: Jean-Sebastien Pedron Date: Fri, 28 Nov 2014 12:45:14 +0100 Subject: No need to load/unload the plugin around the plugin_provided_by_otp/1 check While here, remove the unused PluginVsn variable. --- src/rabbit_plugins.erl | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/src/rabbit_plugins.erl b/src/rabbit_plugins.erl index 3aad48d1..dfde5289 100644 --- a/src/rabbit_plugins.erl +++ b/src/rabbit_plugins.erl @@ -102,7 +102,8 @@ list(PluginsDir) -> _ -> rabbit_log:warning( "Problem reading some plugins: ~p~n", [Problems]) end, - Plugins = lists:filter(fun keep_plugin/1, AvailablePlugins), + Plugins = lists:filter(fun(P) -> not plugin_provided_by_otp(P) end, + AvailablePlugins), ensure_dependencies(Plugins). %% @doc Read the list of enabled plugins from the supplied term file. @@ -135,19 +136,7 @@ dependencies(Reverse, Sources, AllPlugins) -> %% For a few known cases, an externally provided plugin can be trusted. %% In this special case, it overrides the plugin. -keep_plugin(#plugin{name = App} = Plugin) -> - case application:load(App) of - {error, {already_loaded, _}} -> - not plugin_provided_by_otp(Plugin); - ok -> - Ret = not plugin_provided_by_otp(Plugin), - application:unload(App), - Ret; - _ -> - true - end. - -plugin_provided_by_otp(#plugin{name = eldap, version = PluginVsn}) -> +plugin_provided_by_otp(#plugin{name = eldap}) -> %% eldap was added to Erlang/OTP R15B01 (ERTS 5.9.1). In this case, %% we prefer this version to the plugin. rabbit_misc:version_compare(erlang:system_info(version), "5.9.1", gte); -- cgit v1.2.1