From 4746204f6f0ff5ed0d0dd1350180b64b189c1580 Mon Sep 17 00:00:00 2001 From: JimMoen Date: Tue, 29 Mar 2022 18:09:50 +0800 Subject: [PATCH] fix(authz): rm authz source update dry_run --- apps/emqx_authz/src/emqx_authz.erl | 29 ------------------- .../emqx_authz/src/emqx_authz_api_sources.erl | 10 +------ apps/emqx_authz/src/emqx_authz_file.erl | 7 ----- apps/emqx_authz/src/emqx_authz_http.erl | 4 --- apps/emqx_authz/src/emqx_authz_mnesia.erl | 3 -- apps/emqx_authz/src/emqx_authz_mongodb.erl | 4 --- apps/emqx_authz/src/emqx_authz_mysql.erl | 4 --- apps/emqx_authz/src/emqx_authz_postgresql.erl | 4 --- apps/emqx_authz/src/emqx_authz_redis.erl | 4 --- apps/emqx_authz/test/emqx_authz_SUITE.erl | 1 - .../test/emqx_authz_api_sources_SUITE.erl | 9 ++---- .../emqx_authz/test/emqx_authz_http_SUITE.erl | 14 --------- 12 files changed, 3 insertions(+), 90 deletions(-) diff --git a/apps/emqx_authz/src/emqx_authz.erl b/apps/emqx_authz/src/emqx_authz.erl index 916b08632..3e5a53649 100644 --- a/apps/emqx_authz/src/emqx_authz.erl +++ b/apps/emqx_authz/src/emqx_authz.erl @@ -70,12 +70,6 @@ %% An authz backend will not be used after `destroy`. -callback(destroy(source()) -> ok). -%% Check if a configuration map is valid for further -%% authz backend initialization. -%% The callback must deallocate all resources allocated -%% during verification. --callback(dry_run(source()) -> ok | {error, term()}). - %% Authorize client action. -callback(authorize( emqx_types:clientinfo(), @@ -140,18 +134,6 @@ do_pre_config_update({?CMD_APPEND, Source}, Sources) -> NSources = Sources ++ [NSource], ok = check_dup_types(NSources), NSources; -do_pre_config_update({{?CMD_REPLACE, Type}, #{<<"enable">> := Enable} = Source}, Sources) - when ?IS_ENABLED(Enable) -> - NSource = maybe_write_files(Source), - {_Old, Front, Rear} = take(Type, Sources), - case create_dry_run(Type, NSource) of - ok -> - NSources = Front ++ [NSource | Rear], - ok = check_dup_types(NSources), - NSources; - {error, _} = Error -> - throw(Error) - end; do_pre_config_update({{?CMD_REPLACE, Type}, Source}, Sources) -> NSource = maybe_write_files(Source), {_Old, Front, Rear} = take(Type, Sources), @@ -250,11 +232,6 @@ check_dup_types([Source | Sources], Checked) -> check_dup_types(Sources, [Type | Checked]) end. -create_dry_run(Type, Source) -> - [CheckedSource] = check_sources([Source]), - Module = authz_module(Type), - Module:dry_run(CheckedSource). - init_sources(Sources) -> {_Enabled, Disabled} = lists:partition(fun(#{enable := Enable}) -> Enable end, Sources), case Disabled =/= [] of @@ -329,12 +306,6 @@ do_authorize(Client, PubSub, Topic, %% Internal function %%-------------------------------------------------------------------- -check_sources(RawSources) -> - Schema = #{roots => emqx_authz_schema:fields("authorization"), fields => #{}}, - Conf = #{<<"sources">> => RawSources}, - #{sources := Sources} = hocon_tconf:check_plain(Schema, Conf, #{atom_key => true}), - Sources. - take(Type) -> take(Type, lookup()). %% Take the source of give type, the sources list is split into two parts diff --git a/apps/emqx_authz/src/emqx_authz_api_sources.erl b/apps/emqx_authz/src/emqx_authz_api_sources.erl index 5a62e11e1..22a4f0f86 100644 --- a/apps/emqx_authz/src/emqx_authz_api_sources.erl +++ b/apps/emqx_authz/src/emqx_authz_api_sources.erl @@ -194,15 +194,7 @@ source(get, #{bindings := #{type := Type}}) -> source(put, #{bindings := #{type := <<"file">>}, body := #{<<"type">> := <<"file">>} = Body}) -> update_authz_file(Body); source(put, #{bindings := #{type := Type}, body := Body}) -> - case emqx_authz:update({?CMD_REPLACE, Type}, Body) of - {ok, _} -> {204}; - {error, {emqx_conf_schema, _}} -> - {400, #{code => <<"BAD_REQUEST">>, - message => <<"BAD_SCHEMA">>}}; - {error, Reason} -> - {400, #{code => <<"BAD_REQUEST">>, - message => bin(Reason)}} - end; + update_config({?CMD_REPLACE, Type}, Body); source(delete, #{bindings := #{type := Type}}) -> update_config({?CMD_DELETE, Type}, #{}). diff --git a/apps/emqx_authz/src/emqx_authz_file.erl b/apps/emqx_authz/src/emqx_authz_file.erl index b9a4284e5..729646714 100644 --- a/apps/emqx_authz/src/emqx_authz_file.erl +++ b/apps/emqx_authz/src/emqx_authz_file.erl @@ -30,7 +30,6 @@ -export([ description/0 , init/1 , destroy/1 - , dry_run/1 , authorize/4 ]). @@ -54,11 +53,5 @@ init(#{path := Path} = Source) -> destroy(_Source) -> ok. -dry_run(#{path := Path}) -> - case file:consult(Path) of - {ok, _} -> ok; - {error, _} = Error -> Error - end. - authorize(Client, PubSub, Topic, #{annotations := #{rules := Rules}}) -> emqx_authz_rule:matches(Client, PubSub, Topic, Rules). diff --git a/apps/emqx_authz/src/emqx_authz_http.erl b/apps/emqx_authz/src/emqx_authz_http.erl index 3c11660d7..592c59b03 100644 --- a/apps/emqx_authz/src/emqx_authz_http.erl +++ b/apps/emqx_authz/src/emqx_authz_http.erl @@ -27,7 +27,6 @@ -export([ description/0 , init/1 , destroy/1 - , dry_run/1 , authorize/4 , parse_url/1 ]). @@ -58,9 +57,6 @@ init(Config) -> destroy(#{annotations := #{id := Id}}) -> ok = emqx_resource:remove_local(Id). -dry_run(Config) -> - emqx_resource:create_dry_run_local(emqx_connector_http, parse_config(Config)). - authorize( Client , PubSub , Topic diff --git a/apps/emqx_authz/src/emqx_authz_mnesia.erl b/apps/emqx_authz/src/emqx_authz_mnesia.erl index 9f4e0ffeb..f02419b0c 100644 --- a/apps/emqx_authz/src/emqx_authz_mnesia.erl +++ b/apps/emqx_authz/src/emqx_authz_mnesia.erl @@ -47,7 +47,6 @@ -export([ description/0 , init/1 , destroy/1 - , dry_run/1 , authorize/4 ]). @@ -90,8 +89,6 @@ init(Source) -> Source. destroy(_Source) -> ok. -dry_run(_Source) -> ok. - authorize(#{username := Username, clientid := Clientid } = Client, PubSub, Topic, #{type := 'built_in_database'}) -> diff --git a/apps/emqx_authz/src/emqx_authz_mongodb.erl b/apps/emqx_authz/src/emqx_authz_mongodb.erl index 61c23f3a9..158bff152 100644 --- a/apps/emqx_authz/src/emqx_authz_mongodb.erl +++ b/apps/emqx_authz/src/emqx_authz_mongodb.erl @@ -27,7 +27,6 @@ -export([ description/0 , init/1 , destroy/1 - , dry_run/1 , authorize/4 ]). @@ -52,9 +51,6 @@ init(#{selector := Selector} = Source) -> ?PLACEHOLDERS)} end. -dry_run(Source) -> - emqx_resource:create_dry_run_local(emqx_connector_mongo, Source). - destroy(#{annotations := #{id := Id}}) -> ok = emqx_resource:remove_local(Id). diff --git a/apps/emqx_authz/src/emqx_authz_mysql.erl b/apps/emqx_authz/src/emqx_authz_mysql.erl index c864d8ecc..31e8dcd98 100644 --- a/apps/emqx_authz/src/emqx_authz_mysql.erl +++ b/apps/emqx_authz/src/emqx_authz_mysql.erl @@ -27,7 +27,6 @@ -export([ description/0 , init/1 , destroy/1 - , dry_run/1 , authorize/4 ]). @@ -56,9 +55,6 @@ init(#{query := SQL} = Source) -> ?PLACEHOLDERS)}} end. -dry_run(Source) -> - emqx_resource:create_dry_run_local(emqx_connector_mysql, Source). - destroy(#{annotations := #{id := Id}}) -> ok = emqx_resource:remove_local(Id). diff --git a/apps/emqx_authz/src/emqx_authz_postgresql.erl b/apps/emqx_authz/src/emqx_authz_postgresql.erl index 9a783ddcf..275b39f2b 100644 --- a/apps/emqx_authz/src/emqx_authz_postgresql.erl +++ b/apps/emqx_authz/src/emqx_authz_postgresql.erl @@ -27,7 +27,6 @@ -export([ description/0 , init/1 , destroy/1 - , dry_run/1 , authorize/4 ]). @@ -68,9 +67,6 @@ init(#{query := SQL0} = Source) -> destroy(#{annotations := #{id := Id}}) -> ok = emqx_resource:remove_local(Id). -dry_run(Source) -> - emqx_resource:create_dry_run_local(emqx_connector_pgsql, Source). - authorize(Client, PubSub, Topic, #{annotations := #{id := ResourceID, placeholders := Placeholders diff --git a/apps/emqx_authz/src/emqx_authz_redis.erl b/apps/emqx_authz/src/emqx_authz_redis.erl index 4e67746ec..7e9ea8325 100644 --- a/apps/emqx_authz/src/emqx_authz_redis.erl +++ b/apps/emqx_authz/src/emqx_authz_redis.erl @@ -27,7 +27,6 @@ -export([ description/0 , init/1 , destroy/1 - , dry_run/1 , authorize/4 ]). @@ -57,9 +56,6 @@ init(#{cmd := CmdStr} = Source) -> destroy(#{annotations := #{id := Id}}) -> ok = emqx_resource:remove_local(Id). -dry_run(Source) -> - emqx_resource:create_dry_run_local(emqx_connector_redis, Source). - authorize(Client, PubSub, Topic, #{cmd_template := CmdTemplate, annotations := #{id := ResourceID} diff --git a/apps/emqx_authz/test/emqx_authz_SUITE.erl b/apps/emqx_authz/test/emqx_authz_SUITE.erl index 605a8782b..2d2648913 100644 --- a/apps/emqx_authz/test/emqx_authz_SUITE.erl +++ b/apps/emqx_authz/test/emqx_authz_SUITE.erl @@ -33,7 +33,6 @@ init_per_suite(Config) -> meck:new(emqx_resource, [non_strict, passthrough, no_history, no_link]), meck:expect(emqx_resource, create_local, fun(_, _, _, _) -> {ok, meck_data} end), meck:expect(emqx_resource, remove_local, fun(_) -> ok end), - meck:expect(emqx_resource, create_dry_run_local, fun(_, _) -> ok end), meck:expect(emqx_authz, acl_conf_file, fun() -> emqx_common_test_helpers:deps_path(emqx_authz, "etc/acl.conf") diff --git a/apps/emqx_authz/test/emqx_authz_api_sources_SUITE.erl b/apps/emqx_authz/test/emqx_authz_api_sources_SUITE.erl index 72d43d23d..7e7d5db09 100644 --- a/apps/emqx_authz/test/emqx_authz_api_sources_SUITE.erl +++ b/apps/emqx_authz/test/emqx_authz_api_sources_SUITE.erl @@ -101,11 +101,6 @@ init_per_suite(Config) -> ok = stop_apps([emqx_resource, emqx_connector]), meck:new(emqx_resource, [non_strict, passthrough, no_history, no_link]), meck:expect(emqx_resource, create_local, fun(_, _, _, _) -> {ok, meck_data} end), - meck:expect(emqx_resource, create_dry_run_local, - fun(emqx_connector_mysql, _) -> ok; - (emqx_connector_mongo, _) -> ok; - (T, C) -> meck:passthrough([T, C]) - end), meck:expect(emqx_resource, health_check, fun(St) -> {ok, St} end), meck:expect(emqx_resource, remove_local, fun(_) -> ok end ), meck:expect(emqx_authz, acl_conf_file, @@ -294,11 +289,11 @@ t_api(_) -> uri(["authorization", "sources", "mysql"]), ?SOURCE3#{<<"server">> := <<"192.168.1.100:3306">>}), - {ok, 400, _} = request( + {ok, 204, _} = request( put, uri(["authorization", "sources", "postgresql"]), ?SOURCE4#{<<"server">> := <<"fake">>}), - {ok, 400, _} = request( + {ok, 204, _} = request( put, uri(["authorization", "sources", "redis"]), ?SOURCE5#{<<"servers">> := [<<"192.168.1.100:6379">>, diff --git a/apps/emqx_authz/test/emqx_authz_http_SUITE.erl b/apps/emqx_authz/test/emqx_authz_http_SUITE.erl index 42a265a9e..e6aeb703c 100644 --- a/apps/emqx_authz/test/emqx_authz_http_SUITE.erl +++ b/apps/emqx_authz/test/emqx_authz_http_SUITE.erl @@ -294,20 +294,6 @@ t_create_replace(_Config) -> allow, emqx_access_control:authorize(ClientInfo, publish, <<"t">>)), - %% Changing to other bad config does not work - BadConfig = maps:merge( - raw_http_authz_config(), - #{<<"url">> => - <<"http://127.0.0.1:33332/authz/users/?topic=${topic}&action=${action}">>}), - - ?assertMatch( - {error, _}, - emqx_authz:update({?CMD_REPLACE, http}, BadConfig)), - - ?assertEqual( - allow, - emqx_access_control:authorize(ClientInfo, publish, <<"t">>)), - %% Changing to valid config OkConfig = maps:merge( raw_http_authz_config(),