From dad23e3766fb90c2c9b6bca7b5531273242659ad Mon Sep 17 00:00:00 2001 From: Alexander Strizhakov Date: Sat, 8 Feb 2020 12:55:37 +0300 Subject: [PATCH] need_reboot flag --- docs/API/admin_api.md | 20 ++- lib/pleroma/config/transfer_task.ex | 4 +- .../web/admin_api/admin_api_controller.ex | 48 +++--- restarter/lib/pleroma.ex | 64 ++++++-- test/config/transfer_task_test.exs | 8 +- .../admin_api/admin_api_controller_test.exs | 146 ++++++++++++++---- 6 files changed, 218 insertions(+), 72 deletions(-) diff --git a/docs/API/admin_api.md b/docs/API/admin_api.md index fb6dfcb08..47acd240e 100644 --- a/docs/API/admin_api.md +++ b/docs/API/admin_api.md @@ -682,6 +682,8 @@ Note: Available `:permission_group` is currently moderator and admin. 404 is ret ### Get list of merged default settings with saved in database. +*If `need_reboot` flag exists in response, instance must be restarted, so reboot time settings can take effect.* + **Only works when configuration from database is enabled.** - Params: @@ -692,20 +694,24 @@ Note: Available `:permission_group` is currently moderator and admin. 404 is ret ```json { - configs: [ + "configs": [ { "group": ":pleroma", "key": "Pleroma.Upload", "value": [] } - ] + ], + "need_reboot": true } ``` + need_reboot - *optional*, if were changed reboot time settings. ## `POST /api/pleroma/admin/config` ### Update config settings +*If `need_reboot` flag exists in response, instance must be restarted, so reboot time settings can take effect.* + **Only works when configuration from database is enabled.** Some modifications are necessary to save the config settings correctly: @@ -793,7 +799,7 @@ config :quack, ``` ```json { - configs: [ + "configs": [ {"group": ":quack", "key": ":level", "value": ":debug"}, {"group": ":quack", "key": ":meta", "value": [":all"]}, ... @@ -804,7 +810,7 @@ config :quack, ```json { - configs: [ + "configs": [ { "group": ":pleroma", "key": "Pleroma.Upload", @@ -836,15 +842,17 @@ config :quack, - 400 Bad Request `"To use this endpoint you need to enable configuration from database."` ```json { - configs: [ + "configs": [ { "group": ":pleroma", "key": "Pleroma.Upload", "value": [...] } - ] + ], + "need_reboot": true } ``` +need_reboot - *optional*, if were changed reboot time settings. ## ` GET /api/pleroma/admin/config/descriptions` diff --git a/lib/pleroma/config/transfer_task.ex b/lib/pleroma/config/transfer_task.ex index 6c5ba1f95..f037ce8a5 100644 --- a/lib/pleroma/config/transfer_task.ex +++ b/lib/pleroma/config/transfer_task.ex @@ -146,9 +146,7 @@ defmodule Pleroma.Config.TransferTask do defp update_env(group, key, nil), do: Application.delete_env(group, key) defp update_env(group, key, value), do: Application.put_env(group, key, value) - defp restart(_, :pleroma, :test), do: Logger.warn("pleroma restarted") - - defp restart(_, :pleroma, _), do: send(Restarter.Pleroma, :after_boot) + defp restart(_, :pleroma, env), do: Restarter.Pleroma.restart_after_boot(env) defp restart(started_applications, app, _) do with {^app, _, _} <- List.keyfind(started_applications, app, 0), diff --git a/lib/pleroma/web/admin_api/admin_api_controller.ex b/lib/pleroma/web/admin_api/admin_api_controller.ex index c95cd182d..67222ebae 100644 --- a/lib/pleroma/web/admin_api/admin_api_controller.ex +++ b/lib/pleroma/web/admin_api/admin_api_controller.ex @@ -8,6 +8,7 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIController do import Pleroma.Web.ControllerHelper, only: [json_response: 3] alias Pleroma.Activity + alias Pleroma.Config alias Pleroma.ConfigDB alias Pleroma.ModerationLog alias Pleroma.Plugs.OAuthScopesPlug @@ -570,8 +571,8 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIController do @doc "Sends registration invite via email" def email_invite(%{assigns: %{user: user}} = conn, %{"email" => email} = params) do with true <- - Pleroma.Config.get([:instance, :invites_enabled]) && - !Pleroma.Config.get([:instance, :registrations_open]), + Config.get([:instance, :invites_enabled]) && + !Config.get([:instance, :registrations_open]), {:ok, invite_token} <- UserInviteToken.create_invite(), email <- Pleroma.Emails.UserEmail.user_invitation_email( @@ -808,7 +809,7 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIController do configs = ConfigDB.get_all_as_keyword() merged = - Pleroma.Config.Holder.config() + Config.Holder.config() |> ConfigDB.merge(configs) |> Enum.map(fn {group, values} -> Enum.map(values, fn {key, value} -> @@ -838,7 +839,16 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIController do end) |> List.flatten() - json(conn, %{configs: merged}) + response = %{configs: merged} + + response = + if Restarter.Pleroma.need_reboot?() do + Map.put(response, :need_reboot, true) + else + response + end + + json(conn, response) end end @@ -863,20 +873,26 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIController do Ecto.get_meta(config, :state) == :deleted end) - Pleroma.Config.TransferTask.load_and_update_env(deleted, false) + Config.TransferTask.load_and_update_env(deleted, false) need_reboot? = - Enum.any?(updated, fn config -> - group = ConfigDB.from_string(config.group) - key = ConfigDB.from_string(config.key) - value = ConfigDB.from_binary(config.value) - Pleroma.Config.TransferTask.pleroma_need_restart?(group, key, value) - end) + Restarter.Pleroma.need_reboot?() || + Enum.any?(updated, fn config -> + group = ConfigDB.from_string(config.group) + key = ConfigDB.from_string(config.key) + value = ConfigDB.from_binary(config.value) + Config.TransferTask.pleroma_need_restart?(group, key, value) + end) response = %{configs: updated} response = - if need_reboot?, do: Map.put(response, :need_reboot, need_reboot?), else: response + if need_reboot? do + Restarter.Pleroma.need_reboot() + Map.put(response, :need_reboot, need_reboot?) + else + response + end conn |> put_view(ConfigView) @@ -886,18 +902,14 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIController do def restart(conn, _params) do with :ok <- configurable_from_database(conn) do - if Pleroma.Config.get(:env) == :test do - Logger.warn("pleroma restarted") - else - send(Restarter.Pleroma, {:restart, 50}) - end + Restarter.Pleroma.restart(Config.get(:env), 50) json(conn, %{}) end end defp configurable_from_database(conn) do - if Pleroma.Config.get(:configurable_from_database) do + if Config.get(:configurable_from_database) do :ok else errors( diff --git a/restarter/lib/pleroma.ex b/restarter/lib/pleroma.ex index da714654c..d7817909d 100644 --- a/restarter/lib/pleroma.ex +++ b/restarter/lib/pleroma.ex @@ -1,26 +1,72 @@ defmodule Restarter.Pleroma do use GenServer + require Logger + def start_link(_) do GenServer.start_link(__MODULE__, [], name: __MODULE__) end - def init(_), do: {:ok, %{}} + def init(_), do: {:ok, %{need_reboot?: false}} - def handle_info(:after_boot, %{after_boot: true} = state), do: {:noreply, state} + def need_reboot? do + GenServer.call(__MODULE__, :need_reboot?) + end - def handle_info(:after_boot, state) do - restart(:pleroma) + def need_reboot do + GenServer.cast(__MODULE__, :need_reboot) + end + + def refresh do + GenServer.cast(__MODULE__, :refresh) + end + + def restart(env, delay) do + GenServer.cast(__MODULE__, {:restart, env, delay}) + end + + def restart_after_boot(env) do + GenServer.cast(__MODULE__, {:after_boot, env}) + end + + def handle_call(:need_reboot?, _from, state) do + {:reply, state[:need_reboot?], state} + end + + def handle_cast(:refresh, _state) do + {:noreply, %{need_reboot?: false}} + end + + def handle_cast(:need_reboot, %{need_reboot?: true} = state), do: {:noreply, state} + + def handle_cast(:need_reboot, state) do + {:noreply, Map.put(state, :need_reboot?, true)} + end + + def handle_cast({:restart, :test, _}, state) do + Logger.warn("pleroma restarted") + {:noreply, Map.put(state, :need_reboot?, false)} + end + + def handle_cast({:restart, _, delay}, state) do + Process.sleep(delay) + do_restart(:pleroma) + {:noreply, Map.put(state, :need_reboot?, false)} + end + + def handle_cast({:after_boot, _}, %{after_boot: true} = state), do: {:noreply, state} + + def handle_cast({:after_boot, :test}, state) do + Logger.warn("pleroma restarted") {:noreply, Map.put(state, :after_boot, true)} end - def handle_info({:restart, delay}, state) do - Process.sleep(delay) - restart(:pleroma) - {:noreply, state} + def handle_cast({:after_boot, _}, state) do + do_restart(:pleroma) + {:noreply, Map.put(state, :after_boot, true)} end - defp restart(app) do + defp do_restart(app) do :ok = Application.ensure_started(app) :ok = Application.stop(app) :ok = Application.start(app) diff --git a/test/config/transfer_task_test.exs b/test/config/transfer_task_test.exs index ebdc951cf..3d7218dde 100644 --- a/test/config/transfer_task_test.exs +++ b/test/config/transfer_task_test.exs @@ -109,6 +109,10 @@ defmodule Pleroma.Config.TransferTaskTest do end describe "pleroma restart" do + setup do + on_exit(fn -> Restarter.Pleroma.refresh() end) + end + test "don't restart if no reboot time settings were changed" do emoji = Application.get_env(:pleroma, :emoji) on_exit(fn -> Application.put_env(:pleroma, :emoji, emoji) end) @@ -125,7 +129,7 @@ defmodule Pleroma.Config.TransferTaskTest do ) end - test "restart pleroma on reboot time key" do + test "on reboot time key" do chat = Application.get_env(:pleroma, :chat) on_exit(fn -> Application.put_env(:pleroma, :chat, chat) end) @@ -138,7 +142,7 @@ defmodule Pleroma.Config.TransferTaskTest do assert capture_log(fn -> TransferTask.start_link([]) end) =~ "pleroma restarted" end - test "restart pleroma on reboot time subkey" do + test "on reboot time subkey" do captcha = Application.get_env(:pleroma, Pleroma.Captcha) on_exit(fn -> Application.put_env(:pleroma, Pleroma.Captcha, captcha) end) diff --git a/test/web/admin_api/admin_api_controller_test.exs b/test/web/admin_api/admin_api_controller_test.exs index 5fbdf96f6..60db58144 100644 --- a/test/web/admin_api/admin_api_controller_test.exs +++ b/test/web/admin_api/admin_api_controller_test.exs @@ -6,7 +6,11 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIControllerTest do use Pleroma.Web.ConnCase use Oban.Testing, repo: Pleroma.Repo + import Pleroma.Factory + import ExUnit.CaptureLog + alias Pleroma.Activity + alias Pleroma.Config alias Pleroma.ConfigDB alias Pleroma.HTML alias Pleroma.ModerationLog @@ -19,7 +23,6 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIControllerTest do alias Pleroma.Web.CommonAPI alias Pleroma.Web.MastodonAPI.StatusView alias Pleroma.Web.MediaProxy - import Pleroma.Factory setup_all do Tesla.Mock.mock_global(fn env -> apply(HttpRequestMock, :request, [env]) end) @@ -41,7 +44,7 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIControllerTest do describe "with [:auth, :enforce_oauth_admin_scope_usage]," do clear_config([:auth, :enforce_oauth_admin_scope_usage]) do - Pleroma.Config.put([:auth, :enforce_oauth_admin_scope_usage], true) + Config.put([:auth, :enforce_oauth_admin_scope_usage], true) end test "GET /api/pleroma/admin/users/:nickname requires admin:read:accounts or broader scope", @@ -91,7 +94,7 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIControllerTest do describe "unless [:auth, :enforce_oauth_admin_scope_usage]," do clear_config([:auth, :enforce_oauth_admin_scope_usage]) do - Pleroma.Config.put([:auth, :enforce_oauth_admin_scope_usage], false) + Config.put([:auth, :enforce_oauth_admin_scope_usage], false) end test "GET /api/pleroma/admin/users/:nickname requires " <> @@ -579,11 +582,11 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIControllerTest do describe "POST /api/pleroma/admin/email_invite, with valid config" do clear_config([:instance, :registrations_open]) do - Pleroma.Config.put([:instance, :registrations_open], false) + Config.put([:instance, :registrations_open], false) end clear_config([:instance, :invites_enabled]) do - Pleroma.Config.put([:instance, :invites_enabled], true) + Config.put([:instance, :invites_enabled], true) end test "sends invitation and returns 204", %{admin: admin, conn: conn} do @@ -602,8 +605,8 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIControllerTest do assert token_record refute token_record.used - notify_email = Pleroma.Config.get([:instance, :notify_email]) - instance_name = Pleroma.Config.get([:instance, :name]) + notify_email = Config.get([:instance, :notify_email]) + instance_name = Config.get([:instance, :name]) email = Pleroma.Emails.UserEmail.user_invitation_email( @@ -639,8 +642,8 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIControllerTest do clear_config([:instance, :invites_enabled]) test "it returns 500 if `invites_enabled` is not enabled", %{conn: conn} do - Pleroma.Config.put([:instance, :registrations_open], false) - Pleroma.Config.put([:instance, :invites_enabled], false) + Config.put([:instance, :registrations_open], false) + Config.put([:instance, :invites_enabled], false) conn = post(conn, "/api/pleroma/admin/users/email_invite?email=foo@bar.com&name=JD") @@ -648,8 +651,8 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIControllerTest do end test "it returns 500 if `registrations_open` is enabled", %{conn: conn} do - Pleroma.Config.put([:instance, :registrations_open], true) - Pleroma.Config.put([:instance, :invites_enabled], true) + Config.put([:instance, :registrations_open], true) + Config.put([:instance, :invites_enabled], true) conn = post(conn, "/api/pleroma/admin/users/email_invite?email=foo@bar.com&name=JD") @@ -1886,13 +1889,13 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIControllerTest do describe "GET /api/pleroma/admin/config" do clear_config(:configurable_from_database) do - Pleroma.Config.put(:configurable_from_database, true) + Config.put(:configurable_from_database, true) end test "when configuration from database is off", %{conn: conn} do - initial = Pleroma.Config.get(:configurable_from_database) - Pleroma.Config.put(:configurable_from_database, false) - on_exit(fn -> Pleroma.Config.put(:configurable_from_database, initial) end) + initial = Config.get(:configurable_from_database) + Config.put(:configurable_from_database, false) + on_exit(fn -> Config.put(:configurable_from_database, initial) end) conn = get(conn, "/api/pleroma/admin/config") assert json_response(conn, 400) == @@ -2036,11 +2039,12 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIControllerTest do Application.delete_env(:pleroma, Pleroma.Captcha.NotReal) Application.put_env(:pleroma, :http, http) Application.put_env(:tesla, :adapter, Tesla.Mock) + Restarter.Pleroma.refresh() end) end clear_config(:configurable_from_database) do - Pleroma.Config.put(:configurable_from_database, true) + Config.put(:configurable_from_database, true) end @tag capture_log: true @@ -2249,21 +2253,19 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIControllerTest do end test "saving config which need pleroma reboot", %{conn: conn} do - chat = Pleroma.Config.get(:chat) - on_exit(fn -> Pleroma.Config.put(:chat, chat) end) + chat = Config.get(:chat) + on_exit(fn -> Config.put(:chat, chat) end) - conn = - post( - conn, - "/api/pleroma/admin/config", - %{ - configs: [ - %{group: ":pleroma", key: ":chat", value: [%{"tuple" => [":enabled", true]}]} - ] - } - ) - - assert json_response(conn, 200) == %{ + assert post( + conn, + "/api/pleroma/admin/config", + %{ + configs: [ + %{group: ":pleroma", key: ":chat", value: [%{"tuple" => [":enabled", true]}]} + ] + } + ) + |> json_response(200) == %{ "configs" => [ %{ "db" => [":enabled"], @@ -2274,6 +2276,80 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIControllerTest do ], "need_reboot" => true } + + configs = + conn + |> get("/api/pleroma/admin/config") + |> json_response(200) + + assert configs["need_reboot"] + + capture_log(fn -> + assert conn |> get("/api/pleroma/admin/restart") |> json_response(200) == %{} + end) =~ "pleroma restarted" + + configs = + conn + |> get("/api/pleroma/admin/config") + |> json_response(200) + + refute Map.has_key?(configs, "need_reboot") + end + + test "update setting which need reboot, don't change reboot flag until reboot", %{conn: conn} do + chat = Config.get(:chat) + on_exit(fn -> Config.put(:chat, chat) end) + + assert post( + conn, + "/api/pleroma/admin/config", + %{ + configs: [ + %{group: ":pleroma", key: ":chat", value: [%{"tuple" => [":enabled", true]}]} + ] + } + ) + |> json_response(200) == %{ + "configs" => [ + %{ + "db" => [":enabled"], + "group" => ":pleroma", + "key" => ":chat", + "value" => [%{"tuple" => [":enabled", true]}] + } + ], + "need_reboot" => true + } + + assert post(conn, "/api/pleroma/admin/config", %{ + configs: [ + %{group: ":pleroma", key: ":key1", value: [%{"tuple" => [":key3", 3]}]} + ] + }) + |> json_response(200) == %{ + "configs" => [ + %{ + "group" => ":pleroma", + "key" => ":key1", + "value" => [ + %{"tuple" => [":key3", 3]} + ], + "db" => [":key3"] + } + ], + "need_reboot" => true + } + + capture_log(fn -> + assert conn |> get("/api/pleroma/admin/restart") |> json_response(200) == %{} + end) =~ "pleroma restarted" + + configs = + conn + |> get("/api/pleroma/admin/config") + |> json_response(200) + + refute Map.has_key?(configs, "need_reboot") end test "saving config with nested merge", %{conn: conn} do @@ -2410,7 +2486,7 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIControllerTest do {ExSyslogger, :ex_syslogger} ] - ExUnit.CaptureLog.capture_log(fn -> + capture_log(fn -> require Logger Logger.warn("Ooops...") end) =~ "Ooops..." @@ -2543,7 +2619,7 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIControllerTest do }) assert Application.get_env(:tesla, :adapter) == Tesla.Adapter.Httpc - assert Pleroma.Config.get([Pleroma.Captcha.NotReal, :name]) == "Pleroma" + assert Config.get([Pleroma.Captcha.NotReal, :name]) == "Pleroma" assert json_response(conn, 200) == %{ "configs" => [ @@ -2979,13 +3055,15 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIControllerTest do describe "GET /api/pleroma/admin/restart" do clear_config(:configurable_from_database) do - Pleroma.Config.put(:configurable_from_database, true) + Config.put(:configurable_from_database, true) end test "pleroma restarts", %{conn: conn} do - ExUnit.CaptureLog.capture_log(fn -> + capture_log(fn -> assert conn |> get("/api/pleroma/admin/restart") |> json_response(200) == %{} end) =~ "pleroma restarted" + + refute Restarter.Pleroma.need_reboot?() end end