Merge pull request #7508 from zmstone/0402-4.3-fix-certs-file-path-when-not-uploaded

fix(ssl): try to guess cert file paths

apps/emqx_plugin_libs/src/emqx_plugin_libs.app.src

@@ -1,6 +1,6 @@
 {application, emqx_plugin_libs,
  [{description, "EMQ X Plugin utility libs"},
-  {vsn, "4.3.1"},
+  {vsn, "4.3.2"},
   {modules, []},
   {applications, [kernel,stdlib]},
   {env, []}

apps/emqx_plugin_libs/src/emqx_plugin_libs.appup.src

@@ -2,13 +2,13 @@
 
 {VSN,
   [
-    {<<"4.3.0">>, [
+    {<<"4\\.3\\.[0-1]">>, [
       {load_module, emqx_plugin_libs_ssl, brutal_purge, soft_purge, []}
     ]},
     {<<".*">>, []}
   ],
   [
-    {<<"4.3.0">>, [
+    {<<"4\\.3\\.[0-1]">>, [
       {load_module, emqx_plugin_libs_ssl, brutal_purge, soft_purge, []}
     ]},
     {<<".*">>, []}

apps/emqx_plugin_libs/src/emqx_plugin_libs_ssl.erl

@@ -58,9 +58,9 @@ save_files_return_opts(Options, Dir) ->
     KeyFile = Get(<<"keyfile">>),
     CertFile = Get(<<"certfile">>),
     CAFile = GetD(<<"cacertfile">>, Get(<<"cafile">>)),
-    Key = do_save_file(KeyFile, Dir),
-    Cert = do_save_file(CertFile, Dir),
-    CA = do_save_file(CAFile, Dir),
+    Key = maybe_save_file(KeyFile, Dir),
+    Cert = maybe_save_file(CertFile, Dir),
+    CA = maybe_save_file(CAFile, Dir),
     Verify = case GetD(<<"verify">>, false) of
                   false -> verify_none;
                   _ -> verify_peer
@@ -80,25 +80,47 @@ save_files_return_opts(Options, Dir) ->
 -spec save_file(file_input(), atom() | string() | binary()) -> string().
 save_file(Param, SubDir) ->
    Dir = filename:join([emqx:get_env(data_dir), SubDir]),
-   do_save_file( Param, Dir).
+   maybe_save_file(Param, Dir).
 
 filter([]) -> [];
 filter([{_, ""} | T]) -> filter(T);
 filter([{_, undefined} | T]) -> filter(T);
 filter([H | T]) -> [H | filter(T)].
 
-do_save_file(#{<<"filename">> := FileName, <<"file">> := Content}, Dir)
+maybe_save_file(#{<<"filename">> := FileName, <<"file">> := Content}, Dir)
   when FileName =/= undefined andalso Content =/= undefined ->
-    do_save_file(ensure_str(FileName), iolist_to_binary(Content), Dir);
-do_save_file(FilePath, _) when is_binary(FilePath) ->
+    maybe_save_file(ensure_str(FileName), iolist_to_binary(Content), Dir);
+maybe_save_file(FilePath, _) when is_binary(FilePath) ->
     ensure_str(FilePath);
-do_save_file(FilePath, _) when is_list(FilePath) ->
+maybe_save_file(FilePath, _) when is_list(FilePath) ->
     FilePath;
-do_save_file(_, _) -> "".
+maybe_save_file(_, _) -> "".
 
-do_save_file("", _, _Dir) -> ""; %% ignore
-do_save_file(_, <<>>, _Dir) -> ""; %% ignore
-do_save_file(FileName, Content, Dir) ->
+maybe_save_file("", _, _Dir) -> ""; %% no filename, ignore
+maybe_save_file(FileName, <<>>, Dir) ->  %% no content, see if file exists
+    {ok, Cwd} = file:get_cwd(),
+    %% NOTE: when FileName is an absolute path, filename:join has no effect
+    CwdFile = ensure_str(filename:join([Cwd, FileName])),
+    DataDirFile = ensure_str(filename:join([Dir, FileName])),
+    Possibles0 = case CwdFile =:= DataDirFile of
+                    true -> [CwdFile];
+                    false -> [CwdFile, DataDirFile]
+                end,
+    Possibles = Possibles0 ++
+                 case FileName of
+                     "etc/certs/" ++ Path ->
+                         %% this is the dir hard-coded in rule-engine resources as
+                         %% default, unfortunatly we cannot change the deaults
+                         %% due to compatibilty reasons, so we have to make a guess
+                         ["/etc/emqx/certs/" ++ Path];
+                     _ ->
+                         []
+                 end,
+    case find_exist_file(FileName, Possibles) of
+        false -> erlang:throw({bad_cert_file, Possibles});
+        Found -> Found
+    end;
+maybe_save_file(FileName, Content, Dir) ->
      FullFilename = filename:join([Dir, FileName]),
      ok = filelib:ensure_dir(FullFilename),
      case file:write_file(FullFilename, Content) of
@@ -112,3 +134,9 @@ do_save_file(FileName, Content, Dir) ->
 ensure_str(L) when is_list(L) -> L;
 ensure_str(B) when is_binary(B) -> unicode:characters_to_list(B, utf8).
 
+find_exist_file(_Name, []) -> false;
+find_exist_file(Name, [F | Rest]) ->
+    case filelib:is_regular(F) of
+        true -> F;
+        false -> find_exist_file(Name, Rest)
+    end.

apps/emqx_plugin_libs/test/emqx_plugin_libs_ssl_tests.erl

@@ -42,7 +42,8 @@ prop_file_or_content() ->
                         {prop_cert_file_name(), proper_types:binary()}]).
 
 prop_cert_file_name() ->
-    proper_types:oneof(["certname1", <<"certname2">>, "", <<>>, undefined]).
+    File = code:which(?MODULE), %% existing
+    proper_types:oneof(["", <<>>, undefined, File]).
 
 prop_tls_versions() ->
     proper_types:oneof(["tlsv1.3",
@@ -76,3 +77,10 @@ file_or_content({Name, Content}) ->
     #{<<"file">> => Content, <<"filename">> => Name};
 file_or_content(Name) ->
     Name.
+
+bad_cert_file_test() ->
+    Input = #{<<"keyfile">> =>
+                #{<<"filename">> => "notafile",
+                  <<"file">> => ""}},
+    ?assertThrow({bad_cert_file, _},
+                 emqx_plugin_libs_ssl:save_files_return_opts(Input, "test-data")).

apps/emqx_rule_engine/src/emqx_rule_engine.erl

@@ -232,7 +232,10 @@ delete_rule(RuleId) ->
     end.
 
 -spec(create_resource(#{type := _, config := _, _ => _}) -> {ok, resource()} | {error, Reason :: term()}).
-create_resource(#{type := Type, config := Config0} = Params) ->
+create_resource(Params) ->
+    create_resource(Params, with_retry).
+
+create_resource(#{type := Type, config := Config0} = Params, Retry) ->
     case emqx_rule_registry:find_resource_type(Type) of
         {ok, #resource_type{on_create = {M, F}, params_spec = ParamSpec}} ->
             Config = emqx_rule_validator:validate_params(Config0, ParamSpec),
@@ -244,10 +247,20 @@ create_resource(#{type := Type, config := Config0} = Params) ->
                                  created_at = erlang:system_time(millisecond)
                                 },
             ok = emqx_rule_registry:add_resource(Resource),
-            %% Note that we will return OK in case of resource creation failure,
-            %% A timer is started to re-start the resource later.
-            catch _ = ?CLUSTER_CALL(init_resource, [M, F, ResId, Config]),
-            {ok, Resource};
+            case Retry of
+                with_retry ->
+                    %% Note that we will return OK in case of resource creation failure,
+                    %% A timer is started to re-start the resource later.
+                    _ = (catch (?CLUSTER_CALL(init_resource, [M, F, ResId, Config]))),
+                    {ok, Resource};
+                no_retry ->
+                    try
+                        _ = ?CLUSTER_CALL(init_resource, [M, F, ResId, Config]),
+                        {ok, Resource}
+                    catch throw : Reason ->
+                        {error, Reason}
+                    end
+            end;
         not_found ->
             {error, {resource_type_not_found, Type}}
     end.
@@ -320,9 +333,19 @@ test_resource(#{type := Type} = Params) ->
         {ok, #resource_type{}} ->
             ResId = maps:get(id, Params, resource_id()),
             try
-                _ = create_resource(maps:put(id, ResId, Params)),
-                true = is_source_alive(ResId),
-                ok
+                case create_resource(maps:put(id, ResId, Params), no_retry) of
+                    {ok, _} ->
+                        case is_source_alive(ResId) of
+                            true ->
+                                ok;
+                            false ->
+                                %% in is_source_alive, the cluster-call RPC logs errors
+                                %% so we do not log anything here
+                                {error, {resource_down, ResId}}
+                        end;
+                    {error, Reason} ->
+                        {error, Reason}
+                end
             catch E:R:S ->
                 ?LOG(warning, "test resource failed, ~0p:~0p ~0p", [E, R, S]),
                 {error, R}

bin/install_upgrade.escript

@@ -262,19 +262,23 @@ make_symlink_or_copy(Filename, ReleaseLink) ->
 
 unpack_zipballs(RelNameStr, Version) ->
     {ok, Cwd} = file:get_cwd(),
-    GzFile = filename:absname(filename:join(["releases", RelNameStr ++ "-" ++ Version ++ ".tar.gz"])),
-    ZipFiles = filelib:wildcard(filename:join(["releases", RelNameStr ++ "-*" ++ Version ++ "*.zip"])),
-    ?INFO("unzip ~p", [ZipFiles]),
-    [begin
-        TmdTarD="/tmp/emqx_untar_" ++ integer_to_list(erlang:system_time()),
-        ok = filelib:ensure_dir(filename:join([TmdTarD, "dummy"])),
-        {ok, _} = file:copy(Zip, filename:join([TmdTarD, "emqx.zip"])),
-        ok = file:set_cwd(filename:join([TmdTarD])),
-        {ok, _FileList} = zip:unzip("emqx.zip"),
-        ok = file:set_cwd(filename:join([TmdTarD, "emqx"])),
-        ok = erl_tar:create(GzFile, filelib:wildcard("*"), [compressed])
-     end || Zip <- ZipFiles],
-    file:set_cwd(Cwd).
+    try
+        GzFile = filename:absname(filename:join(["releases", RelNameStr ++ "-" ++ Version ++ ".tar.gz"])),
+        ZipFiles = filelib:wildcard(filename:join(["releases", RelNameStr ++ "-*" ++ Version ++ "*.zip"])),
+        ?INFO("unzip ~p", [ZipFiles]),
+        [begin
+            TmdTarD="/tmp/emqx_untar_" ++ integer_to_list(erlang:system_time()),
+            ok = filelib:ensure_dir(filename:join([TmdTarD, "dummy"])),
+            {ok, _} = file:copy(Zip, filename:join([TmdTarD, "emqx.zip"])),
+            ok = file:set_cwd(filename:join([TmdTarD])),
+            {ok, _FileList} = zip:unzip("emqx.zip"),
+            ok = file:set_cwd(filename:join([TmdTarD, "emqx"])),
+            ok = erl_tar:create(GzFile, filelib:wildcard("*"), [compressed])
+        end || Zip <- ZipFiles]
+    after
+        % restore cwd
+        file:set_cwd(Cwd)
+    end.
 
 first_value(_Fun, []) -> no_value;
 first_value(Fun, [Value | Rest]) ->