Pin all dependencies to exact versions.

Add input validation to sanitize data at API level.
Write a test for incorrect playbook paths.

README.md

@@ -27,8 +27,6 @@ services:
     image: my-lso:latest
     env_file: 
       .env  # Load default environment variables from the .env file
-    environment:
-      ANSIBLE_ROLES_PATH: /app/lso/ansible_roles  # Set specific Ansible roles path
     volumes:
       - "/home/user/ansible_inventory:/opt/ansible_inventory:ro"
       - "~/.ssh/id_ed25519.pub:/root/.ssh/id_ed25519.pub:ro"

lso/__init__.py

@@ -27,7 +27,7 @@
 
 def create_app() -> FastAPI:
     """Initialise the :term:`LSO` app."""
-    app = FastAPI()
+    app = FastAPI(docs_url="/api/doc", redoc_url="/api/redoc", openapi_url="/api/openapi.json")
 
     app.add_middleware(
         CORSMiddleware, allow_origins=["*"], allow_credentials=True, allow_methods=["*"], allow_headers=["*"]

lso/playbook.py

@@ -18,9 +18,6 @@
 from pathlib import Path
 from typing import Any
 
-import ansible_runner
-from fastapi import status
-from fastapi.responses import JSONResponse
 from pydantic import HttpUrl
 
 from lso.config import ExecutorType, settings
@@ -43,36 +40,17 @@ def get_thread_pool() -> ThreadPoolExecutor:
     return _executor
 
 
-def get_playbook_path(playbook_name: str) -> Path:
+def get_playbook_path(playbook_name: Path) -> Path:
     """Get the path of a playbook on the local filesystem."""
     return Path(settings.ANSIBLE_PLAYBOOKS_ROOT_DIR) / playbook_name
 
 
-def playbook_launch_success(job_id: str) -> JSONResponse:
-    """Return a :class:`PlaybookLaunchResponse` for the successful start of a playbook execution.
-
-    :return JSONResponse: A playbook launch response that's successful.
-    """
-    return JSONResponse(content={"job_id": job_id}, status_code=status.HTTP_201_CREATED)
-
-
-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.
-
-    :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 JSONResponse(content={"error": reason}, status_code=status_code)
-
-
 def run_playbook(
     playbook_path: Path,
     extra_vars: dict[str, Any],
     inventory: dict[str, Any] | str,
     callback: HttpUrl,
-) -> JSONResponse:
+) -> uuid.UUID:
     """Run an Ansible playbook against a specified inventory.
 
     :param Path playbook_path: playbook to be executed.
@@ -83,24 +61,16 @@ def run_playbook(
     :return: Result of playbook launch, this could either be successful or unsuccessful.
     :rtype: :class:`fastapi.responses.JSONResponse`
     """
-    if not Path.exists(playbook_path):
-        msg = f"Filename '{playbook_path}' does not exist."
-        return playbook_launch_error(reason=msg, status_code=status.HTTP_404_NOT_FOUND)
-
-    if not ansible_runner.utils.isinventory(inventory):
-        msg = "Invalid inventory provided. Should be a string, or JSON object."
-        return playbook_launch_error(reason=msg, status_code=status.HTTP_400_BAD_REQUEST)
-
-    job_id = str(uuid.uuid4())
+    job_id = uuid.uuid4()
     if settings.EXECUTOR == ExecutorType.THREADPOOL:
         executor = get_thread_pool()
         executor_handle = executor.submit(
-            run_playbook_proc_task, job_id, str(playbook_path), extra_vars, inventory, str(callback)
+            run_playbook_proc_task, str(job_id), str(playbook_path), extra_vars, inventory, str(callback)
         )
         if settings.TESTING:
             executor_handle.result()
 
     elif settings.EXECUTOR == ExecutorType.WORKER:
-        run_playbook_proc_task.delay(job_id, str(playbook_path), extra_vars, inventory, str(callback))
+        run_playbook_proc_task.delay(str(job_id), str(playbook_path), extra_vars, inventory, str(callback))
 
-    return playbook_launch_success(job_id=job_id)
+    return job_id

lso/routes/playbook.py

@@ -15,14 +15,16 @@
 
 import json
 import tempfile
+import uuid
 from contextlib import redirect_stderr
 from io import StringIO
+from pathlib import Path
 from typing import Annotated, Any
 
+import ansible_runner
 from ansible.inventory.manager import InventoryManager
 from ansible.parsing.dataloader import DataLoader
 from fastapi import APIRouter, HTTPException, status
-from fastapi.responses import JSONResponse
 from pydantic import AfterValidator, BaseModel, HttpUrl
 
 from lso.playbook import get_playbook_path, run_playbook
@@ -31,11 +33,19 @@
 
 
 def _inventory_validator(inventory: dict[str, Any] | str) -> dict[str, Any] | str:
-    """Validate the format of the provided inventory by trying to parse it.
+    """Validate the provided inventory format.
 
-    If an inventory can't be parsed without warnings or errors, these are returned to the user by means of an HTTP
-    status 422 for 'unprocessable entity'.
+    Attempts to parse the inventory to verify its validity. If the inventory cannot be parsed or the inventory
+    format is incorrect an HTTP 400 error is raised. If , an HTTP 400 error is raised.
+
+    :param inventory: The inventory to validate, can be a dictionary or a string.
+    :return: The validated inventory if no errors are found.
+    :raises HTTPException: If parsing fails or the format is incorrect.
     """
+    if not ansible_runner.utils.isinventory(inventory):
+        error_messages = "Invalid inventory provided. Should be a string, or JSON object."
+        raise HTTPException(status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, detail=error_messages)
+
     loader = DataLoader()
     output = StringIO()
     with tempfile.NamedTemporaryFile(mode="w+") as temp_inv, redirect_stderr(output):
@@ -53,15 +63,31 @@ def _inventory_validator(inventory: dict[str, Any] | str) -> dict[str, Any] | st
     return inventory
 
 
+def _playbook_path_validator(playbook_name: Path) -> Path:
+    playbook_path = get_playbook_path(playbook_name)
+    if not Path.exists(playbook_path):
+        msg = f"Filename '{playbook_path}' does not exist."
+        raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=msg)
+
+    return playbook_path
+
+
 PlaybookInventory = Annotated[dict[str, Any] | str, AfterValidator(_inventory_validator)]
+PlaybookName = Annotated[Path, AfterValidator(_playbook_path_validator)]
+
+
+class PlaybookRunResponse(BaseModel):
+    """PlaybookRunResponse domain model schema."""
+
+    job_id: uuid.UUID
 
 
 class PlaybookRunParams(BaseModel):
     """Parameters for executing an Ansible playbook."""
 
     #: The filename of a playbook that's executed. It should be present inside the directory defined in the
     #: configuration option ``ANSIBLE_PLAYBOOKS_ROOT_DIR``.
-    playbook_name: str
+    playbook_name: PlaybookName
     #: The address where LSO should call back to upon completion.
     callback: HttpUrl
     #: The inventory to run the playbook against. This inventory can also include any host vars, if needed. When
@@ -74,18 +100,20 @@ class PlaybookRunParams(BaseModel):
     extra_vars: dict[str, Any] = {}
 
 
-@router.post("/")
-def run_playbook_endpoint(params: PlaybookRunParams) -> JSONResponse:
+@router.post("/", response_model=PlaybookRunResponse, status_code=status.HTTP_201_CREATED)
+def run_playbook_endpoint(params: PlaybookRunParams) -> PlaybookRunResponse:
     """Launch an Ansible playbook to modify or deploy a subscription instance.
 
     The response will contain either a job ID, or error information.
 
     :param PlaybookRunParams params: Parameters for executing a playbook.
     :return JSONResponse: Response from the Ansible runner, including a run ID.
     """
-    return run_playbook(
-        playbook_path=get_playbook_path(params.playbook_name),
+    job_id = run_playbook(
+        playbook_path=params.playbook_name,
         extra_vars=params.extra_vars,
         inventory=params.inventory,
         callback=params.callback,
     )
+
+    return PlaybookRunResponse(job_id=job_id)

lso/tasks.py

@@ -30,6 +30,10 @@
 logger = logging.getLogger(__name__)
 
 
+class CallbackFailedError(Exception):
+    """Exception raised when a callback url can't be reached."""
+
+
 @celery.task(name=RUN_PLAYBOOK)  # type: ignore[misc]
 def run_playbook_proc_task(
     job_id: str, playbook_path: str, extra_vars: dict[str, Any], inventory: dict[str, Any] | str, callback: str
@@ -58,3 +62,4 @@ def run_playbook_proc_task(
     if not status.HTTP_200_OK <= request_result.status_code < status.HTTP_300_MULTIPLE_CHOICES:
         msg = f"Callback failed: {request_result.text}, url: {callback}"
         logger.error(msg)
+        raise CallbackFailedError(msg)

pyproject.toml

@@ -29,15 +29,15 @@ classifiers = [
     "Programming Language :: Python :: 3.12",
 ]
 dependencies = [
-    "ansible-runner~=2.4.0",
-    "ansible~=10.5.0",
-    "fastapi~=0.115.2",
-    "httpx~=0.27.2",
-    "uvicorn[standard]~=0.32.0",
-    "requests~=2.32.3",
-    "pydantic-settings~=2.5.2",
-    "celery~=5.4.0",
-    "redis~=5.2.0",
+    "ansible-runner==2.4.0",
+    "ansible==10.6.0",
+    "fastapi==0.115.5",
+    "httpx==0.27.2",
+    "uvicorn[standard]==0.32.0",
+    "requests==2.32.3",
+    "pydantic-settings==2.5.2",
+    "celery==5.4.0",
+    "redis==5.2.0",
 ]
 
 readme = "README.md"

test/routes/test_playbook.py

@@ -13,6 +13,7 @@
 import re
 from collections.abc import Callable
 from contextlib import contextmanager
+from pathlib import Path
 from unittest.mock import MagicMock, patch
 
 import pytest
@@ -21,6 +22,7 @@
 from fastapi.testclient import TestClient
 
 from lso.config import ExecutorType, settings
+from lso.playbook import get_playbook_path
 
 TEST_CALLBACK_URL = "https://fqdn.abc.xyz/api/resume"
 
@@ -49,7 +51,7 @@ def test_playbook_endpoint_dict_inventory_success(client: TestClient, mocked_ans
         "extra_vars": {"dry_run": True, "commit_comment": "I am a robot!"},
     }
 
-    with patch("lso.playbook.ansible_runner.run", new=mocked_ansible_runner_run) as _:
+    with patch("lso.routes.playbook.ansible_runner.run", new=mocked_ansible_runner_run):
         rv = client.post("/api/playbook/", json=params)
         assert rv.status_code == status.HTTP_201_CREATED
         response = rv.json()
@@ -69,7 +71,7 @@ def test_playbook_endpoint_str_inventory_success(client: TestClient, mocked_ansi
         "inventory": {"all": {"hosts": "host1.local\nhost2.local\nhost3.local"}},
     }
 
-    with patch("lso.playbook.ansible_runner.run", new=mocked_ansible_runner_run) as _:
+    with patch("lso.routes.playbook.ansible_runner.run", new=mocked_ansible_runner_run):
         rv = client.post("/api/playbook/", json=params)
         assert rv.status_code == status.HTTP_201_CREATED
         response = rv.json()
@@ -90,7 +92,7 @@ def test_playbook_endpoint_invalid_host_vars(client: TestClient, mocked_ansible_
         },
     }
 
-    with patch("lso.playbook.ansible_runner.run", new=mocked_ansible_runner_run) as _:
+    with patch("lso.routes.playbook.ansible_runner.run", new=mocked_ansible_runner_run) as _:
         rv = client.post("/api/playbook/", json=params)
         assert rv.status_code == status.HTTP_422_UNPROCESSABLE_ENTITY
         response = rv.json()
@@ -114,7 +116,7 @@ def test_playbook_endpoint_invalid_hosts(client: TestClient, mocked_ansible_runn
         },
     }
 
-    with patch("lso.playbook.ansible_runner.run", new=mocked_ansible_runner_run) as _:
+    with patch("lso.routes.playbook.ansible_runner.run", new=mocked_ansible_runner_run):
         rv = client.post("/api/playbook/", json=params)
         assert rv.status_code == status.HTTP_422_UNPROCESSABLE_ENTITY
         response = rv.json()
@@ -198,3 +200,25 @@ def test_run_playbook_invalid_inventory(client: TestClient, executor_type: Execu
 
         rv = client.post("/api/playbook/", json=params)
         assert rv.status_code == status.HTTP_422_UNPROCESSABLE_ENTITY
+
+
+@responses.activate
+def test_run_playbook_invalid_playbook_path(client: TestClient) -> None:
+    """Test that the playbook runs fails with invalid playbook name/path."""
+    responses.post(url=TEST_CALLBACK_URL, status=status.HTTP_200_OK)
+
+    params = {
+        "playbook_name": "invalid.yaml",
+        "callback": TEST_CALLBACK_URL,
+        "inventory": {
+            "_meta": {"vars": {"host1.local": {"foo": "bar"}}},
+            "all": {"hosts": {"host1.local": None}},
+        },
+        "extra_vars": {"dry_run": True},
+    }
+
+    with patch("lso.tasks.run_playbook_proc_task.delay"):
+        rv = client.post("/api/playbook/", json=params)
+        assert rv.status_code == status.HTTP_404_NOT_FOUND
+        response = rv.json()
+        assert response["detail"] == f"Filename '{get_playbook_path(Path('invalid.yaml'))}' does not exist."