From 4c0e3e17f6f4e1110d07fe7ee7f3a2e3d9bb93c3 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Tue, 11 Jun 2024 11:46:15 -0300 Subject: [PATCH 1/7] chore: remove bin/monthly (#7522) * chore: fix Weekly -> Monthly Let's get it right before it goes away... :-) * chore: remove bin/monthly * chore: fix accidentally committed change --- bin/monthly | 17 ----------------- 1 file changed, 17 deletions(-) delete mode 100755 bin/monthly diff --git a/bin/monthly b/bin/monthly deleted file mode 100755 index 1d36abc210..0000000000 --- a/bin/monthly +++ /dev/null @@ -1,17 +0,0 @@ -#!/bin/bash - -# Weekly datatracker jobs. -# -# This script is expected to be triggered by cron from -# /etc/cron.d/datatracker -export LANG=en_US.UTF-8 -export PYTHONIOENCODING=utf-8 - -DTDIR=/a/www/ietf-datatracker/web -cd $DTDIR/ - -# Set up the virtual environment -source $DTDIR/env/bin/activate - -logger -p user.info -t cron "Running $DTDIR/bin/monthly" - From bdc4b618bbaabac442a17dba35266ea7f82d6302 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Thu, 13 Jun 2024 09:35:43 -0300 Subject: [PATCH 2/7] chore: configure logging for k8s (#7525) --- k8s/settings_local.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/k8s/settings_local.py b/k8s/settings_local.py index 3c77551a9d..b3bb0f6d19 100644 --- a/k8s/settings_local.py +++ b/k8s/settings_local.py @@ -262,3 +262,9 @@ def _multiline_to_list(s): _csrf_trusted_origins_str = os.environ.get("DATATRACKER_CSRF_TRUSTED_ORIGINS") if _csrf_trusted_origins_str is not None: CSRF_TRUSTED_ORIGINS = _multiline_to_list(_csrf_trusted_origins_str) + +# Send logs to console instead of debug_console when running in kubernetes +LOGGING["loggers"]["django"]["handlers"] = ["console", "mail_admins"] +LOGGING["loggers"]["django.security"]["handlers"] = ["console"] +LOGGING["loggers"]["datatracker"]["handlers"] = ["console"] +LOGGING["loggers"]["celery"]["handlers"] = ["console"] From c1941df7e767e729f4b874631faf21921280b5ae Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Fri, 14 Jun 2024 11:28:14 -0300 Subject: [PATCH 3/7] chore: adjustments for k8s deployment (#7531) * chore: handle TERM in datatracker-start.sh * chore: delay celery start if migration needed * chore: skip-checks when migrating * chore: label beat/celery as deleteBeforeUpgrade Used by the infra-k8s deployment process to flag these as needing to be shut down before a new release rolls out. * chore: increase termination grace periods --- dev/build/celery-start.sh | 8 ++ dev/build/datatracker-start.sh | 19 +++- k8s/beat.yaml | 124 ++++++++++++------------- k8s/celery.yaml | 162 +++++++++++++++++---------------- k8s/datatracker.yaml | 2 +- 5 files changed, 171 insertions(+), 144 deletions(-) diff --git a/dev/build/celery-start.sh b/dev/build/celery-start.sh index c229defa2f..c8d4450da3 100644 --- a/dev/build/celery-start.sh +++ b/dev/build/celery-start.sh @@ -5,6 +5,14 @@ echo "Running Datatracker checks..." ./ietf/manage.py check +if ! ietf/manage.py migrate --skip-checks --check ; then + echo "Unapplied migrations found, waiting to start..." + sleep 5 + while ! ietf/manage.py migrate --skip-checks --check ; do + sleep 5 + done +fi + cleanup () { # Cleanly terminate the celery app by sending it a TERM, then waiting for it to exit. if [[ -n "${celery_pid}" ]]; then diff --git a/dev/build/datatracker-start.sh b/dev/build/datatracker-start.sh index 390b46af86..7fee9394f0 100644 --- a/dev/build/datatracker-start.sh +++ b/dev/build/datatracker-start.sh @@ -4,14 +4,29 @@ echo "Running Datatracker checks..." ./ietf/manage.py check echo "Running Datatracker migrations..." -./ietf/manage.py migrate --settings=settings_local +./ietf/manage.py migrate --skip-checks --settings=settings_local echo "Starting Datatracker..." +# trap TERM and shut down gunicorn +cleanup () { + if [[ -n "${gunicorn_pid}" ]]; then + echo "Terminating gunicorn..." + kill -TERM "${gunicorn_pid}" + wait "${gunicorn_pid}" + fi +} + +trap 'trap "" TERM; cleanup' TERM + +# start gunicorn in the background so we can trap the TERM signal gunicorn \ --workers "${DATATRACKER_GUNICORN_WORKERS:-9}" \ --max-requests "${DATATRACKER_GUNICORN_MAX_REQUESTS:-32768}" \ --timeout "${DATATRACKER_GUNICORN_TIMEOUT:-180}" \ --bind :8000 \ --log-level "${DATATRACKER_GUNICORN_LOG_LEVEL:-info}" \ - ietf.wsgi:application + ${DATATRACKER_GUNICORN_EXTRA_ARGS} \ + ietf.wsgi:application & +gunicorn_pid=$! +wait "${gunicorn_pid}" diff --git a/k8s/beat.yaml b/k8s/beat.yaml index 3400b21cbf..99317ab77a 100644 --- a/k8s/beat.yaml +++ b/k8s/beat.yaml @@ -1,61 +1,63 @@ -apiVersion: apps/v1 -kind: Deployment -metadata: - name: beat -spec: - replicas: 1 - revisionHistoryLimit: 2 - selector: - matchLabels: - app: beat - strategy: - type: Recreate - template: - metadata: - labels: - app: beat - spec: - securityContext: - runAsNonRoot: true - containers: - - name: beat - image: "ghcr.io/ietf-tools/datatracker:$APP_IMAGE_TAG" - imagePullPolicy: Always - ports: - - containerPort: 8000 - name: http - protocol: TCP - volumeMounts: - - name: dt-vol - mountPath: /a - - name: dt-tmp - mountPath: /tmp - - name: dt-cfg - mountPath: /workspace/ietf/settings_local.py - subPath: settings_local.py - env: - - name: "CONTAINER_ROLE" - value: "beat" - envFrom: - - configMapRef: - name: django-config - securityContext: - allowPrivilegeEscalation: false - capabilities: - drop: - - ALL - readOnlyRootFilesystem: true - runAsUser: 1000 - runAsGroup: 1000 - volumes: - # To be overriden with the actual shared volume - - name: dt-vol - - name: dt-tmp - emptyDir: - sizeLimit: "2Gi" - - name: dt-cfg - configMap: - name: files-cfgmap - dnsPolicy: ClusterFirst - restartPolicy: Always - terminationGracePeriodSeconds: 30 +apiVersion: apps/v1 +kind: Deployment +metadata: + name: beat + labels: + deleteBeforeUpgrade: yes +spec: + replicas: 1 + revisionHistoryLimit: 2 + selector: + matchLabels: + app: beat + strategy: + type: Recreate + template: + metadata: + labels: + app: beat + spec: + securityContext: + runAsNonRoot: true + containers: + - name: beat + image: "ghcr.io/ietf-tools/datatracker:$APP_IMAGE_TAG" + imagePullPolicy: Always + ports: + - containerPort: 8000 + name: http + protocol: TCP + volumeMounts: + - name: dt-vol + mountPath: /a + - name: dt-tmp + mountPath: /tmp + - name: dt-cfg + mountPath: /workspace/ietf/settings_local.py + subPath: settings_local.py + env: + - name: "CONTAINER_ROLE" + value: "beat" + envFrom: + - configMapRef: + name: django-config + securityContext: + allowPrivilegeEscalation: false + capabilities: + drop: + - ALL + readOnlyRootFilesystem: true + runAsUser: 1000 + runAsGroup: 1000 + volumes: + # To be overriden with the actual shared volume + - name: dt-vol + - name: dt-tmp + emptyDir: + sizeLimit: "2Gi" + - name: dt-cfg + configMap: + name: files-cfgmap + dnsPolicy: ClusterFirst + restartPolicy: Always + terminationGracePeriodSeconds: 600 diff --git a/k8s/celery.yaml b/k8s/celery.yaml index 407f21e66d..132e71761f 100644 --- a/k8s/celery.yaml +++ b/k8s/celery.yaml @@ -1,80 +1,82 @@ -apiVersion: apps/v1 -kind: Deployment -metadata: - name: celery -spec: - replicas: 1 - revisionHistoryLimit: 2 - selector: - matchLabels: - app: celery - strategy: - type: Recreate - template: - metadata: - labels: - app: celery - spec: - securityContext: - runAsNonRoot: true - containers: - # ----------------------------------------------------- - # ScoutAPM Container - # ----------------------------------------------------- - - name: scoutapm - image: "scoutapp/scoutapm:version-1.4.0" - imagePullPolicy: IfNotPresent - livenessProbe: - exec: - command: - - "sh" - - "-c" - - "./core-agent probe --tcp 0.0.0.0:6590 | grep -q 'Agent found'" - securityContext: - readOnlyRootFilesystem: true - runAsUser: 65534 # "nobody" user by default - runAsGroup: 65534 # "nogroup" group by default - # ----------------------------------------------------- - # Celery Container - # ----------------------------------------------------- - - name: celery - image: "ghcr.io/ietf-tools/datatracker:$APP_IMAGE_TAG" - imagePullPolicy: Always - ports: - - containerPort: 8000 - name: http - protocol: TCP - volumeMounts: - - name: dt-vol - mountPath: /a - - name: dt-tmp - mountPath: /tmp - - name: dt-cfg - mountPath: /workspace/ietf/settings_local.py - subPath: settings_local.py - env: - - name: "CONTAINER_ROLE" - value: "celery" - envFrom: - - configMapRef: - name: django-config - securityContext: - allowPrivilegeEscalation: false - capabilities: - drop: - - ALL - readOnlyRootFilesystem: true - runAsUser: 1000 - runAsGroup: 1000 - volumes: - # To be overriden with the actual shared volume - - name: dt-vol - - name: dt-tmp - emptyDir: - sizeLimit: "2Gi" - - name: dt-cfg - configMap: - name: files-cfgmap - dnsPolicy: ClusterFirst - restartPolicy: Always - terminationGracePeriodSeconds: 30 +apiVersion: apps/v1 +kind: Deployment +metadata: + name: celery + labels: + deleteBeforeUpgrade: yes +spec: + replicas: 1 + revisionHistoryLimit: 2 + selector: + matchLabels: + app: celery + strategy: + type: Recreate + template: + metadata: + labels: + app: celery + spec: + securityContext: + runAsNonRoot: true + containers: + # ----------------------------------------------------- + # ScoutAPM Container + # ----------------------------------------------------- + - name: scoutapm + image: "scoutapp/scoutapm:version-1.4.0" + imagePullPolicy: IfNotPresent + livenessProbe: + exec: + command: + - "sh" + - "-c" + - "./core-agent probe --tcp 0.0.0.0:6590 | grep -q 'Agent found'" + securityContext: + readOnlyRootFilesystem: true + runAsUser: 65534 # "nobody" user by default + runAsGroup: 65534 # "nogroup" group by default + # ----------------------------------------------------- + # Celery Container + # ----------------------------------------------------- + - name: celery + image: "ghcr.io/ietf-tools/datatracker:$APP_IMAGE_TAG" + imagePullPolicy: Always + ports: + - containerPort: 8000 + name: http + protocol: TCP + volumeMounts: + - name: dt-vol + mountPath: /a + - name: dt-tmp + mountPath: /tmp + - name: dt-cfg + mountPath: /workspace/ietf/settings_local.py + subPath: settings_local.py + env: + - name: "CONTAINER_ROLE" + value: "celery" + envFrom: + - configMapRef: + name: django-config + securityContext: + allowPrivilegeEscalation: false + capabilities: + drop: + - ALL + readOnlyRootFilesystem: true + runAsUser: 1000 + runAsGroup: 1000 + volumes: + # To be overriden with the actual shared volume + - name: dt-vol + - name: dt-tmp + emptyDir: + sizeLimit: "2Gi" + - name: dt-cfg + configMap: + name: files-cfgmap + dnsPolicy: ClusterFirst + restartPolicy: Always + terminationGracePeriodSeconds: 600 diff --git a/k8s/datatracker.yaml b/k8s/datatracker.yaml index 7ca92ba99e..9771623138 100644 --- a/k8s/datatracker.yaml +++ b/k8s/datatracker.yaml @@ -77,7 +77,7 @@ spec: name: files-cfgmap dnsPolicy: ClusterFirst restartPolicy: Always - terminationGracePeriodSeconds: 30 + terminationGracePeriodSeconds: 60 --- apiVersion: v1 kind: Service From 774fe78d3fe30d3a5a85980e88e83c40884a9651 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Fri, 14 Jun 2024 12:43:06 -0300 Subject: [PATCH 4/7] chore: gunicorn access logs / capture_output (#7534) --- dev/build/datatracker-start.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dev/build/datatracker-start.sh b/dev/build/datatracker-start.sh index 7fee9394f0..c6f9f3bf4c 100644 --- a/dev/build/datatracker-start.sh +++ b/dev/build/datatracker-start.sh @@ -26,6 +26,8 @@ gunicorn \ --timeout "${DATATRACKER_GUNICORN_TIMEOUT:-180}" \ --bind :8000 \ --log-level "${DATATRACKER_GUNICORN_LOG_LEVEL:-info}" \ + --capture-output \ + --access-logfile -\ ${DATATRACKER_GUNICORN_EXTRA_ARGS} \ ietf.wsgi:application & gunicorn_pid=$! From 4e6abcbaadeb9d32e0c1ffa2522cb525872810e2 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Fri, 14 Jun 2024 17:49:44 -0300 Subject: [PATCH 5/7] refactor: make WG summary view into a task (#7529) * feat: generate_wg_summary_files_task() * refactor: wg summaries from filesys for view * refactor: use new helper for charter views * refactor: use FileResponse * refactor: don't use FileResponse FileResponse generates a StreamingHttpResponse which brings with it differences I don't fully understand, so let's stay with HttpResponse * test: update view tests * test: test_generate_wg_summary_files_task() * chore: create PeriodicTask N.B. that this makes it hourly instead of daily --- ietf/group/tasks.py | 38 +++++ ietf/group/tests_info.py | 161 ++++++++++++------ ietf/group/views.py | 43 ++--- ietf/settings.py | 1 + .../management/commands/periodic_tasks.py | 10 ++ 5 files changed, 171 insertions(+), 82 deletions(-) diff --git a/ietf/group/tasks.py b/ietf/group/tasks.py index f7717616d1..f19246fb55 100644 --- a/ietf/group/tasks.py +++ b/ietf/group/tasks.py @@ -14,6 +14,7 @@ from .models import Group from .utils import fill_in_charter_info, fill_in_wg_drafts, fill_in_wg_roles +from .views import extract_last_name, roles @shared_task @@ -59,3 +60,40 @@ def generate_wg_charters_files_task(): log.log( f"Error copying {charters_by_acronym_file} to {charter_copy_dest}: {err}" ) + + +@shared_task +def generate_wg_summary_files_task(): + # Active WGs (all should have a parent, but filter to be sure) + groups = ( + Group.objects.filter(type="wg", state="active") + .exclude(parent=None) + .order_by("acronym") + ) + # Augment groups with chairs list + for group in groups: + group.chairs = sorted(roles(group, "chair"), key=extract_last_name) + + # Active areas with one or more active groups in them + areas = Group.objects.filter( + type="area", + state="active", + group__in=groups, + ).distinct().order_by("name") + # Augment areas with their groups + for area in areas: + area.groups = [g for g in groups if g.parent_id == area.pk] + summary_path = Path(settings.GROUP_SUMMARY_PATH) + summary_file = summary_path / "1wg-summary.txt" + summary_file.write_text( + render_to_string("group/1wg-summary.txt", {"areas": areas}), + encoding="utf8", + ) + summary_by_acronym_file = summary_path / "1wg-summary-by-acronym.txt" + summary_by_acronym_file.write_text( + render_to_string( + "group/1wg-summary-by-acronym.txt", + {"areas": areas, "groups": groups}, + ), + encoding="utf8", + ) diff --git a/ietf/group/tests_info.py b/ietf/group/tests_info.py index 777671db9d..29a45a32eb 100644 --- a/ietf/group/tests_info.py +++ b/ietf/group/tests_info.py @@ -8,7 +8,7 @@ import io import bleach -from unittest.mock import patch +from unittest.mock import call, patch from pathlib import Path from pyquery import PyQuery from tempfile import NamedTemporaryFile @@ -16,6 +16,7 @@ import debug # pyflakes:ignore from django.conf import settings +from django.http import Http404, HttpResponse from django.test import RequestFactory from django.test.utils import override_settings from django.urls import reverse as urlreverse @@ -35,7 +36,8 @@ DatedGroupMilestoneFactory, DatelessGroupMilestoneFactory) from ietf.group.forms import GroupForm from ietf.group.models import Group, GroupEvent, GroupMilestone, GroupStateTransitions, Role -from ietf.group.tasks import generate_wg_charters_files_task +from ietf.group.tasks import generate_wg_charters_files_task, generate_wg_summary_files_task +from ietf.group.views import response_from_file from ietf.group.utils import save_group_in_history, setup_default_community_list_for_group from ietf.meeting.factories import SessionFactory from ietf.name.models import DocTagName, GroupStateName, GroupTypeName, ExtResourceName, RoleName @@ -58,7 +60,11 @@ def pklist(docs): return [ str(doc.pk) for doc in docs.all() ] class GroupPagesTests(TestCase): - settings_temp_path_overrides = TestCase.settings_temp_path_overrides + ['CHARTER_PATH', 'CHARTER_COPY_PATH'] + settings_temp_path_overrides = TestCase.settings_temp_path_overrides + [ + "CHARTER_PATH", + "CHARTER_COPY_PATH", + "GROUP_SUMMARY_PATH", + ] def test_active_groups(self): area = GroupFactory.create(type_id='area') @@ -112,63 +118,90 @@ def test_group_home(self): self.assertContains(r, draft.name) self.assertContains(r, draft.title) - def test_wg_summaries(self): - group = CharterFactory(group__type_id='wg',group__parent=GroupFactory(type_id='area')).group - RoleFactory(group=group,name_id='chair',person=PersonFactory()) - RoleFactory(group=group,name_id='ad',person=PersonFactory()) - - chair = Email.objects.filter(role__group=group, role__name="chair")[0] - - url = urlreverse('ietf.group.views.wg_summary_area', kwargs=dict(group_type="wg")) - r = self.client.get(url) + def test_response_from_file(self): + # n.b., GROUP_SUMMARY_PATH is a temp dir that will be cleaned up automatically + fp = Path(settings.GROUP_SUMMARY_PATH) / "some-file.txt" + fp.write_text("This is a charters file with an é") + r = response_from_file(fp) self.assertEqual(r.status_code, 200) - self.assertContains(r, group.parent.name) - self.assertContains(r, group.acronym) - self.assertContains(r, group.name) - self.assertContains(r, chair.address) - - url = urlreverse('ietf.group.views.wg_summary_acronym', kwargs=dict(group_type="wg")) - r = self.client.get(url) - self.assertEqual(r.status_code, 200) - self.assertContains(r, group.acronym) - self.assertContains(r, group.name) - self.assertContains(r, chair.address) - - def test_wg_charters(self): - # file does not exist = 404 - url = urlreverse("ietf.group.views.wg_charters", kwargs=dict(group_type="wg")) - r = self.client.get(url) + self.assertEqual(r.headers["Content-Type"], "text/plain; charset=utf-8") + self.assertEqual(r.content.decode("utf8"), "This is a charters file with an é") + # now try with a nonexistent file + fp.unlink() + with self.assertRaises(Http404): + response_from_file(fp) + + @patch("ietf.group.views.response_from_file") + def test_wg_summary_area(self, mock): + r = self.client.get( + urlreverse("ietf.group.views.wg_summary_area", kwargs={"group_type": "rg"}) + ) # not wg self.assertEqual(r.status_code, 404) - - # should return expected file with expected encoding - wg_path = Path(settings.CHARTER_PATH) / "1wg-charters.txt" - wg_path.write_text("This is a charters file with an é") - r = self.client.get(url) + self.assertFalse(mock.called) + mock.return_value = HttpResponse("yay") + r = self.client.get( + urlreverse("ietf.group.views.wg_summary_area", kwargs={"group_type": "wg"}) + ) self.assertEqual(r.status_code, 200) - self.assertEqual(r.charset, "UTF-8") - self.assertEqual(r.content.decode("utf8"), "This is a charters file with an é") - - # non-wg request = 404 even if the file exists - url = urlreverse("ietf.group.views.wg_charters", kwargs=dict(group_type="rg")) - r = self.client.get(url) + self.assertEqual(r.content.decode(), "yay") + self.assertEqual(mock.call_args, call(Path(settings.GROUP_SUMMARY_PATH) / "1wg-summary.txt")) + + @patch("ietf.group.views.response_from_file") + def test_wg_summary_acronym(self, mock): + r = self.client.get( + urlreverse( + "ietf.group.views.wg_summary_acronym", kwargs={"group_type": "rg"} + ) + ) # not wg self.assertEqual(r.status_code, 404) + self.assertFalse(mock.called) + mock.return_value = HttpResponse("yay") + r = self.client.get( + urlreverse( + "ietf.group.views.wg_summary_acronym", kwargs={"group_type": "wg"} + ) + ) + self.assertEqual(r.status_code, 200) + self.assertEqual(r.content.decode(), "yay") + self.assertEqual( + mock.call_args, call(Path(settings.GROUP_SUMMARY_PATH) / "1wg-summary-by-acronym.txt") + ) - def test_wg_charters_by_acronym(self): - url = urlreverse("ietf.group.views.wg_charters_by_acronym", kwargs=dict(group_type="wg")) - r = self.client.get(url) + @patch("ietf.group.views.response_from_file") + def test_wg_charters(self, mock): + r = self.client.get( + urlreverse("ietf.group.views.wg_charters", kwargs={"group_type": "rg"}) + ) # not wg self.assertEqual(r.status_code, 404) - - wg_path = Path(settings.CHARTER_PATH) / "1wg-charters-by-acronym.txt" - wg_path.write_text("This is a charters file with an é") - r = self.client.get(url) + self.assertFalse(mock.called) + mock.return_value = HttpResponse("yay") + r = self.client.get( + urlreverse("ietf.group.views.wg_charters", kwargs={"group_type": "wg"}) + ) self.assertEqual(r.status_code, 200) - self.assertEqual(r.charset, "UTF-8") - self.assertEqual(r.content.decode("utf8"), "This is a charters file with an é") - - # non-wg request = 404 even if the file exists - url = urlreverse("ietf.group.views.wg_charters_by_acronym", kwargs=dict(group_type="rg")) - r = self.client.get(url) + self.assertEqual(r.content.decode(), "yay") + self.assertEqual(mock.call_args, call(Path(settings.CHARTER_PATH) / "1wg-charters.txt")) + + @patch("ietf.group.views.response_from_file") + def test_wg_charters_by_acronym(self, mock): + r = self.client.get( + urlreverse( + "ietf.group.views.wg_charters_by_acronym", kwargs={"group_type": "rg"} + ) + ) # not wg self.assertEqual(r.status_code, 404) + self.assertFalse(mock.called) + mock.return_value = HttpResponse("yay") + r = self.client.get( + urlreverse( + "ietf.group.views.wg_charters_by_acronym", kwargs={"group_type": "wg"} + ) + ) + self.assertEqual(r.status_code, 200) + self.assertEqual(r.content.decode(), "yay") + self.assertEqual( + mock.call_args, call(Path(settings.CHARTER_PATH) / "1wg-charters-by-acronym.txt") + ) def test_generate_wg_charters_files_task(self): group = CharterFactory( @@ -254,6 +287,30 @@ def test_generate_wg_charters_files_task_without_copy(self): ) self.assertEqual(not_a_dir.read_text(), "Not a dir") + def test_generate_wg_summary_files_task(self): + group = CharterFactory(group__type_id='wg',group__parent=GroupFactory(type_id='area')).group + RoleFactory(group=group,name_id='chair',person=PersonFactory()) + RoleFactory(group=group,name_id='ad',person=PersonFactory()) + + chair = Email.objects.filter(role__group=group, role__name="chair")[0] + + generate_wg_summary_files_task() + + summary_by_area_contents = ( + Path(settings.GROUP_SUMMARY_PATH) / "1wg-summary.txt" + ).read_text(encoding="utf8") + self.assertIn(group.parent.name, summary_by_area_contents) + self.assertIn(group.acronym, summary_by_area_contents) + self.assertIn(group.name, summary_by_area_contents) + self.assertIn(chair.address, summary_by_area_contents) + + summary_by_acronym_contents = ( + Path(settings.GROUP_SUMMARY_PATH) / "1wg-summary-by-acronym.txt" + ).read_text(encoding="utf8") + self.assertIn(group.acronym, summary_by_acronym_contents) + self.assertIn(group.name, summary_by_acronym_contents) + self.assertIn(chair.address, summary_by_acronym_contents) + def test_chartering_groups(self): group = CharterFactory(group__type_id='wg',group__parent=GroupFactory(type_id='area'),states=[('charter','intrev')]).group diff --git a/ietf/group/views.py b/ietf/group/views.py index 0c37cca3a0..09b1ac933f 100644 --- a/ietf/group/views.py +++ b/ietf/group/views.py @@ -152,56 +152,39 @@ def check_group_email_aliases(): return False +def response_from_file(fpath: Path) -> HttpResponse: + """Helper to shovel a file back in an HttpResponse""" + try: + content = fpath.read_bytes() + except IOError: + raise Http404 + return HttpResponse(content, content_type="text/plain; charset=utf-8") + + # --- View functions --------------------------------------------------- def wg_summary_area(request, group_type): if group_type != "wg": raise Http404 - areas = Group.objects.filter(type="area", state="active").order_by("name") - for area in areas: - area.groups = Group.objects.filter(parent=area, type="wg", state="active").order_by("acronym") - for group in area.groups: - group.chairs = sorted(roles(group, "chair"), key=extract_last_name) - - areas = [a for a in areas if a.groups] + return response_from_file(Path(settings.GROUP_SUMMARY_PATH) / "1wg-summary.txt") - return render(request, 'group/1wg-summary.txt', - { 'areas': areas }, - content_type='text/plain; charset=UTF-8') def wg_summary_acronym(request, group_type): if group_type != "wg": raise Http404 - areas = Group.objects.filter(type="area", state="active").order_by("name") - groups = Group.objects.filter(type="wg", state="active").order_by("acronym").select_related("parent") - for group in groups: - group.chairs = sorted(roles(group, "chair"), key=extract_last_name) - return render(request, 'group/1wg-summary-by-acronym.txt', - { 'areas': areas, - 'groups': groups }, - content_type='text/plain; charset=UTF-8') + return response_from_file(Path(settings.GROUP_SUMMARY_PATH) / "1wg-summary-by-acronym.txt") def wg_charters(request, group_type): if group_type != "wg": raise Http404 - fpath = Path(settings.CHARTER_PATH) / "1wg-charters.txt" - try: - content = fpath.read_bytes() - except IOError: - raise Http404 - return HttpResponse(content, content_type="text/plain; charset=UTF-8") + return response_from_file(Path(settings.CHARTER_PATH) / "1wg-charters.txt") def wg_charters_by_acronym(request, group_type): if group_type != "wg": raise Http404 - fpath = Path(settings.CHARTER_PATH) / "1wg-charters-by-acronym.txt" - try: - content = fpath.read_bytes() - except IOError: - raise Http404 - return HttpResponse(content, content_type="text/plain; charset=UTF-8") + return response_from_file(Path(settings.CHARTER_PATH) / "1wg-charters-by-acronym.txt") def active_groups(request, group_type=None): diff --git a/ietf/settings.py b/ietf/settings.py index 4f0bba20a9..f04dd15bca 100644 --- a/ietf/settings.py +++ b/ietf/settings.py @@ -671,6 +671,7 @@ def skip_unreadable_post(record): RFC_PATH = '/a/www/ietf-ftp/rfc/' CHARTER_PATH = '/a/ietfdata/doc/charter/' CHARTER_COPY_PATH = '/a/www/ietf-ftp/ietf' # copy 1wg-charters files here if set +GROUP_SUMMARY_PATH = '/a/www/ietf-ftp/ietf' BOFREQ_PATH = '/a/ietfdata/doc/bofreq/' CONFLICT_REVIEW_PATH = '/a/ietfdata/doc/conflict-review' STATUS_CHANGE_PATH = '/a/ietfdata/doc/status-change' diff --git a/ietf/utils/management/commands/periodic_tasks.py b/ietf/utils/management/commands/periodic_tasks.py index 7f0c988dcd..76d362bb24 100644 --- a/ietf/utils/management/commands/periodic_tasks.py +++ b/ietf/utils/management/commands/periodic_tasks.py @@ -231,6 +231,16 @@ def create_default_tasks(self): ), ) + PeriodicTask.objects.get_or_create( + name="Generate WG summary files", + task="ietf.group.tasks.generate_wg_summary_files_task", + defaults=dict( + enabled=False, + crontab=self.crontabs["hourly"], + description="Update 1wg-summary.txt and 1wg-summary-by-acronym.txt", + ), + ) + PeriodicTask.objects.get_or_create( name="Generate I-D bibxml files", task="ietf.doc.tasks.generate_draft_bibxml_files_task", From 7541c21486672819aa74b2ce4101185a1b4acf9b Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Fri, 14 Jun 2024 17:51:21 -0300 Subject: [PATCH 6/7] chore: scoutapm shutdown fix (#7538) --- k8s/celery.yaml | 9 +++++++++ k8s/datatracker.yaml | 9 +++++++++ 2 files changed, 18 insertions(+) diff --git a/k8s/celery.yaml b/k8s/celery.yaml index 132e71761f..e0e5062691 100644 --- a/k8s/celery.yaml +++ b/k8s/celery.yaml @@ -26,6 +26,15 @@ spec: - name: scoutapm image: "scoutapp/scoutapm:version-1.4.0" imagePullPolicy: IfNotPresent + # Replace command with one that will shut down on a TERM signal + # The ./core-agent start command line is from the scoutapm docker image + command: + - "sh" + - "-c" + - >- + trap './core-agent shutdown --tcp 0.0.0.0:6590' TERM; + ./core-agent start --daemonize false --log-level debug --tcp 0.0.0.0:6590 & + wait $! livenessProbe: exec: command: diff --git a/k8s/datatracker.yaml b/k8s/datatracker.yaml index 9771623138..ee82489274 100644 --- a/k8s/datatracker.yaml +++ b/k8s/datatracker.yaml @@ -24,6 +24,15 @@ spec: - name: scoutapm image: "scoutapp/scoutapm:version-1.4.0" imagePullPolicy: IfNotPresent + # Replace command with one that will shut down on a TERM signal + # The ./core-agent start command line is from the scoutapm docker image + command: + - "sh" + - "-c" + - >- + trap './core-agent shutdown --tcp 0.0.0.0:6590' TERM; + ./core-agent start --daemonize false --log-level debug --tcp 0.0.0.0:6590 & + wait $! livenessProbe: exec: command: From 35ab9bf4e464c0559968adc3c285423486a3724d Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Sat, 15 Jun 2024 16:18:01 -0300 Subject: [PATCH 7/7] refactor: adjust mail ingestion api (#7523) --- ietf/api/tests.py | 61 ++++++++++++++++++++++++++++++++++++----- ietf/api/views.py | 70 ++++++++++++++++++++++------------------------- 2 files changed, 87 insertions(+), 44 deletions(-) diff --git a/ietf/api/tests.py b/ietf/api/tests.py index 66bc7a3be7..fd8eb52cd6 100644 --- a/ietf/api/tests.py +++ b/ietf/api/tests.py @@ -10,6 +10,7 @@ from importlib import import_module from pathlib import Path +from random import randrange from django.apps import apps from django.conf import settings @@ -1072,8 +1073,20 @@ def test_ingest_email( self.assertEqual(r.status_code, 400) self.assertFalse(any(m.called for m in mocks)) - # test that valid requests call handlers appropriately + # bad destination message_b64 = base64.b64encode(b"This is a message").decode() + r = self.client.post( + url, + {"dest": "not-a-destination", "message": message_b64}, + content_type="application/json", + headers={"X-Api-Key": "valid-token"}, + ) + self.assertEqual(r.status_code, 200) + self.assertEqual(r.headers["Content-Type"], "application/json") + self.assertEqual(json.loads(r.content), {"result": "bad_dest"}) + self.assertFalse(any(m.called for m in mocks)) + + # test that valid requests call handlers appropriately r = self.client.post( url, {"dest": "iana-review", "message": message_b64}, @@ -1081,6 +1094,8 @@ def test_ingest_email( headers={"X-Api-Key": "valid-token"}, ) self.assertEqual(r.status_code, 200) + self.assertEqual(r.headers["Content-Type"], "application/json") + self.assertEqual(json.loads(r.content), {"result": "ok"}) self.assertTrue(mock_iana_ingest.called) self.assertEqual(mock_iana_ingest.call_args, mock.call(b"This is a message")) self.assertFalse(any(m.called for m in (mocks - {mock_iana_ingest}))) @@ -1093,20 +1108,44 @@ def test_ingest_email( headers={"X-Api-Key": "valid-token"}, ) self.assertEqual(r.status_code, 200) + self.assertEqual(r.headers["Content-Type"], "application/json") + self.assertEqual(json.loads(r.content), {"result": "ok"}) self.assertTrue(mock_ipr_ingest.called) self.assertEqual(mock_ipr_ingest.call_args, mock.call(b"This is a message")) self.assertFalse(any(m.called for m in (mocks - {mock_ipr_ingest}))) mock_ipr_ingest.reset_mock() + # bad nomcom-feedback dest + for bad_nomcom_dest in [ + "nomcom-feedback", # no suffix + "nomcom-feedback-", # no year + "nomcom-feedback-squid", # not a year, + "nomcom-feedback-2024-2025", # also not a year + ]: + r = self.client.post( + url, + {"dest": bad_nomcom_dest, "message": message_b64}, + content_type="application/json", + headers={"X-Api-Key": "valid-token"}, + ) + self.assertEqual(r.status_code, 200) + self.assertEqual(r.headers["Content-Type"], "application/json") + self.assertEqual(json.loads(r.content), {"result": "bad_dest"}) + self.assertFalse(any(m.called for m in mocks)) + + # good nomcom-feedback dest + random_year = randrange(100000) r = self.client.post( url, - {"dest": "nomcom-feedback", "message": message_b64, "year": 2024}, # arbitrary year + {"dest": f"nomcom-feedback-{random_year}", "message": message_b64}, content_type="application/json", headers={"X-Api-Key": "valid-token"}, ) self.assertEqual(r.status_code, 200) + self.assertEqual(r.headers["Content-Type"], "application/json") + self.assertEqual(json.loads(r.content), {"result": "ok"}) self.assertTrue(mock_nomcom_ingest.called) - self.assertEqual(mock_nomcom_ingest.call_args, mock.call(b"This is a message", 2024)) + self.assertEqual(mock_nomcom_ingest.call_args, mock.call(b"This is a message", random_year)) self.assertFalse(any(m.called for m in (mocks - {mock_nomcom_ingest}))) mock_nomcom_ingest.reset_mock() @@ -1118,7 +1157,9 @@ def test_ingest_email( content_type="application/json", headers={"X-Api-Key": "valid-token"}, ) - self.assertEqual(r.status_code, 400) + self.assertEqual(r.status_code, 200) + self.assertEqual(r.headers["Content-Type"], "application/json") + self.assertEqual(json.loads(r.content), {"result": "bad_msg"}) self.assertTrue(mock_iana_ingest.called) self.assertEqual(mock_iana_ingest.call_args, mock.call(b"This is a message")) self.assertFalse(any(m.called for m in (mocks - {mock_iana_ingest}))) @@ -1138,7 +1179,9 @@ def test_ingest_email( content_type="application/json", headers={"X-Api-Key": "valid-token"}, ) - self.assertEqual(r.status_code, 400) + self.assertEqual(r.status_code, 200) + self.assertEqual(r.headers["Content-Type"], "application/json") + self.assertEqual(json.loads(r.content), {"result": "bad_msg"}) self.assertTrue(mock_iana_ingest.called) self.assertEqual(mock_iana_ingest.call_args, mock.call(b"This is a message")) self.assertFalse(any(m.called for m in (mocks - {mock_iana_ingest}))) @@ -1167,7 +1210,9 @@ def test_ingest_email( content_type="application/json", headers={"X-Api-Key": "valid-token"}, ) - self.assertEqual(r.status_code, 400) + self.assertEqual(r.status_code, 200) + self.assertEqual(r.headers["Content-Type"], "application/json") + self.assertEqual(json.loads(r.content), {"result": "bad_msg"}) self.assertTrue(mock_iana_ingest.called) self.assertEqual(mock_iana_ingest.call_args, mock.call(b"This is a message")) self.assertFalse(any(m.called for m in (mocks - {mock_iana_ingest}))) @@ -1192,7 +1237,9 @@ def test_ingest_email( content_type="application/json", headers={"X-Api-Key": "valid-token"}, ) - self.assertEqual(r.status_code, 400) + self.assertEqual(r.status_code, 200) + self.assertEqual(r.headers["Content-Type"], "application/json") + self.assertEqual(json.loads(r.content), {"result": "bad_msg"}) self.assertTrue(mock_iana_ingest.called) self.assertEqual(mock_iana_ingest.call_args, mock.call(b"This is a message")) self.assertFalse(any(m.called for m in (mocks - {mock_iana_ingest}))) diff --git a/ietf/api/views.py b/ietf/api/views.py index 3c3ea0d5a9..6aaed4b6a9 100644 --- a/ietf/api/views.py +++ b/ietf/api/views.py @@ -8,6 +8,7 @@ import pytz import re +from contextlib import suppress from django.conf import settings from django.contrib.auth import authenticate from django.contrib.auth.decorators import login_required @@ -533,32 +534,13 @@ def role_holder_addresses(request): "type": "object", "properties": { "dest": { - "enum": [ - "iana-review", - "ipr-response", - "nomcom-feedback", - ] + "type": "string", }, "message": { "type": "string", # base64-encoded mail message }, }, "required": ["dest", "message"], - "if": { - # If dest == "nomcom-feedback"... - "properties": { - "dest": {"const": "nomcom-feedback"}, - } - }, - "then": { - # ... then also require year, an integer, be present - "properties": { - "year": { - "type": "integer", - }, - }, - "required": ["year"], - }, } ) @@ -630,49 +612,63 @@ def as_emailmessage(self) -> Optional[EmailMessage]: @requires_api_token @csrf_exempt def ingest_email(request): + """Ingest incoming email + + Returns a 4xx or 5xx status code if the HTTP request was invalid or something went + wrong while processing it. If the request was valid, returns a 200. This may or may + not indicate that the message was accepted. + """ - def _err(code, text): + def _http_err(code, text): return HttpResponse(text, status=code, content_type="text/plain") + def _api_response(result): + return JsonResponse(data={"result": result}) + if request.method != "POST": - return _err(405, "Method not allowed") + return _http_err(405, "Method not allowed") if request.content_type != "application/json": - return _err(415, "Content-Type must be application/json") + return _http_err(415, "Content-Type must be application/json") # Validate try: payload = json.loads(request.body) _response_email_json_validator.validate(payload) except json.decoder.JSONDecodeError as err: - return _err(400, f"JSON parse error at line {err.lineno} col {err.colno}: {err.msg}") + return _http_err(400, f"JSON parse error at line {err.lineno} col {err.colno}: {err.msg}") except jsonschema.exceptions.ValidationError as err: - return _err(400, f"JSON schema error at {err.json_path}: {err.message}") + return _http_err(400, f"JSON schema error at {err.json_path}: {err.message}") except Exception: - return _err(400, "Invalid request format") + return _http_err(400, "Invalid request format") try: message = base64.b64decode(payload["message"], validate=True) except binascii.Error: - return _err(400, "Invalid message: bad base64 encoding") + return _http_err(400, "Invalid message: bad base64 encoding") dest = payload["dest"] + valid_dest = False try: if dest == "iana-review": + valid_dest = True iana_ingest_review_email(message) elif dest == "ipr-response": + valid_dest = True ipr_ingest_response_email(message) - elif dest == "nomcom-feedback": - year = payload["year"] - nomcom_ingest_feedback_email(message, year) - else: - # Should never get here - json schema validation should enforce the enum - log.unreachable(date="2024-04-04") - return _err(400, "Invalid dest") # return something reasonable if we got here unexpectedly + elif dest.startswith("nomcom-feedback-"): + maybe_year = dest[len("nomcom-feedback-"):] + if maybe_year.isdecimal(): + valid_dest = True + nomcom_ingest_feedback_email(message, int(maybe_year)) except EmailIngestionError as err: error_email = err.as_emailmessage() if error_email is not None: - send_smtp(error_email) - return _err(400, err.msg) + with suppress(Exception): # send_smtp logs its own exceptions, ignore them here + send_smtp(error_email) + return _api_response("bad_msg") + + if not valid_dest: + return _api_response("bad_dest") - return HttpResponse(status=200) + return _api_response("ok")