Skip to content

fix: read Learn's Fastly service ID from MIT_LEARN_FASTLY_SERVICE_ID - #3794

Merged
ChristopherChudzicki merged 2 commits into
mainfrom
fix/fastly-learn-service-id-env-var
Jul 31, 2026
Merged

fix: read Learn's Fastly service ID from MIT_LEARN_FASTLY_SERVICE_ID#3794
ChristopherChudzicki merged 2 commits into
mainfrom
fix/fastly-learn-service-id-env-var

Conversation

@ChristopherChudzicki

@ChristopherChudzicki ChristopherChudzicki commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

What are the relevant tickets?

Refs https://github.com/mitodl/hq/issues/12381

Description (What does it do?)

  1. Renames the Fastly token and URL settings to read the env vars set by fix: Set the proper env var for MITx Online to pick up the Fastly settings ol-infrastructure#5119 switched to setting (MITX_ONLINE_FASTLY_*),
    • ☝️ renamed env vars somewhat erroneously, but the new names are more consistent with the rest of the env vars used in the repo.
  2. Introduces MIT_LEARN_FASTLY_SERVICE_ID, replacing MITX_ONLINE_FASTLY_SERVICE_ID. The value is Learn's service ID — Learn is what tags its product pages with the mitxonline:* surrogate keys, so purging MITxOnline's own service does nothing. The old name invited exactly that mistake.
    • No live code was using MITX_ONLINE_FASTLY_SERVICE_ID in a way that worked.
  3. Makes service_id an explicit argument to queue_fastly_surrogate_key_purge, so a purge's target is visible at the call site. Defaults to None so messages enqueued by the previous release skip rather than raise mid-deploy.

Also reads these via django.conf.settings instead of importing from main.settings at module import time, which is what hid them from Django's settings fixture in tests.

How can this be tested?

Tests should pass. Not easily testable locally otherwise.

Additional Context

Safe to deploy alone:

It starts working when the infra change below lands.

Soft → hard purge is deliberately excluded, so this infra change can be validated in RC while soft purge is still in place.

Checklist:

  • Add MIT_LEARN_FASTLY_SERVICE_ID to mitxonline/k8s_secrets.py, per environment. Values already exist as learn_service_id_{ci,qa,production} under infrastructure/fastly in src/bridge/secrets/concourse/operations.production.yaml. Not secret-mitxonline's fastly.service_id — that's MITxOnline's own service, and using it is what made the original purge a no-op.
  • Don't restore FASTLY_SERVICE_ID; nothing reads it now.
  • Confirm the token is authorized for Learn's service — purge-by-key needs purge_select on the target service, and this token comes from MITxOnline's Vault path.

@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown

OpenAPI Changes

Show/hide changes
## Changes for v0.yaml:
No changes detected

## Changes for v1.yaml:
No changes detected

## Changes for v2.yaml:
No changes detected

Unexpected changes? Ensure your branch is up-to-date with main (consider rebasing).

The surrogate key purge targets MIT Learn's Fastly service, because MIT
Learn is what tags its product page responses with the mitxonline:*
surrogate keys. The setting holding that service ID was named
MITX_ONLINE_FASTLY_SERVICE_ID and read from a bare FASTLY_SERVICE_ID env
var, which reads as "MITxOnline's own service" and inverts this repo's
convention -- elsewhere the app-prefixed name is the env var (EMAIL_BACKEND
is read from MITX_ONLINE_EMAIL_BACKEND, and 16 others). That inversion is
how the env vars came to be renamed to the setting names in
mitodl/ol-infrastructure#5119, leaving both
settings unset in production and the purge a silent no-op.

Rename to MIT_LEARN_FASTLY_SERVICE_ID, matching the existing MIT_LEARN_*
namespace, whose seven settings all use the env var name unchanged -- the
prefix names the external system, so no app prefix applies. The token and
API URL now follow the app-prefixed convention instead, reading
MITX_ONLINE_FASTLY_AUTH_TOKEN and MITX_ONLINE_FASTLY_URL, which is what the
deployment already sets. So the infra follow-up only has to add the service
ID; nothing has to be reverted.

Pass the service ID into queue_fastly_surrogate_key_purge explicitly rather
than reading it from module state, so the target of a purge is visible at
the call site instead of being an unqualified global in the MITxOnline repo.
It defaults to None so messages enqueued by the previous release skip the
purge rather than raising during a rolling deploy. Read the token and URL
through django.conf.settings, removing the module-import-time coupling that
made these values invisible to Django's settings fixture in tests.

Adds cms/tasks_test.py and cms/signals_test.py; neither module had tests.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ChristopherChudzicki
ChristopherChudzicki force-pushed the fix/fastly-learn-service-id-env-var branch from 905c9a4 to 8a6457e Compare July 29, 2026 01:11
@Ardiea Ardiea self-assigned this Jul 29, 2026
Ardiea added a commit to mitodl/ol-infrastructure that referenced this pull request Jul 29, 2026
The mitxonline Fastly surrogate-key purge must target MIT Learn's Fastly
service (MIT Learn tags product-page responses with the mitxonline:*
surrogate keys), not MITxOnline's own service. Add a new
MIT_LEARN_FASTLY_SERVICE_ID env var, sourced per-environment from a new
fastly.mit_learn_service_id key in each mitxonline SOPS secrets file
(values copied from learn_service_id_{ci,qa,production} under
infrastructure/fastly in operations.production.yaml).

Supports mitodl/mitxonline#3794.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@marslanabdulrauf marslanabdulrauf self-assigned this Jul 30, 2026

@marslanabdulrauf marslanabdulrauf left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a couple of findings:

  • Rolling deploy is handled in one direction only
  • Minor cms/signals_test.py calls purge_fastly_cache_on_publish directly rather than firing page_published

Comment thread cms/tasks.py Outdated
Comment on lines +118 to +121
service_id (str): The Fastly service ID whose cache should be purged.
Callers pass settings.MIT_LEARN_FASTLY_SERVICE_ID. Defaults to None
so that messages enqueued by an older release, before this argument
existed, skip the purge rather than raising during a rolling deploy.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fix is one sided and will always work if the celery is the one to be deployed first.
The other way is if we started passing two positional arguments and the function only accept one.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point.

I do want to keep the service id passing explicit, since i think it's somewhat unexpected that mitxonline's queue_fastly_surrogate_key_purge purges a different service id. But I made the passing optional, and a follow PR, stacked on this one, makes it required.

Comment thread cms/signals_test.py Outdated
Keeps the enqueued Celery message a single positional argument so a
rolling deploy is safe in both directions: an old webapp's one-arg
message runs on a new worker and still purges via the fallback, and a
new webapp's one-arg message runs on an old worker unchanged. Passing
service_id explicitly from the call sites is #3804, once every worker
accepts the argument.

Also drives cms/signals_test.py through Wagtail's real publish rather
than calling the receiver directly, so the page_published.connect()
wiring is covered.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ChristopherChudzicki
ChristopherChudzicki force-pushed the fix/fastly-learn-service-id-env-var branch from 36fcddc to 221cae9 Compare July 30, 2026 14:57

@marslanabdulrauf marslanabdulrauf left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ChristopherChudzicki
ChristopherChudzicki merged commit f104673 into main Jul 31, 2026
14 checks passed
@ChristopherChudzicki
ChristopherChudzicki deleted the fix/fastly-learn-service-id-env-var branch July 31, 2026 13:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants