Merge pull request #7449 from JimMoen/rm-authz-dry-run

fix(authz): rm authz source update dry_run

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

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}, #{}).
 

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).

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

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

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).
 

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).
 

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

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}

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")

apps/emqx_authz/test/emqx_authz_api_sources_SUITE.erl

@@ -99,11 +99,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,
@@ -283,11 +278,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">>,

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