From 5c2a68076f7263a127e53c58b2fb281a59d1c580 Mon Sep 17 00:00:00 2001 From: Andrew Mayorov Date: Mon, 27 May 2024 12:31:19 +0200 Subject: [PATCH 1/9] fix(bridge-v2): report descriptive error on invalid update request Before this commit, generic validation errors were reported as union mismatches of _all_ of the bridge schemas. After this commit, specific schema is chosen before validation. --- apps/emqx_bridge/src/emqx_bridge_v2_api.erl | 19 +++++++++++- .../src/schema/emqx_bridge_v2_schema.erl | 21 ++++++++++++++ .../emqx_bridge_s3_aggreg_upload_SUITE.erl | 29 +++++++++++++++++-- .../src/emqx_dashboard_swagger.erl | 10 +++++-- 4 files changed, 74 insertions(+), 5 deletions(-) diff --git a/apps/emqx_bridge/src/emqx_bridge_v2_api.erl b/apps/emqx_bridge/src/emqx_bridge_v2_api.erl index 56b2cb4ed..b06424ea2 100644 --- a/apps/emqx_bridge/src/emqx_bridge_v2_api.erl +++ b/apps/emqx_bridge/src/emqx_bridge_v2_api.erl @@ -96,7 +96,7 @@ namespace() -> "actions_and_sources". api_spec() -> - emqx_dashboard_swagger:spec(?MODULE, #{check_schema => true}). + emqx_dashboard_swagger:spec(?MODULE, #{check_schema => fun check_api_schema/2}). paths() -> [ @@ -656,6 +656,23 @@ schema("/source_types") -> } }. +%%------------------------------------------------------------------------------ + +check_api_schema(Request, ReqMeta = #{path := Path = "/actions/:id", method := put}) -> + Spec = maps:get(put, schema(Path)), + BridgeId = emqx_utils_maps:deep_get([bindings, id], Request), + try emqx_bridge_resource:parse_bridge_id(BridgeId, #{atom_name => false}) of + {BridgeType, _Name} -> + Schema = emqx_bridge_v2_schema:action_api_schema("put", BridgeType), + SpecRefined = Spec#{'requestBody' => Schema}, + emqx_dashboard_swagger:filter_check_request(Request, ReqMeta#{apispec => SpecRefined}) + catch + throw:#{reason := Reason} -> + ?NOT_FOUND(<<"Invalid bridge ID, ", Reason/binary>>) + end; +check_api_schema(Request, ReqMeta) -> + emqx_dashboard_swagger:filter_check_request(Request, ReqMeta). + %%------------------------------------------------------------------------------ %% Thin Handlers %%------------------------------------------------------------------------------ diff --git a/apps/emqx_bridge/src/schema/emqx_bridge_v2_schema.erl b/apps/emqx_bridge/src/schema/emqx_bridge_v2_schema.erl index caec1f53c..d0b053467 100644 --- a/apps/emqx_bridge/src/schema/emqx_bridge_v2_schema.erl +++ b/apps/emqx_bridge/src/schema/emqx_bridge_v2_schema.erl @@ -31,6 +31,7 @@ actions_get_response/0, actions_put_request/0, actions_post_request/0, + action_api_schema/2, actions_examples/1, action_values/4 ]). @@ -100,6 +101,15 @@ actions_api_schema(Method) -> APISchemas = ?MODULE:registered_actions_api_schemas(Method), hoconsc:union(bridge_api_union(APISchemas)). +action_api_schema(Method, BridgeV2Type) -> + APISchemas = ?MODULE:registered_actions_api_schemas(Method), + case lists:keyfind(atom_to_binary(BridgeV2Type), 1, APISchemas) of + {_, SchemaRef} -> + hoconsc:mk(SchemaRef); + false -> + unknown_bridge_schema(BridgeV2Type) + end. + registered_actions_api_schemas(Method) -> RegisteredSchemas = emqx_action_info:registered_schema_modules_actions(), [ @@ -231,6 +241,17 @@ bridge_api_union(Refs) -> end end. +-dialyzer({nowarn_function, [unknown_bridge_schema/1]}). +unknown_bridge_schema(BridgeV2Type) -> + hoconsc:mk(typerefl:any(), #{ + validator => fun(_) -> + throw(#{ + value => BridgeV2Type, + reason => <<"unknown bridge type">> + }) + end + }). + -spec method_values(action | source, http_method(), atom()) -> schema_example_map(). method_values(Kind, post, Type) -> KindBin = atom_to_binary(Kind), diff --git a/apps/emqx_bridge_s3/test/emqx_bridge_s3_aggreg_upload_SUITE.erl b/apps/emqx_bridge_s3/test/emqx_bridge_s3_aggreg_upload_SUITE.erl index 09cf12329..b7c17bbaa 100644 --- a/apps/emqx_bridge_s3/test/emqx_bridge_s3_aggreg_upload_SUITE.erl +++ b/apps/emqx_bridge_s3/test/emqx_bridge_s3_aggreg_upload_SUITE.erl @@ -156,12 +156,15 @@ t_create_via_http(Config) -> t_on_get_status(Config) -> emqx_bridge_v2_testlib:t_on_get_status(Config, #{}). -t_invalid_config(Config) -> +t_create_invalid_config(Config) -> ?assertMatch( {error, {_Status, _, #{ <<"code">> := <<"BAD_REQUEST">>, - <<"message">> := #{<<"kind">> := <<"validation_error">>} + <<"message">> := #{ + <<"kind">> := <<"validation_error">>, + <<"reason">> := <<"Inconsistent 'min_part_size'", _/bytes>> + } }}}, emqx_bridge_v2_testlib:create_bridge_api( Config, @@ -174,6 +177,28 @@ t_invalid_config(Config) -> ) ). +t_update_invalid_config(Config) -> + ?assertMatch({ok, _Bridge}, emqx_bridge_v2_testlib:create_bridge(Config)), + ?assertMatch( + {error, + {_Status, _, #{ + <<"code">> := <<"BAD_REQUEST">>, + <<"message">> := #{ + <<"kind">> := <<"validation_error">>, + <<"reason">> := <<"Inconsistent 'min_part_size'", _/bytes>> + } + }}}, + emqx_bridge_v2_testlib:update_bridge_api( + Config, + _Overrides = #{ + <<"parameters">> => #{ + <<"min_part_size">> => <<"5GB">>, + <<"max_part_size">> => <<"100MB">> + } + } + ) + ). + t_aggreg_upload(Config) -> Bucket = ?config(s3_bucket, Config), BridgeName = ?config(bridge_name, Config), diff --git a/apps/emqx_dashboard/src/emqx_dashboard_swagger.erl b/apps/emqx_dashboard/src/emqx_dashboard_swagger.erl index 4ada5994c..29623bfba 100644 --- a/apps/emqx_dashboard/src/emqx_dashboard_swagger.erl +++ b/apps/emqx_dashboard/src/emqx_dashboard_swagger.erl @@ -335,8 +335,8 @@ filter_check_request_and_translate_body(Request, RequestMeta) -> filter_check_request(Request, RequestMeta) -> translate_req(Request, RequestMeta, fun check_only/3). -translate_req(Request, #{module := Module, path := Path, method := Method}, CheckFun) -> - #{Method := Spec} = apply(Module, schema, [Path]), +translate_req(Request, ReqMeta = #{module := Module}, CheckFun) -> + Spec = find_req_apispec(ReqMeta), try Params = maps:get(parameters, Spec, []), Body = maps:get('requestBody', Spec, []), @@ -349,6 +349,12 @@ translate_req(Request, #{module := Module, path := Path, method := Method}, Chec {400, 'BAD_REQUEST', Msg} end. +find_req_apispec(#{apispec := Spec}) -> + Spec; +find_req_apispec(#{module := Module, path := Path, method := Method}) -> + #{Method := Spec} = apply(Module, schema, [Path]), + Spec. + check_and_translate(Schema, Map, Opts) -> hocon_tconf:check_plain(Schema, Map, Opts). From ed7c29ec26d96e5682ab81613b9b97611a6989f3 Mon Sep 17 00:00:00 2001 From: Andrew Mayorov Date: Mon, 27 May 2024 16:57:45 +0200 Subject: [PATCH 2/9] fix(api): mention spec override in "request metadata" type --- apps/emqx_dashboard/src/emqx_dashboard_swagger.erl | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/apps/emqx_dashboard/src/emqx_dashboard_swagger.erl b/apps/emqx_dashboard/src/emqx_dashboard_swagger.erl index 29623bfba..a6005254a 100644 --- a/apps/emqx_dashboard/src/emqx_dashboard_swagger.erl +++ b/apps/emqx_dashboard/src/emqx_dashboard_swagger.erl @@ -91,7 +91,14 @@ -define(DEFAULT_ROW, 100). -type request() :: #{bindings => map(), query_string => map(), body => map()}. --type request_meta() :: #{module => module(), path => string(), method => atom()}. +-type request_meta() :: #{ + module := module(), + path := string(), + method := atom(), + %% API Operation specification override. + %% Takes precedence over the API specification defined in the module. + apispec => map() +}. %% More exact types are defined in minirest.hrl, but we don't want to include it %% because it defines a lot of types and they may clash with the types declared locally. From 08d88ea814257e19497fb4f0f7033e60f3861d84 Mon Sep 17 00:00:00 2001 From: Andrew Mayorov Date: Mon, 27 May 2024 17:14:37 +0200 Subject: [PATCH 3/9] feat(bridge-api): improve error messages for Update Source API --- apps/emqx_bridge/src/emqx_bridge_v2_api.erl | 25 ++++++++++++++++--- .../src/schema/emqx_bridge_v2_schema.erl | 22 +++++++++++++--- 2 files changed, 40 insertions(+), 7 deletions(-) diff --git a/apps/emqx_bridge/src/emqx_bridge_v2_api.erl b/apps/emqx_bridge/src/emqx_bridge_v2_api.erl index b06424ea2..8ba2ef487 100644 --- a/apps/emqx_bridge/src/emqx_bridge_v2_api.erl +++ b/apps/emqx_bridge/src/emqx_bridge_v2_api.erl @@ -658,21 +658,38 @@ schema("/source_types") -> %%------------------------------------------------------------------------------ -check_api_schema(Request, ReqMeta = #{path := Path = "/actions/:id", method := put}) -> - Spec = maps:get(put, schema(Path)), +check_api_schema(Request, ReqMeta = #{path := "/actions/:id", method := put}) -> BridgeId = emqx_utils_maps:deep_get([bindings, id], Request), try emqx_bridge_resource:parse_bridge_id(BridgeId, #{atom_name => false}) of + %% NOTE + %% Bridge type is known, refine the API schema to get more specific error messages. {BridgeType, _Name} -> Schema = emqx_bridge_v2_schema:action_api_schema("put", BridgeType), - SpecRefined = Spec#{'requestBody' => Schema}, - emqx_dashboard_swagger:filter_check_request(Request, ReqMeta#{apispec => SpecRefined}) + emqx_dashboard_swagger:filter_check_request(Request, refine_api_schema(Schema, ReqMeta)) catch throw:#{reason := Reason} -> ?NOT_FOUND(<<"Invalid bridge ID, ", Reason/binary>>) end; +check_api_schema(Request, ReqMeta = #{path := "/sources/:id", method := put}) -> + SourceId = emqx_utils_maps:deep_get([bindings, id], Request), + try emqx_bridge_resource:parse_bridge_id(SourceId, #{atom_name => false}) of + %% NOTE + %% Source type is known, refine the API schema to get more specific error messages. + {BridgeType, _Name} -> + Schema = emqx_bridge_v2_schema:source_api_schema("put", BridgeType), + emqx_dashboard_swagger:filter_check_request(Request, refine_api_schema(Schema, ReqMeta)) + catch + throw:#{reason := Reason} -> + ?NOT_FOUND(<<"Invalid source ID, ", Reason/binary>>) + end; check_api_schema(Request, ReqMeta) -> emqx_dashboard_swagger:filter_check_request(Request, ReqMeta). +refine_api_schema(Schema, ReqMeta = #{path := Path, method := Method}) -> + Spec = maps:get(Method, schema(Path)), + SpecRefined = Spec#{'requestBody' => Schema}, + ReqMeta#{apispec => SpecRefined}. + %%------------------------------------------------------------------------------ %% Thin Handlers %%------------------------------------------------------------------------------ diff --git a/apps/emqx_bridge/src/schema/emqx_bridge_v2_schema.erl b/apps/emqx_bridge/src/schema/emqx_bridge_v2_schema.erl index d0b053467..d7270d206 100644 --- a/apps/emqx_bridge/src/schema/emqx_bridge_v2_schema.erl +++ b/apps/emqx_bridge/src/schema/emqx_bridge_v2_schema.erl @@ -40,6 +40,7 @@ sources_get_response/0, sources_put_request/0, sources_post_request/0, + source_api_schema/2, sources_examples/1, source_values/4 ]). @@ -169,6 +170,15 @@ sources_api_schema(Method) -> APISchemas = ?MODULE:registered_sources_api_schemas(Method), hoconsc:union(bridge_api_union(APISchemas)). +source_api_schema(Method, SourceType) -> + APISchemas = ?MODULE:registered_sources_api_schemas(Method), + case lists:keyfind(atom_to_binary(SourceType), 1, APISchemas) of + {_, SchemaRef} -> + hoconsc:mk(SchemaRef); + false -> + unknown_source_schema(SourceType) + end. + registered_sources_api_schemas(Method) -> RegisteredSchemas = emqx_action_info:registered_schema_modules_sources(), [ @@ -241,13 +251,19 @@ bridge_api_union(Refs) -> end end. --dialyzer({nowarn_function, [unknown_bridge_schema/1]}). unknown_bridge_schema(BridgeV2Type) -> + erroneous_value_schema(BridgeV2Type, <<"unknown bridge type">>). + +unknown_source_schema(SourceType) -> + erroneous_value_schema(SourceType, <<"unknown source type">>). + +-dialyzer({nowarn_function, [erroneous_value_schema/2]}). +erroneous_value_schema(Value, Reason) -> hoconsc:mk(typerefl:any(), #{ validator => fun(_) -> throw(#{ - value => BridgeV2Type, - reason => <<"unknown bridge type">> + value => Value, + reason => Reason }) end }). From 663f4fd39f24dfda814e027e7d320cd24355704e Mon Sep 17 00:00:00 2001 From: Andrew Mayorov Date: Mon, 27 May 2024 17:23:08 +0200 Subject: [PATCH 4/9] chore: bump application versions --- apps/emqx_bridge/src/emqx_bridge.app.src | 2 +- apps/emqx_dashboard/src/emqx_dashboard.app.src | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/emqx_bridge/src/emqx_bridge.app.src b/apps/emqx_bridge/src/emqx_bridge.app.src index a4e2d6fd1..8abfb075e 100644 --- a/apps/emqx_bridge/src/emqx_bridge.app.src +++ b/apps/emqx_bridge/src/emqx_bridge.app.src @@ -1,7 +1,7 @@ %% -*- mode: erlang -*- {application, emqx_bridge, [ {description, "EMQX bridges"}, - {vsn, "0.2.0"}, + {vsn, "0.2.1"}, {registered, [emqx_bridge_sup]}, {mod, {emqx_bridge_app, []}}, {applications, [ diff --git a/apps/emqx_dashboard/src/emqx_dashboard.app.src b/apps/emqx_dashboard/src/emqx_dashboard.app.src index 7feefa04e..427f2958a 100644 --- a/apps/emqx_dashboard/src/emqx_dashboard.app.src +++ b/apps/emqx_dashboard/src/emqx_dashboard.app.src @@ -2,7 +2,7 @@ {application, emqx_dashboard, [ {description, "EMQX Web Dashboard"}, % strict semver, bump manually! - {vsn, "5.1.0"}, + {vsn, "5.1.1"}, {modules, []}, {registered, [emqx_dashboard_sup]}, {applications, [ From 16d14259828508a463d1cb8d8a97494924470bb3 Mon Sep 17 00:00:00 2001 From: ieQu1 <99872536+ieQu1@users.noreply.github.com> Date: Mon, 27 May 2024 23:24:44 +0200 Subject: [PATCH 5/9] refactor(emqx): Move files related to durable session to a subdir --- apps/emqx/src/emqx_persistent_session_ds.erl | 2 +- .../emqx_persistent_message_ds_gc_worker.erl | 2 +- .../emqx_persistent_session_bookkeeper.erl | 0 .../emqx_persistent_session_ds_gc_worker.erl | 2 +- .../emqx_persistent_session_ds_inflight.erl | 0 .../emqx_persistent_session_ds_router.erl | 0 .../emqx_persistent_session_ds_state.erl | 2 +- .../emqx_persistent_session_ds_stream_scheduler.erl | 2 +- .../emqx_persistent_session_ds_subs.erl | 2 +- .../emqx_persistent_session_ds_sup.erl | 0 .../session_internals.hrl} | 4 ++-- 11 files changed, 8 insertions(+), 8 deletions(-) rename apps/emqx/src/{ => emqx_persistent_session_ds}/emqx_persistent_message_ds_gc_worker.erl (99%) rename apps/emqx/src/{ => emqx_persistent_session_ds}/emqx_persistent_session_bookkeeper.erl (100%) rename apps/emqx/src/{ => emqx_persistent_session_ds}/emqx_persistent_session_ds_gc_worker.erl (99%) rename apps/emqx/src/{ => emqx_persistent_session_ds}/emqx_persistent_session_ds_inflight.erl (100%) rename apps/emqx/src/{ => emqx_persistent_session_ds}/emqx_persistent_session_ds_router.erl (100%) rename apps/emqx/src/{ => emqx_persistent_session_ds}/emqx_persistent_session_ds_state.erl (99%) rename apps/emqx/src/{ => emqx_persistent_session_ds}/emqx_persistent_session_ds_stream_scheduler.erl (99%) rename apps/emqx/src/{ => emqx_persistent_session_ds}/emqx_persistent_session_ds_subs.erl (99%) rename apps/emqx/src/{ => emqx_persistent_session_ds}/emqx_persistent_session_ds_sup.erl (100%) rename apps/emqx/src/{emqx_persistent_session_ds.hrl => emqx_persistent_session_ds/session_internals.hrl} (97%) diff --git a/apps/emqx/src/emqx_persistent_session_ds.erl b/apps/emqx/src/emqx_persistent_session_ds.erl index 4bc6b4183..1177cf653 100644 --- a/apps/emqx/src/emqx_persistent_session_ds.erl +++ b/apps/emqx/src/emqx_persistent_session_ds.erl @@ -25,7 +25,7 @@ -include("emqx_mqtt.hrl"). --include("emqx_persistent_session_ds.hrl"). +-include("emqx_persistent_session_ds/session_internals.hrl"). -ifdef(TEST). -include_lib("proper/include/proper.hrl"). diff --git a/apps/emqx/src/emqx_persistent_message_ds_gc_worker.erl b/apps/emqx/src/emqx_persistent_session_ds/emqx_persistent_message_ds_gc_worker.erl similarity index 99% rename from apps/emqx/src/emqx_persistent_message_ds_gc_worker.erl rename to apps/emqx/src/emqx_persistent_session_ds/emqx_persistent_message_ds_gc_worker.erl index e59d73db0..e9a00cd70 100644 --- a/apps/emqx/src/emqx_persistent_message_ds_gc_worker.erl +++ b/apps/emqx/src/emqx_persistent_session_ds/emqx_persistent_message_ds_gc_worker.erl @@ -21,7 +21,7 @@ -include_lib("stdlib/include/qlc.hrl"). -include_lib("stdlib/include/ms_transform.hrl"). --include("emqx_persistent_session_ds.hrl"). +-include("session_internals.hrl"). %% API -export([ diff --git a/apps/emqx/src/emqx_persistent_session_bookkeeper.erl b/apps/emqx/src/emqx_persistent_session_ds/emqx_persistent_session_bookkeeper.erl similarity index 100% rename from apps/emqx/src/emqx_persistent_session_bookkeeper.erl rename to apps/emqx/src/emqx_persistent_session_ds/emqx_persistent_session_bookkeeper.erl diff --git a/apps/emqx/src/emqx_persistent_session_ds_gc_worker.erl b/apps/emqx/src/emqx_persistent_session_ds/emqx_persistent_session_ds_gc_worker.erl similarity index 99% rename from apps/emqx/src/emqx_persistent_session_ds_gc_worker.erl rename to apps/emqx/src/emqx_persistent_session_ds/emqx_persistent_session_ds_gc_worker.erl index 9fe33beea..8b7205dc1 100644 --- a/apps/emqx/src/emqx_persistent_session_ds_gc_worker.erl +++ b/apps/emqx/src/emqx_persistent_session_ds/emqx_persistent_session_ds_gc_worker.erl @@ -21,7 +21,7 @@ -include_lib("stdlib/include/qlc.hrl"). -include_lib("stdlib/include/ms_transform.hrl"). --include("emqx_persistent_session_ds.hrl"). +-include("session_internals.hrl"). %% API -export([ diff --git a/apps/emqx/src/emqx_persistent_session_ds_inflight.erl b/apps/emqx/src/emqx_persistent_session_ds/emqx_persistent_session_ds_inflight.erl similarity index 100% rename from apps/emqx/src/emqx_persistent_session_ds_inflight.erl rename to apps/emqx/src/emqx_persistent_session_ds/emqx_persistent_session_ds_inflight.erl diff --git a/apps/emqx/src/emqx_persistent_session_ds_router.erl b/apps/emqx/src/emqx_persistent_session_ds/emqx_persistent_session_ds_router.erl similarity index 100% rename from apps/emqx/src/emqx_persistent_session_ds_router.erl rename to apps/emqx/src/emqx_persistent_session_ds/emqx_persistent_session_ds_router.erl diff --git a/apps/emqx/src/emqx_persistent_session_ds_state.erl b/apps/emqx/src/emqx_persistent_session_ds/emqx_persistent_session_ds_state.erl similarity index 99% rename from apps/emqx/src/emqx_persistent_session_ds_state.erl rename to apps/emqx/src/emqx_persistent_session_ds/emqx_persistent_session_ds_state.erl index 8f5f01e5c..1e8295e1d 100644 --- a/apps/emqx/src/emqx_persistent_session_ds_state.erl +++ b/apps/emqx/src/emqx_persistent_session_ds/emqx_persistent_session_ds_state.erl @@ -79,7 +79,7 @@ ]). -include("emqx_mqtt.hrl"). --include("emqx_persistent_session_ds.hrl"). +-include("session_internals.hrl"). -include_lib("snabbkaffe/include/trace.hrl"). -include_lib("stdlib/include/qlc.hrl"). diff --git a/apps/emqx/src/emqx_persistent_session_ds_stream_scheduler.erl b/apps/emqx/src/emqx_persistent_session_ds/emqx_persistent_session_ds_stream_scheduler.erl similarity index 99% rename from apps/emqx/src/emqx_persistent_session_ds_stream_scheduler.erl rename to apps/emqx/src/emqx_persistent_session_ds/emqx_persistent_session_ds_stream_scheduler.erl index c6a968a9a..7bf80cc2b 100644 --- a/apps/emqx/src/emqx_persistent_session_ds_stream_scheduler.erl +++ b/apps/emqx/src/emqx_persistent_session_ds/emqx_persistent_session_ds_stream_scheduler.erl @@ -29,7 +29,7 @@ -include_lib("emqx/include/logger.hrl"). -include("emqx_mqtt.hrl"). --include("emqx_persistent_session_ds.hrl"). +-include("session_internals.hrl"). %%================================================================================ %% Type declarations diff --git a/apps/emqx/src/emqx_persistent_session_ds_subs.erl b/apps/emqx/src/emqx_persistent_session_ds/emqx_persistent_session_ds_subs.erl similarity index 99% rename from apps/emqx/src/emqx_persistent_session_ds_subs.erl rename to apps/emqx/src/emqx_persistent_session_ds/emqx_persistent_session_ds_subs.erl index 8b4f70a69..3fa65eed5 100644 --- a/apps/emqx/src/emqx_persistent_session_ds_subs.erl +++ b/apps/emqx/src/emqx_persistent_session_ds/emqx_persistent_session_ds_subs.erl @@ -41,7 +41,7 @@ -export_type([subscription_state_id/0, subscription/0, subscription_state/0]). --include("emqx_persistent_session_ds.hrl"). +-include("session_internals.hrl"). -include("emqx_mqtt.hrl"). -include_lib("snabbkaffe/include/trace.hrl"). diff --git a/apps/emqx/src/emqx_persistent_session_ds_sup.erl b/apps/emqx/src/emqx_persistent_session_ds/emqx_persistent_session_ds_sup.erl similarity index 100% rename from apps/emqx/src/emqx_persistent_session_ds_sup.erl rename to apps/emqx/src/emqx_persistent_session_ds/emqx_persistent_session_ds_sup.erl diff --git a/apps/emqx/src/emqx_persistent_session_ds.hrl b/apps/emqx/src/emqx_persistent_session_ds/session_internals.hrl similarity index 97% rename from apps/emqx/src/emqx_persistent_session_ds.hrl rename to apps/emqx/src/emqx_persistent_session_ds/session_internals.hrl index fdbf2c6ea..d763a7d26 100644 --- a/apps/emqx/src/emqx_persistent_session_ds.hrl +++ b/apps/emqx/src/emqx_persistent_session_ds/session_internals.hrl @@ -13,8 +13,8 @@ %% See the License for the specific language governing permissions and %% limitations under the License. %%-------------------------------------------------------------------- --ifndef(EMQX_PERSISTENT_SESSION_DS_HRL_HRL). --define(EMQX_PERSISTENT_SESSION_DS_HRL_HRL, true). +-ifndef(EMQX_SESSION_DS_INTERNALS_HRL). +-define(EMQX_SESSION_DS_INTERNALS_HRL, true). -include("emqx_persistent_message.hrl"). From 04305d004fca741f1e5fb6f354640666d44d1ad8 Mon Sep 17 00:00:00 2001 From: ieQu1 <99872536+ieQu1@users.noreply.github.com> Date: Mon, 27 May 2024 23:36:20 +0200 Subject: [PATCH 6/9] refactor(sessds): Extract metadata keys to a header --- .../include/emqx_durable_session_metadata.hrl | 35 +++++++++++++++++++ .../session_internals.hrl | 14 +------- .../src/emqx_mgmt_api_clients.erl | 3 +- 3 files changed, 38 insertions(+), 14 deletions(-) create mode 100644 apps/emqx/include/emqx_durable_session_metadata.hrl diff --git a/apps/emqx/include/emqx_durable_session_metadata.hrl b/apps/emqx/include/emqx_durable_session_metadata.hrl new file mode 100644 index 000000000..74c037dca --- /dev/null +++ b/apps/emqx/include/emqx_durable_session_metadata.hrl @@ -0,0 +1,35 @@ +%%-------------------------------------------------------------------- +%% Copyright (c) 2022, 2024 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. +%%-------------------------------------------------------------------- + +%% @doc This header contains definitions of durable session metadata +%% keys, that can be consumed by the external code. +-ifndef(EMQX_DURABLE_SESSION_META_HRL). +-define(EMQX_DURABLE_SESSION_META_HRL, true). + +%% Session metadata keys: +-define(created_at, created_at). +-define(last_alive_at, last_alive_at). +-define(expiry_interval, expiry_interval). +%% Unique integer used to create unique identities: +-define(last_id, last_id). +%% Connection info (relevent for the dashboard): +-define(peername, peername). +-define(will_message, will_message). +-define(clientinfo, clientinfo). +-define(protocol, protocol). +-define(offline_info, offline_info). + +-endif. diff --git a/apps/emqx/src/emqx_persistent_session_ds/session_internals.hrl b/apps/emqx/src/emqx_persistent_session_ds/session_internals.hrl index d763a7d26..e18e2cb7f 100644 --- a/apps/emqx/src/emqx_persistent_session_ds/session_internals.hrl +++ b/apps/emqx/src/emqx_persistent_session_ds/session_internals.hrl @@ -17,6 +17,7 @@ -define(EMQX_SESSION_DS_INTERNALS_HRL, true). -include("emqx_persistent_message.hrl"). +-include("emqx_durable_session_metadata.hrl"). -define(SESSION_TAB, emqx_ds_session). -define(SESSION_SUBSCRIPTIONS_TAB, emqx_ds_session_subscriptions). @@ -70,17 +71,4 @@ sub_state_id :: emqx_persistent_session_ds_subs:subscription_state_id() }). -%% Session metadata keys: --define(created_at, created_at). --define(last_alive_at, last_alive_at). --define(expiry_interval, expiry_interval). -%% Unique integer used to create unique identities: --define(last_id, last_id). -%% Connection info (relevent for the dashboard): --define(peername, peername). --define(will_message, will_message). --define(clientinfo, clientinfo). --define(protocol, protocol). --define(offline_info, offline_info). - -endif. diff --git a/apps/emqx_management/src/emqx_mgmt_api_clients.erl b/apps/emqx_management/src/emqx_mgmt_api_clients.erl index ecef5b720..754209257 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_clients.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_clients.erl @@ -24,6 +24,7 @@ -include_lib("hocon/include/hoconsc.hrl"). -include_lib("emqx/include/logger.hrl"). -include_lib("emqx_utils/include/emqx_utils_api.hrl"). +-include_lib("emqx/include/emqx_durable_session_metadata.hrl"). -include("emqx_mgmt.hrl"). @@ -1739,7 +1740,7 @@ format_channel_info(undefined, {ClientId, PSInfo0 = #{}}, _Opts) -> format_persistent_session_info( _ClientId, #{ - metadata := #{offline_info := #{chan_info := ChanInfo, stats := Stats} = OfflineInfo} = + metadata := #{?offline_info := #{chan_info := ChanInfo, stats := Stats} = OfflineInfo} = Metadata } = PSInfo From 8fbeca43210f7a8615073509fabcdeeaa4b5f9bd Mon Sep 17 00:00:00 2001 From: ieQu1 <99872536+ieQu1@users.noreply.github.com> Date: Tue, 28 May 2024 00:14:01 +0200 Subject: [PATCH 7/9] chore: Version bumps --- apps/emqx/src/emqx.app.src | 2 +- apps/emqx_auth/src/emqx_auth.app.src | 2 +- apps/emqx_management/src/emqx_management.app.src | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/emqx/src/emqx.app.src b/apps/emqx/src/emqx.app.src index a5d9eb5e6..c391bc1c8 100644 --- a/apps/emqx/src/emqx.app.src +++ b/apps/emqx/src/emqx.app.src @@ -2,7 +2,7 @@ {application, emqx, [ {id, "emqx"}, {description, "EMQX Core"}, - {vsn, "5.3.0"}, + {vsn, "5.4.0"}, {modules, []}, {registered, []}, {applications, [ diff --git a/apps/emqx_auth/src/emqx_auth.app.src b/apps/emqx_auth/src/emqx_auth.app.src index 01bb7d381..35f3ff8b0 100644 --- a/apps/emqx_auth/src/emqx_auth.app.src +++ b/apps/emqx_auth/src/emqx_auth.app.src @@ -1,7 +1,7 @@ %% -*- mode: erlang -*- {application, emqx_auth, [ {description, "EMQX Authentication and authorization"}, - {vsn, "0.3.0"}, + {vsn, "0.4.0"}, {modules, []}, {registered, [emqx_auth_sup]}, {applications, [ diff --git a/apps/emqx_management/src/emqx_management.app.src b/apps/emqx_management/src/emqx_management.app.src index a1432f367..f01f5c7b9 100644 --- a/apps/emqx_management/src/emqx_management.app.src +++ b/apps/emqx_management/src/emqx_management.app.src @@ -2,7 +2,7 @@ {application, emqx_management, [ {description, "EMQX Management API and CLI"}, % strict semver, bump manually! - {vsn, "5.2.0"}, + {vsn, "5.3.0"}, {modules, []}, {registered, [emqx_management_sup]}, {applications, [ From c04aaad0a40f38096faa50d6b431f16571bfc1a8 Mon Sep 17 00:00:00 2001 From: Andrew Mayorov Date: Tue, 28 May 2024 10:31:14 +0200 Subject: [PATCH 8/9] chore(bridge-v2): leave comment describing need for `nowarn_function` --- apps/emqx_bridge/src/schema/emqx_bridge_v2_schema.erl | 2 ++ 1 file changed, 2 insertions(+) diff --git a/apps/emqx_bridge/src/schema/emqx_bridge_v2_schema.erl b/apps/emqx_bridge/src/schema/emqx_bridge_v2_schema.erl index d7270d206..6dbad456b 100644 --- a/apps/emqx_bridge/src/schema/emqx_bridge_v2_schema.erl +++ b/apps/emqx_bridge/src/schema/emqx_bridge_v2_schema.erl @@ -257,6 +257,8 @@ unknown_bridge_schema(BridgeV2Type) -> unknown_source_schema(SourceType) -> erroneous_value_schema(SourceType, <<"unknown source type">>). +%% @doc Construct a schema that always emits validation error. +%% We need to silence dialyzer because inner anonymous function always throws. -dialyzer({nowarn_function, [erroneous_value_schema/2]}). erroneous_value_schema(Value, Reason) -> hoconsc:mk(typerefl:any(), #{ From e9cc88cb9569f1b6afc11983384a39c20a40dd3f Mon Sep 17 00:00:00 2001 From: zmstone Date: Tue, 28 May 2024 10:57:20 +0200 Subject: [PATCH 9/9] test: fix test case to have deterministic base config --- .../test/emqx_mgmt_api_configs_2_SUITE.erl | 154 ++++++++++++++++++ .../test/emqx_mgmt_api_configs_SUITE.erl | 55 ------- 2 files changed, 154 insertions(+), 55 deletions(-) create mode 100644 apps/emqx_management/test/emqx_mgmt_api_configs_2_SUITE.erl diff --git a/apps/emqx_management/test/emqx_mgmt_api_configs_2_SUITE.erl b/apps/emqx_management/test/emqx_mgmt_api_configs_2_SUITE.erl new file mode 100644 index 000000000..93ffa3eb8 --- /dev/null +++ b/apps/emqx_management/test/emqx_mgmt_api_configs_2_SUITE.erl @@ -0,0 +1,154 @@ +%%-------------------------------------------------------------------- +%% Copyright (c) 2024 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. +%%-------------------------------------------------------------------- +-module(emqx_mgmt_api_configs_2_SUITE). + +-compile(export_all). +-compile(nowarn_export_all). + +-include_lib("eunit/include/eunit.hrl"). +-include_lib("common_test/include/ct.hrl"). + +all() -> + emqx_common_test_helpers:all(?MODULE). + +init_per_suite(Config) -> + _ = application:load(emqx), + CertDir = filename:join([code:lib_dir(emqx), "etc", "certs"]), + Cert = fun(Name) -> filename:join(CertDir, Name) end, + %% keep it the same as default conf in emqx_dashboard.conf + Conf = + [ + "dashboard.listeners.http { enable = true, bind = 18083 }", + "dashboard.listeners.https {\n", + " bind = 0 # disabled by default\n", + " ssl_options {\n", + " certfile = \"" ++ Cert("cert.pem") ++ "\"\n", + " keyfile = \"" ++ Cert("key.pem") ++ "\"\n", + " }\n" + "}\n" + ], + Apps = emqx_cth_suite:start( + [ + emqx_conf, + emqx_management, + emqx_mgmt_api_test_util:emqx_dashboard(Conf) + ], + #{work_dir => emqx_cth_suite:work_dir(Config)} + ), + [{suite_apps, Apps} | Config]. + +end_per_suite(Config) -> + ok = emqx_cth_suite:stop(?config(suite_apps, Config)). + +init_per_testcase(_TestCase, Config) -> + Config. + +end_per_testcase(_TestCase, Config) -> + Config. + +t_dashboard(_Config) -> + {ok, Dashboard = #{<<"listeners">> := Listeners}} = get_config("dashboard"), + Https1 = #{enable => true, bind => 18084}, + %% Ensure HTTPS listener can be enabled with just changing bind to a non-zero number + %% i.e. the default certs should work + ?assertMatch( + {ok, _}, + update_config("dashboard", Dashboard#{<<"listeners">> => Listeners#{<<"https">> => Https1}}) + ), + + Https2 = #{ + <<"bind">> => 18084, + <<"ssl_options">> => + #{ + <<"keyfile">> => "etc/certs/badkey.pem", + <<"cacertfile">> => "etc/certs/badcacert.pem", + <<"certfile">> => "etc/certs/badcert.pem" + } + }, + Dashboard2 = Dashboard#{<<"listeners">> => Listeners#{<<"https">> => Https2}}, + ?assertMatch( + {error, {"HTTP/1.1", 400, _}}, + update_config("dashboard", Dashboard2) + ), + + FilePath = fun(Name) -> + iolist_to_binary( + emqx_common_test_helpers:app_path(emqx, filename:join(["etc", "certs", Name])) + ) + end, + KeyFile = FilePath("key.pem"), + CertFile = FilePath("cert.pem"), + CacertFile = FilePath("cacert.pem"), + Https3 = #{ + <<"bind">> => 18084, + <<"ssl_options">> => #{ + <<"keyfile">> => KeyFile, + <<"cacertfile">> => CacertFile, + <<"certfile">> => CertFile + } + }, + Dashboard3 = Dashboard#{<<"listeners">> => Listeners#{<<"https">> => Https3}}, + ?assertMatch({ok, _}, update_config("dashboard", Dashboard3)), + + Dashboard4 = Dashboard#{<<"listeners">> => Listeners#{<<"https">> => #{<<"bind">> => 0}}}, + ?assertMatch({ok, _}, update_config("dashboard", Dashboard4)), + {ok, Dashboard41} = get_config("dashboard"), + ?assertMatch( + #{ + <<"bind">> := 0, + <<"ssl_options">> := + #{ + <<"keyfile">> := KeyFile, + <<"cacertfile">> := CacertFile, + <<"certfile">> := CertFile + } + }, + read_conf([<<"dashboard">>, <<"listeners">>, <<"https">>]), + Dashboard41 + ), + + ?assertMatch({ok, _}, update_config("dashboard", Dashboard)), + {ok, Dashboard1} = get_config("dashboard"), + ?assertEqual(Dashboard, Dashboard1), + timer:sleep(1500), + ok. + +%% Helpers + +get_config(Name) -> + Path = emqx_mgmt_api_test_util:api_path(["configs", Name]), + case emqx_mgmt_api_test_util:request_api(get, Path) of + {ok, Res} -> + {ok, emqx_utils_json:decode(Res, [return_maps])}; + Error -> + Error + end. + +update_config(Name, Change) -> + AuthHeader = emqx_mgmt_api_test_util:auth_header_(), + UpdatePath = emqx_mgmt_api_test_util:api_path(["configs", Name]), + case emqx_mgmt_api_test_util:request_api(put, UpdatePath, "", AuthHeader, Change) of + {ok, Update} -> {ok, emqx_utils_json:decode(Update, [return_maps])}; + Error -> Error + end. + +read_conf(RootKeys) when is_list(RootKeys) -> + case emqx_config:read_override_conf(#{override_to => cluster}) of + undefined -> undefined; + Conf -> emqx_utils_maps:deep_get(RootKeys, Conf, undefined) + end; +read_conf(RootKey) -> + read_conf([RootKey]). diff --git a/apps/emqx_management/test/emqx_mgmt_api_configs_SUITE.erl b/apps/emqx_management/test/emqx_mgmt_api_configs_SUITE.erl index 272f1e9be..a2d4d21af 100644 --- a/apps/emqx_management/test/emqx_mgmt_api_configs_SUITE.erl +++ b/apps/emqx_management/test/emqx_mgmt_api_configs_SUITE.erl @@ -225,61 +225,6 @@ update_global_zone(Change) -> % ?assertEqual(undefined, emqx_config:get_raw([zones, new_zone], undefined)), % ok. -t_dashboard(_Config) -> - {ok, Dashboard = #{<<"listeners">> := Listeners}} = get_config("dashboard"), - Https1 = #{enable => true, bind => 18084}, - ?assertMatch( - {error, {"HTTP/1.1", 400, _}}, - update_config("dashboard", Dashboard#{<<"listeners">> => Listeners#{<<"https">> => Https1}}) - ), - - Https2 = #{ - <<"bind">> => 18084, - <<"ssl_options">> => - #{ - <<"keyfile">> => "etc/certs/badkey.pem", - <<"cacertfile">> => "etc/certs/badcacert.pem", - <<"certfile">> => "etc/certs/badcert.pem" - } - }, - Dashboard2 = Dashboard#{<<"listeners">> => Listeners#{<<"https">> => Https2}}, - ?assertMatch( - {error, {"HTTP/1.1", 400, _}}, - update_config("dashboard", Dashboard2) - ), - - KeyFile = emqx_common_test_helpers:app_path(emqx, filename:join(["etc", "certs", "key.pem"])), - CertFile = emqx_common_test_helpers:app_path(emqx, filename:join(["etc", "certs", "cert.pem"])), - CacertFile = emqx_common_test_helpers:app_path( - emqx, filename:join(["etc", "certs", "cacert.pem"]) - ), - Https3 = #{ - <<"bind">> => 18084, - <<"ssl_options">> => #{ - <<"keyfile">> => list_to_binary(KeyFile), - <<"cacertfile">> => list_to_binary(CacertFile), - <<"certfile">> => list_to_binary(CertFile) - } - }, - Dashboard3 = Dashboard#{<<"listeners">> => Listeners#{<<"https">> => Https3}}, - ?assertMatch({ok, _}, update_config("dashboard", Dashboard3)), - - Dashboard4 = Dashboard#{<<"listeners">> => Listeners#{<<"https">> => #{<<"bind">> => 0}}}, - ?assertMatch({ok, _}, update_config("dashboard", Dashboard4)), - {ok, Dashboard41} = get_config("dashboard"), - ?assertEqual( - Https3#{<<"bind">> => 0}, - read_conf([<<"dashboard">>, <<"listeners">>, <<"https">>]), - Dashboard41 - ), - - ?assertMatch({ok, _}, update_config("dashboard", Dashboard)), - - {ok, Dashboard1} = get_config("dashboard"), - ?assertNotEqual(Dashboard, Dashboard1), - timer:sleep(1500), - ok. - %% v1 version json t_configs_node({'init', Config}) -> Node = node(),