From 510a2d74b5d1054c6bcbb5e1e9f1096f01b1f1a0 Mon Sep 17 00:00:00 2001 From: Israel Date: Thu, 18 Jan 2024 09:19:49 -0300 Subject: [PATCH] Add support for remote `barman config-update` execution (#89) So far we had only operations related with Barman servers in the API: * `RecoveryOperation`: to perform a `barman recover` of a Barman server * `ConfigSwitchOperation`: to perform a `barman config-switch` to a Barman server However, we needed to add an operation to the API which is performed at the Barman instance level (global), not to a specific Barman server: the `ConfigUpdateOperation`. With that in mind this PR changes the classes below so we will be able to create Barman instance operations: * `OperationServer`: turn `server_name` argument optional. When it is `None`, that is considered an instance operation. Thus, the `job` and `output` files will be written under the Barman home in that case instead of under a Barman server directory * `Operation`: turn `server_name` argument optional, so it creates `OperationServer` accordingly We also added a couple new endpoints to the API, so we can handle instance operations: * `/operations/`: used to get the status of an instance operation -- similar to what we have for server operations through `/servers//operations/`; * `/operations`: used to get a list of or instance operations, or to create a new instance operation -- similar to what we have for server operations through `/servers//operations`. The `BarmanConfigUpdate` operation has been created and uses the above API endpoints. Unit tests changed accordingly, so we check both server and instance operations, including the introduced operation. References: BAR-126. --- pg_backup_api/pg_backup_api/__main__.py | 13 +- .../pg_backup_api/logic/utility_controller.py | 152 +++++++++++-- pg_backup_api/pg_backup_api/run.py | 21 +- .../pg_backup_api/server_operation.py | 165 +++++++++++--- .../pg_backup_api/tests/test_main.py | 20 +- pg_backup_api/pg_backup_api/tests/test_run.py | 39 +++- .../tests/test_server_operation.py | 130 +++++++++-- .../tests/test_utility_controller.py | 210 ++++++++++++++++++ 8 files changed, 686 insertions(+), 64 deletions(-) diff --git a/pg_backup_api/pg_backup_api/__main__.py b/pg_backup_api/pg_backup_api/__main__.py index b455df3..f57ce50 100644 --- a/pg_backup_api/pg_backup_api/__main__.py +++ b/pg_backup_api/pg_backup_api/__main__.py @@ -21,7 +21,8 @@ import sys from pg_backup_api.run import (serve, status, recovery_operation, - config_switch_operation) + config_switch_operation, + config_update_operation) def main() -> None: @@ -82,6 +83,16 @@ def main() -> None: help="ID of the operation in the 'pg-backup-api'.") p_ops.set_defaults(func=config_switch_operation) + p_ops = subparsers.add_parser( + "config-update", + description="Perform a 'barman config-update' through the " + "'pg-backup-api'. Can only be run if a config-update " + "operation has been previously registered." + ) + p_ops.add_argument("--operation-id", required=True, + help="ID of the operation in the 'pg-backup-api'.") + p_ops.set_defaults(func=config_update_operation) + args = p.parse_args() if hasattr(args, "func") is False: p.print_help() diff --git a/pg_backup_api/pg_backup_api/logic/utility_controller.py b/pg_backup_api/pg_backup_api/logic/utility_controller.py index afa27b3..46b0858 100644 --- a/pg_backup_api/pg_backup_api/logic/utility_controller.py +++ b/pg_backup_api/pg_backup_api/logic/utility_controller.py @@ -19,7 +19,7 @@ """Define the Flask endpoints of the pg-backup-api REST API server.""" import json import subprocess -from typing import Any, Dict, Tuple, Union, TYPE_CHECKING +from typing import Any, Dict, Optional, Tuple, Union, TYPE_CHECKING from flask import abort, jsonify, request @@ -37,6 +37,7 @@ DEFAULT_OP_TYPE, RecoveryOperation, ConfigSwitchOperation, + ConfigUpdateOperation, MalformedContent) if TYPE_CHECKING: # pragma: no cover @@ -112,16 +113,13 @@ def resource_not_found(error: Any) -> Tuple['Response', int]: return jsonify(error=str(error)), 404 -@app.route("/servers//operations/") -def servers_operation_id_get(server_name: str, operation_id: str) \ +def _operation_id_get(server_name: Optional[str], operation_id: str) \ -> 'Response': """ - ``GET`` request to ``/servers/*server_name*/operations/*operation_id*``. + Get status of an operation with ID *operation_id*. - Get status of an operation with ID *operation_id* for Barman server named - *server_name*. - - :param server_name: name of the Barman server related to the operation. + :param server_name: name of the Barman server related to the operation, if + it's a server operation, ``None`` if it's an instance operation. :param operation_id: ID of the operation previously created through pg-backup-api. :return: if *server_name* and *operation_id* are valid, return a JSON @@ -146,6 +144,37 @@ def servers_operation_id_get(server_name: str, operation_id: str) \ abort(404, description="Resource not found") +@app.route("/servers//operations/") +def servers_operation_id_get(server_name: str, operation_id: str) \ + -> 'Response': + """ + ``GET`` request to ``/servers/*server_name*/operations/*operation_id*``. + + Get status of an operation with ID *operation_id* for Barman server named + *server_name*. + + :param server_name: name of the Barman server related to the operation. + :param operation_id: ID of the operation previously created through + pg-backup-api. + :return: see :func:`_operation_id_get` for details. + """ + return _operation_id_get(server_name, operation_id) + + +@app.route("/operations/") +def instance_operation_id_get(operation_id: str) -> 'Response': + """ + ``GET`` request to ``/operations/*operation_id*``. + + Get status of an operation with ID *operation_id* for the Barman instance. + + :param operation_id: ID of the operation previously created through + pg-backup-api. + :return: see :func:`_operation_id_get` for details. + """ + return _operation_id_get(None, operation_id) + + def servers_operations_post(server_name: str, request: 'Request') -> Dict[str, str]: """ @@ -235,6 +264,26 @@ def servers_operations_post(server_name: str, return {"operation_id": operation.id} +def _operations_get(server_name: Optional[str]) \ + -> Union[Tuple['Response', int], 'Response']: + """ + Get a list of operations for a Barman server or instance. + + :param server_name: name of the Barman server to fetch operations from, or + ``None`` for instance operations. + + :return: a JSON response with ``operations`` key containing a list of + operations for a Barman server or instance. Each item in the list + contains the operation ID and the operation type. + """ + try: + operation = OperationServer(server_name) + available_operations = {"operations": operation.get_operations_list()} + return jsonify(available_operations) + except OperationServerConfigError as e: + abort(404, description=str(e)) + + @app.route("/servers//operations", methods=("GET", "POST")) def server_operation(server_name: str) \ -> Union[Tuple['Response', int], 'Response']: @@ -262,9 +311,86 @@ def server_operation(server_name: str) \ if request.method == "POST": return jsonify(servers_operations_post(server_name, request)), 202 + return _operations_get(server_name) + + +def instance_operations_post(request: 'Request') -> Dict[str, str]: + """ + Handle ``POST`` request to ``/operations``. + + :param request: the flask request that has been received by the routing + function. + + Should contain a JSON body with a key ``type``, which identifies the + type of the operation. The rest of the content depends on the type of + operation being requested: + + * ``config_update``: + + * ``changes``: an array of dictionaries to be used in + ``barman config-update`` + + :return: if the JSON body informed through the ``POST`` request is valid, + return a JSON response containing a key ``operation_id`` with the ID of + the operation that has been created. + + Otherwise, if any issue is identified, return a response with either of + the following statuses and the relevant error message: + + * ``400``: if any required option is missing in the JSON request body. + * ``404``: if any value in the JSON request body is invalid. + """ + request_body = request.get_json() + + if not request_body: + msg_400 = "Minimum barman options not met for instance operation" + abort(400, description=msg_400) + + operation = None + cmd = None + op_type = OperationType(request_body.get("type")) + + if op_type == OperationType.CONFIG_UPDATE: + operation = ConfigUpdateOperation(None) + cmd = "pg-backup-api config-update" + + if TYPE_CHECKING: # pragma: no cover + assert isinstance(operation, Operation) + assert isinstance(cmd, str) + try: - operation = OperationServer(server_name) - available_operations = {"operations": operation.get_operations_list()} - return jsonify(available_operations) - except OperationServerConfigError as e: - abort(404, description=str(e)) + operation.write_job_file(request_body) + except MalformedContent: + msg_400 = "Make sure all options/arguments are met and try again" + abort(400, description=msg_400) + + cmd += f" --operation-id {operation.id}" + subprocess.Popen(cmd.split()) + + return {"operation_id": operation.id} + + +@app.route("/operations", methods=("GET", "POST")) +def instance_operation() -> Union[Tuple['Response', int], 'Response']: + """ + Handle ``GET``/``POST`` request to ``/operations``. + + Get a list of operations for the Barman instance, if a ``GET`` request, or + create a new operation for the instance, if a ``POST`` request. + + :return: the returned response varies: + + * If a successful ``GET`` request, then return a JSON response with + ``operations`` key containing a list of operations for the Barman + instance. Each item in the list contain the operation ID and the + operation type; + * If a successful ``POST`` request, then return a JSON response with + HTTP status ``202`` containing an ``operation_id`` key with the ID + of the operation that has been created for the Barman instance; + * If any issue is faced when processing the request, return an HTTP + ``400`` or ``404`` response with the relevant error message. + """ + if request.method == "POST": + return jsonify(instance_operations_post(request)), 202 + + return _operations_get(None) diff --git a/pg_backup_api/pg_backup_api/run.py b/pg_backup_api/pg_backup_api/run.py index ee88b8b..419fab2 100644 --- a/pg_backup_api/pg_backup_api/run.py +++ b/pg_backup_api/pg_backup_api/run.py @@ -30,7 +30,8 @@ from pg_backup_api.utils import create_app, load_barman_config from pg_backup_api.server_operation import (RecoveryOperation, - ConfigSwitchOperation) + ConfigSwitchOperation, + ConfigUpdateOperation) if TYPE_CHECKING: # pragma: no cover @@ -153,3 +154,21 @@ def config_switch_operation(args: 'argparse.Namespace') -> Tuple[None, bool]: """ return _run_operation(ConfigSwitchOperation(args.server_name, args.operation_id)) + + +def config_update_operation(args: 'argparse.Namespace') -> Tuple[None, bool]: + """ + Perform a ``barman config-update`` through the pg-backup-api. + + .. note:: + See :func:`_run_operation` for more details. + + :param args: command-line arguments for ``pg-backup-api config-update`` + command. Contains the operation ID to be run. + :return: a tuple consisting of two items: + + * ``None`` -- output of :meth:`ConfigUpdateOperation.write_output_file` + * ``True`` if ``barman config-update`` was successful, ``False`` + otherwise. + """ + return _run_operation(ConfigUpdateOperation(None, args.operation_id)) diff --git a/pg_backup_api/pg_backup_api/server_operation.py b/pg_backup_api/pg_backup_api/server_operation.py index 443a077..154887a 100644 --- a/pg_backup_api/pg_backup_api/server_operation.py +++ b/pg_backup_api/pg_backup_api/server_operation.py @@ -49,6 +49,7 @@ class OperationType(Enum): """Describe operations that can be performed through pg-backup-api.""" RECOVERY = "recovery" CONFIG_SWITCH = "config_switch" + CONFIG_UPDATE = "config_update" DEFAULT_OP_TYPE = OperationType.RECOVERY @@ -71,15 +72,17 @@ class OperationNotExists(LookupError): class OperationServer: """ - Contain metadata about a Barman server and logic to handle operations. + Contain logic to handle operations for a Barman instance or Barman server. - :ivar name: name of the Barman server. - :ivar config: Barman configuration of the Barman server. + :ivar name: name of the Barman server, if it's server operation, otherwise + ``None`` for a "global" (instance) operation. + :ivar config: Barman configuration of the Barman server, if it's a server + operation, otherwise ``None`` for a "global" (instance) operation. :ivar jobs_basedir: directory where to save files of operations that have - been created for this Barman server. + been created for this Barman server or instance. :ivar output_basedir: directory where to save files with output of - operations that have been finished for this Barman server -- both for - failed and successful executions. + operations that have been finished for this Barman server or instance + -- both for failed and successful executions. """ # Name of the pg-backup-api ``jobs`` directory. Files created under this @@ -94,39 +97,50 @@ class OperationServer: # Set of required keys when creating an operation output file. _REQUIRED_OUTPUT_KEYS = ("success", "end_time", "output",) - def __init__(self, name: str) -> None: + def __init__(self, name: Optional[str]) -> None: """ Initialize a new instance of :class:`OperationServer`. - Fill all the metadata required by pg-backup-api of a given Barman - server named *name*, if it exists in Barman. Also prepare the Barman - server to execute pg-backup-api operations. + Fill all the metadata required by pg-backup-api for a given Barman + server named *name*, if a Barman server opartion and the server exists + in Barman. Also prepare the Barman server or instance to execute + pg-backup-api operations. - :param name: name of the Barman server. + :param name: name of the Barman server, if it's a Barman server + operation, ``None`` for a "global" (instance) operation. :raises: :exc:`OperationServerConfigError`: if no Barman configuration could - be found for server *name*. + be found for server *name*, in case of a Barman server + operation. """ self.name = name - self.config = get_server_by_name(name) - - if not self.config: - raise OperationServerConfigError( - f"No barman config found for '{name}'." - ) + self.config = None load_barman_config() + if name: + self.config = get_server_by_name(name) + + if not self.config: + raise OperationServerConfigError( + f"No barman config found for '{name}'." + ) + if TYPE_CHECKING: # pragma: no cover assert isinstance(barman.__config__, BarmanConfig) barman_home = barman.__config__.barman_home - self.jobs_basedir = join(barman_home, name, self._JOBS_DIR_NAME) - self._create_jobs_dir() + if name: + self.jobs_basedir = join(barman_home, name, self._JOBS_DIR_NAME) + self.output_basedir = join(barman_home, name, + self._OUTPUT_DIR_NAME) + else: + self.jobs_basedir = join(barman_home, self._JOBS_DIR_NAME) + self.output_basedir = join(barman_home, self._OUTPUT_DIR_NAME) - self.output_basedir = join(barman_home, name, self._OUTPUT_DIR_NAME) + self._create_jobs_dir() self._create_output_dir() @staticmethod @@ -151,11 +165,11 @@ def _create_dir(dir_path: str) -> None: os.makedirs(dir_path) def _create_jobs_dir(self) -> None: - """Create the ``jobs`` directory of this Barman server.""" + """Create the ``jobs`` directory of Barman server or instance.""" self._create_dir(self.jobs_basedir) def _create_output_dir(self) -> None: - """Create the ``outputs`` directory of this Barman server.""" + """Create the ``outputs`` directory of Barman server or instance.""" self._create_dir(self.output_basedir) def get_job_file_path(self, op_id: str) -> str: @@ -318,16 +332,16 @@ def read_output_file(self, op_id: str) -> Dict[str, Any]: def get_operations_list(self, op_type: Optional[OperationType] = None) \ -> List[Dict[str, Any]]: """ - Get the list of operations of this Barman server. + Get the list of operations of this Barman server or instance. Fetch operation from all ``.json`` files found under the - :attr:`jobs_basedir` of this server. + :attr:`jobs_basedir` of this server or instance. :param op_type: if ``None`` retrieve all operations. If something other than ``None``, filter by the given type. - :return: list of operations of this Barman server. Each item has the - following keys: + :return: list of operations of this Barman server or instance. Each + item has the following keys: * ``id``: ID of the operation; * ``type``: type of the operation. @@ -393,12 +407,13 @@ class Operation: :ivar id: ID of this operation. """ - def __init__(self, server_name: str, id: Optional[str] = None) -> None: + def __init__(self, server_name: Optional[str], + id: Optional[str] = None) -> None: """ Initialize a new instance of :class:`Operation`. - :param server_name: name of the Barman server, so we can manage this - operation. + :param server_name: name of the Barman server, in case of a Barman + server operation, ``None`` in case of a Barman instance operation. :param id: ID of the operation. Useful when querying an existing operation. Use ``None`` when creating an operation, so this class generates a new ID. @@ -612,6 +627,7 @@ def _get_args(self) -> List[str]: remote_ssh_command = job_content.get("remote_ssh_command") if TYPE_CHECKING: # pragma: no cover + assert isinstance(self.server.name, str) assert isinstance(backup_id, str) assert isinstance(destination_directory, str) assert isinstance(remote_ssh_command, str) @@ -727,6 +743,7 @@ def _get_args(self) -> List[str]: reset = job_content.get("reset") if TYPE_CHECKING: # pragma: no cover + assert isinstance(self.server.name, str) assert model_name is None or isinstance(model_name, str) assert reset is None or isinstance(reset, bool) @@ -757,6 +774,92 @@ def _run_logic(self) -> \ return self._run_subprocess(cmd) +class ConfigUpdateOperation(Operation): + """ + Contain information and logic to process a config update operation. + + :cvar REQUIRED_ARGUMENTS: required arguments when creating a config update + operation. + :cvar TYPE: enum type of this operation. + """ + + REQUIRED_ARGUMENTS = ("changes",) + TYPE = OperationType.CONFIG_UPDATE + + @classmethod + def _validate_job_content(cls, content: Dict[str, Any]) -> None: + """ + Validate the content of the job file before creating it. + + :param content: Python dictionary representing the JSON content of the + job file. + + :raises: + :exc:`MalformedContent`: if the set of options in *content* is + either missing required keys. + """ + required_args: Set[str] = set(cls.REQUIRED_ARGUMENTS) + missing_args = required_args - set(content.keys()) + + if missing_args: + msg = ( + "Missing required arguments: " + f"{', '.join(sorted(missing_args))}" + ) + raise MalformedContent(msg) + + def write_job_file(self, content: Dict[str, Any]) -> None: + """ + Write the job file with *content*. + + .. note:: + See :meth:`Operation.write_job_file` for more details. + + :param content: Python dictionary representing the JSON content of the + job file. Besides what is contained in *content*, this method adds + the following keys: + + * ``operation_type``: ``config_update``; + * ``start_time``: current timestamp. + """ + content["operation_type"] = self.TYPE.value + content["start_time"] = self.time_event_now() + self._validate_job_content(content) + super().write_job_file(content) + + def _get_args(self) -> List[str]: + """ + Get arguments for running ``barman config-update`` command. + + :return: list of arguments for ``barman config-update`` command. + """ + job_content = self.read_job_file() + + json_changes = json.dumps(job_content.get("changes")) + + if TYPE_CHECKING: # pragma: no cover + assert isinstance(json_changes, str) + + return [json_changes] + + def _run_logic(self) -> \ + Tuple[Union[str, bytearray, memoryview], Union[int, Any]]: + """ + Logic to be ran when executing the config update operation. + + Run ``barman config-update`` command with the configured arguments. + + Will be called when running :meth:`Operation.run`. + + :return: a tuple consisting of: + + * ``stdout``/``stderr`` of ``barman config-update``; + * exit code of ``barman config-update``. + """ + cmd = ["barman", "config-update"] + self._get_args() + return self._run_subprocess(cmd) + + def main(callback: Callable[..., Any], *args: Tuple[Any, ...]) -> int: """ Execute *callback* with *args* and log its output as an ``INFO`` message. @@ -789,7 +892,7 @@ def main(callback: Callable[..., Any], *args: Tuple[Any, ...]) -> int: "get information about jobs without a running REST API.", ) parser.add_argument( - "--server-name", required=True, + "--server-name", help="Name of the Barman server related to the operation.", ) parser.add_argument( diff --git a/pg_backup_api/pg_backup_api/tests/test_main.py b/pg_backup_api/pg_backup_api/tests/test_main.py index 7394157..fc3d558 100644 --- a/pg_backup_api/pg_backup_api/tests/test_main.py +++ b/pg_backup_api/pg_backup_api/tests/test_main.py @@ -28,17 +28,18 @@ _HELP_OUTPUT = { "pg-backup-api --help": dedent("""\ - usage: pg-backup-api [-h] {serve,status,recovery,config-switch} ... + usage: pg-backup-api [-h] + {serve,status,recovery,config-switch,config-update} ... positional arguments: - {serve,status,recovery,config-switch} + {serve,status,recovery,config-switch,config-update} optional arguments: -h, --help show this help message and exit Postgres Backup API by EnterpriseDB (www.enterprisedb.com) \ - """), + """), # noqa: E501 "pg-backup-api serve --help": dedent("""\ usage: pg-backup-api serve [-h] [--port PORT] @@ -89,6 +90,18 @@ switched. --operation-id OPERATION_ID ID of the operation in the 'pg-backup-api'. +\ + """), # noqa: E501 + "pg-backup-api config-update --help": dedent("""\ + usage: pg-backup-api config-update [-h] --operation-id OPERATION_ID + + Perform a 'barman config-update' through the 'pg-backup-api'. Can only be run + if a config-update operation has been previously registered. + + optional arguments: + -h, --help show this help message and exit + --operation-id OPERATION_ID + ID of the operation in the 'pg-backup-api'. \ """), # noqa: E501 } @@ -98,6 +111,7 @@ "pg-backup-api status": "status", "pg-backup-api recovery --server-name SOME_SERVER --operation-id SOME_OP_ID": "recovery_operation", # noqa: E501 "pg-backup-api config-switch --server-name SOME_SERVER --operation-id SOME_OP_ID": "config_switch_operation", # noqa: E501 + "pg-backup-api config-update --operation-id SOME_OP_ID": "config_update_operation", # noqa: E501 } diff --git a/pg_backup_api/pg_backup_api/tests/test_run.py b/pg_backup_api/pg_backup_api/tests/test_run.py index f53b0cd..f4d7d91 100644 --- a/pg_backup_api/pg_backup_api/tests/test_run.py +++ b/pg_backup_api/pg_backup_api/tests/test_run.py @@ -25,7 +25,8 @@ import pytest from pg_backup_api.run import (serve, status, recovery_operation, - config_switch_operation) + config_switch_operation, + config_update_operation) @pytest.mark.parametrize("port", [7480, 7481]) @@ -160,3 +161,39 @@ def test_config_switch_operation(mock_cs_op, server_name, operation_id, rc): ]) mock_write_output.assert_called_once_with(mock_read_job.return_value) + + +@pytest.mark.parametrize("operation_id", ["OPERATION_1", "OPERATION_2"]) +@pytest.mark.parametrize("rc", [0, 1]) +@patch("pg_backup_api.run.ConfigUpdateOperation") +def test_config_update_operation(mock_cu_op, operation_id, rc): + """Test :func:`config_update_operation`. + + Ensure the operation is created and executed, and that the expected values + are returned depending on the return code. + """ + args = argparse.Namespace(operation_id=operation_id) + + mock_cu_op.return_value.run.return_value = ("SOME_OUTPUT", rc) + mock_write_output = mock_cu_op.return_value.write_output_file + mock_time_event = mock_cu_op.return_value.time_event_now + mock_read_job = mock_cu_op.return_value.read_job_file + + assert config_update_operation(args) == (mock_write_output.return_value, + rc == 0) + + mock_cu_op.assert_called_once_with(None, operation_id) + mock_cu_op.return_value.run.assert_called_once_with() + mock_time_event.assert_called_once_with() + mock_read_job.assert_called_once_with() + + # Make sure the expected content was added to `read_job_file` output before + # writing it to the output file. + assert len(mock_read_job.return_value.__setitem__.mock_calls) == 3 + mock_read_job.return_value.__setitem__.assert_has_calls([ + call('success', rc == 0), + call('end_time', mock_time_event.return_value), + call('output', "SOME_OUTPUT"), + ]) + + mock_write_output.assert_called_once_with(mock_read_job.return_value) diff --git a/pg_backup_api/pg_backup_api/tests/test_server_operation.py b/pg_backup_api/pg_backup_api/tests/test_server_operation.py index 00a0e86..38b403e 100644 --- a/pg_backup_api/pg_backup_api/tests/test_server_operation.py +++ b/pg_backup_api/pg_backup_api/tests/test_server_operation.py @@ -31,6 +31,7 @@ Operation, RecoveryOperation, ConfigSwitchOperation, + ConfigUpdateOperation, ) @@ -41,34 +42,44 @@ class TestOperationServer: """Run tests for :class:`OperationServer`.""" - @pytest.fixture + @pytest.fixture(params=[_BARMAN_SERVER, None]) @patch("pg_backup_api.server_operation.get_server_by_name", Mock()) @patch("pg_backup_api.server_operation.load_barman_config", Mock()) @patch.object(OperationServer, "_create_dir", Mock()) - def op_server(self): + def op_server(self, request): """Create a :class:`OperationServer` instance for testing. :return: :class:`OperationServer` instance for testing. """ with patch("barman.__config__") as mock_config: mock_config.barman_home = _BARMAN_HOME - return OperationServer(_BARMAN_SERVER) + return OperationServer(request.param) def test___init__(self, op_server): """Test :meth:`OperationServer.__init__`. Ensure its attributes are set as expected. """ + # Handle the 2 possible fixtures, one for server operations and another + # for instance operations + expected_name = None + expected_jobs = os.path.join(_BARMAN_HOME, "jobs") + expected_output = os.path.join(_BARMAN_HOME, "output") + + if op_server.name is not None: + expected_name = _BARMAN_SERVER + expected_jobs = os.path.join(_BARMAN_HOME, _BARMAN_SERVER, "jobs") + expected_output = os.path.join(_BARMAN_HOME, _BARMAN_SERVER, + "output") + # Ensure name is as expected. - assert op_server.name == _BARMAN_SERVER + assert op_server.name == expected_name # Ensure "jobs" directory is created in expected path. - expected = os.path.join(_BARMAN_HOME, _BARMAN_SERVER, "jobs") - assert op_server.jobs_basedir == expected + assert op_server.jobs_basedir == expected_jobs # Ensure "output" directory is created in the expected path. - expected = os.path.join(_BARMAN_HOME, _BARMAN_SERVER, "output") - assert op_server.output_basedir == expected + assert op_server.output_basedir == expected_output @patch("os.path.isdir") @patch("os.path.exists") @@ -586,14 +597,14 @@ def test_get_operation_status_exception(self, mock_read_job_file, class TestOperation: """Run tests for :class:`Operation`.""" - @pytest.fixture - @patch("pg_backup_api.server_operation.OperationServer", MagicMock()) - def operation(self): + @pytest.fixture(params=[_BARMAN_SERVER, None]) + @patch("pg_backup_api.server_operation.OperationServer") + def operation(self, mock_op_server, request): """Create an :class:`Operation` instance for testing. :return: a new instance of :class:`Operation` for testing. """ - return Operation(_BARMAN_SERVER) + return Operation(request.param) def test___init___auto_id(self, operation): """Test :meth:`Operation.__init__`. @@ -604,7 +615,7 @@ def test___init___auto_id(self, operation): with patch.object(Operation, "_generate_id") as mock_generate_id: mock_generate_id.return_value = id - operation = Operation(_BARMAN_SERVER) + operation = Operation(operation.server.name) assert operation.id == id mock_generate_id.assert_called_once() @@ -616,7 +627,7 @@ def test___init___custom_id(self, operation): id = "CUSTOM_OP_ID" with patch.object(Operation, "_generate_id") as mock_generate_id: - operation = Operation(_BARMAN_SERVER, id) + operation = Operation(operation.server.name, id) assert operation.id == id mock_generate_id.assert_not_called() @@ -1042,3 +1053,94 @@ def test__run_logic(self, mock_get_args, mock_run_subprocess, operation): mock_run_subprocess.assert_called_once_with( ["barman", "config-switch"] + arguments, ) + + +@patch("pg_backup_api.server_operation.OperationServer", MagicMock()) +class TestConfigUpdateOperation: + """Run tests for :class:`ConfigUpdateOperation`.""" + + @pytest.fixture + @patch("pg_backup_api.server_operation.OperationServer", MagicMock()) + def operation(self): + """Create a :class:`ConfigUpdateOperation` instance for testing. + + :return: a new instance of :class:`ConfigUpdateOperation` for testing. + """ + return ConfigUpdateOperation(None) + + def test__validate_job_content_content_missing_keys(self, operation): + """Test :meth:`ConfigUpdateOperation._validate_job_content`. + + Ensure an exception is raised if the content is missing the required + keys. + """ + with pytest.raises(MalformedContent) as exc: + operation._validate_job_content({}) + + assert str(exc.value) == ( + "Missing required arguments: changes" + ) + + def test__validate_job_content_ok(self, operation): + """Test :meth:`ConfigUpdateOperation._validate_job_content`. + + Ensure execution is fine if required keys are giving. + """ + operation._validate_job_content({"changes": "SOME_CHANGES"}) + + @patch("pg_backup_api.server_operation.Operation.time_event_now") + @patch("pg_backup_api.server_operation.Operation.write_job_file") + def test_write_job_file(self, mock_write_job_file, mock_time_event_now, + operation): + """Test :meth:`ConfigUpdateOperation.write_job_file`. + + Ensure the underlying methods are called as expected. + """ + content = { + "SOME": "CONTENT", + } + extended_content = { + "SOME": "CONTENT", + "operation_type": OperationType.CONFIG_UPDATE.value, + "start_time": "SOME_TIMESTAMP", + } + + with patch.object(operation, "_validate_job_content") as mock: + mock_time_event_now.return_value = "SOME_TIMESTAMP" + + operation.write_job_file(content) + + mock_time_event_now.assert_called_once() + mock.assert_called_once_with(extended_content) + mock_write_job_file.assert_called_once_with(extended_content) + + def test__get_args(self, operation): + """Test :meth:`ConfigUpdateOperation._get_args`. + + Ensure it returns the correct arguments for ``barman config-update``. + """ + with patch.object(operation, "read_job_file") as mock: + mock.return_value = {"changes": [{"SOME": "CHANGES"}]} + + expected = ['[{"SOME": "CHANGES"}]'] + assert operation._get_args() == expected + + @patch("pg_backup_api.server_operation.Operation._run_subprocess") + @patch("pg_backup_api.server_operation.ConfigUpdateOperation._get_args") + def test__run_logic(self, mock_get_args, mock_run_subprocess, operation): + """Test :meth:`ConfigUpdateOperation._run_logic`. + + Ensure the underlying calls occur as expected. + """ + arguments = ["SOME", "ARGUMENTS"] + output = ("SOME OUTPUT", 0) + + mock_get_args.return_value = arguments + mock_run_subprocess.return_value = output + + assert operation._run_logic() == output + + mock_get_args.assert_called_once() + mock_run_subprocess.assert_called_once_with( + ["barman", "config-update"] + arguments, + ) diff --git a/pg_backup_api/pg_backup_api/tests/test_utility_controller.py b/pg_backup_api/pg_backup_api/tests/test_utility_controller.py index f691250..b7cc281 100644 --- a/pg_backup_api/pg_backup_api/tests/test_utility_controller.py +++ b/pg_backup_api/pg_backup_api/tests/test_utility_controller.py @@ -193,6 +193,62 @@ def test_servers_operation_id_get_not_allowed(self, client): self._ensure_http_methods_not_allowed(_HTTP_METHODS - {"GET"}, path, client) + @pytest.mark.parametrize("status", ["IN_PROGRESS", "DONE", "FAILED"]) + @patch("pg_backup_api.logic.utility_controller.OperationServer") + def test_instance_operation_id_get_ok(self, mock_op_server, status, + client): + """Test ``/operations/`` endpoint. + + Ensure a ``GET`` request returns ``200`` and the expected JSON output + according to the status of the operation. + """ + path = "/operations/SOME_OPERATION_ID" + + mock_op_server.return_value.config = object() + mock_get_status = mock_op_server.return_value.get_operation_status + + mock_get_status.return_value = status + + response = client.get(path) + + mock_op_server.assert_called_once_with(None) + mock_get_status.assert_called_once_with("SOME_OPERATION_ID") + + assert response.status_code == 200 + expected = ( + '{"operation_id":"SOME_OPERATION_ID",' + f'"status":"{status}"}}\n' + ).encode() + assert response.data == expected + + @patch("pg_backup_api.logic.utility_controller.OperationServer") + def test_instance_operation_id_get_operation_does_not_exist(self, + mock_op_server, + client): + """Test ``/operations/`` endpoint. + + Ensure ``GET`` returns ``404`` if the operation doesn't exist. + """ + path = "/operations/SOME_OPERATION_ID" + + mock_get_status = mock_op_server.return_value.get_operation_status + mock_op_server.return_value.config = object() + mock_get_status.side_effect = OperationNotExists("NOT_FOUND") + + response = client.get(path) + assert response.status_code == 404 + expected = b'{"error":"404 Not Found: Resource not found"}\n' + assert response.data == expected + + def test_instance_operation_id_get_not_allowed(self, client): + """Test ``/operations/`` endpoint. + + Ensure all other HTTP request methods return an error. + """ + path = "/operations/SOME_OPERATION_ID" + self._ensure_http_methods_not_allowed(_HTTP_METHODS - {"GET"}, path, + client) + @patch("pg_backup_api.logic.utility_controller.OperationServer") def test_server_operation_get_ok(self, mock_op_server, client): """Test ``/servers//operations`` endpoint. @@ -643,3 +699,157 @@ def test_server_operation_not_allowed(self, client): self._ensure_http_methods_not_allowed(_HTTP_METHODS - {"GET", "POST"}, path, client) + + @patch("pg_backup_api.logic.utility_controller.OperationServer") + def test_instance_operation_get_ok(self, mock_op_server, client): + """Test ``/operations`` endpoint. + + Ensure a ``GET`` request returns ``200`` and the expected JSON output. + """ + path = "/operations" + + mock_op_server.return_value.config = object() + mock_get_ops = mock_op_server.return_value.get_operations_list + mock_get_ops.return_value = [ + { + "id": "SOME_ID_1", + "type": "SOME_TYPE_1", + }, + { + "id": "SOME_ID_2", + "type": "SOME_TYPE_2", + }, + ] + + response = client.get(path) + + mock_op_server.assert_called_once_with(None) + mock_get_ops.assert_called_once_with() + + assert response.status_code == 200 + data = json.dumps({"operations": mock_get_ops.return_value}) + data = data.replace(" ", "") + "\n" + expected = data.encode() + assert response.data == expected + + def test_instance_operation_post_not_json(self, client): + """Test ``/operations`` endpoint. + + Ensure ``POST`` request won't work without data in JSON format. + """ + path = "/operations" + + response = client.post(path, data={}) + + expected_status_code = 415 + expected_data = b"Unsupported Media Type" + version = sys.version_info + + # This is an issue which was detected while running tests through + # GitHub Actions when using Python 3.7 and Flask 2.2.5. We might want + # to remove this once we remove support for Python 3.7 + if version.major <= 3 and version.minor <= 7 and \ + StrictVersion(flask.__version__) <= StrictVersion("2.2.5"): + expected_status_code = 400 + expected_data = b"Bad Request" + + assert response.status_code == expected_status_code + assert expected_data in response.data + + @patch("pg_backup_api.logic.utility_controller.OperationServer", Mock()) + @patch("pg_backup_api.logic.utility_controller.OperationType") + @patch("subprocess.Popen") + def test_instance_operation_post_empty_json(self, mock_popen, mock_op_type, + client): + """Test ``/operations`` endpoint. + + Ensure ``POST`` request returns ``400`` if JSON data is empty. + """ + path = "/operations" + + response = client.post(path, json={}) + + assert response.status_code == 400 + expected = b"Minimum barman options not met for instance operation" + assert expected in response.data + + mock_op_type.assert_not_called() + mock_popen.assert_not_called() + + @patch("pg_backup_api.logic.utility_controller.OperationServer", Mock()) + @patch("pg_backup_api.logic.utility_controller.OperationType") + @patch("pg_backup_api.logic.utility_controller.ConfigUpdateOperation") + @patch("subprocess.Popen") + def test_instance_operation_post_cu_op_missing_options(self, mock_popen, + mock_cu_op, + mock_op_type, + client): + """Test ``operations`` endpoint. + + Ensure ``POST`` request returns ``400`` if any option is missing when + requesting a config update operation. + """ + path = "operations" + json_data = { + "type": "config_update", + } + + mock_op_type.return_value = mock_op_type.CONFIG_UPDATE + mock_cu_op.return_value.id = "SOME_OP_ID" + mock_write_job = mock_cu_op.return_value.write_job_file + mock_write_job.side_effect = MalformedContent("SOME_ERROR") + + response = client.post(path, json=json_data) + + mock_op_type.assert_called_once_with("config_update") + mock_cu_op.assert_called_once_with(None) + mock_write_job.assert_called_once_with(json_data) + mock_popen.assert_not_called() + + assert response.status_code == 400 + expected = b"Make sure all options/arguments are met and try again" + assert expected in response.data + + @patch("pg_backup_api.logic.utility_controller.OperationServer", Mock()) + @patch("pg_backup_api.logic.utility_controller.OperationType") + @patch("pg_backup_api.logic.utility_controller.ConfigUpdateOperation") + @patch("subprocess.Popen") + def test_instance_operation_post_cu_ok(self, mock_popen, mock_cu_op, + mock_op_type, client): + """Test ``operations`` endpoint. + + Ensure ``POST`` request returns ``202`` if everything is ok when + requesting a config-update operation, and ensure the subprocess is + started. + """ + path = "/operations" + json_data = { + "type": "config_update", + "changes": "SOME_CHANGES", + } + + mock_op_type.return_value = mock_op_type.CONFIG_UPDATE + mock_cu_op.return_value.id = "SOME_OP_ID" + + response = client.post(path, json=json_data) + + mock_write_job = mock_cu_op.return_value.write_job_file + mock_op_type.assert_called_once_with("config_update") + mock_cu_op.assert_called_once_with(None) + mock_write_job.assert_called_once_with(json_data) + mock_popen.assert_called_once_with(["pg-backup-api", "config-update", + "--operation-id", + "SOME_OP_ID"]) + + assert response.status_code == 202 + assert response.data == b'{"operation_id":"SOME_OP_ID"}\n' + + def test_instance_operation_not_allowed(self, client): + """Test ``/operations`` endpoint. + + Ensure all other HTTP request methods return an error. + """ + path = "/operations" + + self._ensure_http_methods_not_allowed(_HTTP_METHODS - {"GET", "POST"}, + path, client)