From 7668753f5068b0fb4c2eb65cc2d4bba340e2f7cb Mon Sep 17 00:00:00 2001 From: zhongwencool Date: Wed, 14 Jun 2023 15:17:12 +0800 Subject: [PATCH 1/7] feat: hide zone/authn in listeners and remove /listeners/:id/authentication api --- apps/emqx/src/emqx_listeners.erl | 2 +- apps/emqx/src/emqx_schema.erl | 9 +++--- apps/emqx_authn/src/emqx_authn_api.erl | 15 +++++----- .../src/emqx_authn_user_import_api.erl | 5 ++-- apps/emqx_authn/test/emqx_authn_api_SUITE.erl | 28 ++++++++++--------- .../src/emqx_mgmt_api_configs.erl | 5 ++-- .../src/emqx_mgmt_api_listeners.erl | 3 +- .../test/emqx_mgmt_api_configs_SUITE.erl | 25 +++++++++-------- 8 files changed, 48 insertions(+), 44 deletions(-) diff --git a/apps/emqx/src/emqx_listeners.erl b/apps/emqx/src/emqx_listeners.erl index 410519a6c..dccd2d1a4 100644 --- a/apps/emqx/src/emqx_listeners.erl +++ b/apps/emqx/src/emqx_listeners.erl @@ -117,7 +117,7 @@ format_raw_listeners({Type0, Conf}) -> ({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), + LConf1 = maps:without([<<"authentication">>, <<"zone">>], LConf0), LConf2 = maps:put(<<"running">>, Running, LConf1), CurrConn = case Running of diff --git a/apps/emqx/src/emqx_schema.erl b/apps/emqx/src/emqx_schema.erl index 387f2d64e..40910614a 100644 --- a/apps/emqx/src/emqx_schema.erl +++ b/apps/emqx/src/emqx_schema.erl @@ -209,7 +209,7 @@ roots(high) -> map("name", ref("zone")), #{ desc => ?DESC(zones), - importance => ?IMPORTANCE_LOW + importance => ?IMPORTANCE_HIDDEN } )}, {?EMQX_AUTHENTICATION_CONFIG_ROOT_NAME, authentication(global)}, @@ -1794,7 +1794,8 @@ base_listener(Bind) -> atom(), #{ desc => ?DESC(base_listener_zone), - default => 'default' + default => 'default', + importance => ?IMPORTANCE_HIDDEN } )}, {"limiter", @@ -3409,7 +3410,7 @@ mqtt_general() -> )}, {"server_keepalive", sc( - hoconsc:union([integer(), disabled]), + hoconsc:union([pos_integer(), disabled]), #{ default => disabled, desc => ?DESC(mqtt_server_keepalive) @@ -3481,7 +3482,7 @@ mqtt_session() -> )}, {"max_awaiting_rel", sc( - hoconsc:union([integer(), infinity]), + hoconsc:union([non_neg_integer(), infinity]), #{ default => 100, desc => ?DESC(mqtt_max_awaiting_rel), diff --git a/apps/emqx_authn/src/emqx_authn_api.erl b/apps/emqx_authn/src/emqx_authn_api.erl index b237108c2..ca6a28f51 100644 --- a/apps/emqx_authn/src/emqx_authn_api.erl +++ b/apps/emqx_authn/src/emqx_authn_api.erl @@ -103,14 +103,15 @@ paths() -> "/authentication/:id/status", "/authentication/:id/position/:position", "/authentication/:id/users", - "/authentication/:id/users/:user_id", + "/authentication/:id/users/:user_id" - "/listeners/:listener_id/authentication", - "/listeners/:listener_id/authentication/:id", - "/listeners/:listener_id/authentication/:id/status", - "/listeners/:listener_id/authentication/:id/position/:position", - "/listeners/:listener_id/authentication/:id/users", - "/listeners/:listener_id/authentication/:id/users/:user_id" + %% hide listener authn api since 5.1.0 + %% "/listeners/:listener_id/authentication", + %% "/listeners/:listener_id/authentication/:id", + %% "/listeners/:listener_id/authentication/:id/status", + %% "/listeners/:listener_id/authentication/:id/position/:position", + %% "/listeners/:listener_id/authentication/:id/users", + %% "/listeners/:listener_id/authentication/:id/users/:user_id" ]. roots() -> diff --git a/apps/emqx_authn/src/emqx_authn_user_import_api.erl b/apps/emqx_authn/src/emqx_authn_user_import_api.erl index 86cfc6247..30836d3ba 100644 --- a/apps/emqx_authn/src/emqx_authn_user_import_api.erl +++ b/apps/emqx_authn/src/emqx_authn_user_import_api.erl @@ -48,8 +48,9 @@ api_spec() -> paths() -> [ - "/authentication/:id/import_users", - "/listeners/:listener_id/authentication/:id/import_users" + "/authentication/:id/import_users" + %% hide the deprecated api since 5.1.0 + %% "/listeners/:listener_id/authentication/:id/import_users" ]. schema("/authentication/:id/import_users") -> diff --git a/apps/emqx_authn/test/emqx_authn_api_SUITE.erl b/apps/emqx_authn/test/emqx_authn_api_SUITE.erl index 6d9203c95..c0b3fe22f 100644 --- a/apps/emqx_authn/test/emqx_authn_api_SUITE.erl +++ b/apps/emqx_authn/test/emqx_authn_api_SUITE.erl @@ -120,23 +120,23 @@ t_authenticator_position(_) -> t_authenticator_import_users(_) -> test_authenticator_import_users([]). -t_listener_authenticators(_) -> - test_authenticators(["listeners", ?TCP_DEFAULT]). +%t_listener_authenticators(_) -> +% test_authenticators(["listeners", ?TCP_DEFAULT]). -t_listener_authenticator(_) -> - test_authenticator(["listeners", ?TCP_DEFAULT]). +%t_listener_authenticator(_) -> +% test_authenticator(["listeners", ?TCP_DEFAULT]). -t_listener_authenticator_users(_) -> - test_authenticator_users(["listeners", ?TCP_DEFAULT]). +%t_listener_authenticator_users(_) -> +% test_authenticator_users(["listeners", ?TCP_DEFAULT]). -t_listener_authenticator_user(_) -> - test_authenticator_user(["listeners", ?TCP_DEFAULT]). +%t_listener_authenticator_user(_) -> +% test_authenticator_user(["listeners", ?TCP_DEFAULT]). -t_listener_authenticator_position(_) -> - test_authenticator_position(["listeners", ?TCP_DEFAULT]). +%t_listener_authenticator_position(_) -> +% test_authenticator_position(["listeners", ?TCP_DEFAULT]). -t_listener_authenticator_import_users(_) -> - test_authenticator_import_users(["listeners", ?TCP_DEFAULT]). +%t_listener_authenticator_import_users(_) -> +% test_authenticator_import_users(["listeners", ?TCP_DEFAULT]). t_aggregate_metrics(_) -> Metrics = #{ @@ -683,7 +683,9 @@ test_authenticator_import_users(PathPrefix) -> {filename, "user-credentials.csv", CSVData} ]). -t_switch_to_global_chain(_) -> +%% listener authn api is not supported since 5.1.0 +%% Don't support listener switch to global chain. +ignore_switch_to_global_chain(_) -> {ok, 200, _} = request( post, uri([?CONF_NS]), diff --git a/apps/emqx_management/src/emqx_mgmt_api_configs.erl b/apps/emqx_management/src/emqx_mgmt_api_configs.erl index 71e009589..114798756 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_configs.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_configs.erl @@ -43,9 +43,8 @@ <<"alarm">>, <<"sys_topics">>, <<"sysmon">>, - <<"log">>, - <<"persistent_session_store">>, - <<"zones">> + <<"log">> + %% <<"zones">> ]). api_spec() -> diff --git a/apps/emqx_management/src/emqx_mgmt_api_listeners.erl b/apps/emqx_management/src/emqx_mgmt_api_listeners.erl index fae9228a4..ec42090eb 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_listeners.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_listeners.erl @@ -825,8 +825,7 @@ tcp_schema_example() -> send_timeout => <<"15s">>, send_timeout_close => true }, - type => tcp, - zone => default + type => tcp }. create_listener(Body) -> 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 735db7ebb..8c2be7c9a 100644 --- a/apps/emqx_management/test/emqx_mgmt_api_configs_SUITE.erl +++ b/apps/emqx_management/test/emqx_mgmt_api_configs_SUITE.erl @@ -199,18 +199,19 @@ get_global_zone() -> update_global_zone(Change) -> update_config("global_zone", Change). -t_zones(_Config) -> - {ok, Zones} = get_config("zones"), - {ok, #{<<"mqtt">> := OldMqtt} = Zone1} = get_global_zone(), - Mqtt1 = maps:remove(<<"max_subscriptions">>, OldMqtt), - {ok, #{}} = update_config("zones", Zones#{<<"new_zone">> => Zone1#{<<"mqtt">> => Mqtt1}}), - NewMqtt = emqx_config:get_raw([zones, new_zone, mqtt]), - %% we remove max_subscription from global zone, so the new zone should not have it. - ?assertEqual(Mqtt1, NewMqtt), - %% delete the new zones - {ok, #{}} = update_config("zones", Zones), - ?assertEqual(undefined, emqx_config:get_raw([zones, new_zone], undefined)), - ok. +%% hide /configs/zones api in 5.1.0, so we comment this test. +%t_zones(_Config) -> +% {ok, Zones} = get_config("zones"), +% {ok, #{<<"mqtt">> := OldMqtt} = Zone1} = get_global_zone(), +% Mqtt1 = maps:remove(<<"max_subscriptions">>, OldMqtt), +% {ok, #{}} = update_config("zones", Zones#{<<"new_zone">> => Zone1#{<<"mqtt">> => Mqtt1}}), +% NewMqtt = emqx_config:get_raw([zones, new_zone, mqtt]), +% %% we remove max_subscription from global zone, so the new zone should not have it. +% ?assertEqual(Mqtt1, NewMqtt), +% %% delete the new zones +% {ok, #{}} = update_config("zones", Zones), +% ?assertEqual(undefined, emqx_config:get_raw([zones, new_zone], undefined)), +% ok. t_dashboard(_Config) -> {ok, Dashboard = #{<<"listeners">> := Listeners}} = get_config("dashboard"), From 0d4dbba059f520ff520477173cfa49b30845a252 Mon Sep 17 00:00:00 2001 From: zhongwencool Date: Wed, 14 Jun 2023 15:20:43 +0800 Subject: [PATCH 2/7] chore: update dashboard to e1.1.0-beta.5 --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 2a083b9d8..5316f1bb2 100644 --- a/Makefile +++ b/Makefile @@ -16,7 +16,7 @@ endif # Dashbord version # from https://github.com/emqx/emqx-dashboard5 export EMQX_DASHBOARD_VERSION ?= v1.2.6-beta.1 -export EMQX_EE_DASHBOARD_VERSION ?= e1.1.0-beta.4 +export EMQX_EE_DASHBOARD_VERSION ?= e1.1.0-beta.5 # `:=` should be used here, otherwise the `$(shell ...)` will be executed every time when the variable is used # In make 4.4+, for backward-compatibility the value from the original environment is used. From fa1519ad11d222f10fd31e6cb24596d24e71d0bd Mon Sep 17 00:00:00 2001 From: zhongwencool Date: Wed, 14 Jun 2023 15:24:18 +0800 Subject: [PATCH 3/7] chore: update 11045's changelog --- changes/ce/feat-11045.en.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changes/ce/feat-11045.en.md diff --git a/changes/ce/feat-11045.en.md b/changes/ce/feat-11045.en.md new file mode 100644 index 000000000..310ec9885 --- /dev/null +++ b/changes/ce/feat-11045.en.md @@ -0,0 +1 @@ +The listener's authentication and zone related apis have been officially removed in version `5.1.0`. From 121cd9124a044bdf784f6d36b9772c88b38c865d Mon Sep 17 00:00:00 2001 From: zhongwencool Date: Wed, 14 Jun 2023 15:53:55 +0800 Subject: [PATCH 4/7] chore: improve show_keys output --- apps/emqx_conf/src/emqx_conf_cli.erl | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/apps/emqx_conf/src/emqx_conf_cli.erl b/apps/emqx_conf/src/emqx_conf_cli.erl index 4231149d6..5f443ae7e 100644 --- a/apps/emqx_conf/src/emqx_conf_cli.erl +++ b/apps/emqx_conf/src/emqx_conf_cli.erl @@ -151,7 +151,8 @@ status() -> emqx_ctl:print("-----------------------------------------------\n"). print_keys(Config) -> - print(lists:sort(maps:keys(Config))). + Keys = lists:sort(maps:keys(Config)), + emqx_ctl:print("~1p~n", [[binary_to_existing_atom(K) || K <- Keys]]). print(Json) -> emqx_ctl:print("~ts~n", [emqx_logger_jsonfmt:best_effort_json(Json)]). @@ -166,11 +167,10 @@ get_config() -> drop_hidden_roots(AllConf). drop_hidden_roots(Conf) -> - Hidden = hidden_roots(), - maps:without(Hidden, Conf). + lists:foldl(fun(K, Acc) -> maps:remove(K, Acc) end, Conf, hidden_roots()). hidden_roots() -> - [trace, stats, broker]. + [<<"trace">>, <<"stats">>, <<"broker">>]. get_config(Key) -> case emqx:get_raw_config([Key], undefined) of From bd33dcb0494e035696cb0e21703e570291126ae3 Mon Sep 17 00:00:00 2001 From: zhongwencool Date: Wed, 14 Jun 2023 16:55:19 +0800 Subject: [PATCH 5/7] feat: remove zone from /clients api --- .../test/emqx_authn_enable_flag_SUITE.erl | 1 - apps/emqx_conf/src/emqx_conf_cli.erl | 2 +- .../src/emqx_mgmt_api_clients.erl | 16 ++-------------- 3 files changed, 3 insertions(+), 16 deletions(-) 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 98215e853..143a24152 100644 --- a/apps/emqx_authn/test/emqx_authn_enable_flag_SUITE.erl +++ b/apps/emqx_authn/test/emqx_authn_enable_flag_SUITE.erl @@ -75,7 +75,6 @@ listener_mqtt_tcp_conf(Port, EnableAuthn) -> PortS = integer_to_binary(Port), #{ <<"acceptors">> => 16, - <<"zone">> => <<"default">>, <<"access_rules">> => ["allow all"], <<"bind">> => <<"0.0.0.0:", PortS/binary>>, <<"max_connections">> => 1024000, diff --git a/apps/emqx_conf/src/emqx_conf_cli.erl b/apps/emqx_conf/src/emqx_conf_cli.erl index 5f443ae7e..eec35a61e 100644 --- a/apps/emqx_conf/src/emqx_conf_cli.erl +++ b/apps/emqx_conf/src/emqx_conf_cli.erl @@ -170,7 +170,7 @@ drop_hidden_roots(Conf) -> lists:foldl(fun(K, Acc) -> maps:remove(K, Acc) end, Conf, hidden_roots()). hidden_roots() -> - [<<"trace">>, <<"stats">>, <<"broker">>]. + [<<"trace">>, <<"stats">>, <<"broker">>, <<"persistent_session_store">>]. get_config(Key) -> case emqx:get_raw_config([Key], undefined) of diff --git a/apps/emqx_management/src/emqx_mgmt_api_clients.erl b/apps/emqx_management/src/emqx_mgmt_api_clients.erl index ec236c06c..d9418589d 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_clients.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_clients.erl @@ -63,7 +63,6 @@ -define(CLIENT_QSCHEMA, [ {<<"node">>, atom}, {<<"username">>, binary}, - {<<"zone">>, atom}, {<<"ip_address">>, ip}, {<<"conn_state">>, atom}, {<<"clean_start">>, atom}, @@ -122,11 +121,6 @@ schema("/clients") -> required => false, desc => <<"User name">> })}, - {zone, - hoconsc:mk(binary(), #{ - in => query, - required => false - })}, {ip_address, hoconsc:mk(binary(), #{ in => query, @@ -549,12 +543,7 @@ fields(client) -> " Maximum number of subscriptions allowed by this client">> })}, {username, hoconsc:mk(binary(), #{desc => <<"User name of client when connecting">>})}, - {mountpoint, hoconsc:mk(binary(), #{desc => <<"Topic mountpoint">>})}, - {zone, - hoconsc:mk(binary(), #{ - desc => - <<"Indicate the configuration group used by the client">> - })} + {mountpoint, hoconsc:mk(binary(), #{desc => <<"Topic mountpoint">>})} ]; fields(authz_cache) -> [ @@ -848,8 +837,6 @@ ms(clientid, X) -> #{clientinfo => #{clientid => X}}; ms(username, X) -> #{clientinfo => #{username => X}}; -ms(zone, X) -> - #{clientinfo => #{zone => X}}; ms(conn_state, X) -> #{conn_state => X}; ms(ip_address, X) -> @@ -930,6 +917,7 @@ format_channel_info(WhichNode, {_, ClientInfo0, ClientStats}) -> sockname, retry_interval, upgrade_qos, + zone, %% sessionID, defined in emqx_session.erl id ], From 9a0856fd6b13f821a0d759389fc309f9c129ce6e Mon Sep 17 00:00:00 2001 From: zhongwencool Date: Wed, 14 Jun 2023 18:00:36 +0800 Subject: [PATCH 6/7] fix: authn/authz's keys should be binary --- apps/emqx_conf/src/emqx_conf_cli.erl | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/apps/emqx_conf/src/emqx_conf_cli.erl b/apps/emqx_conf/src/emqx_conf_cli.erl index eec35a61e..70e9c3a5e 100644 --- a/apps/emqx_conf/src/emqx_conf_cli.erl +++ b/apps/emqx_conf/src/emqx_conf_cli.erl @@ -212,9 +212,9 @@ load_config(Path, ReplaceOrMerge) -> {error, bad_hocon_file} end. -update_config_cluster(?EMQX_AUTHORIZATION_CONFIG_ROOT_NAME = Key, Conf, merge) -> +update_config_cluster(?EMQX_AUTHORIZATION_CONFIG_ROOT_NAME_BINARY = Key, Conf, merge) -> check_res(Key, emqx_authz:merge(Conf)); -update_config_cluster(?EMQX_AUTHENTICATION_CONFIG_ROOT_NAME = Key, Conf, merge) -> +update_config_cluster(?EMQX_AUTHENTICATION_CONFIG_ROOT_NAME_BINARY = Key, Conf, merge) -> check_res(Key, emqx_authn:merge_config(Conf)); update_config_cluster(Key, NewConf, merge) -> Merged = merge_conf(Key, NewConf), @@ -223,9 +223,9 @@ update_config_cluster(Key, Value, replace) -> check_res(Key, emqx_conf:update([Key], Value, ?OPTIONS)). -define(LOCAL_OPTIONS, #{rawconf_with_defaults => true, persistent => false}). -update_config_local(?EMQX_AUTHORIZATION_CONFIG_ROOT_NAME = Key, Conf, merge) -> +update_config_local(?EMQX_AUTHORIZATION_CONFIG_ROOT_NAME_BINARY = Key, Conf, merge) -> check_res(node(), Key, emqx_authz:merge_local(Conf, ?LOCAL_OPTIONS)); -update_config_local(?EMQX_AUTHENTICATION_CONFIG_ROOT_NAME = Key, Conf, merge) -> +update_config_local(?EMQX_AUTHENTICATION_CONFIG_ROOT_NAME_BINARY = Key, Conf, merge) -> check_res(node(), Key, emqx_authn:merge_config_local(Conf, ?LOCAL_OPTIONS)); update_config_local(Key, NewConf, merge) -> Merged = merge_conf(Key, NewConf), From f6df52020c174383fd5f267c227dba1a0c948a0f Mon Sep 17 00:00:00 2001 From: zhongwencool Date: Wed, 14 Jun 2023 18:43:41 +0800 Subject: [PATCH 7/7] test: conf_cli failed with replace --- apps/emqx_conf/test/emqx_conf_cli_SUITE.erl | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/apps/emqx_conf/test/emqx_conf_cli_SUITE.erl b/apps/emqx_conf/test/emqx_conf_cli_SUITE.erl index 6eca6b620..c7701b431 100644 --- a/apps/emqx_conf/test/emqx_conf_cli_SUITE.erl +++ b/apps/emqx_conf/test/emqx_conf_cli_SUITE.erl @@ -33,14 +33,20 @@ init_per_suite(Config) -> end_per_suite(_Config) -> emqx_mgmt_api_test_util:end_suite([emqx_conf, emqx_authz]). -t_load_config_with(Config) -> +t_load_config(Config) -> Authz = authorization, Conf = emqx_conf:get_raw([Authz]), %% set sources to [] - ConfBin0 = hocon_pp:do(#{<<"authorization">> => #{<<"sources">> => []}}, #{}), + ConfBin = hocon_pp:do(#{<<"authorization">> => #{<<"sources">> => []}}, #{}), + ConfFile = prepare_conf_file(?FUNCTION_NAME, ConfBin, Config), + ok = emqx_conf_cli:conf(["load", "--replace", ConfFile]), + ?assertEqual(#{<<"sources">> => []}, emqx_conf:get_raw([Authz])), + + ConfBin0 = hocon_pp:do(#{<<"authorization">> => Conf#{<<"sources">> => []}}, #{}), ConfFile0 = prepare_conf_file(?FUNCTION_NAME, ConfBin0, Config), - ok = emqx_conf_cli:conf(["load", "--merge", ConfFile0]), + ok = emqx_conf_cli:conf(["load", "--replace", ConfFile0]), ?assertEqual(Conf#{<<"sources">> => []}, emqx_conf:get_raw([Authz])), + %% remove sources, it will reset to default file source. ConfBin1 = hocon_pp:do(#{<<"authorization">> => maps:remove(<<"sources">>, Conf)}, #{}), ConfFile1 = prepare_conf_file(?FUNCTION_NAME, ConfBin1, Config),