From 1ce66dc407ac0ebbb02ddfc4908f86c743088585 Mon Sep 17 00:00:00 2001 From: Antoine Nguyen Date: Wed, 26 Mar 2025 13:22:39 +0100 Subject: [PATCH 1/5] Added support for Redis sentinel --- .github/workflows/modoboa.yml | 24 ++++++++++++++++-- docker-compose.yml | 32 ++++++++++++++++++++++++ docker/compose.redis-sentinel.yml | 25 +++++++++++++++++++ modoboa/policyd/core.py | 35 +++++++++++++++------------ modoboa/policyd/handlers.py | 13 +++++++--- test_project/envfile | 2 ++ test_project/test_project/settings.py | 9 +++++++ 7 files changed, 119 insertions(+), 21 deletions(-) create mode 100644 docker/compose.redis-sentinel.yml create mode 100644 test_project/envfile diff --git a/.github/workflows/modoboa.yml b/.github/workflows/modoboa.yml index 893468a62..02c84ab8e 100644 --- a/.github/workflows/modoboa.yml +++ b/.github/workflows/modoboa.yml @@ -108,8 +108,9 @@ jobs: run: | pip install -r mysql-requirements.txt echo "DB=mysql" >> $GITHUB_ENV - - name: Test with pytest + - name: Run tests if: ${{ matrix.python-version != '3.12' || matrix.database != 'postgres' }} + uses: hoverkraft-tech/compose-action@v2.0.1 run: | python ./tests.py cd test_project @@ -126,7 +127,26 @@ jobs: LDAP_SERVER_PORT: ${{ job.services.openldap.ports[389] }} OIDC_RSA_PRIVATE_KEY: ${{ vars.OIDC_RSA_PRIVATE_KEY }} - - name: Test with pytest and coverage + - name: Start redis sentinel + if: ${{ matrix.python-version == '3.12' || matrix.database == 'postgres' }} + uses: hoverkraft-tech/compose-action@v2.0.1 + with: + compose-file: "./docker/compose.redis-sentinel.yml" + + - name: Run tests againt redis sentinel + if: ${{ matrix.python-version == '3.12' || matrix.database == 'postgres' }} + run: | + cd test_project + coverage run manage.py test modoboa.policyd + env: + POSTGRES_PORT: ${{ job.services.postgres.ports[5432] }} + REDIS_SENTINEL: 1 + REDIS_SENTINEL_HOST: localhost + REDIS_SENTINEL_PORT: 26379 + REDIS_MASTER: mymaster + OIDC_RSA_PRIVATE_KEY: ${{ vars.OIDC_RSA_PRIVATE_KEY }} + + - name: Run tests and coverage if: ${{ matrix.python-version == '3.12' && matrix.database == 'postgres' }} run: | python ./tests.py diff --git a/docker-compose.yml b/docker-compose.yml index d26b288c5..2da91cf22 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -1,3 +1,7 @@ +networks: + redis-net: + external: true + services: redis: image: redis:5-alpine @@ -41,6 +45,9 @@ services: python3 manage.py runserver_plus 0.0.0.0:8000 --cert-file cert.crt" volumes: - .:/code + networks: + - default + - redis-net api-unsecured: container_name: modoboa_api_unsecured @@ -89,3 +96,28 @@ services: - api volumes: - ./docker/dovecot_config:/etc/dovecot/conf.d + + policyd: + container_name: modoboa_policyd + restart: unless-stopped + build: + context: . + dockerfile: docker/Dockerfile.dev + ports: + - 9999:9999 + env_file: + - test_project/oidc_key + environment: + DB: "SQLITE" + DEBUG: "True" + REDIS_HOST: redis + PYTHONPATH: ".." + PYTHONUNBUFFERED: 1 + command: > + sh -c "cd test_project + python3 manage.py policy_daemon --host 0.0.0.0 --port 9999 --debug" + volumes: + - .:/code + networks: + - default + - redis-net diff --git a/docker/compose.redis-sentinel.yml b/docker/compose.redis-sentinel.yml new file mode 100644 index 000000000..3fed7b41b --- /dev/null +++ b/docker/compose.redis-sentinel.yml @@ -0,0 +1,25 @@ +services: + redis-master: + image: redis:7-alpine + + redis-slave: + image: redis:7-alpine + command: redis-server --slaveof redis-master 6379 + depends_on: + - redis-master + + redis-sentinel: + image: redis:7-alpine + depends_on: + - redis-master + - redis-slave + ports: + - "26379:26379" + command: > + sh -c 'echo "bind 0.0.0.0" > /etc/sentinel.conf && + echo "sentinel monitor mymaster redis-master 6379 2" >> /etc/sentinel.conf && + echo "sentinel resolve-hostnames yes" >> /etc/sentinel.conf && + echo "sentinel down-after-milliseconds mymaster 10000" >> /etc/sentinel.conf && + echo "sentinel failover-timeout mymaster 10000" >> /etc/sentinel.conf && + echo "sentinel parallel-syncs mymaster 1" >> /etc/sentinel.conf && + redis-sentinel /etc/sentinel.conf' diff --git a/modoboa/policyd/core.py b/modoboa/policyd/core.py index 96a085bf0..cf68abec7 100644 --- a/modoboa/policyd/core.py +++ b/modoboa/policyd/core.py @@ -31,6 +31,15 @@ SUCCESS_ACTION = b"dunno" FAILURE_ACTION = b"defer_if_permit Daily limit reached, retry later" +def get_redis_client(): + if not getattr(settings, "REDIS_SENTINEL", False): + return aioredis.from_url( + settings.REDIS_URL, encoding="utf-8", decode_responses=True + ) + sentinel = aioredis.sentinel.Sentinel(settings.REDIS_SENTINELS, socket_timeout=0.1) + return sentinel.master_for(settings.REDIS_MASTER, socket_timeout=0.1) + + def close_db_connections(func, *args, **kwargs): """ Make sure to close all connections to DB. @@ -144,9 +153,7 @@ async def apply_policies(attributes): sasl_username = attributes.get("sasl_username") if not sasl_username: return SUCCESS_ACTION - rclient = aioredis.from_url( - settings.REDIS_URL, encoding="utf-8", decode_responses=True - ) + rclient = get_redis_client() decr_domain = False decr_user = False localpart, domain = split_mailbox(sasl_username) @@ -200,16 +207,14 @@ async def handle_connection(reader, writer): async def new_connection(reader, writer): - try: - await asyncio.wait_for(handle_connection(reader, writer), timeout=5) - except asyncio.TimeoutError as err: - logger.warning("Timeout received while handling connection: %s", err) - finally: - writer.close() - if hasattr(writer, "wait_closed"): - # Python 3.7+ only - await writer.wait_closed() - logger.info("exit") + # try: + await asyncio.wait_for(handle_connection(reader, writer), timeout=5) + # except asyncio.TimeoutError as err: + # logger.warning(f"Timeout received while handling connection: {err}") + # finally: + writer.close() + await writer.wait_closed() + logger.info("exit") def get_next_execution_dt(): @@ -255,9 +260,7 @@ def get_mailboxes_to_reset(): async def reset_counters(): """Reset all counters.""" - rclient = aioredis.from_url( - settings.REDIS_URL, encoding="utf-8", decode_responses=True - ) + rclient = get_redis_client() logger.info("Resetting all counters") for domain in await get_domains_to_reset(): await rclient.hset(constants.REDIS_HASHNAME, domain.name, domain.message_limit) diff --git a/modoboa/policyd/handlers.py b/modoboa/policyd/handlers.py index d4e05465f..78cb8ef48 100644 --- a/modoboa/policyd/handlers.py +++ b/modoboa/policyd/handlers.py @@ -16,9 +16,16 @@ def set_message_limit(instance, key): old_message_limit = instance._loaded_values.get("message_limit") if old_message_limit == instance.message_limit: return - rclient = redis.Redis( - host=settings.REDIS_HOST, port=settings.REDIS_PORT, db=settings.REDIS_QUOTA_DB - ) + if not getattr(settings, "REDIS_SENTINEL", False): + rclient = redis.Redis( + host=settings.REDIS_HOST, + port=settings.REDIS_PORT, + db=settings.REDIS_QUOTA_DB, + ) + else: + sentinel = redis.sentinel.Sentinel(settings.REDIS_SENTINELS, socket_timeout=0.1) + rclient = sentinel.master_for(settings.REDIS_MASTER, socket_timeout=0.1) + if instance.message_limit is None: # delete existing key if rclient.hexists(constants.REDIS_HASHNAME, key): diff --git a/test_project/envfile b/test_project/envfile new file mode 100644 index 000000000..915208d47 --- /dev/null +++ b/test_project/envfile @@ -0,0 +1,2 @@ +WEBMAIL_DEV_USERNAME=tonio@ngyn.org +WEBMAIL_DEV_PASSWORD=Aka/12ctogael diff --git a/test_project/test_project/settings.py b/test_project/test_project/settings.py index 428529887..385d5c96b 100644 --- a/test_project/test_project/settings.py +++ b/test_project/test_project/settings.py @@ -256,6 +256,15 @@ PID_FILE_STORAGE_PATH = "/tmp" # REDIS +REDIS_SENTINEL = bool(os.environ.get("REDIS_SENTINEL", False)) +REDIS_SENTINELS = [ + ( + os.environ.get("REDIS_SENTINEL_HOST", "127.0.0.1"), + os.environ.get("REDIS_SENTINEL_PORT", 26379), + ) +] +REDIS_MASTER = os.environ.get("REDIS_MASTER", "mymaster") + REDIS_HOST = os.environ.get("REDIS_HOST", "127.0.0.1") REDIS_PORT = os.environ.get("REDIS_PORT", 6379) REDIS_QUOTA_DB = 0 From 19cf039df27142498db4c6168cfa46e6980faa5c Mon Sep 17 00:00:00 2001 From: Antoine Nguyen Date: Wed, 26 Mar 2025 13:26:40 +0100 Subject: [PATCH 2/5] Fixed wrong workflow config --- .github/workflows/modoboa.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/modoboa.yml b/.github/workflows/modoboa.yml index 02c84ab8e..17a9587e0 100644 --- a/.github/workflows/modoboa.yml +++ b/.github/workflows/modoboa.yml @@ -110,7 +110,6 @@ jobs: echo "DB=mysql" >> $GITHUB_ENV - name: Run tests if: ${{ matrix.python-version != '3.12' || matrix.database != 'postgres' }} - uses: hoverkraft-tech/compose-action@v2.0.1 run: | python ./tests.py cd test_project From d4774708a17ecfdc9598d8def1855071dbeba225 Mon Sep 17 00:00:00 2001 From: Antoine Nguyen Date: Wed, 26 Mar 2025 13:29:29 +0100 Subject: [PATCH 3/5] Run tests in 2.3.x maintenance branch --- .github/workflows/modoboa.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/modoboa.yml b/.github/workflows/modoboa.yml index 17a9587e0..b8c5c1bf2 100644 --- a/.github/workflows/modoboa.yml +++ b/.github/workflows/modoboa.yml @@ -5,7 +5,7 @@ name: Modoboa App on: push: - branches: [ master ] + branches: [ master, 2.3.x ] paths-ignore: - 'modoboa/**.po' - 'frontend/**' From 5f4ad848eda1d5dba20eea67ca9439d6201f2839 Mon Sep 17 00:00:00 2001 From: Antoine Nguyen Date: Wed, 26 Mar 2025 13:56:00 +0100 Subject: [PATCH 4/5] Fixed wrong condition --- .github/workflows/modoboa.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/modoboa.yml b/.github/workflows/modoboa.yml index b8c5c1bf2..ca35a4c41 100644 --- a/.github/workflows/modoboa.yml +++ b/.github/workflows/modoboa.yml @@ -127,13 +127,13 @@ jobs: OIDC_RSA_PRIVATE_KEY: ${{ vars.OIDC_RSA_PRIVATE_KEY }} - name: Start redis sentinel - if: ${{ matrix.python-version == '3.12' || matrix.database == 'postgres' }} + if: ${{ matrix.python-version == '3.12' && matrix.database == 'postgres' }} uses: hoverkraft-tech/compose-action@v2.0.1 with: compose-file: "./docker/compose.redis-sentinel.yml" - name: Run tests againt redis sentinel - if: ${{ matrix.python-version == '3.12' || matrix.database == 'postgres' }} + if: ${{ matrix.python-version == '3.12' && matrix.database == 'postgres' }} run: | cd test_project coverage run manage.py test modoboa.policyd From b434958a79cd85beaf18be9492e36eed1bec0673 Mon Sep 17 00:00:00 2001 From: Antoine Nguyen Date: Wed, 26 Mar 2025 14:24:48 +0100 Subject: [PATCH 5/5] Make sure to use the right client everywhere --- modoboa/policyd/handlers.py | 15 ++------------- modoboa/policyd/tests.py | 8 ++------ modoboa/policyd/utils.py | 12 +++++++++--- 3 files changed, 13 insertions(+), 22 deletions(-) diff --git a/modoboa/policyd/handlers.py b/modoboa/policyd/handlers.py index 78cb8ef48..04d8e7e1c 100644 --- a/modoboa/policyd/handlers.py +++ b/modoboa/policyd/handlers.py @@ -1,14 +1,12 @@ """App related signal handlers.""" -import redis - -from django.conf import settings from django.db.models import signals from django.dispatch import receiver from modoboa.admin import models as admin_models from . import constants +from . import utils def set_message_limit(instance, key): @@ -16,16 +14,7 @@ def set_message_limit(instance, key): old_message_limit = instance._loaded_values.get("message_limit") if old_message_limit == instance.message_limit: return - if not getattr(settings, "REDIS_SENTINEL", False): - rclient = redis.Redis( - host=settings.REDIS_HOST, - port=settings.REDIS_PORT, - db=settings.REDIS_QUOTA_DB, - ) - else: - sentinel = redis.sentinel.Sentinel(settings.REDIS_SENTINELS, socket_timeout=0.1) - rclient = sentinel.master_for(settings.REDIS_MASTER, socket_timeout=0.1) - + rclient = utils.get_redis_connection() if instance.message_limit is None: # delete existing key if rclient.hexists(constants.REDIS_HASHNAME, key): diff --git a/modoboa/policyd/tests.py b/modoboa/policyd/tests.py index 000e541f3..0776b9e91 100644 --- a/modoboa/policyd/tests.py +++ b/modoboa/policyd/tests.py @@ -22,6 +22,7 @@ from modoboa.lib.tests import ModoTestCase, ParametersMixin from modoboa.policyd import core as policyd_core from . import constants +from . import utils def start_policy_daemon(): @@ -33,12 +34,7 @@ class RedisTestCaseMixin: def setUp(self): super().setUp() - self.rclient = redis.Redis( - host=settings.REDIS_HOST, - port=settings.REDIS_PORT, - db=settings.REDIS_QUOTA_DB, - ) - self.rclient.set_response_callback("HGET", int) + self.rclient = utils.get_redis_connection() self.rclient.delete(constants.REDIS_HASHNAME) diff --git a/modoboa/policyd/utils.py b/modoboa/policyd/utils.py index ca4b88c17..5c2a06b1b 100644 --- a/modoboa/policyd/utils.py +++ b/modoboa/policyd/utils.py @@ -9,9 +9,15 @@ from . import constants def get_redis_connection(): """Return a client connection to Redis server.""" - rclient = redis.Redis( - host=settings.REDIS_HOST, port=settings.REDIS_PORT, db=settings.REDIS_QUOTA_DB - ) + if not getattr(settings, "REDIS_SENTINEL", False): + rclient = redis.Redis( + host=settings.REDIS_HOST, + port=settings.REDIS_PORT, + db=settings.REDIS_QUOTA_DB, + ) + else: + sentinel = redis.sentinel.Sentinel(settings.REDIS_SENTINELS, socket_timeout=0.1) + rclient = sentinel.master_for(settings.REDIS_MASTER, socket_timeout=0.1) rclient.set_response_callback("HGET", int) return rclient