-
Notifications
You must be signed in to change notification settings - Fork 3.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Validate plugin compatibility with version ranges #642
Merged
Merged
Changes from 13 commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
b40dd37
Rabbit broker version check in plugins
de353f8
Clean version check function
a2222ac
plugin versions
03a7fde
Plugin versions check
d3676b5
Check deps versions before missing dependencies
2882618
Validate plugins before enabling
47baba0
Error messages in rabbitmq-plugins
9b23813
Log invalid plugins during startup
20a2d28
Test exports
36d9c69
Merge branch 'master' into rabbitmq-server-591
77cf284
Reverted version in app.src
64540f2
Naming
0648d40
Merge branch 'master' into rabbitmq-server-591
michaelklishin 2f58c3a
Naming
michaelklishin 75e64e7
Naming, cosmetics
michaelklishin 5ff8cec
Wording
michaelklishin 38ad975
Versions "" and "0.0.0" always match
9f59ed5
Log development version ignore
aeecbec
Cosmetics
michaelklishin 5cdc5d5
Wording
michaelklishin de2dbed
Wording
michaelklishin File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,9 +21,12 @@ | |
-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,31 +100,31 @@ 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, | ||
Files), | ||
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 +149,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 +203,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 +253,103 @@ 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]); | ||
format_invalid_plugin_error({broker_version_mismatch, Version, Required}) -> | ||
io_lib:format(" Broker version is invalid." | ||
" Current version: ~p Required: ~p~n", [Version, Required]); | ||
format_invalid_plugin_error({{version_mismatch, Version, Required}, Name}) -> | ||
io_lib:format(" ~p plugin version is invalid." | ||
" Current version: ~p Required: ~p~n", | ||
[Name, Version, 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, RabbitVersion) -> | ||
lists:foldl( | ||
fun(#plugin{name = Name, | ||
broker_version_requirements = RabbitmqVersions, | ||
dependency_version_requirements = DepsVersions} = Plugin, | ||
{Plugins0, Errors}) -> | ||
case is_version_supported(RabbitVersion, RabbitmqVersions) of | ||
true -> | ||
case check_plugins_versions(Plugins0, DepsVersions) of | ||
ok -> {[Plugin | Plugins0], Errors}; | ||
{error, Err} -> {Plugins0, [{Name, Err} | Errors]} | ||
end; | ||
false -> | ||
Error = [{broker_version_mismatch, RabbitVersion, RabbitmqVersions}], | ||
{Plugins0, [{Name, Error} | Errors]} | ||
end | ||
end, | ||
{[],[]}, | ||
Plugins). | ||
|
||
check_plugins_versions(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 -> Acc; | ||
false -> | ||
[{{version_mismatch, Version, Versions}, Name} | Acc] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should rename this to |
||
end | ||
end | ||
end, | ||
[], | ||
RequiredVersions), | ||
case Problems of | ||
[] -> ok; | ||
_ -> {error, Problems} | ||
end. | ||
|
||
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 +422,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, []), | ||
RabbitmqVersions = 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 = RabbitmqVersions, | ||
dependency_version_requirements = DepsVersions}. | ||
|
||
read_app_file(EZ) -> | ||
case zip:list_dir(EZ) of | ||
|
@@ -366,4 +462,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). |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we address this?