From a271ba5ff9ec53741a9f3ac85dd46fe6bc9e4bae Mon Sep 17 00:00:00 2001 From: Zhongwen Deng Date: Mon, 10 Apr 2023 14:52:49 +0800 Subject: [PATCH] feat: don't check_permission on local.conf --- apps/emqx/src/emqx_config_handler.erl | 117 +------------------ apps/emqx/test/emqx_config_handler_SUITE.erl | 49 -------- 2 files changed, 5 insertions(+), 161 deletions(-) diff --git a/apps/emqx/src/emqx_config_handler.erl b/apps/emqx/src/emqx_config_handler.erl index ee22c297e..3e18a172e 100644 --- a/apps/emqx/src/emqx_config_handler.erl +++ b/apps/emqx/src/emqx_config_handler.erl @@ -43,7 +43,6 @@ terminate/2, code_change/3 ]). --export([is_mutable/3]). -define(MOD, {mod}). -define(WKEY, '?'). @@ -230,26 +229,15 @@ process_update_request([_], _Handlers, {remove, _Opts}) -> process_update_request(ConfKeyPath, _Handlers, {remove, Opts}) -> OldRawConf = emqx_config:get_root_raw(ConfKeyPath), BinKeyPath = bin_path(ConfKeyPath), - case check_permissions(remove, BinKeyPath, OldRawConf, Opts) of - allow -> - NewRawConf = emqx_map_lib:deep_remove(BinKeyPath, OldRawConf), - OverrideConf = remove_from_override_config(BinKeyPath, Opts), - {ok, NewRawConf, OverrideConf, Opts}; - {deny, Reason} -> - {error, {permission_denied, Reason}} - end; + NewRawConf = emqx_map_lib:deep_remove(BinKeyPath, OldRawConf), + OverrideConf = remove_from_override_config(BinKeyPath, Opts), + {ok, NewRawConf, OverrideConf, Opts}; process_update_request(ConfKeyPath, Handlers, {{update, UpdateReq}, Opts}) -> OldRawConf = emqx_config:get_root_raw(ConfKeyPath), case do_update_config(ConfKeyPath, Handlers, OldRawConf, UpdateReq) of {ok, NewRawConf} -> - BinKeyPath = bin_path(ConfKeyPath), - case check_permissions(update, BinKeyPath, NewRawConf, Opts) of - allow -> - OverrideConf = merge_to_override_config(NewRawConf, Opts), - {ok, NewRawConf, OverrideConf, Opts}; - {deny, Reason} -> - {error, {permission_denied, Reason}} - end; + OverrideConf = merge_to_override_config(NewRawConf, Opts), + {ok, NewRawConf, OverrideConf, Opts}; Error -> Error end. @@ -550,98 +538,3 @@ load_prev_handlers() -> save_handlers(Handlers) -> application:set_env(emqx, ?MODULE, Handlers). - -check_permissions(_Action, _ConfKeyPath, _NewRawConf, #{override_to := local}) -> - allow; -check_permissions(Action, ConfKeyPath, NewRawConf, _Opts) -> - case emqx_map_lib:deep_find(ConfKeyPath, NewRawConf) of - {ok, NewRaw} -> - LocalOverride = emqx_config:read_override_conf(#{override_to => local}), - case emqx_map_lib:deep_find(ConfKeyPath, LocalOverride) of - {ok, LocalRaw} -> - case is_mutable(Action, NewRaw, LocalRaw) of - ok -> - allow; - {error, Error} -> - ?SLOG(error, #{ - msg => "prevent_remove_local_conf", - config_key_path => ConfKeyPath, - error => Error - }), - {deny, "Disable changed from local conf"} - end; - {not_found, _, _} -> - allow - end; - {not_found, _, _} -> - allow - end. - -is_mutable(Action, NewRaw, LocalRaw) -> - try - KeyPath = [], - is_mutable(KeyPath, Action, NewRaw, LocalRaw) - catch - throw:Error -> Error - end. - --define(REMOVE_FAILED, "remove_failed"). --define(UPDATE_FAILED, "update_failed"). - -is_mutable(KeyPath, Action, New = #{}, Local = #{}) -> - maps:foreach( - fun(Key, SubLocal) -> - case maps:find(Key, New) of - error -> ok; - {ok, SubNew} -> is_mutable(KeyPath ++ [Key], Action, SubNew, SubLocal) - end - end, - Local - ); -is_mutable(KeyPath, remove, Update, Origin) -> - throw({error, {?REMOVE_FAILED, KeyPath, Update, Origin}}); -is_mutable(_KeyPath, update, Val, Val) -> - ok; -is_mutable(KeyPath, update, Update, Origin) -> - throw({error, {?UPDATE_FAILED, KeyPath, Update, Origin}}). - --ifdef(TEST). --include_lib("eunit/include/eunit.hrl"). - -is_mutable_update_test() -> - Action = update, - ?assertEqual(ok, is_mutable(Action, #{}, #{})), - ?assertEqual(ok, is_mutable(Action, #{a => #{b => #{c => #{}}}}, #{a => #{b => #{c => #{}}}})), - ?assertEqual(ok, is_mutable(Action, #{a => #{b => #{c => 1}}}, #{a => #{b => #{c => 1}}})), - ?assertEqual( - {error, {?UPDATE_FAILED, [a, b, c], 1, 2}}, - is_mutable(Action, #{a => #{b => #{c => 1}}}, #{a => #{b => #{c => 2}}}) - ), - ?assertEqual( - {error, {?UPDATE_FAILED, [a, b, d], 2, 3}}, - is_mutable(Action, #{a => #{b => #{c => 1, d => 2}}}, #{a => #{b => #{c => 1, d => 3}}}) - ), - ok. - -is_mutable_remove_test() -> - Action = remove, - ?assertEqual(ok, is_mutable(Action, #{}, #{})), - ?assertEqual(ok, is_mutable(Action, #{a => #{b => #{c => #{}}}}, #{a1 => #{b => #{c => #{}}}})), - ?assertEqual(ok, is_mutable(Action, #{a => #{b => #{c => 1}}}, #{a => #{b1 => #{c => 1}}})), - ?assertEqual(ok, is_mutable(Action, #{a => #{b => #{c => 1}}}, #{a => #{b => #{c1 => 1}}})), - - ?assertEqual( - {error, {?REMOVE_FAILED, [a, b, c], 1, 1}}, - is_mutable(Action, #{a => #{b => #{c => 1}}}, #{a => #{b => #{c => 1}}}) - ), - ?assertEqual( - {error, {?REMOVE_FAILED, [a, b, c], 1, 2}}, - is_mutable(Action, #{a => #{b => #{c => 1}}}, #{a => #{b => #{c => 2}}}) - ), - ?assertEqual( - {error, {?REMOVE_FAILED, [a, b, c], 1, 1}}, - is_mutable(Action, #{a => #{b => #{c => 1, d => 2}}}, #{a => #{b => #{c => 1, d => 3}}}) - ), - ok. - --endif. diff --git a/apps/emqx/test/emqx_config_handler_SUITE.erl b/apps/emqx/test/emqx_config_handler_SUITE.erl index 6e2dbfef1..cea956825 100644 --- a/apps/emqx/test/emqx_config_handler_SUITE.erl +++ b/apps/emqx/test/emqx_config_handler_SUITE.erl @@ -200,55 +200,6 @@ t_sub_key_update_remove(_Config) -> ok = emqx_config_handler:remove_handler(KeyPath2), ok. -t_local_override_update_remove(_Config) -> - application:set_env(emqx, local_override_conf_file, ?LOCAL_CONF), - application:set_env(emqx, cluster_override_conf_file, ?CLUSTER_CONF), - KeyPath = [sysmon, os, cpu_high_watermark], - ok = emqx_config_handler:add_handler(KeyPath, ?MODULE), - LocalOpts = #{override_to => local}, - {ok, Res} = emqx:update_config(KeyPath, <<"70%">>, LocalOpts), - ?assertMatch( - #{ - config := 0.7, - post_config_update := #{}, - raw_config := <<"70%">> - }, - Res - ), - ClusterOpts = #{override_to => cluster}, - ?assertMatch( - {error, {permission_denied, _}}, emqx:update_config(KeyPath, <<"71%">>, ClusterOpts) - ), - ?assertMatch(0.7, emqx:get_config(KeyPath)), - - %% remove - ?assertMatch({error, {permission_denied, _}}, emqx:remove_config(KeyPath)), - ?assertEqual( - {ok, #{post_config_update => #{}}}, - emqx:remove_config(KeyPath, #{override_to => local}) - ), - ?assertEqual( - {ok, #{post_config_update => #{}}}, - emqx:remove_config(KeyPath) - ), - ?assertError({config_not_found, KeyPath}, emqx:get_raw_config(KeyPath)), - OSKey = maps:keys(emqx:get_raw_config([sysmon, os])), - ?assertEqual(false, lists:member(<<"cpu_high_watermark">>, OSKey)), - ?assert(length(OSKey) > 0), - - ?assertEqual( - {ok, #{config => 0.8, post_config_update => #{}, raw_config => <<"80%">>}}, - emqx:reset_config(KeyPath, ClusterOpts) - ), - OSKey1 = maps:keys(emqx:get_raw_config([sysmon, os])), - ?assertEqual(true, lists:member(<<"cpu_high_watermark">>, OSKey1)), - ?assert(length(OSKey1) > 1), - - ok = emqx_config_handler:remove_handler(KeyPath), - application:unset_env(emqx, local_override_conf_file), - application:unset_env(emqx, cluster_override_conf_file), - ok. - t_check_failed(_Config) -> KeyPath = [sysmon, os, cpu_check_interval], Opts = #{rawconf_with_defaults => true},