fix(web-app-suitecrm): keep sessions in Redis so replicas share them

With three swarm replicas the PHP session files on the shared NFS volume
were read as empty under concurrent requests of one browser, so SuiteCRM
answered 403 on /api/graphql and dropped the session. Sessions now live in
Redis with session locking that waits like flock did; the phpredis version
is pinned in services.yml with its own update source, and the engine lookup
renders the save path.

Refs #879

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

plugins/lookup/engine.py

@@ -2,6 +2,7 @@
 
     {{ lookup('engine', 'redis', application_id) }}            # full dict
     {{ lookup('engine', 'redis', application_id, 'url') }}     # one field
+    {{ lookup('engine', 'redis', application_id, 'session_path') }}  # phpredis session.save_path
 
 Mirrors the ``database`` lookup but for the non-RDBMS engines registered in
 :data:`utils.roles.applications.services.engines.ENGINES` (redis, memcached,
@@ -16,6 +17,7 @@ See ``docs/architecture/central-engines.md``.
 from __future__ import annotations
 
 from typing import Any
+from urllib.parse import quote
 
 from ansible.errors import AnsibleError
 from ansible.plugins.loader import lookup_loader
@@ -44,9 +46,31 @@ _FIELDS = (
     "prefix",
     "url",
     "container",
+    "session_path",
 )
 
 
+def redis_session_path(
+    host: str, port: int, prefix: str, username: str, password: str
+) -> str:
+    """Return the phpredis ``session.save_path`` of one consumer.
+
+    Args:
+        host: redis host the consumer reaches.
+        port: redis port.
+        prefix: consumer prefix; session keys become ``<prefix>:session:<id>``.
+        username: ACL user, sent only together with a password.
+        password: redis password; empty for an embedded sidecar.
+    """
+    path = f"tcp://{host}:{port}?prefix={quote(prefix, safe='')}:session:"
+    if password:
+        path += (
+            f"&auth[user]={quote(username, safe='')}"
+            f"&auth[pass]={quote(password, safe='')}"
+        )
+    return path
+
+
 def qdrant_consumer_key(applications: dict[str, Any], svc_id: str) -> str:
     """Return the engine-wide key a data-owning qdrant consumer presents.
 
@@ -197,6 +221,17 @@ class LookupModule(LookupBase):
             "prefix": prefix,
             "url": url,
             "container": container,
+            "session_path": (
+                redis_session_path(
+                    host,
+                    port,
+                    prefix,
+                    username if shared else "",
+                    password if shared else "",
+                )
+                if engine == "redis"
+                else ""
+            ),
         }
         if want == "all":
             return [resolved]

roles/web-app-suitecrm/files/Dockerfile

@@ -5,6 +5,7 @@ FROM ${SUITECRM_BASE_IMAGE}:${SUITECRM_BASE_VERSION} AS base
 
 ARG SUITECRM_APP_DIR
 ARG SUITECRM_PHP_MEMORY_LIMIT
+ARG SUITECRM_PHPREDIS_VERSION
 
 RUN rm -rf /var/lib/apt/lists/* \
   && apt-get -o Acquire::Retries=3 update && apt-get -o Acquire::Retries=3 install -y \
@@ -32,6 +33,10 @@ RUN rm -rf /var/lib/apt/lists/* \
       ldap \
   && rm -rf /var/lib/apt/lists/*
 
+RUN : "${SUITECRM_PHPREDIS_VERSION:?}" \
+  && pecl install "redis-${SUITECRM_PHPREDIS_VERSION}" \
+  && docker-php-ext-enable redis
+
 RUN : "${SUITECRM_PHP_MEMORY_LIMIT:?}" && { \
       echo 'file_uploads = On'; \
       echo 'upload_max_filesize = 32M'; \

roles/web-app-suitecrm/meta/services.yml

@@ -29,6 +29,16 @@ mariadb:
   bond: 1
   enabled: true # nocheck: dynamic-flag
   shared: "{{ 'svc-db-mariadb' in group_names }}"
+redis:
+  bond: 1
+  backup:
+    disabled: true
+  enabled: true # nocheck: dynamic-flag
+  shared: "{{ 'svc-db-redis' in group_names }}"
+  cpus: "0.25"
+  mem_reservation: 128m
+  mem_limit: 256m
+  pids_limit: 256
 # nocheck: playwright-service-flag — self-provider entry; test_env_services_match.py:28-32 grants the forbidden-self-gate exemption
 suitecrm:
   healthcheck:
@@ -49,12 +59,16 @@ suitecrm:
   version: 8.2-apache
   custom: true
   app_version: 8.9.1
+  phpredis_version: 6.3.0
   update:
-    key: app_version
-    type: git_tags
-    repository: https://github.com/salesagility/SuiteCRM-Core.git
-    match: "v"
-    strip: true
+    - key: app_version
+      type: git_tags
+      repository: https://github.com/salesagility/SuiteCRM-Core.git
+      match: "v"
+      strip: true
+    - key: phpredis_version
+      type: git_tags
+      repository: https://github.com/phpredis/phpredis.git
   name: suitecrm
   min_storage: 376MB
   cpus: 1.5

roles/web-app-suitecrm/meta/variants.yml

@@ -24,6 +24,8 @@
       shared: true
     mariadb:
       shared: true
+    redis:
+      shared: true
     matomo:
       enabled: true
       shared: true
@@ -56,6 +58,8 @@
       shared: false # nocheck: variants-static-override backup shared stays literally true
     mariadb:
       shared: false # nocheck: variants-static-override backup shared stays literally true
+    redis:
+      shared: false
     matomo:
       enabled: false
       shared: false # nocheck: variants-static-override backup shared stays literally true

roles/web-app-suitecrm/templates/compose.yml.j2

@@ -6,6 +6,7 @@
         SUITECRM_BASE_IMAGE: "{{ SUITECRM_BASE_IMAGE }}"
         SUITECRM_BASE_VERSION: "{{ SUITECRM_BASE_VERSION }}"
         SUITECRM_APP_VERSION: "{{ SUITECRM_APP_VERSION }}"
+        SUITECRM_PHPREDIS_VERSION: "{{ SUITECRM_PHPREDIS_VERSION }}"
         SUITECRM_ENTRYPOINT_SCRIPT_HOST_REL: "{{ SUITECRM_ENTRYPOINT_SCRIPT_HOST_REL }}"
         SUITECRM_ENTRYPOINT_SCRIPT_DOCKER: "{{ SUITECRM_ENTRYPOINT_SCRIPT_DOCKER }}"
         SUITECRM_LDAP_EXTENSION_DIR: "{{ SUITECRM_LDAP_EXTENSION_DIR }}"

roles/web-app-suitecrm/templates/env.j2

@@ -21,6 +21,8 @@ SUITECRM_ADMIN_EMAIL={{ lookup('users', 'administrator').email }}
 
 SUITECRM_URL={{ SUITECRM_URL }}
 
+SUITECRM_SESSION_SAVE_PATH={{ lookup('engine', 'redis', application_id, 'session_path') }}
+
 {% if SUITECRM_SSO_ENABLED | bool %}
 AUTH_TYPE=saml
 

roles/web-app-suitecrm/templates/php.ini.j2

@@ -3,5 +3,7 @@ display_startup_errors = Off
 log_errors = On
 error_reporting = E_ALL & ~E_DEPRECATED & ~E_USER_DEPRECATED
 
-session.save_handler = files
-session.save_path = "{{ SUITECRM_APP_DIR }}/tmp"
+session.save_handler = redis
+session.save_path = ${SUITECRM_SESSION_SAVE_PATH}
+redis.session.locking_enabled = 1
+redis.session.lock_retries = -1

roles/web-app-suitecrm/vars/main.yml

@@ -11,6 +11,7 @@ SUITECRM_PHP_CONFIG_HOST: "{{ [ lookup('container', application_id, 'directories
 SUITECRM_BASE_IMAGE: "{{ lookup('config', application_id, 'services.' ~ entity_name ~ '.image') }}"
 SUITECRM_BASE_VERSION: "{{ lookup('config', application_id, 'services.' ~ entity_name ~ '.version') }}"
 SUITECRM_APP_VERSION: "{{ lookup('config', application_id, 'services.' ~ entity_name ~ '.app_version') }}"
+SUITECRM_PHPREDIS_VERSION: "{{ lookup('config', application_id, 'services.' ~ entity_name ~ '.phpredis_version') }}"
 
 SUITECRM_CONTAINER: "{{ lookup('config', application_id, 'services.' ~ entity_name ~ '.name') }}"
 SUITECRM_CONTAINER_ADDRESS: "{{ lookup('container_address', application_id, entity_name) }}"

tests/unit/python/plugins/lookup/test_engine_redis_session_path.py

@@ -0,0 +1,49 @@
+"""phpredis reads ``auth[user]``/``auth[pass]`` and ``prefix`` from the query of
+``session.save_path``; a password with a reserved character that is not
+percent-encoded splits the query and the handler authenticates with a fragment.
+"""
+
+from __future__ import annotations
+
+import importlib
+import unittest
+from urllib.parse import parse_qs, urlsplit
+
+plugin_module = importlib.import_module("plugins.lookup.engine")
+redis_session_path = plugin_module.redis_session_path
+
+
+class TestRedisSessionPath(unittest.TestCase):
+    def test_an_embedded_sidecar_gets_no_credentials(self) -> None:
+        self.assertEqual(
+            "tcp://redis:6379?prefix=suitecrm:session:",
+            redis_session_path("redis", 6379, "suitecrm", "", ""),
+        )
+
+    def test_a_shared_redis_gets_user_and_password(self) -> None:
+        query = parse_qs(
+            urlsplit(
+                redis_session_path("redis-central", 6379, "suitecrm", "default", "pw")
+            ).query
+        )
+        self.assertEqual(["default"], query["auth[user]"])
+        self.assertEqual(["pw"], query["auth[pass]"])
+
+    def test_a_password_with_reserved_characters_survives_the_query(self) -> None:
+        password = "a&b=c?d#e/f+g h"
+        query = parse_qs(
+            urlsplit(
+                redis_session_path("redis-central", 6379, "suitecrm", "default", password)
+            ).query
+        )
+        self.assertEqual([password], query["auth[pass]"])
+        self.assertEqual(["suitecrm:session:"], query["prefix"])
+
+    def test_a_user_without_a_password_is_not_sent(self) -> None:
+        self.assertNotIn(
+            "auth", redis_session_path("redis", 6379, "suitecrm", "default", "")
+        )
+
+
+if __name__ == "__main__":
+    unittest.main()