diff --git a/apps/emqx/test/emqx_crl_cache_SUITE.erl b/apps/emqx/test/emqx_crl_cache_SUITE.erl index 83675f11e..31738e980 100644 --- a/apps/emqx/test/emqx_crl_cache_SUITE.erl +++ b/apps/emqx/test/emqx_crl_cache_SUITE.erl @@ -1104,14 +1104,9 @@ do_t_validations(_Config) -> emqx_utils_json:decode(ResRaw1, [return_maps]), ?assertMatch( #{ - <<"mismatches">> := - #{ - <<"listeners:ssl_not_required_bind">> := - #{ - <<"reason">> := - <<"verify must be verify_peer when CRL check is enabled">> - } - } + <<"kind">> := <<"validation_error">>, + <<"reason">> := + <<"verify must be verify_peer when CRL check is enabled">> }, emqx_utils_json:decode(MsgRaw1, [return_maps]) ), diff --git a/apps/emqx/test/emqx_ocsp_cache_SUITE.erl b/apps/emqx/test/emqx_ocsp_cache_SUITE.erl index 8bf965cc3..fefa998f8 100644 --- a/apps/emqx/test/emqx_ocsp_cache_SUITE.erl +++ b/apps/emqx/test/emqx_ocsp_cache_SUITE.erl @@ -912,14 +912,9 @@ do_t_validations(_Config) -> emqx_utils_json:decode(ResRaw1, [return_maps]), ?assertMatch( #{ - <<"mismatches">> := - #{ - <<"listeners:ssl_not_required_bind">> := - #{ - <<"reason">> := - <<"The responder URL is required for OCSP stapling">> - } - } + <<"kind">> := <<"validation_error">>, + <<"reason">> := + <<"The responder URL is required for OCSP stapling">> }, emqx_utils_json:decode(MsgRaw1, [return_maps]) ), @@ -942,14 +937,9 @@ do_t_validations(_Config) -> emqx_utils_json:decode(ResRaw2, [return_maps]), ?assertMatch( #{ - <<"mismatches">> := - #{ - <<"listeners:ssl_not_required_bind">> := - #{ - <<"reason">> := - <<"The issuer PEM path is required for OCSP stapling">> - } - } + <<"kind">> := <<"validation_error">>, + <<"reason">> := + <<"The issuer PEM path is required for OCSP stapling">> }, emqx_utils_json:decode(MsgRaw2, [return_maps]) ), diff --git a/apps/emqx_bridge_kafka/test/emqx_bridge_kafka_impl_producer_SUITE.erl b/apps/emqx_bridge_kafka/test/emqx_bridge_kafka_impl_producer_SUITE.erl index e52f5b07b..6b7b961f6 100644 --- a/apps/emqx_bridge_kafka/test/emqx_bridge_kafka_impl_producer_SUITE.erl +++ b/apps/emqx_bridge_kafka/test/emqx_bridge_kafka_impl_producer_SUITE.erl @@ -132,7 +132,7 @@ t_query_mode(CtConfig) -> begin publish_with_config_template_parameters(CtConfig1, #{"query_mode" => "sync"}) end, - fun(RunStageResult, Trace) -> + fun(Trace) -> %% We should have a sync Snabbkaffe trace ?assertMatch([_], ?of_kind(emqx_bridge_kafka_impl_producer_sync_query, Trace)) end @@ -141,7 +141,7 @@ t_query_mode(CtConfig) -> begin publish_with_config_template_parameters(CtConfig1, #{"query_mode" => "async"}) end, - fun(RunStageResult, Trace) -> + fun(Trace) -> %% We should have a sync Snabbkaffe trace ?assertMatch([_], ?of_kind(emqx_bridge_kafka_impl_producer_async_query, Trace)) end diff --git a/apps/emqx_management/src/emqx_mgmt_api_listeners.erl b/apps/emqx_management/src/emqx_mgmt_api_listeners.erl index 5c2419ccf..fae9228a4 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_listeners.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_listeners.erl @@ -277,10 +277,39 @@ fields(Type) -> listener_schema(Opts) -> emqx_dashboard_swagger:schema_with_example( - ?UNION(lists:map(fun(#{ref := Ref}) -> Ref end, listeners_info(Opts))), + hoconsc:union(listener_union_member_selector(Opts)), tcp_schema_example() ). +listener_union_member_selector(Opts) -> + ListenersInfo = listeners_info(Opts), + Index = maps:from_list([ + {iolist_to_binary(ListenerType), Ref} + || #{listener_type := ListenerType, ref := Ref} <- ListenersInfo + ]), + fun + (all_union_members) -> + maps:values(Index); + ({value, V}) -> + case V of + #{<<"type">> := T} -> + case maps:get(T, Index, undefined) of + undefined -> + throw(#{ + field_name => type, + reason => <<"unknown listener type">> + }); + Ref -> + [Ref] + end; + _ -> + throw(#{ + field_name => type, + reason => <<"unknown listener type">> + }) + end + end. + create_listener_schema(Opts) -> Schemas = [ ?R_REF(Mod, {Type, with_name}) @@ -311,6 +340,7 @@ listeners_info(Opts) -> TypeAtom = list_to_existing_atom(ListenerType), #{ ref => ?R_REF(Ref), + listener_type => ListenerType, schema => [ {type, ?HOCON(?ENUM([TypeAtom]), #{desc => "Listener type", required => true})}, {running, ?HOCON(boolean(), #{desc => "Listener status", required => false})}, 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 b038863a8..a4ec3fe92 100644 --- a/apps/emqx_management/test/emqx_mgmt_api_listeners_SUITE.erl +++ b/apps/emqx_management/test/emqx_mgmt_api_listeners_SUITE.erl @@ -19,6 +19,7 @@ -compile(nowarn_export_all). -include_lib("eunit/include/eunit.hrl"). +-include_lib("common_test/include/ct.hrl"). -define(PORT(Base), (Base + ?LINE)). -define(PORT, ?PORT(20000)). @@ -404,6 +405,62 @@ t_action_listeners(Config) when is_list(Config) -> action_listener(ID, "start", true), action_listener(ID, "restart", true). +t_update_validation_error_message({init, Config}) -> + NewListenerId = <<"ssl:new", (integer_to_binary(?LINE))/binary>>, + NewPath = emqx_mgmt_api_test_util:api_path(["listeners", NewListenerId]), + ListenerId = "ssl:default", + OriginalPath = emqx_mgmt_api_test_util:api_path(["listeners", ListenerId]), + OriginalListener = request(get, OriginalPath, [], []), + [ + {new_listener_id, NewListenerId}, + {new_path, NewPath}, + {original_listener, OriginalListener} + | Config + ]; +t_update_validation_error_message(Config) when is_list(Config) -> + NewListenerId = ?config(new_listener_id, Config), + NewPath = ?config(new_path, Config), + OriginalListener = ?config(original_listener, Config), + Port = integer_to_binary(?PORT), + NewListener = OriginalListener#{ + <<"id">> := NewListenerId, + <<"bind">> => <<"0.0.0.0:", Port/binary>> + }, + CreateResp = request(post, NewPath, [], NewListener), + ?assertEqual(lists:sort(maps:keys(OriginalListener)), lists:sort(maps:keys(CreateResp))), + + %% check that a validation error is user-friendly + WrongConf1a = emqx_utils_maps:deep_put( + [<<"ssl_options">>, <<"enable_crl_check">>], + CreateResp, + true + ), + WrongConf1 = emqx_utils_maps:deep_put( + [<<"ssl_options">>, <<"verify">>], + WrongConf1a, + <<"verify_none">> + ), + Result1 = request(put, NewPath, [], WrongConf1, #{return_all => true}), + ?assertMatch({error, {{_, 400, _}, _Headers, _Body}}, Result1), + {error, {{_, _Code, _}, _Headers, Body1}} = Result1, + #{<<"message">> := RawMsg1} = emqx_utils_json:decode(Body1, [return_maps]), + Msg1 = emqx_utils_json:decode(RawMsg1, [return_maps]), + %% No confusing union type errors. + ?assertNotMatch(#{<<"mismatches">> := _}, Msg1), + ?assertMatch( + #{ + <<"kind">> := <<"validation_error">>, + <<"reason">> := <<"verify must be verify_peer when CRL check is enabled">>, + <<"value">> := #{} + }, + Msg1 + ), + ok; +t_update_validation_error_message({'end', Config}) -> + NewPath = ?config(new_path, Config), + ?assertEqual([], delete(NewPath)), + ok. + action_listener(ID, Action, Running) -> Path = emqx_mgmt_api_test_util:api_path(["listeners", ID, Action]), {ok, _} = emqx_mgmt_api_test_util:request_api(post, Path), @@ -413,8 +470,11 @@ action_listener(ID, Action, Running) -> listener_stats(Listener, Running). request(Method, Url, QueryParams, Body) -> + request(Method, Url, QueryParams, Body, _Opts = #{}). + +request(Method, Url, QueryParams, Body, Opts) -> AuthHeader = emqx_mgmt_api_test_util:auth_header_(), - case emqx_mgmt_api_test_util:request_api(Method, Url, QueryParams, AuthHeader, Body) of + case emqx_mgmt_api_test_util:request_api(Method, Url, QueryParams, AuthHeader, Body, Opts) of {ok, Res} -> emqx_utils_json:decode(Res, [return_maps]); Error -> Error end. diff --git a/changes/ce/fix-11030.en.md b/changes/ce/fix-11030.en.md new file mode 100644 index 000000000..6b2e2a95a --- /dev/null +++ b/changes/ce/fix-11030.en.md @@ -0,0 +1 @@ +Improved error messages when a validation error occurs while using the Listeners HTTP API.