Merge pull request #11030 from thalesmg/fix-listener-api-union-member-r51

fix(listener_mgmt_api): use union member selector fn for better error messages (r5.1)

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])
     ),

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])
     ),

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

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})},

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.

changes/ce/fix-11030.en.md

@@ -0,0 +1 @@
+Improved error messages when a validation error occurs while using the Listeners HTTP API.