From b46fd0d58fec16de554f2c259df9e4ff16d3d499 Mon Sep 17 00:00:00 2001
From: Karel van Klink <karel.vanklink@geant.org>
Date: Mon, 4 Dec 2023 16:20:09 +0100
Subject: [PATCH] replace PlaybookLaunchResponse with a generic JSONResponse

---
 lso/playbook.py        | 61 ++++++++++++------------------------------
 lso/routes/ip_trunk.py | 25 ++++++-----------
 lso/routes/playbook.py | 11 ++++----
 lso/routes/router.py   |  7 +++--
 4 files changed, 33 insertions(+), 71 deletions(-)

diff --git a/lso/playbook.py b/lso/playbook.py
index 25dc8ee..a2de84c 100644
--- a/lso/playbook.py
+++ b/lso/playbook.py
@@ -4,7 +4,6 @@ import json
 import logging
 import threading
 import uuid
-from enum import StrEnum, auto
 from pathlib import Path
 from typing import Any
 
@@ -12,8 +11,9 @@ import ansible_runner
 import requests
 import xmltodict
 from dictdiffer import diff
-from fastapi import Response, status
-from pydantic import BaseModel, HttpUrl
+from fastapi import status
+from fastapi.responses import JSONResponse
+from pydantic import HttpUrl
 
 from lso import config
 from lso.config import DEFAULT_REQUEST_TIMEOUT
@@ -21,53 +21,29 @@ from lso.config import DEFAULT_REQUEST_TIMEOUT
 logger = logging.getLogger(__name__)
 
 
-class PlaybookJobStatus(StrEnum):
-    """Enumerator for status codes of a playbook job that's running."""
-
-    #: All is well.
-    OK = auto()
-    #: An error has occurred.
-    ERROR = auto()
-
-
-class PlaybookLaunchResponse(BaseModel):
-    """Running a playbook gives this response.
-
-    :param PlaybookJobStatus status:
-    :param job_id:
-    :type job_id: str, optional
-    :param info:
-    :type info: str, optional
-    """
-
-    #: Status of a Playbook job.
-    status: PlaybookJobStatus
-    #: The ID assigned to a job.
-    job_id: str = ""
-    #: Information on a job.
-    info: str = ""
-
-
 def get_playbook_path(playbook_name: str) -> Path:
     """Get the path of a playbook on the local filesystem."""
     config_params = config.load()
     return Path(config_params.ansible_playbooks_root_dir) / playbook_name
 
 
-def playbook_launch_success(job_id: str) -> PlaybookLaunchResponse:
+def playbook_launch_success(job_id: str) -> JSONResponse:
     """Return a :class:`PlaybookLaunchResponse` for the successful start of a playbook execution.
 
-    :return PlaybookLaunchResponse: A playbook launch response that's successful.
+    :return JSONResponse: A playbook launch response that's successful.
     """
-    return PlaybookLaunchResponse(status=PlaybookJobStatus.OK, job_id=job_id)
+    return JSONResponse(content={"job_id": job_id}, status_code=status.HTTP_201_CREATED)
 
 
-def playbook_launch_error(reason: str) -> PlaybookLaunchResponse:
+def playbook_launch_error(reason: str, status_code: int = status.HTTP_400_BAD_REQUEST) -> JSONResponse:
     """Return a :class:`PlaybookLaunchResponse` for the erroneous start of a playbook execution.
 
-    :return PlaybookLaunchResponse: A playbook launch response that's unsuccessful.
+    :param str reason: The reason why a request has failed.
+    :param status status_code: The HTTP status code that should be associated with this request. Defaults to HTTP 400
+                               "Bad request".
+    :return JSONResponse: A playbook launch response that's unsuccessful.
     """
-    return PlaybookLaunchResponse(status=PlaybookJobStatus.ERROR, info=reason)
+    return JSONResponse(content={"error": reason}, status_code=status_code)
 
 
 def _process_json_output(runner: ansible_runner.Runner) -> list[dict[Any, Any]]:  # ignore: C901
@@ -151,7 +127,7 @@ def _run_playbook_proc(
     :param str job_id: Identifier of the job that's executed.
     :param str playbook_path: Ansible playbook to be executed.
     :param dict extra_vars: Extra variables passed to the Ansible playbook.
-    :param str callback: Callback URL to PUT to when execution is completed.
+    :param str callback: Callback URL to return output to when execution is completed.
     :param dict[str, Any] | str inventory: Ansible inventory to run the playbook against.
     """
     ansible_playbook_run = ansible_runner.run(
@@ -181,8 +157,7 @@ def run_playbook(
     extra_vars: dict[str, Any],
     inventory: dict[str, Any] | str,
     callback: HttpUrl,
-    response: Response,
-) -> PlaybookLaunchResponse:
+) -> JSONResponse:
     """Run an Ansible playbook against a specified inventory.
 
     :param Path playbook_path: playbook to be executed.
@@ -191,17 +166,15 @@ def run_playbook(
     :param :class:`HttpUrl` callback: Callback URL where the playbook should send a status update when execution is
         completed. This is used for workflow-orchestrator to continue with the next step in a workflow.
     :return: Result of playbook launch, this could either be successful or unsuccessful.
-    :rtype: :class:`PlaybookLaunchResponse`
+    :rtype: :class:`fastapi.responses.JSONResponse`
     """
     if not Path.exists(playbook_path):
-        response.status_code = status.HTTP_404_NOT_FOUND
         msg = f"Filename '{playbook_path}' does not exist."
-        return playbook_launch_error(msg)
+        return playbook_launch_error(reason=msg, status_code=status.HTTP_404_NOT_FOUND)
 
     if not ansible_runner.utils.isinventory(inventory):
-        response.status_code = status.HTTP_400_BAD_REQUEST
         msg = "Invalid inventory provided. Should be a string, or JSON object."
-        return playbook_launch_error(msg)
+        return playbook_launch_error(reason=msg, status_code=status.HTTP_400_BAD_REQUEST)
 
     job_id = str(uuid.uuid4())
     thread = threading.Thread(
diff --git a/lso/routes/ip_trunk.py b/lso/routes/ip_trunk.py
index af9e6d4..7e4f8f0 100644
--- a/lso/routes/ip_trunk.py
+++ b/lso/routes/ip_trunk.py
@@ -1,9 +1,10 @@
 """Routes for handling events related to the IP trunk service."""
 
-from fastapi import APIRouter, Response
+from fastapi import APIRouter
+from fastapi.responses import JSONResponse
 from pydantic import BaseModel, HttpUrl
 
-from lso.playbook import PlaybookLaunchResponse, get_playbook_path, run_playbook
+from lso.playbook import get_playbook_path, run_playbook
 
 router = APIRouter()
 
@@ -72,7 +73,7 @@ class IPTrunkDeleteParams(IPTrunkParams):
 
 
 @router.post("/")
-def provision_ip_trunk(params: IPTrunkProvisioningParams, response: Response) -> PlaybookLaunchResponse:
+def provision_ip_trunk(params: IPTrunkProvisioningParams) -> JSONResponse:
     """Launch a playbook to provision a new IP trunk service.
 
     The response will contain either a job ID, or error information.
@@ -80,7 +81,6 @@ def provision_ip_trunk(params: IPTrunkProvisioningParams, response: Response) ->
     :param params: The parameters that define the new subscription object that
         is to be deployed.
     :type params: :class:`IPTrunkProvisioningParams`
-    :param Response response: A FastAPI response used for returning error codes if needed
     :return: Response from the Ansible runner, including a run ID.
     :rtype: :class:`lso.playbook.PlaybookLaunchResponse`
     """
@@ -100,17 +100,15 @@ def provision_ip_trunk(params: IPTrunkProvisioningParams, response: Response) ->
         f"{params.subscription['iptrunk']['iptrunk_sides'][1]['iptrunk_side_node']['router_fqdn']}\n",
         extra_vars=extra_vars,
         callback=params.callback,
-        response=response,
     )
 
 
 @router.put("/")
-def modify_ip_trunk(params: IPTrunkModifyParams, response: Response) -> PlaybookLaunchResponse:
+def modify_ip_trunk(params: IPTrunkModifyParams) -> JSONResponse:
     """Launch a playbook that modifies an existing IP trunk service.
 
     :param params: The parameters that define the change in configuration.
     :type params: :class:`IPTrunkModifyParams`
-    :param Response response: A FastAPI response used for returning error codes if needed
     :return: Response from the Ansible runner, including a run ID.
     :rtype: :class:`lso.playbook.PlaybookLaunchResponse`
     """
@@ -130,18 +128,16 @@ def modify_ip_trunk(params: IPTrunkModifyParams, response: Response) -> Playbook
         f"{params.subscription['iptrunk']['iptrunk_sides'][1]['iptrunk_side_node']['router_fqdn']}\n",
         extra_vars=extra_vars,
         callback=params.callback,
-        response=response,
     )
 
 
 @router.delete("/")
-def delete_ip_trunk(params: IPTrunkDeleteParams, response: Response) -> PlaybookLaunchResponse:
+def delete_ip_trunk(params: IPTrunkDeleteParams) -> JSONResponse:
     """Launch a playbook that deletes an existing IP trunk service.
 
     :param params: Parameters that define the subscription that should get
         terminated.
     :type params: :class:`IPTrunkDeleteParams`
-    :param Response response: A FastAPI response used for returning error codes if needed
     :return: Response from the Ansible runner, including a run ID.
     :rtype: :class:`lso.playbook.PlaybookLaunchResponse`
     """
@@ -161,18 +157,16 @@ def delete_ip_trunk(params: IPTrunkDeleteParams, response: Response) -> Playbook
         f"{params.subscription['iptrunk']['iptrunk_sides'][1]['iptrunk_side_node']['router_fqdn']}\n",
         extra_vars=extra_vars,
         callback=params.callback,
-        response=response,
     )
 
 
 @router.post("/perform_check")
-def check_ip_trunk(params: IPTrunkCheckParams, response: Response) -> PlaybookLaunchResponse:
+def check_ip_trunk(params: IPTrunkCheckParams) -> JSONResponse:
     """Launch a playbook that performs a check on an IP trunk service instance.
 
     :param params: Parameters that define the check that is going to be
         executed, including on which relevant subscription.
     :type params: :class:`IPTrunkCheckParams`
-    :param Response response: A FastAPI response used for returning error codes if needed
     :return: Response from the Ansible runner, including a run ID.
     :rtype: :class:`lso.playbook.PlaybookLaunchResponse`
     """
@@ -184,19 +178,17 @@ def check_ip_trunk(params: IPTrunkCheckParams, response: Response) -> PlaybookLa
         inventory=params.subscription["iptrunk"]["iptrunk_sides"][0]["iptrunk_side_node"]["router_fqdn"],
         extra_vars=extra_vars,
         callback=params.callback,
-        response=response,
     )
 
 
 @router.post("/migrate")
-def migrate_ip_trunk(params: IPTrunkMigrationParams, response: Response) -> PlaybookLaunchResponse:
+def migrate_ip_trunk(params: IPTrunkMigrationParams) -> JSONResponse:
     """Launch a playbook to provision a new IP trunk service.
 
     The response will contain either a job ID, or error information.
 
     :param params: The parameters that define the new subscription object that is to be migrated.
     :type params: :class:`IPTrunkMigrationParams`
-    :param Response response: A FastAPI response used for returning error codes if needed
     :return: Response from the Ansible runner, including a run ID.
     :rtype: :class:`lso.playbook.PlaybookLaunchResponse`
     """
@@ -220,5 +212,4 @@ def migrate_ip_trunk(params: IPTrunkMigrationParams, response: Response) -> Play
         f"{params.new_side['new_node']['router']['router_fqdn']}\n",
         extra_vars=extra_vars,
         callback=params.callback,
-        response=response,
     )
diff --git a/lso/routes/playbook.py b/lso/routes/playbook.py
index 8827ea5..86b1d7b 100644
--- a/lso/routes/playbook.py
+++ b/lso/routes/playbook.py
@@ -2,10 +2,11 @@
 
 from typing import Any
 
-from fastapi import APIRouter, Response
+from fastapi import APIRouter
+from fastapi.responses import JSONResponse
 from pydantic import BaseModel, HttpUrl
 
-from lso.playbook import PlaybookLaunchResponse, get_playbook_path, run_playbook
+from lso.playbook import get_playbook_path, run_playbook
 
 router = APIRouter()
 
@@ -29,20 +30,18 @@ class PlaybookRunParams(BaseModel):
 
 
 @router.post("/")
-def run_playbook_endpoint(params: PlaybookRunParams, response: Response) -> PlaybookLaunchResponse:
+def run_playbook_endpoint(params: PlaybookRunParams) -> JSONResponse:
     """Launch an Ansible playbook to modify or deploy a subscription instance.
 
     The response will contain either a job ID, or error information.
 
     :param params :class:`PlaybookRunParams`: Parameters for executing a playbook.
-    :param Response response: A FastAPI response used for returning error codes if needed
     :return: Response from the Ansible runner, including a run ID.
-    :rtype: :class:`lso.playbook.PlaybookLaunchResponse`
+    :rtype: :class:`fastapi.responses.JSONResponse`
     """
     return run_playbook(
         playbook_path=get_playbook_path(params.playbook_name),
         extra_vars=params.extra_vars,
         inventory=params.inventory,
         callback=params.callback,
-        response=response,
     )
diff --git a/lso/routes/router.py b/lso/routes/router.py
index 6861e5b..e65a7c7 100644
--- a/lso/routes/router.py
+++ b/lso/routes/router.py
@@ -1,6 +1,7 @@
 """Routes for handling device/base_config-related requests."""
 
-from fastapi import APIRouter, Response
+from fastapi import APIRouter
+from fastapi.responses import JSONResponse
 from pydantic import BaseModel, HttpUrl
 
 from lso import playbook
@@ -35,12 +36,11 @@ class NodeProvisioningParams(BaseModel):
 
 
 @router.post("/")
-async def provision_node(params: NodeProvisioningParams, response: Response) -> playbook.PlaybookLaunchResponse:
+async def provision_node(params: NodeProvisioningParams) -> JSONResponse:
     """Launch a playbook to provision a new node. The response will contain either a job id or error information.
 
     :param params: Parameters for provisioning a new node
     :type params: :class:`NodeProvisioningParams`
-    :param Response response: A FastAPI response used for returning error codes if needed
     :return: Response from the Ansible runner, including a run ID.
     :rtype: :class:`lso.playbook.PlaybookLaunchResponse`
     """
@@ -56,5 +56,4 @@ async def provision_node(params: NodeProvisioningParams, response: Response) ->
         inventory=f"{params.subscription['router']['router_fqdn']}",
         extra_vars=extra_vars,
         callback=params.callback,
-        response=response,
     )
-- 
GitLab