From ee6383b7e1017ad737b2400503ba1a69b9af0abd Mon Sep 17 00:00:00 2001 From: Nelson Vides Date: Tue, 18 Oct 2022 13:59:27 +0200 Subject: [PATCH 1/6] Test that a domain can be queued for async deletion --- big_tests/tests/domain_rest_helper.erl | 8 ++++++++ big_tests/tests/service_domain_db_SUITE.erl | 15 ++++++++++++++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/big_tests/tests/domain_rest_helper.erl b/big_tests/tests/domain_rest_helper.erl index b41d77e8c1..dc3bddd032 100644 --- a/big_tests/tests/domain_rest_helper.erl +++ b/big_tests/tests/domain_rest_helper.erl @@ -11,6 +11,7 @@ putt_domain_with_custom_body/2, rest_select_domain/2, rest_delete_domain/3, + request_delete_domain/3, delete_custom/4, patch_custom/4]). @@ -73,6 +74,13 @@ rest_delete_domain(Config, Domain, HostType) -> creds => make_creds(Config), body => Params }). +request_delete_domain(Config, Domain, HostType) -> + Params = #{<<"host_type">> => HostType, <<"request">> => true}, + rest_helper:make_request(#{ role => admin, method => <<"DELETE">>, + path => domain_path(Domain), + creds => make_creds(Config), + body => Params }). + delete_custom(Config, Role, Path, Body) -> rest_helper:make_request(#{ role => Role, method => <<"DELETE">>, path => Path, creds => make_creds(Config), diff --git a/big_tests/tests/service_domain_db_SUITE.erl b/big_tests/tests/service_domain_db_SUITE.erl index 23aca71251..95fe99e961 100644 --- a/big_tests/tests/service_domain_db_SUITE.erl +++ b/big_tests/tests/service_domain_db_SUITE.erl @@ -130,11 +130,11 @@ rest_auth_cases(true) -> rest_cannot_enable_domain_without_auth, rest_cannot_disable_domain_without_auth, rest_cannot_select_domain_without_auth]. - rest_cases() -> [rest_can_insert_domain, rest_can_disable_domain, rest_can_delete_domain, + rest_request_can_delete_domain, rest_cannot_delete_domain_without_correct_type, rest_delete_missing_domain, rest_cannot_insert_domain_twice_with_another_host_type, @@ -821,6 +821,19 @@ rest_can_disable_domain(Config) -> {ok, #{host_type := <<"type1">>, status := disabled}} = select_domain(mim(), <<"example.db">>). +rest_request_can_delete_domain(Config) -> + %% Put a new domain to delete later + rest_put_domain(Config, <<"example.db">>, <<"type1">>), + %% Request delete domain + {{<<"202">>, _}, _} = domain_rest_helper:request_delete_domain(Config, <<"example.db">>, <<"type1">>), + %% Wait until it is not found anymore + Return = {{<<"404">>, <<"Not Found">>}, <<"Domain not found">>}, + F1 = fun() -> rest_select_domain(Config, <<"example.db">>) end, + mongoose_helper:wait_until(F1, Return, #{time_left => timer:seconds(15)}), + %% Double-check + F2 = fun() -> select_domain(mim(), <<"example.db">>) end, + mongoose_helper:wait_until(F2, {error, not_found}, #{time_left => timer:seconds(5)}). + rest_can_delete_domain(Config) -> rest_put_domain(Config, <<"example.db">>, <<"type1">>), {{<<"204">>, _}, _} = From da69797a7db2e45f6cf11d1ea58ced802ec9edd2 Mon Sep 17 00:00:00 2001 From: Nelson Vides Date: Tue, 18 Oct 2022 14:00:06 +0200 Subject: [PATCH 2/6] Test that a domain in deleting process cannot be re-enabled --- big_tests/tests/service_domain_db_SUITE.erl | 51 +++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/big_tests/tests/service_domain_db_SUITE.erl b/big_tests/tests/service_domain_db_SUITE.erl index 95fe99e961..ae3c004e05 100644 --- a/big_tests/tests/service_domain_db_SUITE.erl +++ b/big_tests/tests/service_domain_db_SUITE.erl @@ -51,6 +51,7 @@ db_cases() -> [ db_deleted_domain_from_db, db_deleted_domain_fails_with_wrong_host_type, db_deleted_domain_from_core, + rest_cannot_enable_deleting, db_disabled_domain_is_in_db, db_disabled_domain_not_in_core, db_reenabled_domain_is_in_db, @@ -180,6 +181,7 @@ init_per_suite(Config) -> prepare_test_queries(mim2()), erase_database(mim()), Config2 = ejabberd_node_utils:init(mim(), Config1), + mongoose_helper:inject_module(?MODULE), escalus:init_per_suite([{service_setup, per_testcase} | Config2]). end_per_suite(Config) -> @@ -220,6 +222,10 @@ end_per_group(_GroupName, Config) -> per_testcase -> ok end. +init_per_testcase(rest_cannot_enable_deleting, Config) -> + HostType = <<"type1">>, + Server = start_domain_removal_hook(HostType), + init_per_testcase(generic, [{server, Server}, {host_type, HostType} | Config]); init_per_testcase(db_crash_on_initial_load_restarts_service, Config) -> maybe_setup_meck(db_crash_on_initial_load_restarts_service), restart_domain_core(mim(), [], []), @@ -237,6 +243,11 @@ service_opts(db_events_table_gets_truncated) -> service_opts(_) -> #{}. +end_per_testcase(rest_cannot_enable_deleting, Config) -> + Server = ?config(server, Config), + HostType = ?config(host_type, Config), + stop_domain_removal_hook(HostType, Server), + exit(Server, normal); end_per_testcase(TestcaseName, Config) -> end_per_testcase2(TestcaseName, Config), case TestcaseName of @@ -366,6 +377,19 @@ db_inserted_domain_is_in_core(_) -> sync(), {ok, <<"type1">>} = get_host_type(mim(), <<"example.db">>). +rest_cannot_enable_deleting(Config) -> + HostType = ?config(host_type, Config), + Domain = <<"example.db">>, + ok = insert_domain(mim(), Domain, HostType), + {ok, #{status := enabled}} = select_domain(mim(), Domain), + ok = request_delete_domain(mim(), Domain, HostType), + {ok, #{status := deleting}} = select_domain(mim(), Domain), + {error, domain_deleted} = enable_domain(mim(), Domain), + Server = ?config(server, Config), + Server ! continue, + F = fun () -> select_domain(mim(), <<"example.db">>) end, + mongoose_helper:wait_until(F, {error, not_found}, #{time_left => timer:seconds(15)}). + db_deleted_domain_from_db(_) -> ok = insert_domain(mim(), <<"example.db">>, <<"type1">>), ok = delete_domain(mim(), <<"example.db">>, <<"type1">>), @@ -1091,6 +1115,9 @@ insert_domain(Node, Domain, HostType) -> delete_domain(Node, Domain, HostType) -> rpc(Node, mongoose_domain_api, delete_domain, [Domain, HostType]). +request_delete_domain(Node, Domain, HostType) -> + rpc(Node, mongoose_domain_api, request_delete_domain, [Domain, HostType]). + select_domain(Node, Domain) -> rpc(Node, mongoose_domain_sql, select_domain, [Domain]). @@ -1159,6 +1186,30 @@ disable_domain(Node, Domain) -> enable_domain(Node, Domain) -> rpc(Node, mongoose_domain_api, enable_domain, [Domain]). +start_domain_removal_hook(HostType) -> + Server = spawn(fun stopper/0), + rpc(mim(), gen_hook, add_handler, + [ remove_domain, HostType, fun ?MODULE:domain_removal_hook_fn/3, + #{server => Server}, 30]), %% Priority is so that it comes before muclight and mam + Server. + +stop_domain_removal_hook(HostType, Server) -> + rpc(mim(), gen_hook, delete_handler, + [ remove_domain, HostType, fun ?MODULE:domain_removal_hook_fn/3, + #{server => Server}, 30]). + +domain_removal_hook_fn(Acc, _Params, #{server := Server}) -> + Server ! {wait, self()}, + receive continue -> ok end, + Acc. + +stopper() -> + receive + {wait, From} -> + receive continue -> ok end, + From ! continue + end. + %% force_check_for_updates is already sent by insert or delete commands. %% But it is async. %% So, the only thing is left to sync is to call ping to the gen_server From 18978c91074b433a58184a8e59dcb0c33debb49f Mon Sep 17 00:00:00 2001 From: Nelson Vides Date: Tue, 18 Oct 2022 14:00:39 +0200 Subject: [PATCH 3/6] Test GraphQL endpoint for requesting an async domain removal --- big_tests/tests/graphql_domain_SUITE.erl | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/big_tests/tests/graphql_domain_SUITE.erl b/big_tests/tests/graphql_domain_SUITE.erl index 8fa4a3fd0b..7b5c626d87 100644 --- a/big_tests/tests/graphql_domain_SUITE.erl +++ b/big_tests/tests/graphql_domain_SUITE.erl @@ -41,6 +41,7 @@ domain_tests() -> get_domains_by_host_type, get_domain_details, delete_domain, + request_delete_domain, get_domains_after_deletion, set_domain_password, set_nonexistent_domain_password, @@ -177,6 +178,20 @@ delete_domain(Config) -> <<"domain">> := #{<<"domain">> := ?SECOND_EXAMPLE_DOMAIN}}, ParsedResult2). +request_delete_domain(Config) -> + Domain = <<"exampleDomain">>, + Result1 = request_remove_domain(Domain, ?HOST_TYPE, Config), + ParsedResult1 = get_ok_value([data, domain, requestRemoveDomain], Result1), + ?assertMatch(#{<<"msg">> := <<"Domain disabled and enqueued for deletion">>, + <<"domain">> := #{<<"domain">> := Domain, + <<"status">> := <<"DELETING">>}}, + ParsedResult1), + F = fun() -> + Result = get_domain_details(Domain, Config), + domain_not_found_error_formatting(Result) + end, + mongoose_helper:wait_until(F, ok, #{time_left => timer:seconds(5)}). + get_domains_after_deletion(Config) -> Result = get_domains_by_host_type(?HOST_TYPE, Config), ParsedResult = get_ok_value([data, domain, domainsByHostType], Result), @@ -263,6 +278,10 @@ remove_domain(Domain, HostType, Config) -> Vars = #{domain => Domain, hostType => HostType}, execute_command(<<"domain">>, <<"removeDomain">>, Vars, Config). +request_remove_domain(Domain, HostType, Config) -> + Vars = #{domain => Domain, hostType => HostType}, + execute_command(<<"domain">>, <<"requestRemoveDomain">>, Vars, Config). + get_domains_by_host_type(HostType, Config) -> Vars = #{hostType => HostType}, execute_command(<<"domain">>, <<"domainsByHostType">>, Vars, Config). From 5d81b134c793625a0b8a899771cc4d9d1f164286 Mon Sep 17 00:00:00 2001 From: Nelson Vides Date: Tue, 18 Oct 2022 14:01:31 +0200 Subject: [PATCH 4/6] Implement an async REST endpoint for deletion --- .../Administration-backend_swagger.yml | 13 ++++++++++ src/domain/mongoose_domain_api.erl | 19 +++++++++++---- src/domain/mongoose_domain_db_cleaner.erl | 18 +++++++++++++- .../mongoose_admin_api_domain.erl | 24 +++++++++++++++++-- 4 files changed, 67 insertions(+), 7 deletions(-) diff --git a/doc/rest-api/Administration-backend_swagger.yml b/doc/rest-api/Administration-backend_swagger.yml index 1120dc7fc1..a96c27216e 100644 --- a/doc/rest-api/Administration-backend_swagger.yml +++ b/doc/rest-api/Administration-backend_swagger.yml @@ -767,7 +767,20 @@ paths: host_type: example: "type1" type: string + - in: body + name: request + description: Flag to indicate if the request should be async. + required: false + schema: + title: request + type: object + properties: + request: + example: true + type: boolean responses: + 202: + description: "The domain has been disabled and is enqueued for removal." 204: description: "The domain is removed or not found." 403: diff --git a/src/domain/mongoose_domain_api.erl b/src/domain/mongoose_domain_api.erl index 15c2076908..ca165592e9 100644 --- a/src/domain/mongoose_domain_api.erl +++ b/src/domain/mongoose_domain_api.erl @@ -11,6 +11,7 @@ %% domain API -export([insert_domain/2, delete_domain/2, + request_delete_domain/2, disable_domain/1, enable_domain/1, get_domain_host_type/1, @@ -30,7 +31,8 @@ get_all_subdomains_for_domain/1]). %% Helper for remove_domain --export([remove_domain_wrapper/3]). +-export([remove_domain_wrapper/3, + do_delete_domain_in_progress/3]). %% For testing -export([get_all_dynamic/0]). @@ -82,13 +84,20 @@ insert_domain(Domain, HostType) -> %% Domain should be nameprepped using `jid:nameprep'. -spec delete_domain(domain(), host_type()) -> delete_domain_return(). delete_domain(Domain, HostType) -> + do_delete_domain(Domain, HostType, sync). + +-spec request_delete_domain(domain(), host_type()) -> delete_domain_return(). +request_delete_domain(Domain, HostType) -> + do_delete_domain(Domain, HostType, async). + +do_delete_domain(Domain, HostType, RequestType) -> case check_domain(Domain, HostType) of ok -> Res0 = check_db(mongoose_domain_sql:set_domain_for_deletion(Domain, HostType)), case Res0 of ok -> delete_domain_password(Domain), - do_delete_domain_in_progress(Domain, HostType); + do_delete_domain_in_progress(Domain, HostType, RequestType); Other -> Other end; @@ -98,8 +107,10 @@ delete_domain(Domain, HostType) -> %% This is ran only in the context of `do_delete_domain', %% so it can already skip some checks --spec do_delete_domain_in_progress(domain(), host_type()) -> delete_domain_return(). -do_delete_domain_in_progress(Domain, HostType) -> +-spec do_delete_domain_in_progress(domain(), host_type(), sync | async) -> delete_domain_return(). +do_delete_domain_in_progress(Domain, HostType, async) -> + mongoose_domain_db_cleaner:request_delete_domain(Domain, HostType); +do_delete_domain_in_progress(Domain, HostType, sync) -> case mongoose_hooks:remove_domain(HostType, Domain) of #{failed := []} -> check_db(mongoose_domain_sql:delete_domain(Domain, HostType)); diff --git a/src/domain/mongoose_domain_db_cleaner.erl b/src/domain/mongoose_domain_db_cleaner.erl index 17fc6c1813..ff2bc8e3e9 100644 --- a/src/domain/mongoose_domain_db_cleaner.erl +++ b/src/domain/mongoose_domain_db_cleaner.erl @@ -4,7 +4,7 @@ -include("mongoose_logger.hrl"). -export([start/1, stop/0]). --export([start_link/1]). +-export([start_link/1, request_delete_domain/2]). %% gen_server callbacks -export([init/1, handle_call/3, handle_cast/2, handle_info/2, @@ -37,6 +37,10 @@ stop() -> start_link(Opts) -> gen_server:start_link({local, ?MODULE}, ?MODULE, Opts, []). +-spec request_delete_domain(jid:lserver(), mongooseim:host_type()) -> ok. +request_delete_domain(Domain, HostType) -> + gen_server:cast(?MODULE, {delete_domain, Domain, HostType}). + %% --------------------------------------------------------------------------- %% Server callbacks @@ -51,6 +55,8 @@ handle_call(Request, From, State) -> ?UNEXPECTED_CALL(Request, From), {reply, ok, State}. +handle_cast({delete_domain, Domain, HostType}, State) -> + {noreply, handle_delete_domain(Domain, HostType, State)}; handle_cast(Msg, State) -> ?UNEXPECTED_CAST(Msg), {noreply, State}. @@ -90,3 +96,13 @@ schedule_removal(State = #{max_age := MaxAge}) -> handle_timeout(_TimerRef, {do_removal, LastEventId}, State) -> mongoose_domain_sql:delete_events_older_than(LastEventId), State. + +handle_delete_domain(Domain, HostType, State) -> + try + mongoose_domain_api:do_delete_domain_in_progress(Domain, HostType, sync) + catch Class:Reason:Stacktrace -> + ?LOG_ERROR(#{what => domain_deletion_failed, + domain => Domain, host_type => HostType, + class => Class, reason => Reason, stacktrace => Stacktrace}) + end, + State. diff --git a/src/mongoose_admin_api/mongoose_admin_api_domain.erl b/src/mongoose_admin_api/mongoose_admin_api_domain.erl index affede9ca6..f14ec62119 100644 --- a/src/mongoose_admin_api/mongoose_admin_api_domain.erl +++ b/src/mongoose_admin_api/mongoose_admin_api_domain.erl @@ -11,7 +11,8 @@ allowed_methods/2, to_json/2, from_json/2, - delete_resource/2]). + delete_resource/2, + delete_completed/2]). -ignore_xref([to_json/2, from_json/2]). @@ -69,6 +70,12 @@ from_json(Req, State) -> delete_resource(Req, State) -> try_handle_request(Req, State, fun handle_delete/2). +-spec delete_completed(req(), state()) -> {boolean(), req(), state()}. +delete_completed(Req, #{deletion := in_process} = State) -> + {false, Req, State}; +delete_completed(Req, State) -> + {true, Req, State}. + %% Internal functions handle_get(Req, State) -> @@ -128,7 +135,20 @@ handle_delete(Req, State) -> Bindings = cowboy_req:bindings(Req), Domain = get_domain(Bindings), Args = parse_body(Req), - HostType = get_host_type(Args), + handle_delete(Req, State, Domain, Args). + +handle_delete(Req, State, Domain, #{host_type := HostType, request := true}) -> + async_delete(Req, State, Domain, HostType); +handle_delete(Req, State, Domain, #{host_type := HostType}) -> + sync_delete(Req, State, Domain, HostType); +handle_delete(_Req, _State, _Domain, #{}) -> + throw_error(bad_request, <<"'host_type' field is missing">>). + +async_delete(Req, State, Domain, HostType) -> + mongoose_domain_api:request_delete_domain(Domain, HostType), + {true, Req, State#{deletion => in_process}}. + +sync_delete(Req, State, Domain, HostType) -> case mongoose_domain_api:delete_domain(Domain, HostType) of ok -> {true, Req, State}; From 31a37f92a194ce8f3a68451ec4581a764a42b5d2 Mon Sep 17 00:00:00 2001 From: Nelson Vides Date: Tue, 18 Oct 2022 14:01:46 +0200 Subject: [PATCH 5/6] Implement an async GraphQL endpoint for deletion --- priv/graphql/schemas/admin/domain.gql | 3 +++ .../admin/mongoose_graphql_domain_admin_mutation.erl | 8 ++++++++ 2 files changed, 11 insertions(+) diff --git a/priv/graphql/schemas/admin/domain.gql b/priv/graphql/schemas/admin/domain.gql index 64cf83ebb9..d65aa790b9 100644 --- a/priv/graphql/schemas/admin/domain.gql +++ b/priv/graphql/schemas/admin/domain.gql @@ -14,6 +14,9 @@ type DomainAdminMutation @protected{ "Remove domain. Only for global admin" removeDomain(domain: String!, hostType: String!): RemoveDomainPayload @protected(type: GLOBAL) + "Remove domain asynchronously. Only for global admin" + requestRemoveDomain(domain: String!, hostType: String!): RemoveDomainPayload + @protected(type: GLOBAL) "Enable domain. Only for global admin" enableDomain(domain: String!): Domain @protected(type: GLOBAL) diff --git a/src/graphql/admin/mongoose_graphql_domain_admin_mutation.erl b/src/graphql/admin/mongoose_graphql_domain_admin_mutation.erl index 1512503ce9..6e44de5f72 100644 --- a/src/graphql/admin/mongoose_graphql_domain_admin_mutation.erl +++ b/src/graphql/admin/mongoose_graphql_domain_admin_mutation.erl @@ -22,6 +22,14 @@ execute(_Ctx, admin, <<"removeDomain">>, #{<<"domain">> := Domain, <<"hostType"> {error, Error} -> error_handler(Error, Domain, HostType) end; +execute(_Ctx, admin, <<"requestRemoveDomain">>, #{<<"domain">> := Domain, <<"hostType">> := HostType}) -> + case mongoose_domain_api:request_delete_domain(Domain, HostType) of + ok -> + DomainObj = #domain{domain = Domain, host_type = HostType, status = deleting}, + {ok, #{<<"domain">> => DomainObj, <<"msg">> => <<"Domain disabled and enqueued for deletion">>}}; + {error, Error} -> + error_handler(Error, Domain, HostType) + end; execute(_Ctx, admin, <<"enableDomain">>, #{<<"domain">> := Domain}) -> case mongoose_domain_api:enable_domain(Domain) of ok -> From aa6e24d273c7ff3adc67ba71866742cc518ee3c8 Mon Sep 17 00:00:00 2001 From: Nelson Vides Date: Tue, 18 Oct 2022 14:02:21 +0200 Subject: [PATCH 6/6] Ensure a deleting domain status is not changed on request --- src/domain/mongoose_domain_sql.erl | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/domain/mongoose_domain_sql.erl b/src/domain/mongoose_domain_sql.erl index 7844d4a1f1..b474cfff51 100644 --- a/src/domain/mongoose_domain_sql.erl +++ b/src/domain/mongoose_domain_sql.erl @@ -340,6 +340,8 @@ set_status(Domain, Status) -> case mongoose_domain_core:is_host_type_allowed(HostType) of false -> {error, unknown_host_type}; + true when deleting =:= CurrentStatus -> + {error, domain_deleted}; true when Status =:= CurrentStatus -> ok; true ->