Merge pull request #642 from rabbitmq/rabbitmq-server-591

Validate plugin compatibility with version ranges

3 files changed, 178 insertions, 34 deletions

diff --git a/src/rabbit.app.src b/src/rabbit.app.src
index 83e7237c80..738a38e2bb 100644
--- a/src/rabbit.app.src
+++ b/src/rabbit.app.src
@@ -1,4 +1,5 @@
-{application, rabbit,           %% -*- erlang -*-
+%% -*- erlang -*-
+{application, rabbit,
  [{description, "RabbitMQ"},
   {id, "RabbitMQ"},
   {vsn, "0.0.0"},
diff --git a/src/rabbit_plugins.erl b/src/rabbit_plugins.erl
index 2f084ed28a..8f7319182e 100644
--- a/src/rabbit_plugins.erl
+++ b/src/rabbit_plugins.erl
@@ -21,9 +21,11 @@
 -export([setup/0, active/0, read_enabled/1, list/1, list/2, dependencies/3]).
 -export([ensure/1]).
 -export([extract_schemas/1]).
+-export([validate_plugins/1, format_invalid_plugins/1]).
 
+% Export for testing purpose.
+-export([is_version_supported/2, validate_plugins/2]).
 %%----------------------------------------------------------------------------
-
 -ifdef(use_specs).
 
 -type(plugin_name() :: atom()).
@@ -97,14 +99,14 @@ extract_schemas(SchemaDir) ->
     ok.
 
 extract_schema(#plugin{type = ez, location = Location}, SchemaDir) ->
-    {ok, Files} = zip:extract(Location, 
-                              [memory, {file_filter, 
-                                        fun(#zip_file{name = Name}) -> 
-                                            string:str(Name, "priv/schema") > 0 
+    {ok, Files} = zip:extract(Location,
+                              [memory, {file_filter,
+                                        fun(#zip_file{name = Name}) ->
+                                            string:str(Name, "priv/schema") > 0
                                         end}]),
     lists:foreach(
         fun({FileName, Content}) ->
-            ok = file:write_file(filename:join([SchemaDir, 
+            ok = file:write_file(filename:join([SchemaDir,
                                                 filename:basename(FileName)]),
                                  Content)
         end,
@@ -112,16 +114,16 @@ extract_schema(#plugin{type = ez, location = Location}, SchemaDir) ->
     ok;
 extract_schema(#plugin{type = dir, location = Location}, SchemaDir) ->
     PluginSchema = filename:join([Location,
-                                  "priv", 
-                                  "schema"]), 
+                                  "priv",
+                                  "schema"]),
     case rabbit_file:is_dir(PluginSchema) of
         false -> ok;
-        true  -> 
-            PluginSchemaFiles = 
+        true  ->
+            PluginSchemaFiles =
                 [ filename:join(PluginSchema, FileName)
-                  || FileName <- rabbit_file:wildcard(".*\\.schema", 
+                  || FileName <- rabbit_file:wildcard(".*\\.schema",
                                                       PluginSchema) ],
-            [ file:copy(SchemaFile, SchemaDir) 
+            [ file:copy(SchemaFile, SchemaDir)
               || SchemaFile <- PluginSchemaFiles ]
     end.
 
@@ -146,27 +148,31 @@ list(PluginsDir, IncludeRequiredDeps) ->
     %% instance.
     application:load(rabbit),
     {ok, RabbitDeps} = application:get_key(rabbit, applications),
+    AllPlugins = [plugin_info(PluginsDir, Plug) || Plug <- EZs ++ FreeApps],
     {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]),
+        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, {[], []},
+            AllPlugins),
     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).
@@ -196,8 +202,9 @@ dependencies(Reverse, Sources, AllPlugins) ->
                 false -> digraph_utils:reachable(Sources, G);
                 true  -> digraph_utils:reaching(Sources, G)
             end,
+    OrderedDests = digraph_utils:postorder(digraph_utils:subgraph(G, Dests)),
     true = digraph:delete(G),
-    Dests.
+    OrderedDests.
 
 %% For a few known cases, an externally provided plugin can be trusted.
 %% In this special case, it overrides the plugin.
@@ -245,19 +252,124 @@ prepare_plugins(Enabled) ->
     AllPlugins = list(PluginsDistDir),
     Wanted = dependencies(false, Enabled, AllPlugins),
     WantedPlugins = lookup_plugins(Wanted, AllPlugins),
-
+    {ValidPlugins, Problems} = validate_plugins(WantedPlugins),
+    %% TODO: error message formatting
+    rabbit_log:warning(format_invalid_plugins(Problems)),
     case filelib:ensure_dir(ExpandDir ++ "/") of
         ok          -> ok;
         {error, E2} -> throw({error, {cannot_create_plugins_expand_dir,
                                       [ExpandDir, E2]}})
     end,
-
-    [prepare_plugin(Plugin, ExpandDir) || Plugin <- WantedPlugins],
+    [prepare_plugin(Plugin, ExpandDir) || Plugin <- ValidPlugins],
 
     [prepare_dir_plugin(PluginAppDescPath) ||
         PluginAppDescPath <- filelib:wildcard(ExpandDir ++ "/*/ebin/*.app")],
     Wanted.
 
+format_invalid_plugins(InvalidPlugins) ->
+    lists:flatten(["Failed to enable some plugins: \r\n"
+                   | [format_invalid_plugin(Plugin)
+                      || Plugin <- InvalidPlugins]]).
+
+format_invalid_plugin({Name, Errors}) ->
+    [io_lib:format("    ~p:~n", [Name])
+     | [format_invalid_plugin_error(Err) || Err <- Errors]].
+
+format_invalid_plugin_error({missing_dependency, Dep}) ->
+    io_lib:format("        Dependency is missing or invalid: ~p~n", [Dep]);
+%% a plugin doesn't support the effective broker version
+format_invalid_plugin_error({broker_version_mismatch, Version, Required}) ->
+    io_lib:format("        Plugin doesn't support current server version."
+                  " Actual broker version: ~p, supported by the plugin: ~p~n", [Version, Required]);
+%% one of dependencies of a plugin doesn't match its version requirements
+format_invalid_plugin_error({{dependency_version_mismatch, Version, Required}, Name}) ->
+    io_lib:format("        Version '~p' of dependency '~p' is unsupported."
+                  " Version ranges supported by the plugin: ~p~n",
+                  [Version, Name, Required]);
+format_invalid_plugin_error(Err) ->
+    io_lib:format("        Unknown error ~p~n", [Err]).
+
+validate_plugins(Plugins) ->
+    application:load(rabbit),
+    RabbitVersion = RabbitVersion = case application:get_key(rabbit, vsn) of
+                                        undefined -> "0.0.0";
+                                        {ok, Val} -> Val
+                                    end,
+    validate_plugins(Plugins, RabbitVersion).
+
+validate_plugins(Plugins, BrokerVersion) ->
+    lists:foldl(
+        fun(#plugin{name = Name,
+                    broker_version_requirements = BrokerVersionReqs,
+                    dependency_version_requirements = DepsVersions} = Plugin,
+            {Plugins0, Errors}) ->
+            case is_version_supported(BrokerVersion, BrokerVersionReqs) of
+                true  ->
+                    case BrokerVersion of
+                        "0.0.0" ->
+                            rabbit_log:warning(
+                                "Running development version of the broker."
+                                " Requirement ~p for plugin ~p is ignored.",
+                                [BrokerVersionReqs, Name]);
+                        _ -> ok
+                    end,
+                    case check_plugins_versions(Name, Plugins0, DepsVersions) of
+                        ok           -> {[Plugin | Plugins0], Errors};
+                        {error, Err} -> {Plugins0, [{Name, Err} | Errors]}
+                    end;
+                false ->
+                    Error = [{broker_version_mismatch, BrokerVersion, BrokerVersionReqs}],
+                    {Plugins0, [{Name, Error} | Errors]}
+            end
+        end,
+        {[],[]},
+        Plugins).
+
+check_plugins_versions(PluginName, AllPlugins, RequiredVersions) ->
+    ExistingVersions = [{Name, Vsn}
+                        || #plugin{name = Name, version = Vsn} <- AllPlugins],
+    Problems = lists:foldl(
+        fun({Name, Versions}, Acc) ->
+            case proplists:get_value(Name, ExistingVersions) of
+                undefined -> [{missing_dependency, Name} | Acc];
+                Version   ->
+                    case is_version_supported(Version, Versions) of
+                        true  ->
+                            case Version of
+                                "" ->
+                                    rabbit_log:warning(
+                                        "~p plugin version is not defined."
+                                        " Requirement ~p for plugin ~p is ignored",
+                                        [Versions, PluginName]);
+                                _  -> ok
+                            end,
+                            Acc;
+                        false ->
+                            [{{dependency_version_mismatch, Version, Versions}, Name} | Acc]
+                    end
+            end
+        end,
+        [],
+        RequiredVersions),
+    case Problems of
+        [] -> ok;
+        _  -> {error, Problems}
+    end.
+
+is_version_supported("", _)        -> true;
+is_version_supported("0.0.0", _)   -> true;
+is_version_supported(_Version, []) -> true;
+is_version_supported(Version, ExpectedVersions) ->
+    case lists:any(fun(ExpectedVersion) ->
+                       rabbit_misc:version_minor_equivalent(ExpectedVersion, Version)
+                       andalso
+                       rabbit_misc:version_compare(ExpectedVersion, Version, lte)
+                   end,
+                   ExpectedVersions) of
+        true  -> true;
+        false -> false
+    end.
+
 clean_plugins(Plugins) ->
     {ok, ExpandDir} = application:get_env(rabbit, plugins_expand_dir),
     [clean_plugin(Plugin, ExpandDir) || Plugin <- Plugins].
@@ -330,8 +442,12 @@ mkplugin(Name, Props, Type, Location) ->
     Version = proplists:get_value(vsn, Props, "0"),
     Description = proplists:get_value(description, Props, ""),
     Dependencies = proplists:get_value(applications, Props, []),
+    BrokerVersions = proplists:get_value(broker_version_requirements, Props, []),
+    DepsVersions = proplists:get_value(dependency_version_requirements, Props, []),
     #plugin{name = Name, version = Version, description = Description,
-            dependencies = Dependencies, location = Location, type = Type}.
+            dependencies = Dependencies, location = Location, type = Type,
+            broker_version_requirements = BrokerVersions,
+            dependency_version_requirements = DepsVersions}.
 
 read_app_file(EZ) ->
     case zip:list_dir(EZ) of
@@ -366,4 +482,9 @@ plugin_names(Plugins) ->
     [Name || #plugin{name = Name} <- Plugins].
 
 lookup_plugins(Names, AllPlugins) ->
-    [P || P = #plugin{name = Name} <- AllPlugins, lists:member(Name, Names)].
+    % Preserve order of Names
+    lists:map(
+        fun(Name) ->
+            lists:keyfind(Name, #plugin.name, AllPlugins)
+        end,
+        Names).
diff --git a/src/rabbit_plugins_main.erl b/src/rabbit_plugins_main.erl
index e248989a7a..d4df931f64 100644
--- a/src/rabbit_plugins_main.erl
+++ b/src/rabbit_plugins_main.erl
@@ -99,6 +99,12 @@ action(enable, Node, ToEnable0, Opts, State = #cli{all      = All,
         _  -> throw({error_string, fmt_missing(Missing)})
     end,
     NewEnabled = lists:usort(Enabled ++ ToEnable),
+    Invalid = validate_plugins(NewEnabled, State),
+    case Invalid of
+        [] -> ok;
+        _  -> throw({error_string, 
+                     rabbit_plugins:format_invalid_plugins(Invalid)})
+    end,
     NewImplicit = write_enabled_plugins(NewEnabled, State),
     case NewEnabled -- Implicit of
         [] -> io:format("Plugin configuration unchanged.~n");
@@ -115,6 +121,12 @@ action(set, Node, NewEnabled0, Opts, State = #cli{all      = All,
         [] -> ok;
         _  -> throw({error_string, fmt_missing(Missing)})
     end,
+    Invalid = validate_plugins(NewEnabled, State),
+    case Invalid of
+        [] -> ok;
+        _  -> throw({error_string, 
+                     rabbit_plugins:format_invalid_plugins(Invalid)})
+    end,
     NewImplicit = write_enabled_plugins(NewEnabled, State),
     case NewImplicit of
         [] -> io:format("All plugins are now disabled.~n");
@@ -155,6 +167,16 @@ action(help, _Node, _Args, _Opts, _State) ->
 
 %%----------------------------------------------------------------------------
 
+validate_plugins(Names, #cli{all = All}) ->
+    Deps = rabbit_plugins:dependencies(false, Names, All),
+    DepsPlugins = lists:map(
+        fun(Name) ->
+            lists:keyfind(Name, #plugin.name, All)
+        end,
+        Deps),
+    {_, Errors} = rabbit_plugins:validate_plugins(DepsPlugins),
+    Errors.
+
 %% Pretty print a list of plugins.
 format_plugins(Node, Pattern, Opts, #cli{all      = All,
                                          enabled  = Enabled,