diff --git a/plugins/lookup/engine.py b/plugins/lookup/engine.py index 5c769cc538..c428c9acdf 100644 --- a/plugins/lookup/engine.py +++ b/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 ``:session:``. + 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] diff --git a/roles/web-app-suitecrm/files/Dockerfile b/roles/web-app-suitecrm/files/Dockerfile index 681672774b..08c8f95e71 100644 --- a/roles/web-app-suitecrm/files/Dockerfile +++ b/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'; \ diff --git a/roles/web-app-suitecrm/meta/services.yml b/roles/web-app-suitecrm/meta/services.yml index 68c73b211d..cc7a861a50 100644 --- a/roles/web-app-suitecrm/meta/services.yml +++ b/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 diff --git a/roles/web-app-suitecrm/meta/variants.yml b/roles/web-app-suitecrm/meta/variants.yml index 6353170e71..23ed53e8a9 100644 --- a/roles/web-app-suitecrm/meta/variants.yml +++ b/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 diff --git a/roles/web-app-suitecrm/templates/compose.yml.j2 b/roles/web-app-suitecrm/templates/compose.yml.j2 index 62057023e5..a4a9f433e8 100644 --- a/roles/web-app-suitecrm/templates/compose.yml.j2 +++ b/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 }}" diff --git a/roles/web-app-suitecrm/templates/env.j2 b/roles/web-app-suitecrm/templates/env.j2 index 826cb31386..acc107ad15 100644 --- a/roles/web-app-suitecrm/templates/env.j2 +++ b/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 diff --git a/roles/web-app-suitecrm/templates/php.ini.j2 b/roles/web-app-suitecrm/templates/php.ini.j2 index e818dbc83e..1ff9b031e4 100644 --- a/roles/web-app-suitecrm/templates/php.ini.j2 +++ b/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 diff --git a/roles/web-app-suitecrm/vars/main.yml b/roles/web-app-suitecrm/vars/main.yml index 2b1708dfdd..aaf9602ea6 100644 --- a/roles/web-app-suitecrm/vars/main.yml +++ b/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) }}" diff --git a/tests/unit/python/plugins/lookup/test_engine_redis_session_path.py b/tests/unit/python/plugins/lookup/test_engine_redis_session_path.py new file mode 100644 index 0000000000..0b8dba098e --- /dev/null +++ b/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()