diff --git a/CHANGELOG.md b/CHANGELOG.md index 5afd0a4456..c1cd245504 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,9 +44,9 @@ The types of changes are: * Resolved a failure with populating applicable data subject rights to a data map * Updated `fideslog` to v1.1.5, resolving an issue where some exceptions thrown by the SDK were not handled as expected * Host static files via fidesapi [#621](https://github.com/ethyca/fides/pull/621) +* Updated the webserver so that it won't fail if the database is inaccessible [#649](https://github.com/ethyca/fides/pull/649) * Handle complex characters in external tests [#661](https://github.com/ethyca/fides/pull/661) - ## [1.6.0](https://github.com/ethyca/fides/compare/1.5.3...1.6.0) - 2022-05-02 ### Added diff --git a/src/fidesapi/database/database.py b/src/fidesapi/database/database.py index da585c0265..dc8d6a1869 100644 --- a/src/fidesapi/database/database.py +++ b/src/fidesapi/database/database.py @@ -3,15 +3,21 @@ """ from os import path -from alembic import command +from alembic import command, script from alembic.config import Config from alembic.migration import MigrationContext +from alembic.runtime import migration from fideslang import DEFAULT_TAXONOMY from loguru import logger as log +from sqlalchemy import create_engine from sqlalchemy_utils.functions import create_database, database_exists from fidesapi.sql_models import SqlAlchemyBase, sql_model_map -from fidesapi.utils.errors import AlreadyExistsError, QueryError +from fidesapi.utils.errors import ( + AlreadyExistsError, + QueryError, + get_full_exception_name, +) from fidesctl.core.utils import get_db_engine from .crud import create_resource, upsert_resources @@ -108,3 +114,23 @@ def reset_db(database_url: str) -> None: version = migration_context._version # pylint: disable=protected-access if version.exists(connection): version.drop(connection) + + +def get_db_health(database_url: str) -> str: + """Checks if the db is reachable and up to date in alembic migrations""" + try: + engine = create_engine(database_url) + alembic_config = get_alembic_config(database_url) + alembic_script_directory = script.ScriptDirectory.from_config(alembic_config) + with engine.begin() as conn: + context = migration.MigrationContext.configure(conn) + if ( + context.get_current_revision() + != alembic_script_directory.get_current_head() + ): + return "needs migration" + return "healthy" + except Exception as error: # pylint: disable=broad-except + error_type = get_full_exception_name(error) + log.error(f"Unable to reach the database: {error_type}: {error}") + return "unhealthy" diff --git a/src/fidesapi/main.py b/src/fidesapi/main.py index 2e4de98ae1..b53e35e573 100644 --- a/src/fidesapi/main.py +++ b/src/fidesapi/main.py @@ -2,6 +2,7 @@ Contains the code that sets up the API. """ + from datetime import datetime from enum import Enum from logging import WARNING @@ -17,8 +18,10 @@ import fidesctl from fidesapi import view from fidesapi.database import database +from fidesapi.database.database import get_db_health from fidesapi.routes import crud, visualize from fidesapi.routes.util import API_PREFIX, WEBAPP_DIRECTORY, WEBAPP_INDEX +from fidesapi.utils.errors import get_full_exception_name from fidesapi.utils.logger import setup as setup_logging from fidesctl.core.config import FidesctlConfig, get_config @@ -42,8 +45,12 @@ def configure_routes() -> None: async def configure_db(database_url: str) -> None: "Set up the db to be used by the app." - database.create_db_if_not_exists(database_url) - await database.init_db(database_url) + try: + database.create_db_if_not_exists(database_url) + await database.init_db(database_url) + except Exception as error: # pylint: disable=broad-except + error_type = get_full_exception_name(error) + log.error(f"Unable to configure database: {error_type}: {error}") @app.on_event("startup") @@ -96,20 +103,44 @@ async def log_request(request: Request, call_next: Callable) -> Response: "example": { "status": "healthy", "version": "1.0.0", + "database": "healthy", + } + } + } + }, + status.HTTP_503_SERVICE_UNAVAILABLE: { + "content": { + "application/json": { + "example": { + "detail": { + "status": "healthy", + "version": "1.0.0", + "database": "unhealthy", + } } } } - } + }, }, tags=["Health"], ) async def health() -> Dict: "Confirm that the API is running and healthy." - return { + database_health = get_db_health(CONFIG.api.sync_database_url) + response = { "status": "healthy", "version": str(fidesctl.__version__), + "database": database_health, } + for key in response: + if response[key] == "unhealthy": + raise HTTPException( + status_code=status.HTTP_503_SERVICE_UNAVAILABLE, detail=response + ) + + return response + class DBActions(str, Enum): "The available path parameters for the `/admin/db/{action}` endpoint." @@ -152,7 +183,9 @@ def read_other_paths(request: Request) -> FileResponse: # raise 404 for anything that should be backend endpoint but we can't find it if path.startswith(API_PREFIX[1:]): - raise HTTPException(status_code=404, detail="Item not found") + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, detail="Item not found" + ) # otherwise return the index return FileResponse(WEBAPP_INDEX) diff --git a/src/fidesapi/utils/errors.py b/src/fidesapi/utils/errors.py index 05259f9617..95e0098c39 100644 --- a/src/fidesapi/utils/errors.py +++ b/src/fidesapi/utils/errors.py @@ -54,3 +54,13 @@ def __init__(self) -> None: status.HTTP_500_INTERNAL_SERVER_ERROR, detail={"error": "a database query failed"}, ) + + +def get_full_exception_name(exception: Exception) -> str: + """Get the full exception name + i.e. get sqlalchemy.exc.IntegrityError instead of just IntegrityError + """ + module = exception.__class__.__module__ + if module is None or module == str.__class__.__module__: + return exception.__class__.__name__ + return module + "." + exception.__class__.__name__ diff --git a/tests/core/test_api.py b/tests/core/test_api.py index 24a9681475..efed1e8ffd 100644 --- a/tests/core/test_api.py +++ b/tests/core/test_api.py @@ -1,17 +1,27 @@ # pylint: disable=missing-docstring, redefined-outer-name """Integration tests for the API module.""" from json import loads -from typing import Dict +from typing import Dict, Generator import pytest import requests from fideslang import model_list, parse +from pytest import MonkeyPatch +from starlette.testclient import TestClient +from fidesapi import main from fidesapi.routes.util import API_PREFIX from fidesctl.core import api as _api from fidesctl.core.config import FidesctlConfig +@pytest.fixture() +def test_client() -> Generator: + """Starlette test client fixture. Easier to use mocks with when testing out API calls""" + with TestClient(main.app) as test_client: + yield test_client + + # Helper Functions def get_existing_key(test_config: FidesctlConfig, resource_type: str) -> int: """Get an ID that is known to exist.""" @@ -47,140 +57,122 @@ def test_generate_resource_urls_with_id(test_config: FidesctlConfig) -> None: assert expected_url == result_url -# Integration Tests -@pytest.mark.integration -def test_api_ping(test_config: FidesctlConfig) -> None: - assert ( - _api.ping(test_config.cli.server_url + API_PREFIX + "/health").status_code - == 200 - ) - - @pytest.mark.integration -@pytest.mark.parametrize("endpoint", model_list) -def test_api_create( - test_config: FidesctlConfig, resources_dict: Dict, endpoint: str -) -> None: - manifest = resources_dict[endpoint] - print(manifest.json(exclude_none=True)) - result = _api.create( - url=test_config.cli.server_url, - resource_type=endpoint, - json_resource=manifest.json(exclude_none=True), - headers=test_config.user.request_headers, - ) - print(result.text) - assert result.status_code == 201 - - -@pytest.mark.integration -@pytest.mark.parametrize("endpoint", model_list) -def test_api_ls(test_config: FidesctlConfig, endpoint: str) -> None: - result = _api.ls( - url=test_config.cli.server_url, - resource_type=endpoint, - headers=test_config.user.request_headers, - ) - print(result.text) - assert result.status_code == 200 - - -@pytest.mark.integration -@pytest.mark.parametrize("endpoint", model_list) -def test_api_get(test_config: FidesctlConfig, endpoint: str) -> None: - existing_id = get_existing_key(test_config, endpoint) - result = _api.get( - url=test_config.cli.server_url, - headers=test_config.user.request_headers, - resource_type=endpoint, - resource_id=existing_id, - ) - print(result.text) - assert result.status_code == 200 - - -@pytest.mark.integration -@pytest.mark.parametrize("endpoint", model_list) -def test_sent_is_received( - test_config: FidesctlConfig, resources_dict: Dict, endpoint: str -) -> None: - """ - Confirm that the resource and values that we send are the - same as the resource that the server returns. - """ - manifest = resources_dict[endpoint] - resource_key = manifest.fides_key if endpoint != "user" else manifest.userName - - print(manifest.json(exclude_none=True)) - result = _api.get( - url=test_config.cli.server_url, - headers=test_config.user.request_headers, - resource_type=endpoint, - resource_id=resource_key, - ) - print(result.text) - assert result.status_code == 200 - parsed_result = parse.parse_dict(endpoint, result.json()) - - assert parsed_result == manifest - - -@pytest.mark.integration -@pytest.mark.parametrize("endpoint", model_list) -def test_api_update( - test_config: FidesctlConfig, resources_dict: Dict, endpoint: str -) -> None: - - manifest = resources_dict[endpoint] - result = _api.update( - url=test_config.cli.server_url, - headers=test_config.user.request_headers, - resource_type=endpoint, - json_resource=manifest.json(exclude_none=True), - ) - print(result.text) - assert result.status_code == 200 - - -@pytest.mark.integration -@pytest.mark.parametrize("endpoint", model_list) -def test_api_upsert( - test_config: FidesctlConfig, resources_dict: Dict, endpoint: str -) -> None: - manifest = resources_dict[endpoint] - result = _api.upsert( - url=test_config.cli.server_url, - headers=test_config.user.request_headers, - resource_type=endpoint, - resources=[loads(manifest.json())], - ) - - assert result.status_code == 200 - - -@pytest.mark.integration -@pytest.mark.parametrize("endpoint", model_list) -def test_api_delete( - test_config: FidesctlConfig, resources_dict: Dict, endpoint: str -) -> None: - manifest = resources_dict[endpoint] - resource_key = manifest.fides_key if endpoint != "user" else manifest.userName - - result = _api.delete( - url=test_config.cli.server_url, - resource_type=endpoint, - resource_id=resource_key, - headers=test_config.user.request_headers, - ) - print(result.text) - assert result.status_code == 200 - - +class TestCrud: + @pytest.mark.parametrize("endpoint", model_list) + def test_api_create( + self, test_config: FidesctlConfig, resources_dict: Dict, endpoint: str + ) -> None: + manifest = resources_dict[endpoint] + print(manifest.json(exclude_none=True)) + result = _api.create( + url=test_config.cli.server_url, + resource_type=endpoint, + json_resource=manifest.json(exclude_none=True), + headers=test_config.user.request_headers, + ) + print(result.text) + assert result.status_code == 201 + + @pytest.mark.parametrize("endpoint", model_list) + def test_api_ls(self, test_config: FidesctlConfig, endpoint: str) -> None: + result = _api.ls( + url=test_config.cli.server_url, + resource_type=endpoint, + headers=test_config.user.request_headers, + ) + print(result.text) + assert result.status_code == 200 + + @pytest.mark.parametrize("endpoint", model_list) + def test_api_get(self, test_config: FidesctlConfig, endpoint: str) -> None: + existing_id = get_existing_key(test_config, endpoint) + result = _api.get( + url=test_config.cli.server_url, + headers=test_config.user.request_headers, + resource_type=endpoint, + resource_id=existing_id, + ) + print(result.text) + assert result.status_code == 200 + + @pytest.mark.parametrize("endpoint", model_list) + def test_sent_is_received( + self, test_config: FidesctlConfig, resources_dict: Dict, endpoint: str + ) -> None: + """ + Confirm that the resource and values that we send are the + same as the resource that the server returns. + """ + manifest = resources_dict[endpoint] + resource_key = manifest.fides_key if endpoint != "user" else manifest.userName + + print(manifest.json(exclude_none=True)) + result = _api.get( + url=test_config.cli.server_url, + headers=test_config.user.request_headers, + resource_type=endpoint, + resource_id=resource_key, + ) + print(result.text) + assert result.status_code == 200 + parsed_result = parse.parse_dict(endpoint, result.json()) + + assert parsed_result == manifest + + @pytest.mark.parametrize("endpoint", model_list) + def test_api_update( + self, test_config: FidesctlConfig, resources_dict: Dict, endpoint: str + ) -> None: + manifest = resources_dict[endpoint] + result = _api.update( + url=test_config.cli.server_url, + headers=test_config.user.request_headers, + resource_type=endpoint, + json_resource=manifest.json(exclude_none=True), + ) + print(result.text) + assert result.status_code == 200 + + @pytest.mark.parametrize("endpoint", model_list) + def test_api_upsert( + self, test_config: FidesctlConfig, resources_dict: Dict, endpoint: str + ) -> None: + manifest = resources_dict[endpoint] + result = _api.upsert( + url=test_config.cli.server_url, + headers=test_config.user.request_headers, + resource_type=endpoint, + resources=[loads(manifest.json())], + ) + assert result.status_code == 200 + + @pytest.mark.parametrize("endpoint", model_list) + def test_api_delete( + self, test_config: FidesctlConfig, resources_dict: Dict, endpoint: str + ) -> None: + manifest = resources_dict[endpoint] + resource_key = manifest.fides_key if endpoint != "user" else manifest.userName + + result = _api.delete( + url=test_config.cli.server_url, + resource_type=endpoint, + resource_id=resource_key, + headers=test_config.user.request_headers, + ) + print(result.text) + assert result.status_code == 200 + + +# This test will fail if certain other tests run before it, due to a non-deterministic bug in the code +# Keeping the order as-is is a temporary fix @pytest.mark.integration @pytest.mark.parametrize( "resource_type", ["data_category", "data_use", "data_qualifier"] ) -def test_visualize(test_config: FidesctlConfig, resource_type: str) -> None: +def test_visualize( + setup_db: str, test_config: FidesctlConfig, resource_type: str +) -> None: response = requests.get( f"{test_config.cli.server_url}{API_PREFIX}/{resource_type}/visualize/graphs" ) @@ -201,3 +193,24 @@ def test_404_on_api_routes(test_config: FidesctlConfig) -> None: f"{test_config.cli.server_url}{API_PREFIX}/path/that/does/not/exist" ) assert response.status_code == 404 + + +# Integration Tests +@pytest.mark.integration +@pytest.mark.parametrize( + "database_health, expected_status_code", + [("healthy", 200), ("needs migration", 200), ("unhealthy", 503)], +) +def test_api_ping( + test_config: FidesctlConfig, + database_health: str, + expected_status_code: int, + monkeypatch: MonkeyPatch, + test_client: TestClient, +) -> None: + def mock_get_db_health(url: str) -> str: + return database_health + + monkeypatch.setattr(main, "get_db_health", mock_get_db_health) + response = test_client.get(test_config.cli.server_url + API_PREFIX + "/health") + assert response.status_code == expected_status_code