From 4cbaef6b12c9bfbd1e845c8a4b7072477f70f9d9 Mon Sep 17 00:00:00 2001
From: jenriordan <150944262+jenriordan@users.noreply.github.com>
Date: Fri, 3 Jan 2025 13:43:52 +0000
Subject: [PATCH] DST-835: Fix the s3 health check (#162)

---
 django_app/healthcheck/checks/s3.py           | 18 +++++----
 django_app/healthcheck/views.py               |  6 +--
 .../test_unit/test_healthcheck/test_views.py  | 38 +++++++++++++++++++
 3 files changed, 52 insertions(+), 10 deletions(-)
 create mode 100644 tests/test_unit/test_healthcheck/test_views.py

diff --git a/django_app/healthcheck/checks/s3.py b/django_app/healthcheck/checks/s3.py
index f43e671a..be0be227 100644
--- a/django_app/healthcheck/checks/s3.py
+++ b/django_app/healthcheck/checks/s3.py
@@ -1,18 +1,22 @@
+import boto3
 import sentry_sdk
-from core.document_storage import PermanentDocumentStorage, TemporaryDocumentStorage
+from django.conf import settings
 
 
 def s3_check() -> bool:
     """
-    Performs a check on the S3 connection
+    Check if the S3 bucket exists and ensure the app can access it.
+    https://boto3.amazonaws.com/v1/documentation/api/1.35.9/reference/services/s3/client/head_bucket.html
     """
-    temporary_document_bucket = TemporaryDocumentStorage().bucket
-    permanent_document_bucket = PermanentDocumentStorage().bucket
+    client = boto3.client("s3")
+
+    bucket_names = [settings.TEMPORARY_S3_BUCKET_NAME, settings.PERMANENT_S3_BUCKET_NAME]
 
     try:
-        assert temporary_document_bucket.creation_date
-        assert permanent_document_bucket.creation_date
-        return True
+        for bucket_name in bucket_names:
+            client.head_bucket(Bucket=bucket_name)
     except Exception as e:
         sentry_sdk.capture_exception(e)
         return False
+
+    return True
diff --git a/django_app/healthcheck/views.py b/django_app/healthcheck/views.py
index 92b5e701..f83ccf10 100644
--- a/django_app/healthcheck/views.py
+++ b/django_app/healthcheck/views.py
@@ -4,7 +4,7 @@
 from django.http import HttpRequest, HttpResponse
 from django.shortcuts import render
 from django.views.generic import View
-from healthcheck.checks import db_check
+from healthcheck.checks import db_check, s3_check
 
 
 class HealthCheckView(View):
@@ -20,8 +20,8 @@ def get(self, request: HttpRequest, *args: object, **kwargs: object) -> HttpResp
 
         start = time.time()
         is_db_good = db_check()
-        # is_s3_good = s3_check()
-        all_good = is_db_good and True
+        is_s3_good = s3_check()
+        all_good = is_db_good and is_s3_good
 
         end = time.time()
         time_taken = round(end - start, 3)
diff --git a/tests/test_unit/test_healthcheck/test_views.py b/tests/test_unit/test_healthcheck/test_views.py
new file mode 100644
index 00000000..33134abf
--- /dev/null
+++ b/tests/test_unit/test_healthcheck/test_views.py
@@ -0,0 +1,38 @@
+from unittest.mock import patch
+
+import pytest
+from django.urls import reverse
+
+from tests.helpers import get_response_content
+
+
+@pytest.fixture(autouse=True)
+def setup():
+    """Need to fix the Sites context processor as healthcheck views don't have a site."""
+    with patch("core.sites.context_processors.sites", return_value={}):
+        yield
+
+
+@patch("healthcheck.checks.s3.boto3")
+def test_successful_healthcheck(mock_s3_client, al_client):
+    mock_s3_client.head_bucket.return_value = {"test": True}
+    response = al_client.get(reverse("healthcheck:healthcheck_ping"))
+    content = get_response_content(response)
+    assert "OK" in content
+    assert response.status_code == 200
+
+
+@patch("healthcheck.views.s3_check", return_value=False)
+def test_s3_broken_healthcheck(mock_s3_check, al_client):
+    response = al_client.get(reverse("healthcheck:healthcheck_ping"))
+    content = get_response_content(response)
+    assert "FAIL" in content
+    assert response.status_code == 200
+
+
+@patch("healthcheck.views.db_check", return_value=False)
+def test_db_broken_healthcheck(mock_db_check, al_client):
+    response = al_client.get(reverse("healthcheck:healthcheck_ping"))
+    content = get_response_content(response)
+    assert "FAIL" in content
+    assert response.status_code == 200