diff --git a/.env.example b/.env.example index 51a7ade..8d4185b 100644 --- a/.env.example +++ b/.env.example @@ -1,4 +1,5 @@ LOGS_FILE="object_effects" +# Explicit API roots (e.g. https://host/urban_api) are preserved; bare origins use /api. URBAN_API="https://urban-api.testing" MCP_URBAN_API="https://urban-api.testing" PROMETHEUS_PORT=9464 diff --git a/.github/workflows/ci-dev.yml b/.github/workflows/ci-dev.yml new file mode 100644 index 0000000..96472c4 --- /dev/null +++ b/.github/workflows/ci-dev.yml @@ -0,0 +1,27 @@ +name: Kubernetes dev release + +on: + push: + branches: + - dev + +permissions: + contents: read + +jobs: + release: + uses: IDUclub/urban-assistant-deploy/.github/workflows/reusable-application-release.yaml@main + with: + service: object-effects + test-command: | + python -m pip install --disable-pip-version-check uv==0.12.10 + uv python install 3.11 + uv venv --python 3.11 + uv pip install --python .venv/bin/python \ + --requirement requirements.txt \ + --requirement requirements-auth.txt \ + pytest \ + pytest-asyncio + cp .env.example .env.test + APP_ENV=test .venv/bin/python -m pytest --verbose tests + secrets: inherit diff --git a/README.md b/README.md index ab0a4ee..8dd256b 100644 --- a/README.md +++ b/README.md @@ -1,2 +1,6 @@ # ObjectEffectsAPI Repository for evaluation effects by ObjectNat library + +### Logs + +`GET /logs` downloads the application log file without authorization. diff --git a/app/common/api_handler/api_handler.py b/app/common/api_handler/api_handler.py index 125c75c..2ebcb6d 100644 --- a/app/common/api_handler/api_handler.py +++ b/app/common/api_handler/api_handler.py @@ -1,6 +1,7 @@ import aiohttp from idu_service_auth import KeycloakTokenClient +from app.common.api_handler.urban_api_url import normalize_urban_api_url from app.common.exceptions.http_exception_wrapper import http_exception @@ -19,7 +20,7 @@ def __init__( None """ - self.base_url = base_url + self.base_url = normalize_urban_api_url(base_url) self.service_auth = service_auth async def _service_headers(self, headers: dict | None) -> dict[str, str]: @@ -99,7 +100,8 @@ async def get( session=session, ) headers = await self._service_headers(headers) - url = self.base_url + endpoint_url + endpoint = endpoint_url.lstrip("/").removeprefix("api/") + url = f"{self.base_url}/{endpoint}" async with session.get(url=url, headers=headers, params=params) as response: result = await self._check_response_status(response) if isinstance(result, list): @@ -149,7 +151,8 @@ async def post( session=session, ) headers = await self._service_headers(headers) - url = self.base_url + endpoint_url + endpoint = endpoint_url.lstrip("/").removeprefix("api/") + url = f"{self.base_url}/{endpoint}" async with session.post( url=url, headers=headers, @@ -196,7 +199,8 @@ async def put( session=session, ) headers = await self._service_headers(headers) - url = self.base_url + endpoint_url + endpoint = endpoint_url.lstrip("/").removeprefix("api/") + url = f"{self.base_url}/{endpoint}" async with session.put( url=url, headers=headers, @@ -243,7 +247,8 @@ async def delete( session=session, ) headers = await self._service_headers(headers) - url = self.base_url + endpoint_url + endpoint = endpoint_url.lstrip("/").removeprefix("api/") + url = f"{self.base_url}/{endpoint}" async with session.delete( url=url, headers=headers, diff --git a/app/common/api_handler/urban_api_url.py b/app/common/api_handler/urban_api_url.py new file mode 100644 index 0000000..cda0900 --- /dev/null +++ b/app/common/api_handler/urban_api_url.py @@ -0,0 +1,19 @@ +"""Urban API roots for direct connections and load-balancer mounts.""" + +from urllib.parse import urlsplit, urlunsplit + + +def normalize_urban_api_url(base_url: str) -> str: + """Preserve explicit API roots; use /api only for an origin without a path.""" + url = urlsplit(base_url.strip()) + if ( + url.scheme not in {"http", "https"} + or not url.netloc + or url.query + or url.fragment + ): + raise ValueError( + "Urban API URL must be an HTTP(S) base URL without query or fragment" + ) + path = url.path.rstrip("/") or "/api" + return urlunsplit((url.scheme, url.netloc, path, "", "")) diff --git a/app/main.py b/app/main.py index d519550..1fe9e09 100644 --- a/app/main.py +++ b/app/main.py @@ -1,13 +1,12 @@ from contextlib import asynccontextmanager -from fastapi import Depends, FastAPI +from fastapi import FastAPI from fastapi.middleware.cors import CORSMiddleware from fastapi.responses import FileResponse, RedirectResponse from fastmcp.utilities.lifespan import combine_lifespans from loguru import logger from .__version__ import APP_VERSION -from .common.auth.service_auth import require_service_token from .common.middlewares.exception_handler import ExceptionHandlerMiddleware from .common.middlewares.prometheus_handler import ObservabilityMiddleware from .dependencies import config, http_exception, service_auth @@ -82,7 +81,7 @@ async def read_root(): return {"status": "OK"} -@app.get("/logs", dependencies=[Depends(require_service_token)]) +@app.get("/logs") async def get_logs(): """ Get logs file from app diff --git a/app/provision/provision_mcp.py b/app/provision/provision_mcp.py index 43ff8c4..d63e9d7 100644 --- a/app/provision/provision_mcp.py +++ b/app/provision/provision_mcp.py @@ -52,9 +52,7 @@ async def calc_service_provision( service_type_id=service_type_id, target_population=target_population, ) - result = await provision_mcp_service.calculate_provision( - provision_dto, user_id - ) + result = await provision_mcp_service.calculate_provision(provision_dto, user_id) return result.model_dump() except Exception as e: tb = traceback.format_exc() diff --git a/tests/test_public_diagnostics.py b/tests/test_public_diagnostics.py new file mode 100644 index 0000000..7bb78e9 --- /dev/null +++ b/tests/test_public_diagnostics.py @@ -0,0 +1,12 @@ +from fastapi.testclient import TestClient + +from app.main import app + + +def test_logs_are_public(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + (tmp_path / ".log").write_text("diagnostic log\n", encoding="utf-8") + client = TestClient(app) + response = client.get("/logs") + assert response.status_code == 200 + assert response.text == "diagnostic log\n" diff --git a/tests/test_urban_api_url.py b/tests/test_urban_api_url.py new file mode 100644 index 0000000..e5aa1de --- /dev/null +++ b/tests/test_urban_api_url.py @@ -0,0 +1,83 @@ +from unittest.mock import AsyncMock, MagicMock + +import pytest + +from app.common.api_handler.api_handler import APIHandler +from app.common.api_handler.urban_api_url import normalize_urban_api_url + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "base, api_root", + [ + ("https://urban.test:8443", "https://urban.test:8443/api"), + ("https://urban.test:8443/", "https://urban.test:8443/api"), + ("https://urban.test:8443/api", "https://urban.test:8443/api"), + (" https://urban.test:8443/api/// ", "https://urban.test:8443/api"), + ( + "https://prostor-api.idu.actocgnitive.org/urban_api", + "https://prostor-api.idu.actocgnitive.org/urban_api", + ), + ( + "https://prostor-api.idu.actocgnitive.org/urban_api/", + "https://prostor-api.idu.actocgnitive.org/urban_api", + ), + ( + "https://urban.test/gateway/urban_api/", + "https://urban.test/gateway/urban_api", + ), + ], +) +@pytest.mark.parametrize("method", ["get", "post", "put", "delete"]) +@pytest.mark.parametrize( + "endpoint", + ["/api/v1/scenarios/7", "api/v1/scenarios/7", "/v1/scenarios/7", "v1/scenarios/7"], +) +async def test_requests_use_configured_api_root(base, api_root, method, endpoint): + auth = MagicMock() + auth.get_authorization_headers = AsyncMock( + return_value={"Authorization": "Bearer service"} + ) + handler = APIHandler(base, auth) + session = MagicMock() + response = MagicMock(status=200) + response.json = AsyncMock(return_value={"ok": True}) + request = getattr(session, method) + request.return_value.__aenter__.return_value = response + + assert await getattr(handler, method)( + endpoint, session=session, params={"page": 2} + ) == {"ok": True} + + assert request.call_args.kwargs["url"] == f"{api_root}/v1/scenarios/7" + assert request.call_args.kwargs["params"] == {"page": 2} + assert request.call_args.kwargs["headers"] == {"Authorization": "Bearer service"} + + +@pytest.mark.parametrize( + "base, expected", + [ + ("http://api", "http://api/api"), + ("https://urban.test/gateway/api/", "https://urban.test/gateway/api"), + ("https://urban.test/gateway/", "https://urban.test/gateway"), + ("https://urban.test/api/api/", "https://urban.test/api/api"), + ], +) +def test_normalization_preserves_authority_and_proxy_path(base, expected): + assert normalize_urban_api_url(base) == expected + assert normalize_urban_api_url(expected) == expected + + +@pytest.mark.parametrize( + "base", + [ + "", + "urban.test", + "ftp://urban.test", + "https://urban.test?x=1", + "https://urban.test#fragment", + ], +) +def test_invalid_base_url_is_rejected(base): + with pytest.raises(ValueError, match="Urban API URL"): + normalize_urban_api_url(base)