From 027274389d7aaa22ec3dad3dca47727090d2e50a Mon Sep 17 00:00:00 2001
From: Zeid Zabaneh <zeid@mozilla.com>
Date: Fri, 15 Dec 2023 12:16:18 -0500
Subject: [PATCH] settings: add cache configuration (bug 1870285)

- add fallback DB backend
- add environment based redis configuration
- remove code that is no longer needed
---
 landoapi/cache.py        | 64 ----------------------------------------
 src/lando/settings.py    | 21 +++++++++++++
 tests/conftest.py        | 22 --------------
 tests/test_dockerflow.py |  4 +--
 tests/test_health.py     | 21 -------------
 5 files changed, 23 insertions(+), 109 deletions(-)
 delete mode 100644 landoapi/cache.py

diff --git a/landoapi/cache.py b/landoapi/cache.py
deleted file mode 100644
index 6d8053b9..00000000
--- a/landoapi/cache.py
+++ /dev/null
@@ -1,64 +0,0 @@
-# This Source Code Form is subject to the terms of the Mozilla Public
-# License, v. 2.0. If a copy of the MPL was not distributed with this
-# file, You can obtain one at http://mozilla.org/MPL/2.0/.
-
-from __future__ import annotations
-
-import logging
-
-from flask_caching import Cache
-from flask_caching.backends.rediscache import RedisCache
-from redis import RedisError
-
-from landoapi.redis import SuppressRedisFailure
-from landoapi.systems import Subsystem
-
-# 60s * 60m * 24h
-DEFAULT_CACHE_KEY_TIMEOUT_SECONDS = 60 * 60 * 24
-
-logger = logging.getLogger(__name__)
-cache = Cache()
-cache.suppress_failure = SuppressRedisFailure
-
-
-class CacheSubsystem(Subsystem):
-    name = "cache"
-
-    def init_app(self, app):
-        super().init_app(app)
-        host = self.flask_app.config.get("CACHE_REDIS_HOST")
-        if self.flask_app.config.get("CACHE_DISABLED"):
-            # Default to not caching for testing.
-            logger.warning("Cache initialized in null mode.")
-            cache_config = {"CACHE_TYPE": "NullCache"}
-        elif not host:
-            logger.warning("Cache initialized in filesystem mode.")
-            cache_config = {"CACHE_TYPE": "FileSystemCache", "CACHE_DIR": "/tmp/cache"}
-        else:
-            cache_config = {"CACHE_TYPE": "redis", "CACHE_REDIS_HOST": host}
-            config_keys = ("CACHE_REDIS_PORT", "CACHE_REDIS_PASSWORD", "CACHE_REDIS_DB")
-            for k in config_keys:
-                v = self.flask_app.config.get(k)
-                if v is not None:
-                    cache_config[k] = v
-
-        cache.init_app(self.flask_app, config=cache_config)
-
-    def healthy(self) -> bool | str:
-        if not isinstance(cache.cache, RedisCache):
-            return "Cache is not configured to use redis"
-
-        # Dirty, but if this breaks in the future we can instead
-        # create our own redis-py client with its own connection
-        # pool.
-        redis = cache.cache._read_client
-
-        try:
-            redis.ping()
-        except RedisError as exc:
-            return "RedisError: {!s}".format(exc)
-
-        return True
-
-
-cache_subsystem = CacheSubsystem()
diff --git a/src/lando/settings.py b/src/lando/settings.py
index 55d02071..487d0c0b 100644
--- a/src/lando/settings.py
+++ b/src/lando/settings.py
@@ -86,6 +86,27 @@
     }
 }
 
+REDIS_CONFIG = {
+        "HOST": os.getenv("REDIS_HOST"),
+        "PORT": os.getenv("REDIS_PORT"),
+        "USER": os.getenv("REDIS_USER"),
+        "PASSWORD": os.getenv("REDIS_PASSWORD"),
+    }
+
+CACHES = {}
+
+if all(REDIS_CONFIG.values()):
+    CACHES["default"] = {
+        "BACKEND": "django.core.cache.backends.redis.RedisCache",
+        "LOCATION": [
+            "redis://{USER}:{PASSWORD}@{HOST}:{PORT}".format(**REDIS_CONFIG),
+        ],
+    }
+else:
+    CACHES["default"] = {
+        "BACKEND": "django.core.cache.backends.db.DatabaseCache",
+        "LOCATION": "lando_cache",
+    }
 
 # Password validation
 # https://docs.djangoproject.com/en/dev/ref/settings/#auth-password-validators
diff --git a/tests/conftest.py b/tests/conftest.py
index d999186c..0683d8a9 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -11,7 +11,6 @@
 
 import flask.testing
 import pytest
-import redis
 import requests
 import requests_mock
 import sqlalchemy
@@ -19,7 +18,6 @@
 from pytest_flask.plugin import JSONResponse
 
 from landoapi.app import SUBSYSTEMS, construct_app, load_config
-from landoapi.cache import cache
 from landoapi.mocks.auth import TEST_JWKS, MockAuth0
 from landoapi.phabricator import PhabricatorClient
 from landoapi.projects import (
@@ -159,9 +157,6 @@ def docker_env_vars(versionfile, monkeypatch):
     monkeypatch.setenv("BUGZILLA_URL", "asdfasdfasdfasdfasdfasdf")
     monkeypatch.setenv("OIDC_IDENTIFIER", "lando-api")
     monkeypatch.setenv("OIDC_DOMAIN", "lando-api.auth0.test")
-    # Explicitly shut off cache use for all tests.  Tests can re-enable the cache
-    # with the redis_cache fixture.
-    monkeypatch.delenv("CACHE_REDIS_HOST", raising=False)
     monkeypatch.delenv("CSP_REPORTING_URL", raising=False)
     # Don't suppress email in tests, but point at localhost so that any
     # real attempt would fail.
@@ -362,23 +357,6 @@ def get_client(api_key=None):
     return get_client
 
 
-@pytest.fixture
-def redis_cache(app):
-    cache.init_app(
-        app, config={"CACHE_TYPE": "redis", "CACHE_REDIS_HOST": "redis.cache"}
-    )
-    try:
-        cache.clear()
-    except redis.exceptions.ConnectionError:
-        if EXTERNAL_SERVICES_SHOULD_BE_PRESENT:
-            raise
-        else:
-            pytest.skip("Could not connect to Redis")
-    yield cache
-    cache.clear()
-    cache.init_app(app, config={"CACHE_TYPE": "null", "CACHE_NO_NULL_WARNING": True})
-
-
 @pytest.fixture
 def celery_app(app):
     """Configure our app's Celery instance for use with the celery_worker fixture."""
diff --git a/tests/test_dockerflow.py b/tests/test_dockerflow.py
index d3242037..aa7ca53b 100644
--- a/tests/test_dockerflow.py
+++ b/tests/test_dockerflow.py
@@ -23,13 +23,13 @@ def test_dockerflow_version_matches_disk_contents(client, versionfile):
 
 
 def test_heartbeat_returns_200(
-    client, db, phabdouble, request_mocker, redis_cache, jwks, treestatusdouble
+    client, db, phabdouble, request_mocker, jwks, treestatusdouble
 ):
     assert client.get("/__heartbeat__").status_code == 200
 
 
 def test_heartbeat_returns_http_502_if_phabricator_ping_returns_error(
-    client, request_mocker, redis_cache, jwks, treestatusdouble
+    client, request_mocker, jwks, treestatusdouble
 ):
     error_json = {
         "result": None,
diff --git a/tests/test_health.py b/tests/test_health.py
index cde7456f..b7a53f31 100644
--- a/tests/test_health.py
+++ b/tests/test_health.py
@@ -4,11 +4,9 @@
 
 from unittest.mock import Mock
 
-import redis
 from sqlalchemy.exc import SQLAlchemyError
 
 from landoapi.auth import auth0_subsystem
-from landoapi.cache import cache_subsystem
 from landoapi.phabricator import PhabricatorAPIException, phabricator_subsystem
 from landoapi.storage import db_subsystem
 
@@ -37,25 +35,6 @@ def raises(*args, **kwargs):
     assert phabricator_subsystem.healthy() is not True
 
 
-def test_cache_healthy(redis_cache):
-    assert cache_subsystem.healthy() is True
-
-
-def test_cache_unhealthy_configuration():
-    assert cache_subsystem.healthy() is not True
-
-
-def test_cache_unhealthy_service(redis_cache, monkeypatch):
-    mock_cache = Mock(redis_cache)
-    mock_cache.cache._read_client.ping.side_effect = redis.TimeoutError
-    monkeypatch.setattr("landoapi.cache.cache", mock_cache)
-    monkeypatch.setattr("landoapi.cache.RedisCache", type(mock_cache.cache))
-
-    health = cache_subsystem.healthy()
-    assert health is not True
-    assert health.startswith("RedisError:")
-
-
 def test_auth0_healthy(app, jwks):
     assert auth0_subsystem.healthy() is True