From e18fb73fb68b51032ccf557b56f86001e798d418 Mon Sep 17 00:00:00 2001 From: Andrea Dell'Amico Date: Thu, 10 Sep 2026 18:34:18 +0200 Subject: [PATCH] See #4577. Backports some hyperkitty fixes. --- README.md | 48 +++- defaults/main.yml | 13 +- handlers/main.yml | 4 + tasks/mailman.yml | 1 + tasks/postorius-hyperkitty.yml | 44 +++- templates/hyperkitty-archive-cleanup.py.j2 | 233 ++++++++++++++++++ ...erkitty-archive-cleanup.service.systemd.j2 | 15 ++ ...yperkitty-archive-cleanup.timer.systemd.j2 | 11 + ...lman-legacy-hyperkitty-compatibility.py.j2 | 57 ++++- 9 files changed, 419 insertions(+), 7 deletions(-) create mode 100644 templates/hyperkitty-archive-cleanup.py.j2 create mode 100644 templates/hyperkitty-archive-cleanup.service.systemd.j2 create mode 100644 templates/hyperkitty-archive-cleanup.timer.systemd.j2 diff --git a/README.md b/README.md index 7008b9d..8e51d26 100644 --- a/README.md +++ b/README.md @@ -54,7 +54,7 @@ the listener does not become available after starting or restarting Mailman. ## Legacy HyperKitty compatibility fixes -The optional compatibility patcher addresses three failures observed with the +The optional compatibility patcher addresses failures observed with the legacy Python 3.6 web stack: malformed `From`/`Subject` headers, nested `message/rfc822` parts without `Content-Transfer-Encoding`, and unreadable gettext catalogs. It also normalizes non-ASCII sender addresses and backports @@ -64,6 +64,9 @@ Named MIME parts whose content type is missing or empty are extracted as binary attachments instead of being appended to the plain-text message body. As an additional safeguard, the NUL filtering introduced by django-mailman3 1.3.6 is backported for scrubbed text fields only; binary attachments remain unchanged. +It also backports the HyperKitty 1.3.5 guards for all four asynchronous cache +tasks that may still reference a mailing list after its archive has been +deleted. Those obsolete tasks are logged and skipped instead of being retried. Enable it only for the known package versions: ```yaml @@ -78,10 +81,47 @@ and installed atomically. The task fails without changing any source file if the installed versions or expected source fragments do not match. Invalid `django_extensions` `.mo` files are preserved beside the original name with a `.disabled-by-ansible` suffix, allowing Django to fall back to another locale. -The uWSGI and Mailman services are restarted only when a source file or catalog -actually changes. After restarting Mailman, the handler requires the configured -LMTP listener to become available. +The standalone HyperKitty qcluster, uWSGI and Mailman services are restarted +only when a source file or catalog actually changes. Restarting qcluster is +required because its workers otherwise retain the old Python module in memory. +After restarting Mailman, the handler requires the configured LMTP listener to +become available. Disable this option before upgrading HyperKitty, django-mailman3, or mailman-hyperkitty, then review whether the compatibility patches are still needed with the new versions. + +## HyperKitty archive cleanup + +Mailman Core does not delete HyperKitty data when a list is removed, and an +`archive_policy` of `never` only prevents future archiving. The optional +cleanup timer removes the corresponding HyperKitty `MailingList`, threads and +messages in both cases: + +```yaml +mailman_enable_hyperkitty_archive_cleanup: true +mailman_hyperkitty_archive_cleanup_on_calendar: 'Sun *-*-* 03:00:00' +mailman_hyperkitty_archive_cleanup_deleted_lists: true +mailman_hyperkitty_archive_cleanup_disabled_lists: true +mailman_hyperkitty_archive_cleanup_max_deletions: 50 +``` + +Deletion is irreversible. Before changing the database, the cleanup script +retrieves a complete list from Mailman Core and rechecks each candidate. It +aborts if Core is unavailable, if Core returns no lists, or if the configured +maximum number of deletions would be exceeded. HyperKitty deletion signals +that would enqueue cache rebuilds for the objects being removed are disabled +only inside the cleanup process. This avoids the known failure in legacy +HyperKitty releases where those asynchronous jobs try to load a list after it +has been deleted. + +The timer is not persistent, so a missed run is not started after boot. A +read-only preview can be run before the first scheduled execution: + +```bash +sudo -u mailman /usr/local/sbin/hyperkitty-archive-cleanup --dry-run +``` + +The monthly HyperKitty job already configured by this role removes stale +entries from the full-text search index. Therefore the weekly cleanup does not +run an additional, expensive full index scan. diff --git a/defaults/main.yml b/defaults/main.yml index e7d9235..62b94db 100644 --- a/defaults/main.yml +++ b/defaults/main.yml @@ -52,7 +52,18 @@ mailman_legacy_django_mailman3_version: '1.3.4' mailman_legacy_mailman_hyperkitty_version: '1.1.0' mailman_legacy_hyperkitty_patch_script: '/usr/local/sbin/mailman-legacy-hyperkitty-compatibility' - # Documentation that must be followed to configure the social auth providers +# Remove HyperKitty archives whose list no longer exists in Mailman Core, or +# whose archive policy has been changed to "never". This is deliberately +# opt-in because archive deletion is irreversible. +mailman_enable_hyperkitty_archive_cleanup: false +mailman_hyperkitty_archive_cleanup_on_calendar: 'Sun *-*-* 03:00:00' +mailman_hyperkitty_archive_cleanup_deleted_lists: true +mailman_hyperkitty_archive_cleanup_disabled_lists: true +mailman_hyperkitty_archive_cleanup_max_deletions: 50 +mailman_hyperkitty_archive_cleanup_script: '/usr/local/sbin/hyperkitty-archive-cleanup' +mailman_hyperkitty_archive_cleanup_lock: '{{ mailman_lock_dir }}/hyperkitty-archive-cleanup.lock' + +# Documentation that must be followed to configure the social auth providers # https://django-allauth.readthedocs.io/en/latest/installation.html mailman_use_social_account_providers: False mailman_postorius_social_auth_providers: diff --git a/handlers/main.yml b/handlers/main.yml index 16f0372..61b3596 100644 --- a/handlers/main.yml +++ b/handlers/main.yml @@ -1,4 +1,8 @@ --- +- name: Restart hyperkitty qcluster + service: name=hyperkitty-qcluster state=restarted + when: mailman_hyperkitty_qcluster_standalone_service | bool + - name: Restart mailmansuite service: name={{ mailman_postorius_uwsgi_servicename }} state=restarted diff --git a/tasks/mailman.yml b/tasks/mailman.yml index ba28cbe..259967b 100644 --- a/tasks/mailman.yml +++ b/tasks/mailman.yml @@ -73,6 +73,7 @@ - mailman_enable_legacy_hyperkitty_compatibility_patches | bool - not ansible_check_mode notify: + - Restart hyperkitty qcluster - Restart mailmansuite - Restart mailman tags: [ 'mailman', 'mailman_conf', 'mailman_hyperkitty_compatibility' ] diff --git a/tasks/postorius-hyperkitty.yml b/tasks/postorius-hyperkitty.yml index c14146b..b9e7b10 100644 --- a/tasks/postorius-hyperkitty.yml +++ b/tasks/postorius-hyperkitty.yml @@ -98,6 +98,49 @@ when: not mailman_hyperkitty_qcluster_standalone_service tags: [ 'mailman', 'postorius', 'hyperkitty', 'mailman_conf', 'hyperkitty_async' ] +- name: Configure automatic cleanup of obsolete HyperKitty archives + block: + - name: Install the HyperKitty archive cleanup script + ansible.builtin.template: + src: hyperkitty-archive-cleanup.py.j2 + dest: '{{ mailman_hyperkitty_archive_cleanup_script }}' + owner: root + group: '{{ mailman_user }}' + mode: '0750' + + - name: Install the HyperKitty archive cleanup service + ansible.builtin.template: + src: hyperkitty-archive-cleanup.service.systemd.j2 + dest: /etc/systemd/system/hyperkitty-archive-cleanup.service + owner: root + group: root + mode: '0644' + register: mailman_hyperkitty_archive_cleanup_service_install + + - name: Install the HyperKitty archive cleanup timer + ansible.builtin.template: + src: hyperkitty-archive-cleanup.timer.systemd.j2 + dest: /etc/systemd/system/hyperkitty-archive-cleanup.timer + owner: root + group: root + mode: '0644' + register: mailman_hyperkitty_archive_cleanup_timer_install + + - name: Reload systemd after installing the HyperKitty cleanup units + ansible.builtin.systemd: + daemon_reload: true + when: >- + mailman_hyperkitty_archive_cleanup_service_install is changed or + mailman_hyperkitty_archive_cleanup_timer_install is changed + + - name: Set the HyperKitty archive cleanup timer state + ansible.builtin.systemd: + name: hyperkitty-archive-cleanup.timer + state: "{{ mailman_enable_hyperkitty_archive_cleanup | bool | ternary('started', 'stopped') }}" + enabled: '{{ mailman_enable_hyperkitty_archive_cleanup | bool }}' + + tags: [ 'mailman', 'postorius', 'hyperkitty', 'mailman_conf', 'hyperkitty_archive_cleanup' ] + - name: Setup the postorius cron jobs block: - name: add a cron job that syncs the mailman core and postorius settings @@ -138,4 +181,3 @@ - name: Install a script that should be run after an update of the mailman suite template: src=mailman-postupdate.sh.j2 dest={{ mailman_bindir }}/mailman-postupdate owner={{ mailman_user }} group={{ mailman_user }} mode='0750' tags: [ 'mailman', 'postorius', 'hyperkitty', 'mailman_upgrade', 'postorius_upgrade' ] - diff --git a/templates/hyperkitty-archive-cleanup.py.j2 b/templates/hyperkitty-archive-cleanup.py.j2 new file mode 100644 index 0000000..d22cfa2 --- /dev/null +++ b/templates/hyperkitty-archive-cleanup.py.j2 @@ -0,0 +1,233 @@ +#!{{ mailman_bindir }}/python3 + +"""Delete HyperKitty archives that Mailman Core no longer archives.""" + +import argparse +import fcntl +import logging +import os +import sys +from urllib.error import HTTPError + + +PROJECT_DIR = {{ mailman_postorius_dir | to_json }} +LOCK_FILE = {{ mailman_hyperkitty_archive_cleanup_lock | to_json }} +DELETE_DELETED_LISTS = {{ mailman_hyperkitty_archive_cleanup_deleted_lists | bool | ternary('True', 'False') }} +DELETE_DISABLED_LISTS = {{ mailman_hyperkitty_archive_cleanup_disabled_lists | bool | ternary('True', 'False') }} +MAX_DELETIONS = {{ mailman_hyperkitty_archive_cleanup_max_deletions | int }} +# HyperKitty 1.3.3 itself uses pages of 10 with the legacy Core client. +PAGE_SIZE = 10 + + +logging.basicConfig( + level=logging.INFO, + format='hyperkitty-archive-cleanup: %(levelname)s: %(message)s') +LOG = logging.getLogger(__name__) + + +def normalize_name(value): + return value.strip().casefold() + + +def normalize_policy(value): + if hasattr(value, 'name'): + value = value.name + if isinstance(value, int): + return {0: 'never', 1: 'private', 2: 'public'}.get(value, str(value)) + return str(value).strip().lower() + + +def get_archive_policy(core_list): + try: + value = core_list.archive_policy + except AttributeError: + value = core_list.settings['archive_policy'] + return normalize_policy(value) + + +def load_core_lists(client): + """Fetch a complete Core snapshot before deciding what may be deleted.""" + result = {} + page_number = 1 + + while True: + page = list(client.get_list_page(count=PAGE_SIZE, page=page_number)) + if not page: + break + + previous_count = len(result) + for core_list in page: + result[normalize_name(core_list.fqdn_listname)] = get_archive_policy(core_list) + + if len(result) == previous_count: + raise RuntimeError( + 'Mailman Core pagination returned no new lists on page {}'.format( + page_number)) + if len(page) < PAGE_SIZE: + break + page_number += 1 + + if not result: + raise RuntimeError( + 'Mailman Core returned no lists; refusing to consider every archive orphaned') + return result + + +def recheck_candidate(client, name): + """Return the current Core policy, or None only for a confirmed 404.""" + try: + core_list = client.get_list(name) + except HTTPError as error: + if error.code == 404: + return None + raise + if core_list is None: + return None + return get_archive_policy(core_list) + + +def find_candidates(core_lists, archive_lists): + candidates = [] + for archive in archive_lists: + policy = core_lists.get(normalize_name(archive.name)) + if policy is None and DELETE_DELETED_LISTS: + candidates.append((archive, 'deleted from Mailman Core')) + elif policy == 'never' and DELETE_DISABLED_LISTS: + candidates.append((archive, 'archive policy is never')) + return candidates + + +def confirm_candidates(client, candidates): + confirmed = [] + for archive, original_reason in candidates: + current_policy = recheck_candidate(client, archive.name) + if current_policy is None and DELETE_DELETED_LISTS: + confirmed.append((archive, 'deleted from Mailman Core')) + elif current_policy == 'never' and DELETE_DISABLED_LISTS: + confirmed.append((archive, 'archive policy is never')) + else: + LOG.warning( + 'skipping %s: Core changed since the initial snapshot (%s)', + archive.name, original_reason) + return confirmed + + +def delete_archives(candidates, dry_run): + from django.core.cache import cache + from django.db import transaction + from django.db.models.signals import post_delete, pre_delete + from hyperkitty.models import Email, MailingList, Thread, Vote + from hyperkitty.signals import ( + Email_on_post_delete, + Email_on_pre_delete, + Thread_on_post_delete, + Vote_on_post_delete, + ) + + signal_bindings = ( + (pre_delete, Email_on_pre_delete, Email), + (post_delete, Email_on_post_delete, Email), + (post_delete, Thread_on_post_delete, Thread), + (post_delete, Vote_on_post_delete, Vote), + ) + + if dry_run: + for archive, reason in candidates: + LOG.info( + 'would delete %s (%s; %d threads, %d messages)', + archive.name, reason, archive.threads.count(), archive.emails.count()) + return 0 + + disconnected = [] + try: + # Bulk archive deletion must not enqueue cache rebuilds for objects that + # cease to exist before django-q executes the tasks (HyperKitty #440). + for signal, receiver, sender in signal_bindings: + if signal.disconnect(receiver, sender=sender): + disconnected.append((signal, receiver, sender)) + + deleted = 0 + for archive, reason in candidates: + with transaction.atomic(): + try: + locked_archive = MailingList.objects.select_for_update().get( + pk=archive.pk) + except MailingList.DoesNotExist: + LOG.warning('archive %s disappeared before deletion', archive.name) + continue + + thread_count = locked_archive.threads.count() + message_count = locked_archive.emails.count() + # Match HyperKitty's administrative delete view: deleting + # threads explicitly also works around backend FK constraints. + locked_archive.threads.all().delete() + locked_archive.delete() + deleted += 1 + LOG.info( + 'deleted %s (%s; %d threads, %d messages)', + archive.name, reason, thread_count, message_count) + finally: + for signal, receiver, sender in disconnected: + signal.connect(receiver, sender=sender) + + if deleted: + cache.clear() + return deleted + + +def main(): + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument( + '--dry-run', action='store_true', + help='report candidates without deleting archives') + arguments = parser.parse_args() + + os.chdir(PROJECT_DIR) + sys.path.insert(0, PROJECT_DIR) + os.environ.setdefault('DJANGO_SETTINGS_MODULE', 'settings') + + import django + django.setup() + + from django_mailman3.lib.mailman import get_mailman_client + from hyperkitty.models import MailingList + from mailmanclient import MailmanConnectionError + + try: + with open(LOCK_FILE, 'w') as lock_handle: + try: + fcntl.flock(lock_handle, fcntl.LOCK_EX | fcntl.LOCK_NB) + except OSError: + LOG.error('another cleanup process holds %s', LOCK_FILE) + return 1 + + try: + client = get_mailman_client() + core_lists = load_core_lists(client) + archives = list(MailingList.objects.all().order_by('name')) + candidates = find_candidates(core_lists, archives) + candidates = confirm_candidates(client, candidates) + except (HTTPError, MailmanConnectionError, RuntimeError) as error: + LOG.error('cannot obtain a reliable Mailman Core view: %s', error) + return 1 + + if MAX_DELETIONS > 0 and len(candidates) > MAX_DELETIONS: + LOG.error( + 'refusing to delete %d archives; safety limit is %d', + len(candidates), MAX_DELETIONS) + for archive, reason in candidates: + LOG.error('candidate %s (%s)', archive.name, reason) + return 1 + + deleted = delete_archives(candidates, arguments.dry_run) + LOG.info( + '%d Core lists, %d HyperKitty archives, %d candidates, %d deleted', + len(core_lists), len(archives), len(candidates), deleted) + return 0 + except OSError as error: + LOG.error('cannot use cleanup lock %s: %s', LOCK_FILE, error) + return 1 + + +if __name__ == '__main__': + sys.exit(main()) diff --git a/templates/hyperkitty-archive-cleanup.service.systemd.j2 b/templates/hyperkitty-archive-cleanup.service.systemd.j2 new file mode 100644 index 0000000..ccf8383 --- /dev/null +++ b/templates/hyperkitty-archive-cleanup.service.systemd.j2 @@ -0,0 +1,15 @@ +[Unit] +Description=Remove obsolete HyperKitty archives +After=network-online.target mailman.service +Wants=network-online.target +ConditionPathExists={{ mailman_postorius_dir }}/settings.py + +[Service] +Type=oneshot +User={{ mailman_user }} +Group={{ mailman_user }} +WorkingDirectory={{ mailman_postorius_dir }} +ExecStart={{ mailman_hyperkitty_archive_cleanup_script }} +TimeoutStartSec=2h +Nice=10 +IOSchedulingClass=idle diff --git a/templates/hyperkitty-archive-cleanup.timer.systemd.j2 b/templates/hyperkitty-archive-cleanup.timer.systemd.j2 new file mode 100644 index 0000000..0a26e44 --- /dev/null +++ b/templates/hyperkitty-archive-cleanup.timer.systemd.j2 @@ -0,0 +1,11 @@ +[Unit] +Description=Weekly cleanup of obsolete HyperKitty archives + +[Timer] +OnCalendar={{ mailman_hyperkitty_archive_cleanup_on_calendar }} +AccuracySec=5min +Persistent=false +Unit=hyperkitty-archive-cleanup.service + +[Install] +WantedBy=timers.target diff --git a/templates/mailman-legacy-hyperkitty-compatibility.py.j2 b/templates/mailman-legacy-hyperkitty-compatibility.py.j2 index de351ca..52b6110 100644 --- a/templates/mailman-legacy-hyperkitty-compatibility.py.j2 +++ b/templates/mailman-legacy-hyperkitty-compatibility.py.j2 @@ -7,6 +7,7 @@ import gettext import glob import hashlib import os +import re import stat import struct import sys @@ -156,6 +157,22 @@ MAILMAN_HYPERKITTY_SEND_NEW = ''' message_text = msg.as_bytes() except (MessageError, KeyError, UnicodeEncodeError) as error: ''' +HYPERKITTY_TASKS_WITH_MAILING_LIST = ( + '_rebuild_mailinglist_cache_recent', + '_rebuild_mailinglist_cache_for_month', + '_rebuild_cache_popular_threads', + '_update_from_mailman', +) +HYPERKITTY_TASKS_GET_OLD = ( + " mlist = MailingList.objects.get(name=mlist_name)\n" +) +HYPERKITTY_TASKS_GET_NEW = ''' try: + mlist = MailingList.objects.get(name=mlist_name) + except MailingList.DoesNotExist: + log.warning('MailingList with name %s does not exist', mlist_name) + return +''' + def distribution(name): dist = pkg_resources.get_distribution(name) @@ -253,6 +270,38 @@ def patch_mailman_hyperkitty(source): 'mailman-hyperkitty byte delivery'), True +def patch_hyperkitty_tasks(source): + """Backport the upstream stale-list guards to all affected tasks.""" + updated = source + changed = False + for function_name in HYPERKITTY_TASKS_WITH_MAILING_LIST: + pattern = re.compile( + r'^def {}\([^\n]*\):\n.*?(?=^def |\Z)'.format( + re.escape(function_name)), + re.MULTILINE | re.DOTALL) + matches = list(pattern.finditer(updated)) + if len(matches) != 1: + raise RuntimeError( + 'HyperKitty tasks: expected exactly one {} function, found {}'.format( + function_name, len(matches))) + + function_source = matches[0].group(0) + if 'except MailingList.DoesNotExist:' in function_source: + continue + if function_source.count(HYPERKITTY_TASKS_GET_OLD) != 1: + raise RuntimeError( + 'HyperKitty tasks: {} does not match the expected source'.format( + function_name)) + + patched_function = function_source.replace( + HYPERKITTY_TASKS_GET_OLD, HYPERKITTY_TASKS_GET_NEW, 1) + updated = ( + updated[:matches[0].start()] + patched_function + + updated[matches[0].end():]) + changed = True + return updated, changed + + def load_source(path): with open(path, 'rb') as source_file: return source_file.read().decode('utf-8') @@ -307,18 +356,21 @@ def main(): mailman_hyperkitty = distribution('mailman-hyperkitty') incoming_path = os.path.join(hyperkitty.location, 'hyperkitty', 'lib', 'incoming.py') + tasks_path = os.path.join(hyperkitty.location, 'hyperkitty', 'tasks.py') scrub_path = os.path.join( django_mailman3.location, 'django_mailman3', 'lib', 'scrub.py') mailman_hyperkitty_path = os.path.join( mailman_hyperkitty.location, 'mailman_hyperkitty', '__init__.py') incoming_source, incoming_changed = patch_hyperkitty(load_source(incoming_path)) + tasks_source, tasks_changed = patch_hyperkitty_tasks(load_source(tasks_path)) scrub_source, scrub_changed = patch_scrubber(load_source(scrub_path)) mailman_hyperkitty_source, mailman_hyperkitty_changed = ( patch_mailman_hyperkitty(load_source(mailman_hyperkitty_path))) # Compile all transformed files before replacing any one of them. compile(incoming_source, incoming_path, 'exec') + compile(tasks_source, tasks_path, 'exec') compile(scrub_source, scrub_path, 'exec') compile(mailman_hyperkitty_source, mailman_hyperkitty_path, 'exec') @@ -332,6 +384,9 @@ def main(): if incoming_changed: atomic_write(incoming_path, incoming_source) print('CHANGED: patched {}'.format(incoming_path)) + if tasks_changed: + atomic_write(tasks_path, tasks_source) + print('CHANGED: patched {}'.format(tasks_path)) if scrub_changed: atomic_write(scrub_path, scrub_source) print('CHANGED: patched {}'.format(scrub_path)) @@ -343,7 +398,7 @@ def main(): print('CHANGED: quarantined {} as {} ({})'.format( catalog_path, destination, error)) - if (not incoming_changed and not scrub_changed and + if (not incoming_changed and not tasks_changed and not scrub_changed and not mailman_hyperkitty_changed and not catalogs): print('OK: compatibility fixes are already applied')