Merge pull request #1016 from binarin/multiple-plugins-dir

Support multiple plugins directories

5 files changed, 198 insertions, 49 deletions

diff --git a/scripts/rabbitmq-defaults b/scripts/rabbitmq-defaults
index baffce80de..d4cb5889ea 100755
--- a/scripts/rabbitmq-defaults
+++ b/scripts/rabbitmq-defaults
@@ -41,4 +41,15 @@ ENABLED_PLUGINS_FILE=${SYS_PREFIX}/etc/rabbitmq/enabled_plugins
 
 PLUGINS_DIR="${RABBITMQ_HOME}/plugins"
 
+# RABBIT_HOME can contain a version number, so default plugins
+# directory can be hard to find if we want to package some plugin
+# separately. When RABBITMQ_HOME points to a standard location where
+# it's usally being installed by package managers, we add
+# "/usr/lib/rabbitmq/plugins" to plugin search path.
+case "$RABBITMQ_HOME" in
+    /usr/lib/rabbitmq/*)
+        PLUGINS_DIR="/usr/lib/rabbitmq/plugins:$PLUGINS_DIR"
+        ;;
+esac
+
 CONF_ENV_FILE=${SYS_PREFIX}/etc/rabbitmq/rabbitmq-env.conf
diff --git a/scripts/rabbitmq-env b/scripts/rabbitmq-env
index 8c33e7c0b7..3055552d88 100755
--- a/scripts/rabbitmq-env
+++ b/scripts/rabbitmq-env
@@ -57,6 +57,20 @@ rmq_realpath() {
     fi
 }
 
+path_contains_existing_directory() {
+    local path="${1:?}"
+    local dir
+    local rc
+    local IFS="
+    "
+    for dir in $(echo "$path" | tr ':' '\n'); do
+        if [ -d "$dir" ]; then
+            return 0
+        fi
+    done
+    return 1
+}
+
 RABBITMQ_HOME="$(rmq_realpath "${RABBITMQ_SCRIPTS_DIR}/..")"
 
 ## Set defaults
@@ -107,8 +121,9 @@ fi
 rmq_normalize_path() {
     local path=$1
 
-    # Remove redundant slashes and strip a trailing slash
-    echo "$path" | sed -e 's#/\{2,\}#/#g' -e 's#/$##'
+    # Remove redundant slashes and strip a trailing slash for a
+    # PATH-like vars - ':' is the delimiter
+    echo "$path" | sed -e 's#/\{2,\}#/#g' -e 's#/$##' -e 's#/:#:#g'
 }
 
 rmq_normalize_path_var() {
@@ -266,7 +281,7 @@ if [ "${RABBITMQ_DEV_ENV}" ]; then
         fi
     fi
 
-    if [ -d "${RABBITMQ_PLUGINS_DIR}" ]; then
+    if path_contains_existing_directory "${RABBITMQ_PLUGINS_DIR}" ; then
         # RabbitMQ was started with "make run-broker" from its own
         # source tree. Take rabbit_common from the plugins directory.
         ERL_LIBS="${RABBITMQ_PLUGINS_DIR}:${ERL_LIBS}"
@@ -290,7 +305,7 @@ if [ "${RABBITMQ_DEV_ENV}" ]; then
         ERL_LIBS="${DEPS_DIR_norm}:${ERL_LIBS}"
     fi
 else
-    if [ -d "${RABBITMQ_PLUGINS_DIR}" ]; then
+    if path_contains_existing_directory "${RABBITMQ_PLUGINS_DIR}" ; then
         # RabbitMQ was started from its install directory. Take
         # rabbit_common from the plugins directory.
         ERL_LIBS="${RABBITMQ_PLUGINS_DIR}:${ERL_LIBS}"
diff --git a/src/rabbit_plugins.erl b/src/rabbit_plugins.erl
index 4d8966f7e2..bde4ff5388 100644
--- a/src/rabbit_plugins.erl
+++ b/src/rabbit_plugins.erl
@@ -84,42 +84,16 @@ active() ->
             lists:member(App, InstalledPlugins)].
 
 %% @doc Get the list of plugins which are ready to be enabled.
-list(PluginsDir) ->
-    list(PluginsDir, false).
+list(PluginsPath) ->
+    list(PluginsPath, false).
 
-list(PluginsDir, IncludeRequiredDeps) ->
-    EZs = [{ez, EZ} || EZ <- filelib:wildcard("*.ez", PluginsDir)],
-    FreeApps = [{app, App} ||
-                   App <- filelib:wildcard("*/ebin/*.app", PluginsDir)],
-    %% We load the "rabbit" application to be sure we can get the
-    %% "applications" key. This is required for rabbitmq-plugins for
-    %% instance.
-    application:load(rabbit),
-    {ok, RabbitDeps} = application:get_key(rabbit, applications),
-    {AvailablePlugins, Problems} =
-        lists:foldl(fun ({error, EZ, Reason}, {Plugins1, Problems1}) ->
-                            {Plugins1, [{EZ, Reason} | Problems1]};
-                        (Plugin = #plugin{name = Name}, {Plugins1, Problems1}) ->
-                            %% Applications RabbitMQ depends on (eg.
-                            %% "rabbit_common") can't be considered
-                            %% plugins, otherwise rabbitmq-plugins would
-                            %% list them and the user may believe he can
-                            %% disable them.
-                            case IncludeRequiredDeps orelse
-                              not lists:member(Name, RabbitDeps) of
-                                true  -> {[Plugin|Plugins1], Problems1};
-                                false -> {Plugins1, Problems1}
-                            end
-                    end, {[], []},
-                    [plugin_info(PluginsDir, Plug) || Plug <- EZs ++ FreeApps]),
-    case Problems of
-        [] -> ok;
-        _  -> rabbit_log:warning(
-                "Problem reading some plugins: ~p~n", [Problems])
-    end,
-    Plugins = lists:filter(fun(P) -> not plugin_provided_by_otp(P) end,
-                           AvailablePlugins),
-    ensure_dependencies(Plugins).
+list(PluginsPath, IncludeRequiredDeps) ->
+    {AllPlugins, LoadingProblems} = discover_plugins(split_path(PluginsPath)),
+    {UniquePlugins, DuplicateProblems} = remove_duplicate_plugins(AllPlugins),
+    Plugins1 = maybe_keep_required_deps(IncludeRequiredDeps, UniquePlugins),
+    Plugins2 = remove_otp_overrideable_plugins(Plugins1),
+    maybe_report_plugin_loading_problems(LoadingProblems ++ DuplicateProblems),
+    ensure_dependencies(Plugins2).
 
 %% @doc Read the list of enabled plugins from the supplied term file.
 read_enabled(PluginsFile) ->
@@ -259,14 +233,12 @@ prepare_plugin(#plugin{type = dir, name = Name, location = Location},
                ExpandDir) ->
     rabbit_file:recursive_copy(Location, filename:join([ExpandDir, Name])).
 
-plugin_info(Base, {ez, EZ0}) ->
-    EZ = filename:join([Base, EZ0]),
+plugin_info({ez, EZ}) ->
     case read_app_file(EZ) of
         {application, Name, Props} -> mkplugin(Name, Props, ez, EZ);
         {error, Reason}            -> {error, EZ, Reason}
     end;
-plugin_info(Base, {app, App0}) ->
-    App = filename:join([Base, App0]),
+plugin_info({app, App}) ->
     case rabbit_file:read_term_file(App) of
         {ok, [{application, Name, Props}]} ->
             mkplugin(Name, Props, dir,
@@ -317,3 +289,93 @@ plugin_names(Plugins) ->
 
 lookup_plugins(Names, AllPlugins) ->
     [P || P = #plugin{name = Name} <- AllPlugins, lists:member(Name, Names)].
+
+%% Split PATH-like value into its components.
+split_path(PathString) ->
+    Delimiters = case os:type() of
+                     {unix, _} -> ":";
+                     {win32, _} -> ";"
+                 end,
+    string:tokens(PathString, Delimiters).
+
+%% Search for files using glob in a given dir. Returns full filenames of those files.
+full_path_wildcard(Glob, Dir) ->
+    [filename:join([Dir, File]) || File <- filelib:wildcard(Glob, Dir)].
+
+%% Returns list off all .ez files in a given set of directories
+list_ezs([]) ->
+    [];
+list_ezs([Dir|Rest]) ->
+    [{ez, EZ} || EZ <- full_path_wildcard("*.ez", Dir)] ++ list_ezs(Rest).
+
+%% Returns list of all files that look like OTP applications in a
+%% given set of directories.
+list_free_apps([]) ->
+    [];
+list_free_apps([Dir|Rest]) ->
+    [{app, App} || App <- full_path_wildcard("*/ebin/*.app", Dir)]
+        ++ list_free_apps(Rest).
+
+compare_by_name_and_version(#plugin{name = Name, version = VersionA},
+                            #plugin{name = Name, version = VersionB}) ->
+    ec_semver:lte(VersionA, VersionB);
+compare_by_name_and_version(#plugin{name = NameA},
+                            #plugin{name = NameB}) ->
+    NameA =< NameB.
+
+-spec discover_plugins([Directory]) -> {[#plugin{}], [Problem]} when
+      Directory :: file:name(),
+      Problem :: {file:name(), term()}.
+discover_plugins(PluginsDirs) ->
+    EZs = list_ezs(PluginsDirs),
+    FreeApps = list_free_apps(PluginsDirs),
+    read_plugins_info(EZs ++ FreeApps, {[], []}).
+
+read_plugins_info([], Acc) ->
+    Acc;
+read_plugins_info([Path|Paths], {Plugins, Problems}) ->
+    case plugin_info(Path) of
+        #plugin{} = Plugin ->
+            read_plugins_info(Paths, {[Plugin|Plugins], Problems});
+        {error, Location, Reason} ->
+            read_plugins_info(Paths, {Plugins, [{Location, Reason}|Problems]})
+    end.
+
+remove_duplicate_plugins(Plugins) ->
+    %% Reverse order ensures that if there are several versions of the
+    %% same plugin, the most recent one comes first.
+    Sorted = lists:reverse(
+               lists:sort(fun compare_by_name_and_version/2, Plugins)),
+    remove_duplicate_plugins(Sorted, {[], []}).
+
+remove_duplicate_plugins([], Acc) ->
+    Acc;
+remove_duplicate_plugins([Best = #plugin{name = Name}, Offender = #plugin{name = Name} | Rest],
+                  {Plugins0, Problems0}) ->
+    Problems1 = [{Offender#plugin.location, duplicate_plugin}|Problems0],
+    remove_duplicate_plugins([Best|Rest], {Plugins0, Problems1});
+remove_duplicate_plugins([Plugin|Rest], {Plugins0, Problems0}) ->
+    Plugins1 = [Plugin|Plugins0],
+    remove_duplicate_plugins(Rest, {Plugins1, Problems0}).
+
+maybe_keep_required_deps(true, Plugins) ->
+    Plugins;
+maybe_keep_required_deps(false, Plugins) ->
+    %% We load the "rabbit" application to be sure we can get the
+    %% "applications" key. This is required for rabbitmq-plugins for
+    %% instance.
+    application:load(rabbit),
+    {ok, RabbitDeps} = application:get_key(rabbit, applications),
+    lists:filter(fun(#plugin{name = Name}) ->
+                         not lists:member(Name, RabbitDeps)
+                 end,
+                 Plugins).
+
+remove_otp_overrideable_plugins(Plugins) ->
+    lists:filter(fun(P) -> not plugin_provided_by_otp(P) end,
+                 Plugins).
+
+maybe_report_plugin_loading_problems([]) ->
+    ok;
+maybe_report_plugin_loading_problems(Problems) ->
+    rabbit_log:warning("Problem reading some plugins: ~p~n", [Problems]).
diff --git a/src/rabbit_plugins_main.erl b/src/rabbit_plugins_main.erl
index ff516268c6..758a9acaa4 100644
--- a/src/rabbit_plugins_main.erl
+++ b/src/rabbit_plugins_main.erl
@@ -169,9 +169,9 @@ format_plugins(Node, Pattern, Opts, #cli{all      = All,
 
     EnabledImplicitly = Implicit -- Enabled,
     {StatusMsg, Running} =
-        case rabbit_misc:rpc_call(Node, rabbit_plugins, active, []) of
-            {badrpc, _} -> {"[failed to contact ~s - status not shown]", []};
-            Active      -> {"* = running on ~s", Active}
+        case remote_running_plugins(Node) of
+            {ok, Active} -> {"* = running on ~s", Active};
+            error        -> {"[failed to contact ~s - status not shown]", []}
         end,
     {ok, RE} = re:compile(Pattern),
     Plugins = [ Plugin ||
@@ -196,7 +196,7 @@ format_plugins(Node, Pattern, Opts, #cli{all      = All,
                    Format, MaxWidth) || P <- Plugins1],
     ok.
 
-format_plugin(#plugin{name = Name, version = Version,
+format_plugin(#plugin{name = Name, version = OnDiskVersion,
                       description = Description, dependencies = Deps},
               Enabled, EnabledImplicitly, Running, Format,
               MaxWidth) ->
@@ -206,7 +206,7 @@ format_plugin(#plugin{name = Name, version = Version,
                        {false, true} -> "e";
                        _             -> " "
                    end,
-    RunningGlyph = case lists:member(Name, Running) of
+    RunningGlyph = case lists:keymember(Name, 1, Running) of
                        true  -> "*";
                        false -> " "
                    end,
@@ -214,6 +214,7 @@ format_plugin(#plugin{name = Name, version = Version,
     Opt = fun (_F, A, A) -> ok;
               ( F, A, _) -> io:format(F, [A])
           end,
+    Version = format_running_plugin_version(Name, OnDiskVersion, Running),
     case Format of
         minimal -> io:format("~s~n", [Name]);
         normal  -> io:format("~s ~-" ++ integer_to_list(MaxWidth) ++ "w ",
@@ -305,3 +306,31 @@ rpc_call(Node, Online, Mod, Fun, Args) ->
 
 plur([_]) -> "";
 plur(_)   -> "s".
+
+-spec remote_running_plugins(node()) -> [{atom(), Vsn :: string()}].
+remote_running_plugins(Node) ->
+    case rabbit_misc:rpc_call(Node, rabbit_plugins, active, []) of
+        {badrpc, _} -> error;
+        Active      -> maybe_augment_with_versions(Node, Active)
+    end.
+
+-spec maybe_augment_with_versions(node(), [atom()]) -> [{atom(), Vsn :: string()}].
+maybe_augment_with_versions(Node, Plugins) ->
+    case rabbit_misc:rpc_call(Node, rabbit_misc, which_applications, []) of
+        {badrpc, _} ->
+            error;
+        All ->
+            {ok, [{App, Vsn} || {App, _, Vsn} <- All,
+                                lists:member(App, Plugins)]}
+    end.
+
+-spec format_running_plugin_version(atom(), string(), [{atom(), Vsn :: string()}]) -> string().
+format_running_plugin_version(Name, OnDiskVersion, RunningPlugins) ->
+    case lists:keyfind(Name, 1, RunningPlugins) of
+        false ->
+            OnDiskVersion;
+        {_, OnDiskVersion} ->
+            OnDiskVersion;
+        {_, RunningVersion} ->
+            io_lib:format("~s (pending upgrade to ~s)", [RunningVersion, OnDiskVersion])
+    end.
diff --git a/test/unit_SUITE.erl b/test/unit_SUITE.erl
index 8ac3d41afe..8499fd2abc 100644
--- a/test/unit_SUITE.erl
+++ b/test/unit_SUITE.erl
@@ -32,7 +32,6 @@ groups() ->
     [
       {parallel_tests, [parallel], [
           arguments_parser,
-          mutually_exclusive_flags_parsing,
           {basic_header_handling, [parallel], [
               write_table_with_invalid_existing_type,
               invalid_existing_headers,
@@ -43,6 +42,8 @@ groups() ->
           content_framing,
           content_transcoding,
           decrypt_config,
+          listing_plugins_from_multiple_directories,
+          mutually_exclusive_flags_parsing,
           rabbitmqctl_encode,
           pg_local,
           pmerge,
@@ -968,3 +969,34 @@ unfold(_Config) ->
                                        (N) -> {true, N*2, N-1}
                                    end, 10),
     passed.
+
+listing_plugins_from_multiple_directories(Config) ->
+    %% Generate some fake plugins in .ez files
+    FirstDir = filename:join([?config(priv_dir, Config), "listing_plugins_from_multiple_directories-1"]),
+    SecondDir = filename:join([?config(priv_dir, Config), "listing_plugins_from_multiple_directories-2"]),
+    ok = file:make_dir(FirstDir),
+    ok = file:make_dir(SecondDir),
+    lists:foreach(fun({Dir, AppName, Vsn}) ->
+                          EzName = filename:join([Dir, io_lib:format("~s-~s.ez", [AppName, Vsn])]),
+                          AppFileName = lists:flatten(io_lib:format("~s-~s/ebin/~s.app", [AppName, Vsn, AppName])),
+                          AppFileContents = list_to_binary(io_lib:format("~p.", [{application, AppName, [{vsn, Vsn}]}])),
+                          {ok, {_, EzData}} = zip:zip(EzName, [{AppFileName, AppFileContents}], [memory]),
+                          ok = file:write_file(EzName, EzData)
+                  end,
+                  [{FirstDir, plugin_first_dir, "3"},
+                   {SecondDir, plugin_second_dir, "4"},
+                   {FirstDir, plugin_both, "1"},
+                   {SecondDir, plugin_both, "2"}]),
+
+    %% Everything was collected from both directories, plugin with higher version should take precedence
+    Path = FirstDir ++ ":" ++ SecondDir,
+    Got = lists:sort([{Name, Vsn} || #plugin{name = Name, version = Vsn} <- rabbit_plugins:list(Path)]),
+    Expected = [{plugin_both, "2"}, {plugin_first_dir, "3"}, {plugin_second_dir, "4"}],
+    case Got of
+        Expected ->
+            ok;
+        _ ->
+            ct:pal("Got ~p~nExpected: ~p", [Got, Expected]),
+            exit({wrong_plugins_list, Got})
+    end,
+    ok.