Merge pull request #14289 from zmstone/241127-fix-log-file-paths

fix(config): do not resolve EMQX_LOG_DIR in config converter

apps/emqx/src/config/emqx_config_logger.erl

@@ -169,7 +169,7 @@ tr_file_handler({HandlerName, SubConf}) ->
         level => conf_get("level", SubConf),
         config => HandlerConf#{
             type => Type,
-            file => FilePath,
+            file => emqx_schema:naive_env_interpolation(FilePath),
             max_no_files => RotationCount,
             max_no_bytes => RotationSize
         },
@@ -187,6 +187,9 @@ logger_file_handlers(Conf) ->
     logger_handlers(Handlers).
 
 logger_handlers(Handlers) ->
+    keep_only_enabeld(Handlers).
+
+keep_only_enabeld(Handlers) ->
     lists:filter(
         fun({_Name, Handler}) ->
             conf_get("enable", Handler, false)

apps/emqx_conf/src/emqx_conf_schema.erl

@@ -41,7 +41,13 @@
 -export([tr_prometheus_collectors/1]).
 
 %% internal exports for `emqx_enterprise_schema' only.
--export([ensure_unicode_path/2, convert_rotation/2, log_handler_common_confs/2]).
+-export([
+    log_file_path_converter/2,
+    fix_old_version_abs_log_path/1,
+    ensure_unicode_path/2,
+    convert_rotation/2,
+    log_handler_common_confs/2
+]).
 
 -define(DEFAULT_NODE_NAME, <<"emqx@127.0.0.1">>).
 
@@ -955,9 +961,7 @@ fields("log_file_handler") ->
                     default => <<"${EMQX_LOG_DIR}/emqx.log">>,
                     aliases => [file, to],
                     importance => ?IMPORTANCE_HIGH,
-                    converter => fun(Path, Opts) ->
-                        emqx_schema:naive_env_interpolation(ensure_unicode_path(Path, Opts))
-                    end
+                    converter => fun log_file_path_converter/2
                 }
             )},
         {"rotation_count",
@@ -1519,6 +1523,59 @@ convert_rotation(#{} = Rotation, _Opts) -> maps:get(<<"count">>, Rotation, 10);
 convert_rotation(Count, _Opts) when is_integer(Count) -> Count;
 convert_rotation(Count, _Opts) -> throw({"bad_rotation", Count}).
 
+log_file_path_converter(Path, Opts) ->
+    Fixed = fix_old_version_abs_log_path(Path),
+    ensure_unicode_path(Fixed, Opts).
+
+%% Prior to 5.8.3, the log file paths are resolved by scehma module
+%% and the interpolated paths (absolute paths) are exported.
+%% When exported from docker but import to a non-docker environment,
+%% the absolute paths are not valid anymore.
+%% Here we try to fix the old version absolute paths.
+fix_old_version_abs_log_path(Bin) when is_binary(Bin) ->
+    try
+        List = [_ | _] = unicode:characters_to_list(Bin, utf8),
+        Fixed = fix_old_version_abs_log_path(List),
+        unicode:characters_to_binary(Fixed, utf8)
+    catch
+        _:_ ->
+            %% defer the validation to ensure_unicode_path
+            Bin
+    end;
+fix_old_version_abs_log_path("/opt/emqx/log/" ++ Name) ->
+    maybe_subst_log_dir("/opt/emqx/log", Name);
+fix_old_version_abs_log_path("/var/log/emqx/" ++ Name) ->
+    maybe_subst_log_dir("/var/log/emqx", Name);
+fix_old_version_abs_log_path(Other) ->
+    %% undefined, or other log dir
+    Other.
+
+%% Substitute the log dir with environment variable EMQX_LOG_DIR
+%% when possible
+maybe_subst_log_dir(Dir, Name) ->
+    Env = os:getenv("EMQX_LOG_DIR"),
+    IsEnvSet = (Env =/= false andalso Env =/= ""),
+    case Env =:= Dir of
+        true ->
+            %% the path is the same as the environment variable
+            %% substitute it with the environment variable
+            "${EMQX_LOG_DIR}/" ++ Name;
+        false ->
+            case filelib:is_dir(Dir) of
+                true ->
+                    %% the path exists, keep it
+                    filename:join(Dir, Name);
+                false when IsEnvSet ->
+                    %% the path does not exist, but the environment variable is set
+                    %% substitute it with the environment variable
+                    "${EMQX_LOG_DIR}/" ++ Name;
+                false ->
+                    %% the path does not exist, and the environment variable is not set
+                    %% keep it
+                    filename:join(Dir, Name)
+            end
+    end.
+
 ensure_unicode_path(Path, Opts) ->
     emqx_schema:ensure_unicode_path(Path, Opts).
 

apps/emqx_conf/test/emqx_conf_schema_tests.erl

@@ -509,7 +509,7 @@ log_path_test_() ->
     end,
 
     [
-        {"default-values", fun() -> Assert(default, "log/emqx.log", check(#{})) end},
+        {"default-values", fun() -> Assert(default, "${EMQX_LOG_DIR}/emqx.log", check(#{})) end},
         {"file path with space", fun() -> Assert(name1, "a /b", check(Fh(<<"a /b">>))) end},
         {"windows", fun() -> Assert(name1, "c:\\a\\ b\\", check(Fh(<<"c:\\a\\ b\\">>))) end},
         {"unicoded", fun() -> Assert(name1, "路 径", check(Fh(<<"路 径"/utf8>>))) end},
@@ -714,3 +714,33 @@ node_role_conf(Role0) ->
     Hocon = <<"node { role =", Role/binary, ", cookie = \"cookie\", data_dir = \".\" }">>,
     {ok, ConfMap} = hocon:binary(Hocon, #{format => map}),
     ConfMap.
+
+fix_log_dir_path_test() ->
+    ?assertEqual(
+        "/opt/emqx/log/a.log",
+        emqx_conf_schema:fix_old_version_abs_log_path("/opt/emqx/log/a.log")
+    ),
+    ?assertEqual(
+        "/var/log/emqx/a.log",
+        emqx_conf_schema:fix_old_version_abs_log_path("/var/log/emqx/a.log")
+    ),
+    try
+        os:putenv("EMQX_LOG_DIR", "foobar"),
+        %% assumption: the two hard coded paths below do not exist in CT test runner
+        ?assertEqual(
+            "${EMQX_LOG_DIR}/a.log",
+            emqx_conf_schema:fix_old_version_abs_log_path("/var/log/emqx/a.log")
+        ),
+        ?assertEqual(
+            "${EMQX_LOG_DIR}/a.log",
+            emqx_conf_schema:fix_old_version_abs_log_path("/opt/emqx/log/a.log")
+        ),
+        %% binary in binary out
+        ?assertEqual(
+            <<"${EMQX_LOG_DIR}/a.log">>,
+            emqx_conf_schema:fix_old_version_abs_log_path(<<"/var/log/emqx/a.log">>)
+        )
+    after
+        os:unsetenv("EMQX_LOG_DIR")
+    end,
+    ok.

apps/emqx_enterprise/src/emqx_enterprise.app.src

@@ -1,6 +1,6 @@
 {application, emqx_enterprise, [
     {description, "EMQX Enterprise Edition"},
-    {vsn, "0.2.4"},
+    {vsn, "0.2.5"},
     {registered, []},
     {applications, [
         kernel,

apps/emqx_enterprise/src/emqx_enterprise_schema.erl

@@ -64,11 +64,7 @@ fields("log_audit_handler") ->
                     desc => ?DESC(emqx_conf_schema, "audit_file_handler_path"),
                     default => <<"${EMQX_LOG_DIR}/audit.log">>,
                     importance => ?IMPORTANCE_HIGH,
-                    converter => fun(Path, Opts) ->
-                        emqx_schema:naive_env_interpolation(
-                            emqx_conf_schema:ensure_unicode_path(Path, Opts)
-                        )
-                    end
+                    converter => fun emqx_conf_schema:log_file_path_converter/2
                 }
             )},
         {"rotation_count",

apps/emqx_enterprise/test/emqx_enterprise_schema_SUITE.erl

@@ -78,7 +78,7 @@ t_audit_log_conf(_Config) ->
         <<"rotation_count">> => 10,
         <<"rotation_size">> => <<"50MB">>,
         <<"time_offset">> => <<"system">>,
-        <<"path">> => <<"log/emqx.log">>,
+        <<"path">> => <<"${EMQX_LOG_DIR}/emqx.log">>,
         <<"timestamp_format">> => <<"auto">>,
         <<"payload_encode">> => <<"text">>
     },
@@ -100,7 +100,7 @@ t_audit_log_conf(_Config) ->
             #{
                 <<"enable">> => false,
                 <<"level">> => <<"info">>,
-                <<"path">> => <<"log/audit.log">>,
+                <<"path">> => <<"${EMQX_LOG_DIR}/audit.log">>,
                 <<"ignore_high_frequency_request">> => true,
                 <<"max_filter_size">> => 5000,
                 <<"rotation_count">> => 10,

changes/ce/fix-14289.en.md

@@ -0,0 +1,8 @@
+Fix log file path issue when importing config from a different environment.
+
+Environment variable `EMQX_LOG_DIR` in docker is `/opt/emqx/log`, but `/var/log/emqx/` when installed from RPM/DEB packages.
+
+Prior to this fix, log file paths (default file handler and audit handler) are environment-variable interpolated when being exported.
+This causes the config import to a different environment crash due to the directory being absent.
+
+This fix ensures that the log file paths are not environment-variable interpolated when being exported. It also made sure that the absolute paths log directory from old version export are converted back to environment-variable if the path does not exist in the new environment.