feat(ocw-studio): drop the nginx sidecar and let APISix talk to Granian directly

docs/plans/remove-nginx-sidecar.md

@@ -13,10 +13,12 @@ precedent this plan generalizes.
 
 ## What the sidecar actually does, per app
 
+(Struck-through rows no longer have a sidecar.)
+
 | | static from | `/media` | `hash.txt` | X-Forwarded-Proto | body cap | other |
 |---|---|---|---|---|---|---|
-| odl_video_service | *nothing* | — | — | passthrough | 500M | 5 YouTube redirects |
-| ocw_studio | `/src/staticfiles` | — | yes | passthrough | 25M | |
+| ~~odl_video_service~~ | *nothing* | — | — | passthrough | 500M | 5 YouTube redirects |
+| ~~ocw_studio~~ | `/src/staticfiles` | — | yes | passthrough | 25M | |
 | xpro | `/src/staticfiles` | — | yes | passthrough | 25M | |
 | micromasters | `$uri`, `/src/staticfiles` | — | yes | passthrough | 25M | |
 | learn_ai | `$uri`, `/src/staticfiles` | — | — | *unset* | — | `proxy_buffering off` |
@@ -74,28 +76,59 @@ sidecar removes the cap. Any app that relies on it needs an explicit
 ## Component gaps in `GranianConfig`
 
 `static_path_mounts` alone does not cover what the six static-serving apps need.
-Granian's CLI (verified against `granian/cli.py`) exposes `--static-path-mount`
-(repeatable), `--static-path-route` (repeatable, default `/static`) and
-`--static-path-expires` (default `86400`). `GranianConfig` only wires up the
-first. Concretely:
+Granian's CLI (verified against `granian/cli.py` at v2.7.4, the version in the
+ocw-studio image) exposes `--static-path-mount` (repeatable),
+`--static-path-route` (repeatable, default `/static`) and
+`--static-path-expires` (default `86400`). Concretely:
 
 1. **No `static_path_routes`.** mit_learn serves `/media/*` from
-   `/src/django_media`; that needs a second route/mount pair.
-2. **No `static_path_expires`.** Granian's 1-day default is a large regression
-   from nginx's `expires max` (10 years) on content-hashed assets.
+   `/src/django_media`; that needs a second route/mount pair. *Still open —
+   nothing before mit_learn needs it.*
+2. ~~**No `static_path_expires`.**~~ **Closed.** `GranianConfig.static_path_expires`
+   emits the flag; `STATIC_ASSET_MAX_AGE_SECONDS` (315360000) in
+   `bridge/lib/magic_numbers.py` is what nginx's `expires max` resolves to.
+   Granian's static handler emits `Cache-Control: max-age=<n>` and nothing else
+   (`src/files.rs`), so this reproduces the sidecar's directive but not its
+   legacy `Expires` header.
 3. **No per-file override.** `/static/hash.txt` (`expires -1`,
    `Cache-Control: private`) is not expressible in Granian. It needs an APISix
    route with `response-rewrite`, on ocw_studio, xpro, micromasters and
-   mitxonline.
-4. **No CORS header on Granian-served static.** Covered by the shared plugin
-   config where one is attached; must be verified per app rather than assumed.
+   mitxonline. *By design — this stays at the gateway.*
+4. **No CORS header on Granian-served static.** Confirmed: Granian sets no CORS
+   header at all. Apps with a shared plugin config get one from APISix; the rest
+   need an explicit `/static/*` route. Verify per app rather than assume.
 5. **Two-directory fallback unverified.** Five configs do
    `try_files $uri $uri/ /staticfiles/$1`, i.e. `/src/<uri>` *then*
    `/src/staticfiles/<uri>`. Whether two Granian mounts on the same route fall
    through to the second on a miss is not documented and needs a live test.
-
-`/.well-known/dnt-policy.txt → 204` needs one APISix route per app (or a decision
-to drop it).
+   *Still open — gates micromasters, learn_ai, mitxonline, mit_learn.*
+
+`/.well-known/dnt-policy.txt → 204` needs one APISix route per app. Kept rather
+than dropped: it costs one `mocking` plugin and keeps a crawled path off the
+Granian blocking-thread pool, where Django would answer it with a 404.
+
+### The named-port trap in `OLApisixHTTPRoute`
+
+Gateway API `backendRef.port` must be numeric, so
+`OLApisixHTTPRoute._resolve_backend_port` maps the port *name* `"http"` to a
+hardcoded `8071` — `DEFAULT_NGINX_PORT`. Apps on the Gateway API path
+(ocw_studio, xpro, learn_ai, and any other caller passing
+`application_lb_service_port_name`) therefore keep routing to the sidecar's port
+after the sidecar is gone, and 502 on every request, with nothing in the Pulumi
+diff to hint at it. `OLApplicationK8s` now publishes the resolved number as
+`application_lb_service_port`; every route on a sidecar-free app must use it.
+This does not affect the `ApisixRoute` CRD path (odl_video_service), which takes
+the number directly.
+
+### Probe ports follow the application port
+
+`OLApplicationK8sConfig.probe_configs` defaulted to a literal dict pinned to
+`DEFAULT_NGINX_PORT`, so every app dropping its sidecar had to restate all three
+probes just to change a port. It now defaults to `None` and the component builds
+them from the resolved application port via `default_probe_configs()`. Verified
+no-op: a `pulumi preview` of learn_ai (sidecar still on, default probes) shows no
+Deployment diff. The six apps that pass `probe_configs` explicitly are unchanged
+and still own the port they name.
 
 ## Proof of concept: odl_video_service
 
@@ -127,20 +160,57 @@ routes render with the intended priorities.
 
 ### Rollout note
 
-The Service port and the ApisixRoute `servicePort` change in the same update with
-no ordering guarantee between them, so there is a short window where the route
-points at a port the Service does not yet expose. Watch for 502s during the
-apply rather than trying to engineer around it.
+The Service port and the route's backend port (`servicePort` on ApisixRoute,
+`backendRefs[].port` on HTTPRoute) change in the same update with no ordering
+guarantee between them, so there is a short window where the route points at a
+port the Service does not yet expose. Watch for 502s during the apply rather
+than trying to engineer around it. This applies to every remaining app.
+
+## Stage 2: ocw_studio
+
+Done — see the accompanying commit. The first of the seven sidecar apps where
+Granian actually carries the `/static/*` traffic nginx used to serve (OVS had
+`dj_static.Cling` doing that already; edxapp LMS/Studio have been on Granian
+static in production since before this project started), and the first app on
+the Gateway API route path.
+
+- `STATIC_ROOT` is the *relative* `"staticfiles"`, resolved against the image's
+  `WORKDIR /src`, so it lands on the same `/src/staticfiles` emptyDir the
+  collectstatic init container populates. `STATIC_URL` is `/static/`, which is
+  Granian's default route, so no route override is needed.
+- `hash.txt` is load-bearing here, not vestigial: the Dockerfile writes
+  `$GIT_REF` into `/src/static/hash.txt` and `useAppVersionCheck` polls
+  `/static/hash.txt` on every route change to force a reload after a deploy. Its
+  APISix route sets `Cache-Control: private, no-cache`, which is what nginx's
+  `expires -1` plus `add_header Cache-Control private` produced. The fetch
+  already passes `cache: "no-store"`, so this only matters for intermediaries —
+  and ocw_studio sits directly behind APISix with no CDN, so there are none
+  today.
+- The `/static/*` CORS route is *not* redundant here: ocw_studio has no shared
+  plugin config, so without it the header the sidecar added simply disappears.
+- Probes and the Service port move 8071 → 8073 via the component; the four
+  HTTPRoute rules name `application_lb_service_port` explicitly (see the
+  named-port trap above).
+
+`pulumi preview --stack CI`: 3 creates (one `PluginConfig` per new route), 8
+updates, 1 delete (the `nginx-config` ConfigMap), 104 unchanged. The webapp
+container drops the nginx sidecar and picks up `--static-path-mount
+/src/staticfiles --static-path-expires 315360000`.
 
 ## Rollout order
 
-1. **odl_video_service** — no static block, no shared plugin config, lowest
-   traffic. Validates the port move, the redirect translation, and Granian static
-   serving in one go.
-2. **ocw_studio, xpro** — single static directory (`try_files /staticfiles/$1`),
-   so no fallback question. Need `hash.txt` and dnt-policy routes.
-3. **micromasters, learn_ai** — two-directory static fallback; gated on gap 5.
-4. **mit_learn, mitxonline** — highest traffic, and mit_learn additionally needs
+1. ~~**odl_video_service**~~ — done, PR #5281.
+2. ~~**ocw_studio**~~ — done, this commit.
+3. **xpro** — the other single-static-directory app, the same shape as
+   ocw_studio. Deliberately *not* bundled with it: PR #5344 is concurrently
+   changing xpro's Granian concurrency, and overlapping two live changes on one
+   app makes any regression ambiguous. Do it once #5344 has landed and settled.
+   Note xpro's `hash.txt` block has no `try_files`, so it resolves against
+   `root /src` to `/src/static/hash.txt` — the source dir, not the collectstatic
+   output. Granian will serve `/src/staticfiles/hash.txt` instead: same content,
+   different file.
+4. **micromasters, learn_ai** — two-directory static fallback; gated on gap 5.
+5. **mit_learn, mitxonline** — highest traffic, and mit_learn additionally needs
    the `/media` route (gap 1) and the JSON gzip moved to the APISix `gzip` plugin.
 
 ## Out of scope

src/bridge/lib/magic_numbers.py

@@ -65,6 +65,11 @@
 ONE_MEGABYTE_BYTE = 1048576
 ONE_MONTH_SECONDS = 60 * 60 * HOURS_IN_MONTH
 SECONDS_IN_ONE_DAY = 86400
+# What nginx's `expires max` resolves to (10 years). The Django apps served their
+# content-hashed static assets with that directive behind the nginx sidecar, so
+# apps moving static onto Granian pass this as --static-path-expires rather than
+# silently dropping to Granian's own 1-day default.
+STATIC_ASSET_MAX_AGE_SECONDS = 315360000
 VAULT_CLUSTER_PORT = 8201
 VAULT_HTTP_PORT = 8200
 XQUEUE_SERVICE_PORT = 8040

src/ol_infrastructure/applications/ocw_studio/__main__.py

@@ -24,6 +24,7 @@
 from bridge.lib.magic_numbers import (
     DEFAULT_POSTGRES_PORT,
     DEFAULT_REDIS_PORT,
+    STATIC_ASSET_MAX_AGE_SECONDS,
 )
 from bridge.secrets.sops import read_yaml_secrets
 from ol_infrastructure.applications.ocw_studio.k8s_secrets import (
@@ -36,6 +37,7 @@
     OLMediaConvert,
 )
 from ol_infrastructure.components.aws.s3 import OLBucket, S3BucketConfig
+from ol_infrastructure.components.services.apisix import OLApisixPluginConfig
 from ol_infrastructure.components.services.apisix_gateway_api import (
     OLApisixHTTPRoute,
     OLApisixHTTPRouteConfig,
@@ -622,10 +624,26 @@
             # blocking threads, 16 backpressure, runtime defaults).
             blocking_threads_idle_timeout=120,
             enable_metrics=True,
+            # Serve /static/* from Granian's Rust layer instead of the sidecar.
+            # STATIC_ROOT is the relative path "staticfiles" against the image's
+            # WORKDIR of /src, i.e. the emptyDir the collectstatic init container
+            # populates, and STATIC_URL is "/static/" -- which is Granian's
+            # default static route, so no route override is needed.
+            static_path_mounts=["/src/staticfiles"],
+            # The sidecar served this directory with `expires max`; Granian would
+            # otherwise fall back to its own 1-day default.
+            static_path_expires=STATIC_ASSET_MAX_AGE_SECONDS,
         ),
         vault_k8s_resource_auth_name=vault_k8s_resources.auth_name,
         registry="dockerhub",
-        import_nginx_config=True,
+        # The sidecar's only real job here was serving /static/*, which Granian
+        # now does directly. What is left of it -- a /nginx-health endpoint
+        # nothing probes (the probes use django-health-check), an
+        # X-Forwarded-Proto re-forward of the header APISix already sets, and a
+        # 25M body cap APISix does not enforce anyway -- is either redundant or
+        # translated to the APISix routes below. See
+        # docs/plans/remove-nginx-sidecar.md.
+        import_nginx_config=False,
         init_migrations=True,
         init_collectstatic=True,
         pre_deploy_commands=[
@@ -685,15 +703,104 @@
     ),
 )
 
+# The `location` blocks that used to live in the nginx sidecar. HTTPRoute has no
+# priority field -- APISix resolves overlapping rules by longest matching path
+# prefix -- so /static/hash.txt outranks /static, which outranks /*, which is the
+# same ordering nginx applied.
+#
+# Every rule names the numeric Service port rather than the "http" port name:
+# OLApisixHTTPRoute has to give Gateway API a number and maps that name to the
+# nginx sidecar's 8071, which this app no longer listens on.
 ocw_studio_apisix_httproute = OLApisixHTTPRoute(
     f"ocw-studio-apisix-httproute-{stack_info.env_suffix}",
     route_configs=[
+        # hash.txt carries the deployed git ref and is polled by
+        # useAppVersionCheck to force a reload after a deploy, so it is the one
+        # file under /static that must not inherit the 10-year max-age Granian
+        # now stamps on that directory. nginx did this with `expires -1` plus an
+        # `add_header Cache-Control private`.
+        #
+        # Left as a prefix match, unlike dnt-policy below: nginx matched this one
+        # with a `location ~*` regex, which caught descendants too, so a prefix
+        # is the closer translation.
+        OLApisixHTTPRouteConfig(
+            route_name="static-hash",
+            hosts=[app_domain],
+            paths=["/static/hash.txt"],
+            backend_service_name=ocw_studio_k8s_app.application_lb_service_name,
+            backend_service_port=ocw_studio_k8s_app.application_lb_service_port,
+            backend_import_nginx_config=False,
+            plugins=[
+                OLApisixPluginConfig(
+                    name="response-rewrite",
+                    secretRef=None,
+                    config={"headers": {"set": {"Cache-Control": "private, no-cache"}}},
+                ),
+            ],
+        ),
+        # Granian serves static without a CORS header; the sidecar added a
+        # blanket one. Preserved rather than dropped because this app has no
+        # shared plugin config supplying `cors`, so removing the sidecar would
+        # otherwise silently take the header away from cross-origin font and
+        # asset loads.
+        OLApisixHTTPRouteConfig(
+            route_name="static",
+            hosts=[app_domain],
+            paths=["/static/*"],
+            backend_service_name=ocw_studio_k8s_app.application_lb_service_name,
+            backend_service_port=ocw_studio_k8s_app.application_lb_service_port,
+            backend_import_nginx_config=False,
+            plugins=[
+                OLApisixPluginConfig(
+                    name="response-rewrite",
+                    secretRef=None,
+                    config={
+                        "headers": {"set": {"Access-Control-Allow-Origin": "*"}},
+                    },
+                ),
+            ],
+        ),
+        # A 204 here is the EFF Do Not Track convention for "no policy
+        # published". Kept as a mock so the crawlers that request it are
+        # answered at the gateway rather than burning a Granian blocking thread
+        # on a Django 404.
+        #
+        # Exact, not prefix: this replaces an nginx `location = ` block, so
+        # /.well-known/dnt-policy.txt/anything must fall through to Django
+        # rather than collect a mocked 204.
+        #
+        # The empty response_example is not decoration -- the APISix mocking
+        # plugin schema is `anyOf: [required: response_example, required:
+        # response_schema]`, so a config with neither fails validation and the
+        # route never serves anything.
+        OLApisixHTTPRouteConfig(
+            route_name="dnt-policy",
+            hosts=[app_domain],
+            paths=["/.well-known/dnt-policy.txt"],
+            path_match_type="Exact",
+            backend_service_name=ocw_studio_k8s_app.application_lb_service_name,
+            backend_service_port=ocw_studio_k8s_app.application_lb_service_port,
+            backend_import_nginx_config=False,
+            plugins=[
+                OLApisixPluginConfig(
+                    name="mocking",
+                    secretRef=None,
+                    config={
+                        "response_status": 204,
+                        "response_example": "",
+                        "content_type": "text/plain",
+                        "with_mock_header": False,
+                    },
+                ),
+            ],
+        ),
         OLApisixHTTPRouteConfig(
             route_name="passthrough",
             hosts=[app_domain],
             paths=["/*"],
             backend_service_name=ocw_studio_k8s_app.application_lb_service_name,
-            backend_service_port=ocw_studio_k8s_app.application_lb_service_port_name,
+            backend_service_port=ocw_studio_k8s_app.application_lb_service_port,
+            backend_import_nginx_config=False,
             plugins=[],
         ),
     ],