fix(listener_api): do not allow update listener with unknown zone name

This commit is contained in:
zmstone 2024-05-08 15:45:41 +02:00
parent 3a2ff34433
commit 9edbad5459
3 changed files with 55 additions and 3 deletions

View File

@ -20,6 +20,7 @@
%% API %% API
-export([add_handler/0, remove_handler/0, pre_config_update/3]). -export([add_handler/0, remove_handler/0, pre_config_update/3]).
-export([is_olp_enabled/0]). -export([is_olp_enabled/0]).
-export([assert_zone_exists/1]).
-define(ZONES, [zones]). -define(ZONES, [zones]).
@ -44,3 +45,26 @@ is_olp_enabled() ->
false, false,
emqx_config:get([zones], #{}) emqx_config:get([zones], #{})
). ).
-spec assert_zone_exists(binary() | atom()) -> ok.
assert_zone_exists(Name0) when is_binary(Name0) ->
%% an existing zone must have already an atom-name
Name =
try
binary_to_existing_atom(Name0)
catch
_:_ ->
throw({unknown_zone, Name0})
end,
assert_zone_exists(Name);
assert_zone_exists(default) ->
%% there is always a 'default' zone
ok;
assert_zone_exists(Name) when is_atom(Name) ->
try
_ = emqx_config:get([zones, Name]),
ok
catch
error:{config_not_found, _} ->
throw({unknown_zone, Name})
end.

View File

@ -124,7 +124,7 @@ format_raw_listeners({Type0, Conf}) ->
Bind = parse_bind(LConf0), Bind = parse_bind(LConf0),
MaxConn = maps:get(<<"max_connections">>, LConf0, default_max_conn()), MaxConn = maps:get(<<"max_connections">>, LConf0, default_max_conn()),
Running = is_running(Type, listener_id(Type, LName), LConf0#{bind => Bind}), Running = is_running(Type, listener_id(Type, LName), LConf0#{bind => Bind}),
LConf1 = maps:without([<<"authentication">>, <<"zone">>], LConf0), LConf1 = maps:without([<<"authentication">>], LConf0),
LConf2 = maps:put(<<"running">>, Running, LConf1), LConf2 = maps:put(<<"running">>, Running, LConf1),
CurrConn = CurrConn =
case Running of case Running of
@ -526,6 +526,7 @@ pre_config_update([?ROOT_KEY, _Type, _Name], {update, _Request}, undefined) ->
pre_config_update([?ROOT_KEY, Type, Name], {update, Request}, RawConf) -> pre_config_update([?ROOT_KEY, Type, Name], {update, Request}, RawConf) ->
RawConf1 = emqx_utils_maps:deep_merge(RawConf, Request), RawConf1 = emqx_utils_maps:deep_merge(RawConf, Request),
RawConf2 = ensure_override_limiter_conf(RawConf1, Request), RawConf2 = ensure_override_limiter_conf(RawConf1, Request),
ok = assert_zone_exists(RawConf2),
{ok, convert_certs(Type, Name, RawConf2)}; {ok, convert_certs(Type, Name, RawConf2)};
pre_config_update([?ROOT_KEY, _Type, _Name], {action, _Action, Updated}, RawConf) -> pre_config_update([?ROOT_KEY, _Type, _Name], {action, _Action, Updated}, RawConf) ->
{ok, emqx_utils_maps:deep_merge(RawConf, Updated)}; {ok, emqx_utils_maps:deep_merge(RawConf, Updated)};
@ -884,6 +885,11 @@ convert_certs(Type, Name, Conf) ->
filter_stacktrace({Reason, _Stacktrace}) -> Reason; filter_stacktrace({Reason, _Stacktrace}) -> Reason;
filter_stacktrace(Reason) -> Reason. filter_stacktrace(Reason) -> Reason.
assert_zone_exists(#{<<"zone">> := Zone}) ->
emqx_config_zones:assert_zone_exists(Zone);
assert_zone_exists(_) ->
ok.
%% limiter config should override, not merge %% limiter config should override, not merge
ensure_override_limiter_conf(Conf, #{<<"limiter">> := Limiter}) -> ensure_override_limiter_conf(Conf, #{<<"limiter">> := Limiter}) ->
Conf#{<<"limiter">> => Limiter}; Conf#{<<"limiter">> => Limiter};

View File

@ -36,9 +36,12 @@ groups() ->
MaxConnTests = [ MaxConnTests = [
t_max_connection_default t_max_connection_default
], ],
ZoneTests = [
t_update_listener_zone
],
[ [
{with_defaults_in_file, AllTests -- MaxConnTests}, {with_defaults_in_file, AllTests -- MaxConnTests},
{without_defaults_in_file, AllTests -- MaxConnTests}, {without_defaults_in_file, AllTests -- (MaxConnTests ++ ZoneTests)},
{max_connections, MaxConnTests} {max_connections, MaxConnTests}
]. ].
@ -403,6 +406,21 @@ crud_listeners_by_id(ListenerId, NewListenerId, MinListenerId, BadId, Type, Port
?assertMatch({error, {"HTTP/1.1", 404, _}}, request(delete, NewPath, [], [])), ?assertMatch({error, {"HTTP/1.1", 404, _}}, request(delete, NewPath, [], [])),
ok. ok.
t_update_listener_zone({init, Config}) ->
%% fake a zone
Config;
t_update_listener_zone({'end', _Config}) ->
ok;
t_update_listener_zone(_Config) ->
ListenerId = <<"tcp:default">>,
Path = emqx_mgmt_api_test_util:api_path(["listeners", ListenerId]),
Conf = request(get, Path, [], []),
%% update
AddConf1 = Conf#{<<"zone">> => <<"unknownzone">>},
AddConf2 = Conf#{<<"zone">> => <<"zone1">>},
?assertMatch({error, {_, 400, _}}, request(put, Path, [], AddConf1)),
?assertMatch(#{<<"zone">> := <<"zone1">>}, request(put, Path, [], AddConf2)).
t_delete_nonexistent_listener(Config) when is_list(Config) -> t_delete_nonexistent_listener(Config) when is_list(Config) ->
NonExist = emqx_mgmt_api_test_util:api_path(["listeners", "tcp:nonexistent"]), NonExist = emqx_mgmt_api_test_util:api_path(["listeners", "tcp:nonexistent"]),
?assertMatch( ?assertMatch(
@ -518,5 +536,9 @@ cert_file(Name) ->
default_listeners_hocon_text() -> default_listeners_hocon_text() ->
Sc = #{roots => emqx_schema:listeners()}, Sc = #{roots => emqx_schema:listeners()},
Listeners = hocon_tconf:make_serializable(Sc, #{}, #{}), Listeners = hocon_tconf:make_serializable(Sc, #{}, #{}),
Config = #{<<"listeners">> => Listeners}, Zones = #{<<"zone1">> => #{<<"mqtt">> => #{<<"max_inflight">> => 2}}},
Config = #{
<<"listeners">> => Listeners,
<<"zones">> => Zones
},
hocon_pp:do(Config, #{}). hocon_pp:do(Config, #{}).