From 5f4215b3333f8007bf6c1004417ab12da43161ef Mon Sep 17 00:00:00 2001
From: zmstone <zmstone@gmail.com>
Date: Sat, 27 Apr 2024 21:02:09 +0200
Subject: [PATCH 1/6] fix(mgmt): avoid 500 error when hocon syntax error

---
 apps/emqx/src/emqx_logger_jsonfmt.erl         |  9 +++--
 .../src/emqx_mgmt_api_configs.erl             |  7 +---
 .../test/emqx_mgmt_api_configs_SUITE.erl      | 38 ++++++++++++++-----
 3 files changed, 36 insertions(+), 18 deletions(-)

diff --git a/apps/emqx/src/emqx_logger_jsonfmt.erl b/apps/emqx/src/emqx_logger_jsonfmt.erl
index 2efb0e032..92c0bb561 100644
--- a/apps/emqx/src/emqx_logger_jsonfmt.erl
+++ b/apps/emqx/src/emqx_logger_jsonfmt.erl
@@ -32,7 +32,7 @@
 -export([format/2]).
 
 %% For CLI HTTP API outputs
--export([best_effort_json/1, best_effort_json/2]).
+-export([best_effort_json/1, best_effort_json/2, best_effort_json_obj/1]).
 
 -ifdef(TEST).
 -include_lib("proper/include/proper.hrl").
@@ -65,10 +65,13 @@
 best_effort_json(Input) ->
     best_effort_json(Input, [pretty, force_utf8]).
 best_effort_json(Input, Opts) ->
-    Config = #{depth => unlimited, single_line => true, chars_limit => unlimited},
-    JsonReady = best_effort_json_obj(Input, Config),
+    JsonReady = best_effort_json_obj(Input),
     emqx_utils_json:encode(JsonReady, Opts).
 
+best_effort_json_obj(Input) ->
+    Config = #{depth => unlimited, single_line => true, chars_limit => unlimited},
+    best_effort_json_obj(Input, Config).
+
 -spec format(logger:log_event(), config()) -> iodata().
 format(#{level := Level, msg := Msg, meta := Meta}, Config0) when is_map(Config0) ->
     Config = add_default_config(Config0),
diff --git a/apps/emqx_management/src/emqx_mgmt_api_configs.erl b/apps/emqx_management/src/emqx_mgmt_api_configs.erl
index f013dfcd1..decabb64b 100644
--- a/apps/emqx_management/src/emqx_mgmt_api_configs.erl
+++ b/apps/emqx_management/src/emqx_mgmt_api_configs.erl
@@ -366,11 +366,8 @@ configs(put, #{body := Conf, query_string := #{<<"mode">> := Mode}}, _Req) ->
         ok ->
             {200};
         %% bad hocon format
-        {error, MsgList = [{_, _} | _]} ->
-            JsonFun = fun(K, V) -> {K, emqx_utils_maps:binary_string(V)} end,
-            JsonMap = emqx_utils_maps:jsonable_map(maps:from_list(MsgList), JsonFun),
-            {400, #{<<"content-type">> => <<"text/plain">>}, JsonMap};
-        {error, Msg} ->
+        {error, Errors} ->
+            Msg = emqx_logger_jsonfmt:best_effort_json_obj(#{errors => Errors}),
             {400, #{<<"content-type">> => <<"text/plain">>}, Msg}
     end.
 
diff --git a/apps/emqx_management/test/emqx_mgmt_api_configs_SUITE.erl b/apps/emqx_management/test/emqx_mgmt_api_configs_SUITE.erl
index 2c90c9dac..80b9aa9df 100644
--- a/apps/emqx_management/test/emqx_mgmt_api_configs_SUITE.erl
+++ b/apps/emqx_management/test/emqx_mgmt_api_configs_SUITE.erl
@@ -336,13 +336,15 @@ t_configs_key(_Config) ->
     BadLog = emqx_utils_maps:deep_put([<<"log">>, <<"console">>, <<"level">>], Log, <<"erro1r">>),
     {error, Error} = update_configs_with_binary(iolist_to_binary(hocon_pp:do(BadLog, #{}))),
     ExpectError = #{
-        <<"log">> =>
-            #{
-                <<"kind">> => <<"validation_error">>,
-                <<"path">> => <<"log.console.level">>,
-                <<"reason">> => <<"unable_to_convert_to_enum_symbol">>,
-                <<"value">> => <<"erro1r">>
-            }
+        <<"errors">> => #{
+            <<"log">> =>
+                #{
+                    <<"kind">> => <<"validation_error">>,
+                    <<"path">> => <<"log.console.level">>,
+                    <<"reason">> => <<"unable_to_convert_to_enum_symbol">>,
+                    <<"value">> => <<"erro1r">>
+                }
+        }
     },
     ?assertEqual(ExpectError, emqx_utils_json:decode(Error, [return_maps])),
     ReadOnlyConf = #{
@@ -355,7 +357,7 @@ t_configs_key(_Config) ->
     },
     ReadOnlyBin = iolist_to_binary(hocon_pp:do(ReadOnlyConf, #{})),
     {error, ReadOnlyError} = update_configs_with_binary(ReadOnlyBin),
-    ?assertEqual(<<"Cannot update read-only key 'cluster'.">>, ReadOnlyError),
+    ?assertEqual(<<"{\"errors\":\"Cannot update read-only key 'cluster'.\"}">>, ReadOnlyError),
     ok.
 
 t_get_configs_in_different_accept(_Config) ->
@@ -487,6 +489,22 @@ t_create_webhook_v1_bridges_api(Config) ->
     ?assertEqual(#{<<"webhook">> => #{}}, emqx_conf:get_raw([<<"bridges">>])),
     ok.
 
+t_config_update_parse_error(_Config) ->
+    ?assertMatch(
+        {error, <<"{\"errors\":\"{parse_error,", _/binary>>},
+        update_configs_with_binary(<<"not an object">>)
+    ),
+    ?assertMatch(
+        {error, <<"{\"errors\":\"{parse_error,", _/binary>>},
+        update_configs_with_binary(<<"a = \"tlsv1\"\"\"3e-01">>)
+    ).
+
+t_config_update_unknown_root(_Config) ->
+    ?assertMatch(
+        {error, <<"{\"errors\":{\"a\":\"{root_key_not_found,", _/binary>>},
+        update_configs_with_binary(<<"a = \"tlsv1.3\"">>)
+    ).
+
 %% Helpers
 
 get_config(Name) ->
@@ -547,10 +565,10 @@ update_configs_with_binary(Bin) ->
             Code >= 200 andalso Code =< 299
         ->
             Body;
-        {ok, {{"HTTP/1.1", _Code, _}, _Headers, Body}} ->
+        {ok, {{"HTTP/1.1", 400, _}, _Headers, Body}} ->
             {error, Body};
         Error ->
-            Error
+            error({unexpected, Error})
     end.
 
 update_config(Name, Change) ->

From e76c350d302c7a625f82688862beda89feccfa71 Mon Sep 17 00:00:00 2001
From: zmstone <zmstone@gmail.com>
Date: Sat, 27 Apr 2024 21:19:40 +0200
Subject: [PATCH 2/6] docs: add changelog for PR 12940

---
 changes/ce/fix-12940.en.md | 3 +++
 1 file changed, 3 insertions(+)
 create mode 100644 changes/ce/fix-12940.en.md

diff --git a/changes/ce/fix-12940.en.md b/changes/ce/fix-12940.en.md
new file mode 100644
index 000000000..8304effd5
--- /dev/null
+++ b/changes/ce/fix-12940.en.md
@@ -0,0 +1,3 @@
+Fix config update API `/configs` 500 error when config has bad HOCON syntax.
+
+Now bad syntax will cause the API to return 400 (BAD_REQUEST).

From 10625eacacc8cbced485b22a06d3643f57c4609f Mon Sep 17 00:00:00 2001
From: zmstone <zmstone@gmail.com>
Date: Sun, 28 Apr 2024 14:05:30 +0200
Subject: [PATCH 3/6] chore: upgrade to hocon-0.42.2

hocon pretty-print quotes more string values
if a string has '.' or '-', or if it starts with a digit 0-9,
then it's quoted.

see details here: https://github.com/emqx/hocon/pull/293
---
 apps/emqx/rebar.config | 2 +-
 mix.exs                | 2 +-
 rebar.config           | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/apps/emqx/rebar.config b/apps/emqx/rebar.config
index b58734cf8..b58cd0cb7 100644
--- a/apps/emqx/rebar.config
+++ b/apps/emqx/rebar.config
@@ -30,7 +30,7 @@
     {esockd, {git, "https://github.com/emqx/esockd", {tag, "5.11.2"}}},
     {ekka, {git, "https://github.com/emqx/ekka", {tag, "0.19.3"}}},
     {gen_rpc, {git, "https://github.com/emqx/gen_rpc", {tag, "3.3.1"}}},
-    {hocon, {git, "https://github.com/emqx/hocon.git", {tag, "0.42.1"}}},
+    {hocon, {git, "https://github.com/emqx/hocon.git", {tag, "0.42.2"}}},
     {emqx_http_lib, {git, "https://github.com/emqx/emqx_http_lib.git", {tag, "0.5.3"}}},
     {pbkdf2, {git, "https://github.com/emqx/erlang-pbkdf2.git", {tag, "2.0.4"}}},
     {recon, {git, "https://github.com/ferd/recon", {tag, "2.5.1"}}},
diff --git a/mix.exs b/mix.exs
index bbb11cc52..2cc44099a 100644
--- a/mix.exs
+++ b/mix.exs
@@ -72,7 +72,7 @@ defmodule EMQXUmbrella.MixProject do
       # in conflict by emqtt and hocon
       {:getopt, "1.0.2", override: true},
       {:snabbkaffe, github: "kafka4beam/snabbkaffe", tag: "1.0.8", override: true},
-      {:hocon, github: "emqx/hocon", tag: "0.42.1", override: true},
+      {:hocon, github: "emqx/hocon", tag: "0.42.2", override: true},
       {:emqx_http_lib, github: "emqx/emqx_http_lib", tag: "0.5.3", override: true},
       {:esasl, github: "emqx/esasl", tag: "0.2.1"},
       {:jose, github: "potatosalad/erlang-jose", tag: "1.11.2"},
diff --git a/rebar.config b/rebar.config
index e0f88893c..cd70493e5 100644
--- a/rebar.config
+++ b/rebar.config
@@ -97,7 +97,7 @@
     {system_monitor, {git, "https://github.com/ieQu1/system_monitor", {tag, "3.0.3"}}},
     {getopt, "1.0.2"},
     {snabbkaffe, {git, "https://github.com/kafka4beam/snabbkaffe.git", {tag, "1.0.8"}}},
-    {hocon, {git, "https://github.com/emqx/hocon.git", {tag, "0.42.1"}}},
+    {hocon, {git, "https://github.com/emqx/hocon.git", {tag, "0.42.2"}}},
     {emqx_http_lib, {git, "https://github.com/emqx/emqx_http_lib.git", {tag, "0.5.3"}}},
     {esasl, {git, "https://github.com/emqx/esasl", {tag, "0.2.1"}}},
     {jose, {git, "https://github.com/potatosalad/erlang-jose", {tag, "1.11.2"}}},

From 07cbdc6e90116bd8a678e93b50d0e3e077ebdbcd Mon Sep 17 00:00:00 2001
From: zmstone <zmstone@gmail.com>
Date: Sun, 28 Apr 2024 14:45:53 +0200
Subject: [PATCH 4/6] feat(mgmt): add ignore_readonly qeury-string to PUT
 /configs API

---
 apps/emqx_conf/src/emqx_conf_cli.erl          | 22 +++++++++++--------
 .../src/emqx_mgmt_api_configs.erl             | 13 ++++++++---
 .../test/emqx_mgmt_api_configs_SUITE.erl      | 20 +++++++++++++----
 3 files changed, 39 insertions(+), 16 deletions(-)

diff --git a/apps/emqx_conf/src/emqx_conf_cli.erl b/apps/emqx_conf/src/emqx_conf_cli.erl
index d6462a0b6..7085eb897 100644
--- a/apps/emqx_conf/src/emqx_conf_cli.erl
+++ b/apps/emqx_conf/src/emqx_conf_cli.erl
@@ -242,7 +242,7 @@ load_config(Bin, Opts) when is_binary(Bin) ->
 load_config_from_raw(RawConf0, Opts) ->
     SchemaMod = emqx_conf:schema_module(),
     RawConf1 = emqx_config:upgrade_raw_conf(SchemaMod, RawConf0),
-    case check_config(RawConf1) of
+    case check_config(RawConf1, Opts) of
         {ok, RawConf} ->
             %% It has been ensured that the connector is always the first configuration to be updated.
             %% However, when deleting the connector, we need to clean up the dependent actions/sources first;
@@ -395,24 +395,28 @@ suggest_msg(#{kind := validation_error, reason := unknown_fields}, Mode) ->
 suggest_msg(_, _) ->
     <<"">>.
 
-check_config(Conf) ->
-    case check_keys_is_not_readonly(Conf) of
-        ok ->
-            Conf1 = emqx_config:fill_defaults(Conf),
-            case check_config_schema(Conf1) of
-                ok -> {ok, Conf1};
+check_config(Conf0, Opts) ->
+    case check_keys_is_not_readonly(Conf0, Opts) of
+        {ok, Conf1} ->
+            Conf = emqx_config:fill_defaults(Conf1),
+            case check_config_schema(Conf) of
+                ok -> {ok, Conf};
                 {error, Reason} -> {error, Reason}
             end;
         Error ->
             Error
     end.
 
-check_keys_is_not_readonly(Conf) ->
+check_keys_is_not_readonly(Conf, Opts) ->
+    IgnoreReadonly = maps:get(ignore_readonly, Opts, false),
     Keys = maps:keys(Conf),
     ReadOnlyKeys = [atom_to_binary(K) || K <- ?READONLY_KEYS],
     case lists:filter(fun(K) -> lists:member(K, Keys) end, ReadOnlyKeys) of
         [] ->
-            ok;
+            {ok, Conf};
+        BadKeys when IgnoreReadonly ->
+            ?SLOG(warning, #{msg => "readonly_root_keys_ignored", keys => BadKeys}),
+            {ok, maps:without(BadKeys, Conf)};
         BadKeys ->
             BadKeysStr = lists:join(<<",">>, BadKeys),
             {error, ?UPDATE_READONLY_KEYS_PROHIBITED, BadKeysStr}
diff --git a/apps/emqx_management/src/emqx_mgmt_api_configs.erl b/apps/emqx_management/src/emqx_mgmt_api_configs.erl
index decabb64b..75341facc 100644
--- a/apps/emqx_management/src/emqx_mgmt_api_configs.erl
+++ b/apps/emqx_management/src/emqx_mgmt_api_configs.erl
@@ -147,7 +147,9 @@ schema("/configs") ->
                     hoconsc:mk(
                         hoconsc:enum([replace, merge]),
                         #{in => query, default => merge, required => false}
-                    )}
+                    )},
+                {ignore_readonly,
+                    hoconsc:mk(boolean(), #{in => query, default => false, required => false})}
             ],
             'requestBody' => #{
                 content =>
@@ -361,8 +363,13 @@ configs(get, #{query_string := QueryStr, headers := Headers}, _Req) ->
         {ok, <<"text/plain">>} -> get_configs_v2(QueryStr);
         {error, _} = Error -> {400, #{code => 'INVALID_ACCEPT', message => ?ERR_MSG(Error)}}
     end;
-configs(put, #{body := Conf, query_string := #{<<"mode">> := Mode}}, _Req) ->
-    case emqx_conf_cli:load_config(Conf, #{mode => Mode, log => none}) of
+configs(put, #{body := Conf, query_string := #{<<"mode">> := Mode} = QS}, _Req) ->
+    IngnoreReadonly = maps:get(<<"ignore_readonly">>, QS, false),
+    case
+        emqx_conf_cli:load_config(Conf, #{
+            mode => Mode, log => none, ignore_readonly => IngnoreReadonly
+        })
+    of
         ok ->
             {200};
         %% bad hocon format
diff --git a/apps/emqx_management/test/emqx_mgmt_api_configs_SUITE.erl b/apps/emqx_management/test/emqx_mgmt_api_configs_SUITE.erl
index 80b9aa9df..6d4d94013 100644
--- a/apps/emqx_management/test/emqx_mgmt_api_configs_SUITE.erl
+++ b/apps/emqx_management/test/emqx_mgmt_api_configs_SUITE.erl
@@ -331,7 +331,7 @@ t_configs_key(_Config) ->
         Log
     ),
     Log1 = emqx_utils_maps:deep_put([<<"log">>, <<"console">>, <<"level">>], Log, <<"error">>),
-    ?assertEqual(<<>>, update_configs_with_binary(iolist_to_binary(hocon_pp:do(Log1, #{})))),
+    ?assertEqual({ok, <<>>}, update_configs_with_binary(iolist_to_binary(hocon_pp:do(Log1, #{})))),
     ?assertEqual(<<"error">>, read_conf([<<"log">>, <<"console">>, <<"level">>])),
     BadLog = emqx_utils_maps:deep_put([<<"log">>, <<"console">>, <<"level">>], Log, <<"erro1r">>),
     {error, Error} = update_configs_with_binary(iolist_to_binary(hocon_pp:do(BadLog, #{}))),
@@ -358,6 +358,7 @@ t_configs_key(_Config) ->
     ReadOnlyBin = iolist_to_binary(hocon_pp:do(ReadOnlyConf, #{})),
     {error, ReadOnlyError} = update_configs_with_binary(ReadOnlyBin),
     ?assertEqual(<<"{\"errors\":\"Cannot update read-only key 'cluster'.\"}">>, ReadOnlyError),
+    ?assertMatch({ok, <<>>}, update_configs_with_binary(ReadOnlyBin, _InogreReadonly = true)),
     ok.
 
 t_get_configs_in_different_accept(_Config) ->
@@ -407,7 +408,7 @@ t_create_webhook_v1_bridges_api(Config) ->
     WebHookFile = filename:join(?config(data_dir, Config), "webhook_v1.conf"),
     ?assertMatch({ok, _}, hocon:files([WebHookFile])),
     {ok, WebHookBin} = file:read_file(WebHookFile),
-    ?assertEqual(<<>>, update_configs_with_binary(WebHookBin)),
+    ?assertEqual({ok, <<>>}, update_configs_with_binary(WebHookBin)),
     Actions =
         #{
             <<"http">> =>
@@ -557,14 +558,25 @@ get_configs_with_binary(Key, Node) ->
     end.
 
 update_configs_with_binary(Bin) ->
-    Path = emqx_mgmt_api_test_util:api_path(["configs"]),
+    update_configs_with_binary(Bin, _InogreReadonly = undefined).
+
+update_configs_with_binary(Bin, IgnoreReadonly) ->
+    Path =
+        case IgnoreReadonly of
+            undefined ->
+                emqx_mgmt_api_test_util:api_path(["configs"]);
+            Boolean ->
+                emqx_mgmt_api_test_util:api_path([
+                    "configs?ignore_readonly=" ++ atom_to_list(Boolean)
+                ])
+        end,
     Auth = emqx_mgmt_api_test_util:auth_header_(),
     Headers = [{"accept", "text/plain"}, Auth],
     case httpc:request(put, {Path, Headers, "text/plain", Bin}, [], [{body_format, binary}]) of
         {ok, {{"HTTP/1.1", Code, _}, _Headers, Body}} when
             Code >= 200 andalso Code =< 299
         ->
-            Body;
+            {ok, Body};
         {ok, {{"HTTP/1.1", 400, _}, _Headers, Body}} ->
             {error, Body};
         Error ->

From c3d27347b03947e349e54806a25d914cc91c5d53 Mon Sep 17 00:00:00 2001
From: zmstone <zmstone@gmail.com>
Date: Sun, 28 Apr 2024 14:54:01 +0200
Subject: [PATCH 5/6] docs: update changlog for pr 12940

---
 changes/ce/feat-12940.en.md | 12 ++++++++++++
 changes/ce/fix-12940.en.md  |  3 ---
 2 files changed, 12 insertions(+), 3 deletions(-)
 create mode 100644 changes/ce/feat-12940.en.md
 delete mode 100644 changes/ce/fix-12940.en.md

diff --git a/changes/ce/feat-12940.en.md b/changes/ce/feat-12940.en.md
new file mode 100644
index 000000000..24bdcc4e2
--- /dev/null
+++ b/changes/ce/feat-12940.en.md
@@ -0,0 +1,12 @@
+Add `ignore_readonly` argument to `PUT /configs` API.
+
+Prior to this change, EMQX would retrun 400 (BAD_REQUEST) if the raw config
+included readonly root keys (`cluster`, `rpc`, and `node`).
+
+After this enhancement it can be called as `PUT /configs?ignore_readonly=true`,
+EMQX will in this case ignore readonly root config keys, and apply the rest.
+
+A warning message is logged to warn that the readonly keys are dropped.
+
+Also fixed an exception when config has bad HOCON syntax (returns 500).
+Now bad syntax will cause the API to return 400 (BAD_REQUEST).
diff --git a/changes/ce/fix-12940.en.md b/changes/ce/fix-12940.en.md
deleted file mode 100644
index 8304effd5..000000000
--- a/changes/ce/fix-12940.en.md
+++ /dev/null
@@ -1,3 +0,0 @@
-Fix config update API `/configs` 500 error when config has bad HOCON syntax.
-
-Now bad syntax will cause the API to return 400 (BAD_REQUEST).

From 1db932df213c8980cf317299ae93c9aadeab7571 Mon Sep 17 00:00:00 2001
From: zmstone <zmstone@gmail.com>
Date: Mon, 29 Apr 2024 09:33:43 +0200
Subject: [PATCH 6/6] chore(mgmt): PUT /configs?ignore_readonly=true, lower log
 to info level

---
 apps/emqx_conf/src/emqx_conf_cli.erl | 2 +-
 changes/ce/feat-12940.en.md          | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/apps/emqx_conf/src/emqx_conf_cli.erl b/apps/emqx_conf/src/emqx_conf_cli.erl
index 7085eb897..f1909e59b 100644
--- a/apps/emqx_conf/src/emqx_conf_cli.erl
+++ b/apps/emqx_conf/src/emqx_conf_cli.erl
@@ -415,7 +415,7 @@ check_keys_is_not_readonly(Conf, Opts) ->
         [] ->
             {ok, Conf};
         BadKeys when IgnoreReadonly ->
-            ?SLOG(warning, #{msg => "readonly_root_keys_ignored", keys => BadKeys}),
+            ?SLOG(info, #{msg => "readonly_root_keys_ignored", keys => BadKeys}),
             {ok, maps:without(BadKeys, Conf)};
         BadKeys ->
             BadKeysStr = lists:join(<<",">>, BadKeys),
diff --git a/changes/ce/feat-12940.en.md b/changes/ce/feat-12940.en.md
index 24bdcc4e2..77e626194 100644
--- a/changes/ce/feat-12940.en.md
+++ b/changes/ce/feat-12940.en.md
@@ -6,7 +6,7 @@ included readonly root keys (`cluster`, `rpc`, and `node`).
 After this enhancement it can be called as `PUT /configs?ignore_readonly=true`,
 EMQX will in this case ignore readonly root config keys, and apply the rest.
 
-A warning message is logged to warn that the readonly keys are dropped.
+For observability purposes, an info level message is logged if any readonly keys are dropped.
 
 Also fixed an exception when config has bad HOCON syntax (returns 500).
 Now bad syntax will cause the API to return 400 (BAD_REQUEST).