From 6706fd90e1f1a2214a863f076a8fc37bd1fe83dd Mon Sep 17 00:00:00 2001 From: firest Date: Wed, 26 Apr 2023 16:10:35 +0800 Subject: [PATCH 01/41] fix(rocketmq): keep sensitive data safe in rocketmq logs and state --- .../src/emqx_ee_connector_rocketmq.erl | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/lib-ee/emqx_ee_connector/src/emqx_ee_connector_rocketmq.erl b/lib-ee/emqx_ee_connector/src/emqx_ee_connector_rocketmq.erl index 70a27ef6e..73f89491b 100644 --- a/lib-ee/emqx_ee_connector/src/emqx_ee_connector_rocketmq.erl +++ b/lib-ee/emqx_ee_connector/src/emqx_ee_connector_rocketmq.erl @@ -112,18 +112,19 @@ on_start( sync_timeout => SyncTimeout, templates => Templates, producers_map_pid => ProducersMapPID, - producers_opts => ProducerOpts + producers_opts => emqx_secret:wrap(ProducerOpts) }, case rocketmq:ensure_supervised_client(ClientId, Servers, ClientCfg) of {ok, _Pid} -> {ok, State}; - {error, _Reason} = Error -> + {error, Reason0} -> + Reason = redact(Reason0), ?tp( rocketmq_connector_start_failed, - #{error => _Reason} + #{error => Reason} ), - Error + {error, Reason} end. on_stop(InstanceId, #{client_id := ClientId, topic := RawTopic, producers_map_pid := Pid} = _State) -> @@ -220,7 +221,7 @@ safe_do_produce(InstanceId, QueryFunc, ClientId, TopicKey, Data, ProducerOpts, R produce(InstanceId, QueryFunc, Producers, Data, RequestTimeout) catch _Type:Reason -> - {error, {unrecoverable_error, Reason}} + {error, {unrecoverable_error, redact(Reason)}} end. produce(_InstanceId, QueryFunc, Producers, Data, RequestTimeout) -> @@ -335,7 +336,7 @@ get_producers(ClientId, {_, Topic1} = TopicKey, ProducerOpts) -> _ -> ProducerGroup = iolist_to_binary([atom_to_list(ClientId), "_", Topic1]), {ok, Producers0} = rocketmq:ensure_supervised_producers( - ClientId, ProducerGroup, Topic1, ProducerOpts + ClientId, ProducerGroup, Topic1, emqx_secret:unwrap(ProducerOpts) ), ets:insert(ClientId, {TopicKey, Producers0}), Producers0 From 1c4f4037a5374e7d8416b51447f2a23032d11066 Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Wed, 26 Apr 2023 13:41:42 +0200 Subject: [PATCH 02/41] test(ct/run.sh): remove the trailing / in app name --- scripts/ct/run.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/ct/run.sh b/scripts/ct/run.sh index a85aa36af..f8ed3dff0 100755 --- a/scripts/ct/run.sh +++ b/scripts/ct/run.sh @@ -47,7 +47,7 @@ while [ "$#" -gt 0 ]; do exit 0 ;; --app) - WHICH_APP="$2" + WHICH_APP="${2%/}" shift 2 ;; --only-up) From c83d630c97763cecc6945f4eae99d61b88cecf6a Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Wed, 26 Apr 2023 14:30:08 +0200 Subject: [PATCH 03/41] fix(cassandra): ensure async calls return connection pid so the buffer worker can monitor it and perform retries if the connection restarted --- .../test/emqx_ee_bridge_cassa_SUITE.erl | 6 ++--- .../src/emqx_ee_connector_cassa.erl | 23 +++++++++++++++---- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/lib-ee/emqx_ee_bridge/test/emqx_ee_bridge_cassa_SUITE.erl b/lib-ee/emqx_ee_bridge/test/emqx_ee_bridge_cassa_SUITE.erl index 3e442a926..2e3510aed 100644 --- a/lib-ee/emqx_ee_bridge/test/emqx_ee_bridge_cassa_SUITE.erl +++ b/lib-ee/emqx_ee_bridge/test/emqx_ee_bridge_cassa_SUITE.erl @@ -404,7 +404,7 @@ t_setup_via_config_and_publish(Config) -> end, fun(Trace0) -> Trace = ?of_kind(cassandra_connector_query_return, Trace0), - ?assertMatch([#{result := ok}], Trace), + ?assertMatch([#{result := {ok, _Pid}}], Trace), ok end ), @@ -443,7 +443,7 @@ t_setup_via_http_api_and_publish(Config) -> end, fun(Trace0) -> Trace = ?of_kind(cassandra_connector_query_return, Trace0), - ?assertMatch([#{result := ok}], Trace), + ?assertMatch([#{result := {ok, _Pid}}], Trace), ok end ), @@ -603,7 +603,7 @@ t_missing_data(Config) -> fun(Trace0) -> %% 1. ecql driver will return `ok` first in async query Trace = ?of_kind(cassandra_connector_query_return, Trace0), - ?assertMatch([#{result := ok}], Trace), + ?assertMatch([#{result := {ok, _Pid}}], Trace), %% 2. then it will return an error in callback function Trace1 = ?of_kind(handle_async_reply, Trace0), ?assertMatch([#{result := {error, {8704, _}}}], Trace1), diff --git a/lib-ee/emqx_ee_connector/src/emqx_ee_connector_cassa.erl b/lib-ee/emqx_ee_connector/src/emqx_ee_connector_cassa.erl index 86b908038..c4f3e9b87 100644 --- a/lib-ee/emqx_ee_connector/src/emqx_ee_connector_cassa.erl +++ b/lib-ee/emqx_ee_connector/src/emqx_ee_connector_cassa.erl @@ -278,7 +278,7 @@ proc_cql_params(query, SQL, Params, _State) -> exec_cql_query(InstId, PoolName, Type, Async, PreparedKey, Data) when Type == query; Type == prepared_query -> - case ecpool:pick_and_do(PoolName, {?MODULE, Type, [Async, PreparedKey, Data]}, no_handover) of + case exec(PoolName, {?MODULE, Type, [Async, PreparedKey, Data]}) of {error, Reason} = Result -> ?tp( error, @@ -292,7 +292,7 @@ exec_cql_query(InstId, PoolName, Type, Async, PreparedKey, Data) when end. exec_cql_batch_query(InstId, PoolName, Async, CQLs) -> - case ecpool:pick_and_do(PoolName, {?MODULE, batch_query, [Async, CQLs]}, no_handover) of + case exec(PoolName, {?MODULE, batch_query, [Async, CQLs]}) of {error, Reason} = Result -> ?tp( error, @@ -305,6 +305,13 @@ exec_cql_batch_query(InstId, PoolName, Async, CQLs) -> Result end. +%% Pick one of the pool members to do the query. +%% Using 'no_handoever' strategy, +%% meaning the buffer worker does the gen_server call or gen_server cast +%% towards the connection process. +exec(PoolName, Query) -> + ecpool:pick_and_do(PoolName, Query, no_handover). + on_get_status(_InstId, #{poolname := Pool} = State) -> case emqx_plugin_libs_pool:health_check_ecpool_workers(Pool, fun ?MODULE:do_get_status/1) of true -> @@ -343,17 +350,23 @@ do_check_prepares(State = #{poolname := PoolName, prepare_cql := {error, Prepare query(Conn, sync, CQL, Params) -> ecql:query(Conn, CQL, Params); query(Conn, {async, Callback}, CQL, Params) -> - ecql:async_query(Conn, CQL, Params, one, Callback). + ok = ecql:async_query(Conn, CQL, Params, one, Callback), + %% return the connection pid for buffer worker to monitor + {ok, Conn}. prepared_query(Conn, sync, PreparedKey, Params) -> ecql:execute(Conn, PreparedKey, Params); prepared_query(Conn, {async, Callback}, PreparedKey, Params) -> - ecql:async_execute(Conn, PreparedKey, Params, Callback). + ok = ecql:async_execute(Conn, PreparedKey, Params, Callback), + %% return the connection pid for buffer worker to monitor + {ok, Conn}. batch_query(Conn, sync, Rows) -> ecql:batch(Conn, Rows); batch_query(Conn, {async, Callback}, Rows) -> - ecql:async_batch(Conn, Rows, Callback). + ok = ecql:async_batch(Conn, Rows, Callback), + %% return the connection pid for buffer worker to monitor + {ok, Conn}. %%-------------------------------------------------------------------- %% callbacks for ecpool From c53741a08c09d3b43b58a6d7635cebf2bcb30697 Mon Sep 17 00:00:00 2001 From: Thales Macedo Garitezi Date: Wed, 19 Apr 2023 17:42:27 -0300 Subject: [PATCH 04/41] fix(buffer_worker): avoid sending late reply messages to callers Fixes https://emqx.atlassian.net/browse/EMQX-9635 During a sync call from process `A` to a buffer worker `B`, its call to the underlying resource `C` can be very slow. In those cases, `A` will receive a timeout response and expect no more messages from `B` nor `C`. However, prior to this fix, if `B` is stuck in a long sync call to `C` and then gets its response after `A` timed out, `B` would still send the late response to `A`, polluting its mailbox. --- .../src/emqx_resource_buffer_worker.erl | 17 +++++-- .../test/emqx_connector_demo.erl | 6 ++- .../test/emqx_resource_SUITE.erl | 45 +++++++++++++++++++ changes/ce/fix-10455.en.md | 9 ++++ 4 files changed, 72 insertions(+), 5 deletions(-) create mode 100644 changes/ce/fix-10455.en.md diff --git a/apps/emqx_resource/src/emqx_resource_buffer_worker.erl b/apps/emqx_resource/src/emqx_resource_buffer_worker.erl index e34cf5d0a..2e2cd5631 100644 --- a/apps/emqx_resource/src/emqx_resource_buffer_worker.erl +++ b/apps/emqx_resource/src/emqx_resource_buffer_worker.erl @@ -52,7 +52,7 @@ -export([queue_item_marshaller/1, estimate_size/1]). --export([handle_async_reply/2, handle_async_batch_reply/2]). +-export([handle_async_reply/2, handle_async_batch_reply/2, reply_call/2]). -export([clear_disk_queue_dir/2]). @@ -293,10 +293,8 @@ code_change(_OldVsn, State, _Extra) -> pick_call(Id, Key, Query, Timeout) -> ?PICK(Id, Key, Pid, begin - Caller = self(), MRef = erlang:monitor(process, Pid, [{alias, reply_demonitor}]), - From = {Caller, MRef}, - ReplyTo = {fun gen_statem:reply/2, [From]}, + ReplyTo = {fun ?MODULE:reply_call/2, [MRef]}, erlang:send(Pid, ?SEND_REQ(ReplyTo, Query)), receive {MRef, Response} -> @@ -1703,6 +1701,17 @@ default_resume_interval(_RequestTimeout = infinity, HealthCheckInterval) -> default_resume_interval(RequestTimeout, HealthCheckInterval) -> max(1, min(HealthCheckInterval, RequestTimeout div 3)). +-spec reply_call(reference(), term()) -> ok. +reply_call(Alias, Response) -> + %% Since we use a reference created with `{alias, + %% reply_demonitor}', after we `demonitor' it in case of a + %% timeout, we won't send any more messages that the caller is not + %% expecting anymore. Using `gen_statem:reply({pid(), + %% reference()}, _)' would still send a late reply even after the + %% demonitor. + erlang:send(Alias, {Alias, Response}), + ok. + -ifdef(TEST). -include_lib("eunit/include/eunit.hrl"). adjust_batch_time_test_() -> diff --git a/apps/emqx_resource/test/emqx_connector_demo.erl b/apps/emqx_resource/test/emqx_connector_demo.erl index a1393c574..5be854e93 100644 --- a/apps/emqx_resource/test/emqx_connector_demo.erl +++ b/apps/emqx_resource/test/emqx_connector_demo.erl @@ -144,7 +144,11 @@ on_query(_InstId, {sleep_before_reply, For}, #{pid := Pid}) -> Result after 1000 -> {error, timeout} - end. + end; +on_query(_InstId, {sync_sleep_before_reply, SleepFor}, _State) -> + %% This simulates a slow sync call + timer:sleep(SleepFor), + {ok, slept}. on_query_async(_InstId, block, ReplyFun, #{pid := Pid}) -> Pid ! {block, ReplyFun}, diff --git a/apps/emqx_resource/test/emqx_resource_SUITE.erl b/apps/emqx_resource/test/emqx_resource_SUITE.erl index 385b4cb91..e098c2e1c 100644 --- a/apps/emqx_resource/test/emqx_resource_SUITE.erl +++ b/apps/emqx_resource/test/emqx_resource_SUITE.erl @@ -2751,6 +2751,51 @@ t_volatile_offload_mode(_Config) -> end ). +t_late_call_reply(_Config) -> + emqx_connector_demo:set_callback_mode(always_sync), + RequestTimeout = 500, + ?assertMatch( + {ok, _}, + emqx_resource:create( + ?ID, + ?DEFAULT_RESOURCE_GROUP, + ?TEST_RESOURCE, + #{name => test_resource}, + #{ + buffer_mode => memory_only, + request_timeout => RequestTimeout, + query_mode => sync + } + ) + ), + ?check_trace( + begin + %% Sleep for longer than the request timeout; the call reply will + %% have been already returned (a timeout), but the resource will + %% still send a message with the reply. + %% The demo connector will reply with `{error, timeout}' after 1 s. + SleepFor = RequestTimeout + 500, + ?assertMatch( + {error, {resource_error, #{reason := timeout}}}, + emqx_resource:query( + ?ID, + {sync_sleep_before_reply, SleepFor}, + #{timeout => RequestTimeout} + ) + ), + %% Our process shouldn't receive any late messages. + receive + LateReply -> + ct:fail("received late reply: ~p", [LateReply]) + after SleepFor -> + ok + end, + ok + end, + [] + ), + ok. + %%------------------------------------------------------------------------------ %% Helpers %%------------------------------------------------------------------------------ diff --git a/changes/ce/fix-10455.en.md b/changes/ce/fix-10455.en.md new file mode 100644 index 000000000..07d8c71db --- /dev/null +++ b/changes/ce/fix-10455.en.md @@ -0,0 +1,9 @@ +Fixed an issue that could cause (otherwise harmless) noise in the logs. + +During some particularly slow synchronous calls to bridges, some late replies could be sent to connections processes that were no longer expecting a reply, and then emit an error log like: + +``` +2023-04-19T18:24:35.350233+00:00 [error] msg: unexpected_info, mfa: emqx_channel:handle_info/2, line: 1278, peername: 172.22.0.1:36384, clientid: caribdis_bench_sub_1137967633_4788, info: {#Ref<0.408802983.1941504010.189402>,{ok,200,[{<<"cache-control">>,<<"max-age=0, ...">>}} +``` + +Those logs are harmless, but they could flood and worry the users without need. From 7967090de0f71c5920f0e1fa3954403d65d00ff0 Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Wed, 26 Apr 2023 20:09:33 +0200 Subject: [PATCH 05/41] chore(emqx_dashboard): ignore everything in priv dir --- .gitignore | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index 62e8ddc81..6c4de9272 100644 --- a/.gitignore +++ b/.gitignore @@ -43,8 +43,7 @@ tmp/ _packages elvis emqx_dialyzer_*_plt -*/emqx_dashboard/priv/www -*/emqx_dashboard/priv/i18n.conf +*/emqx_dashboard/priv/ dist.zip scripts/git-token apps/*/etc/*.all From a8b000f06291a80e999b0c16b5e8adf0e0acaf7f Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Wed, 26 Apr 2023 20:44:56 +0200 Subject: [PATCH 06/41] refactor(sqlserver): support only sync mode at connector level --- .../src/emqx_ee_connector_sqlserver.erl | 52 ++----------------- 1 file changed, 4 insertions(+), 48 deletions(-) diff --git a/lib-ee/emqx_ee_connector/src/emqx_ee_connector_sqlserver.erl b/lib-ee/emqx_ee_connector/src/emqx_ee_connector_sqlserver.erl index 6cbd9de4e..97a46152d 100644 --- a/lib-ee/emqx_ee_connector/src/emqx_ee_connector_sqlserver.erl +++ b/lib-ee/emqx_ee_connector/src/emqx_ee_connector_sqlserver.erl @@ -34,8 +34,6 @@ on_stop/2, on_query/3, on_batch_query/3, - on_query_async/4, - on_batch_query_async/4, on_get_status/2 ]). @@ -43,7 +41,7 @@ -export([connect/1]). %% Internal exports used to execute code with ecpool worker --export([do_get_status/1, worker_do_insert/3, do_async_reply/2]). +-export([do_get_status/1, worker_do_insert/3]). -import(emqx_plugin_libs_rule, [str/1]). -import(hoconsc, [mk/2, enum/1, ref/2]). @@ -51,7 +49,6 @@ -define(ACTION_SEND_MESSAGE, send_message). -define(SYNC_QUERY_MODE, handover). --define(ASYNC_QUERY_MODE(REPLY), {handover_async, {?MODULE, do_async_reply, [REPLY]}}). -define(SQLSERVER_HOST_OPTIONS, #{ default_port => 1433 @@ -169,7 +166,7 @@ server() -> %% Callbacks defined in emqx_resource %%==================================================================== -callback_mode() -> async_if_possible. +callback_mode() -> always_sync. is_buffer_supported() -> false. @@ -253,28 +250,6 @@ on_query(InstanceId, {?ACTION_SEND_MESSAGE, _Msg} = Query, State) -> ), do_query(InstanceId, Query, ?SYNC_QUERY_MODE, State). --spec on_query_async( - manager_id(), - {?ACTION_SEND_MESSAGE, map()}, - {ReplyFun :: function(), Args :: list()}, - state() -) -> - {ok, any()} - | {error, term()}. -on_query_async( - InstanceId, - {?ACTION_SEND_MESSAGE, _Msg} = Query, - ReplyFunAndArgs, - %% #{poolname := PoolName, sql_templates := Templates} = State - State -) -> - ?TRACE( - "SINGLE_QUERY_ASYNC", - "bridge_sqlserver_received", - #{requests => Query, connector => InstanceId, state => State} - ), - do_query(InstanceId, Query, ?ASYNC_QUERY_MODE(ReplyFunAndArgs), State). - -spec on_batch_query( manager_id(), [{?ACTION_SEND_MESSAGE, map()}], @@ -292,20 +267,6 @@ on_batch_query(InstanceId, BatchRequests, State) -> ), do_query(InstanceId, BatchRequests, ?SYNC_QUERY_MODE, State). --spec on_batch_query_async( - manager_id(), - [{?ACTION_SEND_MESSAGE, map()}], - {ReplyFun :: function(), Args :: list()}, - state() -) -> {ok, any()}. -on_batch_query_async(InstanceId, Requests, ReplyFunAndArgs, State) -> - ?TRACE( - "BATCH_QUERY_ASYNC", - "bridge_sqlserver_received", - #{requests => Requests, connector => InstanceId, state => State} - ), - do_query(InstanceId, Requests, ?ASYNC_QUERY_MODE(ReplyFunAndArgs), State). - on_get_status(_InstanceId, #{poolname := Pool} = _State) -> Health = emqx_plugin_libs_pool:health_check_ecpool_workers( Pool, {?MODULE, do_get_status, []} @@ -365,13 +326,11 @@ conn_str([{password, Password} | Opts], Acc) -> conn_str([{_, _} | Opts], Acc) -> conn_str(Opts, Acc). -%% Sync & Async query with singe & batch sql statement +%% Query with singe & batch sql statement -spec do_query( manager_id(), Query :: {?ACTION_SEND_MESSAGE, map()} | [{?ACTION_SEND_MESSAGE, map()}], - ApplyMode :: - handover - | {handover_async, {?MODULE, do_async_reply, [{ReplyFun :: function(), Args :: list()}]}}, + ApplyMode :: handover, state() ) -> {ok, list()} @@ -531,6 +490,3 @@ apply_template(Query, Templates) -> %% TODO: more detail infomatoin ?SLOG(error, #{msg => "apply sql template failed", query => Query, templates => Templates}), {error, failed_to_apply_sql_template}. - -do_async_reply(Result, {ReplyFun, Args}) -> - erlang:apply(ReplyFun, Args ++ [Result]). From 7e24b35bb3bc5b5735af67a7335d8a507f21fefe Mon Sep 17 00:00:00 2001 From: Thales Macedo Garitezi Date: Wed, 26 Apr 2023 17:09:39 -0300 Subject: [PATCH 07/41] chore: bump release version to `e5.0.3-alpha.4` --- 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 6d91b2528..9e7e53b32 100644 --- a/apps/emqx/include/emqx_release.hrl +++ b/apps/emqx/include/emqx_release.hrl @@ -35,7 +35,7 @@ -define(EMQX_RELEASE_CE, "5.0.22"). %% Enterprise edition --define(EMQX_RELEASE_EE, "5.0.3-alpha.3"). +-define(EMQX_RELEASE_EE, "5.0.3-alpha.4"). %% the HTTP API version -define(EMQX_API_VERSION, "5.0"). From e789e57b653e78952d4acbed858f7d9f56228213 Mon Sep 17 00:00:00 2001 From: Zhongwen Deng Date: Thu, 27 Apr 2023 14:41:23 +0800 Subject: [PATCH 08/41] chore: change node.data_dir from hidden to low --- apps/emqx_conf/src/emqx_conf_schema.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/emqx_conf/src/emqx_conf_schema.erl b/apps/emqx_conf/src/emqx_conf_schema.erl index 8e5e6937f..39dce0b71 100644 --- a/apps/emqx_conf/src/emqx_conf_schema.erl +++ b/apps/emqx_conf/src/emqx_conf_schema.erl @@ -471,7 +471,7 @@ fields("node") -> %% for now, it's tricky to use a different data_dir %% otherwise data paths in cluster config may differ %% TODO: change configurable data file paths to relative - importance => ?IMPORTANCE_HIDDEN, + importance => ?IMPORTANCE_LOW, desc => ?DESC(node_data_dir) } )}, From 66155a8636e59f6ba5edb3af2985a70ef0abf39e Mon Sep 17 00:00:00 2001 From: JimMoen Date: Wed, 26 Apr 2023 17:55:33 +0800 Subject: [PATCH 09/41] chore: add ms msodbcsql docker file base on emqx-enterprise --- deploy/docker/Dockerfile.msodbc | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 deploy/docker/Dockerfile.msodbc diff --git a/deploy/docker/Dockerfile.msodbc b/deploy/docker/Dockerfile.msodbc new file mode 100644 index 000000000..d7b3457ac --- /dev/null +++ b/deploy/docker/Dockerfile.msodbc @@ -0,0 +1,25 @@ +## This Dockerfile should not run in GitHub Action or any other automated process. +## It should be manually executed by the needs of the user. +## +## Before manaually execute: +## Please confirm the EMQX-Enterprise version you are using and modify the base layer image tag +## ```bash +## $ docker build -f=Dockerfile.msodbc -t emqx-enterprise-with-msodbc:5.0.3-alpha.2 . +## ``` + +# FROM emqx/emqx-enterprise:latest +FROM emqx/emqx-enterprise:5.0.3-alpha.2 + +USER root + +RUN apt-get update \ + && apt-get install -y gnupg2 curl apt-utils \ + && curl https://packages.microsoft.com/keys/microsoft.asc | apt-key add - \ + && curl https://packages.microsoft.com/config/debian/11/prod.list > /etc/apt/sources.list.d/mssql-mkc crelease.list \ + && apt-get update \ + && ACCEPT_EULA=Y apt-get install -y msodbcsql17 unixodbc-dev \ + && sed -i 's/ODBC Driver 17 for SQL Server/ms-sql/g' /etc/odbcinst.ini \ + && apt-get clean \ + && rm -rf /var/lib/apt/lists/* + +USER emqx From d845c4807d2499637edb1ea13fea94fa90b8be94 Mon Sep 17 00:00:00 2001 From: Thales Macedo Garitezi Date: Thu, 27 Apr 2023 11:38:30 -0300 Subject: [PATCH 10/41] fix(ocsp): disable periodic refresh when listener or stapling are disabled Fixes https://emqx.atlassian.net/browse/EMQX-9773 --- apps/emqx/src/emqx_listeners.erl | 31 ++++++++-- apps/emqx/src/emqx_ocsp_cache.erl | 16 +++++ apps/emqx/test/emqx_ocsp_cache_SUITE.erl | 77 +++++++++++++++++++++++- 3 files changed, 117 insertions(+), 7 deletions(-) diff --git a/apps/emqx/src/emqx_listeners.erl b/apps/emqx/src/emqx_listeners.erl index f82aebe7c..18ddcaba2 100644 --- a/apps/emqx/src/emqx_listeners.erl +++ b/apps/emqx/src/emqx_listeners.erl @@ -441,6 +441,7 @@ post_config_update([listeners, Type, Name], {create, _Request}, NewConf, undefin start_listener(Type, Name, NewConf); post_config_update([listeners, Type, Name], {update, _Request}, NewConf, OldConf, _AppEnvs) -> try_clear_ssl_files(certs_dir(Type, Name), NewConf, OldConf), + ok = maybe_unregister_ocsp_stapling_refresh(Type, Name, NewConf), case NewConf of #{enabled := true} -> restart_listener(Type, Name, {OldConf, NewConf}); _ -> ok @@ -448,6 +449,7 @@ post_config_update([listeners, Type, Name], {update, _Request}, NewConf, OldConf post_config_update([listeners, _Type, _Name], '$remove', undefined, undefined, _AppEnvs) -> ok; post_config_update([listeners, Type, Name], '$remove', undefined, OldConf, _AppEnvs) -> + ok = unregister_ocsp_stapling_refresh(Type, Name), case stop_listener(Type, Name, OldConf) of ok -> _ = emqx_authentication:delete_chain(listener_id(Type, Name)), @@ -460,10 +462,18 @@ post_config_update([listeners, Type, Name], {action, _Action, _}, NewConf, OldCo #{enabled := NewEnabled} = NewConf, #{enabled := OldEnabled} = OldConf, case {NewEnabled, OldEnabled} of - {true, true} -> restart_listener(Type, Name, {OldConf, NewConf}); - {true, false} -> start_listener(Type, Name, NewConf); - {false, true} -> stop_listener(Type, Name, OldConf); - {false, false} -> stop_listener(Type, Name, OldConf) + {true, true} -> + ok = maybe_unregister_ocsp_stapling_refresh(Type, Name, NewConf), + restart_listener(Type, Name, {OldConf, NewConf}); + {true, false} -> + ok = maybe_unregister_ocsp_stapling_refresh(Type, Name, NewConf), + start_listener(Type, Name, NewConf); + {false, true} -> + ok = unregister_ocsp_stapling_refresh(Type, Name), + stop_listener(Type, Name, OldConf); + {false, false} -> + ok = unregister_ocsp_stapling_refresh(Type, Name), + stop_listener(Type, Name, OldConf) end; post_config_update(_Path, _Request, _NewConf, _OldConf, _AppEnvs) -> ok. @@ -813,3 +823,16 @@ inject_crl_config( }; inject_crl_config(Conf) -> Conf. + +maybe_unregister_ocsp_stapling_refresh( + ssl = Type, Name, #{ssl_options := #{ocsp := #{enable_ocsp_stapling := false}}} = _Conf +) -> + unregister_ocsp_stapling_refresh(Type, Name), + ok; +maybe_unregister_ocsp_stapling_refresh(_Type, _Name, _Conf) -> + ok. + +unregister_ocsp_stapling_refresh(Type, Name) -> + ListenerId = listener_id(Type, Name), + emqx_ocsp_cache:unregister_listener(ListenerId), + ok. diff --git a/apps/emqx/src/emqx_ocsp_cache.erl b/apps/emqx/src/emqx_ocsp_cache.erl index 3bb10ee5c..ef0411b37 100644 --- a/apps/emqx/src/emqx_ocsp_cache.erl +++ b/apps/emqx/src/emqx_ocsp_cache.erl @@ -30,6 +30,7 @@ sni_fun/2, fetch_response/1, register_listener/2, + unregister_listener/1, inject_sni_fun/2 ]). @@ -107,6 +108,9 @@ fetch_response(ListenerID) -> register_listener(ListenerID, Opts) -> gen_server:call(?MODULE, {register_listener, ListenerID, Opts}, ?CALL_TIMEOUT). +unregister_listener(ListenerID) -> + gen_server:cast(?MODULE, {unregister_listener, ListenerID}). + -spec inject_sni_fun(emqx_listeners:listener_id(), map()) -> map(). inject_sni_fun(ListenerID, Conf0) -> SNIFun = emqx_const_v1:make_sni_fun(ListenerID), @@ -160,6 +164,18 @@ handle_call({register_listener, ListenerID, Conf}, _From, State0) -> handle_call(Call, _From, State) -> {reply, {error, {unknown_call, Call}}, State}. +handle_cast({unregister_listener, ListenerID}, State0) -> + State2 = + case maps:take(?REFRESH_TIMER(ListenerID), State0) of + error -> + State0; + {TRef, State1} -> + emqx_utils:cancel_timer(TRef), + State1 + end, + State = maps:remove({refresh_interval, ListenerID}, State2), + ?tp(ocsp_cache_listener_unregistered, #{listener_id => ListenerID}), + {noreply, State}; handle_cast(_Cast, State) -> {noreply, State}. diff --git a/apps/emqx/test/emqx_ocsp_cache_SUITE.erl b/apps/emqx/test/emqx_ocsp_cache_SUITE.erl index 15ca29853..75c41b9fb 100644 --- a/apps/emqx/test/emqx_ocsp_cache_SUITE.erl +++ b/apps/emqx/test/emqx_ocsp_cache_SUITE.erl @@ -254,10 +254,15 @@ does_module_exist(Mod) -> end. assert_no_http_get() -> + Timeout = 0, + Error = should_be_cached, + assert_no_http_get(Timeout, Error). + +assert_no_http_get(Timeout, Error) -> receive {http_get, _URL} -> - error(should_be_cached) - after 0 -> + error(Error) + after Timeout -> ok end. @@ -702,7 +707,9 @@ do_t_update_listener(Config) -> %% the API converts that to an internally %% managed file <<"issuer_pem">> => IssuerPem, - <<"responder_url">> => <<"http://localhost:9877">> + <<"responder_url">> => <<"http://localhost:9877">>, + %% for quicker testing; min refresh in tests is 5 s. + <<"refresh_interval">> => <<"5s">> } } }, @@ -739,6 +746,70 @@ do_t_update_listener(Config) -> ) ), assert_http_get(1, 5_000), + + %% Disable OCSP Stapling; the periodic refreshes should stop + RefreshInterval = emqx_config:get([listeners, ssl, default, ssl_options, ocsp, refresh_interval]), + OCSPConfig1 = + #{ + <<"ssl_options">> => + #{ + <<"ocsp">> => + #{ + <<"enable_ocsp_stapling">> => false + } + } + }, + ListenerData3 = emqx_utils_maps:deep_merge(ListenerData2, OCSPConfig1), + {ok, {_, _, ListenerData4}} = update_listener_via_api(ListenerId, ListenerData3), + ?assertMatch( + #{ + <<"ssl_options">> := + #{ + <<"ocsp">> := + #{ + <<"enable_ocsp_stapling">> := false + } + } + }, + ListenerData4 + ), + + assert_no_http_get(2 * RefreshInterval, should_stop_refreshing), + + ok. + +t_double_unregister(_Config) -> + ListenerID = <<"ssl:test_ocsp">>, + Conf = emqx_config:get_listener_conf(ssl, test_ocsp, []), + ?check_trace( + begin + {ok, {ok, _}} = + ?wait_async_action( + emqx_ocsp_cache:register_listener(ListenerID, Conf), + #{?snk_kind := ocsp_http_fetch_and_cache, listener_id := ListenerID}, + 5_000 + ), + assert_http_get(1), + + {ok, {ok, _}} = + ?wait_async_action( + emqx_ocsp_cache:unregister_listener(ListenerID), + #{?snk_kind := ocsp_cache_listener_unregistered, listener_id := ListenerID}, + 5_000 + ), + + %% Should be idempotent and not crash + {ok, {ok, _}} = + ?wait_async_action( + emqx_ocsp_cache:unregister_listener(ListenerID), + #{?snk_kind := ocsp_cache_listener_unregistered, listener_id := ListenerID}, + 5_000 + ), + ok + end, + [] + ), + ok. t_ocsp_responder_error_responses(_Config) -> From 77f5e461a304cdcefc534e95baff2539f41c99c6 Mon Sep 17 00:00:00 2001 From: Thales Macedo Garitezi Date: Thu, 27 Apr 2023 09:24:35 -0300 Subject: [PATCH 11/41] chore: bump ehttpc -> 0.4.8 Fixes https://emqx.atlassian.net/browse/EMQX-9656 See also https://github.com/emqx/ehttpc/pull/45 This fixes a race condition where the remote server would close the connection before or during requests, and, depending on timing, an `{error, normal}` response would be returned. In those cases, we should just retry the request without using up "retry credits". --- changes/ce/fix-10548.en.md | 2 ++ mix.exs | 2 +- rebar.config | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) create mode 100644 changes/ce/fix-10548.en.md diff --git a/changes/ce/fix-10548.en.md b/changes/ce/fix-10548.en.md new file mode 100644 index 000000000..d96f0b57f --- /dev/null +++ b/changes/ce/fix-10548.en.md @@ -0,0 +1,2 @@ +Fixed a race condition in the HTTP driver that would result in an error rather than a retry of the request. +Related fix in the driver: https://github.com/emqx/ehttpc/pull/45 diff --git a/mix.exs b/mix.exs index 2b8de4c54..1d0ce7063 100644 --- a/mix.exs +++ b/mix.exs @@ -49,7 +49,7 @@ defmodule EMQXUmbrella.MixProject do {:redbug, "2.0.8"}, {:covertool, github: "zmstone/covertool", tag: "2.0.4.1", override: true}, {:typerefl, github: "ieQu1/typerefl", tag: "0.9.1", override: true}, - {:ehttpc, github: "emqx/ehttpc", tag: "0.4.7", override: true}, + {:ehttpc, github: "emqx/ehttpc", tag: "0.4.8", override: true}, {:gproc, github: "uwiger/gproc", tag: "0.8.0", override: true}, {:jiffy, github: "emqx/jiffy", tag: "1.0.5", override: true}, {:cowboy, github: "emqx/cowboy", tag: "2.9.0", override: true}, diff --git a/rebar.config b/rebar.config index 7e783b56d..d14a8099a 100644 --- a/rebar.config +++ b/rebar.config @@ -56,7 +56,7 @@ , {gpb, "4.19.5"} %% gpb only used to build, but not for release, pin it here to avoid fetching a wrong version due to rebar plugins scattered in all the deps , {typerefl, {git, "https://github.com/ieQu1/typerefl", {tag, "0.9.1"}}} , {gun, {git, "https://github.com/emqx/gun", {tag, "1.3.9"}}} - , {ehttpc, {git, "https://github.com/emqx/ehttpc", {tag, "0.4.7"}}} + , {ehttpc, {git, "https://github.com/emqx/ehttpc", {tag, "0.4.8"}}} , {gproc, {git, "https://github.com/uwiger/gproc", {tag, "0.8.0"}}} , {jiffy, {git, "https://github.com/emqx/jiffy", {tag, "1.0.5"}}} , {cowboy, {git, "https://github.com/emqx/cowboy", {tag, "2.9.0"}}} From 6e36139a1743a27a036a757a3554196839531d5d Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Thu, 27 Apr 2023 22:56:34 +0200 Subject: [PATCH 12/41] chore: bump to e5.0.3-alpha.5 --- 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 9e7e53b32..21eb85dfb 100644 --- a/apps/emqx/include/emqx_release.hrl +++ b/apps/emqx/include/emqx_release.hrl @@ -35,7 +35,7 @@ -define(EMQX_RELEASE_CE, "5.0.22"). %% Enterprise edition --define(EMQX_RELEASE_EE, "5.0.3-alpha.4"). +-define(EMQX_RELEASE_EE, "5.0.3-alpha.5"). %% the HTTP API version -define(EMQX_API_VERSION, "5.0"). From d3a26b45beb011a64c0b47140353730a73f0c3a0 Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Thu, 27 Apr 2023 12:21:13 +0200 Subject: [PATCH 13/41] docs: update config note --- .gitignore | 2 ++ apps/emqx_conf/etc/emqx_conf.conf | 15 ++++++++------- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/.gitignore b/.gitignore index 6c4de9272..ceb12182f 100644 --- a/.gitignore +++ b/.gitignore @@ -70,3 +70,5 @@ apps/emqx/test/emqx_static_checks_data/master.bpapi lux_logs/ /.prepare bom.json +ct_run*/ +apps/emqx_conf/etc/emqx.conf.all.rendered* diff --git a/apps/emqx_conf/etc/emqx_conf.conf b/apps/emqx_conf/etc/emqx_conf.conf index 86147bf25..4c03c10c6 100644 --- a/apps/emqx_conf/etc/emqx_conf.conf +++ b/apps/emqx_conf/etc/emqx_conf.conf @@ -1,12 +1,13 @@ ## NOTE: -## Configs in this file might be overridden by: -## 1. Environment variables which start with 'EMQX_' prefix -## 2. File $EMQX_NODE__DATA_DIR/configs/cluster-override.conf -## 3. File $EMQX_NODE__DATA_DIR/configs/local-override.conf +## This config file overrides data/configs/cluster.hocon, +## and is merged with environment variables which start with 'EMQX_' prefix. ## -## The *-override.conf files are overwritten at runtime when changes -## are made from EMQX dashboard UI, management HTTP API, or CLI. -## All configuration details can be found in emqx.conf.example +## Config changes made from EMQX dashboard UI, management HTTP API, or CLI +## are stored in data/configs/cluster.hocon. +## To avoid confusion, please do not store the same configs in both files. +## +## See https://docs.emqx.com/en/enterprise/v5.0/configuration/configuration.html +## Configuration full example can be found in emqx.conf.example node { name = "emqx@127.0.0.1" From 7a81b96be0380789d16f75294f0d60a242e1ff94 Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Thu, 27 Apr 2023 20:48:53 +0200 Subject: [PATCH 14/41] fix(emqx_conf_app): print init_load failure to standard_error logger may not get the chance to spit out the logs before the vm dies, no matter how long sleep is added before init:stop(1) --- apps/emqx_conf/src/emqx_conf_app.erl | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/apps/emqx_conf/src/emqx_conf_app.erl b/apps/emqx_conf/src/emqx_conf_app.erl index fbfb97a79..dedb9aeab 100644 --- a/apps/emqx_conf/src/emqx_conf_app.erl +++ b/apps/emqx_conf/src/emqx_conf_app.erl @@ -32,12 +32,8 @@ start(_StartType, _StartArgs) -> ok = init_conf() catch C:E:St -> - ?SLOG(critical, #{ - msg => failed_to_init_config, - exception => C, - reason => E, - stacktrace => St - }), + %% logger is not quite ready. + io:format(standard_error, "Failed to load config~n~p~n~p~n~p~n", [C, E, St]), init:stop(1) end, ok = emqx_config_logger:refresh_config(), From a19621e533e015897f07d204379ffb293c858fc5 Mon Sep 17 00:00:00 2001 From: Thales Macedo Garitezi Date: Fri, 28 Apr 2023 14:43:21 -0300 Subject: [PATCH 15/41] fix(webhook): treat `{shutdown, normal}` and `{closed, _}` async reply as retriable Apparently, the async reply returned by ehttpc can be `{shutdown, normal}` or `{closed, "The connection was lost."}`, in which case the request should be retried. ``` Apr 28 17:40:41 emqx-0.int.thales bash[48880]: 17:40:41.803 [error] [id: "bridge:webhook:webhook", msg: :unrecoverable_error, reason: {:shutdown, :normal}] Apr 28 18:36:37 emqx-0.int.thales bash[53368]: 18:36:37.605 [error] [id: "bridge:webhook:webhook", msg: :unrecoverable_error, reason: {:closed, 'The connection was lost.'}] ``` --- .../src/emqx_connector_http.erl | 26 +++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/apps/emqx_connector/src/emqx_connector_http.erl b/apps/emqx_connector/src/emqx_connector_http.erl index ef2e11eb7..c89285727 100644 --- a/apps/emqx_connector/src/emqx_connector_http.erl +++ b/apps/emqx_connector/src/emqx_connector_http.erl @@ -306,7 +306,20 @@ on_query( Retry ) of - {error, Reason} when Reason =:= econnrefused; Reason =:= timeout -> + {error, Reason} when + Reason =:= econnrefused; + Reason =:= timeout; + Reason =:= {shutdown, normal}; + Reason =:= {shutdown, closed} + -> + ?SLOG(warning, #{ + msg => "http_connector_do_request_failed", + reason => Reason, + connector => InstId + }), + {error, {recoverable_error, Reason}}; + {error, {closed, _Message} = Reason} -> + %% _Message = "The connection was lost." ?SLOG(warning, #{ msg => "http_connector_do_request_failed", reason => Reason, @@ -568,7 +581,16 @@ reply_delegator(ReplyFunAndArgs, Result) -> case Result of %% The normal reason happens when the HTTP connection times out before %% the request has been fully processed - {error, Reason} when Reason =:= econnrefused; Reason =:= timeout; Reason =:= normal -> + {error, Reason} when + Reason =:= econnrefused; + Reason =:= timeout; + Reason =:= normal; + Reason =:= {shutdown, normal} + -> + Result1 = {error, {recoverable_error, Reason}}, + emqx_resource:apply_reply_fun(ReplyFunAndArgs, Result1); + {error, {closed, _Message} = Reason} -> + %% _Message = "The connection was lost." Result1 = {error, {recoverable_error, Reason}}, emqx_resource:apply_reply_fun(ReplyFunAndArgs, Result1); _ -> From c58ffce75f61e1dd2045894be47a7d92442aa8a1 Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Fri, 28 Apr 2023 22:49:52 +0200 Subject: [PATCH 16/41] fix(hocon): pin 0.38.2 with the map type value converter fixed --- apps/emqx/rebar.config | 2 +- mix.exs | 2 +- rebar.config | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/emqx/rebar.config b/apps/emqx/rebar.config index d954a6b1e..26f58ff69 100644 --- a/apps/emqx/rebar.config +++ b/apps/emqx/rebar.config @@ -29,7 +29,7 @@ {esockd, {git, "https://github.com/emqx/esockd", {tag, "5.9.6"}}}, {ekka, {git, "https://github.com/emqx/ekka", {tag, "0.14.6"}}}, {gen_rpc, {git, "https://github.com/emqx/gen_rpc", {tag, "2.8.1"}}}, - {hocon, {git, "https://github.com/emqx/hocon.git", {tag, "0.38.1"}}}, + {hocon, {git, "https://github.com/emqx/hocon.git", {tag, "0.38.2"}}}, {emqx_http_lib, {git, "https://github.com/emqx/emqx_http_lib.git", {tag, "0.5.2"}}}, {pbkdf2, {git, "https://github.com/emqx/erlang-pbkdf2.git", {tag, "2.0.4"}}}, {recon, {git, "https://github.com/ferd/recon", {tag, "2.5.1"}}}, diff --git a/mix.exs b/mix.exs index 1d0ce7063..93f7417c9 100644 --- a/mix.exs +++ b/mix.exs @@ -72,7 +72,7 @@ defmodule EMQXUmbrella.MixProject do # in conflict by emqtt and hocon {:getopt, "1.0.2", override: true}, {:snabbkaffe, github: "kafka4beam/snabbkaffe", tag: "1.0.7", override: true}, - {:hocon, github: "emqx/hocon", tag: "0.38.1", override: true}, + {:hocon, github: "emqx/hocon", tag: "0.38.2", override: true}, {:emqx_http_lib, github: "emqx/emqx_http_lib", tag: "0.5.2", override: true}, {:esasl, github: "emqx/esasl", tag: "0.2.0"}, {:jose, github: "potatosalad/erlang-jose", tag: "1.11.2"}, diff --git a/rebar.config b/rebar.config index d14a8099a..267f60ee4 100644 --- a/rebar.config +++ b/rebar.config @@ -75,7 +75,7 @@ , {system_monitor, {git, "https://github.com/ieQu1/system_monitor", {tag, "3.0.3"}}} , {getopt, "1.0.2"} , {snabbkaffe, {git, "https://github.com/kafka4beam/snabbkaffe.git", {tag, "1.0.7"}}} - , {hocon, {git, "https://github.com/emqx/hocon.git", {tag, "0.38.1"}}} + , {hocon, {git, "https://github.com/emqx/hocon.git", {tag, "0.38.2"}}} , {emqx_http_lib, {git, "https://github.com/emqx/emqx_http_lib.git", {tag, "0.5.2"}}} , {esasl, {git, "https://github.com/emqx/esasl", {tag, "0.2.0"}}} , {jose, {git, "https://github.com/potatosalad/erlang-jose", {tag, "1.11.2"}}} From b0f3a654ee864eb5a533f1fb6b009f28d1a62fa6 Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Thu, 27 Apr 2023 12:22:09 +0200 Subject: [PATCH 17/41] refactor: delete default listeners from default config The new config overriding rule is very much confusing for people who wants to persist listener config changes made from dashboard This commit moves the default values from default config file to schema source code. In order to support build-time cert path at runtime, there is also a naive environment variable interplation feature added. --- apps/emqx/etc/emqx.conf | 43 ---------------- apps/emqx/src/emqx_schema.erl | 53 ++++++++++++++++++++ apps/emqx/src/emqx_tls_lib.erl | 91 ++++++++++++++++++++++++++++------ 3 files changed, 129 insertions(+), 58 deletions(-) diff --git a/apps/emqx/etc/emqx.conf b/apps/emqx/etc/emqx.conf index ee345e9d6..e69de29bb 100644 --- a/apps/emqx/etc/emqx.conf +++ b/apps/emqx/etc/emqx.conf @@ -1,43 +0,0 @@ -listeners.tcp.default { - bind = "0.0.0.0:1883" - max_connections = 1024000 -} - -listeners.ssl.default { - bind = "0.0.0.0:8883" - max_connections = 512000 - ssl_options { - keyfile = "{{ platform_etc_dir }}/certs/key.pem" - certfile = "{{ platform_etc_dir }}/certs/cert.pem" - cacertfile = "{{ platform_etc_dir }}/certs/cacert.pem" - } -} - -listeners.ws.default { - bind = "0.0.0.0:8083" - max_connections = 1024000 - websocket.mqtt_path = "/mqtt" -} - -listeners.wss.default { - bind = "0.0.0.0:8084" - max_connections = 512000 - websocket.mqtt_path = "/mqtt" - ssl_options { - keyfile = "{{ platform_etc_dir }}/certs/key.pem" - certfile = "{{ platform_etc_dir }}/certs/cert.pem" - cacertfile = "{{ platform_etc_dir }}/certs/cacert.pem" - } -} - -# listeners.quic.default { -# enabled = true -# bind = "0.0.0.0:14567" -# max_connections = 1024000 -# ssl_options { -# verify = verify_none -# keyfile = "{{ platform_etc_dir }}/certs/key.pem" -# certfile = "{{ platform_etc_dir }}/certs/cert.pem" -# cacertfile = "{{ platform_etc_dir }}/certs/cacert.pem" -# } -# } diff --git a/apps/emqx/src/emqx_schema.erl b/apps/emqx/src/emqx_schema.erl index ba333f111..c56fc9b48 100644 --- a/apps/emqx/src/emqx_schema.erl +++ b/apps/emqx/src/emqx_schema.erl @@ -779,6 +779,7 @@ fields("listeners") -> map(name, ref("mqtt_tcp_listener")), #{ desc => ?DESC(fields_listeners_tcp), + default => default_listener(tcp), required => {false, recursively} } )}, @@ -787,6 +788,7 @@ fields("listeners") -> map(name, ref("mqtt_ssl_listener")), #{ desc => ?DESC(fields_listeners_ssl), + default => default_listener(ssl), required => {false, recursively} } )}, @@ -795,6 +797,7 @@ fields("listeners") -> map(name, ref("mqtt_ws_listener")), #{ desc => ?DESC(fields_listeners_ws), + default => default_listener(ws), required => {false, recursively} } )}, @@ -803,6 +806,7 @@ fields("listeners") -> map(name, ref("mqtt_wss_listener")), #{ desc => ?DESC(fields_listeners_wss), + default => default_listener(wss), required => {false, recursively} } )}, @@ -3083,3 +3087,52 @@ assert_required_field(Conf, Key, ErrorMessage) -> _ -> ok end. + +default_listener(tcp) -> + #{ + <<"default">> => + #{ + <<"bind">> => <<"0.0.0.0:1883">>, + <<"max_connections">> => 1024000 + } + }; +default_listener(ws) -> + #{ + <<"default">> => + #{ + <<"bind">> => <<"0.0.0.0:8083">>, + <<"max_connections">> => 1024000, + <<"websocket">> => #{<<"mqtt_path">> => <<"/mqtt">>} + } + }; +default_listener(SSLListener) -> + %% The env variable is resolved in emqx_tls_lib + CertFile = fun(Name) -> + iolist_to_binary("${EMQX_ETC_DIR}/" ++ filename:join(["certs", Name])) + end, + SslOptions = #{ + <<"cacertfile">> => CertFile(<<"cacert.pem">>), + <<"certfile">> => CertFile(<<"cert.pem">>), + <<"keyfile">> => CertFile(<<"key.pem">>) + }, + case SSLListener of + ssl -> + #{ + <<"default">> => + #{ + <<"bind">> => <<"0.0.0.0:8883">>, + <<"max_connections">> => 512000, + <<"ssl_options">> => SslOptions + } + }; + wss -> + #{ + <<"default">> => + #{ + <<"bind">> => <<"0.0.0.0:8084">>, + <<"max_connections">> => 512000, + <<"ssl_options">> => SslOptions, + <<"websocket">> => #{<<"mqtt_path">> => <<"/mqtt">>} + } + } + end. diff --git a/apps/emqx/src/emqx_tls_lib.erl b/apps/emqx/src/emqx_tls_lib.erl index d1c57bf0d..c555059cf 100644 --- a/apps/emqx/src/emqx_tls_lib.erl +++ b/apps/emqx/src/emqx_tls_lib.erl @@ -309,19 +309,19 @@ ensure_ssl_files(Dir, SSL, Opts) -> case ensure_ssl_file_key(SSL, RequiredKeys) of ok -> KeyPaths = ?SSL_FILE_OPT_PATHS ++ ?SSL_FILE_OPT_PATHS_A, - ensure_ssl_files(Dir, SSL, KeyPaths, Opts); + ensure_ssl_files_per_key(Dir, SSL, KeyPaths, Opts); {error, _} = Error -> Error end. -ensure_ssl_files(_Dir, SSL, [], _Opts) -> +ensure_ssl_files_per_key(_Dir, SSL, [], _Opts) -> {ok, SSL}; -ensure_ssl_files(Dir, SSL, [KeyPath | KeyPaths], Opts) -> +ensure_ssl_files_per_key(Dir, SSL, [KeyPath | KeyPaths], Opts) -> case ensure_ssl_file(Dir, KeyPath, SSL, emqx_utils_maps:deep_get(KeyPath, SSL, undefined), Opts) of {ok, NewSSL} -> - ensure_ssl_files(Dir, NewSSL, KeyPaths, Opts); + ensure_ssl_files_per_key(Dir, NewSSL, KeyPaths, Opts); {error, Reason} -> {error, Reason#{which_options => [KeyPath]}} end. @@ -347,7 +347,8 @@ delete_ssl_files(Dir, NewOpts0, OldOpts0) -> delete_old_file(New, Old) when New =:= Old -> ok; delete_old_file(_New, _Old = undefined) -> ok; -delete_old_file(_New, Old) -> +delete_old_file(_New, Old0) -> + Old = resolve_cert_path(Old0), case is_generated_file(Old) andalso filelib:is_regular(Old) andalso file:delete(Old) of ok -> ok; @@ -355,7 +356,7 @@ delete_old_file(_New, Old) -> false -> ok; {error, Reason} -> - ?SLOG(error, #{msg => "failed_to_delete_ssl_file", file_path => Old, reason => Reason}) + ?SLOG(error, #{msg => "failed_to_delete_ssl_file", file_path => Old0, reason => Reason}) end. ensure_ssl_file(_Dir, _KeyPath, SSL, undefined, _Opts) -> @@ -414,7 +415,8 @@ is_pem(MaybePem) -> %% To make it simple, the file is always overwritten. %% Also a potentially half-written PEM file (e.g. due to power outage) %% can be corrected with an overwrite. -save_pem_file(Dir, KeyPath, Pem, DryRun) -> +save_pem_file(Dir0, KeyPath, Pem, DryRun) -> + Dir = resolve_cert_path(Dir0), Path = pem_file_name(Dir, KeyPath, Pem), case filelib:ensure_dir(Path) of ok when DryRun -> @@ -472,7 +474,8 @@ hex_str(Bin) -> iolist_to_binary([io_lib:format("~2.16.0b", [X]) || <> <= Bin]). %% @doc Returns 'true' when the file is a valid pem, otherwise {error, Reason}. -is_valid_pem_file(Path) -> +is_valid_pem_file(Path0) -> + Path = resolve_cert_path(Path0), case file:read_file(Path) of {ok, Pem} -> is_pem(Pem) orelse {error, not_pem}; {error, Reason} -> {error, Reason} @@ -513,10 +516,15 @@ do_drop_invalid_certs([KeyPath | KeyPaths], SSL) -> to_server_opts(Type, Opts) -> Versions = integral_versions(Type, maps:get(versions, Opts, undefined)), Ciphers = integral_ciphers(Versions, maps:get(ciphers, Opts, undefined)), - maps:to_list(Opts#{ - ciphers => Ciphers, - versions => Versions - }). + filter( + maps:to_list(Opts#{ + keyfile => resolve_cert_path_strict(maps:get(keyfile, Opts, undefined)), + certfile => resolve_cert_path_strict(maps:get(certfile, Opts, undefined)), + cacertfile => resolve_cert_path_strict(maps:get(cacertfile, Opts, undefined)), + ciphers => Ciphers, + versions => Versions + }) + ). %% @doc Convert hocon-checked tls client options (map()) to %% proplist accepted by ssl library. @@ -532,9 +540,9 @@ to_client_opts(Type, Opts) -> Get = fun(Key) -> GetD(Key, undefined) end, case GetD(enable, false) of true -> - KeyFile = ensure_str(Get(keyfile)), - CertFile = ensure_str(Get(certfile)), - CAFile = ensure_str(Get(cacertfile)), + KeyFile = resolve_cert_path_strict(Get(keyfile)), + CertFile = resolve_cert_path_strict(Get(certfile)), + CAFile = resolve_cert_path_strict(Get(cacertfile)), Verify = GetD(verify, verify_none), SNI = ensure_sni(Get(server_name_indication)), Versions = integral_versions(Type, Get(versions)), @@ -556,6 +564,59 @@ to_client_opts(Type, Opts) -> [] end. +resolve_cert_path_strict(Path) -> + case resolve_cert_path(Path) of + undefined -> + undefined; + ResolvedPath -> + case filelib:is_regular(ResolvedPath) of + true -> + ResolvedPath; + false -> + PathToLog = ensure_str(Path), + LogData = + case PathToLog =:= ResolvedPath of + true -> + #{path => PathToLog}; + false -> + #{path => PathToLog, resolved_path => ResolvedPath} + end, + ?SLOG(error, LogData#{msg => "cert_file_not_found"}), + undefined + end + end. + +resolve_cert_path(undefined) -> + undefined; +resolve_cert_path(Path) -> + case ensure_str(Path) of + "$" ++ Maybe -> + naive_env_resolver(Maybe); + Other -> + Other + end. + +%% resolves a file path like "ENV_VARIABLE/sub/path" or "{ENV_VARIABLE}/sub/path" +%% in windows, it could be "ENV_VARIABLE/sub\path" or "{ENV_VARIABLE}/sub\path" +naive_env_resolver(Maybe) -> + case string:split(Maybe, "/") of + [_] -> + Maybe; + [Env, SubPath] -> + case os:getenv(trim_env_name(Env)) of + false -> + SubPath; + "" -> + SubPath; + EnvValue -> + filename:join(EnvValue, SubPath) + end + end. + +%% delete the first and last curly braces +trim_env_name(Env) -> + string:trim(Env, both, "{}"). + filter([]) -> []; filter([{_, undefined} | T]) -> filter(T); filter([{_, ""} | T]) -> filter(T); From 7c5a9e0e2041a70e09dc9c4101c6bbade062b6e4 Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Thu, 27 Apr 2023 14:20:57 +0200 Subject: [PATCH 18/41] refactor: move the env interpolation function to emqx_schema also added test cases --- apps/emqx/src/emqx_schema.erl | 72 ++++++++++++++++++++----- apps/emqx/src/emqx_tls_lib.erl | 60 ++++++--------------- apps/emqx/test/emqx_schema_tests.erl | 78 ++++++++++++++++++++++++++++ 3 files changed, 153 insertions(+), 57 deletions(-) diff --git a/apps/emqx/src/emqx_schema.erl b/apps/emqx/src/emqx_schema.erl index c56fc9b48..8fdd2557e 100644 --- a/apps/emqx/src/emqx_schema.erl +++ b/apps/emqx/src/emqx_schema.erl @@ -66,7 +66,8 @@ user_lookup_fun_tr/2, validate_alarm_actions/1, non_empty_string/1, - validations/0 + validations/0, + naive_env_interpolation/1 ]). -export([qos/0]). @@ -825,7 +826,7 @@ fields("crl_cache") -> %% same URL. If they had diverging timeout options, it would be %% confusing. [ - {"refresh_interval", + {refresh_interval, sc( duration(), #{ @@ -833,7 +834,7 @@ fields("crl_cache") -> desc => ?DESC("crl_cache_refresh_interval") } )}, - {"http_timeout", + {http_timeout, sc( duration(), #{ @@ -841,7 +842,7 @@ fields("crl_cache") -> desc => ?DESC("crl_cache_refresh_http_timeout") } )}, - {"capacity", + {capacity, sc( pos_integer(), #{ @@ -1358,7 +1359,7 @@ fields("ssl_client_opts") -> client_ssl_opts_schema(#{}); fields("ocsp") -> [ - {"enable_ocsp_stapling", + {enable_ocsp_stapling, sc( boolean(), #{ @@ -1366,7 +1367,7 @@ fields("ocsp") -> desc => ?DESC("server_ssl_opts_schema_enable_ocsp_stapling") } )}, - {"responder_url", + {responder_url, sc( url(), #{ @@ -1374,7 +1375,7 @@ fields("ocsp") -> desc => ?DESC("server_ssl_opts_schema_ocsp_responder_url") } )}, - {"issuer_pem", + {issuer_pem, sc( binary(), #{ @@ -1382,7 +1383,7 @@ fields("ocsp") -> desc => ?DESC("server_ssl_opts_schema_ocsp_issuer_pem") } )}, - {"refresh_interval", + {refresh_interval, sc( duration(), #{ @@ -1390,7 +1391,7 @@ fields("ocsp") -> desc => ?DESC("server_ssl_opts_schema_ocsp_refresh_interval") } )}, - {"refresh_http_timeout", + {refresh_http_timeout, sc( duration(), #{ @@ -2317,12 +2318,12 @@ server_ssl_opts_schema(Defaults, IsRanchListener) -> Field || not IsRanchListener, Field <- [ - {"gc_after_handshake", + {gc_after_handshake, sc(boolean(), #{ default => false, desc => ?DESC(server_ssl_opts_schema_gc_after_handshake) })}, - {"ocsp", + {ocsp, sc( ref("ocsp"), #{ @@ -2330,7 +2331,7 @@ server_ssl_opts_schema(Defaults, IsRanchListener) -> validator => fun ocsp_inner_validator/1 } )}, - {"enable_crl_check", + {enable_crl_check, sc( boolean(), #{ @@ -3106,7 +3107,7 @@ default_listener(ws) -> } }; default_listener(SSLListener) -> - %% The env variable is resolved in emqx_tls_lib + %% The env variable is resolved in emqx_tls_lib by calling naive_env_interpolate CertFile = fun(Name) -> iolist_to_binary("${EMQX_ETC_DIR}/" ++ filename:join(["certs", Name])) end, @@ -3136,3 +3137,48 @@ default_listener(SSLListener) -> } } end. + +%% @doc This function helps to perform a naive string interpolation which +%% only looks at the first segment of the string and tries to replace it. +%% For example +%% "$MY_FILE_PATH" +%% "${MY_FILE_PATH}" +%% "$ENV_VARIABLE/sub/path" +%% "${ENV_VARIABLE}/sub/path" +%% "${ENV_VARIABLE}\sub\path" # windows +%% This function returns undefined if the input is undefined +%% otherwise always return string. +naive_env_interpolation(undefined) -> + undefined; +naive_env_interpolation(Bin) when is_binary(Bin) -> + naive_env_interpolation(unicode:characters_to_list(Bin, utf8)); +naive_env_interpolation("$" ++ Maybe = Original) -> + {Env, Tail} = split_path(Maybe), + case resolve_env(Env) of + {ok, Path} -> + filename:join([Path, Tail]); + error -> + Original + end; +naive_env_interpolation(Other) -> + Other. + +split_path(Path) -> + split_path(Path, []). + +split_path([], Acc) -> + {lists:reverse(Acc), []}; +split_path([Char | Rest], Acc) when Char =:= $/ orelse Char =:= $\\ -> + {lists:reverse(Acc), string:trim(Rest, leading, "/\\")}; +split_path([Char | Rest], Acc) -> + split_path(Rest, [Char | Acc]). + +resolve_env(Name0) -> + Name = string:trim(Name0, both, "{}"), + Value = os:getenv(Name), + case Value =/= false andalso Value =/= "" of + true -> + {ok, Value}; + false -> + error + end. diff --git a/apps/emqx/src/emqx_tls_lib.erl b/apps/emqx/src/emqx_tls_lib.erl index c555059cf..2683d2a9d 100644 --- a/apps/emqx/src/emqx_tls_lib.erl +++ b/apps/emqx/src/emqx_tls_lib.erl @@ -347,8 +347,7 @@ delete_ssl_files(Dir, NewOpts0, OldOpts0) -> delete_old_file(New, Old) when New =:= Old -> ok; delete_old_file(_New, _Old = undefined) -> ok; -delete_old_file(_New, Old0) -> - Old = resolve_cert_path(Old0), +delete_old_file(_New, Old) -> case is_generated_file(Old) andalso filelib:is_regular(Old) andalso file:delete(Old) of ok -> ok; @@ -356,7 +355,7 @@ delete_old_file(_New, Old0) -> false -> ok; {error, Reason} -> - ?SLOG(error, #{msg => "failed_to_delete_ssl_file", file_path => Old0, reason => Reason}) + ?SLOG(error, #{msg => "failed_to_delete_ssl_file", file_path => Old, reason => Reason}) end. ensure_ssl_file(_Dir, _KeyPath, SSL, undefined, _Opts) -> @@ -415,8 +414,7 @@ is_pem(MaybePem) -> %% To make it simple, the file is always overwritten. %% Also a potentially half-written PEM file (e.g. due to power outage) %% can be corrected with an overwrite. -save_pem_file(Dir0, KeyPath, Pem, DryRun) -> - Dir = resolve_cert_path(Dir0), +save_pem_file(Dir, KeyPath, Pem, DryRun) -> Path = pem_file_name(Dir, KeyPath, Pem), case filelib:ensure_dir(Path) of ok when DryRun -> @@ -475,7 +473,7 @@ hex_str(Bin) -> %% @doc Returns 'true' when the file is a valid pem, otherwise {error, Reason}. is_valid_pem_file(Path0) -> - Path = resolve_cert_path(Path0), + Path = resolve_cert_path_for_read(Path0), case file:read_file(Path) of {ok, Pem} -> is_pem(Pem) orelse {error, not_pem}; {error, Reason} -> {error, Reason} @@ -516,11 +514,12 @@ do_drop_invalid_certs([KeyPath | KeyPaths], SSL) -> to_server_opts(Type, Opts) -> Versions = integral_versions(Type, maps:get(versions, Opts, undefined)), Ciphers = integral_ciphers(Versions, maps:get(ciphers, Opts, undefined)), + Path = fun(Key) -> resolve_cert_path_for_read_strict(maps:get(Key, Opts, undefined)) end, filter( maps:to_list(Opts#{ - keyfile => resolve_cert_path_strict(maps:get(keyfile, Opts, undefined)), - certfile => resolve_cert_path_strict(maps:get(certfile, Opts, undefined)), - cacertfile => resolve_cert_path_strict(maps:get(cacertfile, Opts, undefined)), + keyfile => Path(keyfile), + certfile => Path(certfile), + cacertfile => Path(cacertfile), ciphers => Ciphers, versions => Versions }) @@ -538,11 +537,12 @@ to_client_opts(Opts) -> to_client_opts(Type, Opts) -> GetD = fun(Key, Default) -> fuzzy_map_get(Key, Opts, Default) end, Get = fun(Key) -> GetD(Key, undefined) end, + Path = fun(Key) -> resolve_cert_path_for_read_strict(Get(Key)) end, case GetD(enable, false) of true -> - KeyFile = resolve_cert_path_strict(Get(keyfile)), - CertFile = resolve_cert_path_strict(Get(certfile)), - CAFile = resolve_cert_path_strict(Get(cacertfile)), + KeyFile = Path(keyfile), + CertFile = Path(certfile), + CAFile = Path(cacertfile), Verify = GetD(verify, verify_none), SNI = ensure_sni(Get(server_name_indication)), Versions = integral_versions(Type, Get(versions)), @@ -564,8 +564,8 @@ to_client_opts(Type, Opts) -> [] end. -resolve_cert_path_strict(Path) -> - case resolve_cert_path(Path) of +resolve_cert_path_for_read_strict(Path) -> + case resolve_cert_path_for_read(Path) of undefined -> undefined; ResolvedPath -> @@ -586,36 +586,8 @@ resolve_cert_path_strict(Path) -> end end. -resolve_cert_path(undefined) -> - undefined; -resolve_cert_path(Path) -> - case ensure_str(Path) of - "$" ++ Maybe -> - naive_env_resolver(Maybe); - Other -> - Other - end. - -%% resolves a file path like "ENV_VARIABLE/sub/path" or "{ENV_VARIABLE}/sub/path" -%% in windows, it could be "ENV_VARIABLE/sub\path" or "{ENV_VARIABLE}/sub\path" -naive_env_resolver(Maybe) -> - case string:split(Maybe, "/") of - [_] -> - Maybe; - [Env, SubPath] -> - case os:getenv(trim_env_name(Env)) of - false -> - SubPath; - "" -> - SubPath; - EnvValue -> - filename:join(EnvValue, SubPath) - end - end. - -%% delete the first and last curly braces -trim_env_name(Env) -> - string:trim(Env, both, "{}"). +resolve_cert_path_for_read(Path) -> + emqx_schema:naive_env_interpolation(Path). filter([]) -> []; filter([{_, undefined} | T]) -> filter(T); diff --git a/apps/emqx/test/emqx_schema_tests.erl b/apps/emqx/test/emqx_schema_tests.erl index 5176f4fad..b4bb85a8a 100644 --- a/apps/emqx/test/emqx_schema_tests.erl +++ b/apps/emqx/test/emqx_schema_tests.erl @@ -513,3 +513,81 @@ url_type_test_() -> typerefl:from_string(emqx_schema:url(), <<"">>) ) ]. + +env_test_() -> + Do = fun emqx_schema:naive_env_interpolation/1, + [ + {"undefined", fun() -> ?assertEqual(undefined, Do(undefined)) end}, + {"full env abs path", + with_env_fn( + "MY_FILE", + "/path/to/my/file", + fun() -> ?assertEqual("/path/to/my/file", Do("$MY_FILE")) end + )}, + {"full env relative path", + with_env_fn( + "MY_FILE", + "path/to/my/file", + fun() -> ?assertEqual("path/to/my/file", Do("${MY_FILE}")) end + )}, + %% we can not test windows style file join though + {"windows style", + with_env_fn( + "MY_FILE", + "path\\to\\my\\file", + fun() -> ?assertEqual("path\\to\\my\\file", Do("$MY_FILE")) end + )}, + {"dir no {}", + with_env_fn( + "MY_DIR", + "/mydir", + fun() -> ?assertEqual("/mydir/foobar", Do(<<"$MY_DIR/foobar">>)) end + )}, + {"dir with {}", + with_env_fn( + "MY_DIR", + "/mydir", + fun() -> ?assertEqual("/mydir/foobar", Do(<<"${MY_DIR}/foobar">>)) end + )}, + %% a trailing / should not cause the sub path to become absolute + {"env dir with trailing /", + with_env_fn( + "MY_DIR", + "/mydir//", + fun() -> ?assertEqual("/mydir/foobar", Do(<<"${MY_DIR}/foobar">>)) end + )}, + {"string dir with doulbe /", + with_env_fn( + "MY_DIR", + "/mydir/", + fun() -> ?assertEqual("/mydir/foobar", Do(<<"${MY_DIR}//foobar">>)) end + )}, + {"env not found", + with_env_fn( + "MY_DIR", + "/mydir/", + fun() -> ?assertEqual("${MY_DIR2}//foobar", Do(<<"${MY_DIR2}//foobar">>)) end + )} + ]. + +with_env_fn(Name, Value, F) -> + fun() -> + with_envs(F, [{Name, Value}]) + end. + +with_envs(Fun, Envs) -> + with_envs(Fun, [], Envs). + +with_envs(Fun, Args, [{_Name, _Value} | _] = Envs) -> + set_envs(Envs), + try + apply(Fun, Args) + after + unset_envs(Envs) + end. + +set_envs([{_Name, _Value} | _] = Envs) -> + lists:map(fun({Name, Value}) -> os:putenv(Name, Value) end, Envs). + +unset_envs([{_Name, _Value} | _] = Envs) -> + lists:map(fun({Name, _}) -> os:unsetenv(Name) end, Envs). From 5acf0e281eaf93fecbe80ad4ad32d5d0090ece31 Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Thu, 27 Apr 2023 14:21:54 +0200 Subject: [PATCH 19/41] refactor: delete default authz config from emqx.conf --- apps/emqx_authz/etc/emqx_authz.conf | 10 ---------- apps/emqx_authz/src/emqx_authz_file.erl | 3 ++- apps/emqx_authz/src/emqx_authz_schema.erl | 9 ++++++++- 3 files changed, 10 insertions(+), 12 deletions(-) diff --git a/apps/emqx_authz/etc/emqx_authz.conf b/apps/emqx_authz/etc/emqx_authz.conf index 3bdc180c5..167b12b3f 100644 --- a/apps/emqx_authz/etc/emqx_authz.conf +++ b/apps/emqx_authz/etc/emqx_authz.conf @@ -2,14 +2,4 @@ authorization { deny_action = ignore no_match = allow cache = { enable = true } - sources = [ - { - type = file - enable = true - # This file is immutable to EMQX. - # Once new rules are created from dashboard UI or HTTP API, - # the file 'data/authz/acl.conf' is used instead of this one - path = "{{ platform_etc_dir }}/acl.conf" - } - ] } diff --git a/apps/emqx_authz/src/emqx_authz_file.erl b/apps/emqx_authz/src/emqx_authz_file.erl index ede4a9582..63e7be781 100644 --- a/apps/emqx_authz/src/emqx_authz_file.erl +++ b/apps/emqx_authz/src/emqx_authz_file.erl @@ -38,7 +38,8 @@ description() -> "AuthZ with static rules". -create(#{path := Path} = Source) -> +create(#{path := Path0} = Source) -> + Path = emqx_schema:naive_env_interpolation(Path0), Rules = case file:consult(Path) of {ok, Terms} -> diff --git a/apps/emqx_authz/src/emqx_authz_schema.erl b/apps/emqx_authz/src/emqx_authz_schema.erl index 39bbcc360..280b9b16c 100644 --- a/apps/emqx_authz/src/emqx_authz_schema.erl +++ b/apps/emqx_authz/src/emqx_authz_schema.erl @@ -491,7 +491,7 @@ authz_fields() -> ?HOCON( ?ARRAY(?UNION(UnionMemberSelector)), #{ - default => [], + default => [default_authz()], desc => ?DESC(sources), %% doc_lift is force a root level reference instead of nesting sub-structs extra => #{doc_lift => true}, @@ -501,3 +501,10 @@ authz_fields() -> } )} ]. + +default_authz() -> + #{ + <<"type">> => <<"file">>, + <<"enable">> => true, + <<"path">> => <<"${EMQX_ETC_DIR}/acl.conf">> + }. From e0dc5645ab408476350a7c86a2dc2e38094c2e4e Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Thu, 27 Apr 2023 16:01:14 +0200 Subject: [PATCH 20/41] fix(config): always fill defautls for all roots prior to this commit, if a root existed in config files it skips populating default values in raw config, this made impossible to add default values for authz sources. --- apps/emqx/src/emqx_config.erl | 67 ++++++++++++++++++----------------- apps/emqx/src/emqx_schema.erl | 1 + 2 files changed, 36 insertions(+), 32 deletions(-) diff --git a/apps/emqx/src/emqx_config.erl b/apps/emqx/src/emqx_config.erl index 9561263ca..c117bffba 100644 --- a/apps/emqx/src/emqx_config.erl +++ b/apps/emqx/src/emqx_config.erl @@ -35,7 +35,6 @@ save_to_config_map/2, save_to_override_conf/3 ]). --export([raw_conf_with_default/4]). -export([merge_envs/2]). -export([ @@ -329,7 +328,7 @@ init_load(SchemaMod, ConfFiles) -> -spec init_load(module(), [string()] | binary() | hocon:config()) -> ok. init_load(SchemaMod, Conf, Opts) when is_list(Conf) orelse is_binary(Conf) -> HasDeprecatedFile = has_deprecated_file(), - RawConf = parse_hocon(HasDeprecatedFile, Conf), + RawConf = load_config_files(HasDeprecatedFile, Conf), init_load(HasDeprecatedFile, SchemaMod, RawConf, Opts). init_load(true, SchemaMod, RawConf, Opts) when is_map(RawConf) -> @@ -339,18 +338,16 @@ init_load(true, SchemaMod, RawConf, Opts) when is_map(RawConf) -> RawConfWithEnvs = merge_envs(SchemaMod, RawConf), Overrides = read_override_confs(), RawConfWithOverrides = hocon:deep_merge(RawConfWithEnvs, Overrides), - RootNames = get_root_names(), - RawConfAll = raw_conf_with_default(SchemaMod, RootNames, RawConfWithOverrides, Opts), + RawConfAll = maybe_fill_defaults(SchemaMod, RawConfWithOverrides, Opts), %% check configs against the schema {AppEnvs, CheckedConf} = check_config(SchemaMod, RawConfAll, #{}), save_to_app_env(AppEnvs), ok = save_to_config_map(CheckedConf, RawConfAll); init_load(false, SchemaMod, RawConf, Opts) when is_map(RawConf) -> ok = save_schema_mod_and_names(SchemaMod), - RootNames = get_root_names(), %% Merge environment variable overrides on top RawConfWithEnvs = merge_envs(SchemaMod, RawConf), - RawConfAll = raw_conf_with_default(SchemaMod, RootNames, RawConfWithEnvs, Opts), + RawConfAll = maybe_fill_defaults(SchemaMod, RawConfWithEnvs, Opts), %% check configs against the schema {AppEnvs, CheckedConf} = check_config(SchemaMod, RawConfAll, #{}), save_to_app_env(AppEnvs), @@ -363,47 +360,53 @@ read_override_confs() -> hocon:deep_merge(ClusterOverrides, LocalOverrides). %% keep the raw and non-raw conf has the same keys to make update raw conf easier. -raw_conf_with_default(SchemaMod, RootNames, RawConf, #{raw_with_default := true}) -> - Fun = fun(Name, Acc) -> - case maps:is_key(Name, RawConf) of - true -> - Acc; - false -> - case lists:keyfind(Name, 1, hocon_schema:roots(SchemaMod)) of - false -> - Acc; - {_, {_, Schema}} -> - Acc#{Name => schema_default(Schema)} - end - end - end, - RawDefault = lists:foldl(Fun, #{}, RootNames), - maps:merge(RawConf, fill_defaults(SchemaMod, RawDefault, #{})); -raw_conf_with_default(_SchemaMod, _RootNames, RawConf, _Opts) -> +maybe_fill_defaults(SchemaMod, RawConf0, #{raw_with_default := true}) -> + RootSchemas = hocon_schema:roots(SchemaMod), + %% the roots which are missing from the loaded configs + MissingRoots = lists:filtermap( + fun({BinName, Sc}) -> + case maps:is_key(BinName, RawConf0) of + true -> false; + false -> {true, Sc} + end + end, + RootSchemas + ), + RawConf = lists:foldl( + fun({RootName, Schema}, Acc) -> + Acc#{bin(RootName) => seed_default(Schema)} + end, + RawConf0, + MissingRoots + ), + fill_defaults(RawConf); +maybe_fill_defaults(_SchemaMod, RawConf, _Opts) -> RawConf. -schema_default(Schema) -> - case hocon_schema:field_schema(Schema, type) of - ?ARRAY(_) -> - []; - _ -> - #{} +%% if a root is not found in the raw conf, fill it with default values. +seed_default(Schema) -> + case hocon_schema:field_schema(Schema, default) of + undefined -> + %% so far all roots without a default value are objects + #{}; + Value -> + Value end. -parse_hocon(HasDeprecatedFile, Conf) -> +load_config_files(HasDeprecatedFile, Conf) -> IncDirs = include_dirs(), case do_parse_hocon(HasDeprecatedFile, Conf, IncDirs) of {ok, HoconMap} -> HoconMap; {error, Reason} -> ?SLOG(error, #{ - msg => "failed_to_load_hocon_file", + msg => "failed_to_load_config_file", reason => Reason, pwd => file:get_cwd(), include_dirs => IncDirs, config_file => Conf }), - error(failed_to_load_hocon_file) + error(failed_to_load_config_file) end. do_parse_hocon(true, Conf, IncDirs) -> diff --git a/apps/emqx/src/emqx_schema.erl b/apps/emqx/src/emqx_schema.erl index 8fdd2557e..db3046bcf 100644 --- a/apps/emqx/src/emqx_schema.erl +++ b/apps/emqx/src/emqx_schema.erl @@ -2794,6 +2794,7 @@ authentication(Which) -> hoconsc:mk(Type, #{ desc => Desc, converter => fun ensure_array/2, + default => [], importance => Importance }). From a1551213c892325b076b62a99085da7c05300e1b Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Thu, 27 Apr 2023 16:23:32 +0200 Subject: [PATCH 21/41] test: EMQX_ETC_DIR for test is app's etc dir --- apps/emqx/src/emqx_schema.erl | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/apps/emqx/src/emqx_schema.erl b/apps/emqx/src/emqx_schema.erl index db3046bcf..c1fe656da 100644 --- a/apps/emqx/src/emqx_schema.erl +++ b/apps/emqx/src/emqx_schema.erl @@ -3181,5 +3181,13 @@ resolve_env(Name0) -> true -> {ok, Value}; false -> - error + special_env(Name) end. + +-ifdef(TEST). +%% when running tests, we need to mock the env variables +special_env("EMQX_ETC_DIR") -> + {ok, filename:join([code:lib_dir(emqx), etc])}. +-else. +special_env(_Name) -> error. +-endif. From 41f13330ba42825f8494c8cd7f896c8ee829616f Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Thu, 27 Apr 2023 16:29:25 +0200 Subject: [PATCH 22/41] refactor: export EMQX_LOG_DIR --- apps/emqx_conf/src/emqx_conf_schema.erl | 2 +- bin/emqx | 10 +++++----- bin/node_dump | 10 +++++----- rel/emqx_vars | 3 ++- 4 files changed, 13 insertions(+), 12 deletions(-) diff --git a/apps/emqx_conf/src/emqx_conf_schema.erl b/apps/emqx_conf/src/emqx_conf_schema.erl index 39dce0b71..754b3c3fc 100644 --- a/apps/emqx_conf/src/emqx_conf_schema.erl +++ b/apps/emqx_conf/src/emqx_conf_schema.erl @@ -1199,7 +1199,7 @@ log_handler_common_confs(Enable) -> ]. crash_dump_file_default() -> - case os:getenv("RUNNER_LOG_DIR") of + case os:getenv("EMQX_LOG_DIR") of false -> %% testing, or running emqx app as deps <<"log/erl_crash.dump">>; diff --git a/bin/emqx b/bin/emqx index fc0124b96..a181ae6a9 100755 --- a/bin/emqx +++ b/bin/emqx @@ -304,7 +304,7 @@ if [ "$ES" -ne 0 ]; then fi # Make sure log directory exists -mkdir -p "$RUNNER_LOG_DIR" +mkdir -p "$EMQX_LOG_DIR" # turn off debug as this is static set +x @@ -757,7 +757,7 @@ generate_config() { local node_name="$2" ## Delete the *.siz files first or it can't start after ## changing the config 'log.rotation.size' - rm -f "${RUNNER_LOG_DIR}"/*.siz + rm -f "${EMQX_LOG_DIR}"/*.siz ## timestamp for each generation local NOW_TIME @@ -979,7 +979,7 @@ diagnose_boot_failure_and_die() { local ps_line ps_line="$(find_emqx_process)" if [ -z "$ps_line" ]; then - echo "Find more information in the latest log file: ${RUNNER_LOG_DIR}/erlang.log.*" + echo "Find more information in the latest log file: ${EMQX_LOG_DIR}/erlang.log.*" exit 1 fi if ! relx_nodetool "ping" > /dev/null; then @@ -990,7 +990,7 @@ diagnose_boot_failure_and_die() { fi if ! relx_nodetool 'eval' 'true = emqx:is_running()' > /dev/null; then logerr "$NAME node is started, but failed to complete the boot sequence in time." - echo "Please collect the logs in ${RUNNER_LOG_DIR} and report a bug to EMQX team at https://github.com/emqx/emqx/issues/new/choose" + echo "Please collect the logs in ${EMQX_LOG_DIR} and report a bug to EMQX team at https://github.com/emqx/emqx/issues/new/choose" pipe_shutdown exit 3 fi @@ -1065,7 +1065,7 @@ case "${COMMAND}" in mkdir -p "$PIPE_DIR" - "$BINDIR/run_erl" -daemon "$PIPE_DIR" "$RUNNER_LOG_DIR" \ + "$BINDIR/run_erl" -daemon "$PIPE_DIR" "$EMQX_LOG_DIR" \ "$(relx_start_command)" WAIT_TIME=${EMQX_WAIT_FOR_START:-120} diff --git a/bin/node_dump b/bin/node_dump index 1c4df08b5..60c995885 100755 --- a/bin/node_dump +++ b/bin/node_dump @@ -10,10 +10,10 @@ echo "Running node dump in ${RUNNER_ROOT_DIR}" cd "${RUNNER_ROOT_DIR}" -DUMP="$RUNNER_LOG_DIR/node_dump_$(date +"%Y%m%d_%H%M%S").tar.gz" -CONF_DUMP="$RUNNER_LOG_DIR/conf.dump" -LICENSE_INFO="$RUNNER_LOG_DIR/license_info.txt" -SYSINFO="$RUNNER_LOG_DIR/sysinfo.txt" +DUMP="$EMQX_LOG_DIR/node_dump_$(date +"%Y%m%d_%H%M%S").tar.gz" +CONF_DUMP="$EMQX_LOG_DIR/conf.dump" +LICENSE_INFO="$EMQX_LOG_DIR/license_info.txt" +SYSINFO="$EMQX_LOG_DIR/sysinfo.txt" LOG_MAX_AGE_DAYS=3 @@ -74,7 +74,7 @@ done # Pack files { - find "$RUNNER_LOG_DIR" -mtime -"${LOG_MAX_AGE_DAYS}" \( -name '*.log.*' -or -name 'run_erl.log*' \) + find "$EMQX_LOG_DIR" -mtime -"${LOG_MAX_AGE_DAYS}" \( -name '*.log.*' -or -name 'run_erl.log*' \) echo "${SYSINFO}" echo "${CONF_DUMP}" echo "${LICENSE_INFO}" diff --git a/rel/emqx_vars b/rel/emqx_vars index e3965d40c..f37968f1f 100644 --- a/rel/emqx_vars +++ b/rel/emqx_vars @@ -11,7 +11,8 @@ RUNNER_LIB_DIR="{{ runner_lib_dir }}" IS_ELIXIR="${IS_ELIXIR:-{{ is_elixir }}}" ## Allow users to pre-set `EMQX_LOG_DIR` because it only affects boot commands like `start` and `console`, ## but not other commands such as `ping` and `ctl`. -RUNNER_LOG_DIR="${EMQX_LOG_DIR:-${RUNNER_LOG_DIR:-{{ runner_log_dir }}}}" +## RUNNER_LOG_DIR is kept for backward compatibility. +export EMQX_LOG_DIR="${EMQX_LOG_DIR:-${RUNNER_LOG_DIR:-{{ runner_log_dir }}}}" EMQX_ETC_DIR="{{ emqx_etc_dir }}" RUNNER_USER="{{ runner_user }}" SCHEMA_MOD="{{ emqx_schema_mod }}" From 4d705817d85ce343dd8ec25fe5075090fba3cea8 Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Thu, 27 Apr 2023 16:35:39 +0200 Subject: [PATCH 23/41] refactor(log): move default values to schema --- apps/emqx/src/emqx_schema.erl | 7 ++- apps/emqx/test/emqx_common_test_helpers.erl | 3 +- apps/emqx/test/emqx_listeners_SUITE.erl | 3 +- apps/emqx_conf/etc/emqx_conf.conf | 7 --- apps/emqx_conf/src/emqx_conf_schema.erl | 63 +++++++++++++++++---- bin/emqx | 14 +++-- deploy/docker/docker-entrypoint.sh | 6 +- deploy/packages/emqx.service | 4 +- mix.exs | 2 - rebar.config.erl | 2 - rel/i18n/emqx_conf_schema.hocon | 9 ++- 11 files changed, 78 insertions(+), 42 deletions(-) diff --git a/apps/emqx/src/emqx_schema.erl b/apps/emqx/src/emqx_schema.erl index c1fe656da..418a2db56 100644 --- a/apps/emqx/src/emqx_schema.erl +++ b/apps/emqx/src/emqx_schema.erl @@ -3187,7 +3187,12 @@ resolve_env(Name0) -> -ifdef(TEST). %% when running tests, we need to mock the env variables special_env("EMQX_ETC_DIR") -> - {ok, filename:join([code:lib_dir(emqx), etc])}. + {ok, filename:join([code:lib_dir(emqx), etc])}; +special_env("EMQX_LOG_DIR") -> + {ok, "log"}; +special_env(_Name) -> + %% only in tests + error. -else. special_env(_Name) -> error. -endif. diff --git a/apps/emqx/test/emqx_common_test_helpers.erl b/apps/emqx/test/emqx_common_test_helpers.erl index e9ddc61a8..c6b04eed1 100644 --- a/apps/emqx/test/emqx_common_test_helpers.erl +++ b/apps/emqx/test/emqx_common_test_helpers.erl @@ -271,8 +271,7 @@ mustache_vars(App, Opts) -> ExtraMustacheVars = maps:get(extra_mustache_vars, Opts, #{}), Defaults = #{ platform_data_dir => app_path(App, "data"), - platform_etc_dir => app_path(App, "etc"), - platform_log_dir => app_path(App, "log") + platform_etc_dir => app_path(App, "etc") }, maps:merge(Defaults, ExtraMustacheVars). diff --git a/apps/emqx/test/emqx_listeners_SUITE.erl b/apps/emqx/test/emqx_listeners_SUITE.erl index 107f3d4e7..8d965e8dd 100644 --- a/apps/emqx/test/emqx_listeners_SUITE.erl +++ b/apps/emqx/test/emqx_listeners_SUITE.erl @@ -266,8 +266,7 @@ render_config_file() -> mustache_vars() -> [ {platform_data_dir, local_path(["data"])}, - {platform_etc_dir, local_path(["etc"])}, - {platform_log_dir, local_path(["log"])} + {platform_etc_dir, local_path(["etc"])} ]. generate_config() -> diff --git a/apps/emqx_conf/etc/emqx_conf.conf b/apps/emqx_conf/etc/emqx_conf.conf index 4c03c10c6..76e3c0805 100644 --- a/apps/emqx_conf/etc/emqx_conf.conf +++ b/apps/emqx_conf/etc/emqx_conf.conf @@ -15,13 +15,6 @@ node { data_dir = "{{ platform_data_dir }}" } -log { - file_handlers.default { - level = warning - file = "{{ platform_log_dir }}/emqx.log" - } -} - cluster { name = emqxcl discovery_strategy = manual diff --git a/apps/emqx_conf/src/emqx_conf_schema.erl b/apps/emqx_conf/src/emqx_conf_schema.erl index 754b3c3fc..ee718b3a1 100644 --- a/apps/emqx_conf/src/emqx_conf_schema.erl +++ b/apps/emqx_conf/src/emqx_conf_schema.erl @@ -93,7 +93,10 @@ roots() -> {"log", sc( ?R_REF("log"), - #{translate_to => ["kernel"]} + #{ + translate_to => ["kernel"], + importance => ?IMPORTANCE_HIGH + } )}, {"rpc", sc( @@ -862,15 +865,25 @@ fields("rpc") -> ]; fields("log") -> [ - {"console_handler", ?R_REF("console_handler")}, + {"console_handler", + sc( + ?R_REF("console_handler"), + #{importance => ?IMPORTANCE_HIGH} + )}, {"file_handlers", sc( map(name, ?R_REF("log_file_handler")), - #{desc => ?DESC("log_file_handlers")} + #{ + desc => ?DESC("log_file_handlers"), + %% because file_handlers is a map + %% so there has to be a default value in order to populate the raw configs + default => #{<<"default">> => #{<<"level">> => <<"warning">>}}, + importance => ?IMPORTANCE_HIGH + } )} ]; fields("console_handler") -> - log_handler_common_confs(false); + log_handler_common_confs(console); fields("log_file_handler") -> [ {"file", @@ -878,6 +891,8 @@ fields("log_file_handler") -> file(), #{ desc => ?DESC("log_file_handler_file"), + default => <<"${EMQX_LOG_DIR}/emqx.log">>, + converter => fun emqx_schema:naive_env_interpolation/1, validator => fun validate_file_location/1 } )}, @@ -891,10 +906,11 @@ fields("log_file_handler") -> hoconsc:union([infinity, emqx_schema:bytesize()]), #{ default => <<"50MB">>, - desc => ?DESC("log_file_handler_max_size") + desc => ?DESC("log_file_handler_max_size"), + importance => ?IMPORTANCE_MEDIUM } )} - ] ++ log_handler_common_confs(true); + ] ++ log_handler_common_confs(file); fields("log_rotation") -> [ {"enable", @@ -1103,14 +1119,33 @@ tr_logger_level(Conf) -> tr_logger_handlers(Conf) -> emqx_config_logger:tr_handlers(Conf). -log_handler_common_confs(Enable) -> +log_handler_common_confs(Handler) -> + lists:map( + fun + ({_Name, #{importance := _}} = F) -> F; + ({Name, Sc}) -> {Name, Sc#{importance => ?IMPORTANCE_LOW}} + end, + do_log_handler_common_confs(Handler) + ). +do_log_handler_common_confs(Handler) -> + %% we rarely support dynamic defaults like this + %% for this one, we have build-time defualut the same as runtime default + %% so it's less tricky + EnableValues = + case Handler of + console -> ["console", "both"]; + file -> ["file", "both", "", false] + end, + EnvValue = os:getenv("EMQX_DEFAULT_LOG_HANDLER"), + Enable = lists:member(EnvValue, EnableValues), [ {"enable", sc( boolean(), #{ default => Enable, - desc => ?DESC("common_handler_enable") + desc => ?DESC("common_handler_enable"), + importance => ?IMPORTANCE_LOW } )}, {"level", @@ -1127,7 +1162,8 @@ log_handler_common_confs(Enable) -> #{ default => <<"system">>, desc => ?DESC("common_handler_time_offset"), - validator => fun validate_time_offset/1 + validator => fun validate_time_offset/1, + importance => ?IMPORTANCE_LOW } )}, {"chars_limit", @@ -1135,7 +1171,8 @@ log_handler_common_confs(Enable) -> hoconsc:union([unlimited, range(100, inf)]), #{ default => unlimited, - desc => ?DESC("common_handler_chars_limit") + desc => ?DESC("common_handler_chars_limit"), + importance => ?IMPORTANCE_LOW } )}, {"formatter", @@ -1143,7 +1180,8 @@ log_handler_common_confs(Enable) -> hoconsc:enum([text, json]), #{ default => text, - desc => ?DESC("common_handler_formatter") + desc => ?DESC("common_handler_formatter"), + importance => ?IMPORTANCE_MEDIUM } )}, {"single_line", @@ -1151,7 +1189,8 @@ log_handler_common_confs(Enable) -> boolean(), #{ default => true, - desc => ?DESC("common_handler_single_line") + desc => ?DESC("common_handler_single_line"), + importance => ?IMPORTANCE_LOW } )}, {"sync_mode_qlen", diff --git a/bin/emqx b/bin/emqx index a181ae6a9..dd0b0791e 100755 --- a/bin/emqx +++ b/bin/emqx @@ -861,7 +861,13 @@ wait_until_return_val() { done } -# backward compatible with 4.x +# First, there is EMQX_DEFAULT_LOG_HANDLER which can control the default values +# to be used when generating configs. +# It's set in docker entrypoint and in systemd service file. +# +# To be backward compatible with 4.x and v5.0.0 ~ v5.0.24/e5.0.2: +# if EMQX_LOG__TO is set, we try to enable handlers from environment variables. +# i.e. it overrides the default value set in EMQX_DEFAULT_LOG_HANDLER tr_log_to_env() { local log_to=${EMQX_LOG__TO:-undefined} # unset because it's unknown to 5.0 @@ -893,13 +899,11 @@ tr_log_to_env() { maybe_log_to_console() { if [ "${EMQX_LOG__TO:-}" = 'default' ]; then - # want to use config file defaults, do nothing + # want to use defaults, do nothing unset EMQX_LOG__TO else tr_log_to_env - # ensure defaults - export EMQX_LOG__CONSOLE_HANDLER__ENABLE="${EMQX_LOG__CONSOLE_HANDLER__ENABLE:-true}" - export EMQX_LOG__FILE_HANDLERS__DEFAULT__ENABLE="${EMQX_LOG__FILE_HANDLERS__DEFAULT__ENABLE:-false}" + export EMQX_DEFAULT_LOG_HANDLER=${EMQX_DEFAULT_LOG_HANDLER:-console} fi } diff --git a/deploy/docker/docker-entrypoint.sh b/deploy/docker/docker-entrypoint.sh index 1824e1ee0..056f0675f 100755 --- a/deploy/docker/docker-entrypoint.sh +++ b/deploy/docker/docker-entrypoint.sh @@ -1,9 +1,7 @@ #!/usr/bin/env bash -## EMQ docker image start script -# Huang Rui -# EMQX Team -## Shell setting +## EMQ docker image start script + if [[ -n "$DEBUG" ]]; then set -ex else diff --git a/deploy/packages/emqx.service b/deploy/packages/emqx.service index d826e358b..2dbe550bc 100644 --- a/deploy/packages/emqx.service +++ b/deploy/packages/emqx.service @@ -10,8 +10,8 @@ Group=emqx Type=simple Environment=HOME=/var/lib/emqx -# Enable logging to file -Environment=EMQX_LOG__TO=default +# log to file by default (if no log handler config) +Environment=EMQX_DEFAULT_LOG_HANDLER=file # Start 'foreground' but not 'start' (daemon) mode. # Because systemd monitor/restarts 'simple' services diff --git a/mix.exs b/mix.exs index 93f7417c9..41a35e0e7 100644 --- a/mix.exs +++ b/mix.exs @@ -665,7 +665,6 @@ defmodule EMQXUmbrella.MixProject do emqx_default_erlang_cookie: default_cookie(), platform_data_dir: "data", platform_etc_dir: "etc", - platform_log_dir: "log", platform_plugins_dir: "plugins", runner_bin_dir: "$RUNNER_ROOT_DIR/bin", emqx_etc_dir: "$RUNNER_ROOT_DIR/etc", @@ -688,7 +687,6 @@ defmodule EMQXUmbrella.MixProject do emqx_default_erlang_cookie: default_cookie(), platform_data_dir: "/var/lib/emqx", platform_etc_dir: "/etc/emqx", - platform_log_dir: "/var/log/emqx", platform_plugins_dir: "/var/lib/emqx/plugins", runner_bin_dir: "/usr/bin", emqx_etc_dir: "/etc/emqx", diff --git a/rebar.config.erl b/rebar.config.erl index 7c00622c2..9ef6c5e00 100644 --- a/rebar.config.erl +++ b/rebar.config.erl @@ -335,7 +335,6 @@ overlay_vars_pkg(bin) -> [ {platform_data_dir, "data"}, {platform_etc_dir, "etc"}, - {platform_log_dir, "log"}, {platform_plugins_dir, "plugins"}, {runner_bin_dir, "$RUNNER_ROOT_DIR/bin"}, {emqx_etc_dir, "$RUNNER_ROOT_DIR/etc"}, @@ -348,7 +347,6 @@ overlay_vars_pkg(pkg) -> [ {platform_data_dir, "/var/lib/emqx"}, {platform_etc_dir, "/etc/emqx"}, - {platform_log_dir, "/var/log/emqx"}, {platform_plugins_dir, "/var/lib/emqx/plugins"}, {runner_bin_dir, "/usr/bin"}, {emqx_etc_dir, "/etc/emqx"}, diff --git a/rel/i18n/emqx_conf_schema.hocon b/rel/i18n/emqx_conf_schema.hocon index b252353f8..9cff400e6 100644 --- a/rel/i18n/emqx_conf_schema.hocon +++ b/rel/i18n/emqx_conf_schema.hocon @@ -1369,9 +1369,12 @@ this is where to look.""" desc_log { desc { - en: """EMQX logging supports multiple sinks for the log events. -Each sink is represented by a _log handler_, which can be configured independently.""" - zh: """EMQX 日志记录支持日志事件的多个接收器。 每个接收器由一个_log handler_表示,可以独立配置。""" + en: """EMQX supports multiple log handlers, one console handler and multiple file handlers. +EMQX by default logs to console when running in docker or in console/foreground mode, +otherwise it logs to file $EMQX_LOG_DIR/emqx.log. +For advanced configuration, you can find more parameters in this section.""" + zh: """EMQX 支持同时多个日志输出,一个控制台输出,和多个文件输出。 +默认情况下,EMQX 运行在容器中,或者在 'console' 或 'foreground' 模式下运行时,会输出到 控制台,否则输出到文件。""" } label { en: "Log" From dbf9bae7dcbc04726791f72d00d5fa098c589413 Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Fri, 28 Apr 2023 09:09:57 +0200 Subject: [PATCH 24/41] test(statsd): fix raw config default value --- apps/emqx_statsd/test/emqx_statsd_SUITE.erl | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/apps/emqx_statsd/test/emqx_statsd_SUITE.erl b/apps/emqx_statsd/test/emqx_statsd_SUITE.erl index b5669e4b9..1f7c4688e 100644 --- a/apps/emqx_statsd/test/emqx_statsd_SUITE.erl +++ b/apps/emqx_statsd/test/emqx_statsd_SUITE.erl @@ -92,8 +92,10 @@ t_server_validator(_) -> ok = emqx_common_test_helpers:load_config(emqx_statsd_schema, ?DEFAULT_CONF, #{ raw_with_default => true }), - undefined = emqx_conf:get_raw([statsd, server], undefined), - ?assertMatch("127.0.0.1:8125", emqx_conf:get([statsd, server])), + DefaultServer = default_server(), + ?assertEqual(DefaultServer, emqx_conf:get_raw([statsd, server])), + DefaultServerStr = binary_to_list(DefaultServer), + ?assertEqual(DefaultServerStr, emqx_conf:get([statsd, server])), %% recover ok = emqx_common_test_helpers:load_config(emqx_statsd_schema, ?BASE_CONF, #{ raw_with_default => true @@ -204,3 +206,7 @@ request(Method, Body) -> {ok, _Status, _} -> error end. + +default_server() -> + {server, Schema} = lists:keyfind(server, 1, emqx_statsd_schema:fields("statsd")), + hocon_schema:field_schema(Schema, default). From b3c0abf4946660c3b58517b062c251a2caf829c4 Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Fri, 28 Apr 2023 12:45:16 +0200 Subject: [PATCH 25/41] test(emqx_management): fix listeners api test cases --- .../test/emqx_mgmt_api_listeners_SUITE.erl | 60 +++++++++++++------ 1 file changed, 42 insertions(+), 18 deletions(-) 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 62f689a84..e73f67f95 100644 --- a/apps/emqx_management/test/emqx_mgmt_api_listeners_SUITE.erl +++ b/apps/emqx_management/test/emqx_mgmt_api_listeners_SUITE.erl @@ -20,18 +20,27 @@ -include_lib("eunit/include/eunit.hrl"). --define(PORT, (20000 + ?LINE)). +-define(PORT(Base), (Base + ?LINE)). +-define(PORT, ?PORT(20000)). all() -> emqx_common_test_helpers:all(?MODULE). init_per_suite(Config) -> + %% we have to materialize the config file with default values for this test suite + %% because we want to test the deletion of non-existing listener + %% if there is no config file, the such deletion would result in a deletion + %% of the default listener. + Name = atom_to_list(?MODULE) ++ "-default-listeners", + TmpConfFullPath = inject_tmp_config_content(Name, default_listeners_hcon_text()), emqx_mgmt_api_test_util:init_suite([emqx_conf]), - Config. + [{injected_conf_file, TmpConfFullPath} | Config]. -end_per_suite(_) -> +end_per_suite(Config) -> emqx_conf:remove([listeners, tcp, new], #{override_to => cluster}), emqx_conf:remove([listeners, tcp, new1], #{override_to => local}), + {_, File} = lists:keyfind(injected_conf_file, 1, Config), + ok = file:delete(File), emqx_mgmt_api_test_util:end_suite([emqx_conf]). init_per_testcase(Case, Config) -> @@ -52,17 +61,12 @@ end_per_testcase(Case, Config) -> t_max_connection_default({init, Config}) -> emqx_mgmt_api_test_util:end_suite([emqx_conf]), - Etc = filename:join(["etc", "emqx.conf.all"]), - TmpConfName = atom_to_list(?FUNCTION_NAME) ++ ".conf", - Inc = filename:join(["etc", TmpConfName]), - ConfFile = emqx_common_test_helpers:app_path(emqx_conf, Etc), - IncFile = emqx_common_test_helpers:app_path(emqx_conf, Inc), Port = integer_to_binary(?PORT), Bin = <<"listeners.tcp.max_connection_test {bind = \"0.0.0.0:", Port/binary, "\"}">>, - ok = file:write_file(IncFile, Bin), - ok = file:write_file(ConfFile, ["include \"", TmpConfName, "\""], [append]), + TmpConfName = atom_to_list(?FUNCTION_NAME) ++ ".conf", + TmpConfFullPath = inject_tmp_config_content(TmpConfName, Bin), emqx_mgmt_api_test_util:init_suite([emqx_conf]), - [{tmp_config_file, IncFile} | Config]; + [{tmp_config_file, TmpConfFullPath} | Config]; t_max_connection_default({'end', Config}) -> ok = file:delete(proplists:get_value(tmp_config_file, Config)); t_max_connection_default(Config) when is_list(Config) -> @@ -123,7 +127,7 @@ t_tcp_crud_listeners_by_id(Config) when is_list(Config) -> MinListenerId = <<"tcp:min">>, BadId = <<"tcp:bad">>, Type = <<"tcp">>, - crud_listeners_by_id(ListenerId, NewListenerId, MinListenerId, BadId, Type). + crud_listeners_by_id(ListenerId, NewListenerId, MinListenerId, BadId, Type, 31000). t_ssl_crud_listeners_by_id(Config) when is_list(Config) -> ListenerId = <<"ssl:default">>, @@ -131,7 +135,7 @@ t_ssl_crud_listeners_by_id(Config) when is_list(Config) -> MinListenerId = <<"ssl:min">>, BadId = <<"ssl:bad">>, Type = <<"ssl">>, - crud_listeners_by_id(ListenerId, NewListenerId, MinListenerId, BadId, Type). + crud_listeners_by_id(ListenerId, NewListenerId, MinListenerId, BadId, Type, 32000). t_ws_crud_listeners_by_id(Config) when is_list(Config) -> ListenerId = <<"ws:default">>, @@ -139,7 +143,7 @@ t_ws_crud_listeners_by_id(Config) when is_list(Config) -> MinListenerId = <<"ws:min">>, BadId = <<"ws:bad">>, Type = <<"ws">>, - crud_listeners_by_id(ListenerId, NewListenerId, MinListenerId, BadId, Type). + crud_listeners_by_id(ListenerId, NewListenerId, MinListenerId, BadId, Type, 33000). t_wss_crud_listeners_by_id(Config) when is_list(Config) -> ListenerId = <<"wss:default">>, @@ -147,7 +151,7 @@ t_wss_crud_listeners_by_id(Config) when is_list(Config) -> MinListenerId = <<"wss:min">>, BadId = <<"wss:bad">>, Type = <<"wss">>, - crud_listeners_by_id(ListenerId, NewListenerId, MinListenerId, BadId, Type). + crud_listeners_by_id(ListenerId, NewListenerId, MinListenerId, BadId, Type, 34000). t_api_listeners_list_not_ready(Config) when is_list(Config) -> net_kernel:start(['listeners@127.0.0.1', longnames]), @@ -266,16 +270,18 @@ cluster(Specs) -> end} ]). -crud_listeners_by_id(ListenerId, NewListenerId, MinListenerId, BadId, Type) -> +crud_listeners_by_id(ListenerId, NewListenerId, MinListenerId, BadId, Type, PortBase) -> OriginPath = emqx_mgmt_api_test_util:api_path(["listeners", ListenerId]), NewPath = emqx_mgmt_api_test_util:api_path(["listeners", NewListenerId]), OriginListener = request(get, OriginPath, [], []), + ct:pal("raw conf: ~p~n", [emqx_config:get_raw([listeners])]), + ct:pal("OriginListener:~p", [OriginListener]), %% create with full options ?assertEqual({error, not_found}, is_running(NewListenerId)), ?assertMatch({error, {"HTTP/1.1", 404, _}}, request(get, NewPath, [], [])), - Port1 = integer_to_binary(?PORT), - Port2 = integer_to_binary(?PORT), + Port1 = integer_to_binary(?PORT(PortBase)), + Port2 = integer_to_binary(?PORT(PortBase)), NewConf = OriginListener#{ <<"id">> => NewListenerId, <<"bind">> => <<"0.0.0.0:", Port1/binary>> @@ -417,3 +423,21 @@ data_file(Name) -> cert_file(Name) -> data_file(filename:join(["certs", Name])). + +default_listeners_hcon_text() -> + Sc = #{roots => emqx_schema:fields("listeners")}, + Listeners = hocon_tconf:make_serializable(Sc, #{}, #{}), + Config = #{<<"listeners">> => Listeners}, + hocon_pp:do(Config, #{}). + +%% inject a 'include' at the end of emqx.conf.all +%% the 'include' can be kept after test, +%% as long as the file has been deleted it is a no-op +inject_tmp_config_content(TmpFile, Content) -> + Etc = filename:join(["etc", "emqx.conf.all"]), + Inc = filename:join(["etc", TmpFile]), + ConfFile = emqx_common_test_helpers:app_path(emqx_conf, Etc), + TmpFileFullPath = emqx_common_test_helpers:app_path(emqx_conf, Inc), + ok = file:write_file(TmpFileFullPath, Content), + ok = file:write_file(ConfFile, ["\ninclude \"", TmpFileFullPath, "\"\n"], [append]), + TmpFileFullPath. From c13a972bf01326421f83e9143f585f08c0109781 Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Fri, 28 Apr 2023 12:57:29 +0200 Subject: [PATCH 26/41] test(emqx_management): add test group for listener API --- .../test/emqx_mgmt_api_listeners_SUITE.erl | 34 ++++++++++++++++--- 1 file changed, 29 insertions(+), 5 deletions(-) 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 e73f67f95..cb4e370d3 100644 --- a/apps/emqx_management/test/emqx_mgmt_api_listeners_SUITE.erl +++ b/apps/emqx_management/test/emqx_mgmt_api_listeners_SUITE.erl @@ -24,10 +24,29 @@ -define(PORT, ?PORT(20000)). all() -> - emqx_common_test_helpers:all(?MODULE). + [ + {group, with_defaults_in_file}, + {group, without_defaults_in_file} + ]. + +groups() -> + AllTests = emqx_common_test_helpers:all(?MODULE), + [ + {with_defaults_in_file, AllTests}, + {without_defaults_in_file, AllTests} + ]. init_per_suite(Config) -> - %% we have to materialize the config file with default values for this test suite + Config. + +end_per_suite(_Config) -> + ok. + +init_per_group(without_defaults_in_file, Config) -> + emqx_mgmt_api_test_util:init_suite([emqx_conf]), + Config; +init_per_group(with_defaults_in_file, Config) -> + %% we have to materialize the config file with default values for this test group %% because we want to test the deletion of non-existing listener %% if there is no config file, the such deletion would result in a deletion %% of the default listener. @@ -36,11 +55,16 @@ init_per_suite(Config) -> emqx_mgmt_api_test_util:init_suite([emqx_conf]), [{injected_conf_file, TmpConfFullPath} | Config]. -end_per_suite(Config) -> +end_per_group(Group, Config) -> emqx_conf:remove([listeners, tcp, new], #{override_to => cluster}), emqx_conf:remove([listeners, tcp, new1], #{override_to => local}), - {_, File} = lists:keyfind(injected_conf_file, 1, Config), - ok = file:delete(File), + case Group =:= with_defaults_in_file of + true -> + {_, File} = lists:keyfind(injected_conf_file, 1, Config), + ok = file:delete(File); + false -> + ok + end, emqx_mgmt_api_test_util:end_suite([emqx_conf]). init_per_testcase(Case, Config) -> From 2e9dca280c2aeb5096dfa42d5f8cf02d27cbe9d5 Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Fri, 28 Apr 2023 15:22:20 +0200 Subject: [PATCH 27/41] refactor(listener-schema): use a tombstone for deleted listeners --- apps/emqx/src/emqx_config_handler.erl | 16 ++- apps/emqx/src/emqx_listeners.erl | 71 ++++++++---- apps/emqx/src/emqx_schema.erl | 106 ++++++++++++------ apps/emqx_conf/src/emqx_conf.erl | 5 + .../src/emqx_dashboard_swagger.erl | 5 +- .../src/emqx_mgmt_api_listeners.erl | 12 +- .../test/emqx_mgmt_api_listeners_SUITE.erl | 24 ++-- 7 files changed, 158 insertions(+), 81 deletions(-) diff --git a/apps/emqx/src/emqx_config_handler.erl b/apps/emqx/src/emqx_config_handler.erl index e664a7dd7..adbba032a 100644 --- a/apps/emqx/src/emqx_config_handler.erl +++ b/apps/emqx/src/emqx_config_handler.erl @@ -447,11 +447,17 @@ merge_to_override_config(RawConf, Opts) -> up_req({remove, _Opts}) -> '$remove'; up_req({{update, Req}, _Opts}) -> Req. -return_change_result(ConfKeyPath, {{update, _Req}, Opts}) -> - #{ - config => emqx_config:get(ConfKeyPath), - raw_config => return_rawconf(ConfKeyPath, Opts) - }; +return_change_result(ConfKeyPath, {{update, Req}, Opts}) -> + case Req =/= emqx_schema:tombstone() of + true -> + #{ + config => emqx_config:get(ConfKeyPath), + raw_config => return_rawconf(ConfKeyPath, Opts) + }; + false -> + %% like remove, nothing to return + #{} + end; return_change_result(_ConfKeyPath, {remove, _Opts}) -> #{}. diff --git a/apps/emqx/src/emqx_listeners.erl b/apps/emqx/src/emqx_listeners.erl index 18ddcaba2..e00c79b60 100644 --- a/apps/emqx/src/emqx_listeners.erl +++ b/apps/emqx/src/emqx_listeners.erl @@ -22,7 +22,9 @@ -include("emqx_mqtt.hrl"). -include("logger.hrl"). -include_lib("snabbkaffe/include/snabbkaffe.hrl"). - +-ifdef(TEST). +-include_lib("eunit/include/eunit.hrl"). +-endif. %% APIs -export([ list_raw/0, @@ -33,7 +35,8 @@ is_running/1, current_conns/2, max_conns/2, - id_example/0 + id_example/0, + default_max_conn/0 ]). -export([ @@ -61,8 +64,12 @@ -export([certs_dir/2]). -endif. +-type listener_id() :: atom() | binary(). + -define(CONF_KEY_PATH, [listeners, '?', '?']). -define(TYPES_STRING, ["tcp", "ssl", "ws", "wss", "quic"]). +-define(MARK_DEL, marked_for_deletion). +-define(MARK_DEL_BIN, <<"marked_for_deletion">>). -spec id_example() -> atom(). id_example() -> 'tcp:default'. @@ -105,19 +112,22 @@ do_list_raw() -> format_raw_listeners({Type0, Conf}) -> Type = binary_to_atom(Type0), - lists:map( - fun({LName, LConf0}) when is_map(LConf0) -> - Bind = parse_bind(LConf0), - Running = is_running(Type, listener_id(Type, LName), LConf0#{bind => Bind}), - LConf1 = maps:remove(<<"authentication">>, LConf0), - LConf3 = maps:put(<<"running">>, Running, LConf1), - CurrConn = - case Running of - true -> current_conns(Type, LName, Bind); - false -> 0 - end, - LConf4 = maps:put(<<"current_connections">>, CurrConn, LConf3), - {Type0, LName, LConf4} + lists:filtermap( + fun + ({LName, LConf0}) when is_map(LConf0) -> + Bind = parse_bind(LConf0), + Running = is_running(Type, listener_id(Type, LName), LConf0#{bind => Bind}), + LConf1 = maps:remove(<<"authentication">>, LConf0), + LConf3 = maps:put(<<"running">>, Running, LConf1), + CurrConn = + case Running of + true -> current_conns(Type, LName, Bind); + false -> 0 + end, + LConf4 = maps:put(<<"current_connections">>, CurrConn, LConf3), + {true, {Type0, LName, LConf4}}; + ({_LName, _MarkDel}) -> + false end, maps:to_list(Conf) ). @@ -195,7 +205,7 @@ start() -> ok = emqx_config_handler:add_handler(?CONF_KEY_PATH, ?MODULE), foreach_listeners(fun start_listener/3). --spec start_listener(atom()) -> ok | {error, term()}. +-spec start_listener(listener_id()) -> ok | {error, term()}. start_listener(ListenerId) -> apply_on_listener(ListenerId, fun start_listener/3). @@ -246,7 +256,7 @@ start_listener(Type, ListenerName, #{bind := Bind} = Conf) -> restart() -> foreach_listeners(fun restart_listener/3). --spec restart_listener(atom()) -> ok | {error, term()}. +-spec restart_listener(listener_id()) -> ok | {error, term()}. restart_listener(ListenerId) -> apply_on_listener(ListenerId, fun restart_listener/3). @@ -271,7 +281,7 @@ stop() -> _ = emqx_config_handler:remove_handler(?CONF_KEY_PATH), foreach_listeners(fun stop_listener/3). --spec stop_listener(atom()) -> ok | {error, term()}. +-spec stop_listener(listener_id()) -> ok | {error, term()}. stop_listener(ListenerId) -> apply_on_listener(ListenerId, fun stop_listener/3). @@ -419,7 +429,9 @@ do_start_listener(quic, ListenerName, #{bind := Bind} = Opts) -> end. %% Update the listeners at runtime -pre_config_update([listeners, Type, Name], {create, NewConf}, undefined) -> +pre_config_update([listeners, Type, Name], {create, NewConf}, V) when + V =:= undefined orelse V =:= ?MARK_DEL_BIN +-> CertsDir = certs_dir(Type, Name), {ok, convert_certs(CertsDir, NewConf)}; pre_config_update([listeners, _Type, _Name], {create, _NewConf}, _RawConf) -> @@ -434,6 +446,8 @@ pre_config_update([listeners, Type, Name], {update, Request}, RawConf) -> pre_config_update([listeners, _Type, _Name], {action, _Action, Updated}, RawConf) -> NewConf = emqx_utils_maps:deep_merge(RawConf, Updated), {ok, NewConf}; +pre_config_update([listeners, _Type, _Name], ?MARK_DEL, _RawConf) -> + {ok, ?MARK_DEL}; pre_config_update(_Path, _Request, RawConf) -> {ok, RawConf}. @@ -446,9 +460,9 @@ post_config_update([listeners, Type, Name], {update, _Request}, NewConf, OldConf #{enabled := true} -> restart_listener(Type, Name, {OldConf, NewConf}); _ -> ok end; -post_config_update([listeners, _Type, _Name], '$remove', undefined, undefined, _AppEnvs) -> - ok; -post_config_update([listeners, Type, Name], '$remove', undefined, OldConf, _AppEnvs) -> +post_config_update([listeners, Type, Name], Op, _, OldConf, _AppEnvs) when + Op =:= ?MARK_DEL andalso is_map(OldConf) +-> ok = unregister_ocsp_stapling_refresh(Type, Name), case stop_listener(Type, Name, OldConf) of ok -> @@ -611,6 +625,7 @@ format_bind(Bin) when is_binary(Bin) -> listener_id(Type, ListenerName) -> list_to_atom(lists:append([str(Type), ":", str(ListenerName)])). +-spec parse_listener_id(listener_id()) -> {ok, #{type => atom(), name => atom()}} | {error, term()}. parse_listener_id(Id) -> case string:split(str(Id), ":", leading) of [Type, Name] -> @@ -836,3 +851,15 @@ unregister_ocsp_stapling_refresh(Type, Name) -> ListenerId = listener_id(Type, Name), emqx_ocsp_cache:unregister_listener(ListenerId), ok. + +%% There is currently an issue with frontend +%% infinity is not a good value for it, so we use 5m for now +default_max_conn() -> + %% TODO: <<"infinity">> + 5_000_000. + +-ifdef(TEST). +%% since it's a copy-paste. we need to ensure it's the same atom. +ensure_same_atom_test() -> + ?assertEqual(?MARK_DEL, emqx_schema:tombstone()). +-endif. diff --git a/apps/emqx/src/emqx_schema.erl b/apps/emqx/src/emqx_schema.erl index 418a2db56..e36da0e0a 100644 --- a/apps/emqx/src/emqx_schema.erl +++ b/apps/emqx/src/emqx_schema.erl @@ -100,6 +100,13 @@ convert_servers/2 ]). +%% tombstone types +-export([ + tombstone/0, + tombstone_map/2, + get_tombstone_map_value_type/1 +]). + -behaviour(hocon_schema). -reflect_type([ @@ -777,45 +784,48 @@ fields("listeners") -> [ {"tcp", sc( - map(name, ref("mqtt_tcp_listener")), + tombstone_map(name, ref("mqtt_tcp_listener")), #{ desc => ?DESC(fields_listeners_tcp), - default => default_listener(tcp), + converter => fun(X, _) -> + ensure_default_listener(X, tcp) + end, required => {false, recursively} } )}, {"ssl", sc( - map(name, ref("mqtt_ssl_listener")), + tombstone_map(name, ref("mqtt_ssl_listener")), #{ desc => ?DESC(fields_listeners_ssl), - default => default_listener(ssl), + converter => fun(X, _) -> ensure_default_listener(X, ssl) end, required => {false, recursively} } )}, {"ws", sc( - map(name, ref("mqtt_ws_listener")), + tombstone_map(name, ref("mqtt_ws_listener")), #{ desc => ?DESC(fields_listeners_ws), - default => default_listener(ws), + converter => fun(X, _) -> ensure_default_listener(X, ws) end, required => {false, recursively} } )}, {"wss", sc( - map(name, ref("mqtt_wss_listener")), + tombstone_map(name, ref("mqtt_wss_listener")), #{ desc => ?DESC(fields_listeners_wss), - default => default_listener(wss), + converter => fun(X, _) -> ensure_default_listener(X, wss) end, required => {false, recursively} } )}, {"quic", sc( - map(name, ref("mqtt_quic_listener")), + tombstone_map(name, ref("mqtt_quic_listener")), #{ desc => ?DESC(fields_listeners_quic), + converter => fun keep_default_tombstone/2, required => {false, recursively} } )} @@ -1943,7 +1953,7 @@ base_listener(Bind) -> sc( hoconsc:union([infinity, pos_integer()]), #{ - default => <<"infinity">>, + default => emqx_listeners:default_max_conn(), desc => ?DESC(base_listener_max_connections) } )}, @@ -3092,20 +3102,12 @@ assert_required_field(Conf, Key, ErrorMessage) -> default_listener(tcp) -> #{ - <<"default">> => - #{ - <<"bind">> => <<"0.0.0.0:1883">>, - <<"max_connections">> => 1024000 - } + <<"bind">> => <<"0.0.0.0:1883">> }; default_listener(ws) -> #{ - <<"default">> => - #{ - <<"bind">> => <<"0.0.0.0:8083">>, - <<"max_connections">> => 1024000, - <<"websocket">> => #{<<"mqtt_path">> => <<"/mqtt">>} - } + <<"bind">> => <<"0.0.0.0:8083">>, + <<"websocket">> => #{<<"mqtt_path">> => <<"/mqtt">>} }; default_listener(SSLListener) -> %% The env variable is resolved in emqx_tls_lib by calling naive_env_interpolate @@ -3120,22 +3122,14 @@ default_listener(SSLListener) -> case SSLListener of ssl -> #{ - <<"default">> => - #{ - <<"bind">> => <<"0.0.0.0:8883">>, - <<"max_connections">> => 512000, - <<"ssl_options">> => SslOptions - } + <<"bind">> => <<"0.0.0.0:8883">>, + <<"ssl_options">> => SslOptions }; wss -> #{ - <<"default">> => - #{ - <<"bind">> => <<"0.0.0.0:8084">>, - <<"max_connections">> => 512000, - <<"ssl_options">> => SslOptions, - <<"websocket">> => #{<<"mqtt_path">> => <<"/mqtt">>} - } + <<"bind">> => <<"0.0.0.0:8084">>, + <<"ssl_options">> => SslOptions, + <<"websocket">> => #{<<"mqtt_path">> => <<"/mqtt">>} } end. @@ -3196,3 +3190,47 @@ special_env(_Name) -> -else. special_env(_Name) -> error. -endif. + +%% The tombstone atom. +tombstone() -> + marked_for_deletion. + +%% Make a map type, the value of which is allowed to be 'marked_for_deletion' +%% 'marked_for_delition' is a special value which means the key is deleted. +%% This is used to support the 'delete' operation in configs, +%% since deleting the key would result in default value being used. +tombstone_map(Name, Type) -> + %% marked_for_deletion must be the last member of the union + %% because we need to first union member to populate the default values + map(Name, ?UNION([Type, tombstone()])). + +%% inverse of mark_del_map +get_tombstone_map_value_type(Schema) -> + %% TODO: violation of abstraction, expose an API in hoconsc + %% hoconsc:map_value_type(Schema) + ?MAP(_Name, Union) = hocon_schema:field_schema(Schema, type), + %% TODO: violation of abstraction, fix hoconsc:union_members/1 + ?UNION(Members) = Union, + Tombstone = tombstone(), + [Type, Tombstone] = hoconsc:union_members(Members), + Type. + +%% Keep the 'default' tombstone, but delete others. +keep_default_tombstone(Map, _Opts) when is_map(Map) -> + maps:filter( + fun(Key, Value) -> + Key =:= <<"default">> orelse Value =/= atom_to_binary(tombstone()) + end, + Map + ); +keep_default_tombstone(Value, _Opts) -> + Value. + +ensure_default_listener(undefined, ListenerType) -> + %% let the schema's default value do its job + #{<<"default">> => default_listener(ListenerType)}; +ensure_default_listener(#{<<"default">> := _} = Map, _ListenerType) -> + keep_default_tombstone(Map, #{}); +ensure_default_listener(Map, ListenerType) -> + NewMap = Map#{<<"default">> => default_listener(ListenerType)}, + keep_default_tombstone(NewMap, #{}). diff --git a/apps/emqx_conf/src/emqx_conf.erl b/apps/emqx_conf/src/emqx_conf.erl index 1ecda913d..1b37f652b 100644 --- a/apps/emqx_conf/src/emqx_conf.erl +++ b/apps/emqx_conf/src/emqx_conf.erl @@ -24,6 +24,7 @@ -export([get_by_node/2, get_by_node/3]). -export([update/3, update/4]). -export([remove/2, remove/3]). +-export([tombstone/2]). -export([reset/2, reset/3]). -export([dump_schema/1, dump_schema/3]). -export([schema_module/0]). @@ -107,6 +108,10 @@ update(Node, KeyPath, UpdateReq, Opts0) when Node =:= node() -> update(Node, KeyPath, UpdateReq, Opts) -> emqx_conf_proto_v2:update(Node, KeyPath, UpdateReq, Opts). +%% @doc Mark the specified key path as tombstone +tombstone(KeyPath, Opts) -> + update(KeyPath, emqx_schema:tombstone(), Opts). + %% @doc remove all value of key path in cluster-override.conf or local-override.conf. -spec remove(emqx_utils_maps:config_key_path(), emqx_config:update_opts()) -> {ok, emqx_config:update_result()} | {error, emqx_config:update_error()}. diff --git a/apps/emqx_dashboard/src/emqx_dashboard_swagger.erl b/apps/emqx_dashboard/src/emqx_dashboard_swagger.erl index b2ad69997..627ef0719 100644 --- a/apps/emqx_dashboard/src/emqx_dashboard_swagger.erl +++ b/apps/emqx_dashboard/src/emqx_dashboard_swagger.erl @@ -237,8 +237,9 @@ parse_spec_ref(Module, Path, Options) -> erlang:apply(Module, schema, [Path]) %% better error message catch - error:Reason -> - throw({error, #{mfa => {Module, schema, [Path]}, reason => Reason}}) + error:Reason:Stacktrace -> + MoreInfo = #{module => Module, path => Path, reason => Reason}, + erlang:raise(error, MoreInfo, Stacktrace) end, {Specs, Refs} = maps:fold( fun(Method, Meta, {Acc, RefsAcc}) -> diff --git a/apps/emqx_management/src/emqx_mgmt_api_listeners.erl b/apps/emqx_management/src/emqx_mgmt_api_listeners.erl index d7f3ff321..9a338b33f 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_listeners.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_listeners.erl @@ -293,12 +293,14 @@ listeners_type() -> listeners_info(Opts) -> Listeners = hocon_schema:fields(emqx_schema, "listeners"), lists:map( - fun({Type, #{type := ?MAP(_Name, ?R_REF(Mod, Field))}}) -> - Fields0 = hocon_schema:fields(Mod, Field), + fun({ListenerType, Schema}) -> + Type = emqx_schema:get_tombstone_map_value_type(Schema), + ?R_REF(Mod, StructName) = Type, + Fields0 = hocon_schema:fields(Mod, StructName), Fields1 = lists:keydelete("authentication", 1, Fields0), Fields3 = required_bind(Fields1, Opts), - Ref = listeners_ref(Type, Opts), - TypeAtom = list_to_existing_atom(Type), + Ref = listeners_ref(ListenerType, Opts), + TypeAtom = list_to_existing_atom(ListenerType), #{ ref => ?R_REF(Ref), schema => [ @@ -642,7 +644,7 @@ create(Path, Conf) -> wrap(emqx_conf:update(Path, {create, Conf}, ?OPTS(cluster))). ensure_remove(Path) -> - wrap(emqx_conf:remove(Path, ?OPTS(cluster))). + wrap(emqx_conf:update(Path, emqx_schema:tombstone(), ?OPTS(cluster))). wrap({error, {post_config_update, emqx_listeners, Reason}}) -> {error, Reason}; wrap({error, {pre_config_update, emqx_listeners, Reason}}) -> {error, Reason}; 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 cb4e370d3..3a26c948e 100644 --- a/apps/emqx_management/test/emqx_mgmt_api_listeners_SUITE.erl +++ b/apps/emqx_management/test/emqx_mgmt_api_listeners_SUITE.erl @@ -51,13 +51,13 @@ init_per_group(with_defaults_in_file, Config) -> %% if there is no config file, the such deletion would result in a deletion %% of the default listener. Name = atom_to_list(?MODULE) ++ "-default-listeners", - TmpConfFullPath = inject_tmp_config_content(Name, default_listeners_hcon_text()), + TmpConfFullPath = inject_tmp_config_content(Name, default_listeners_hocon_text()), emqx_mgmt_api_test_util:init_suite([emqx_conf]), [{injected_conf_file, TmpConfFullPath} | Config]. end_per_group(Group, Config) -> - emqx_conf:remove([listeners, tcp, new], #{override_to => cluster}), - emqx_conf:remove([listeners, tcp, new1], #{override_to => local}), + emqx_conf:tombstone([listeners, tcp, new], #{override_to => cluster}), + emqx_conf:tombstone([listeners, tcp, new1], #{override_to => local}), case Group =:= with_defaults_in_file of true -> {_, File} = lists:keyfind(injected_conf_file, 1, Config), @@ -94,16 +94,16 @@ t_max_connection_default({init, Config}) -> t_max_connection_default({'end', Config}) -> ok = file:delete(proplists:get_value(tmp_config_file, Config)); t_max_connection_default(Config) when is_list(Config) -> - %% Check infinity is binary not atom. #{<<"listeners">> := Listeners} = emqx_mgmt_api_listeners:do_list_listeners(), Target = lists:filter( fun(#{<<"id">> := Id}) -> Id =:= 'tcp:max_connection_test' end, Listeners ), - ?assertMatch([#{<<"max_connections">> := <<"infinity">>}], Target), + DefaultMaxConn = emqx_listeners:default_max_conn(), + ?assertMatch([#{<<"max_connections">> := DefaultMaxConn}], Target), NewPath = emqx_mgmt_api_test_util:api_path(["listeners", "tcp:max_connection_test"]), - ?assertMatch(#{<<"max_connections">> := <<"infinity">>}, request(get, NewPath, [], [])), - emqx_conf:remove([listeners, tcp, max_connection_test], #{override_to => cluster}), + ?assertMatch(#{<<"max_connections">> := DefaultMaxConn}, request(get, NewPath, [], [])), + emqx_conf:tombstone([listeners, tcp, max_connection_test], #{override_to => cluster}), ok. t_list_listeners(Config) when is_list(Config) -> @@ -114,7 +114,7 @@ t_list_listeners(Config) when is_list(Config) -> %% POST /listeners ListenerId = <<"tcp:default">>, - NewListenerId = <<"tcp:new">>, + NewListenerId = <<"tcp:new11">>, OriginPath = emqx_mgmt_api_test_util:api_path(["listeners", ListenerId]), NewPath = emqx_mgmt_api_test_util:api_path(["listeners", NewListenerId]), @@ -128,7 +128,7 @@ t_list_listeners(Config) when is_list(Config) -> OriginListener2 = maps:remove(<<"id">>, OriginListener), Port = integer_to_binary(?PORT), NewConf = OriginListener2#{ - <<"name">> => <<"new">>, + <<"name">> => <<"new11">>, <<"bind">> => <<"0.0.0.0:", Port/binary>>, <<"max_connections">> := <<"infinity">> }, @@ -298,8 +298,6 @@ crud_listeners_by_id(ListenerId, NewListenerId, MinListenerId, BadId, Type, Port OriginPath = emqx_mgmt_api_test_util:api_path(["listeners", ListenerId]), NewPath = emqx_mgmt_api_test_util:api_path(["listeners", NewListenerId]), OriginListener = request(get, OriginPath, [], []), - ct:pal("raw conf: ~p~n", [emqx_config:get_raw([listeners])]), - ct:pal("OriginListener:~p", [OriginListener]), %% create with full options ?assertEqual({error, not_found}, is_running(NewListenerId)), @@ -314,7 +312,7 @@ crud_listeners_by_id(ListenerId, NewListenerId, MinListenerId, BadId, Type, Port ?assertEqual(lists:sort(maps:keys(OriginListener)), lists:sort(maps:keys(Create))), Get1 = request(get, NewPath, [], []), ?assertMatch(Create, Get1), - ?assert(is_running(NewListenerId)), + ?assertEqual({true, NewListenerId}, {is_running(NewListenerId), NewListenerId}), %% create with required options MinPath = emqx_mgmt_api_test_util:api_path(["listeners", MinListenerId]), @@ -448,7 +446,7 @@ data_file(Name) -> cert_file(Name) -> data_file(filename:join(["certs", Name])). -default_listeners_hcon_text() -> +default_listeners_hocon_text() -> Sc = #{roots => emqx_schema:fields("listeners")}, Listeners = hocon_tconf:make_serializable(Sc, #{}, #{}), Config = #{<<"listeners">> => Listeners}, From 03ae61569f8f2b3dfa59ee91b15575e353b9d581 Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Sun, 30 Apr 2023 10:18:18 +0200 Subject: [PATCH 28/41] test(authn): fix test case after authentication default value added --- apps/emqx_authn/test/emqx_authn_schema_SUITE.erl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apps/emqx_authn/test/emqx_authn_schema_SUITE.erl b/apps/emqx_authn/test/emqx_authn_schema_SUITE.erl index 7e67584ac..cd1c38d06 100644 --- a/apps/emqx_authn/test/emqx_authn_schema_SUITE.erl +++ b/apps/emqx_authn/test/emqx_authn_schema_SUITE.erl @@ -78,7 +78,8 @@ t_check_schema(_Config) -> ). t_union_member_selector(_) -> - ?assertMatch(#{authentication := undefined}, check(undefined)), + %% default value for authentication + ?assertMatch(#{authentication := []}, check(undefined)), C1 = #{<<"backend">> => <<"built_in_database">>}, ?assertThrow( #{ From 57cc854a4a0f7ff7e0f55e4c8dbe19ec370732c6 Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Sun, 30 Apr 2023 10:45:11 +0200 Subject: [PATCH 29/41] test(bridge): fix bridge map type filed converters now the converters on map type fields only work at the wrapping map level but not the values --- apps/emqx_bridge/src/schema/emqx_bridge_schema.erl | 7 ++++++- lib-ee/emqx_ee_bridge/src/emqx_ee_bridge.erl | 12 +++++++++++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/apps/emqx_bridge/src/schema/emqx_bridge_schema.erl b/apps/emqx_bridge/src/schema/emqx_bridge_schema.erl index 4b9b7e3fe..f58805b6b 100644 --- a/apps/emqx_bridge/src/schema/emqx_bridge_schema.erl +++ b/apps/emqx_bridge/src/schema/emqx_bridge_schema.erl @@ -230,7 +230,12 @@ webhook_bridge_converter(Conf0, _HoconOpts) -> undefined -> undefined; _ -> - do_convert_webhook_config(Conf1) + maps:map( + fun(_Name, Conf) -> + do_convert_webhook_config(Conf) + end, + Conf1 + ) end. do_convert_webhook_config( diff --git a/lib-ee/emqx_ee_bridge/src/emqx_ee_bridge.erl b/lib-ee/emqx_ee_bridge/src/emqx_ee_bridge.erl index 3ad5cbbb4..5981904c2 100644 --- a/lib-ee/emqx_ee_bridge/src/emqx_ee_bridge.erl +++ b/lib-ee/emqx_ee_bridge/src/emqx_ee_bridge.erl @@ -181,7 +181,7 @@ kafka_structs() -> #{ desc => <<"Kafka Producer Bridge Config">>, required => false, - converter => fun emqx_bridge_kafka:kafka_producer_converter/2 + converter => fun kafka_producer_converter/2 } )}, {kafka_consumer, @@ -264,3 +264,13 @@ sqlserver_structs() -> } )} ]. + +kafka_producer_converter(undefined, _) -> + undefined; +kafka_producer_converter(Map, Opts) -> + maps:map( + fun(_Name, Config) -> + emqx_bridge_kafka:kafka_producer_converter(Config, Opts) + end, + Map + ). From 475dee32eea3196943617d9f445b2f1aa84d4581 Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Sun, 30 Apr 2023 17:28:41 +0200 Subject: [PATCH 30/41] test(emqx_dashboard): refine spec error --- apps/emqx_dashboard/src/emqx_dashboard_swagger.erl | 14 ++++++++++---- .../test/emqx_swagger_requestBody_SUITE.erl | 4 ++-- .../test/emqx_swagger_response_SUITE.erl | 7 ++----- 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/apps/emqx_dashboard/src/emqx_dashboard_swagger.erl b/apps/emqx_dashboard/src/emqx_dashboard_swagger.erl index 627ef0719..bffedaf3c 100644 --- a/apps/emqx_dashboard/src/emqx_dashboard_swagger.erl +++ b/apps/emqx_dashboard/src/emqx_dashboard_swagger.erl @@ -235,11 +235,17 @@ parse_spec_ref(Module, Path, Options) -> Schema = try erlang:apply(Module, schema, [Path]) - %% better error message catch - error:Reason:Stacktrace -> - MoreInfo = #{module => Module, path => Path, reason => Reason}, - erlang:raise(error, MoreInfo, Stacktrace) + Error:Reason:Stacktrace -> + %% This error is intended to fail the build + %% hence print to standard_error + io:format( + standard_error, + "Failed to generate swagger for path ~p in module ~p~n" + "error:~p~nreason:~p~n~p~n", + [Module, Path, Error, Reason, Stacktrace] + ), + error({failed_to_generate_swagger_spec, Module, Path}) end, {Specs, Refs} = maps:fold( fun(Method, Meta, {Acc, RefsAcc}) -> diff --git a/apps/emqx_dashboard/test/emqx_swagger_requestBody_SUITE.erl b/apps/emqx_dashboard/test/emqx_swagger_requestBody_SUITE.erl index 3150ed097..4dbc8abdc 100644 --- a/apps/emqx_dashboard/test/emqx_swagger_requestBody_SUITE.erl +++ b/apps/emqx_dashboard/test/emqx_swagger_requestBody_SUITE.erl @@ -308,8 +308,8 @@ t_nest_ref(_Config) -> t_none_ref(_Config) -> Path = "/ref/none", - ?assertThrow( - {error, #{mfa := {?MODULE, schema, [Path]}}}, + ?assertError( + {failed_to_generate_swagger_spec, ?MODULE, Path}, emqx_dashboard_swagger:parse_spec_ref(?MODULE, Path, #{}) ), ok. diff --git a/apps/emqx_dashboard/test/emqx_swagger_response_SUITE.erl b/apps/emqx_dashboard/test/emqx_swagger_response_SUITE.erl index 4d1501dae..f357bdc8f 100644 --- a/apps/emqx_dashboard/test/emqx_swagger_response_SUITE.erl +++ b/apps/emqx_dashboard/test/emqx_swagger_response_SUITE.erl @@ -278,11 +278,8 @@ t_bad_ref(_Config) -> t_none_ref(_Config) -> Path = "/ref/none", - ?assertThrow( - {error, #{ - mfa := {?MODULE, schema, ["/ref/none"]}, - reason := function_clause - }}, + ?assertError( + {failed_to_generate_swagger_spec, ?MODULE, Path}, validate(Path, #{}, []) ), ok. From 43c80ba635aa5a7922c3ae695a1d6a40ed7f37b6 Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Sun, 30 Apr 2023 21:24:46 +0200 Subject: [PATCH 31/41] chore: always init_load config wiht defaults populated this effectively eliminates the need for raw_with_default because it's now always set to true everywhere. will remove it in a followup. --- apps/emqx/test/emqx_common_test_helpers.erl | 2 +- apps/emqx/test/emqx_logger_SUITE.erl | 1 - apps/emqx_conf/src/emqx_conf_app.erl | 7 ------- 3 files changed, 1 insertion(+), 9 deletions(-) diff --git a/apps/emqx/test/emqx_common_test_helpers.erl b/apps/emqx/test/emqx_common_test_helpers.erl index c6b04eed1..95427942d 100644 --- a/apps/emqx/test/emqx_common_test_helpers.erl +++ b/apps/emqx/test/emqx_common_test_helpers.erl @@ -489,7 +489,7 @@ load_config(SchemaModule, Config, Opts) -> ok. load_config(SchemaModule, Config) -> - load_config(SchemaModule, Config, #{raw_with_default => false}). + load_config(SchemaModule, Config, #{raw_with_default => true}). -spec is_all_tcp_servers_available(Servers) -> Result when Servers :: [{Host, Port}], diff --git a/apps/emqx/test/emqx_logger_SUITE.erl b/apps/emqx/test/emqx_logger_SUITE.erl index c8ff63c75..e8d7d7a34 100644 --- a/apps/emqx/test/emqx_logger_SUITE.erl +++ b/apps/emqx/test/emqx_logger_SUITE.erl @@ -22,7 +22,6 @@ -include_lib("eunit/include/eunit.hrl"). -define(LOGGER, emqx_logger). --define(a, "a"). -define(SUPPORTED_LEVELS, [emergency, alert, critical, error, warning, notice, info, debug]). all() -> emqx_common_test_helpers:all(?MODULE). diff --git a/apps/emqx_conf/src/emqx_conf_app.erl b/apps/emqx_conf/src/emqx_conf_app.erl index dedb9aeab..2231b8336 100644 --- a/apps/emqx_conf/src/emqx_conf_app.erl +++ b/apps/emqx_conf/src/emqx_conf_app.erl @@ -88,15 +88,8 @@ sync_data_from_node() -> %% Internal functions %% ------------------------------------------------------------------------------ --ifdef(TEST). -init_load() -> - emqx_config:init_load(emqx_conf:schema_module(), #{raw_with_default => false}). - --else. - init_load() -> emqx_config:init_load(emqx_conf:schema_module(), #{raw_with_default => true}). --endif. init_conf() -> %% Workaround for https://github.com/emqx/mria/issues/94: From a03f2dd64bd2f9115d3ef0e59d35e120d7d8bae6 Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Mon, 1 May 2023 15:35:33 +0200 Subject: [PATCH 32/41] test: allow pre-load configs before emqx_conf app --- apps/emqx/src/emqx_config.erl | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/apps/emqx/src/emqx_config.erl b/apps/emqx/src/emqx_config.erl index c117bffba..3d5a9d2a4 100644 --- a/apps/emqx/src/emqx_config.erl +++ b/apps/emqx/src/emqx_config.erl @@ -360,12 +360,13 @@ read_override_confs() -> hocon:deep_merge(ClusterOverrides, LocalOverrides). %% keep the raw and non-raw conf has the same keys to make update raw conf easier. +%% TODO: remove raw_with_default as it's now always true. maybe_fill_defaults(SchemaMod, RawConf0, #{raw_with_default := true}) -> RootSchemas = hocon_schema:roots(SchemaMod), %% the roots which are missing from the loaded configs MissingRoots = lists:filtermap( fun({BinName, Sc}) -> - case maps:is_key(BinName, RawConf0) of + case maps:is_key(BinName, RawConf0) orelse is_already_loaded(BinName) of true -> false; false -> {true, Sc} end @@ -383,6 +384,13 @@ maybe_fill_defaults(SchemaMod, RawConf0, #{raw_with_default := true}) -> maybe_fill_defaults(_SchemaMod, RawConf, _Opts) -> RawConf. +%% So far, this can only return true when testing. +%% e.g. when testing an app, we need to load its config first +%% then start emqx_conf application which will load the +%% possibly empty config again (then filled with defaults). +is_already_loaded(Name) -> + ?MODULE:get_raw([Name], #{}) =/= #{}. + %% if a root is not found in the raw conf, fill it with default values. seed_default(Schema) -> case hocon_schema:field_schema(Schema, default) of From 95cb262067db86f767b4ba08ded3695171f062d5 Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Mon, 1 May 2023 18:42:45 +0200 Subject: [PATCH 33/41] test: fix authn test cases --- apps/emqx/src/emqx_config.erl | 6 ++++-- apps/emqx/test/emqx_common_test_helpers.erl | 11 ++++++++++- apps/emqx/test/emqx_config_SUITE.erl | 4 ++-- apps/emqx_authn/test/emqx_authn_api_SUITE.erl | 2 +- apps/emqx_authn/test/emqx_authn_enable_flag_SUITE.erl | 11 ++++++----- apps/emqx_authn/test/emqx_authn_jwt_SUITE.erl | 2 +- 6 files changed, 24 insertions(+), 12 deletions(-) diff --git a/apps/emqx/src/emqx_config.erl b/apps/emqx/src/emqx_config.erl index 3d5a9d2a4..54648fca6 100644 --- a/apps/emqx/src/emqx_config.erl +++ b/apps/emqx/src/emqx_config.erl @@ -89,7 +89,7 @@ ]). -ifdef(TEST). --export([erase_schema_mod_and_names/0]). +-export([erase_all/0]). -endif. -include("logger.hrl"). @@ -559,7 +559,9 @@ save_schema_mod_and_names(SchemaMod) -> }). -ifdef(TEST). -erase_schema_mod_and_names() -> +erase_all() -> + Names = get_root_names(), + lists:foreach(fun erase/1, Names), persistent_term:erase(?PERSIS_SCHEMA_MODS). -endif. diff --git a/apps/emqx/test/emqx_common_test_helpers.erl b/apps/emqx/test/emqx_common_test_helpers.erl index 95427942d..7698a3389 100644 --- a/apps/emqx/test/emqx_common_test_helpers.erl +++ b/apps/emqx/test/emqx_common_test_helpers.erl @@ -249,11 +249,20 @@ start_app(App, SpecAppConfig, Opts) -> case application:ensure_all_started(App) of {ok, _} -> ok = ensure_dashboard_listeners_started(App), + ok = wait_for_app_processes(App), ok; {error, Reason} -> error({failed_to_start_app, App, Reason}) end. +wait_for_app_processes(emqx_conf) -> + %% emqx_conf app has a gen_server which + %% initializes its state asynchronously + gen_server:call(emqx_cluster_rpc, dummy), + ok; +wait_for_app_processes(_) -> + ok. + app_conf_file(emqx_conf) -> "emqx.conf.all"; app_conf_file(App) -> atom_to_list(App) ++ ".conf". @@ -309,7 +318,7 @@ stop_apps(Apps) -> %% to avoid inter-suite flakiness application:unset_env(emqx, init_config_load_done), persistent_term:erase(?EMQX_AUTHENTICATION_SCHEMA_MODULE_PT_KEY), - emqx_config:erase_schema_mod_and_names(), + emqx_config:erase_all(), ok = emqx_config:delete_override_conf_files(), application:unset_env(emqx, local_override_conf_file), application:unset_env(emqx, cluster_override_conf_file), diff --git a/apps/emqx/test/emqx_config_SUITE.erl b/apps/emqx/test/emqx_config_SUITE.erl index 1704d1476..959c07dda 100644 --- a/apps/emqx/test/emqx_config_SUITE.erl +++ b/apps/emqx/test/emqx_config_SUITE.erl @@ -64,14 +64,14 @@ t_init_load(_Config) -> ConfFile = "./test_emqx.conf", ok = file:write_file(ConfFile, <<"">>), ExpectRootNames = lists:sort(hocon_schema:root_names(emqx_schema)), - emqx_config:erase_schema_mod_and_names(), + emqx_config:erase_all(), {ok, DeprecatedFile} = application:get_env(emqx, cluster_override_conf_file), ?assertEqual(false, filelib:is_regular(DeprecatedFile), DeprecatedFile), %% Don't has deprecated file ok = emqx_config:init_load(emqx_schema, [ConfFile]), ?assertEqual(ExpectRootNames, lists:sort(emqx_config:get_root_names())), ?assertMatch({ok, #{raw_config := 256}}, emqx:update_config([mqtt, max_topic_levels], 256)), - emqx_config:erase_schema_mod_and_names(), + emqx_config:erase_all(), %% Has deprecated file ok = file:write_file(DeprecatedFile, <<"{}">>), ok = emqx_config:init_load(emqx_schema, [ConfFile]), diff --git a/apps/emqx_authn/test/emqx_authn_api_SUITE.erl b/apps/emqx_authn/test/emqx_authn_api_SUITE.erl index c7f718dfc..6d9203c95 100644 --- a/apps/emqx_authn/test/emqx_authn_api_SUITE.erl +++ b/apps/emqx_authn/test/emqx_authn_api_SUITE.erl @@ -67,7 +67,7 @@ init_per_suite(Config) -> emqx_config:erase(?EMQX_AUTHENTICATION_CONFIG_ROOT_NAME_BINARY), _ = application:load(emqx_conf), ok = emqx_mgmt_api_test_util:init_suite( - [emqx_authn] + [emqx_conf, emqx_authn] ), ?AUTHN:delete_chain(?GLOBAL), diff --git a/apps/emqx_authn/test/emqx_authn_enable_flag_SUITE.erl b/apps/emqx_authn/test/emqx_authn_enable_flag_SUITE.erl index 59865ab41..98215e853 100644 --- a/apps/emqx_authn/test/emqx_authn_enable_flag_SUITE.erl +++ b/apps/emqx_authn/test/emqx_authn_enable_flag_SUITE.erl @@ -42,15 +42,16 @@ init_per_testcase(_Case, Config) -> <<"backend">> => <<"built_in_database">>, <<"user_id_type">> => <<"clientid">> }, - emqx:update_config( + {ok, _} = emqx:update_config( ?PATH, {create_authenticator, ?GLOBAL, AuthnConfig} ), - - emqx_conf:update( - [listeners, tcp, listener_authn_enabled], {create, listener_mqtt_tcp_conf(18830, true)}, #{} + {ok, _} = emqx_conf:update( + [listeners, tcp, listener_authn_enabled], + {create, listener_mqtt_tcp_conf(18830, true)}, + #{} ), - emqx_conf:update( + {ok, _} = emqx_conf:update( [listeners, tcp, listener_authn_disabled], {create, listener_mqtt_tcp_conf(18831, false)}, #{} diff --git a/apps/emqx_authn/test/emqx_authn_jwt_SUITE.erl b/apps/emqx_authn/test/emqx_authn_jwt_SUITE.erl index 94c07ca96..bd18367b6 100644 --- a/apps/emqx_authn/test/emqx_authn_jwt_SUITE.erl +++ b/apps/emqx_authn/test/emqx_authn_jwt_SUITE.erl @@ -37,7 +37,7 @@ init_per_testcase(_, Config) -> init_per_suite(Config) -> _ = application:load(emqx_conf), - emqx_common_test_helpers:start_apps([emqx_authn]), + emqx_common_test_helpers:start_apps([emqx_conf, emqx_authn]), application:ensure_all_started(emqx_resource), application:ensure_all_started(emqx_connector), Config. From b65a71b498ea219857c36d8765cea7fd7c88ee0b Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Mon, 1 May 2023 19:35:57 +0200 Subject: [PATCH 34/41] test: allow emqx_ws_connection_SUITE to run without erasing configs --- apps/emqx/test/emqx_common_test_helpers.erl | 12 +++++++++++- apps/emqx/test/emqx_ws_connection_SUITE.erl | 8 ++++++-- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/apps/emqx/test/emqx_common_test_helpers.erl b/apps/emqx/test/emqx_common_test_helpers.erl index 7698a3389..26d908c5e 100644 --- a/apps/emqx/test/emqx_common_test_helpers.erl +++ b/apps/emqx/test/emqx_common_test_helpers.erl @@ -31,6 +31,7 @@ start_apps/2, start_apps/3, stop_apps/1, + stop_apps/2, reload/2, app_path/2, proj_root/0, @@ -313,12 +314,21 @@ generate_config(SchemaModule, ConfigFile) when is_atom(SchemaModule) -> -spec stop_apps(list()) -> ok. stop_apps(Apps) -> + stop_apps(Apps, #{}). + +stop_apps(Apps, Opts) -> [application:stop(App) || App <- Apps ++ [emqx, ekka, mria, mnesia]], ok = mria_mnesia:delete_schema(), %% to avoid inter-suite flakiness application:unset_env(emqx, init_config_load_done), persistent_term:erase(?EMQX_AUTHENTICATION_SCHEMA_MODULE_PT_KEY), - emqx_config:erase_all(), + case Opts of + #{erase_all_configs := false} -> + %% FIXME: this means inter-suite or inter-test dependencies + ok; + _ -> + emqx_config:erase_all() + end, ok = emqx_config:delete_override_conf_files(), application:unset_env(emqx, local_override_conf_file), application:unset_env(emqx, cluster_override_conf_file), diff --git a/apps/emqx/test/emqx_ws_connection_SUITE.erl b/apps/emqx/test/emqx_ws_connection_SUITE.erl index de8b1c9af..e62844547 100644 --- a/apps/emqx/test/emqx_ws_connection_SUITE.erl +++ b/apps/emqx/test/emqx_ws_connection_SUITE.erl @@ -138,13 +138,13 @@ end_per_testcase(t_ws_non_check_origin, Config) -> del_bucket(), PrevConfig = ?config(prev_config, Config), emqx_config:put_listener_conf(ws, default, [websocket], PrevConfig), - emqx_common_test_helpers:stop_apps([]), + stop_apps(), ok; end_per_testcase(_, Config) -> del_bucket(), PrevConfig = ?config(prev_config, Config), emqx_config:put_listener_conf(ws, default, [websocket], PrevConfig), - emqx_common_test_helpers:stop_apps([]), + stop_apps(), Config. init_per_suite(Config) -> @@ -156,6 +156,10 @@ end_per_suite(_) -> emqx_common_test_helpers:stop_apps([]), ok. +%% FIXME: this is a temp fix to tests share configs. +stop_apps() -> + emqx_common_test_helpers:stop_apps([], #{erase_all_configs => false}). + %%-------------------------------------------------------------------- %% Test Cases %%-------------------------------------------------------------------- From a3b1664c06a38ae4c8925bf689c93e283f9e6752 Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Mon, 1 May 2023 20:15:47 +0200 Subject: [PATCH 35/41] test: allow inter-test case config dependency for emqx_exhook_SUITE --- apps/emqx_exhook/test/emqx_exhook_SUITE.erl | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/apps/emqx_exhook/test/emqx_exhook_SUITE.erl b/apps/emqx_exhook/test/emqx_exhook_SUITE.erl index d12f99917..ff313c8c8 100644 --- a/apps/emqx_exhook/test/emqx_exhook_SUITE.erl +++ b/apps/emqx_exhook/test/emqx_exhook_SUITE.erl @@ -301,10 +301,10 @@ t_cluster_name(_) -> ok end, - emqx_common_test_helpers:stop_apps([emqx, emqx_exhook]), + stop_apps([emqx, emqx_exhook]), emqx_common_test_helpers:start_apps([emqx, emqx_exhook], SetEnvFun), on_exit(fun() -> - emqx_common_test_helpers:stop_apps([emqx, emqx_exhook]), + stop_apps([emqx, emqx_exhook]), load_cfg(?CONF_DEFAULT), emqx_common_test_helpers:start_apps([emqx_exhook]), mria:wait_for_tables([?CLUSTER_MFA, ?CLUSTER_COMMIT]) @@ -489,3 +489,7 @@ data_file(Name) -> cert_file(Name) -> data_file(filename:join(["certs", Name])). + +%% FIXME: this creats inter-test dependency +stop_apps(Apps) -> + emqx_common_test_helpers:stop_apps(Apps, #{erase_all_configs => false}). From c825102bed1146aa162e01a81258ada7907beef5 Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Tue, 2 May 2023 09:04:35 +0200 Subject: [PATCH 36/41] fix(authz): ensure acl.conf path template rendered --- apps/emqx_authz/src/emqx_authz_api_sources.erl | 4 ++-- apps/emqx_authz/src/emqx_authz_file.erl | 11 +++++++++-- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/apps/emqx_authz/src/emqx_authz_api_sources.erl b/apps/emqx_authz/src/emqx_authz_api_sources.erl index 2220e8f6e..d332f009f 100644 --- a/apps/emqx_authz/src/emqx_authz_api_sources.erl +++ b/apps/emqx_authz/src/emqx_authz_api_sources.erl @@ -205,7 +205,7 @@ sources(get, _) -> }, AccIn ) -> - case file:read_file(Path) of + case emqx_authz_file:read_file(Path) of {ok, Rules} -> lists:append(AccIn, [ #{ @@ -242,7 +242,7 @@ source(get, #{bindings := #{type := Type}}) -> Type, fun (#{<<"type">> := <<"file">>, <<"enable">> := Enable, <<"path">> := Path}) -> - case file:read_file(Path) of + case emqx_authz_file:read_file(Path) of {ok, Rules} -> {200, #{ type => file, diff --git a/apps/emqx_authz/src/emqx_authz_file.erl b/apps/emqx_authz/src/emqx_authz_file.erl index 63e7be781..54f1775c6 100644 --- a/apps/emqx_authz/src/emqx_authz_file.erl +++ b/apps/emqx_authz/src/emqx_authz_file.erl @@ -32,14 +32,15 @@ create/1, update/1, destroy/1, - authorize/4 + authorize/4, + read_file/1 ]). description() -> "AuthZ with static rules". create(#{path := Path0} = Source) -> - Path = emqx_schema:naive_env_interpolation(Path0), + Path = filename(Path0), Rules = case file:consult(Path) of {ok, Terms} -> @@ -64,3 +65,9 @@ destroy(_Source) -> ok. authorize(Client, PubSub, Topic, #{annotations := #{rules := Rules}}) -> emqx_authz_rule:matches(Client, PubSub, Topic, Rules). + +read_file(Path) -> + file:read_file(filename(Path)). + +filename(PathMaybeTemplate) -> + emqx_schema:naive_env_interpolation(PathMaybeTemplate). From 37bf12c29ee94cc6744f3bd405b67a99e7eef831 Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Tue, 2 May 2023 09:04:10 +0200 Subject: [PATCH 37/41] test(emqx_telemetry_SUITE): fix flakyness --- apps/emqx_modules/test/emqx_telemetry_SUITE.erl | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/apps/emqx_modules/test/emqx_telemetry_SUITE.erl b/apps/emqx_modules/test/emqx_telemetry_SUITE.erl index bb5f39c1f..cc9a8b20f 100644 --- a/apps/emqx_modules/test/emqx_telemetry_SUITE.erl +++ b/apps/emqx_modules/test/emqx_telemetry_SUITE.erl @@ -463,6 +463,16 @@ t_num_clients(_Config) -> ok. t_advanced_mqtt_features(_) -> + try + ok = test_advanced_mqtt_features() + catch + _:_ -> + %% delayed messages' metrics might not be reported yet + timer:sleep(1000), + test_advanced_mqtt_features() + end. + +test_advanced_mqtt_features() -> {ok, TelemetryData} = emqx_telemetry:get_telemetry(), AdvFeats = get_value(advanced_mqtt_features, TelemetryData), ?assertEqual( From 29c9edeb4c4e34241d71a908cd5b0846689dfa49 Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Tue, 2 May 2023 10:41:14 +0200 Subject: [PATCH 38/41] test(emqx_delayed_SUITE): fix flaky test --- apps/emqx_modules/test/emqx_delayed_SUITE.erl | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/apps/emqx_modules/test/emqx_delayed_SUITE.erl b/apps/emqx_modules/test/emqx_delayed_SUITE.erl index ffea436bb..59ab0269c 100644 --- a/apps/emqx_modules/test/emqx_delayed_SUITE.erl +++ b/apps/emqx_modules/test/emqx_delayed_SUITE.erl @@ -169,10 +169,10 @@ t_cluster(_) -> emqx_delayed_proto_v1:get_delayed_message(node(), Id) ), - ?assertEqual( - emqx_delayed:get_delayed_message(Id), - emqx_delayed_proto_v1:get_delayed_message(node(), Id) - ), + %% The 'local' and the 'fake-remote' values should be the same, + %% however there is a race condition, so we are just assert that they are both 'ok' tuples + ?assertMatch({ok, _}, emqx_delayed:get_delayed_message(Id)), + ?assertMatch({ok, _}, emqx_delayed_proto_v1:get_delayed_message(node(), Id)), ok = emqx_delayed_proto_v1:delete_delayed_message(node(), Id), From 674f837f369dc8909695d4ce9dd1c2e812dff502 Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Tue, 2 May 2023 10:42:54 +0200 Subject: [PATCH 39/41] refactor(emqx_listeners): better variable names --- apps/emqx/src/emqx_listeners.erl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/emqx/src/emqx_listeners.erl b/apps/emqx/src/emqx_listeners.erl index e00c79b60..a60168677 100644 --- a/apps/emqx/src/emqx_listeners.erl +++ b/apps/emqx/src/emqx_listeners.erl @@ -118,14 +118,14 @@ format_raw_listeners({Type0, Conf}) -> Bind = parse_bind(LConf0), Running = is_running(Type, listener_id(Type, LName), LConf0#{bind => Bind}), LConf1 = maps:remove(<<"authentication">>, LConf0), - LConf3 = maps:put(<<"running">>, Running, LConf1), + LConf2 = maps:put(<<"running">>, Running, LConf1), CurrConn = case Running of true -> current_conns(Type, LName, Bind); false -> 0 end, - LConf4 = maps:put(<<"current_connections">>, CurrConn, LConf3), - {true, {Type0, LName, LConf4}}; + LConf = maps:put(<<"current_connections">>, CurrConn, LConf2), + {true, {Type0, LName, LConf}}; ({_LName, _MarkDel}) -> false end, From b1dfbf7984bf07633697526a75cef15480326fc7 Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Tue, 2 May 2023 11:05:53 +0200 Subject: [PATCH 40/41] refactor: move shared macros to header file --- apps/emqx/include/emqx_schema.hrl | 22 +++++++++++++++++++ apps/emqx/src/emqx_listeners.erl | 15 ++++--------- apps/emqx/src/emqx_schema.erl | 5 +++-- .../src/emqx_mgmt_api_listeners.erl | 2 +- 4 files changed, 30 insertions(+), 14 deletions(-) create mode 100644 apps/emqx/include/emqx_schema.hrl diff --git a/apps/emqx/include/emqx_schema.hrl b/apps/emqx/include/emqx_schema.hrl new file mode 100644 index 000000000..7f96e15a9 --- /dev/null +++ b/apps/emqx/include/emqx_schema.hrl @@ -0,0 +1,22 @@ +%%-------------------------------------------------------------------- +%% Copyright (c) 2023 EMQ Technologies Co., Ltd. All Rights Reserved. +%% +%% Licensed under the Apache License, Version 2.0 (the "License"); +%% you may not use this file except in compliance with the License. +%% You may obtain a copy of the License at +%% +%% http://www.apache.org/licenses/LICENSE-2.0 +%% +%% Unless required by applicable law or agreed to in writing, software +%% distributed under the License is distributed on an "AS IS" BASIS, +%% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +%% See the License for the specific language governing permissions and +%% limitations under the License. +%%-------------------------------------------------------------------- +-ifndef(EMQX_SCHEMA_HRL). +-define(EMQX_SCHEMA_HRL, true). + +-define(TOMBSTONE, marked_for_deletion). +-define(TOMBSTONE_BIN, <<"marked_for_deletion">>). + +-endif. diff --git a/apps/emqx/src/emqx_listeners.erl b/apps/emqx/src/emqx_listeners.erl index a60168677..769e18e4d 100644 --- a/apps/emqx/src/emqx_listeners.erl +++ b/apps/emqx/src/emqx_listeners.erl @@ -20,11 +20,10 @@ -elvis([{elvis_style, dont_repeat_yourself, #{min_complexity => 10000}}]). -include("emqx_mqtt.hrl"). +-include("emqx_schema.hrl"). -include("logger.hrl"). -include_lib("snabbkaffe/include/snabbkaffe.hrl"). --ifdef(TEST). --include_lib("eunit/include/eunit.hrl"). --endif. + %% APIs -export([ list_raw/0, @@ -68,8 +67,8 @@ -define(CONF_KEY_PATH, [listeners, '?', '?']). -define(TYPES_STRING, ["tcp", "ssl", "ws", "wss", "quic"]). --define(MARK_DEL, marked_for_deletion). --define(MARK_DEL_BIN, <<"marked_for_deletion">>). +-define(MARK_DEL, ?TOMBSTONE). +-define(MARK_DEL_BIN, ?TOMBSTONE_BIN). -spec id_example() -> atom(). id_example() -> 'tcp:default'. @@ -857,9 +856,3 @@ unregister_ocsp_stapling_refresh(Type, Name) -> default_max_conn() -> %% TODO: <<"infinity">> 5_000_000. - --ifdef(TEST). -%% since it's a copy-paste. we need to ensure it's the same atom. -ensure_same_atom_test() -> - ?assertEqual(?MARK_DEL, emqx_schema:tombstone()). --endif. diff --git a/apps/emqx/src/emqx_schema.erl b/apps/emqx/src/emqx_schema.erl index e36da0e0a..f33dcb979 100644 --- a/apps/emqx/src/emqx_schema.erl +++ b/apps/emqx/src/emqx_schema.erl @@ -23,6 +23,7 @@ -dialyzer(no_fail_call). -elvis([{elvis_style, invalid_dynamic_call, disable}]). +-include("emqx_schema.hrl"). -include("emqx_authentication.hrl"). -include("emqx_access_control.hrl"). -include_lib("typerefl/include/types.hrl"). @@ -3193,7 +3194,7 @@ special_env(_Name) -> error. %% The tombstone atom. tombstone() -> - marked_for_deletion. + ?TOMBSTONE. %% Make a map type, the value of which is allowed to be 'marked_for_deletion' %% 'marked_for_delition' is a special value which means the key is deleted. @@ -3219,7 +3220,7 @@ get_tombstone_map_value_type(Schema) -> keep_default_tombstone(Map, _Opts) when is_map(Map) -> maps:filter( fun(Key, Value) -> - Key =:= <<"default">> orelse Value =/= atom_to_binary(tombstone()) + Key =:= <<"default">> orelse Value =/= ?TOMBSTONE_BIN end, Map ); diff --git a/apps/emqx_management/src/emqx_mgmt_api_listeners.erl b/apps/emqx_management/src/emqx_mgmt_api_listeners.erl index 9a338b33f..fdd555bab 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_listeners.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_listeners.erl @@ -644,7 +644,7 @@ create(Path, Conf) -> wrap(emqx_conf:update(Path, {create, Conf}, ?OPTS(cluster))). ensure_remove(Path) -> - wrap(emqx_conf:update(Path, emqx_schema:tombstone(), ?OPTS(cluster))). + wrap(emqx_conf:tombstone(Path, ?OPTS(cluster))). wrap({error, {post_config_update, emqx_listeners, Reason}}) -> {error, Reason}; wrap({error, {pre_config_update, emqx_listeners, Reason}}) -> {error, Reason}; From 2dd919171853b7408a2d32aa0282070f80781047 Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Tue, 2 May 2023 11:56:01 +0200 Subject: [PATCH 41/41] refactor: use different terms for config tombstone there are 3 different kind of Erlang terms for tombstone related configs 1. the schema type (must be an atom) 2. the config value (must be a binary) 3. the config change comamnd (request) which is only used in the code, but never persisted --- apps/emqx/include/emqx_schema.hrl | 5 +++-- apps/emqx/src/emqx_config_handler.erl | 3 ++- apps/emqx/src/emqx_listeners.erl | 7 +++---- apps/emqx/src/emqx_schema.erl | 7 +++---- apps/emqx_conf/src/emqx_conf.erl | 3 ++- 5 files changed, 13 insertions(+), 12 deletions(-) diff --git a/apps/emqx/include/emqx_schema.hrl b/apps/emqx/include/emqx_schema.hrl index 7f96e15a9..307bb20c5 100644 --- a/apps/emqx/include/emqx_schema.hrl +++ b/apps/emqx/include/emqx_schema.hrl @@ -16,7 +16,8 @@ -ifndef(EMQX_SCHEMA_HRL). -define(EMQX_SCHEMA_HRL, true). --define(TOMBSTONE, marked_for_deletion). --define(TOMBSTONE_BIN, <<"marked_for_deletion">>). +-define(TOMBSTONE_TYPE, marked_for_deletion). +-define(TOMBSTONE_VALUE, <<"marked_for_deletion">>). +-define(TOMBSTONE_CONFIG_CHANGE_REQ, mark_it_for_deletion). -endif. diff --git a/apps/emqx/src/emqx_config_handler.erl b/apps/emqx/src/emqx_config_handler.erl index adbba032a..0bad19f9e 100644 --- a/apps/emqx/src/emqx_config_handler.erl +++ b/apps/emqx/src/emqx_config_handler.erl @@ -18,6 +18,7 @@ -module(emqx_config_handler). -include("logger.hrl"). +-include("emqx_schema.hrl"). -include_lib("hocon/include/hoconsc.hrl"). -behaviour(gen_server). @@ -448,7 +449,7 @@ up_req({remove, _Opts}) -> '$remove'; up_req({{update, Req}, _Opts}) -> Req. return_change_result(ConfKeyPath, {{update, Req}, Opts}) -> - case Req =/= emqx_schema:tombstone() of + case Req =/= ?TOMBSTONE_CONFIG_CHANGE_REQ of true -> #{ config => emqx_config:get(ConfKeyPath), diff --git a/apps/emqx/src/emqx_listeners.erl b/apps/emqx/src/emqx_listeners.erl index 769e18e4d..b3043effc 100644 --- a/apps/emqx/src/emqx_listeners.erl +++ b/apps/emqx/src/emqx_listeners.erl @@ -67,8 +67,7 @@ -define(CONF_KEY_PATH, [listeners, '?', '?']). -define(TYPES_STRING, ["tcp", "ssl", "ws", "wss", "quic"]). --define(MARK_DEL, ?TOMBSTONE). --define(MARK_DEL_BIN, ?TOMBSTONE_BIN). +-define(MARK_DEL, ?TOMBSTONE_CONFIG_CHANGE_REQ). -spec id_example() -> atom(). id_example() -> 'tcp:default'. @@ -429,7 +428,7 @@ do_start_listener(quic, ListenerName, #{bind := Bind} = Opts) -> %% Update the listeners at runtime pre_config_update([listeners, Type, Name], {create, NewConf}, V) when - V =:= undefined orelse V =:= ?MARK_DEL_BIN + V =:= undefined orelse V =:= ?TOMBSTONE_VALUE -> CertsDir = certs_dir(Type, Name), {ok, convert_certs(CertsDir, NewConf)}; @@ -446,7 +445,7 @@ pre_config_update([listeners, _Type, _Name], {action, _Action, Updated}, RawConf NewConf = emqx_utils_maps:deep_merge(RawConf, Updated), {ok, NewConf}; pre_config_update([listeners, _Type, _Name], ?MARK_DEL, _RawConf) -> - {ok, ?MARK_DEL}; + {ok, ?TOMBSTONE_VALUE}; pre_config_update(_Path, _Request, RawConf) -> {ok, RawConf}. diff --git a/apps/emqx/src/emqx_schema.erl b/apps/emqx/src/emqx_schema.erl index f33dcb979..cf0f05abd 100644 --- a/apps/emqx/src/emqx_schema.erl +++ b/apps/emqx/src/emqx_schema.erl @@ -103,7 +103,6 @@ %% tombstone types -export([ - tombstone/0, tombstone_map/2, get_tombstone_map_value_type/1 ]). @@ -3194,7 +3193,7 @@ special_env(_Name) -> error. %% The tombstone atom. tombstone() -> - ?TOMBSTONE. + ?TOMBSTONE_TYPE. %% Make a map type, the value of which is allowed to be 'marked_for_deletion' %% 'marked_for_delition' is a special value which means the key is deleted. @@ -3203,7 +3202,7 @@ tombstone() -> tombstone_map(Name, Type) -> %% marked_for_deletion must be the last member of the union %% because we need to first union member to populate the default values - map(Name, ?UNION([Type, tombstone()])). + map(Name, ?UNION([Type, ?TOMBSTONE_TYPE])). %% inverse of mark_del_map get_tombstone_map_value_type(Schema) -> @@ -3220,7 +3219,7 @@ get_tombstone_map_value_type(Schema) -> keep_default_tombstone(Map, _Opts) when is_map(Map) -> maps:filter( fun(Key, Value) -> - Key =:= <<"default">> orelse Value =/= ?TOMBSTONE_BIN + Key =:= <<"default">> orelse Value =/= ?TOMBSTONE_VALUE end, Map ); diff --git a/apps/emqx_conf/src/emqx_conf.erl b/apps/emqx_conf/src/emqx_conf.erl index 1b37f652b..28fcaf624 100644 --- a/apps/emqx_conf/src/emqx_conf.erl +++ b/apps/emqx_conf/src/emqx_conf.erl @@ -18,6 +18,7 @@ -compile({no_auto_import, [get/1, get/2]}). -include_lib("emqx/include/logger.hrl"). -include_lib("hocon/include/hoconsc.hrl"). +-include_lib("emqx/include/emqx_schema.hrl"). -export([add_handler/2, remove_handler/1]). -export([get/1, get/2, get_raw/1, get_raw/2, get_all/1]). @@ -110,7 +111,7 @@ update(Node, KeyPath, UpdateReq, Opts) -> %% @doc Mark the specified key path as tombstone tombstone(KeyPath, Opts) -> - update(KeyPath, emqx_schema:tombstone(), Opts). + update(KeyPath, ?TOMBSTONE_CONFIG_CHANGE_REQ, Opts). %% @doc remove all value of key path in cluster-override.conf or local-override.conf. -spec remove(emqx_utils_maps:config_key_path(), emqx_config:update_opts()) ->