Skip to content

Commit

Permalink
feat(healthz): overhaul health-checks for them to be less intrusive
Browse files Browse the repository at this point in the history 
This commit removes writing to the DB and MinIO as part of the
health-checks. Additionally also DB-permissions, etc are not checked
anymore. Those checks did not provide any real value, but resulted in
unnecessary lead on the systems.
  • Loading branch information
@open-dynaMIX
open-dynaMIX committed Sep 18, 2023
1 parent fb6db63 commit 67aed89
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 762 deletions.
232 changes: 4 additions & 228 deletions caluma/caluma_core/conftest.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,10 @@
import sys

import pytest
import requests
from django.conf import settings
from django.core import management
from django.db import connection
from minio import Minio
from psycopg2 import OperationalError
from requests.models import Response
from watchman.decorators import check as watchman_check_decorator

from caluma.caluma_form import storage_clients
Expand Down Expand Up @@ -83,156 +80,12 @@ def db_broken_connection(transactional_db):
connection.connect()


def switch_user(test=False):
"""Re-connect to database with either user 'tester' or 'caluma'."""
# set username and password
user = "tester" if test else "caluma"
password = "test" if test else "caluma"

# connect new user to database
connection.close()
settings.DATABASES["default"]["USER"] = user
settings.DATABASES["default"]["PASSWORD"] = password
connection.connect()


@pytest.fixture
def db_user_tester(transactional_db):
"""Create new user 'tester' with privileges necessary for health checks.
User is removed after use.
"""
# get db connection
cursor = connection.cursor()

# create test user (with all necessary privileges)
cursor.execute("CREATE USER tester WITH PASSWORD 'test';")
cursor.execute("GRANT ALL ON caluma_logging_accesslog TO tester;")
cursor.execute("GRANT ALL ON caluma_form_file TO tester;")
cursor.execute(
"GRANT USAGE, SELECT ON SEQUENCE caluma_logging_accesslog_id_seq TO tester;"
)
cursor.execute("GRANT ALL ON django_migrations TO tester;")

yield

# get db connection
cursor = connection.cursor()

# revoke privileges and remove user
cursor.execute("REVOKE ALL ON caluma_logging_accesslog FROM tester;")
cursor.execute("REVOKE ALL ON caluma_form_file FROM tester;")
cursor.execute(
"REVOKE USAGE, SELECT ON SEQUENCE caluma_logging_accesslog_id_seq FROM tester;"
)
cursor.execute("REVOKE ALL ON django_migrations FROM tester;")
cursor.execute("DROP USER tester;")


@pytest.fixture
def db_privileges_working(db_user_tester):
"""DB fixture with all necessary privileges for health checks.
Create new test user with necessary privileges and connect to database.
Remove test user and reconnect with original database user after use.
"""
# switch to new test user
switch_user(test=True)

yield

# switch back to caluma user
switch_user()


@pytest.fixture
def db_read_error(db_user_tester):
"""DB fixture with revoked read privileges on caluma_logging_accesslog.
Create new test user without necessary read privileges for table used by
health check and connect to database.
Remove test user and reconnect with original database user after use.
"""
# revoke select rights on accesslog table
cursor = connection.cursor()
cursor.execute("REVOKE SELECT ON caluma_logging_accesslog FROM tester;")

# switch to new test user
switch_user(test=True)

yield

# switch back to caluma user
switch_user()

# grant select rights on accesslog table such that user can be removed
cursor = connection.cursor()
cursor.execute("GRANT SELECT ON caluma_logging_accesslog TO tester;")


@pytest.fixture
def db_write_error(db_user_tester):
"""DB fixture with revoked write privileges on caluma_logging_accesslog.
Create new test user without necessary write privileges for table used by
health check and connect to database.
Remove test user and reconnect with original database user after use.
"""
# revoke insert, update and delete privileges on accesslog table
cursor = connection.cursor()
cursor.execute(
"REVOKE INSERT,UPDATE,DELETE ON caluma_logging_accesslog FROM tester;"
)

# switch to new test user
switch_user(test=True)

yield

# switch back to caluma user
switch_user()

# grant insert, update, delete rights on accesslog table
# such that user can be removed
cursor = connection.cursor()
cursor.execute("GRANT INSERT,UPDATE,DELETE ON caluma_logging_accesslog TO tester;")


@pytest.fixture
def minio_mock_working(minio_mock, mocker):
"""Provide working minio mock for health checks.
Based on existing minio_mock.
Patches requests.get and requests.put to provide necessary responses for
health check.
"""
# determine fixed UUID to be used during tests
uuid = "7c5ad424-e351-4db8-ba37-bbace45d0e0f"
mocker.patch("uuid.uuid4", return_value=uuid)

# mock upload response
upload_response = Response()
upload_response.status_code = 200
mocker.patch("requests.put", return_value=upload_response)

# mock download response
download_response = Response()
download_response.status_code = 200
download_response._content = (
b"--ffc537c8d0da305912d6440576ac0d75\r\n"
b'Content-Disposition: form-data; name="file"; '
b'filename="healthz_tmp-file.txt"\r\n\r\n'
b"Test\nTest\n\r\n"
b"--ffc537c8d0da305912d6440576ac0d75--\r\n"
def minio_mock_working(mocker):
"""Provide working minio mock for health checks."""
return mocker.patch.object(
storage_clients.client.client, "bucket_exists", return_value=False
)
mocker.patch("requests.get", return_value=download_response)

# change object name of existing stat response
stat_response = Minio.stat_object.return_value
stat_response._object_name = "healthz_tmp-object_" + uuid

# update responses compatible with media storage service health check
Minio.stat_object.side_effect = [stat_response, None]


@pytest.fixture
Expand Down Expand Up @@ -276,83 +129,6 @@ def minio_not_configured():
storage_clients.client = prev_client


@pytest.fixture
def minio_mock_read_error(minio_mock, mocker):
"""Produce exception when downloading file or stat-ing object.
This scenario might occur when there is a internal minio issue or if a
previous upload isn't successful (which doesn't issue exception).
"""

# raise exception instead of handling request
def download_side_effect(*args, **kwargs): # pragma: no cover
raise requests.exceptions.RequestException("GET request failed")

def stat_side_effect(*args, **kwargs):
raise Exception("stat_object failed")

# determine fixed UUID to be used during tests
uuid = "7c5ad424-e351-4db8-ba37-bbace45d0e0f"
mocker.patch("uuid.uuid4", return_value=uuid)

# mock upload response
upload_response = Response()
upload_response.status_code = 200
mocker.patch("requests.put", return_value=upload_response)

# mock erroneous download response
download_response = Response()
download_response.status_code = 500
mocker.patch(
"requests.get", return_value=download_response, side_effect=download_side_effect
)

# raise exception
Minio.stat_object.side_effect = stat_side_effect


@pytest.fixture
def minio_mock_write_error(minio_mock, mocker):
"""Produce exception when uploading file.
This scenario might occur when there is a internal minio issue.
"""

# raise exception instead of handling request
def upload_side_effect(*args, **kwargs):
raise requests.exceptions.RequestException("PUT request failed")

# determine fixed UUID to be used during tests
uuid = "7c5ad424-e351-4db8-ba37-bbace45d0e0f"
mocker.patch("uuid.uuid4", return_value=uuid)

# mock upload response
upload_response = Response()
upload_response.status_code = 500
mocker.patch(
"requests.put", return_value=upload_response, side_effect=upload_side_effect
)

# mock download response
download_response = Response()
download_response.status_code = 200
download_response._content = (
b"--ffc537c8d0da305912d6440576ac0d75\r\n"
b'Content-Disposition: form-data; name="file"; '
b'filename="healthz_tmp-file.txt"\r\n\r\n'
b"Test\nTest\n\r\n"
b"--ffc537c8d0da305912d6440576ac0d75--\r\n"
)
mocker.patch("requests.get", return_value=download_response)

# change object name of existing stat response
stat_response = Minio.stat_object.return_value
stat_response._object_name = "healthz_tmp-object_" + uuid

# update responses compatible with media storage service health check
Minio.stat_object.side_effect = [stat_response, None]


@pytest.fixture
def roll_back_migrations():
"""Roll back database migrations to ensure unapplied migrations exist."""
Expand Down
94 changes: 5 additions & 89 deletions caluma/caluma_core/health_checks.py
View file
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
import uuid
from io import StringIO
from uuid import uuid4

import requests
from django.conf import settings
from django.core import management
from watchman.decorators import check

from caluma.caluma_form import storage_clients
from caluma.caluma_logging.models import AccessLog


@check
Expand All @@ -29,87 +27,13 @@ def _check_pending_migrations(db_name):

@check
def _check_media_storage_service():
"""Check media storage service connection and operations."""
# generate object name
storage_client = storage_clients.client
prefix = "healthz_tmp-object_"
object_name = prefix + str(uuid.uuid4())

try:
# request upload url and upload
upload_url = storage_client.upload_url(object_name)

filename = "healthz_tmp-file.txt"
content = "Test\nTest\n"
files = {"file": (filename, content)}
upload_request = requests.put(upload_url, files=files)
assert upload_request.status_code == 200

# stat object
stat = storage_client.stat_object(object_name)
assert stat.object_name == object_name

# request download url and download file
download_url = storage_client.download_url(object_name)

download_request = requests.get(download_url)
assert download_request.status_code == 200
assert filename in download_request.text and content in download_request.text

# remove object
storage_client.remove_object(object_name)
assert not storage_client.stat_object(object_name, suppress_warning=True)

return {"ok": True}

finally:
storage_client.remove_object(object_name)


@check
def _check_models(database):
"""Check model instantion and operations on database."""
log = None
try:
# Instantiate model
log = AccessLog.objects.create(status_code=22)
assert (
log in AccessLog.objects.all()
), "Failed to save model instance to database."

# Retrieve object
retrieved_log = AccessLog.objects.get(pk=log.pk)
assert log == retrieved_log, "Unexpected object retrieved from database."
assert (
retrieved_log.status_code == log.status_code
), "Unexpected object data retrieved from database."

# Update object
log.status_code = 42
log.save()
retrieved_log = AccessLog.objects.get(pk=log.pk)
assert log == retrieved_log, "Unexpected object retrieved from database."
assert (
retrieved_log.status_code == log.status_code
), "Unexpected object data retrieved from database."

# Remove object
log.delete()
log = None
assert (
log not in AccessLog.objects.all()
), "Failed to delete model instance from database."

return {database: {"ok": True}}

finally:
# Cleanup
if log: # pragma: no cover
log.delete()
"""Check media storage service connectivity."""
assert not storage_clients.client.client.bucket_exists(str(uuid4()))
return {"ok": True}


def check_media_storage_service():
"""Check media storage service connection and operations."""
"""Check media storage service connectivity."""
# Check if media storage service is available
if not storage_clients.client:
return {"media storage service (not configured)": {"ok": True}}
Expand All @@ -123,11 +47,3 @@ def check_migrations():
checked_databases = [_check_pending_migrations(db_name) for db_name in databases]

return {"database migrations": checked_databases}


def check_models():
"""Check model instantiation."""
databases = sorted(settings.DATABASES)
checked_databases = [_check_models(db_name) for db_name in databases]

return {"database models": checked_databases}
Loading

0 comments on commit 67aed89

Please sign in to comment.