diff --git a/apps/emqx_management/src/emqx_mgmt_api_plugins.erl b/apps/emqx_management/src/emqx_mgmt_api_plugins.erl index 28deac98d..d1acfee6a 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_plugins.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_plugins.erl @@ -17,7 +17,6 @@ -behaviour(minirest_api). --include_lib("kernel/include/file.hrl"). -include_lib("typerefl/include/types.hrl"). -include_lib("emqx/include/logger.hrl"). %%-include_lib("emqx_plugins/include/emqx_plugins.hrl"). @@ -326,7 +325,8 @@ upload_install(post, #{body := #{<<"plugin">> := Plugin}}) when is_map(Plugin) - %% File bin is too large, we use rpc:multicall instead of cluster_rpc:multicall %% TODO what happens when a new node join in? %% emqx_plugins_monitor should copy plugins from other core node when boot-up. - case emqx_plugins:describe(string:trim(FileName, trailing, ".tar.gz")) of + NameVsn = string:trim(FileName, trailing, ".tar.gz"), + case emqx_plugins:describe(NameVsn) of {error, #{error := "bad_info_file", return := {enoent, _}}} -> case emqx_plugins:parse_name_vsn(FileName) of {ok, AppName, _Vsn} -> @@ -346,6 +346,7 @@ upload_install(post, #{body := #{<<"plugin">> := Plugin}}) when is_map(Plugin) - }} end; {error, Reason} -> + emqx_plugins:delete_package(NameVsn), {400, #{ code => 'BAD_PLUGIN_INFO', message => iolist_to_binary([Reason, ":", FileName]) @@ -367,9 +368,24 @@ upload_install(post, #{}) -> do_install_package(FileName, Bin) -> %% TODO: handle bad nodes {[_ | _] = Res, []} = emqx_mgmt_api_plugins_proto_v1:install_package(FileName, Bin), - %% TODO: handle non-OKs - [] = lists:filter(fun(R) -> R =/= ok end, Res), - {200}. + case lists:filter(fun(R) -> R =/= ok end, Res) of + [] -> + {200}; + Filtered -> + %% crash if we have unexpected errors or results + [] = lists:filter( + fun + ({error, {failed, _}}) -> true; + ({error, _}) -> false + end, + Filtered + ), + {error, #{error := Reason}} = hd(Filtered), + {400, #{ + code => 'BAD_PLUGIN_INFO', + message => iolist_to_binary([Reason, ":", FileName]) + }} + end. plugin(get, #{bindings := #{name := Name}}) -> {Plugins, _} = emqx_mgmt_api_plugins_proto_v1:describe_package(Name), @@ -408,7 +424,15 @@ install_package(FileName, Bin) -> File = filename:join(emqx_plugins:install_dir(), FileName), ok = file:write_file(File, Bin), PackageName = string:trim(FileName, trailing, ".tar.gz"), - emqx_plugins:ensure_installed(PackageName). + case emqx_plugins:ensure_installed(PackageName) of + {error, #{return := not_found}} = NotFound -> + NotFound; + {error, _Reason} = Error -> + _ = file:delete(File), + Error; + Result -> + Result + end. %% For RPC plugin get describe_package(Name) -> diff --git a/apps/emqx_plugins/src/emqx_plugins.app.src b/apps/emqx_plugins/src/emqx_plugins.app.src index 1635bb516..de56099ba 100644 --- a/apps/emqx_plugins/src/emqx_plugins.app.src +++ b/apps/emqx_plugins/src/emqx_plugins.app.src @@ -1,7 +1,7 @@ %% -*- mode: erlang -*- {application, emqx_plugins, [ {description, "EMQX Plugin Management"}, - {vsn, "0.1.0"}, + {vsn, "0.1.1"}, {modules, []}, {mod, {emqx_plugins_app, []}}, {applications, [kernel, stdlib, emqx]}, diff --git a/apps/emqx_plugins/src/emqx_plugins.erl b/apps/emqx_plugins/src/emqx_plugins.erl index 6815760e9..8993404d4 100644 --- a/apps/emqx_plugins/src/emqx_plugins.erl +++ b/apps/emqx_plugins/src/emqx_plugins.erl @@ -16,8 +16,13 @@ -module(emqx_plugins). --include_lib("emqx/include/emqx.hrl"). -include_lib("emqx/include/logger.hrl"). +-include_lib("emqx/include/logger.hrl"). +-include("emqx_plugins.hrl"). + +-ifdef(TEST). +-include_lib("eunit/include/eunit.hrl"). +-endif. -export([ ensure_installed/1, @@ -56,10 +61,6 @@ -compile(nowarn_export_all). -endif. --include_lib("emqx/include/emqx.hrl"). --include_lib("emqx/include/logger.hrl"). --include("emqx_plugins.hrl"). - %% "my_plugin-0.1.0" -type name_vsn() :: binary() | string(). %% the parse result of the JSON info file @@ -87,14 +88,15 @@ ensure_installed(NameVsn) -> do_ensure_installed(NameVsn) -> TarGz = pkg_file(NameVsn), - case erl_tar:extract(TarGz, [{cwd, install_dir()}, compressed]) of - ok -> + case erl_tar:extract(TarGz, [compressed, memory]) of + {ok, TarContent} -> + ok = write_tar_file_content(install_dir(), TarContent), case read_plugin(NameVsn, #{}) of {ok, _} -> ok; {error, Reason} -> ?SLOG(warning, Reason#{msg => "failed_to_read_after_install"}), - _ = ensure_uninstalled(NameVsn), + ok = delete_tar_file_content(install_dir(), TarContent), {error, Reason} end; {error, {_, enoent}} -> @@ -111,6 +113,66 @@ do_ensure_installed(NameVsn) -> }} end. +write_tar_file_content(BaseDir, TarContent) -> + lists:foreach( + fun({Name, Bin}) -> + Filename = filename:join(BaseDir, Name), + ok = filelib:ensure_dir(Filename), + ok = file:write_file(Filename, Bin) + end, + TarContent + ). + +delete_tar_file_content(BaseDir, TarContent) -> + lists:foreach( + fun({Name, _}) -> + Filename = filename:join(BaseDir, Name), + case filelib:is_file(Filename) of + true -> + TopDirOrFile = top_dir(BaseDir, Filename), + ok = file:del_dir_r(TopDirOrFile); + false -> + %% probably already deleted + ok + end + end, + TarContent + ). + +top_dir(BaseDir0, DirOrFile) -> + BaseDir = normalize_dir(BaseDir0), + case filename:dirname(DirOrFile) of + RockBottom when RockBottom =:= "/" orelse RockBottom =:= "." -> + throw({out_of_bounds, DirOrFile}); + BaseDir -> + DirOrFile; + Parent -> + top_dir(BaseDir, Parent) + end. + +normalize_dir(Dir) -> + %% Get rid of possible trailing slash + filename:join([Dir, ""]). + +-ifdef(TEST). +normalize_dir_test_() -> + [ + ?_assertEqual("foo", normalize_dir("foo")), + ?_assertEqual("foo", normalize_dir("foo/")), + ?_assertEqual("/foo", normalize_dir("/foo")), + ?_assertEqual("/foo", normalize_dir("/foo/")) + ]. + +top_dir_test_() -> + [ + ?_assertEqual("base/foo", top_dir("base", filename:join(["base", "foo", "bar"]))), + ?_assertEqual("/base/foo", top_dir("/base", filename:join(["/", "base", "foo", "bar"]))), + ?_assertEqual("/base/foo", top_dir("/base/", filename:join(["/", "base", "foo", "bar"]))), + ?_assertThrow({out_of_bounds, _}, top_dir("/base", filename:join(["/", "base"]))), + ?_assertThrow({out_of_bounds, _}, top_dir("/base", filename:join(["/", "foo", "bar"]))) + ]. +-endif. + %% @doc Ensure files and directories for the given plugin are being deleted. %% If a plugin is running, or enabled, an error is returned. -spec ensure_uninstalled(name_vsn()) -> ok | {error, any()}.