UN-4123 [FEAT] Support TLS to Redis, and keep pooled/idle connections healthy

backend/backend/settings/base.py

@@ -104,6 +104,19 @@ def get_required_setting(setting_key: str, default: str | None = None) -> str |
 REDIS_HOST = os.environ.get("REDIS_HOST", "localhost")
 REDIS_PORT = os.environ.get("REDIS_PORT", "6379")
 REDIS_DB = os.environ.get("REDIS_DB", "")
+# TLS to Redis (UN-4123). Off by default, so the in-cluster/local server is
+# untouched. `rediss://` is what actually selects TLS for both django-redis and
+# kombu; this flag only decides which scheme gets built.
+REDIS_SSL = os.environ.get("REDIS_SSL", "false").strip().lower() == "true"
+REDIS_SSL_CERT_REQS = os.environ.get("REDIS_SSL_CERT_REQS", "required")
+REDIS_SSL_CA_CERTS = os.environ.get("REDIS_SSL_CA_CERTS", "").strip()
+# A full URL overrides the discrete vars above, exactly as it does in
+# unstract.core's create_redis_client — otherwise the workers would follow the URL
+# while this process stayed on REDIS_HOST, and the two would silently sit on
+# DIFFERENT servers. That split is invisible until something written by one side
+# is read by the other: an API deployment returns `result: null` because the
+# execution's cached result was written to the URL's Redis and looked up here.
+REDIS_URL = os.environ.get("REDIS_URL", "").strip()
 SESSION_EXPIRATION_TIME_IN_SECOND = os.environ.get(
     "SESSION_EXPIRATION_TIME_IN_SECOND", 3600
 )
@@ -565,20 +578,65 @@ def filter(self, record):
         _cred_prefix = f"{quote(REDIS_USER, safe='')}:{quote(REDIS_PASSWORD, safe='')}@"
     elif REDIS_PASSWORD:
         _cred_prefix = f":{quote(REDIS_PASSWORD, safe='')}@"
-    SOCKET_IO_MANAGER_URL = f"redis://{_cred_prefix}{REDIS_HOST}:{REDIS_PORT}"
+    _scheme = "rediss" if REDIS_SSL else "redis"
+    _cache_db = int(REDIS_DB) if REDIS_DB else 0
+
+    # Both django-redis (via redis-py) and kombu read TLS settings out of the URL's
+    # query string, so one string configures both — verified against the pinned
+    # versions. The CA is appended rather than required in the URL, since it is a
+    # local path rather than part of the endpoint's identity.
+    _redis_url = REDIS_URL
+    if (
+        _redis_url
+        and REDIS_SSL_CA_CERTS
+        and _redis_url.startswith("rediss://")
+        and "ssl_ca_certs=" not in _redis_url
+    ):
+        _sep = "&" if "?" in _redis_url else "?"
+        _redis_url = (
+            f"{_redis_url}{_sep}ssl_ca_certs={quote(REDIS_SSL_CA_CERTS, safe='/')}"
+        )
+
+    # kombu reads TLS off the scheme, but defaults ssl_cert_reqs to CERT_NONE —
+    # encrypted while accepting ANY certificate, which is not what "TLS" is meant
+    # to buy. The query parameter is the only way to say otherwise here, since
+    # KombuManager takes a URL rather than connection kwargs.
+    _socketio_tls_query = f"?ssl_cert_reqs={REDIS_SSL_CERT_REQS}" if REDIS_SSL else ""
+    SOCKET_IO_MANAGER_URL = _redis_url or (
+        f"{_scheme}://{_cred_prefix}{REDIS_HOST}:{REDIS_PORT}{_socketio_tls_query}"
+    )
     SOCKET_IO_TRANSPORT_OPTIONS = {}
 
+    # django-redis 5.4.0 reads only PASSWORD (plus timeouts) out of OPTIONS — its
+    # ConnectionFactory.make_connection_params ignores USERNAME and DB entirely.
+    # So the db has to travel in the URL path, or this cache silently sits on db 0
+    # while every other service honours REDIS_DB: workers would RPUSH
+    # log_history_queue to db N and the backend would LPOP an empty db 0.
+    # USERNAME is deliberately NOT restored: auth is password-only as the built-in
+    # `default` user (what managed AUTH strings are), and sending a username turns
+    # AUTH into its two-argument ACL form.
+    _cache_options = {
+        "CLIENT_CLASS": "django_redis.client.DefaultClient",
+        "SERIALIZER": "django_redis.serializers.json.JSONSerializer",
+    }
+    if not _redis_url:
+        # Credentials and db travel IN the URL in URL mode; passing them again
+        # through OPTIONS risks one of them winning over the other.
+        _cache_options["DB"] = _cache_db
+        _cache_options["USERNAME"] = REDIS_USER
+        _cache_options["PASSWORD"] = REDIS_PASSWORD
+    if REDIS_SSL:
+        _pool_kwargs = {"ssl_cert_reqs": REDIS_SSL_CERT_REQS}
+        if REDIS_SSL_CA_CERTS:
+            _pool_kwargs["ssl_ca_certs"] = REDIS_SSL_CA_CERTS
+        _cache_options["CONNECTION_POOL_KWARGS"] = _pool_kwargs
+
     CACHES = {
         "default": {
             "BACKEND": "django_redis.cache.RedisCache",
-            "LOCATION": f"redis://{REDIS_HOST}:{REDIS_PORT}",
-            "OPTIONS": {
-                "CLIENT_CLASS": "django_redis.client.DefaultClient",
-                "SERIALIZER": "django_redis.serializers.json.JSONSerializer",
-                "DB": int(REDIS_DB) if REDIS_DB else 0,
-                "USERNAME": REDIS_USER,
-                "PASSWORD": REDIS_PASSWORD,
-            },
+            "LOCATION": _redis_url
+            or f"{_scheme}://{REDIS_HOST}:{REDIS_PORT}/{_cache_db}",
+            "OPTIONS": _cache_options,
             "KEY_FUNCTION": "utils.redis_cache.custom_key_function",
         }
     }

backend/backend/tests/test_redis_settings_derivation.py

@@ -0,0 +1,135 @@
+"""How the Redis settings block derives its URLs (UN-4123).
+
+The block lives inside ``settings/base.py`` and runs at import, so it cannot be
+called directly; these tests execute that exact source range against a controlled
+environment instead of re-implementing it, which would test a copy rather than
+the thing that ships.
+
+The case that matters is REDIS_URL. `create_redis_client` honoured it from the
+start, while this module still built its own URL from REDIS_HOST/REDIS_PORT — so
+the workers moved to the configured endpoint and the backend silently stayed
+behind. Nothing errors: the API deployment simply returns ``result: null``,
+because the execution's cached result was written to one Redis and looked up in
+the other. Found on a live run, not by reading the code.
+"""
+
+from __future__ import annotations
+
+import pathlib
+
+import pytest
+
+_SETTINGS = pathlib.Path(__file__).resolve().parents[1] / "settings" / "base.py"
+
+
+def _derive(**env: str) -> dict:
+    """Execute the standalone Redis block with the given env."""
+    source = _SETTINGS.read_text()
+    start = source.index("REDIS_SENTINEL_MODE = (")
+    end = source.index("SESSION_ENGINE =")
+
+    ns: dict = {}
+    prelude = (
+        "import os\n"
+        "from urllib.parse import quote\n"
+        "REDIS_USER = os.environ.get('REDIS_USER', 'default')\n"
+        "REDIS_PASSWORD = os.environ.get('REDIS_PASSWORD', '')\n"
+        "REDIS_HOST = os.environ.get('REDIS_HOST', 'localhost')\n"
+        "REDIS_PORT = os.environ.get('REDIS_PORT', '6379')\n"
+        "REDIS_DB = os.environ.get('REDIS_DB', '')\n"
+        "REDIS_SSL = os.environ.get('REDIS_SSL', 'false').strip().lower() == 'true'\n"
+        "REDIS_SSL_CERT_REQS = os.environ.get('REDIS_SSL_CERT_REQS', 'required')\n"
+        "REDIS_SSL_CA_CERTS = os.environ.get('REDIS_SSL_CA_CERTS', '').strip()\n"
+        "REDIS_URL = os.environ.get('REDIS_URL', '').strip()\n"
+    )
+    import os as _os
+
+    saved = {k: _os.environ.get(k) for k in list(_os.environ) if "REDIS" in k}
+    for key in saved:
+        _os.environ.pop(key, None)
+    _os.environ.update(env)
+    try:
+        exec(prelude + source[start:end], ns)  # noqa: S102 - the code under test
+    finally:
+        for key in list(_os.environ):
+            if "REDIS" in key:
+                _os.environ.pop(key, None)
+        _os.environ.update({k: v for k, v in saved.items() if v is not None})
+    return ns
+
+
+@pytest.fixture
+def plain() -> dict:
+    return _derive(REDIS_HOST="unstract-redis", REDIS_PORT="6379")
+
+
+class TestDiscreteVars:
+    def test_plaintext_is_unchanged(self, plain):
+        assert plain["CACHES"]["default"]["LOCATION"] == "redis://unstract-redis:6379/0"
+        assert plain["SOCKET_IO_MANAGER_URL"] == "redis://unstract-redis:6379"
+        assert "CONNECTION_POOL_KWARGS" not in plain["CACHES"]["default"]["OPTIONS"]
+
+    def test_db_travels_in_the_location(self):
+        """django-redis 5.4.0 ignores OPTIONS['DB'], so the path is the only route.
+
+        Without it this cache sits on db 0 while every other service honours
+        REDIS_DB — workers RPUSH log_history_queue to db N and the backend LPOPs
+        an empty db 0.
+        """
+        derived = _derive(REDIS_HOST="h", REDIS_DB="3")
+        assert derived["CACHES"]["default"]["LOCATION"].endswith("/3")
+
+    def test_ssl_switches_scheme_and_pool_kwargs(self):
+        derived = _derive(REDIS_HOST="h", REDIS_SSL="true", REDIS_SSL_CA_CERTS="/ca.pem")
+        cache = derived["CACHES"]["default"]
+        assert cache["LOCATION"].startswith("rediss://")
+        assert cache["OPTIONS"]["CONNECTION_POOL_KWARGS"] == {
+            "ssl_cert_reqs": "required",
+            "ssl_ca_certs": "/ca.pem",
+        }
+
+    def test_socketio_pins_certificate_verification(self):
+        """kombu defaults rediss:// to CERT_NONE — encrypted, but unauthenticated."""
+        derived = _derive(REDIS_HOST="h", REDIS_SSL="true")
+        assert "ssl_cert_reqs=required" in derived["SOCKET_IO_MANAGER_URL"]
+
+
+class TestUrlMode:
+    def test_url_drives_both_cache_and_socketio(self):
+        """The regression: these two used to ignore REDIS_URL entirely."""
+        url = "rediss://:pw@managed.example:6380/0?ssl_cert_reqs=required"
+        derived = _derive(REDIS_HOST="in-cluster", REDIS_URL=url)
+        assert derived["CACHES"]["default"]["LOCATION"].startswith(url)
+        assert derived["SOCKET_IO_MANAGER_URL"].startswith(url)
+        assert "in-cluster" not in derived["CACHES"]["default"]["LOCATION"]
+
+    def test_credentials_are_not_passed_twice(self):
+        """In URL mode the URL is the single source for db and credentials."""
+        derived = _derive(REDIS_URL="rediss://:pw@h:6380/2", REDIS_PASSWORD="other")
+        options = derived["CACHES"]["default"]["OPTIONS"]
+        assert "PASSWORD" not in options
+        assert "DB" not in options
+
+    def test_ca_is_appended_for_tls_urls(self):
+        derived = _derive(
+            REDIS_URL="rediss://h:6380/0?ssl_cert_reqs=required",
+            REDIS_SSL_CA_CERTS="/etc/ssl/redis-ca.pem",
+        )
+        assert "ssl_ca_certs=/etc/ssl/redis-ca.pem" in (
+            derived["CACHES"]["default"]["LOCATION"]
+        )
+
+    def test_ca_is_not_appended_to_a_plaintext_url(self):
+        derived = _derive(
+            REDIS_URL="redis://h:6379/0", REDIS_SSL_CA_CERTS="/etc/ssl/redis-ca.pem"
+        )
+        assert "ssl_ca_certs" not in derived["CACHES"]["default"]["LOCATION"]
+
+    def test_an_explicit_ca_in_the_url_wins(self):
+        derived = _derive(
+            REDIS_URL="rediss://h:6380/0?ssl_ca_certs=/in/url.pem",
+            REDIS_SSL_CA_CERTS="/env/ca.pem",
+        )
+        location = derived["CACHES"]["default"]["LOCATION"]
+        assert location.count("ssl_ca_certs") == 1
+        assert "/in/url.pem" in location

backend/sample.env

@@ -53,6 +53,33 @@ REDIS_RETRY_BACKOFF_FACTOR=0.5
 REDIS_SENTINEL_MODE=False
 REDIS_SENTINEL_MASTER_NAME=mymaster
 
+# Managed / external Redis with TLS (UN-4123). All optional — unset means the
+# plaintext connection this file otherwise describes, unchanged.
+#
+# Two ways to configure it, pick one:
+#   1. Discrete vars (what the Helm chart and these samples use). Set REDIS_SSL=true
+#      alongside REDIS_HOST/REDIS_PORT. No URL-encoding to get wrong.
+#   2. REDIS_URL, where the SCHEME carries TLS and nothing else is needed:
+#        REDIS_URL=rediss://:<password>@<host>:6380/0?ssl_cert_reqs=required
+#      A URL wins over the discrete vars above. Percent-encode @ / + in the password.
+#
+# Auth is password-only, as Redis's built-in `default` user — that is what a managed
+# AUTH string is. Leave REDIS_USER empty for a managed endpoint; named ACL users are
+# not supported platform-wide.
+#
+# REDIS_SSL_CA_CERTS is only needed when the server's CA is not in the system trust
+# store (Memorystore). ElastiCache and Azure Cache chain to public CAs.
+# REDIS_SSL=false
+# REDIS_SSL_CERT_REQS=required
+# REDIS_SSL_CA_CERTS=
+# REDIS_URL=
+
+# Proactive health check for pooled connections, in seconds (0 disables).
+# Managed Redis fails over during maintenance and reaps idle connections — Azure
+# Cache closes them at 10 minutes — so a parked connection can be dead before its
+# next command. Defaults to 30.
+# REDIS_HEALTH_CHECK_INTERVAL=30
+
 # Connector OAuth
 SOCIAL_AUTH_EXTRA_DATA_EXPIRATION_TIME_IN_SECOND=3600
 GOOGLE_OAUTH2_KEY=

docker/docker-compose-redis-tls.yaml

@@ -0,0 +1,69 @@
+# A stand-in for a managed Redis, for local development only (UN-4123).
+#
+# Managed offerings differ from the plain dev Redis in exactly two ways that our
+# code has to cope with: they require AUTH, and they speak TLS. This container
+# does both, so the TLS path can be exercised end to end — execution, log
+# streaming, tool sidecars — without cloud access or a VPN.
+#
+# It runs ALONGSIDE the normal `unstract-redis`, on a different port, so nothing
+# switches until you point services at it. That is deliberate: the point of the
+# exercise is to flip between the two and confirm BOTH work.
+#
+#   ./redis-tls/generate-certs.sh
+#   docker compose -f docker-compose-redis-tls.yaml up -d
+#
+# See redis-tls/README.md for the env to set and what to verify.
+services:
+  redis-managed:
+    image: "redis:7.2.3"
+    container_name: unstract-redis-managed
+    restart: unless-stopped
+    # `--port 0` disables the plaintext listener entirely, which is what Azure
+    # Cache does by default. A service that fails to pick up the TLS settings
+    # therefore cannot quietly succeed over plaintext and hide the bug.
+    #
+    # `--tls-auth-clients no` mirrors the managed services: the client verifies
+    # the server, not the other way round. No mTLS, no client certs to ship.
+    command: >
+      redis-server
+      --port 0
+      --tls-port 6380
+      --tls-cert-file /certs/server.crt
+      --tls-key-file /certs/server.key
+      --tls-ca-cert-file /certs/ca.crt
+      --tls-auth-clients no
+      --requirepass ${REDIS_DEV_PASSWORD:-devpassword}
+      --loglevel notice
+    ports:
+      # Same port inside and out, so one hostname:port works from containers
+      # (unstract-redis-managed:6380) and from a backend on the host
+      # (localhost:6380).
+      - "6380:6380"
+    volumes:
+      - ./redis-tls/certs:/certs:ro
+      - redis_managed_data:/data
+    healthcheck:
+      test:
+        [
+          "CMD",
+          "redis-cli",
+          "--tls",
+          "-p", "6380",
+          "--cacert", "/certs/ca.crt",
+          "-a", "${REDIS_DEV_PASSWORD:-devpassword}",
+          "--no-auth-warning",
+          "ping",
+        ]
+      interval: 10s
+      timeout: 5s
+      retries: 5
+    labels:
+      - traefik.enable=false
+
+volumes:
+  redis_managed_data:
+
+networks:
+  default:
+    name: unstract-network
+    external: true

docker/redis-tls/.gitignore

@@ -0,0 +1,2 @@
+# Dev-only self-signed material, regenerated by generate-certs.sh.
+certs/