diff --git a/apps/emqx_dashboard_sso/src/emqx_dashboard_sso_api.erl b/apps/emqx_dashboard_sso/src/emqx_dashboard_sso_api.erl index b7859948e..c023ace51 100644 --- a/apps/emqx_dashboard_sso/src/emqx_dashboard_sso_api.erl +++ b/apps/emqx_dashboard_sso/src/emqx_dashboard_sso_api.erl @@ -271,8 +271,6 @@ handle_backend_update_result(ok, _) -> 204; handle_backend_update_result({error, not_exists}, _) -> {404, #{code => ?BACKEND_NOT_FOUND, message => <<"Backend not found">>}}; -handle_backend_update_result({error, already_exists}, _) -> - {400, #{code => ?BAD_REQUEST, message => <<"Backend already exists">>}}; handle_backend_update_result({error, failed_to_load_metadata}, _) -> {400, #{code => ?BAD_REQUEST, message => <<"Failed to load metadata">>}}; handle_backend_update_result({error, Reason}, _) when is_binary(Reason) -> diff --git a/apps/emqx_dashboard_sso/src/emqx_dashboard_sso_manager.erl b/apps/emqx_dashboard_sso/src/emqx_dashboard_sso_manager.erl index c1b7ce5b8..a3512ac2f 100644 --- a/apps/emqx_dashboard_sso/src/emqx_dashboard_sso_manager.erl +++ b/apps/emqx_dashboard_sso/src/emqx_dashboard_sso_manager.erl @@ -24,8 +24,8 @@ -export([ running/0, - get_backend_status/2, lookup_state/1, + get_backend_status/2, make_resource_id/1, create_resource/3, update_resource/3 @@ -45,7 +45,7 @@ -define(MOD_KEY_PATH, [dashboard, sso]). -define(MOD_KEY_PATH(Sub), [dashboard, sso, Sub]). -define(RESOURCE_GROUP, <<"emqx_dashboard_sso">>). --define(START_ERROR_KEY, start_error). +-define(NO_ERROR, <<>>). -define(DEFAULT_RESOURCE_OPTS, #{ start_after_created => false }). @@ -53,7 +53,7 @@ -record(?MOD_TAB, { backend :: atom(), state :: map(), - last_error = <<>> :: term() + last_error = ?NO_ERROR :: term() }). %%------------------------------------------------------------------------------ @@ -65,7 +65,7 @@ start_link() -> running() -> lists:filtermap( fun - (#?MOD_TAB{backend = Backend, last_error = <<>>}) -> + (#?MOD_TAB{backend = Backend, last_error = ?NO_ERROR}) -> {true, Backend}; (_) -> false @@ -78,7 +78,7 @@ get_backend_status(Backend, false) -> backend => Backend, enable => false, running => false, - last_error => <<>> + last_error => ?NO_ERROR }; get_backend_status(Backend, _) -> case lookup(Backend) of @@ -87,7 +87,7 @@ get_backend_status(Backend, _) -> backend => Backend, enable => true, running => false, - last_error => <<"resource not found">> + last_error => <<"Resource not found">> }; Data -> maps:merge(#{backend => Backend, enable => true}, do_get_backend_status(Data)) @@ -179,54 +179,41 @@ start_backend_services() -> msg => "start_sso_backend_successfully", backend => Backend }), - ets:insert(?MOD_TAB, #?MOD_TAB{backend = Backend, state = State}); + update_state(Backend, State); {error, Reason} -> + SafeReason = emqx_utils:redact(Reason), + update_last_error(Backend, SafeReason), ?SLOG(error, #{ msg => "start_sso_backend_failed", backend => Backend, - reason => emqx_utils:redact(Reason) + reason => SafeReason }) - end, - record_start_error(Backend, false) + end end, maps:to_list(Backends) ). update_config(Backend, UpdateReq) -> - OkFun = fun(Result) -> - ?SLOG(info, #{ - msg => "update_sso_successfully", - backend => Backend, - result => emqx_utils:redact(Result) - }), - Result - end, - - ErrFun = fun({error, Reason} = Error) -> - SafeReason = emqx_utils:redact(Reason), - ?SLOG(error, #{ - msg => "update_sso_failed", - backend => Backend, - reason => SafeReason - }), - Error - end, - %% we always make sure the valid configuration will update successfully, %% ignore the runtime error during its update case emqx_conf:update(?MOD_KEY_PATH(Backend), UpdateReq, #{override_to => cluster}) of - {ok, UpdateResult} -> - #{post_config_update := #{?MODULE := Result}} = UpdateResult, - case Result of - ok -> - OkFun(Result); - {ok, _} -> - OkFun(Result); - {error, _} = Error -> - ErrFun(Error) + {ok, _UpdateResult} -> + case lookup(Backend) of + undefined -> + ok; + #?MOD_TAB{state = State, last_error = ?NO_ERROR} -> + {ok, State}; + Data -> + {error, Data#?MOD_TAB.last_error} end; - {error, _} = Error -> - ErrFun(Error) + {error, Reason} = Error -> + SafeReason = emqx_utils:redact(Reason), + ?SLOG(error, #{ + msg => "update_sso_failed", + backend => Backend, + reason => SafeReason + }), + Error end. pre_config_update(_, {update, _Backend, Config}, _OldConf) -> @@ -237,7 +224,8 @@ pre_config_update(_, {delete, _Backend}, _OldConf) -> {ok, null}. post_config_update(_, UpdateReq, NewConf, _OldConf, _AppEnvs) -> - {ok, on_config_update(UpdateReq, NewConf)}. + _ = on_config_update(UpdateReq, NewConf), + ok. propagated_post_config_update( ?MOD_KEY_PATH(BackendBin) = Path, _UpdateReq, undefined, OldConf, AppEnvs @@ -265,26 +253,23 @@ on_config_update({update, Backend, _RawConfig}, Config) -> on_backend_updated( Backend, emqx_dashboard_sso:create(Provider, Config), - fun(State, LastError) -> - ets:insert( - ?MOD_TAB, - #?MOD_TAB{backend = Backend, state = State, last_error = LastError} - ) + fun(State) -> + update_state(Backend, State) end ); Data -> on_backend_updated( Backend, emqx_dashboard_sso:update(Provider, Config, Data#?MOD_TAB.state), - fun(State, LastError) -> - ets:insert(?MOD_TAB, Data#?MOD_TAB{state = State, last_error = LastError}) + fun(State) -> + update_state(Backend, State) end ) end; on_config_update({delete, Backend}, _NewConf) -> case lookup(Backend) of undefined -> - {error, not_exists}; + on_backend_updated(Backend, {error, not_exists}, undefined); Data -> Provider = provider(Backend), on_backend_updated( @@ -306,31 +291,26 @@ lookup(Backend) -> %% to avoid resource leakage the resource start will never affect the update result, %% so the resource_id will always be recorded -start_resource_if_enabled(ResourceId, {ok, _} = Result, #{enable := true}) -> - clear_start_error(), +start_resource_if_enabled(ResourceId, {ok, _} = Result, #{enable := true, backend := Backend}) -> case emqx_resource:start(ResourceId) of ok -> ok; {error, Reason} -> SafeReason = emqx_utils:redact(Reason), - mark_start_error(SafeReason), ?SLOG(error, #{ msg => "start_backend_failed", resource_id => ResourceId, reason => SafeReason }), + update_last_error(Backend, SafeReason), ok end, Result; start_resource_if_enabled(_ResourceId, Result, _Config) -> Result. -on_backend_updated(Backend, {ok, State} = Ok, Fun) -> - Fun(State, <<>>), - record_start_error(Backend, true), - Ok; -on_backend_updated(_Backend, {ok, State, LastError} = Ok, Fun) -> - Fun(State, LastError), +on_backend_updated(_Backend, {ok, State} = Ok, Fun) -> + Fun(State), Ok; on_backend_updated(_Backend, ok, Fun) -> Fun(), @@ -373,29 +353,26 @@ new_ssl_source(Source, undefined) -> new_ssl_source(Source, SSL) -> Source#{<<"ssl">> => SSL}. -clear_start_error() -> - mark_start_error(<<>>). +update_state(Backend, State) -> + Data = ensure_backend_data(Backend), + ets:insert(?MOD_TAB, Data#?MOD_TAB{state = State}). -mark_start_error(Reason) -> - erlang:put(?START_ERROR_KEY, Reason). +update_last_error(Backend, LastError) -> + Data = ensure_backend_data(Backend), + ets:insert(?MOD_TAB, Data#?MOD_TAB{last_error = LastError}). -record_start_error(Backend, Force) -> - case erlang:get(?START_ERROR_KEY) of - <<>> when Force -> - update_last_error(Backend, <<>>); - <<>> -> - ok; - Reason -> - update_last_error(Backend, Reason) +ensure_backend_data(Backend) -> + case ets:lookup(?MOD_TAB, Backend) of + [Data] -> + Data; + [] -> + #?MOD_TAB{backend = Backend} end. -update_last_error(Backend, Reason) -> - ets:update_element(?MOD_TAB, Backend, {#?MOD_TAB.last_error, Reason}). - do_get_backend_status(#?MOD_TAB{state = #{resource_id := ResourceId}}) -> case emqx_resource_manager:lookup(ResourceId) of {ok, _Group, #{status := connected}} -> - #{running => true, last_error => <<>>}; + #{running => true, last_error => ?NO_ERROR}; {ok, _Group, #{status := Status}} -> #{ running => false, @@ -407,8 +384,8 @@ do_get_backend_status(#?MOD_TAB{state = #{resource_id := ResourceId}}) -> last_error => <<"Resource not found">> } end; -do_get_backend_status(#?MOD_TAB{last_error = <<>>}) -> - #{running => true, last_error => <<>>}; +do_get_backend_status(#?MOD_TAB{last_error = ?NO_ERROR}) -> + #{running => true, last_error => ?NO_ERROR}; do_get_backend_status(#?MOD_TAB{last_error = LastError}) -> #{ running => false,