From 3a2ff344330e4539badfad8d0b0a7fd354d961e7 Mon Sep 17 00:00:00 2001 From: zmstone Date: Wed, 8 May 2024 11:43:10 +0200 Subject: [PATCH 1/3] chore: add zone in listener config example --- apps/emqx_management/src/emqx_mgmt_api_listeners.erl | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/apps/emqx_management/src/emqx_mgmt_api_listeners.erl b/apps/emqx_management/src/emqx_mgmt_api_listeners.erl index 1c581514a..eebbaad08 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_listeners.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_listeners.erl @@ -810,6 +810,7 @@ listener_id_status_example() -> tcp_schema_example() -> #{ + type => tcp, acceptors => 16, access_rules => ["allow all"], bind => <<"0.0.0.0:1884">>, @@ -820,6 +821,7 @@ tcp_schema_example() -> proxy_protocol => false, proxy_protocol_timeout => <<"3s">>, running => true, + zone => default, tcp_options => #{ active_n => 100, backlog => 1024, @@ -829,8 +831,7 @@ tcp_schema_example() -> reuseaddr => true, send_timeout => <<"15s">>, send_timeout_close => true - }, - type => tcp + } }. create_listener(From, Body) -> From 9edbad545968bc3a8a222663c5525af435079d47 Mon Sep 17 00:00:00 2001 From: zmstone Date: Wed, 8 May 2024 15:45:41 +0200 Subject: [PATCH 2/3] fix(listener_api): do not allow update listener with unknown zone name --- apps/emqx/src/config/emqx_config_zones.erl | 24 +++++++++++++++++ apps/emqx/src/emqx_listeners.erl | 8 +++++- .../test/emqx_mgmt_api_listeners_SUITE.erl | 26 +++++++++++++++++-- 3 files changed, 55 insertions(+), 3 deletions(-) diff --git a/apps/emqx/src/config/emqx_config_zones.erl b/apps/emqx/src/config/emqx_config_zones.erl index f4f3c7420..9b2fb224a 100644 --- a/apps/emqx/src/config/emqx_config_zones.erl +++ b/apps/emqx/src/config/emqx_config_zones.erl @@ -20,6 +20,7 @@ %% API -export([add_handler/0, remove_handler/0, pre_config_update/3]). -export([is_olp_enabled/0]). +-export([assert_zone_exists/1]). -define(ZONES, [zones]). @@ -44,3 +45,26 @@ is_olp_enabled() -> false, 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. diff --git a/apps/emqx/src/emqx_listeners.erl b/apps/emqx/src/emqx_listeners.erl index 2b11306eb..dd9024fef 100644 --- a/apps/emqx/src/emqx_listeners.erl +++ b/apps/emqx/src/emqx_listeners.erl @@ -124,7 +124,7 @@ format_raw_listeners({Type0, Conf}) -> Bind = parse_bind(LConf0), MaxConn = maps:get(<<"max_connections">>, LConf0, default_max_conn()), 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), CurrConn = 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) -> RawConf1 = emqx_utils_maps:deep_merge(RawConf, Request), RawConf2 = ensure_override_limiter_conf(RawConf1, Request), + ok = assert_zone_exists(RawConf2), {ok, convert_certs(Type, Name, RawConf2)}; pre_config_update([?ROOT_KEY, _Type, _Name], {action, _Action, Updated}, RawConf) -> {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) -> Reason. +assert_zone_exists(#{<<"zone">> := Zone}) -> + emqx_config_zones:assert_zone_exists(Zone); +assert_zone_exists(_) -> + ok. + %% limiter config should override, not merge ensure_override_limiter_conf(Conf, #{<<"limiter">> := Limiter}) -> Conf#{<<"limiter">> => Limiter}; diff --git a/apps/emqx_management/test/emqx_mgmt_api_listeners_SUITE.erl b/apps/emqx_management/test/emqx_mgmt_api_listeners_SUITE.erl index 75c232f23..07885d93d 100644 --- a/apps/emqx_management/test/emqx_mgmt_api_listeners_SUITE.erl +++ b/apps/emqx_management/test/emqx_mgmt_api_listeners_SUITE.erl @@ -36,9 +36,12 @@ groups() -> MaxConnTests = [ t_max_connection_default ], + ZoneTests = [ + t_update_listener_zone + ], [ {with_defaults_in_file, AllTests -- MaxConnTests}, - {without_defaults_in_file, AllTests -- MaxConnTests}, + {without_defaults_in_file, AllTests -- (MaxConnTests ++ ZoneTests)}, {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, [], [])), 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) -> NonExist = emqx_mgmt_api_test_util:api_path(["listeners", "tcp:nonexistent"]), ?assertMatch( @@ -518,5 +536,9 @@ cert_file(Name) -> default_listeners_hocon_text() -> Sc = #{roots => emqx_schema:listeners()}, Listeners = hocon_tconf:make_serializable(Sc, #{}, #{}), - Config = #{<<"listeners">> => Listeners}, + Zones = #{<<"zone1">> => #{<<"mqtt">> => #{<<"max_inflight">> => 2}}}, + Config = #{ + <<"listeners">> => Listeners, + <<"zones">> => Zones + }, hocon_pp:do(Config, #{}). From c75cee3c04e460c23b4fc2863b027b2861962713 Mon Sep 17 00:00:00 2001 From: zmstone Date: Wed, 8 May 2024 17:04:53 +0200 Subject: [PATCH 3/3] docs: add changelog for PR #12993 --- changes/ce/fix-12993.en.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changes/ce/fix-12993.en.md diff --git a/changes/ce/fix-12993.en.md b/changes/ce/fix-12993.en.md new file mode 100644 index 000000000..3e565841c --- /dev/null +++ b/changes/ce/fix-12993.en.md @@ -0,0 +1,5 @@ +Fix listener config update API when handling an unknown zone. + +Prior to this fix, when a listener config is updated with an unknown zone, for example `{"zone": "unknown"}`, +the change would be accepted, causing all clients to crash when connect. +After this fix, updating listener with an unknown zone name will get a "Bad request" response.