diff --git a/app.json b/app.json index 462723dc4c..0f79ecc68f 100644 --- a/app.json +++ b/app.json @@ -138,14 +138,6 @@ "description": "Expose the OIDC login functionality.", "required": false }, - "FASTLY_AUTH_TOKEN": { - "description": "Optional token for the Fastly purge API.", - "required": false - }, - "FASTLY_URL": { - "description": "The URL to the Fastly API.", - "required": false - }, "GA_TRACKING_ID": { "description": "Google analytics tracking ID", "required": false @@ -482,6 +474,14 @@ "description": "The execution environment that the app is in (e.g. dev, staging, prod)", "required": true }, + "MITX_ONLINE_FASTLY_AUTH_TOKEN": { + "description": "Optional token for the Fastly purge API.", + "required": false + }, + "MITX_ONLINE_FASTLY_URL": { + "description": "The URL to the Fastly API.", + "required": false + }, "MITX_ONLINE_FROM_EMAIL": { "description": "E-mail to use for the from field", "required": false @@ -546,6 +546,10 @@ "description": "Dashboard URL for UAI enrollment emails", "required": false }, + "MIT_LEARN_FASTLY_SERVICE_ID": { + "description": "Fastly service ID for the MIT Learn frontend, used for surrogate key (tag) purging.", + "required": false + }, "MIT_LEARN_FROM_EMAIL": { "description": "From email address for UAI enrollment emails", "required": false diff --git a/cms/signals_test.py b/cms/signals_test.py new file mode 100644 index 0000000000..37684889a0 --- /dev/null +++ b/cms/signals_test.py @@ -0,0 +1,62 @@ +"""Tests for cms.signals""" + +from unittest.mock import patch + +import factory +import pytest +from django.db.models.signals import post_save + +from cms.factories import CoursePageFactory, ProgramPageFactory, ResourcePageFactory + +pytestmark = pytest.mark.django_db + + +@patch("cms.signals.transaction.on_commit", side_effect=lambda callback: callback()) +@patch("cms.tasks.queue_fastly_surrogate_key_purge.delay") +def test_purge_fastly_cache_on_publish_course_page(mock_purge_delay, mock_on_commit): + """Publishing a CoursePage purges the key for its course.""" + course_page = CoursePageFactory.create() + mock_purge_delay.reset_mock() + + # Publishing saves the Course as well, which purges the same key via its own + # post_save receiver; mute post_save so only the publish path is counted. + # This mutes every post_save receiver, Wagtail's index updates included, so + # the publish is not a fully faithful one -- adequate for these assertions. + with factory.django.mute_signals(post_save): + course_page.save_revision().publish() + + mock_purge_delay.assert_called_once_with( + f"mitxonline:course:{course_page.course.readable_id}" + ) + + +@patch("cms.signals.transaction.on_commit", side_effect=lambda callback: callback()) +@patch("cms.tasks.queue_fastly_surrogate_key_purge.delay") +def test_purge_fastly_cache_on_publish_program_page(mock_purge_delay, mock_on_commit): + """Publishing a ProgramPage purges the key for its program.""" + program_page = ProgramPageFactory.create() + mock_purge_delay.reset_mock() + + with factory.django.mute_signals(post_save): + program_page.save_revision().publish() + + mock_purge_delay.assert_called_once_with( + f"mitxonline:program:{program_page.program.readable_id}" + ) + + +@patch("cms.signals.transaction.on_commit", side_effect=lambda callback: callback()) +@patch("cms.tasks.queue_fastly_surrogate_key_purge.delay") +def test_purge_fastly_cache_on_publish_ignores_other_pages( + mock_purge_delay, mock_on_commit +): + """Publishing a page that is not a product page purges nothing.""" + resource_page = ResourcePageFactory.create() + mock_purge_delay.reset_mock() + + # A ResourcePage has no Course or Program, so there is no post_save purge to + # suppress here; muted only to keep the three tests the same shape. + with factory.django.mute_signals(post_save): + resource_page.save_revision().publish() + + mock_purge_delay.assert_not_called() diff --git a/cms/tasks.py b/cms/tasks.py index cf23ce9104..6fbbf14c8b 100644 --- a/cms/tasks.py +++ b/cms/tasks.py @@ -2,17 +2,12 @@ from urllib.parse import urljoin, urlparse import requests +from django.conf import settings from mitol.common.decorators import single_task from cms.api import create_featured_items from cms.models import Page from main.celery import app -from main.settings import ( - MITX_ONLINE_FASTLY_AUTH_TOKEN, - MITX_ONLINE_FASTLY_SERVICE_ID, - MITX_ONLINE_FASTLY_URL, - SITE_BASE_URL, -) def call_fastly_purge_api(relative_url): @@ -21,23 +16,26 @@ def call_fastly_purge_api(relative_url): because it doesn't work for this - the version of it that works with the current API only allows you to purge *everything*, not individual pages.) + Purges by URL against MITxOnline's own site -- the target is identified by + the `host` header taken from SITE_BASE_URL, not by a Fastly service ID. + Args: - relative_url The relative URL to purge. Returns: - Dict of the response (resp.json), or False if there was an error. """ logger = logging.getLogger("fastly_purge") - netloc = urlparse(SITE_BASE_URL)[1] + netloc = urlparse(settings.SITE_BASE_URL)[1] headers = {"host": netloc} if relative_url != "*": headers["fastly-soft-purge"] = "1" - if MITX_ONLINE_FASTLY_AUTH_TOKEN: - headers["fastly-key"] = MITX_ONLINE_FASTLY_AUTH_TOKEN + if settings.FASTLY_AUTH_TOKEN: + headers["fastly-key"] = settings.FASTLY_AUTH_TOKEN - api_url = urljoin(MITX_ONLINE_FASTLY_URL, relative_url) + api_url = urljoin(settings.FASTLY_URL, relative_url) resp = requests.request("PURGE", api_url, headers=headers) # noqa: S113 @@ -98,47 +96,56 @@ def queue_fastly_full_purge(): @app.task -def queue_fastly_surrogate_key_purge(surrogate_key): +def queue_fastly_surrogate_key_purge(surrogate_key, service_id=None): """ Purges all Fastly cached responses tagged with the given surrogate key. Uses the Fastly purge-by-tag API: POST /service/{service_id}/purge/{surrogate_key} - This allows MIT Learn pages to declare which MITxOnline surrogate keys they - subscribe to (via the Surrogate-Key response header), and MITxOnline to - invalidate those pages when course/program data changes. + MIT Learn tags its product page responses with the MITxOnline surrogate keys + they depend on (via the Surrogate-Key response header), which lets MITxOnline + invalidate those pages when course/program data changes. The service is + therefore Learn's -- purging MITxOnline's own Fastly service would do nothing, + since it tags no responses with these keys. Key format: mitxonline:course: or mitxonline:program: Args: surrogate_key (str): The surrogate key to purge, e.g. "mitxonline:course:course-v1:MITx+6.00.1x" + service_id (str): The Fastly service ID whose cache should be purged. + Falls back to settings.MIT_LEARN_FASTLY_SERVICE_ID when omitted. """ logger = logging.getLogger("fastly_purge") - if not MITX_ONLINE_FASTLY_SERVICE_ID: + service_id = service_id or settings.MIT_LEARN_FASTLY_SERVICE_ID + + if not service_id: logger.warning( - "FASTLY_SERVICE_ID is not set; skipping surrogate key purge for %s", + "No Fastly service ID given; skipping surrogate key purge for %s. " + "Is MIT_LEARN_FASTLY_SERVICE_ID set?", surrogate_key, ) return False - if not MITX_ONLINE_FASTLY_AUTH_TOKEN: + if not settings.FASTLY_AUTH_TOKEN: logger.warning( "FASTLY_AUTH_TOKEN is not set; skipping surrogate key purge for %s", surrogate_key, ) return False - logger.info("Purging Fastly surrogate key: %s", surrogate_key) + logger.info( + "Purging Fastly surrogate key %s from service %s", surrogate_key, service_id + ) api_url = urljoin( - MITX_ONLINE_FASTLY_URL, - f"/service/{MITX_ONLINE_FASTLY_SERVICE_ID}/purge/{surrogate_key}", + settings.FASTLY_URL, + f"/service/{service_id}/purge/{surrogate_key}", ) headers = { - "Fastly-Key": MITX_ONLINE_FASTLY_AUTH_TOKEN, + "Fastly-Key": settings.FASTLY_AUTH_TOKEN, "fastly-soft-purge": "1", } diff --git a/cms/tasks_test.py b/cms/tasks_test.py new file mode 100644 index 0000000000..9e3592ba8a --- /dev/null +++ b/cms/tasks_test.py @@ -0,0 +1,124 @@ +"""Tests for cms.tasks""" + +import pytest +import responses + +from cms.tasks import call_fastly_purge_api, queue_fastly_surrogate_key_purge + +# Deliberately not the production default (https://api.fastly.com), so that +# reading these settings at module import time rather than call time would fail +# to match the registered responses. +FASTLY_URL = "https://fastly.test" +SITE_BASE_URL = "https://mitxonline.test" +FASTLY_AUTH_TOKEN = "fastly-token" # noqa: S105 + +LEARN_SERVICE_ID = "test-learn-service-id" +SURROGATE_KEY = "mitxonline:course:course-v1:MITx+6.00.1x" + + +@pytest.fixture +def fastly_settings(settings): + """ + Configure the Fastly settings the purge tasks read. + + MIT_LEARN_FASTLY_SERVICE_ID is set to a value the tests never register a + response for, so that a test meaning to exercise an explicitly passed + service ID cannot pass by falling back to settings. + """ + settings.FASTLY_URL = FASTLY_URL + settings.FASTLY_AUTH_TOKEN = FASTLY_AUTH_TOKEN + settings.SITE_BASE_URL = SITE_BASE_URL + settings.MIT_LEARN_FASTLY_SERVICE_ID = "unregistered-fallback-service-id" + return settings + + +@responses.activate +def test_queue_fastly_surrogate_key_purge_targets_given_service(fastly_settings): + """An explicitly passed service ID takes precedence over the setting.""" + purge = responses.add( + responses.POST, + f"{FASTLY_URL}/service/{LEARN_SERVICE_ID}/purge/{SURROGATE_KEY}", + json={"status": "ok"}, + status=200, + ) + + assert queue_fastly_surrogate_key_purge(SURROGATE_KEY, LEARN_SERVICE_ID) is True + + assert purge.call_count == 1 + assert responses.calls[0].request.headers["Fastly-Key"] == FASTLY_AUTH_TOKEN + + +@responses.activate +def test_queue_fastly_surrogate_key_purge_falls_back_to_settings(fastly_settings): + """ + Called with the surrogate key alone, the task purges Learn's service anyway. + + This is the shape of a message enqueued by a release that does not pass + service_id, so the fallback is what keeps purges working across a rolling + deploy rather than silently skipping them. + """ + fastly_settings.MIT_LEARN_FASTLY_SERVICE_ID = LEARN_SERVICE_ID + purge = responses.add( + responses.POST, + f"{FASTLY_URL}/service/{LEARN_SERVICE_ID}/purge/{SURROGATE_KEY}", + json={"status": "ok"}, + status=200, + ) + + assert queue_fastly_surrogate_key_purge(SURROGATE_KEY) is True + + assert purge.call_count == 1 + + +@responses.activate +def test_queue_fastly_surrogate_key_purge_skips_without_service_id(fastly_settings): + """ + With no service ID passed and none configured, the purge is skipped. + + It must skip rather than raise or request `/service/None/purge/...`. + """ + fastly_settings.MIT_LEARN_FASTLY_SERVICE_ID = None + + assert queue_fastly_surrogate_key_purge(SURROGATE_KEY) is False + assert not responses.calls + + +@responses.activate +def test_queue_fastly_surrogate_key_purge_skips_without_auth_token(fastly_settings): + """A missing auth token skips the purge rather than sending it unauthenticated.""" + fastly_settings.FASTLY_AUTH_TOKEN = None + + assert queue_fastly_surrogate_key_purge(SURROGATE_KEY, LEARN_SERVICE_ID) is False + assert not responses.calls + + +@responses.activate +def test_queue_fastly_surrogate_key_purge_returns_false_on_error(fastly_settings): + """A Fastly error response is reported as a failure rather than swallowed.""" + responses.add( + responses.POST, + f"{FASTLY_URL}/service/{LEARN_SERVICE_ID}/purge/{SURROGATE_KEY}", + status=503, + ) + + assert queue_fastly_surrogate_key_purge(SURROGATE_KEY, LEARN_SERVICE_ID) is False + + +@responses.activate +def test_call_fastly_purge_api_targets_mitxonline_by_host(fastly_settings): + """ + The URL purge identifies MITxOnline's own site by `host`, not by service ID. + + Covers the three settings this helper reads, none of which are exercised + elsewhere. + """ + purge = responses.add( + responses.Response(method="PURGE", url=f"{FASTLY_URL}/catalog/", json={}) + ) + + call_fastly_purge_api("/catalog/") + + assert purge.call_count == 1 + sent_headers = responses.calls[0].request.headers + assert sent_headers["host"] == "mitxonline.test" + assert sent_headers["fastly-key"] == FASTLY_AUTH_TOKEN diff --git a/main/settings.py b/main/settings.py index cc8c7c3071..76b74a8c78 100644 --- a/main/settings.py +++ b/main/settings.py @@ -1350,22 +1350,36 @@ # Fastly configuration -MITX_ONLINE_FASTLY_AUTH_TOKEN = get_string( - name="FASTLY_AUTH_TOKEN", +# MITxOnline's own Fastly config follows the convention used throughout this file: +# the MITX_ONLINE_-prefixed name is the *env var*, the unprefixed one the setting +# (as EMAIL_BACKEND is read from MITX_ONLINE_EMAIL_BACKEND, and 16 others). +# These three previously inverted that -- prefixed settings reading bare env vars -- +# which is how the env vars came to be renamed to the setting names, breaking the +# read entirely: https://github.com/mitodl/ol-infrastructure/pull/5119 +FASTLY_AUTH_TOKEN = get_string( + name="MITX_ONLINE_FASTLY_AUTH_TOKEN", default=None, description="Optional token for the Fastly purge API.", ) -MITX_ONLINE_FASTLY_URL = get_string( - name="FASTLY_URL", +FASTLY_URL = get_string( + name="MITX_ONLINE_FASTLY_URL", default="https://api.fastly.com", description="The URL to the Fastly API.", ) -MITX_ONLINE_FASTLY_SERVICE_ID = get_string( - name="FASTLY_SERVICE_ID", +# Not MITX_ONLINE_-prefixed, because the value is not MITxOnline's: MIT_LEARN_* is +# this file's namespace for MIT Learn config, and those seven settings all use the +# env var name unchanged. +MIT_LEARN_FASTLY_SERVICE_ID = get_string( + name="MIT_LEARN_FASTLY_SERVICE_ID", default=None, - description="Fastly service ID used for surrogate key (tag) purging.", + description=( + "Fastly service ID for the MIT Learn frontend, used for surrogate key " + "(tag) purging. MIT Learn tags its product page responses with MITxOnline " + "surrogate keys, so this is Learn's service ID -- not the service ID of " + "MITxOnline's own Fastly service." + ), ) # Hubspot sync settings