From 5718b28eeb0d6f610cc24113f82837e9e7f114d7 Mon Sep 17 00:00:00 2001 From: Zhongwen Deng Date: Wed, 31 Aug 2022 14:30:03 +0800 Subject: [PATCH 01/34] feat: Deny hot updates to the configuration in local-override.conf --- CHANGES-5.0.md | 3 +- apps/emqx/src/emqx_config_handler.erl | 130 +++++++++++++++--- apps/emqx/test/emqx_config_handler_SUITE.erl | 66 ++++++++- .../src/emqx_mgmt_api_configs.erl | 13 +- 4 files changed, 190 insertions(+), 22 deletions(-) diff --git a/CHANGES-5.0.md b/CHANGES-5.0.md index 742883e27..e6ff9e36f 100644 --- a/CHANGES-5.0.md +++ b/CHANGES-5.0.md @@ -2,7 +2,8 @@ ## Enhancements -* change the `/gateway` API path to plural form. [#8823](https://github.com/emqx/emqx/pull/8823) +* Change the `/gateway` API path to plural form. [#8823](https://github.com/emqx/emqx/pull/8823) +* Don't allow updating config items when they already exist in `local-override.conf`. [#8851](https://github.com/emqx/emqx/pull/8851) # 5.0.7 diff --git a/apps/emqx/src/emqx_config_handler.erl b/apps/emqx/src/emqx_config_handler.erl index ad46c1ee3..0311418a9 100644 --- a/apps/emqx/src/emqx_config_handler.erl +++ b/apps/emqx/src/emqx_config_handler.erl @@ -43,6 +43,7 @@ terminate/2, code_change/3 ]). +-export([is_mutable/3]). -define(MOD, {mod}). -define(WKEY, '?'). @@ -229,15 +230,26 @@ process_update_request([_], _Handlers, {remove, _Opts}) -> process_update_request(ConfKeyPath, _Handlers, {remove, Opts}) -> OldRawConf = emqx_config:get_root_raw(ConfKeyPath), BinKeyPath = bin_path(ConfKeyPath), - NewRawConf = emqx_map_lib:deep_remove(BinKeyPath, OldRawConf), - OverrideConf = remove_from_override_config(BinKeyPath, Opts), - {ok, NewRawConf, OverrideConf, Opts}; + case check_permissions(remove, BinKeyPath, OldRawConf, Opts) of + allow -> + NewRawConf = emqx_map_lib:deep_remove(BinKeyPath, OldRawConf), + OverrideConf = remove_from_override_config(BinKeyPath, Opts), + {ok, NewRawConf, OverrideConf, Opts}; + {deny, Reason} -> + {error, {permission_denied, Reason}} + end; process_update_request(ConfKeyPath, Handlers, {{update, UpdateReq}, Opts}) -> OldRawConf = emqx_config:get_root_raw(ConfKeyPath), case do_update_config(ConfKeyPath, Handlers, OldRawConf, UpdateReq) of {ok, NewRawConf} -> - OverrideConf = update_override_config(NewRawConf, Opts), - {ok, NewRawConf, OverrideConf, Opts}; + BinKeyPath = bin_path(ConfKeyPath), + case check_permissions(update, BinKeyPath, NewRawConf, Opts) of + allow -> + OverrideConf = update_override_config(NewRawConf, Opts), + {ok, NewRawConf, OverrideConf, Opts}; + {deny, Reason} -> + {error, {permission_denied, Reason}} + end; Error -> Error end. @@ -272,12 +284,11 @@ check_and_save_configs( UpdateArgs, Opts ) -> - OldConf = emqx_config:get_root(ConfKeyPath), Schema = schema(SchemaModule, ConfKeyPath), {AppEnvs, NewConf} = emqx_config:check_config(Schema, NewRawConf), + OldConf = emqx_config:get_root(ConfKeyPath), case do_post_config_update(ConfKeyPath, Handlers, OldConf, NewConf, AppEnvs, UpdateArgs, #{}) of {ok, Result0} -> - remove_from_local_if_cluster_change(ConfKeyPath, Opts), ok = emqx_config:save_configs(AppEnvs, NewConf, NewRawConf, OverrideConf, Opts), Result1 = return_change_result(ConfKeyPath, UpdateArgs), {ok, Result1#{post_config_update => Result0}}; @@ -430,16 +441,6 @@ merge_to_old_config(UpdateReq, RawConf) when is_map(UpdateReq), is_map(RawConf) merge_to_old_config(UpdateReq, _RawConf) -> {ok, UpdateReq}. -%% local-override.conf priority is higher than cluster-override.conf -%% If we want cluster to take effect, we must remove the local. -remove_from_local_if_cluster_change(BinKeyPath, #{override_to := cluster} = Opts) -> - Opts1 = Opts#{override_to => local}, - Local = remove_from_override_config(BinKeyPath, Opts1), - _ = emqx_config:save_to_override_conf(Local, Opts1), - ok; -remove_from_local_if_cluster_change(_BinKeyPath, _Opts) -> - ok. - remove_from_override_config(_BinKeyPath, #{persistent := false}) -> undefined; remove_from_override_config(BinKeyPath, Opts) -> @@ -544,3 +545,98 @@ load_prev_handlers() -> save_handlers(Handlers) -> application:set_env(emqx, ?MODULE, Handlers). + +check_permissions(_Action, _ConfKeyPath, _NewRawConf, #{override_to := local}) -> + allow; +check_permissions(Action, ConfKeyPath, NewRawConf, _Opts) -> + case emqx_map_lib:deep_find(ConfKeyPath, NewRawConf) of + {ok, NewRaw} -> + LocalOverride = emqx_config:read_override_conf(#{override_to => local}), + case emqx_map_lib:deep_find(ConfKeyPath, LocalOverride) of + {ok, LocalRaw} -> + case is_mutable(Action, NewRaw, LocalRaw) of + ok -> + allow; + {error, Error} -> + ?SLOG(error, #{ + msg => "prevent_remove_local_override_conf", + config_key_path => ConfKeyPath, + error => Error + }), + {deny, "Disable changed from local-override.conf"} + end; + {not_found, _, _} -> + allow + end; + {not_found, _, _} -> + allow + end. + +is_mutable(Action, NewRaw, LocalRaw) -> + try + KeyPath = [], + is_mutable(KeyPath, Action, NewRaw, LocalRaw) + catch + throw:Error -> Error + end. + +-define(REMOVE_FAILED, "remove_failed"). +-define(UPDATE_FAILED, "update_failed"). + +is_mutable(KeyPath, Action, New = #{}, Local = #{}) -> + maps:foreach( + fun(Key, SubLocal) -> + case maps:find(Key, New) of + error -> ok; + {ok, SubNew} -> is_mutable(KeyPath ++ [Key], Action, SubNew, SubLocal) + end + end, + Local + ); +is_mutable(KeyPath, remove, Update, Origin) -> + throw({error, {?REMOVE_FAILED, KeyPath, Update, Origin}}); +is_mutable(_KeyPath, update, Val, Val) -> + ok; +is_mutable(KeyPath, update, Update, Origin) -> + throw({error, {?UPDATE_FAILED, KeyPath, Update, Origin}}). + +-ifdef(TEST). +-include_lib("eunit/include/eunit.hrl"). + +is_mutable_update_test() -> + Action = update, + ?assertEqual(ok, is_mutable(Action, #{}, #{})), + ?assertEqual(ok, is_mutable(Action, #{a => #{b => #{c => #{}}}}, #{a => #{b => #{c => #{}}}})), + ?assertEqual(ok, is_mutable(Action, #{a => #{b => #{c => 1}}}, #{a => #{b => #{c => 1}}})), + ?assertEqual( + {error, {?UPDATE_FAILED, [a, b, c], 1, 2}}, + is_mutable(Action, #{a => #{b => #{c => 1}}}, #{a => #{b => #{c => 2}}}) + ), + ?assertEqual( + {error, {?UPDATE_FAILED, [a, b, d], 2, 3}}, + is_mutable(Action, #{a => #{b => #{c => 1, d => 2}}}, #{a => #{b => #{c => 1, d => 3}}}) + ), + ok. + +is_mutable_remove_test() -> + Action = remove, + ?assertEqual(ok, is_mutable(Action, #{}, #{})), + ?assertEqual(ok, is_mutable(Action, #{a => #{b => #{c => #{}}}}, #{a1 => #{b => #{c => #{}}}})), + ?assertEqual(ok, is_mutable(Action, #{a => #{b => #{c => 1}}}, #{a => #{b1 => #{c => 1}}})), + ?assertEqual(ok, is_mutable(Action, #{a => #{b => #{c => 1}}}, #{a => #{b => #{c1 => 1}}})), + + ?assertEqual( + {error, {?REMOVE_FAILED, [a, b, c], 1, 1}}, + is_mutable(Action, #{a => #{b => #{c => 1}}}, #{a => #{b => #{c => 1}}}) + ), + ?assertEqual( + {error, {?REMOVE_FAILED, [a, b, c], 1, 2}}, + is_mutable(Action, #{a => #{b => #{c => 1}}}, #{a => #{b => #{c => 2}}}) + ), + ?assertEqual( + {error, {?REMOVE_FAILED, [a, b, c], 1, 1}}, + is_mutable(Action, #{a => #{b => #{c => 1, d => 2}}}, #{a => #{b => #{c => 1, d => 3}}}) + ), + ok. + +-endif. diff --git a/apps/emqx/test/emqx_config_handler_SUITE.erl b/apps/emqx/test/emqx_config_handler_SUITE.erl index ae34bee7a..1fe79f74c 100644 --- a/apps/emqx/test/emqx_config_handler_SUITE.erl +++ b/apps/emqx/test/emqx_config_handler_SUITE.erl @@ -21,6 +21,8 @@ -define(MOD, {mod}). -define(WKEY, '?'). +-define(LOCAL_CONF, "/tmp/local-override.conf"). +-define(CLUSTER_CONF, "/tmp/cluster-override.conf"). -include_lib("eunit/include/eunit.hrl"). -include_lib("common_test/include/ct.hrl"). @@ -36,6 +38,8 @@ end_per_suite(_Config) -> emqx_common_test_helpers:stop_apps([]). init_per_testcase(_Case, Config) -> + _ = file:delete(?LOCAL_CONF), + _ = file:delete(?CLUSTER_CONF), Config. end_per_testcase(_Case, _Config) -> @@ -196,6 +200,62 @@ t_sub_key_update_remove(_Config) -> ok = emqx_config_handler:remove_handler(KeyPath2), ok. +t_local_override_update_remove(_Config) -> + application:set_env(emqx, local_override_conf_file, ?LOCAL_CONF), + application:set_env(emqx, cluster_override_conf_file, ?CLUSTER_CONF), + KeyPath = [sysmon, os, cpu_high_watermark], + ok = emqx_config_handler:add_handler(KeyPath, ?MODULE), + LocalOpts = #{override_to => local}, + {ok, Res} = emqx:update_config(KeyPath, <<"70%">>, LocalOpts), + ?assertMatch( + #{ + config := 0.7, + post_config_update := #{}, + raw_config := <<"70%">> + }, + Res + ), + ClusterOpts = #{override_to => cluster}, + ?assertMatch( + {error, {permission_denied, _}}, emqx:update_config(KeyPath, <<"71%">>, ClusterOpts) + ), + ?assertMatch(0.7, emqx:get_config(KeyPath)), + + KeyPath2 = [sysmon, os, cpu_low_watermark], + ok = emqx_config_handler:add_handler(KeyPath2, ?MODULE), + ?assertMatch( + {error, {permission_denied, _}}, emqx:update_config(KeyPath2, <<"40%">>, ClusterOpts) + ), + + %% remove + ?assertMatch({error, {permission_denied, _}}, emqx:remove_config(KeyPath)), + ?assertEqual( + {ok, #{post_config_update => #{}}}, + emqx:remove_config(KeyPath, #{override_to => local}) + ), + ?assertEqual( + {ok, #{post_config_update => #{}}}, + emqx:remove_config(KeyPath) + ), + ?assertError({config_not_found, KeyPath}, emqx:get_raw_config(KeyPath)), + OSKey = maps:keys(emqx:get_raw_config([sysmon, os])), + ?assertEqual(false, lists:member(<<"cpu_high_watermark">>, OSKey)), + ?assert(length(OSKey) > 0), + + ?assertEqual( + {ok, #{config => 0.8, post_config_update => #{}, raw_config => <<"80%">>}}, + emqx:reset_config(KeyPath, ClusterOpts) + ), + OSKey1 = maps:keys(emqx:get_raw_config([sysmon, os])), + ?assertEqual(true, lists:member(<<"cpu_high_watermark">>, OSKey1)), + ?assert(length(OSKey1) > 1), + + ok = emqx_config_handler:remove_handler(KeyPath), + ok = emqx_config_handler:remove_handler(KeyPath2), + application:unset_env(emqx, local_override_conf_file), + application:unset_env(emqx, cluster_override_conf_file), + ok. + t_check_failed(_Config) -> KeyPath = [sysmon, os, cpu_check_interval], Opts = #{rawconf_with_defaults => true}, @@ -219,7 +279,7 @@ t_stop(_Config) -> ok. t_callback_crash(_Config) -> - CrashPath = [sysmon, os, cpu_high_watermark], + CrashPath = [sysmon, os, procmem_high_watermark], Opts = #{rawconf_with_defaults => true}, ok = emqx_config_handler:add_handler(CrashPath, ?MODULE), Old = emqx:get_raw_config(CrashPath), @@ -334,6 +394,8 @@ pre_config_update([sysmon, os, cpu_check_interval], UpdateReq, _RawConf) -> {ok, UpdateReq}; pre_config_update([sysmon, os, cpu_low_watermark], UpdateReq, _RawConf) -> {ok, UpdateReq}; +pre_config_update([sysmon, os, cpu_high_watermark], UpdateReq, _RawConf) -> + {ok, UpdateReq}; pre_config_update([sysmon, os, sysmem_high_watermark], UpdateReq, _RawConf) -> {ok, UpdateReq}; pre_config_update([sysmon, os, mem_check_interval], _UpdateReq, _RawConf) -> @@ -347,6 +409,8 @@ post_config_update([sysmon, os, cpu_check_interval], _UpdateReq, _NewConf, _OldC {ok, ok}; post_config_update([sysmon, os, cpu_low_watermark], _UpdateReq, _NewConf, _OldConf, _AppEnvs) -> ok; +post_config_update([sysmon, os, cpu_high_watermark], _UpdateReq, _NewConf, _OldConf, _AppEnvs) -> + ok; post_config_update([sysmon, os, sysmem_high_watermark], _UpdateReq, _NewConf, _OldConf, _AppEnvs) -> {error, post_config_update_error}. diff --git a/apps/emqx_management/src/emqx_mgmt_api_configs.erl b/apps/emqx_management/src/emqx_mgmt_api_configs.erl index 7435e5e0d..8eb801952 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_configs.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_configs.erl @@ -141,7 +141,8 @@ schema("/configs_reset/:rootname") -> ], responses => #{ 200 => <<"Rest config successfully">>, - 400 => emqx_dashboard_swagger:error_codes(['NO_DEFAULT_VALUE', 'REST_FAILED']) + 400 => emqx_dashboard_swagger:error_codes(['NO_DEFAULT_VALUE', 'REST_FAILED']), + 403 => emqx_dashboard_swagger:error_codes(['REST_FAILED']) } } }; @@ -160,7 +161,8 @@ schema("/configs/global_zone") -> 'requestBody' => Schema, responses => #{ 200 => Schema, - 400 => emqx_dashboard_swagger:error_codes(['UPDATE_FAILED']) + 400 => emqx_dashboard_swagger:error_codes(['UPDATE_FAILED']), + 403 => emqx_dashboard_swagger:error_codes(['UPDATE_FAILED']) } } }; @@ -226,7 +228,8 @@ schema(Path) -> 'requestBody' => Schema, responses => #{ 200 => Schema, - 400 => emqx_dashboard_swagger:error_codes(['UPDATE_FAILED']) + 400 => emqx_dashboard_swagger:error_codes(['UPDATE_FAILED']), + 403 => emqx_dashboard_swagger:error_codes(['UPDATE_FAILED']) } } }. @@ -254,6 +257,8 @@ config(put, #{body := Body}, Req) -> case emqx_conf:update(Path, Body, ?OPTS) of {ok, #{raw_config := RawConf}} -> {200, RawConf}; + {error, {permission_denied, Reason}} -> + {403, #{code => 'UPDATE_FAILED', message => Reason}}; {error, Reason} -> {400, #{code => 'UPDATE_FAILED', message => ?ERR_MSG(Reason)}} end. @@ -297,6 +302,8 @@ config_reset(post, _Params, Req) -> case emqx_conf:reset(Path, ?OPTS) of {ok, _} -> {200}; + {error, {permission_denied, Reason}} -> + {403, #{code => 'REST_FAILED', message => Reason}}; {error, no_default_value} -> {400, #{code => 'NO_DEFAULT_VALUE', message => <<"No Default Value.">>}}; {error, Reason} -> From 7bb592a2e1806aa08ee7963b3016d4a9b02b14b9 Mon Sep 17 00:00:00 2001 From: Zhongwen Deng Date: Thu, 1 Sep 2022 09:25:23 +0800 Subject: [PATCH 02/34] fix: default override_to is cluster --- apps/emqx/src/emqx_config.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/emqx/src/emqx_config.erl b/apps/emqx/src/emqx_config.erl index 3d602349d..4ab7caf77 100644 --- a/apps/emqx/src/emqx_config.erl +++ b/apps/emqx/src/emqx_config.erl @@ -476,7 +476,7 @@ read_override_conf(#{} = Opts) -> override_conf_file(Opts) when is_map(Opts) -> Key = - case maps:get(override_to, Opts, local) of + case maps:get(override_to, Opts, cluster) of local -> local_override_conf_file; cluster -> cluster_override_conf_file end, From 42e2d00cfa62b161b57f9048c9167c59496d23ed Mon Sep 17 00:00:00 2001 From: firest Date: Wed, 7 Sep 2022 12:11:57 +0800 Subject: [PATCH 03/34] fix(retainer): fix that EMQX can't start when the retainer is disabled --- apps/emqx_retainer/src/emqx_retainer.erl | 7 +------ apps/emqx_retainer/src/emqx_retainer_app.erl | 13 +++++++++++++ 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/apps/emqx_retainer/src/emqx_retainer.erl b/apps/emqx_retainer/src/emqx_retainer.erl index f5a3ad403..5d911b5f4 100644 --- a/apps/emqx_retainer/src/emqx_retainer.erl +++ b/apps/emqx_retainer/src/emqx_retainer.erl @@ -348,16 +348,12 @@ enable_retainer( #{context_id := ContextId} = State, #{ msg_clear_interval := ClearInterval, - backend := BackendCfg, - flow_control := FlowControl + backend := BackendCfg } ) -> NewContextId = ContextId + 1, Context = create_resource(new_context(NewContextId), BackendCfg), load(Context), - emqx_limiter_server:add_bucket( - ?APP, internal, maps:get(batch_deliver_limiter, FlowControl, undefined) - ), State#{ enable := true, context_id := NewContextId, @@ -373,7 +369,6 @@ disable_retainer( } = State ) -> unload(), - emqx_limiter_server:del_bucket(?APP, internal), ok = close_resource(Context), State#{ enable := false, diff --git a/apps/emqx_retainer/src/emqx_retainer_app.erl b/apps/emqx_retainer/src/emqx_retainer_app.erl index 2285d4551..061679cf7 100644 --- a/apps/emqx_retainer/src/emqx_retainer_app.erl +++ b/apps/emqx_retainer/src/emqx_retainer_app.erl @@ -18,6 +18,8 @@ -behaviour(application). +-include("emqx_retainer.hrl"). + -export([ start/2, stop/1 @@ -25,8 +27,19 @@ start(_Type, _Args) -> ok = emqx_retainer_mnesia_cli:load(), + init_bucket(), emqx_retainer_sup:start_link(). stop(_State) -> ok = emqx_retainer_mnesia_cli:unload(), + delete_bucket(), ok. + +init_bucket() -> + #{flow_control := FlowControl} = emqx:get_config([retainer]), + emqx_limiter_server:add_bucket( + ?APP, internal, maps:get(batch_deliver_limiter, FlowControl, undefined) + ). + +delete_bucket() -> + emqx_limiter_server:del_bucket(?APP, internal). From 8768884e2b2b4248bc0f85df3a92e855d2948aff Mon Sep 17 00:00:00 2001 From: firest Date: Wed, 7 Sep 2022 14:49:33 +0800 Subject: [PATCH 04/34] chore: bump emqx_retainer version && update CHANGES-5.0.md --- CHANGES-5.0.md | 1 + apps/emqx_retainer/src/emqx_retainer.app.src | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGES-5.0.md b/CHANGES-5.0.md index c5d47e3df..8bddd1e75 100644 --- a/CHANGES-5.0.md +++ b/CHANGES-5.0.md @@ -8,6 +8,7 @@ * Speed up dispatching of shared subscription messages in a cluster [#8893](https://github.com/emqx/emqx/pull/8893) * Fix the extra / prefix when CoAP gateway parsing client topics. [#8658](https://github.com/emqx/emqx/pull/8658) * Speed up updating the configuration, When some nodes in the cluster are down. [#8857](https://github.com/emqx/emqx/pull/8857) +* Fix that EMQX can't start when the retainer is disabled [#8911](https://github.com/emqx/emqx/pull/8911) ## Enhancements diff --git a/apps/emqx_retainer/src/emqx_retainer.app.src b/apps/emqx_retainer/src/emqx_retainer.app.src index c91ba0eec..888335ab4 100644 --- a/apps/emqx_retainer/src/emqx_retainer.app.src +++ b/apps/emqx_retainer/src/emqx_retainer.app.src @@ -2,7 +2,7 @@ {application, emqx_retainer, [ {description, "EMQX Retainer"}, % strict semver, bump manually! - {vsn, "5.0.4"}, + {vsn, "5.0.5"}, {modules, []}, {registered, [emqx_retainer_sup]}, {applications, [kernel, stdlib, emqx]}, From 9368ae9fa82e8df9bc3274076b8d4e65f754b8c1 Mon Sep 17 00:00:00 2001 From: firest Date: Thu, 8 Sep 2022 19:13:02 +0800 Subject: [PATCH 05/34] test(retainer): add test case for start after retianer is disabled --- .../test/emqx_retainer_SUITE.erl | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/apps/emqx_retainer/test/emqx_retainer_SUITE.erl b/apps/emqx_retainer/test/emqx_retainer_SUITE.erl index d7ddc2424..09e6c4bb4 100644 --- a/apps/emqx_retainer/test/emqx_retainer_SUITE.erl +++ b/apps/emqx_retainer/test/emqx_retainer_SUITE.erl @@ -31,14 +31,16 @@ all() -> [ {group, mnesia_without_indices}, {group, mnesia_with_indices}, - {group, mnesia_reindex} + {group, mnesia_reindex}, + {group, test_disable_then_start} ]. groups() -> [ {mnesia_without_indices, [sequence], common_tests()}, {mnesia_with_indices, [sequence], common_tests()}, - {mnesia_reindex, [sequence], [t_reindex]} + {mnesia_reindex, [sequence], [t_reindex]}, + {test_disable_then_start, [sequence], [test_disable_then_start]} ]. common_tests() -> @@ -624,6 +626,19 @@ t_get_basic_usage_info(_Config) -> ?assertEqual(#{retained_messages => 5}, emqx_retainer:get_basic_usage_info()), ok. +%% test whether the app can start normally after disabling emqx_retainer +%% fix: https://github.com/emqx/emqx/pull/8911 +test_disable_then_start(_Config) -> + emqx_retainer:update_config(#{<<"enable">> => false}), + ?assertNotEqual([], gproc_pool:active_workers(emqx_retainer_dispatcher)), + ok = application:stop(emqx_retainer), + timer:sleep(100), + ?assertEqual([], gproc_pool:active_workers(emqx_retainer_dispatcher)), + ok = application:ensure_started(emqx_retainer), + timer:sleep(100), + ?assertNotEqual([], gproc_pool:active_workers(emqx_retainer_dispatcher)), + ok. + %%-------------------------------------------------------------------- %% Helper functions %%-------------------------------------------------------------------- From 5ba31cb192bc4feb03fde003a51733ae099eaec8 Mon Sep 17 00:00:00 2001 From: firest Date: Fri, 2 Sep 2022 15:15:14 +0800 Subject: [PATCH 06/34] fix(api): add listener create API from `POST /listeners/{type:name}` to `POST /listeners` Old: API: POST /listeners/{type:demo} Body: {"type" : "tcp", "id" : "tcp:demo"} New: API: POST /listeners Body: {"type" : "tcp", "name" : "demo"} --- .../src/emqx_dashboard_swagger.erl | 2 +- .../src/emqx_mgmt_api_listeners.erl | 110 +++++++++++++----- .../test/emqx_mgmt_api_listeners_SUITE.erl | 29 +++++ 3 files changed, 112 insertions(+), 29 deletions(-) diff --git a/apps/emqx_dashboard/src/emqx_dashboard_swagger.erl b/apps/emqx_dashboard/src/emqx_dashboard_swagger.erl index 34f32d8be..52cbc4775 100644 --- a/apps/emqx_dashboard/src/emqx_dashboard_swagger.erl +++ b/apps/emqx_dashboard/src/emqx_dashboard_swagger.erl @@ -778,7 +778,7 @@ to_bin(List) when is_list(List) -> to_bin(Boolean) when is_boolean(Boolean) -> Boolean; to_bin(Atom) when is_atom(Atom) -> atom_to_binary(Atom, utf8); to_bin({Type, Args}) -> - unicode:characters_to_binary(io_lib:format("~p(~p)", [Type, Args])); + unicode:characters_to_binary(io_lib:format("~ts(~p)", [Type, Args])); to_bin(X) -> X. diff --git a/apps/emqx_management/src/emqx_mgmt_api_listeners.erl b/apps/emqx_management/src/emqx_mgmt_api_listeners.erl index 31678e0f6..925c20ff1 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_listeners.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_listeners.erl @@ -96,6 +96,16 @@ schema("/listeners") -> listener_id_status_example() ) } + }, + post => #{ + tags => [<<"listeners">>], + desc => <<"Create the specified listener on all nodes.">>, + parameters => [], + 'requestBody' => create_listener_schema(#{bind => true}), + responses => #{ + 200 => listener_schema(#{bind => true}), + 400 => error_codes(['BAD_LISTENER_ID', 'BAD_REQUEST']) + } } }; schema("/listeners/:id") -> @@ -129,7 +139,8 @@ schema("/listeners/:id") -> responses => #{ 200 => listener_schema(#{bind => true}), 400 => error_codes(['BAD_LISTENER_ID', 'BAD_REQUEST']) - } + }, + deprecated => true }, delete => #{ tags => [<<"listeners">>], @@ -251,10 +262,10 @@ fields(node_status) -> })}, {status, ?HOCON(?R_REF(status))} ]; +fields({Type, with_name}) -> + listener_struct_with_name(Type); fields(Type) -> - Listeners = listeners_info(#{bind => true}) ++ listeners_info(#{bind => false}), - [Schema] = [S || #{ref := ?R_REF(_, T), schema := S} <- Listeners, T =:= Type], - Schema. + listener_struct(Type). listener_schema(Opts) -> emqx_dashboard_swagger:schema_with_example( @@ -262,6 +273,17 @@ listener_schema(Opts) -> tcp_schema_example() ). +create_listener_schema(Opts) -> + Schemas = [ + ?R_REF(Mod, {Type, with_name}) + || #{ref := ?R_REF(Mod, Type)} <- listeners_info(Opts) + ], + Example = maps:remove(id, tcp_schema_example()), + emqx_dashboard_swagger:schema_with_example( + ?UNION(Schemas), + Example#{name => <<"demo">>} + ). + listeners_type() -> lists:map( fun({Type, _}) -> list_to_existing_atom(Type) end, @@ -339,7 +361,9 @@ list_listeners(get, #{query_string := Query}) -> {ok, Type} -> listener_type_filter(atom_to_binary(Type), Listeners); error -> Listeners end, - {200, listener_status_by_id(NodeL)}. + {200, listener_status_by_id(NodeL)}; +list_listeners(post, #{body := Body}) -> + create_listener(Body). crud_listeners_by_id(get, #{bindings := #{id := Id0}}) -> Listeners = @@ -382,23 +406,8 @@ crud_listeners_by_id(put, #{bindings := #{id := Id}, body := Body0}) -> _ -> {400, #{code => 'BAD_LISTENER_ID', message => ?LISTENER_ID_INCONSISTENT}} end; -crud_listeners_by_id(post, #{bindings := #{id := Id}, body := Body0}) -> - case parse_listener_conf(Body0) of - {Id, Type, Name, Conf} -> - Path = [listeners, Type, Name], - case create(Path, Conf) of - {ok, #{raw_config := _RawConf}} -> - crud_listeners_by_id(get, #{bindings => #{id => Id}}); - {error, already_exist} -> - {400, #{code => 'BAD_LISTENER_ID', message => <<"Already Exist">>}}; - {error, Reason} -> - {400, #{code => 'BAD_REQUEST', message => err_msg(Reason)}} - end; - {error, Reason} -> - {400, #{code => 'BAD_REQUEST', message => err_msg(Reason)}}; - _ -> - {400, #{code => 'BAD_LISTENER_ID', message => ?LISTENER_ID_INCONSISTENT}} - end; +crud_listeners_by_id(post, #{body := Body}) -> + create_listener(Body); crud_listeners_by_id(delete, #{bindings := #{id := Id}}) -> {ok, #{type := Type, name := Name}} = emqx_listeners:parse_listener_id(Id), case ensure_remove([listeners, Type, Name]) of @@ -408,13 +417,24 @@ crud_listeners_by_id(delete, #{bindings := #{id := Id}}) -> parse_listener_conf(Conf0) -> Conf1 = maps:without([<<"running">>, <<"current_connections">>], Conf0), - {IdBin, Conf2} = maps:take(<<"id">>, Conf1), - {TypeBin, Conf3} = maps:take(<<"type">>, Conf2), - {ok, #{type := Type, name := Name}} = emqx_listeners:parse_listener_id(IdBin), + {TypeBin, Conf2} = maps:take(<<"type">>, Conf1), TypeAtom = binary_to_existing_atom(TypeBin), - case Type =:= TypeAtom of - true -> {binary_to_existing_atom(IdBin), TypeAtom, Name, Conf3}; - false -> {error, listener_type_inconsistent} + + case maps:take(<<"id">>, Conf2) of + {IdBin, Conf3} -> + {ok, #{type := Type, name := Name}} = emqx_listeners:parse_listener_id(IdBin), + case Type =:= TypeAtom of + true -> {binary_to_existing_atom(IdBin), TypeAtom, Name, Conf3}; + false -> {error, listener_type_inconsistent} + end; + _ -> + case maps:take(<<"name">>, Conf2) of + {Name, Conf3} -> + IdBin = <>, + {binary_to_atom(IdBin), TypeAtom, Name, Conf3}; + _ -> + {error, listener_config_invalid} + end end. stop_listeners_by_id(Method, Body = #{bindings := Bindings}) -> @@ -787,3 +807,37 @@ tcp_schema_example() -> type => tcp, zone => default }. + +create_listener(Body) -> + case parse_listener_conf(Body) of + {Id, Type, Name, Conf} -> + Path = [listeners, Type, Name], + case create(Path, Conf) of + {ok, #{raw_config := _RawConf}} -> + crud_listeners_by_id(get, #{bindings => #{id => Id}}); + {error, already_exist} -> + {400, #{code => 'BAD_LISTENER_ID', message => <<"Already Exist">>}}; + {error, Reason} -> + {400, #{code => 'BAD_REQUEST', message => err_msg(Reason)}} + end; + {error, Reason} -> + {400, #{code => 'BAD_REQUEST', message => err_msg(Reason)}} + end. + +listener_struct(Type) -> + Listeners = listeners_info(#{bind => true}) ++ listeners_info(#{bind => false}), + [Schema] = [S || #{ref := ?R_REF(_, T), schema := S} <- Listeners, T =:= Type], + Schema. + +listener_struct_with_name(Type) -> + BaseSchema = listener_struct(Type), + lists:keyreplace( + id, + 1, + BaseSchema, + {name, + ?HOCON(binary(), #{ + desc => "Listener name", + required => true + })} + ). 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 f72f9b762..10d04db85 100644 --- a/apps/emqx_management/test/emqx_mgmt_api_listeners_SUITE.erl +++ b/apps/emqx_management/test/emqx_mgmt_api_listeners_SUITE.erl @@ -37,6 +37,35 @@ t_list_listeners(_) -> Res = request(get, Path, [], []), #{<<"listeners">> := Expect} = emqx_mgmt_api_listeners:do_list_listeners(), ?assertEqual(length(Expect), length(Res)), + + %% POST /listeners + ListenerId = <<"tcp:default">>, + NewListenerId = <<"tcp:new">>, + + OriginPath = emqx_mgmt_api_test_util:api_path(["listeners", ListenerId]), + NewPath = emqx_mgmt_api_test_util:api_path(["listeners", NewListenerId]), + + OriginListener = request(get, OriginPath, [], []), + + %% create with full options + ?assertEqual({error, not_found}, is_running(NewListenerId)), + ?assertMatch({error, {"HTTP/1.1", 404, _}}, request(get, NewPath, [], [])), + + OriginListener2 = maps:remove(<<"id">>, OriginListener), + NewConf = OriginListener2#{ + <<"name">> => <<"new">>, + <<"bind">> => <<"0.0.0.0:2883">> + }, + Create = request(post, Path, [], NewConf), + ?assertEqual(lists:sort(maps:keys(OriginListener)), lists:sort(maps:keys(Create))), + Get1 = request(get, NewPath, [], []), + ?assertMatch(Create, Get1), + ?assert(is_running(NewListenerId)), + + %% delete + ?assertEqual([], delete(NewPath)), + ?assertEqual({error, not_found}, is_running(NewListenerId)), + ?assertMatch({error, {"HTTP/1.1", 404, _}}, request(get, NewPath, [], [])), ok. t_tcp_crud_listeners_by_id(_) -> From b43eaceab6125ea75f9164eb0e4f02158b944e1d Mon Sep 17 00:00:00 2001 From: firest Date: Thu, 8 Sep 2022 20:30:43 +0800 Subject: [PATCH 07/34] chore: update CHANGES-5.0.md --- CHANGES-5.0.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGES-5.0.md b/CHANGES-5.0.md index 8bddd1e75..9f86a8ca9 100644 --- a/CHANGES-5.0.md +++ b/CHANGES-5.0.md @@ -16,6 +16,7 @@ * Change the `/gateway` API path to plural form. [#8823](https://github.com/emqx/emqx/pull/8823) * Remove `node.etc_dir` from emqx.conf, because it is never used. Also allow user to customize the logging directory [#8892](https://github.com/emqx/emqx/pull/8892) +* Added a new API `POST /listeners` for creating listener. [#8876](https://github.com/emqx/emqx/pull/8876) # 5.0.7 From 061e3c264bd3a7b028b5d55b8ddd135cb4948475 Mon Sep 17 00:00:00 2001 From: firest Date: Fri, 9 Sep 2022 11:41:27 +0800 Subject: [PATCH 08/34] fix(authn_redis): fix that redis authn will deny the unknown users --- .../src/simple_authn/emqx_authn_redis.erl | 4 ++-- apps/emqx_authn/test/emqx_authn_redis_SUITE.erl | 14 +++++++++++++- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/apps/emqx_authn/src/simple_authn/emqx_authn_redis.erl b/apps/emqx_authn/src/simple_authn/emqx_authn_redis.erl index 684d60e49..4cc00322f 100644 --- a/apps/emqx_authn/src/simple_authn/emqx_authn_redis.erl +++ b/apps/emqx_authn/src/simple_authn/emqx_authn_redis.erl @@ -149,8 +149,8 @@ authenticate( of ok -> {ok, emqx_authn_utils:is_superuser(Selected)}; - {error, Reason} -> - {error, Reason} + {error, _Reason} -> + ignore end; {error, Reason} -> ?TRACE_AUTHN_PROVIDER(error, "redis_query_failed", #{ diff --git a/apps/emqx_authn/test/emqx_authn_redis_SUITE.erl b/apps/emqx_authn/test/emqx_authn_redis_SUITE.erl index 889404c5e..f9ed8bcb1 100644 --- a/apps/emqx_authn/test/emqx_authn_redis_SUITE.erl +++ b/apps/emqx_authn/test/emqx_authn_redis_SUITE.erl @@ -173,6 +173,9 @@ test_user_auth(#{ {create_authenticator, ?GLOBAL, AuthConfig} ), + {ok, [#{provider := emqx_authn_redis, state := State}]} = + emqx_authentication:list_authenticators(?GLOBAL), + Credentials = Credentials0#{ listener => 'tcp:default', protocol => mqtt @@ -180,6 +183,15 @@ test_user_auth(#{ ?assertEqual(Result, emqx_access_control:authenticate(Credentials)), + AuthnResult = + case Result of + {error, _} -> + ignore; + Any -> + Any + end, + ?assertEqual(AuthnResult, emqx_authn_redis:authenticate(Credentials, State)), + emqx_authn_test_lib:delete_authenticators( [authentication], ?GLOBAL @@ -466,7 +478,7 @@ user_seeds() -> <<"cmd">> => <<"HMGET mqtt_user:${username} password_hash salt is_superuser">>, <<"password_hash_algorithm">> => #{<<"name">> => <<"bcrypt">>} }, - result => {error, bad_username_or_password} + result => {error, not_authorized} }, #{ From 311fa5288f20fe04e3bf597af3b7f0a6bedb8b79 Mon Sep 17 00:00:00 2001 From: firest Date: Fri, 9 Sep 2022 15:31:50 +0800 Subject: [PATCH 09/34] chore: update CHANGES-5.0.md --- CHANGES-5.0.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGES-5.0.md b/CHANGES-5.0.md index 9f86a8ca9..8f85047e2 100644 --- a/CHANGES-5.0.md +++ b/CHANGES-5.0.md @@ -9,6 +9,7 @@ * Fix the extra / prefix when CoAP gateway parsing client topics. [#8658](https://github.com/emqx/emqx/pull/8658) * Speed up updating the configuration, When some nodes in the cluster are down. [#8857](https://github.com/emqx/emqx/pull/8857) * Fix that EMQX can't start when the retainer is disabled [#8911](https://github.com/emqx/emqx/pull/8911) +* Fix that redis authn will deny the unknown users [#8934](https://github.com/emqx/emqx/pull/8934) ## Enhancements From dfc6e346801fea1023bb5adf0f2132b49c74d839 Mon Sep 17 00:00:00 2001 From: JianBo He Date: Thu, 1 Sep 2022 18:03:05 +0800 Subject: [PATCH 10/34] fix(exproto): avoid udp client process leaking porting from v4.x: - https://github.com/emqx/emqx/pull/8575 - https://github.com/emqx/emqx/pull/8628 - https://github.com/emqx/emqx/pull/8725 --- apps/emqx_exhook/src/emqx_exhook_mgr.erl | 2 +- apps/emqx_exhook/src/emqx_exhook_server.erl | 6 +- .../src/bhvrs/emqx_gateway_conn.erl | 11 ++- .../src/exproto/emqx_exproto_channel.erl | 68 +++++++++++++++---- .../src/exproto/emqx_exproto_gcli.erl | 15 +++- apps/emqx_gateway/test/emqx_exproto_SUITE.erl | 6 +- rebar.config | 2 +- 7 files changed, 85 insertions(+), 25 deletions(-) diff --git a/apps/emqx_exhook/src/emqx_exhook_mgr.erl b/apps/emqx_exhook/src/emqx_exhook_mgr.erl index cf83e8eb9..9b062e914 100644 --- a/apps/emqx_exhook/src/emqx_exhook_mgr.erl +++ b/apps/emqx_exhook/src/emqx_exhook_mgr.erl @@ -298,6 +298,7 @@ handle_info(_Info, State) -> {noreply, State}. terminate(_Reason, State = #{servers := Servers}) -> + _ = unload_exhooks(), _ = maps:fold( fun(Name, _, AccIn) -> do_unload_server(Name, AccIn) @@ -305,7 +306,6 @@ terminate(_Reason, State = #{servers := Servers}) -> State, Servers ), - _ = unload_exhooks(), ok. code_change(_OldVsn, State, _Extra) -> diff --git a/apps/emqx_exhook/src/emqx_exhook_server.erl b/apps/emqx_exhook/src/emqx_exhook_server.erl index b15724ff2..e5075bce4 100644 --- a/apps/emqx_exhook/src/emqx_exhook_server.erl +++ b/apps/emqx_exhook/src/emqx_exhook_server.erl @@ -179,13 +179,15 @@ filter(Ls) -> -spec unload(server()) -> ok. unload(#{name := Name, options := ReqOpts, hookspec := HookSpecs}) -> - _ = do_deinit(Name, ReqOpts), _ = may_unload_hooks(HookSpecs), + _ = do_deinit(Name, ReqOpts), _ = emqx_exhook_sup:stop_grpc_client_channel(Name), ok. do_deinit(Name, ReqOpts) -> - _ = do_call(Name, undefined, 'on_provider_unloaded', #{}, ReqOpts), + %% Using shorter timeout to deinit grpc server to avoid emqx_exhook_mgr + %% force killed by upper supervisor + _ = do_call(Name, undefined, 'on_provider_unloaded', #{}, ReqOpts#{timeout => 3000}), ok. do_init(ChannName, ReqOpts) -> diff --git a/apps/emqx_gateway/src/bhvrs/emqx_gateway_conn.erl b/apps/emqx_gateway/src/bhvrs/emqx_gateway_conn.erl index 9e65c7eea..02d6090b4 100644 --- a/apps/emqx_gateway/src/bhvrs/emqx_gateway_conn.erl +++ b/apps/emqx_gateway/src/bhvrs/emqx_gateway_conn.erl @@ -240,6 +240,11 @@ esockd_send(Data, #state{ esockd_send(Data, #state{socket = {esockd_transport, Sock}}) -> esockd_transport:async_send(Sock, Data). +keepalive_stats(recv_oct) -> + emqx_pd:get_counter(incoming_bytes); +keepalive_stats(send_oct) -> + emqx_pd:get_counter(outgoing_bytes). + is_datadram_socket({esockd_transport, _}) -> false; is_datadram_socket({udp, _, _}) -> true. @@ -651,9 +656,9 @@ handle_timeout( disconnected -> {ok, State}; _ -> - case esockd_getstat(Socket, [Stat]) of - {ok, [{Stat, RecvOct}]} -> - handle_timeout(TRef, {Keepalive, RecvOct}, State); + case keepalive_stats(Stat) of + {ok, Oct} -> + handle_timeout(TRef, {Keepalive, Oct}, State); {error, Reason} -> handle_info({sock_error, Reason}, State) end diff --git a/apps/emqx_gateway/src/exproto/emqx_exproto_channel.erl b/apps/emqx_gateway/src/exproto/emqx_exproto_channel.erl index 861ae3189..3380f35be 100644 --- a/apps/emqx_gateway/src/exproto/emqx_exproto_channel.erl +++ b/apps/emqx_gateway/src/exproto/emqx_exproto_channel.erl @@ -78,11 +78,14 @@ -define(TIMER_TABLE, #{ alive_timer => keepalive, - force_timer => force_close + force_timer => force_close, + idle_timer => force_close_idle }). -define(INFO_KEYS, [conninfo, conn_state, clientinfo, session, will_msg]). +-define(DEFAULT_IDLE_TIMEOUT, 30000). + %%-------------------------------------------------------------------- %% Info, Attrs and Caps %%-------------------------------------------------------------------- @@ -151,14 +154,17 @@ init( Ctx = maps:get(ctx, Options), GRpcChann = maps:get(handler, Options), PoolName = maps:get(pool_name, Options), - NConnInfo = default_conninfo(ConnInfo), + IdleTimeout = proplists:get_value(idle_timeout, Options, ?DEFAULT_IDLE_TIMEOUT), + + NConnInfo = default_conninfo(ConnInfo#{idle_timeout => IdleTimeout}), ListenerId = case maps:get(listener, Options, undefined) of undefined -> undefined; {GwName, Type, LisName} -> emqx_gateway_utils:listener_id(GwName, Type, LisName) end, + EnableAuthn = maps:get(enable_authn, Options, true), - DefaultClientInfo = default_clientinfo(ConnInfo), + DefaultClientInfo = default_clientinfo(NConnInfo), ClientInfo = DefaultClientInfo#{ listener => ListenerId, enable_authn => EnableAuthn @@ -183,7 +189,9 @@ init( } ) }, - try_dispatch(on_socket_created, wrap(Req), Channel). + start_idle_checking_timer( + try_dispatch(on_socket_created, wrap(Req), Channel) + ). %% @private peercert(NoSsl, ConnInfo) when @@ -217,6 +225,12 @@ socktype(dtls) -> 'DTLS'. address({Host, Port}) -> #{host => inet:ntoa(Host), port => Port}. +%% avoid udp connection process leak +start_idle_checking_timer(Channel = #channel{conninfo = #{socktype := udp}}) -> + ensure_timer(idle_timer, Channel); +start_idle_checking_timer(Channel) -> + Channel. + %%-------------------------------------------------------------------- %% Handle incoming packet %%-------------------------------------------------------------------- @@ -285,10 +299,15 @@ handle_timeout( {ok, reset_timer(alive_timer, NChannel)}; {error, timeout} -> Req = #{type => 'KEEPALIVE'}, - {ok, try_dispatch(on_timer_timeout, wrap(Req), Channel)} + NChannel = clean_timer(alive_timer, Channel), + %% close connection if keepalive timeout + Replies = [{event, disconnected}, {close, normal}], + {ok, Replies, try_dispatch(on_timer_timeout, wrap(Req), NChannel)} end; handle_timeout(_TRef, force_close, Channel = #channel{closed_reason = Reason}) -> {shutdown, {error, {force_close, Reason}}, Channel}; +handle_timeout(_TRef, force_close_idle, Channel) -> + {shutdown, idle_timeout, Channel}; handle_timeout(_TRef, Msg, Channel) -> ?SLOG(warning, #{ msg => "unexpected_timeout_signal", @@ -390,7 +409,7 @@ handle_call( NConnInfo = ConnInfo#{keepalive => Interval}, NClientInfo = ClientInfo#{keepalive => Interval}, NChannel = Channel#channel{conninfo = NConnInfo, clientinfo = NClientInfo}, - {reply, ok, ensure_keepalive(NChannel)}; + {reply, ok, [{event, updated}], ensure_keepalive(cancel_timer(idle_timer, NChannel))}; handle_call( {subscribe_from_client, TopicFilter, Qos}, _From, @@ -405,21 +424,21 @@ handle_call( {reply, {error, ?RESP_PERMISSION_DENY, <<"Authorization deny">>}, Channel}; _ -> {ok, _, NChannel} = do_subscribe([{TopicFilter, #{qos => Qos}}], Channel), - {reply, ok, NChannel} + {reply, ok, [{event, updated}], NChannel} end; handle_call({subscribe, Topic, SubOpts}, _From, Channel) -> {ok, [{NTopicFilter, NSubOpts}], NChannel} = do_subscribe([{Topic, SubOpts}], Channel), - {reply, {ok, {NTopicFilter, NSubOpts}}, NChannel}; + {reply, {ok, {NTopicFilter, NSubOpts}}, [{event, updated}], NChannel}; handle_call( {unsubscribe_from_client, TopicFilter}, _From, Channel = #channel{conn_state = connected} ) -> {ok, NChannel} = do_unsubscribe([{TopicFilter, #{}}], Channel), - {reply, ok, NChannel}; + {reply, ok, [{event, updated}], NChannel}; handle_call({unsubscribe, Topic}, _From, Channel) -> {ok, NChannel} = do_unsubscribe([Topic], Channel), - {reply, ok, NChannel}; + {reply, ok, [{event, update}], NChannel}; handle_call(subscriptions, _From, Channel = #channel{subscriptions = Subs}) -> {reply, {ok, maps:to_list(Subs)}, Channel}; handle_call( @@ -446,7 +465,7 @@ handle_call( {reply, ok, Channel} end; handle_call(kick, _From, Channel) -> - {shutdown, kicked, ok, ensure_disconnected(kicked, Channel)}; + {reply, ok, [{event, disconnected}, {close, kicked}], Channel}; handle_call(discard, _From, Channel) -> {shutdown, discarded, ok, Channel}; handle_call(Req, _From, Channel) -> @@ -671,6 +690,12 @@ reset_timer(Name, Channel) -> clean_timer(Name, Channel = #channel{timers = Timers}) -> Channel#channel{timers = maps:remove(Name, Timers)}. +cancel_timer(Name, Channel = #channel{timers = Timers}) -> + emqx_misc:cancel_timer(maps:get(Name, Timers, undefined)), + clean_timer(Name, Channel). + +interval(idle_timer, #channel{conninfo = #{idle_timeout := IdleTimeout}}) -> + IdleTimeout; interval(force_timer, _) -> 15000; interval(alive_timer, #channel{keepalive = Keepalive}) -> @@ -722,10 +747,10 @@ enrich_clientinfo(InClientInfo = #{proto_name := ProtoName}, ClientInfo) -> NClientInfo = maps:merge(ClientInfo, maps:with(Ks, InClientInfo)), NClientInfo#{protocol => proto_name_to_protocol(ProtoName)}. -default_conninfo(ConnInfo) -> +default_conninfo(ConnInfo = #{peername := {PeerHost, PeerPort}}) -> ConnInfo#{ clean_start => true, - clientid => undefined, + clientid => anonymous_clientid(PeerHost, PeerPort), username => undefined, conn_props => #{}, connected => true, @@ -739,14 +764,15 @@ default_conninfo(ConnInfo) -> default_clientinfo(#{ peername := {PeerHost, _}, - sockname := {_, SockPort} + sockname := {_, SockPort}, + clientid := ClientId }) -> #{ zone => default, protocol => exproto, peerhost => PeerHost, sockport => SockPort, - clientid => undefined, + clientid => ClientId, username => undefined, is_bridge => false, is_superuser => false, @@ -764,3 +790,15 @@ proto_name_to_protocol(<<>>) -> exproto; proto_name_to_protocol(ProtoName) when is_binary(ProtoName) -> binary_to_atom(ProtoName). + +anonymous_clientid(PeerHost, PeerPort) -> + iolist_to_binary( + [ + "exproto-anonymous-", + inet:ntoa(PeerHost), + "-", + integer_to_list(PeerPort), + "-", + emqx_misc:gen_id() + ] + ). diff --git a/apps/emqx_gateway/src/exproto/emqx_exproto_gcli.erl b/apps/emqx_gateway/src/exproto/emqx_exproto_gcli.erl index d1bf4ba94..45b502798 100644 --- a/apps/emqx_gateway/src/exproto/emqx_exproto_gcli.erl +++ b/apps/emqx_gateway/src/exproto/emqx_exproto_gcli.erl @@ -61,7 +61,19 @@ async_call( Req = #{conn := Conn}, Options = #{pool_name := PoolName} ) -> - cast(pick(PoolName, Conn), {rpc, FunName, Req, Options, self()}). + case pick(PoolName, Conn) of + false -> + ?SLOG( + error, + #{ + msg => "no_available_grpc_client", + function => FunName, + request => Req + } + ); + Pid when is_pid(Pid) -> + cast(Pid, {rpc, FunName, Req, Options, self()}) + end. %%-------------------------------------------------------------------- %% cast, pick @@ -72,6 +84,7 @@ async_call( cast(Deliver, Msg) -> gen_server:cast(Deliver, Msg). +-spec pick(term(), term()) -> pid() | false. pick(PoolName, Conn) -> gproc_pool:pick_worker(PoolName, Conn). diff --git a/apps/emqx_gateway/test/emqx_exproto_SUITE.erl b/apps/emqx_gateway/test/emqx_exproto_SUITE.erl index 66f780d3f..65725fd19 100644 --- a/apps/emqx_gateway/test/emqx_exproto_SUITE.erl +++ b/apps/emqx_gateway/test/emqx_exproto_SUITE.erl @@ -240,8 +240,10 @@ t_keepalive_timeout(Cfg) -> send(Sock, ConnBin), {ok, ConnAckBin} = recv(Sock, 5000), - DisconnectBin = frame_disconnect(), - {ok, DisconnectBin} = recv(Sock, 10000), + %% Timed out connections are closed immediately, + %% so there may not be a disconnect message here + %%DisconnectBin = frame_disconnect(), + %%{ok, DisconnectBin} = recv(Sock, 10000), SockType =/= udp andalso begin diff --git a/rebar.config b/rebar.config index 271e274da..e9a8cc425 100644 --- a/rebar.config +++ b/rebar.config @@ -56,7 +56,7 @@ , {esockd, {git, "https://github.com/emqx/esockd", {tag, "5.9.4"}}} , {ekka, {git, "https://github.com/emqx/ekka", {tag, "0.13.4"}}} , {gen_rpc, {git, "https://github.com/emqx/gen_rpc", {tag, "2.8.1"}}} - , {grpc, {git, "https://github.com/emqx/grpc-erl", {tag, "0.6.6"}}} + , {grpc, {git, "https://github.com/emqx/grpc-erl", {tag, "0.6.7"}}} , {minirest, {git, "https://github.com/emqx/minirest", {tag, "1.3.7"}}} , {ecpool, {git, "https://github.com/emqx/ecpool", {tag, "0.5.2"}}} , {replayq, "0.3.4"} From ebb2824e1516fa6aa33645c4de92981d10b2b620 Mon Sep 17 00:00:00 2001 From: JianBo He Date: Fri, 2 Sep 2022 10:26:14 +0800 Subject: [PATCH 11/34] test: ensure hooks has unloaded if grpc is blocked --- apps/emqx_exhook/src/emqx_exhook_mgr.erl | 4 ++- apps/emqx_exhook/src/emqx_exhook_server.erl | 6 ++-- apps/emqx_exhook/src/emqx_exhook_sup.erl | 1 + apps/emqx_exhook/test/emqx_exhook_SUITE.erl | 35 +++++++++++++++++++ .../emqx_exhook/test/emqx_exhook_demo_svr.erl | 5 ++- .../src/bhvrs/emqx_gateway_conn.erl | 1 - 6 files changed, 46 insertions(+), 6 deletions(-) diff --git a/apps/emqx_exhook/src/emqx_exhook_mgr.erl b/apps/emqx_exhook/src/emqx_exhook_mgr.erl index 9b062e914..e58555ca1 100644 --- a/apps/emqx_exhook/src/emqx_exhook_mgr.erl +++ b/apps/emqx_exhook/src/emqx_exhook_mgr.erl @@ -21,6 +21,7 @@ -include("emqx_exhook.hrl"). -include_lib("emqx/include/logger.hrl"). +-include_lib("snabbkaffe/include/snabbkaffe.hrl"). %% APIs -export([start_link/0]). @@ -297,7 +298,7 @@ handle_info(refresh_tick, State) -> handle_info(_Info, State) -> {noreply, State}. -terminate(_Reason, State = #{servers := Servers}) -> +terminate(Reason, State = #{servers := Servers}) -> _ = unload_exhooks(), _ = maps:fold( fun(Name, _, AccIn) -> @@ -306,6 +307,7 @@ terminate(_Reason, State = #{servers := Servers}) -> State, Servers ), + ?tp(info, exhook_mgr_terminated, #{reason => Reason, servers => Servers}), ok. code_change(_OldVsn, State, _Extra) -> diff --git a/apps/emqx_exhook/src/emqx_exhook_server.erl b/apps/emqx_exhook/src/emqx_exhook_server.erl index e5075bce4..311cff316 100644 --- a/apps/emqx_exhook/src/emqx_exhook_server.erl +++ b/apps/emqx_exhook/src/emqx_exhook_server.erl @@ -185,9 +185,9 @@ unload(#{name := Name, options := ReqOpts, hookspec := HookSpecs}) -> ok. do_deinit(Name, ReqOpts) -> - %% Using shorter timeout to deinit grpc server to avoid emqx_exhook_mgr - %% force killed by upper supervisor - _ = do_call(Name, undefined, 'on_provider_unloaded', #{}, ReqOpts#{timeout => 3000}), + %% Override the request timeout to deinit grpc server to + %% avoid emqx_exhook_mgr force killed by upper supervisor + _ = do_call(Name, undefined, 'on_provider_unloaded', #{}, ReqOpts#{timeout => 5000}), ok. do_init(ChannName, ReqOpts) -> diff --git a/apps/emqx_exhook/src/emqx_exhook_sup.erl b/apps/emqx_exhook/src/emqx_exhook_sup.erl index fb424ddff..3423eaf59 100644 --- a/apps/emqx_exhook/src/emqx_exhook_sup.erl +++ b/apps/emqx_exhook/src/emqx_exhook_sup.erl @@ -32,6 +32,7 @@ id => Mod, start => {Mod, start_link, Args}, type => Type, + %% long timeout for emqx_exhook_mgr shutdown => 15000 }). diff --git a/apps/emqx_exhook/test/emqx_exhook_SUITE.erl b/apps/emqx_exhook/test/emqx_exhook_SUITE.erl index 62606cf18..a0472b2c3 100644 --- a/apps/emqx_exhook/test/emqx_exhook_SUITE.erl +++ b/apps/emqx_exhook/test/emqx_exhook_SUITE.erl @@ -24,6 +24,7 @@ -include_lib("eunit/include/eunit.hrl"). -include_lib("common_test/include/ct.hrl"). -include_lib("emqx/include/emqx_hooks.hrl"). +-include_lib("snabbkaffe/include/snabbkaffe.hrl"). -define(DEFAULT_CLUSTER_NAME_ATOM, emqxcl). @@ -313,6 +314,40 @@ t_cluster_name(_) -> ), emqx_exhook_mgr:disable(<<"default">>). +t_stop_timeout(_) -> + snabbkaffe:start_trace(), + meck:new(emqx_exhook_demo_svr, [passthrough, no_history]), + meck:expect( + emqx_exhook_demo_svr, + on_provider_unloaded, + fun(Req, Md) -> + %% ensure sleep time greater than emqx_exhook_mgr shutdown timeout + timer:sleep(20000), + meck:passthrough([Req, Md]) + end + ), + + %% stop application + application:stop(emqx_exhook), + ?block_until(#{?snk_kind := exhook_mgr_terminated}, 20000), + + %% all exhook hooked point should be unloaded + Mods = lists:flatten( + lists:map( + fun({hook, _, Cbs}) -> + lists:map(fun({callback, {M, _, _}, _, _}) -> M end, Cbs) + end, + ets:tab2list(emqx_hooks) + ) + ), + ?assertEqual(false, lists:any(fun(M) -> M == emqx_exhook_handler end, Mods)), + + %% ensure started for other tests + emqx_common_test_helpers:start_apps([emqx_exhook]), + + snabbkaffe:stop(), + meck:unload(emqx_exhook_demo_svr). + %%-------------------------------------------------------------------- %% Cases Helpers %%-------------------------------------------------------------------- diff --git a/apps/emqx_exhook/test/emqx_exhook_demo_svr.erl b/apps/emqx_exhook/test/emqx_exhook_demo_svr.erl index ea8398eeb..bf1a42c9a 100644 --- a/apps/emqx_exhook/test/emqx_exhook_demo_svr.erl +++ b/apps/emqx_exhook/test/emqx_exhook_demo_svr.erl @@ -80,7 +80,10 @@ stop() -> stop(Name) -> grpc:stop_server(Name), - to_atom_name(Name) ! stop. + case whereis(to_atom_name(Name)) of + undefined -> ok; + Pid -> Pid ! stop + end. take() -> to_atom_name(?NAME) ! {take, self()}, diff --git a/apps/emqx_gateway/src/bhvrs/emqx_gateway_conn.erl b/apps/emqx_gateway/src/bhvrs/emqx_gateway_conn.erl index 02d6090b4..786142cdd 100644 --- a/apps/emqx_gateway/src/bhvrs/emqx_gateway_conn.erl +++ b/apps/emqx_gateway/src/bhvrs/emqx_gateway_conn.erl @@ -640,7 +640,6 @@ handle_timeout( Keepalive, State = #state{ chann_mod = ChannMod, - socket = Socket, channel = Channel } ) when From f8614196ac1e69ba0e2ca3eba571eb47829ecddb Mon Sep 17 00:00:00 2001 From: JianBo He Date: Fri, 2 Sep 2022 14:50:17 +0800 Subject: [PATCH 12/34] test: ensure udp client keepalive value getting right value --- .../src/bhvrs/emqx_gateway_conn.erl | 19 ++++--- .../src/exproto/emqx_exproto_channel.erl | 9 ++- apps/emqx_gateway/test/emqx_exproto_SUITE.erl | 56 ++++++++++++++----- 3 files changed, 59 insertions(+), 25 deletions(-) diff --git a/apps/emqx_gateway/src/bhvrs/emqx_gateway_conn.erl b/apps/emqx_gateway/src/bhvrs/emqx_gateway_conn.erl index 786142cdd..24d63f02c 100644 --- a/apps/emqx_gateway/src/bhvrs/emqx_gateway_conn.erl +++ b/apps/emqx_gateway/src/bhvrs/emqx_gateway_conn.erl @@ -19,6 +19,7 @@ -include_lib("emqx/include/types.hrl"). -include_lib("emqx/include/logger.hrl"). +-include_lib("snabbkaffe/include/snabbkaffe.hrl"). %% API -export([ @@ -51,6 +52,9 @@ %% Internal callback -export([wakeup_from_hib/2, recvloop/2]). +%% for channel module +-export([keepalive_stats/1]). + -record(state, { %% TCP/SSL/UDP/DTLS Wrapped Socket socket :: {esockd_transport, esockd:socket()} | {udp, _, _}, @@ -573,9 +577,15 @@ terminate( channel = Channel } ) -> - ?SLOG(debug, #{msg => "conn_process_terminated", reason => Reason}), _ = ChannMod:terminate(Reason, Channel), _ = close_socket(State), + ClientId = + try ChannMod:info(clientid, Channel) of + Id -> Id + catch + _:_ -> undefined + end, + ?tp(debug, conn_process_terminated, #{reason => Reason, clientid => ClientId}), exit(Reason). %%-------------------------------------------------------------------- @@ -655,12 +665,7 @@ handle_timeout( disconnected -> {ok, State}; _ -> - case keepalive_stats(Stat) of - {ok, Oct} -> - handle_timeout(TRef, {Keepalive, Oct}, State); - {error, Reason} -> - handle_info({sock_error, Reason}, State) - end + handle_timeout(TRef, {Keepalive, keepalive_stats(Stat)}, State) end; handle_timeout( _TRef, diff --git a/apps/emqx_gateway/src/exproto/emqx_exproto_channel.erl b/apps/emqx_gateway/src/exproto/emqx_exproto_channel.erl index 3380f35be..94ca031aa 100644 --- a/apps/emqx_gateway/src/exproto/emqx_exproto_channel.erl +++ b/apps/emqx_gateway/src/exproto/emqx_exproto_channel.erl @@ -84,8 +84,6 @@ -define(INFO_KEYS, [conninfo, conn_state, clientinfo, session, will_msg]). --define(DEFAULT_IDLE_TIMEOUT, 30000). - %%-------------------------------------------------------------------- %% Info, Attrs and Caps %%-------------------------------------------------------------------- @@ -154,7 +152,7 @@ init( Ctx = maps:get(ctx, Options), GRpcChann = maps:get(handler, Options), PoolName = maps:get(pool_name, Options), - IdleTimeout = proplists:get_value(idle_timeout, Options, ?DEFAULT_IDLE_TIMEOUT), + IdleTimeout = emqx_gateway_utils:idle_timeout(Options), NConnInfo = default_conninfo(ConnInfo#{idle_timeout => IdleTimeout}), ListenerId = @@ -301,7 +299,7 @@ handle_timeout( Req = #{type => 'KEEPALIVE'}, NChannel = clean_timer(alive_timer, Channel), %% close connection if keepalive timeout - Replies = [{event, disconnected}, {close, normal}], + Replies = [{event, disconnected}, {close, keepalive_timeout}], {ok, Replies, try_dispatch(on_timer_timeout, wrap(Req), NChannel)} end; handle_timeout(_TRef, force_close, Channel = #channel{closed_reason = Reason}) -> @@ -667,7 +665,8 @@ ensure_keepalive(Channel = #channel{clientinfo = ClientInfo}) -> ensure_keepalive_timer(Interval, Channel) when Interval =< 0 -> Channel; ensure_keepalive_timer(Interval, Channel) -> - Keepalive = emqx_keepalive:init(timer:seconds(Interval)), + StatVal = emqx_gateway_conn:keepalive_stats(recv_oct), + Keepalive = emqx_keepalive:init(StatVal, timer:seconds(Interval)), ensure_timer(alive_timer, Channel#channel{keepalive = Keepalive}). ensure_timer(Name, Channel = #channel{timers = Timers}) -> diff --git a/apps/emqx_gateway/test/emqx_exproto_SUITE.erl b/apps/emqx_gateway/test/emqx_exproto_SUITE.erl index 65725fd19..73dc2ad45 100644 --- a/apps/emqx_gateway/test/emqx_exproto_SUITE.erl +++ b/apps/emqx_gateway/test/emqx_exproto_SUITE.erl @@ -20,6 +20,7 @@ -compile(nowarn_export_all). -include_lib("emqx/include/emqx_hooks.hrl"). +-include_lib("eunit/include/eunit.hrl"). -import( emqx_exproto_echo_svr, @@ -38,6 +39,7 @@ -include_lib("emqx/include/emqx.hrl"). -include_lib("emqx/include/emqx_mqtt.hrl"). +-include_lib("snabbkaffe/include/snabbkaffe.hrl"). -define(TCPOPTS, [binary, {active, false}]). -define(DTLSOPTS, [binary, {active, false}, {protocol, dtls}]). @@ -223,14 +225,16 @@ t_acl_deny(Cfg) -> close(Sock). t_keepalive_timeout(Cfg) -> + ok = snabbkaffe:start_trace(), SockType = proplists:get_value(listener_type, Cfg), Sock = open(SockType), + ClientId1 = <<"keepalive_test_client1">>, Client = #{ proto_name => <<"demo">>, proto_ver => <<"v0.1">>, - clientid => <<"test_client_1">>, - keepalive => 2 + clientid => ClientId1, + keepalive => 5 }, Password = <<"123456">>, @@ -238,18 +242,41 @@ t_keepalive_timeout(Cfg) -> ConnAckBin = frame_connack(0), send(Sock, ConnBin), - {ok, ConnAckBin} = recv(Sock, 5000), + {ok, ConnAckBin} = recv(Sock), - %% Timed out connections are closed immediately, - %% so there may not be a disconnect message here - %%DisconnectBin = frame_disconnect(), - %%{ok, DisconnectBin} = recv(Sock, 10000), - - SockType =/= udp andalso - begin - {error, closed} = recv(Sock, 5000) - end, - ok. + case SockType of + udp -> + %% another udp client should not affect the first + %% udp client keepalive check + timer:sleep(4000), + Sock2 = open(SockType), + ConnBin2 = frame_connect( + Client#{clientid => <<"keepalive_test_client2">>}, + Password + ), + send(Sock2, ConnBin2), + %% first client will be keepalive timeouted in 6s + ?assertMatch( + {ok, #{ + clientid := ClientId1, + reason := {shutdown, {sock_closed, keepalive_timeout}} + }}, + ?block_until(#{?snk_kind := conn_process_terminated}, 8000) + ); + _ -> + ?assertMatch( + {ok, #{ + clientid := ClientId1, + reason := {shutdown, {sock_closed, keepalive_timeout}} + }}, + ?block_until(#{?snk_kind := conn_process_terminated}, 12000) + ), + Trace = snabbkaffe:collect_trace(), + %% conn process should be terminated + ?assertEqual(1, length(?of_kind(conn_process_terminated, Trace))), + %% socket port should be closed + ?assertEqual({error, closed}, recv(Sock, 5000)) + end. t_hook_connected_disconnected(Cfg) -> SockType = proplists:get_value(listener_type, Cfg), @@ -424,6 +451,9 @@ send({ssl, Sock}, Bin) -> send({dtls, Sock}, Bin) -> ssl:send(Sock, Bin). +recv(Sock) -> + recv(Sock, infinity). + recv({tcp, Sock}, Ts) -> gen_tcp:recv(Sock, 0, Ts); recv({udp, Sock}, Ts) -> From fbc213086566f921609d68178f19ae577f0b4aab Mon Sep 17 00:00:00 2001 From: JianBo He Date: Fri, 2 Sep 2022 15:19:08 +0800 Subject: [PATCH 13/34] test: add test for idle_timeout parameter --- apps/emqx_gateway/test/emqx_exproto_SUITE.erl | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/apps/emqx_gateway/test/emqx_exproto_SUITE.erl b/apps/emqx_gateway/test/emqx_exproto_SUITE.erl index 73dc2ad45..8e9783003 100644 --- a/apps/emqx_gateway/test/emqx_exproto_SUITE.erl +++ b/apps/emqx_gateway/test/emqx_exproto_SUITE.erl @@ -89,6 +89,7 @@ set_special_cfg(emqx_gateway) -> [gateway, exproto], #{ server => #{bind => 9100}, + idle_timeout => 5000, handler => #{address => "http://127.0.0.1:9001"}, listeners => listener_confs(LisType) } @@ -402,6 +403,44 @@ t_hook_message_delivered(Cfg) -> close(Sock), emqx_hooks:del('message.delivered', {?MODULE, hook_fun5}). +t_idle_timeout(Cfg) -> + ok = snabbkaffe:start_trace(), + SockType = proplists:get_value(listener_type, Cfg), + Sock = open(SockType), + + %% need to create udp client by sending something + case SockType of + udp -> + %% nothing to do + meck:new(emqx_exproto_echo_svr, [passthrough, no_history]), + meck:expect( + emqx_exproto_echo_svr, + on_received_bytes, + fun(Stream, _Md) -> {ok, Stream} end + ), + %% send request, but nobody can respond to it + ClientId = <<"idle_test_client1">>, + Client = #{ + proto_name => <<"demo">>, + proto_ver => <<"v0.1">>, + clientid => ClientId, + keepalive => 5 + }, + Password = <<"123456">>, + ConnBin = frame_connect(Client, Password), + send(Sock, ConnBin), + ?assertMatch( + {ok, #{reason := {shutdown, idle_timeout}}}, + ?block_until(#{?snk_kind := conn_process_terminated}, 10000) + ), + meck:unload(emqx_exproto_echo_svr); + _ -> + ?assertMatch( + {ok, #{reason := {shutdown, idle_timeout}}}, + ?block_until(#{?snk_kind := conn_process_terminated}, 10000) + ) + end. + %%-------------------------------------------------------------------- %% Utils From 880371934671bb491bf78e6e7fba605e6e9d38a8 Mon Sep 17 00:00:00 2001 From: JianBo He Date: Fri, 2 Sep 2022 15:24:10 +0800 Subject: [PATCH 14/34] chore: update changes --- CHANGES-5.0.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGES-5.0.md b/CHANGES-5.0.md index 8f85047e2..db1d09758 100644 --- a/CHANGES-5.0.md +++ b/CHANGES-5.0.md @@ -10,6 +10,8 @@ * Speed up updating the configuration, When some nodes in the cluster are down. [#8857](https://github.com/emqx/emqx/pull/8857) * Fix that EMQX can't start when the retainer is disabled [#8911](https://github.com/emqx/emqx/pull/8911) * Fix that redis authn will deny the unknown users [#8934](https://github.com/emqx/emqx/pull/8934) +* Fix ExProto UDP client keepalive checking error. + This causes the clients to not expire as long as a new UDP packet arrives [#8866](https://github.com/emqx/emqx/pull/8866) ## Enhancements @@ -18,6 +20,8 @@ * Remove `node.etc_dir` from emqx.conf, because it is never used. Also allow user to customize the logging directory [#8892](https://github.com/emqx/emqx/pull/8892) * Added a new API `POST /listeners` for creating listener. [#8876](https://github.com/emqx/emqx/pull/8876) +* Close ExProto client process immediately if it's keepalive timeouted. [#8866](https://github.com/emqx/emqx/pull/8866) +* Upgrade grpc-erl driver to 0.6.7 to support batch operation in sending stream. [#8866](https://github.com/emqx/emqx/pull/8866) # 5.0.7 From 533569ad2a1dcd6b6f53af2455df03cea8518cd9 Mon Sep 17 00:00:00 2001 From: JianBo He Date: Fri, 2 Sep 2022 15:44:20 +0800 Subject: [PATCH 15/34] chore: fix app vsn check --- apps/emqx_exhook/src/emqx_exhook.app.src | 2 +- apps/emqx_gateway/src/emqx_gateway.app.src | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/emqx_exhook/src/emqx_exhook.app.src b/apps/emqx_exhook/src/emqx_exhook.app.src index c4a43d846..f10155f0e 100644 --- a/apps/emqx_exhook/src/emqx_exhook.app.src +++ b/apps/emqx_exhook/src/emqx_exhook.app.src @@ -1,7 +1,7 @@ %% -*- mode: erlang -*- {application, emqx_exhook, [ {description, "EMQX Extension for Hook"}, - {vsn, "5.0.4"}, + {vsn, "5.0.3"}, {modules, []}, {registered, []}, {mod, {emqx_exhook_app, []}}, diff --git a/apps/emqx_gateway/src/emqx_gateway.app.src b/apps/emqx_gateway/src/emqx_gateway.app.src index 47245c0a2..9fb78c825 100644 --- a/apps/emqx_gateway/src/emqx_gateway.app.src +++ b/apps/emqx_gateway/src/emqx_gateway.app.src @@ -1,7 +1,7 @@ %% -*- mode: erlang -*- {application, emqx_gateway, [ {description, "The Gateway management application"}, - {vsn, "0.1.5"}, + {vsn, "0.1.4"}, {registered, []}, {mod, {emqx_gateway_app, []}}, {applications, [kernel, stdlib, grpc, emqx, emqx_authn]}, From aed2df58114bca90d4c79bca3dd22590daf57f57 Mon Sep 17 00:00:00 2001 From: JianBo He Date: Fri, 2 Sep 2022 16:09:15 +0800 Subject: [PATCH 16/34] chore: update grpc vsn in mix.exs --- mix.exs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mix.exs b/mix.exs index 3f55de64b..c4c827f92 100644 --- a/mix.exs +++ b/mix.exs @@ -54,7 +54,7 @@ defmodule EMQXUmbrella.MixProject do {:esockd, github: "emqx/esockd", tag: "5.9.4", override: true}, {:ekka, github: "emqx/ekka", tag: "0.13.4", override: true}, {:gen_rpc, github: "emqx/gen_rpc", tag: "2.8.1", override: true}, - {:grpc, github: "emqx/grpc-erl", tag: "0.6.6", override: true}, + {:grpc, github: "emqx/grpc-erl", tag: "0.6.7", override: true}, {:minirest, github: "emqx/minirest", tag: "1.3.7", override: true}, {:ecpool, github: "emqx/ecpool", tag: "0.5.2"}, {:replayq, "0.3.4", override: true}, From 4384cae29e7123469d33fb110ceaee0f12aa770f Mon Sep 17 00:00:00 2001 From: JianBo He Date: Thu, 8 Sep 2022 10:58:20 +0800 Subject: [PATCH 17/34] test: fix failed tests --- apps/emqx_gateway/test/emqx_exproto_SUITE.erl | 26 +++++++++++++------ 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/apps/emqx_gateway/test/emqx_exproto_SUITE.erl b/apps/emqx_gateway/test/emqx_exproto_SUITE.erl index 8e9783003..0e863a14c 100644 --- a/apps/emqx_gateway/test/emqx_exproto_SUITE.erl +++ b/apps/emqx_gateway/test/emqx_exproto_SUITE.erl @@ -64,6 +64,9 @@ all() -> [{group, Name} || Name <- metrics()]. +suite() -> + [{timetrap, {seconds, 30}}]. + groups() -> Cases = emqx_common_test_helpers:all(?MODULE), [{Name, Cases} || Name <- metrics()]. @@ -277,7 +280,8 @@ t_keepalive_timeout(Cfg) -> ?assertEqual(1, length(?of_kind(conn_process_terminated, Trace))), %% socket port should be closed ?assertEqual({error, closed}, recv(Sock, 5000)) - end. + end, + snabbkaffe:stop(). t_hook_connected_disconnected(Cfg) -> SockType = proplists:get_value(listener_type, Cfg), @@ -367,6 +371,8 @@ t_hook_session_subscribed_unsubscribed(Cfg) -> error(hook_is_not_running) end, + send(Sock, frame_disconnect()), + close(Sock), emqx_hooks:del('session.subscribed', {?MODULE, hook_fun3}), emqx_hooks:del('session.unsubscribed', {?MODULE, hook_fun4}). @@ -412,11 +418,14 @@ t_idle_timeout(Cfg) -> case SockType of udp -> %% nothing to do - meck:new(emqx_exproto_echo_svr, [passthrough, no_history]), - meck:expect( - emqx_exproto_echo_svr, - on_received_bytes, - fun(Stream, _Md) -> {ok, Stream} end + ok = meck:new(emqx_exproto_gcli, [passthrough, no_history]), + ok = meck:expect( + emqx_exproto_gcli, + async_call, + fun(FunName, _Req, _GClient) -> + self() ! {hreply, FunName, ok}, + ok + end ), %% send request, but nobody can respond to it ClientId = <<"idle_test_client1">>, @@ -433,13 +442,14 @@ t_idle_timeout(Cfg) -> {ok, #{reason := {shutdown, idle_timeout}}}, ?block_until(#{?snk_kind := conn_process_terminated}, 10000) ), - meck:unload(emqx_exproto_echo_svr); + ok = meck:unload(emqx_exproto_gcli); _ -> ?assertMatch( {ok, #{reason := {shutdown, idle_timeout}}}, ?block_until(#{?snk_kind := conn_process_terminated}, 10000) ) - end. + end, + snabbkaffe:stop(). %%-------------------------------------------------------------------- %% Utils From 44f8108228cae55c3d099391134e66c4f1aa45a3 Mon Sep 17 00:00:00 2001 From: JianBo He Date: Thu, 8 Sep 2022 11:03:17 +0800 Subject: [PATCH 18/34] chore: update app vsn --- apps/emqx_exhook/src/emqx_exhook.app.src | 2 +- apps/emqx_gateway/src/emqx_gateway.app.src | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/emqx_exhook/src/emqx_exhook.app.src b/apps/emqx_exhook/src/emqx_exhook.app.src index f10155f0e..c4a43d846 100644 --- a/apps/emqx_exhook/src/emqx_exhook.app.src +++ b/apps/emqx_exhook/src/emqx_exhook.app.src @@ -1,7 +1,7 @@ %% -*- mode: erlang -*- {application, emqx_exhook, [ {description, "EMQX Extension for Hook"}, - {vsn, "5.0.3"}, + {vsn, "5.0.4"}, {modules, []}, {registered, []}, {mod, {emqx_exhook_app, []}}, diff --git a/apps/emqx_gateway/src/emqx_gateway.app.src b/apps/emqx_gateway/src/emqx_gateway.app.src index 9fb78c825..47245c0a2 100644 --- a/apps/emqx_gateway/src/emqx_gateway.app.src +++ b/apps/emqx_gateway/src/emqx_gateway.app.src @@ -1,7 +1,7 @@ %% -*- mode: erlang -*- {application, emqx_gateway, [ {description, "The Gateway management application"}, - {vsn, "0.1.4"}, + {vsn, "0.1.5"}, {registered, []}, {mod, {emqx_gateway_app, []}}, {applications, [kernel, stdlib, grpc, emqx, emqx_authn]}, From 522f650096eb1da200b0afa6704309f7884a1227 Mon Sep 17 00:00:00 2001 From: JianBo He Date: Tue, 13 Sep 2022 10:28:53 +0800 Subject: [PATCH 19/34] chore: apply review suggestions --- apps/emqx_exhook/include/emqx_exhook.hrl | 2 ++ apps/emqx_exhook/src/emqx_exhook_server.erl | 3 +- apps/emqx_exhook/src/emqx_exhook_sup.erl | 17 +++++++--- .../emqx_exhook/test/emqx_exhook_demo_svr.erl | 10 ++++-- .../src/bhvrs/emqx_gateway_conn.erl | 16 +++++----- .../src/exproto/emqx_exproto_channel.erl | 31 +++++++------------ .../src/exproto/emqx_exproto_gcli.erl | 13 +++----- 7 files changed, 48 insertions(+), 44 deletions(-) diff --git a/apps/emqx_exhook/include/emqx_exhook.hrl b/apps/emqx_exhook/include/emqx_exhook.hrl index 6c386e688..7436b2a1c 100644 --- a/apps/emqx_exhook/include/emqx_exhook.hrl +++ b/apps/emqx_exhook/include/emqx_exhook.hrl @@ -43,6 +43,8 @@ {'message.dropped', {emqx_exhook_handler, on_message_dropped, []}} ]). +-define(SERVER_FORCE_SHUTDOWN_TIMEOUT, 5000). + -endif. -define(CMD_MOVE_FRONT, front). diff --git a/apps/emqx_exhook/src/emqx_exhook_server.erl b/apps/emqx_exhook/src/emqx_exhook_server.erl index 311cff316..9c89915aa 100644 --- a/apps/emqx_exhook/src/emqx_exhook_server.erl +++ b/apps/emqx_exhook/src/emqx_exhook_server.erl @@ -187,7 +187,8 @@ unload(#{name := Name, options := ReqOpts, hookspec := HookSpecs}) -> do_deinit(Name, ReqOpts) -> %% Override the request timeout to deinit grpc server to %% avoid emqx_exhook_mgr force killed by upper supervisor - _ = do_call(Name, undefined, 'on_provider_unloaded', #{}, ReqOpts#{timeout => 5000}), + NReqOpts = ReqOpts#{timeout => ?SERVER_FORCE_SHUTDOWN_TIMEOUT}, + _ = do_call(Name, undefined, 'on_provider_unloaded', #{}, NReqOpts), ok. do_init(ChannName, ReqOpts) -> diff --git a/apps/emqx_exhook/src/emqx_exhook_sup.erl b/apps/emqx_exhook/src/emqx_exhook_sup.erl index 3423eaf59..cd49d89bb 100644 --- a/apps/emqx_exhook/src/emqx_exhook_sup.erl +++ b/apps/emqx_exhook/src/emqx_exhook_sup.erl @@ -16,6 +16,8 @@ -module(emqx_exhook_sup). +-include("emqx_exhook.hrl"). + -behaviour(supervisor). -export([ @@ -28,12 +30,13 @@ stop_grpc_client_channel/1 ]). --define(CHILD(Mod, Type, Args), #{ +-define(DEFAULT_TIMEOUT, 5000). + +-define(CHILD(Mod, Type, Args, Timeout), #{ id => Mod, start => {Mod, start_link, Args}, type => Type, - %% long timeout for emqx_exhook_mgr - shutdown => 15000 + shutdown => Timeout }). %%-------------------------------------------------------------------- @@ -46,7 +49,7 @@ start_link() -> init([]) -> _ = emqx_exhook_metrics:init(), _ = emqx_exhook_mgr:init_ref_counter_table(), - Mngr = ?CHILD(emqx_exhook_mgr, worker, []), + Mngr = ?CHILD(emqx_exhook_mgr, worker, [], force_shutdown_timeout()), {ok, {{one_for_one, 10, 100}, [Mngr]}}. %%-------------------------------------------------------------------- @@ -71,3 +74,9 @@ stop_grpc_client_channel(Name) -> _:_:_ -> ok end. + +%% Calculate the maximum timeout, which will help to shutdown the +%% emqx_exhook_mgr process correctly. +force_shutdown_timeout() -> + Factor = max(3, length(emqx:get_config([exhook, servers])) + 1), + Factor * ?SERVER_FORCE_SHUTDOWN_TIMEOUT. diff --git a/apps/emqx_exhook/test/emqx_exhook_demo_svr.erl b/apps/emqx_exhook/test/emqx_exhook_demo_svr.erl index bf1a42c9a..b566b7ab2 100644 --- a/apps/emqx_exhook/test/emqx_exhook_demo_svr.erl +++ b/apps/emqx_exhook/test/emqx_exhook_demo_svr.erl @@ -81,8 +81,14 @@ stop() -> stop(Name) -> grpc:stop_server(Name), case whereis(to_atom_name(Name)) of - undefined -> ok; - Pid -> Pid ! stop + undefined -> + ok; + Pid -> + Ref = erlang:monitor(process, Pid), + Pid ! stop, + receive + {'DOWN', Ref, process, Pid, _Reason} -> ok + end end. take() -> diff --git a/apps/emqx_gateway/src/bhvrs/emqx_gateway_conn.erl b/apps/emqx_gateway/src/bhvrs/emqx_gateway_conn.erl index 24d63f02c..99ac3a38f 100644 --- a/apps/emqx_gateway/src/bhvrs/emqx_gateway_conn.erl +++ b/apps/emqx_gateway/src/bhvrs/emqx_gateway_conn.erl @@ -244,10 +244,10 @@ esockd_send(Data, #state{ esockd_send(Data, #state{socket = {esockd_transport, Sock}}) -> esockd_transport:async_send(Sock, Data). -keepalive_stats(recv_oct) -> - emqx_pd:get_counter(incoming_bytes); -keepalive_stats(send_oct) -> - emqx_pd:get_counter(outgoing_bytes). +keepalive_stats(recv) -> + emqx_pd:get_counter(recv_pkt); +keepalive_stats(send) -> + emqx_pd:get_counter(send_pkt). is_datadram_socket({esockd_transport, _}) -> false; is_datadram_socket({udp, _, _}) -> true. @@ -656,16 +656,16 @@ handle_timeout( Keepalive == keepalive; Keepalive == keepalive_send -> - Stat = + StatVal = case Keepalive of - keepalive -> recv_oct; - keepalive_send -> send_oct + keepalive -> keepalive_stats(recv); + keepalive_send -> keepalive_stats(send) end, case ChannMod:info(conn_state, Channel) of disconnected -> {ok, State}; _ -> - handle_timeout(TRef, {Keepalive, keepalive_stats(Stat)}, State) + handle_timeout(TRef, {Keepalive, StatVal}, State) end; handle_timeout( _TRef, diff --git a/apps/emqx_gateway/src/exproto/emqx_exproto_channel.erl b/apps/emqx_gateway/src/exproto/emqx_exproto_channel.erl index 94ca031aa..be4cddcaa 100644 --- a/apps/emqx_gateway/src/exproto/emqx_exproto_channel.erl +++ b/apps/emqx_gateway/src/exproto/emqx_exproto_channel.erl @@ -297,7 +297,7 @@ handle_timeout( {ok, reset_timer(alive_timer, NChannel)}; {error, timeout} -> Req = #{type => 'KEEPALIVE'}, - NChannel = clean_timer(alive_timer, Channel), + NChannel = remove_timer_ref(alive_timer, Channel), %% close connection if keepalive timeout Replies = [{event, disconnected}, {close, keepalive_timeout}], {ok, Replies, try_dispatch(on_timer_timeout, wrap(Req), NChannel)} @@ -665,7 +665,7 @@ ensure_keepalive(Channel = #channel{clientinfo = ClientInfo}) -> ensure_keepalive_timer(Interval, Channel) when Interval =< 0 -> Channel; ensure_keepalive_timer(Interval, Channel) -> - StatVal = emqx_gateway_conn:keepalive_stats(recv_oct), + StatVal = emqx_gateway_conn:keepalive_stats(recv), Keepalive = emqx_keepalive:init(StatVal, timer:seconds(Interval)), ensure_timer(alive_timer, Channel#channel{keepalive = Keepalive}). @@ -684,14 +684,14 @@ ensure_timer(Name, Time, Channel = #channel{timers = Timers}) -> Channel#channel{timers = Timers#{Name => TRef}}. reset_timer(Name, Channel) -> - ensure_timer(Name, clean_timer(Name, Channel)). - -clean_timer(Name, Channel = #channel{timers = Timers}) -> - Channel#channel{timers = maps:remove(Name, Timers)}. + ensure_timer(Name, remove_timer_ref(Name, Channel)). cancel_timer(Name, Channel = #channel{timers = Timers}) -> emqx_misc:cancel_timer(maps:get(Name, Timers, undefined)), - clean_timer(Name, Channel). + remove_timer_ref(Name, Channel). + +remove_timer_ref(Name, Channel = #channel{timers = Timers}) -> + Channel#channel{timers = maps:remove(Name, Timers)}. interval(idle_timer, #channel{conninfo = #{idle_timeout := IdleTimeout}}) -> IdleTimeout; @@ -746,10 +746,10 @@ enrich_clientinfo(InClientInfo = #{proto_name := ProtoName}, ClientInfo) -> NClientInfo = maps:merge(ClientInfo, maps:with(Ks, InClientInfo)), NClientInfo#{protocol => proto_name_to_protocol(ProtoName)}. -default_conninfo(ConnInfo = #{peername := {PeerHost, PeerPort}}) -> +default_conninfo(ConnInfo) -> ConnInfo#{ clean_start => true, - clientid => anonymous_clientid(PeerHost, PeerPort), + clientid => anonymous_clientid(), username => undefined, conn_props => #{}, connected => true, @@ -790,14 +790,5 @@ proto_name_to_protocol(<<>>) -> proto_name_to_protocol(ProtoName) when is_binary(ProtoName) -> binary_to_atom(ProtoName). -anonymous_clientid(PeerHost, PeerPort) -> - iolist_to_binary( - [ - "exproto-anonymous-", - inet:ntoa(PeerHost), - "-", - integer_to_list(PeerPort), - "-", - emqx_misc:gen_id() - ] - ). +anonymous_clientid() -> + iolist_to_binary(["exproto-", emqx_misc:gen_id()]). diff --git a/apps/emqx_gateway/src/exproto/emqx_exproto_gcli.erl b/apps/emqx_gateway/src/exproto/emqx_exproto_gcli.erl index 45b502798..cf8ed76a7 100644 --- a/apps/emqx_gateway/src/exproto/emqx_exproto_gcli.erl +++ b/apps/emqx_gateway/src/exproto/emqx_exproto_gcli.erl @@ -56,6 +56,7 @@ start_link(Pool, Id) -> [] ). +-spec async_call(atom(), map(), map()) -> ok. async_call( FunName, Req = #{conn := Conn}, @@ -63,17 +64,11 @@ async_call( ) -> case pick(PoolName, Conn) of false -> - ?SLOG( - error, - #{ - msg => "no_available_grpc_client", - function => FunName, - request => Req - } - ); + reply(self(), FunName, {error, no_available_grpc_client}); Pid when is_pid(Pid) -> cast(Pid, {rpc, FunName, Req, Options, self()}) - end. + end, + ok. %%-------------------------------------------------------------------- %% cast, pick From 56c1ac5a823179f664c92f3d83f94627571c67b1 Mon Sep 17 00:00:00 2001 From: Zhongwen Deng Date: Tue, 13 Sep 2022 18:38:36 +0800 Subject: [PATCH 20/34] chore: deprecated etc_dir since 5.0.8 --- apps/emqx_conf/src/emqx_conf_schema.erl | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/apps/emqx_conf/src/emqx_conf_schema.erl b/apps/emqx_conf/src/emqx_conf_schema.erl index a00bfe6f3..ec61cb508 100644 --- a/apps/emqx_conf/src/emqx_conf_schema.erl +++ b/apps/emqx_conf/src/emqx_conf_schema.erl @@ -536,6 +536,15 @@ fields("node") -> desc => ?DESC(node_applications) } )}, + {"etc_dir", + sc( + string(), + #{ + desc => ?DESC(node_etc_dir), + 'readOnly' => true, + deprecated => {since, "5.0.8"} + } + )}, {"cluster_call", sc( ?R_REF("cluster_call"), From b5bf5d3adb5ce407fafde80af7fd7a4bd82bb90a Mon Sep 17 00:00:00 2001 From: JimMoen Date: Tue, 13 Sep 2022 18:29:32 +0800 Subject: [PATCH 21/34] fix: mqtt bridge payload default value --- CHANGES-5.0.md | 1 + apps/emqx_connector/src/mqtt/emqx_connector_mqtt_schema.erl | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGES-5.0.md b/CHANGES-5.0.md index 6d80240de..42fdf862c 100644 --- a/CHANGES-5.0.md +++ b/CHANGES-5.0.md @@ -13,6 +13,7 @@ * Fix that redis authn will deny the unknown users [#8934](https://github.com/emqx/emqx/pull/8934) * Fix ExProto UDP client keepalive checking error. This causes the clients to not expire as long as a new UDP packet arrives [#8866](https://github.com/emqx/emqx/pull/8866) +* Fix that MQTT Bridge message payload could be empty string. [#8949](https://github.com/emqx/emqx/pull/8949) ## Enhancements diff --git a/apps/emqx_connector/src/mqtt/emqx_connector_mqtt_schema.erl b/apps/emqx_connector/src/mqtt/emqx_connector_mqtt_schema.erl index 438f33b5d..3e683f5fb 100644 --- a/apps/emqx_connector/src/mqtt/emqx_connector_mqtt_schema.erl +++ b/apps/emqx_connector/src/mqtt/emqx_connector_mqtt_schema.erl @@ -177,7 +177,7 @@ fields("ingress") -> sc( binary(), #{ - default => <<"${payload}">>, + default => undefined, desc => ?DESC("payload") } )} @@ -224,7 +224,7 @@ fields("egress") -> sc( binary(), #{ - default => <<"${payload}">>, + default => undefined, desc => ?DESC("payload") } )} From 17ccb77c7910e16a25872e6542334575fb8d09d0 Mon Sep 17 00:00:00 2001 From: Shawn <506895667@qq.com> Date: Tue, 13 Sep 2022 19:20:45 +0800 Subject: [PATCH 22/34] chore: update readme doc for building on apple m1/m2 --- README-CN.md | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/README-CN.md b/README-CN.md index 71767869d..250d3e1c6 100644 --- a/README-CN.md +++ b/README-CN.md @@ -112,6 +112,27 @@ make _build/emqx/rel/emqx/bin/emqx console ``` +### 在 Apple 芯片(M1,M2)上编译 + +基于 Apple 芯片的 Homebrew 将[默认的 home 目录](https://github.com/Homebrew/brew/issues/9177)从 `/usr/local` 改成了 `/opt/homebrew`,这个改变导致了一些兼容性问题。 + +具体到 EMQX 来说,主要影响的是 `unixodbc`,如果使用 Homebrew 安装的 `unixodbc` 包,那么在使用 [kerl](https://github.com/kerl/kerl) 编译 Erlang/OTP 的时候,kerl 会找不到 `unixodbc`。 + +解决此问题的方法如下: + +```bash +brew install unixodbc kerl +sudo ln -s $(realpath $(brew --prefix unixodbc)) /usr/local/odbc +export CC="/usr/bin/gcc -I$(brew --prefix unixodbc)/include" +export LDFLAGS="-L$(brew --prefix unixodbc)/lib" +kerl build 24.3 +mkdir ~/.kerl/installations +kerl install 24.3 ~/.kerl/installations/24.3 +. ~/.kerl/installations/24.3/activate +``` + +然后再使用 `make` 继续编译就可以了。 + ## 开源许可 详见 [LICENSE](./LICENSE)。 From 9e97760520e675fed1617780604670b87c293769 Mon Sep 17 00:00:00 2001 From: firest Date: Wed, 14 Sep 2022 11:23:31 +0800 Subject: [PATCH 23/34] fix(authn_redis): Add new clause for non-existent key check fix #8800 when the key not-existing, redis may return a list that all elements are `undefined` --- .../src/simple_authn/emqx_authn_redis.erl | 26 ++++++----- .../test/emqx_authn_redis_SUITE.erl | 45 +++++++++++++------ 2 files changed, 47 insertions(+), 24 deletions(-) diff --git a/apps/emqx_authn/src/simple_authn/emqx_authn_redis.erl b/apps/emqx_authn/src/simple_authn/emqx_authn_redis.erl index 4cc00322f..c6d2846ab 100644 --- a/apps/emqx_authn/src/simple_authn/emqx_authn_redis.erl +++ b/apps/emqx_authn/src/simple_authn/emqx_authn_redis.erl @@ -141,16 +141,22 @@ authenticate( {ok, []} -> ignore; {ok, Values} -> - Selected = merge(Fields, Values), - case - emqx_authn_utils:check_password_from_selected_map( - Algorithm, Selected, Password - ) - of - ok -> - {ok, emqx_authn_utils:is_superuser(Selected)}; - {error, _Reason} -> - ignore + case lists:all(fun(E) -> E =:= undefined end, Values) of + true -> + %% key not exists + ignore; + _ -> + Selected = merge(Fields, Values), + case + emqx_authn_utils:check_password_from_selected_map( + Algorithm, Selected, Password + ) + of + ok -> + {ok, emqx_authn_utils:is_superuser(Selected)}; + {error, _Reason} = Error -> + Error + end end; {error, Reason} -> ?TRACE_AUTHN_PROVIDER(error, "redis_query_failed", #{ diff --git a/apps/emqx_authn/test/emqx_authn_redis_SUITE.erl b/apps/emqx_authn/test/emqx_authn_redis_SUITE.erl index f9ed8bcb1..cd89a7fa6 100644 --- a/apps/emqx_authn/test/emqx_authn_redis_SUITE.erl +++ b/apps/emqx_authn/test/emqx_authn_redis_SUITE.erl @@ -161,11 +161,13 @@ t_authenticate(_Config) -> user_seeds() ). -test_user_auth(#{ - credentials := Credentials0, - config_params := SpecificConfigParams, - result := Result -}) -> +test_user_auth( + #{ + credentials := Credentials0, + config_params := SpecificConfigParams, + result := Result + } = Config +) -> AuthConfig = maps:merge(raw_redis_auth_config(), SpecificConfigParams), {ok, _} = emqx:update_config( @@ -183,14 +185,12 @@ test_user_auth(#{ ?assertEqual(Result, emqx_access_control:authenticate(Credentials)), - AuthnResult = - case Result of - {error, _} -> - ignore; - Any -> - Any - end, - ?assertEqual(AuthnResult, emqx_authn_redis:authenticate(Credentials, State)), + case maps:get(redis_result, Config, undefined) of + undefined -> + ok; + RedisResult -> + ?assertEqual(RedisResult, emqx_authn_redis:authenticate(Credentials, State)) + end, emqx_authn_test_lib:delete_authenticators( [authentication], @@ -478,7 +478,7 @@ user_seeds() -> <<"cmd">> => <<"HMGET mqtt_user:${username} password_hash salt is_superuser">>, <<"password_hash_algorithm">> => #{<<"name">> => <<"bcrypt">>} }, - result => {error, not_authorized} + result => {error, bad_username_or_password} }, #{ @@ -547,6 +547,23 @@ user_seeds() -> } }, result => {ok, #{is_superuser => true}} + }, + + %% user not exists + #{ + data => #{ + password_hash => <<"plainsalt">>, + salt => <<"salt">>, + is_superuser => <<"1">> + }, + credentials => #{ + username => <<"not_exists">>, + password => <<"plain">> + }, + key => <<"mqtt_user:plain">>, + config_params => #{}, + result => {error, not_authorized}, + redis_result => ignore } ]. From f018b8ab49a63c67977e8032701933b9466d130d Mon Sep 17 00:00:00 2001 From: JimMoen Date: Wed, 14 Sep 2022 14:08:23 +0800 Subject: [PATCH 24/34] fix(bridge/authz/exhook): no need to drop invalid certs --- .../emqx_authz/src/emqx_authz_api_sources.erl | 9 ++------ apps/emqx_bridge/src/emqx_bridge_api.erl | 3 +-- .../emqx_connector/src/emqx_connector_ssl.erl | 23 ------------------- apps/emqx_exhook/src/emqx_exhook_api.erl | 7 +----- 4 files changed, 4 insertions(+), 38 deletions(-) diff --git a/apps/emqx_authz/src/emqx_authz_api_sources.erl b/apps/emqx_authz/src/emqx_authz_api_sources.erl index cb4c6f631..9ff65f8a5 100644 --- a/apps/emqx_authz/src/emqx_authz_api_sources.erl +++ b/apps/emqx_authz/src/emqx_authz_api_sources.erl @@ -223,7 +223,7 @@ sources(get, _) -> ]) end; (Source, AccIn) -> - lists:append(AccIn, [drop_invalid_certs(Source)]) + lists:append(AccIn, [Source]) end, [], get_raw_sources() @@ -257,7 +257,7 @@ source(get, #{bindings := #{type := Type}}) -> }} end; [Source] -> - {200, drop_invalid_certs(Source)} + {200, Source} end; source(put, #{bindings := #{type := <<"file">>}, body := #{<<"type">> := <<"file">>} = Body}) -> update_authz_file(Body); @@ -511,11 +511,6 @@ update_config(Cmd, Sources) -> }} end. -drop_invalid_certs(#{<<"ssl">> := SSL} = Source) when SSL =/= undefined -> - Source#{<<"ssl">> => emqx_tls_lib:drop_invalid_certs(SSL)}; -drop_invalid_certs(Source) -> - Source. - parameters_field() -> [ {type, diff --git a/apps/emqx_bridge/src/emqx_bridge_api.erl b/apps/emqx_bridge/src/emqx_bridge_api.erl index e28df9176..37a42ab3d 100644 --- a/apps/emqx_bridge/src/emqx_bridge_api.erl +++ b/apps/emqx_bridge/src/emqx_bridge_api.erl @@ -584,10 +584,9 @@ pick_bridges_by_id(Type, Name, BridgesAllNodes) -> format_bridge_info([FirstBridge | _] = Bridges) -> Res = maps:remove(node, FirstBridge), - NRes = emqx_connector_ssl:drop_invalid_certs(Res), NodeStatus = collect_status(Bridges), NodeMetrics = collect_metrics(Bridges), - NRes#{ + Res#{ status => aggregate_status(NodeStatus), node_status => NodeStatus, metrics => aggregate_metrics(NodeMetrics), diff --git a/apps/emqx_connector/src/emqx_connector_ssl.erl b/apps/emqx_connector/src/emqx_connector_ssl.erl index 929ffe82e..7f2fc537b 100644 --- a/apps/emqx_connector/src/emqx_connector_ssl.erl +++ b/apps/emqx_connector/src/emqx_connector_ssl.erl @@ -18,7 +18,6 @@ -export([ convert_certs/2, - drop_invalid_certs/1, clear_certs/2 ]). @@ -61,28 +60,6 @@ clear_certs(RltvDir, #{ssl := OldSSL} = _Config) -> clear_certs(_RltvDir, _) -> ok. -drop_invalid_certs(#{<<"connector">> := Connector} = Config) when - is_map(Connector) --> - SSL = map_get_oneof([<<"ssl">>, ssl], Connector, undefined), - NewSSL = emqx_tls_lib:drop_invalid_certs(SSL), - new_ssl_config(Config, NewSSL); -drop_invalid_certs(#{connector := Connector} = Config) when - is_map(Connector) --> - SSL = map_get_oneof([<<"ssl">>, ssl], Connector, undefined), - NewSSL = emqx_tls_lib:drop_invalid_certs(SSL), - new_ssl_config(Config, NewSSL); -drop_invalid_certs(#{<<"ssl">> := SSL} = Config) -> - NewSSL = emqx_tls_lib:drop_invalid_certs(SSL), - new_ssl_config(Config, NewSSL); -drop_invalid_certs(#{ssl := SSL} = Config) -> - NewSSL = emqx_tls_lib:drop_invalid_certs(SSL), - new_ssl_config(Config, NewSSL); -%% for bridges use connector name -drop_invalid_certs(Config) -> - Config. - new_ssl_config(RltvDir, Config, SSL) -> case emqx_tls_lib:ensure_ssl_files(RltvDir, SSL) of {ok, NewSSL} -> diff --git a/apps/emqx_exhook/src/emqx_exhook_api.erl b/apps/emqx_exhook/src/emqx_exhook_api.erl index 83d6c4aa8..62f616903 100644 --- a/apps/emqx_exhook/src/emqx_exhook_api.erl +++ b/apps/emqx_exhook/src/emqx_exhook_api.erl @@ -483,16 +483,11 @@ err_msg(Msg) -> emqx_misc:readable_error_msg(Msg). get_raw_config() -> RawConfig = emqx:get_raw_config([exhook, servers], []), Schema = #{roots => emqx_exhook_schema:fields(exhook), fields => #{}}, - Conf = #{<<"servers">> => lists:map(fun drop_invalid_certs/1, RawConfig)}, + Conf = #{<<"servers">> => RawConfig}, Options = #{only_fill_defaults => true}, #{<<"servers">> := Servers} = hocon_tconf:check_plain(Schema, Conf, Options), Servers. -drop_invalid_certs(#{<<"ssl">> := SSL} = Conf) when SSL =/= undefined -> - Conf#{<<"ssl">> => emqx_tls_lib:drop_invalid_certs(SSL)}; -drop_invalid_certs(Conf) -> - Conf. - position_example() -> #{ front => From 62fd955a0ec4e384a63841aa0e4728caebf5b216 Mon Sep 17 00:00:00 2001 From: firest Date: Wed, 14 Sep 2022 14:16:43 +0800 Subject: [PATCH 25/34] fix(authn_redis): make dialyzer happy --- apps/emqx_authn/src/simple_authn/emqx_authn_redis.erl | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/apps/emqx_authn/src/simple_authn/emqx_authn_redis.erl b/apps/emqx_authn/src/simple_authn/emqx_authn_redis.erl index c6d2846ab..735cfe226 100644 --- a/apps/emqx_authn/src/simple_authn/emqx_authn_redis.erl +++ b/apps/emqx_authn/src/simple_authn/emqx_authn_redis.erl @@ -141,8 +141,8 @@ authenticate( {ok, []} -> ignore; {ok, Values} -> - case lists:all(fun(E) -> E =:= undefined end, Values) of - true -> + case check_query_matched(Values) of + false -> %% key not exists ignore; _ -> @@ -228,3 +228,10 @@ merge(Fields, Values) -> || {K, V} <- lists:zip(Fields, Values), V =/= undefined ] ). + +check_query_matched(undefined) -> + false; +check_query_matched(List) when is_list(List) -> + lists:any(fun(E) -> E =/= undefined end, List); +check_query_matched(_) -> + true. From a461375b30ea94650230dcef016eb6cb3e57c46c Mon Sep 17 00:00:00 2001 From: JianBo He Date: Wed, 14 Sep 2022 12:05:15 +0800 Subject: [PATCH 26/34] chore: support strip double quote in authn/authz more compatibility for https://github.com/emqx/emqx/pull/8827 --- apps/emqx_authn/src/emqx_authn_utils.erl | 3 +- .../test/emqx_authn_mysql_SUITE.erl | 26 +++++++++++++ .../test/emqx_authn_pgsql_SUITE.erl | 26 +++++++++++++ apps/emqx_authz/src/emqx_authz_utils.erl | 3 +- .../test/emqx_authz_mysql_SUITE.erl | 28 ++++++++++++++ .../test/emqx_authz_postgresql_SUITE.erl | 28 ++++++++++++++ .../emqx_plugin_libs/src/emqx_placeholder.erl | 38 ++++++++++++------- .../test/emqx_placeholder_SUITE.erl | 25 +++++++----- 8 files changed, 152 insertions(+), 25 deletions(-) diff --git a/apps/emqx_authn/src/emqx_authn_utils.erl b/apps/emqx_authn/src/emqx_authn_utils.erl index b989da3b4..6f963c6ca 100644 --- a/apps/emqx_authn/src/emqx_authn_utils.erl +++ b/apps/emqx_authn/src/emqx_authn_utils.erl @@ -112,7 +112,8 @@ parse_sql(Template, ReplaceWith) -> Template, #{ replace_with => ReplaceWith, - placeholders => ?AUTHN_PLACEHOLDERS + placeholders => ?AUTHN_PLACEHOLDERS, + strip_double_quote => true } ). diff --git a/apps/emqx_authn/test/emqx_authn_mysql_SUITE.erl b/apps/emqx_authn/test/emqx_authn_mysql_SUITE.erl index 175aa7f1d..74a40c455 100644 --- a/apps/emqx_authn/test/emqx_authn_mysql_SUITE.erl +++ b/apps/emqx_authn/test/emqx_authn_mysql_SUITE.erl @@ -332,6 +332,32 @@ user_seeds() -> result => {ok, #{is_superuser => true}} }, + %% strip double quote support + #{ + data => #{ + username => "sha256", + password_hash => "ac63a624e7074776d677dd61a003b8c803eb11db004d0ec6ae032a5d7c9c5caf", + salt => "salt", + is_superuser_int => 1 + }, + credentials => #{ + username => <<"sha256">>, + password => <<"sha256">> + }, + config_params => #{ + <<"query">> => + << + "SELECT password_hash, salt, is_superuser_int as is_superuser\n" + " FROM users where username = \"${username}\" LIMIT 1" + >>, + <<"password_hash_algorithm">> => #{ + <<"name">> => <<"sha256">>, + <<"salt_position">> => <<"prefix">> + } + }, + result => {ok, #{is_superuser => true}} + }, + #{ data => #{ username => "sha256", diff --git a/apps/emqx_authn/test/emqx_authn_pgsql_SUITE.erl b/apps/emqx_authn/test/emqx_authn_pgsql_SUITE.erl index 02095c07d..238aeaadf 100644 --- a/apps/emqx_authn/test/emqx_authn_pgsql_SUITE.erl +++ b/apps/emqx_authn/test/emqx_authn_pgsql_SUITE.erl @@ -394,6 +394,32 @@ user_seeds() -> result => {ok, #{is_superuser => true}} }, + %% strip double quote support + #{ + data => #{ + username => "sha256", + password_hash => "ac63a624e7074776d677dd61a003b8c803eb11db004d0ec6ae032a5d7c9c5caf", + salt => "salt", + is_superuser_int => 1 + }, + credentials => #{ + username => <<"sha256">>, + password => <<"sha256">> + }, + config_params => #{ + <<"query">> => + << + "SELECT password_hash, salt, is_superuser_int as is_superuser\n" + " FROM users where username = \"${username}\" LIMIT 1" + >>, + <<"password_hash_algorithm">> => #{ + <<"name">> => <<"sha256">>, + <<"salt_position">> => <<"prefix">> + } + }, + result => {ok, #{is_superuser => true}} + }, + #{ data => #{ username => "sha256", diff --git a/apps/emqx_authz/src/emqx_authz_utils.erl b/apps/emqx_authz/src/emqx_authz_utils.erl index d364bc5fa..f106177bd 100644 --- a/apps/emqx_authz/src/emqx_authz_utils.erl +++ b/apps/emqx_authz/src/emqx_authz_utils.erl @@ -110,7 +110,8 @@ parse_sql(Template, ReplaceWith, PlaceHolders) -> Template, #{ replace_with => ReplaceWith, - placeholders => PlaceHolders + placeholders => PlaceHolders, + strip_double_quote => true } ). diff --git a/apps/emqx_authz/test/emqx_authz_mysql_SUITE.erl b/apps/emqx_authz/test/emqx_authz_mysql_SUITE.erl index ce8d03984..930426318 100644 --- a/apps/emqx_authz/test/emqx_authz_mysql_SUITE.erl +++ b/apps/emqx_authz/test/emqx_authz_mysql_SUITE.erl @@ -202,6 +202,34 @@ t_lookups(_Config) -> } ), + ok = emqx_authz_test_lib:test_samples( + ClientInfo, + [ + {allow, subscribe, <<"a">>}, + {deny, subscribe, <<"b">>} + ] + ), + + %% strip double quote support + + ok = init_table(), + ok = q( + << + "INSERT INTO acl(clientid, topic, permission, action)" + "VALUES(?, ?, ?, ?)" + >>, + [<<"clientid">>, <<"a">>, <<"allow">>, <<"subscribe">>] + ), + + ok = setup_config( + #{ + <<"query">> => << + "SELECT permission, action, topic " + "FROM acl WHERE clientid = \"${clientid}\"" + >> + } + ), + ok = emqx_authz_test_lib:test_samples( ClientInfo, [ diff --git a/apps/emqx_authz/test/emqx_authz_postgresql_SUITE.erl b/apps/emqx_authz/test/emqx_authz_postgresql_SUITE.erl index d4aaf7077..fa1672ba7 100644 --- a/apps/emqx_authz/test/emqx_authz_postgresql_SUITE.erl +++ b/apps/emqx_authz/test/emqx_authz_postgresql_SUITE.erl @@ -202,6 +202,34 @@ t_lookups(_Config) -> } ), + ok = emqx_authz_test_lib:test_samples( + ClientInfo, + [ + {allow, subscribe, <<"a">>}, + {deny, subscribe, <<"b">>} + ] + ), + + %% strip double quote support + + ok = init_table(), + ok = insert( + << + "INSERT INTO acl(clientid, topic, permission, action)" + "VALUES($1, $2, $3, $4)" + >>, + [<<"clientid">>, <<"a">>, <<"allow">>, <<"subscribe">>] + ), + + ok = setup_config( + #{ + <<"query">> => << + "SELECT permission, action, topic " + "FROM acl WHERE clientid = \"${clientid}\"" + >> + } + ), + ok = emqx_authz_test_lib:test_samples( ClientInfo, [ diff --git a/apps/emqx_plugin_libs/src/emqx_placeholder.erl b/apps/emqx_plugin_libs/src/emqx_placeholder.erl index 70a1e41a5..dd99724e2 100644 --- a/apps/emqx_plugin_libs/src/emqx_placeholder.erl +++ b/apps/emqx_plugin_libs/src/emqx_placeholder.erl @@ -39,7 +39,10 @@ sql_data/1 ]). --define(EX_PLACE_HOLDER, "(\\$\\{[a-zA-Z0-9\\._]+\\}|\"\\$\\{[a-zA-Z0-9\\._]+\\}\")"). +-define(EX_PLACE_HOLDER, "(\\$\\{[a-zA-Z0-9\\._]+\\})"). + +-define(EX_PLACE_HOLDER_DOUBLE_QUOTE, "(\\$\\{[a-zA-Z0-9\\._]+\\}|\"\\$\\{[a-zA-Z0-9\\._]+\\}\")"). + %% Space and CRLF -define(EX_WITHE_CHARS, "\\s"). @@ -57,7 +60,8 @@ -type preproc_sql_opts() :: #{ placeholders => list(binary()), - replace_with => '?' | '$n' + replace_with => '?' | '$n', + strip_double_quote => boolean() }. -type preproc_deep_opts() :: #{ @@ -89,7 +93,7 @@ preproc_tmpl(Str) -> preproc_tmpl(Str, Opts) -> RE = preproc_var_re(Opts), Tokens = re:split(Str, RE, [{return, binary}, group, trim]), - do_preproc_tmpl(Tokens, []). + do_preproc_tmpl(Opts, Tokens, []). -spec proc_tmpl(tmpl_token(), map()) -> binary(). proc_tmpl(Tokens, Data) -> @@ -140,10 +144,11 @@ preproc_sql(Sql, ReplaceWith) when is_atom(ReplaceWith) -> preproc_sql(Sql, #{replace_with => ReplaceWith}); preproc_sql(Sql, Opts) -> RE = preproc_var_re(Opts), + Strip = maps:get(strip_double_quote, Opts, false), ReplaceWith = maps:get(replace_with, Opts, '?'), case re:run(Sql, RE, [{capture, all_but_first, binary}, global]) of {match, PlaceHolders} -> - PhKs = [parse_nested(unwrap(Phld)) || [Phld | _] <- PlaceHolders], + PhKs = [parse_nested(unwrap(Phld, Strip)) || [Phld | _] <- PlaceHolders], {replace_with(Sql, RE, ReplaceWith), [{var, Phld} || Phld <- PhKs]}; nomatch -> {Sql, []} @@ -234,29 +239,36 @@ get_phld_var(Fun, Data) when is_function(Fun) -> get_phld_var(Phld, Data) -> emqx_rule_maps:nested_get(Phld, Data). -preproc_var_re(#{placeholders := PHs}) -> +preproc_var_re(#{placeholders := PHs, strip_double_quote := true}) -> Res = [ph_to_re(PH) || PH <- PHs], QuoteRes = ["\"" ++ Re ++ "\"" || Re <- Res], "(" ++ string:join(Res ++ QuoteRes, "|") ++ ")"; +preproc_var_re(#{placeholders := PHs}) -> + "(" ++ string:join([ph_to_re(PH) || PH <- PHs], "|") ++ ")"; +preproc_var_re(#{strip_double_quote := true}) -> + ?EX_PLACE_HOLDER_DOUBLE_QUOTE; preproc_var_re(#{}) -> ?EX_PLACE_HOLDER. ph_to_re(VarPH) -> re:replace(VarPH, "[\\$\\{\\}]", "\\\\&", [global, {return, list}]). -do_preproc_tmpl([], Acc) -> +do_preproc_tmpl(_Opts, [], Acc) -> lists:reverse(Acc); -do_preproc_tmpl([[Str, Phld] | Tokens], Acc) -> +do_preproc_tmpl(Opts, [[Str, Phld] | Tokens], Acc) -> + Strip = maps:get(strip_double_quote, Opts, false), do_preproc_tmpl( + Opts, Tokens, put_head( var, - parse_nested(unwrap(Phld)), + parse_nested(unwrap(Phld, Strip)), put_head(str, Str, Acc) ) ); -do_preproc_tmpl([[Str] | Tokens], Acc) -> +do_preproc_tmpl(Opts, [[Str] | Tokens], Acc) -> do_preproc_tmpl( + Opts, Tokens, put_head(str, Str, Acc) ). @@ -293,10 +305,10 @@ parse_nested(Attr) -> Nested -> {path, [{key, P} || P <- Nested]} end. -unwrap(<<"${", Val/binary>>) -> - binary:part(Val, {0, byte_size(Val) - 1}); -unwrap(<<"\"${", Val/binary>>) -> - binary:part(Val, {0, byte_size(Val) - 2}). +unwrap(<<"\"${", Val/binary>>, _StripDoubleQuote = true) -> + binary:part(Val, {0, byte_size(Val) - 2}); +unwrap(<<"${", Val/binary>>, _StripDoubleQuote) -> + binary:part(Val, {0, byte_size(Val) - 1}). quote_sql(Str) -> quote(Str, <<"\\\\'">>). diff --git a/apps/emqx_plugin_libs/test/emqx_placeholder_SUITE.erl b/apps/emqx_plugin_libs/test/emqx_placeholder_SUITE.erl index ea642a9b0..bb83ce40c 100644 --- a/apps/emqx_plugin_libs/test/emqx_placeholder_SUITE.erl +++ b/apps/emqx_plugin_libs/test/emqx_placeholder_SUITE.erl @@ -150,20 +150,25 @@ t_preproc_sql6(_) -> emqx_placeholder:proc_sql(ParamsTokens, Selected) ). -t_preproc_sql7(_) -> +t_preproc_sql_strip_double_quote(_) -> Selected = #{a => <<"a">>, b => <<"b">>}, + Opts = #{replace_with => '$n', placeholders => [<<"${a}">>]}, + + %% no strip_double_quote option: "${key}" -> "value" {PrepareStatement, ParamsTokens} = emqx_placeholder:preproc_sql( <<"a:\"${a}\",b:\"${b}\"">>, - #{ - replace_with => '$n', - placeholders => [<<"${a}">>] - } + Opts ), - ?assertEqual(<<"a:$1,b:\"${b}\"">>, PrepareStatement), - ?assertEqual( - [<<"a">>], - emqx_placeholder:proc_sql(ParamsTokens, Selected) - ). + ?assertEqual(<<"a:\"$1\",b:\"${b}\"">>, PrepareStatement), + ?assertEqual([<<"a">>], emqx_placeholder:proc_sql(ParamsTokens, Selected)), + + %% strip_double_quote = true: "${key}" -> value + {PrepareStatement1, ParamsTokens1} = emqx_placeholder:preproc_sql( + <<"a:\"${a}\",b:\"${b}\"">>, + Opts#{strip_double_quote => true} + ), + ?assertEqual(<<"a:$1,b:\"${b}\"">>, PrepareStatement1), + ?assertEqual([<<"a">>], emqx_placeholder:proc_sql(ParamsTokens1, Selected)). t_preproc_tmpl_deep(_) -> Selected = #{a => <<"1">>, b => 1, c => 1.0, d => #{d1 => <<"hi">>}}, From 8590fef829a8ab611248d888c39d8dd1bc4ba713 Mon Sep 17 00:00:00 2001 From: firest Date: Wed, 14 Sep 2022 16:30:59 +0800 Subject: [PATCH 27/34] fix(authn_redis): Avoid duplicating check for non-existent keys --- .../src/simple_authn/emqx_authn_redis.erl | 25 ++++++++----------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/apps/emqx_authn/src/simple_authn/emqx_authn_redis.erl b/apps/emqx_authn/src/simple_authn/emqx_authn_redis.erl index 735cfe226..215b05637 100644 --- a/apps/emqx_authn/src/simple_authn/emqx_authn_redis.erl +++ b/apps/emqx_authn/src/simple_authn/emqx_authn_redis.erl @@ -141,12 +141,8 @@ authenticate( {ok, []} -> ignore; {ok, Values} -> - case check_query_matched(Values) of - false -> - %% key not exists - ignore; - _ -> - Selected = merge(Fields, Values), + case merge(Fields, Values) of + Selected when Selected =/= #{} -> case emqx_authn_utils:check_password_from_selected_map( Algorithm, Selected, Password @@ -156,7 +152,15 @@ authenticate( {ok, emqx_authn_utils:is_superuser(Selected)}; {error, _Reason} = Error -> Error - end + end; + _ -> + ?TRACE_AUTHN_PROVIDER(info, "redis_query_not_matched", #{ + resource => ResourceId, + cmd => Command, + keys => NKey, + fields => Fields + }), + ignore end; {error, Reason} -> ?TRACE_AUTHN_PROVIDER(error, "redis_query_failed", #{ @@ -228,10 +232,3 @@ merge(Fields, Values) -> || {K, V} <- lists:zip(Fields, Values), V =/= undefined ] ). - -check_query_matched(undefined) -> - false; -check_query_matched(List) when is_list(List) -> - lists:any(fun(E) -> E =/= undefined end, List); -check_query_matched(_) -> - true. From 8054144357697d516cfbf699b72a9af0181dbb53 Mon Sep 17 00:00:00 2001 From: zhouzb Date: Thu, 15 Sep 2022 10:20:25 +0800 Subject: [PATCH 28/34] chore: bump vsn --- apps/emqx/include/emqx_release.hrl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/emqx/include/emqx_release.hrl b/apps/emqx/include/emqx_release.hrl index bc14ef209..511e011cf 100644 --- a/apps/emqx/include/emqx_release.hrl +++ b/apps/emqx/include/emqx_release.hrl @@ -32,7 +32,7 @@ %% `apps/emqx/src/bpapi/README.md' %% Community edition --define(EMQX_RELEASE_CE, "5.0.7"). +-define(EMQX_RELEASE_CE, "5.0.8-alpha.1"). %% Enterprise edition -define(EMQX_RELEASE_EE, "5.0.0-alpha.1"). From a660004fa4c0bb002a42e97abee6acc8ee4e34b4 Mon Sep 17 00:00:00 2001 From: JimMoen Date: Fri, 16 Sep 2022 15:02:03 +0800 Subject: [PATCH 29/34] fix(bridge): mqtt bridge client info `username` & `password` for non configured `username` and `password`, default values `<<>>` should be used. emqtt needs the values. --- .../src/emqx_connector_mqtt.erl | 33 ++++++++++--------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/apps/emqx_connector/src/emqx_connector_mqtt.erl b/apps/emqx_connector/src/emqx_connector_mqtt.erl index 21e201504..d21d373a0 100644 --- a/apps/emqx_connector/src/emqx_connector_mqtt.erl +++ b/apps/emqx_connector/src/emqx_connector_mqtt.erl @@ -222,20 +222,20 @@ make_forward_confs(undefined) -> make_forward_confs(FrowardConf) -> FrowardConf. -basic_config(#{ - server := Server, - reconnect_interval := ReconnIntv, - proto_ver := ProtoVer, - bridge_mode := BridgeMode, - username := User, - password := Password, - clean_start := CleanStart, - keepalive := KeepAlive, - retry_interval := RetryIntv, - max_inflight := MaxInflight, - replayq := ReplayQ, - ssl := #{enable := EnableSsl} = Ssl -}) -> +basic_config( + #{ + server := Server, + reconnect_interval := ReconnIntv, + proto_ver := ProtoVer, + bridge_mode := BridgeMode, + clean_start := CleanStart, + keepalive := KeepAlive, + retry_interval := RetryIntv, + max_inflight := MaxInflight, + replayq := ReplayQ, + ssl := #{enable := EnableSsl} = Ssl + } = Conf +) -> #{ replayq => ReplayQ, %% connection opts @@ -251,8 +251,9 @@ basic_config(#{ %% non-standard mqtt connection packets will be filtered out by LB. %% So let's disable bridge_mode. bridge_mode => BridgeMode, - username => User, - password => Password, + %% should be iolist for emqtt + username => maps:get(username, Conf, <<>>), + password => maps:get(password, Conf, <<>>), clean_start => CleanStart, keepalive => ms_to_s(KeepAlive), retry_interval => RetryIntv, From a9e6e1b6c8707917a8e3192edb9b708b7badde94 Mon Sep 17 00:00:00 2001 From: zhouzb Date: Fri, 16 Sep 2022 19:18:56 +0800 Subject: [PATCH 30/34] chore: bump dsahboard vsn --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index ead7847f8..74204afac 100644 --- a/Makefile +++ b/Makefile @@ -6,7 +6,7 @@ export EMQX_DEFAULT_BUILDER = ghcr.io/emqx/emqx-builder/5.0-17:1.13.4-24.2.1-1-d export EMQX_DEFAULT_RUNNER = debian:11-slim export OTP_VSN ?= $(shell $(CURDIR)/scripts/get-otp-vsn.sh) export ELIXIR_VSN ?= $(shell $(CURDIR)/scripts/get-elixir-vsn.sh) -export EMQX_DASHBOARD_VERSION ?= v1.0.8 +export EMQX_DASHBOARD_VERSION ?= v1.0.9 export EMQX_EE_DASHBOARD_VERSION ?= e1.0.0 export EMQX_REL_FORM ?= tgz export QUICER_DOWNLOAD_FROM_RELEASE = 1 From ba37ab4938ba7ec49803f26c63b97d0fac71da27 Mon Sep 17 00:00:00 2001 From: zhouzb Date: Fri, 16 Sep 2022 19:32:17 +0800 Subject: [PATCH 31/34] chore: bump vsn to 5.0.8 --- apps/emqx/include/emqx_release.hrl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/emqx/include/emqx_release.hrl b/apps/emqx/include/emqx_release.hrl index 511e011cf..ad312e05d 100644 --- a/apps/emqx/include/emqx_release.hrl +++ b/apps/emqx/include/emqx_release.hrl @@ -32,7 +32,7 @@ %% `apps/emqx/src/bpapi/README.md' %% Community edition --define(EMQX_RELEASE_CE, "5.0.8-alpha.1"). +-define(EMQX_RELEASE_CE, "5.0.8"). %% Enterprise edition -define(EMQX_RELEASE_EE, "5.0.0-alpha.1"). From f0db2c0f121b7ab0b82a9b73fd4cc98719d902cc Mon Sep 17 00:00:00 2001 From: zhouzb Date: Sat, 17 Sep 2022 21:56:47 +0800 Subject: [PATCH 32/34] chore: bump vsn for helm chart --- deploy/charts/emqx/Chart.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/deploy/charts/emqx/Chart.yaml b/deploy/charts/emqx/Chart.yaml index 544d848fd..fa265e663 100644 --- a/deploy/charts/emqx/Chart.yaml +++ b/deploy/charts/emqx/Chart.yaml @@ -14,8 +14,8 @@ type: application # This is the chart version. This version number should be incremented each time you make changes # to the chart and its templates, including the app version. -version: 5.0.7 +version: 5.0.8 # This is the version number of the application being deployed. This version number should be # incremented each time you make changes to the application. -appVersion: 5.0.7 +appVersion: 5.0.8 From fc3fdfb994d4450a70c85964c616df7c7df2a33c Mon Sep 17 00:00:00 2001 From: JimMoen Date: Fri, 23 Sep 2022 16:28:35 +0800 Subject: [PATCH 33/34] fix(build): apply PR#8956 changes --- .../src/simple_authn/emqx_authn_redis.erl | 66 +++++++++---------- .../test/emqx_authn_redis_SUITE.erl | 2 +- 2 files changed, 32 insertions(+), 36 deletions(-) diff --git a/apps/emqx_authn/src/simple_authn/emqx_authn_redis.erl b/apps/emqx_authn/src/simple_authn/emqx_authn_redis.erl index 215b05637..69d01f7e8 100644 --- a/apps/emqx_authn/src/simple_authn/emqx_authn_redis.erl +++ b/apps/emqx_authn/src/simple_authn/emqx_authn_redis.erl @@ -133,47 +133,43 @@ authenticate( password_hash_algorithm := Algorithm } ) -> - ?WITH_SUCCESSFUL_RENDER( - begin - NKey = emqx_authn_utils:render_str(KeyTemplate, Credential), - Command = [CommandName, NKey | Fields], - case emqx_resource:query(ResourceId, {cmd, Command}) of - {ok, []} -> - ignore; - {ok, Values} -> - case merge(Fields, Values) of - Selected when Selected =/= #{} -> - case - emqx_authn_utils:check_password_from_selected_map( - Algorithm, Selected, Password - ) - of - ok -> - {ok, emqx_authn_utils:is_superuser(Selected)}; - {error, _Reason} = Error -> - Error - end; - _ -> - ?TRACE_AUTHN_PROVIDER(info, "redis_query_not_matched", #{ - resource => ResourceId, - cmd => Command, - keys => NKey, - fields => Fields - }), - ignore + NKey = emqx_authn_utils:render_str(KeyTemplate, Credential), + Command = [CommandName, NKey | Fields], + case emqx_resource:query(ResourceId, {cmd, Command}) of + {ok, []} -> + ignore; + {ok, Values} -> + case merge(Fields, Values) of + Selected when Selected =/= #{} -> + case + emqx_authn_utils:check_password_from_selected_map( + Algorithm, Selected, Password + ) + of + ok -> + {ok, emqx_authn_utils:is_superuser(Selected)}; + {error, _Reason} = Error -> + Error end; - {error, Reason} -> - ?TRACE_AUTHN_PROVIDER(error, "redis_query_failed", #{ + _ -> + ?TRACE_AUTHN_PROVIDER(info, "redis_query_not_matched", #{ resource => ResourceId, cmd => Command, keys => NKey, - fields => Fields, - reason => Reason + fields => Fields }), ignore - end - end - ). + end; + {error, Reason} -> + ?TRACE_AUTHN_PROVIDER(error, "redis_query_failed", #{ + resource => ResourceId, + cmd => Command, + keys => NKey, + fields => Fields, + reason => Reason + }), + ignore + end. %%------------------------------------------------------------------------------ %% Internal functions diff --git a/apps/emqx_authn/test/emqx_authn_redis_SUITE.erl b/apps/emqx_authn/test/emqx_authn_redis_SUITE.erl index 1261d9116..cbf094549 100644 --- a/apps/emqx_authn/test/emqx_authn_redis_SUITE.erl +++ b/apps/emqx_authn/test/emqx_authn_redis_SUITE.erl @@ -464,7 +464,7 @@ user_seeds() -> <<"cmd">> => <<"HMGET mqtt_user:${username} password_hash salt is_superuser">>, <<"password_hash_algorithm">> => #{<<"name">> => <<"bcrypt">>} }, - result => {error, not_authorized} + result => {error, bad_username_or_password} }, #{ From 2984397e73bf471e6e0ac3ea4db0265e6e25c2b2 Mon Sep 17 00:00:00 2001 From: JimMoen Date: Fri, 23 Sep 2022 16:34:48 +0800 Subject: [PATCH 34/34] chore: apps vsn bump --- apps/emqx/src/emqx.app.src | 2 +- apps/emqx_authn/src/emqx_authn.app.src | 2 +- apps/emqx_authz/src/emqx_authz.app.src | 2 +- apps/emqx_conf/src/emqx_conf.app.src | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/apps/emqx/src/emqx.app.src b/apps/emqx/src/emqx.app.src index edf632740..cc1e7eaea 100644 --- a/apps/emqx/src/emqx.app.src +++ b/apps/emqx/src/emqx.app.src @@ -3,7 +3,7 @@ {id, "emqx"}, {description, "EMQX Core"}, % strict semver, bump manually! - {vsn, "5.0.8"}, + {vsn, "5.0.9"}, {modules, []}, {registered, []}, {applications, [ diff --git a/apps/emqx_authn/src/emqx_authn.app.src b/apps/emqx_authn/src/emqx_authn.app.src index c4c59bd4a..bdb70cd4e 100644 --- a/apps/emqx_authn/src/emqx_authn.app.src +++ b/apps/emqx_authn/src/emqx_authn.app.src @@ -1,7 +1,7 @@ %% -*- mode: erlang -*- {application, emqx_authn, [ {description, "EMQX Authentication"}, - {vsn, "0.1.6"}, + {vsn, "0.1.7"}, {modules, []}, {registered, [emqx_authn_sup, emqx_authn_registry]}, {applications, [kernel, stdlib, emqx_resource, ehttpc, epgsql, mysql, jose]}, diff --git a/apps/emqx_authz/src/emqx_authz.app.src b/apps/emqx_authz/src/emqx_authz.app.src index 4a2186066..3dd2705d1 100644 --- a/apps/emqx_authz/src/emqx_authz.app.src +++ b/apps/emqx_authz/src/emqx_authz.app.src @@ -1,7 +1,7 @@ %% -*- mode: erlang -*- {application, emqx_authz, [ {description, "An OTP application"}, - {vsn, "0.1.5"}, + {vsn, "0.1.6"}, {registered, []}, {mod, {emqx_authz_app, []}}, {applications, [ diff --git a/apps/emqx_conf/src/emqx_conf.app.src b/apps/emqx_conf/src/emqx_conf.app.src index a8707025a..c0d5e1f44 100644 --- a/apps/emqx_conf/src/emqx_conf.app.src +++ b/apps/emqx_conf/src/emqx_conf.app.src @@ -1,6 +1,6 @@ {application, emqx_conf, [ {description, "EMQX configuration management"}, - {vsn, "0.1.4"}, + {vsn, "0.1.5"}, {registered, []}, {mod, {emqx_conf_app, []}}, {applications, [kernel, stdlib]},