spikerj/spikersoft-issues
1
0
Fork 0
Code Issues 196 Pull Requests Actions Packages Projects Releases Wiki Activity

[Backend][MinIO] Replace API static-file media serving with S3-backed delivery (epic #413) #528

New Issue
Closed
opened 2026-07-13 00:40:41 +00:00 by spikerj · 5 comments
spikerj commented 2026-07-13 00:40:41 +00:00
Owner
Copy Link
Copy Source

Parent epic: #413.

Scope: the four API static-file mounts in SpikerSoft.Api/Extensions/WebApplicationExtensions.cs:69-76:

  • Blogs:Path /app/blogs at /blog-pictures
  • Books:Path /app/ebooks at /ebooks
  • Media:Path /app/uploads/media at /geography-media
  • Videos:Path /app/lesson-videos at /lesson-videos

Goal: replace disk-backed UseStaticFiles with S3-backed delivery so the backend API loses its media binds. Decide per route: streaming proxy endpoint (keeps URLs stable, adds backend CPU/IO) vs presigned redirects (cheapest, changes response semantics — check Angular consumers). Note SpikerSoft.Data/Mongos/BookPageImage.cs:46 stores absolute /app/ebooks/... paths in Mongo — needs a path→key mapping shim (Storage:PathPrefix strip, same trick as S3StagedObjectStore.TryMapKey).

Unblocks: the Angular nginx root-bind migration (which serves the same tree from nginx today).

Parent epic: #413. **Scope:** the four API static-file mounts in `SpikerSoft.Api/Extensions/WebApplicationExtensions.cs:69-76`: - `Blogs:Path` `/app/blogs` at `/blog-pictures` - `Books:Path` `/app/ebooks` at `/ebooks` - `Media:Path` `/app/uploads/media` at `/geography-media` - `Videos:Path` `/app/lesson-videos` at `/lesson-videos` **Goal:** replace disk-backed `UseStaticFiles` with S3-backed delivery so the backend API loses its media binds. Decide per route: streaming proxy endpoint (keeps URLs stable, adds backend CPU/IO) vs presigned redirects (cheapest, changes response semantics — check Angular consumers). Note `SpikerSoft.Data/Mongos/BookPageImage.cs:46` stores absolute `/app/ebooks/...` paths in Mongo — needs a path→key mapping shim (`Storage:PathPrefix` strip, same trick as `S3StagedObjectStore.TryMapKey`). Unblocks: the Angular nginx root-bind migration (which serves the same tree from nginx today).
spikerj commented 2026-07-13 20:20:07 +00:00
Author
Owner
Copy Link
Copy Source

Backend PR #263: S3MediaFallbackMiddleware behind the four disk mounts — streaming proxy chosen over presigned (URLs and semantics unchanged → no Angular work; presign stays a later optimization). Range pass-through for video seek via new IObjectStore.OpenReadRangeAsync. Dual-run = byte-for-byte today; cutover = drop binds, same URLs. 13/13 middleware tests. This unblocks #531.

Backend PR #263: S3MediaFallbackMiddleware behind the four disk mounts — streaming proxy chosen over presigned (URLs and semantics unchanged → no Angular work; presign stays a later optimization). Range pass-through for video seek via new IObjectStore.OpenReadRangeAsync. Dual-run = byte-for-byte today; cutover = drop binds, same URLs. 13/13 middleware tests. This unblocks #531.
spikerj commented 2026-07-13 20:33:04 +00:00
Author
Owner
Copy Link
Copy Source

Code MERGED (backend #263 — S3MediaFallbackMiddleware behind the four disk mounts, Range pass-through). Live but inert until the API stack runs Storage__UseS3=true (infra #56 merged the env; needs api-svc key with read on blogs/ebooks/uploads/lesson-videos + API_S3_SECRET_KEY secret). Bind drop rides #529 phase 2; #531 (angular #178) is now unblocked.

Code MERGED (backend #263 — S3MediaFallbackMiddleware behind the four disk mounts, Range pass-through). Live but inert until the API stack runs Storage__UseS3=true (infra #56 merged the env; needs api-svc key with read on blogs/ebooks/uploads/lesson-videos + API_S3_SECRET_KEY secret). Bind drop rides #529 phase 2; #531 (angular #178) is now unblocked.
spikerj commented 2026-07-14 21:55:54 +00:00
Author
Owner
Copy Link
Copy Source

Findings — the code half is done and live; the data half is not. Keeping this open.

Worked this ticket today. The headline: #528's implementation already merged and is running in production (PR spikersoft-backend#263, S3MediaFallbackMiddleware), but the ticket's actual goal — "the backend API loses its media binds" — cannot be done yet, and it is important that nobody assumes it can.

What is already true (verified, not assumed)

  • S3MediaFallbackMiddleware serves all four routes from S3, with a route table mapping /blog-pictures→blogs, /ebooks→ebooks, /geography-media→uploads (key prefix media), /lesson-videos→lesson-videos.
  • It is enabled in production: spikersoft-infrastructure/spikersoft-backend/docker-stack.yml:23 sets Storage__UseS3=true with the api-svc key, and the deploy job refuses to deploy on an empty API_S3_SECRET_KEY (good — that is the #592 failure mode, already guarded here).
  • The decision the ticket asked for was made: streaming proxy, not presigned redirects. That is the right call — every Angular consumer binds these URLs straight into <img src> / <video src> / poster with no rewriting layer, and the native <video> player relies on Range requests for seeking. A 302 to a presigned URL adds seek amplification + mid-playback expiry risk for long videos and would need MinIO-side CORS (which does not exist).

Proof that the seam is correct

The existing unit suite mocks IObjectStore, so it cannot see what a real S3 server decides. I added a real-MinIO integration suite — spikersoft-backend PR #294 (proof only, no production code changed):

12/12  SpikerSoft.Tests.Integration  (real MinIO, production image, path-style)
13/13  S3MediaFallbackMiddlewareTests (unit)
34/34  SpikerSoft.Storage.Tests
       dotnet build SpikerSoft.UnitTests.slnf -> 0 Error(s)

It drives the production middleware order (four disk mounts, then the S3 fallback) over a live bucket and pins: 206/Content-Range with exact bytes, suffix ranges, 416, HEAD, the media key prefix, traversal containment, and the ServeUnknownTypes split. The load-bearing case is DiskMount_TakesPrecedenceOverBucket_DuringDualRun — #528 ships the buckets behind the binds, so a wrong-way precedence bug would silently serve stale bucket bytes over live disk bytes. Nothing asserted that before.

Why this stays OPEN — three blockers to removing the binds

Because the middleware is a fallback behind the disk mounts, every S3 miss is currently invisible. Drop the binds today and those misses become user-visible 404s:

  1. The lesson-videos bucket is empty — guaranteed. Its only writer, lesson-video-processor, has never deployed (#581: VIDEOS_S3_SECRET_KEY was never created, so its own deploy guard rejects every run). Nothing has ever written that bucket.
  2. /geography-media has no writer at all. /app/uploads/media is a static tree; StagingMirror only mirrors files flowing through the upload lifecycle. Only a backfill can populate it.
  3. The Phase 0 mc mirror backfill appears never to have run. Nothing in the repo, runbooks, or tickets records it as done.

Additionally — a cutover-day landmine that is not tracked anywhere: SpikerSoft.EventHandlers.UploadCoordinator/Services/UploadOrchestrator.cs:891 does a raw File.Exists(image.AbsoluteImagePath) on /app/ebooks/.... Dropping the ebooks bind makes that silently return false — book page images get marked missing and descriptions never generate. Filing separately.

Recommended next steps (in order)

  1. Fix #581 (issue VIDEOS_S3_SECRET_KEY) so lesson-video-processor can actually deploy and start writing its bucket.
  2. Run Phase 0: create/verify buckets, then mc mirror the four trees; confirm with mc du against du -sh on disk.
  3. Route AbsoluteImagePath consumers through IObjectStore (separate ticket).
  4. Then drop the binds as part of the #529 coordinated batch — not before.

Doc correctness note

docs/minio-storage-migration.md:3 says "MinIO deployed, zero services migrated" and .claude/rules/infrastructure-reality.md repeats "No service has migrated yet." Both are now false and actively misleading — 8+ services carry S3 code with Storage__UseS3=true. The accurate map is docs/uploads-batch-cutover-runbook.md. Worth correcting before someone plans a cutover off the stale line.

## Findings — the code half is done and live; the data half is not. Keeping this open. Worked this ticket today. The headline: **#528's implementation already merged and is running in production** (PR spikersoft-backend#263, `S3MediaFallbackMiddleware`), but the ticket's actual goal — *"the backend API loses its media binds"* — **cannot be done yet**, and it is important that nobody assumes it can. ### What is already true (verified, not assumed) - `S3MediaFallbackMiddleware` serves all four routes from S3, with a route table mapping `/blog-pictures`→`blogs`, `/ebooks`→`ebooks`, `/geography-media`→`uploads` (key prefix `media`), `/lesson-videos`→`lesson-videos`. - It is **enabled in production**: `spikersoft-infrastructure/spikersoft-backend/docker-stack.yml:23` sets `Storage__UseS3=true` with the `api-svc` key, and the deploy job refuses to deploy on an empty `API_S3_SECRET_KEY` (good — that is the #592 failure mode, already guarded here). - The decision the ticket asked for was made: **streaming proxy, not presigned redirects.** That is the right call — every Angular consumer binds these URLs straight into `<img src>` / `<video src>` / `poster` with no rewriting layer, and the native `<video>` player relies on Range requests for seeking. A 302 to a presigned URL adds seek amplification + mid-playback expiry risk for long videos and would need MinIO-side CORS (which does not exist). ### Proof that the seam is correct The existing unit suite mocks `IObjectStore`, so it cannot see what a real S3 server decides. I added a real-MinIO integration suite — **spikersoft-backend PR #294** (proof only, no production code changed): ``` 12/12 SpikerSoft.Tests.Integration (real MinIO, production image, path-style) 13/13 S3MediaFallbackMiddlewareTests (unit) 34/34 SpikerSoft.Storage.Tests dotnet build SpikerSoft.UnitTests.slnf -> 0 Error(s) ``` It drives the production middleware order (four disk mounts, then the S3 fallback) over a live bucket and pins: 206/`Content-Range` with exact bytes, suffix ranges, 416, HEAD, the `media` key prefix, traversal containment, and the `ServeUnknownTypes` split. The load-bearing case is **`DiskMount_TakesPrecedenceOverBucket_DuringDualRun`** — #528 ships the buckets *behind* the binds, so a wrong-way precedence bug would silently serve stale bucket bytes over live disk bytes. Nothing asserted that before. ### Why this stays OPEN — three blockers to removing the binds Because the middleware is a **fallback behind the disk mounts, every S3 miss is currently invisible.** Drop the binds today and those misses become user-visible 404s: 1. **The `lesson-videos` bucket is empty — guaranteed.** Its only writer, lesson-video-processor, has **never deployed** (#581: `VIDEOS_S3_SECRET_KEY` was never created, so its own deploy guard rejects every run). Nothing has ever written that bucket. 2. **`/geography-media` has no writer at all.** `/app/uploads/media` is a static tree; `StagingMirror` only mirrors files flowing through the upload lifecycle. Only a backfill can populate it. 3. **The Phase 0 `mc mirror` backfill appears never to have run.** Nothing in the repo, runbooks, or tickets records it as done. Additionally — **a cutover-day landmine that is not tracked anywhere**: `SpikerSoft.EventHandlers.UploadCoordinator/Services/UploadOrchestrator.cs:891` does a raw `File.Exists(image.AbsoluteImagePath)` on `/app/ebooks/...`. Dropping the ebooks bind makes that silently return false — book page images get marked missing and descriptions never generate. Filing separately. ### Recommended next steps (in order) 1. Fix #581 (issue `VIDEOS_S3_SECRET_KEY`) so lesson-video-processor can actually deploy and start writing its bucket. 2. Run Phase 0: create/verify buckets, then `mc mirror` the four trees; confirm with `mc du` against `du -sh` on disk. 3. Route `AbsoluteImagePath` consumers through `IObjectStore` (separate ticket). 4. *Then* drop the binds as part of the #529 coordinated batch — not before. ### Doc correctness note `docs/minio-storage-migration.md:3` says *"MinIO deployed, zero services migrated"* and `.claude/rules/infrastructure-reality.md` repeats *"No service has migrated yet."* **Both are now false and actively misleading** — 8+ services carry S3 code with `Storage__UseS3=true`. The accurate map is `docs/uploads-batch-cutover-runbook.md`. Worth correcting before someone plans a cutover off the stale line.
spikerj commented 2026-07-14 22:39:00 +00:00
Author
Owner
Copy Link
Copy Source

Update — Phase 0 backfill is now actually run, and one of the two blockers is fixed.

Following the findings above, we did the work on SERVER rather than just documenting it.

Phase 0 backfill — DONE

We found 6 of the 7 buckets did not exist — mc mb had never been run, so the whole migration was blocked at step 1. Ran Phase 0 via a one-shot minio/mc job pinned to SERVER (additive only: mc mb -p + mc mirror --overwrite, no --remove, disk mounted read-only). Result, verified with mc du vs du -sh:

bucket before after disk
ebooks 1.1 GiB / 1403 obj 1.6 GiB / 2301 obj 1.7 G ✓
blogs did not exist 2.3 MiB / 3 obj 2.3 M ✓
uploads 51 MiB / 7 obj unchanged 200 K
lesson-videos, dlls, assets, quarantine, reference-data did not exist created, 0 obj empty on disk ✓

The zero-object buckets are correct — those trees are empty on disk. Note there are no lesson videos anywhere (disk or bucket), which independently confirms #581: nothing has ever produced one, because lesson-video-processor has never deployed.

Blocker #598 — FIXED (spikersoft-backend PR #297)

The UploadOrchestrator raw-File.Exists on /app/ebooks/... that would fail books as "corrupt" once the bind drops now goes through an IBookImageLocator that falls back to the ebooks bucket (filesystem-first, same contract the Python image-description worker already uses). 19/19 tests, UnitTests.slnf clean.

What still blocks the bind removal

  1. #581 — lesson-videos bucket is empty because its writer never deployed. Fix the VIDEOS_S3_SECRET_KEY secret so it can start writing before anyone drops the lesson-videos bind.
  2. Scoped-key policies — before #598 can be enabled, uploads-svc needs read on ebooks (runbook Phase 0 step 2), and #522 needs blogs-svc write on blogs confirmed. Buckets existing ≠ keys authorized; a 403 is as silent as a missing bucket was.
  3. #599 — the /lesson-videos and /geography-media URLs are root-relative and resolve against nginx, not the API, so the media routes are unreachable from the SPA regardless of what's in the buckets.

Net: the storage half is now real (buckets populated) and one code blocker is fixed. The remaining blockers are secret/policy provisioning (#581, key policies) and the URL-origin bug (#599) — none in the #528 middleware itself, which our integration suite (PR #294) proves correct.

**Update — Phase 0 backfill is now actually run, and one of the two blockers is fixed.** Following the findings above, we did the work on SERVER rather than just documenting it. ## Phase 0 backfill — DONE We found **6 of the 7 buckets did not exist** — `mc mb` had never been run, so the whole migration was blocked at step 1. Ran Phase 0 via a one-shot `minio/mc` job pinned to SERVER (additive only: `mc mb -p` + `mc mirror --overwrite`, no `--remove`, disk mounted read-only). Result, verified with `mc du` vs `du -sh`: | bucket | before | after | disk | |---|---|---|---| | ebooks | 1.1 GiB / 1403 obj | **1.6 GiB / 2301 obj** | 1.7 G ✓ | | blogs | *did not exist* | **2.3 MiB / 3 obj** | 2.3 M ✓ | | uploads | 51 MiB / 7 obj | unchanged | 200 K | | lesson-videos, dlls, assets, quarantine, reference-data | *did not exist* | **created, 0 obj** | empty on disk ✓ | The zero-object buckets are correct — those trees are empty on disk. Note **there are no lesson videos anywhere** (disk or bucket), which independently confirms #581: nothing has ever produced one, because lesson-video-processor has never deployed. ## Blocker #598 — FIXED (spikersoft-backend PR #297) The `UploadOrchestrator` raw-`File.Exists` on `/app/ebooks/...` that would fail books as "corrupt" once the bind drops now goes through an `IBookImageLocator` that falls back to the `ebooks` bucket (filesystem-first, same contract the Python image-description worker already uses). 19/19 tests, `UnitTests.slnf` clean. ## What still blocks the bind removal 1. **#581** — lesson-videos bucket is empty because its writer never deployed. Fix the `VIDEOS_S3_SECRET_KEY` secret so it can start writing before anyone drops the lesson-videos bind. 2. **Scoped-key policies** — before #598 can be enabled, `uploads-svc` needs **read on `ebooks`** (runbook Phase 0 step 2), and #522 needs `blogs-svc` **write on `blogs`** confirmed. Buckets existing ≠ keys authorized; a 403 is as silent as a missing bucket was. 3. **#599** — the `/lesson-videos` and `/geography-media` URLs are root-relative and resolve against nginx, not the API, so the media routes are unreachable from the SPA regardless of what's in the buckets. Net: the storage half is now real (buckets populated) and one code blocker is fixed. The remaining blockers are secret/policy provisioning (#581, key policies) and the URL-origin bug (#599) — none in the #528 middleware itself, which our integration suite (PR #294) proves correct.
spikerj commented 2026-07-18 18:00:25 +00:00
Author
Owner
Copy Link
Copy Source

Epic #413 accuracy pass (2026-07-18) — closing: all three named blockers from the 2026-07-14 findings are resolved; the sole remaining action is #529 phase 2 itself.

Where each blocker stands now (verified):

  1. lesson-videos bucket empty because its writer never deployed → #581 CLOSED (2026-07-15, backend #303 — key now fetched from OpenBao, deploy guard passes). The bucket being empty is correct: no lesson videos exist on disk either.
  2. Phase 0 backfill never run → DONE 2026-07-14 (buckets created, ebooks 1.6 GiB/2301 obj ✓ matches disk, blogs ✓; documented in the update above).
  3. /lesson-videos + /geography-media unreachable from the SPA → #599 CLOSED (2026-07-16).

Also landed since: the cutover-day landmine (UploadOrchestrator raw File.Exists on /app/ebooks) is fixed — BookImageLocator with ebooks-bucket fallback is on backend master (#598, PR #297). The S3MediaFallbackMiddleware itself is merged, enabled in prod (Storage__UseS3=true + api-svc on the API stack), and pinned by the real-MinIO integration suite (PR #294), including the load-bearing disk-over-bucket dual-run precedence test.

This ticket's remaining goal — the API loses its media binds — is by definition the #529 phase-2 bind drop (its four binds are in the batch table, row 1). One residual to carry there: uploads-svc needs read on ebooks before the drop (for BookImageLocator), plus the geography-media prefix backfill check. Both added to #529.

Also note: the doc-correctness item from the findings is finally fixed — infra PR #125 corrects docs/minio-storage-migration.md + the infrastructure-reality rule mirrors.

Residual tracked in: #529.

**Epic #413 accuracy pass (2026-07-18) — closing: all three named blockers from the 2026-07-14 findings are resolved; the sole remaining action is #529 phase 2 itself.** Where each blocker stands now (verified): 1. ~~lesson-videos bucket empty because its writer never deployed~~ → **#581 CLOSED** (2026-07-15, backend #303 — key now fetched from OpenBao, deploy guard passes). The bucket being empty is correct: no lesson videos exist on disk either. 2. ~~Phase 0 backfill never run~~ → **DONE 2026-07-14** (buckets created, ebooks 1.6 GiB/2301 obj ✓ matches disk, blogs ✓; documented in the update above). 3. ~~`/lesson-videos` + `/geography-media` unreachable from the SPA~~ → **#599 CLOSED** (2026-07-16). Also landed since: the cutover-day landmine (`UploadOrchestrator` raw `File.Exists` on `/app/ebooks`) is fixed — `BookImageLocator` with ebooks-bucket fallback is on backend master (#598, PR #297). The `S3MediaFallbackMiddleware` itself is merged, **enabled in prod** (`Storage__UseS3=true` + `api-svc` on the API stack), and pinned by the real-MinIO integration suite (PR #294), including the load-bearing disk-over-bucket dual-run precedence test. This ticket's remaining goal — *the API loses its media binds* — is by definition the #529 phase-2 bind drop (its four binds are in the batch table, row 1). One residual to carry there: **`uploads-svc` needs read on `ebooks`** before the drop (for `BookImageLocator`), plus the geography-media prefix backfill check. Both added to #529. Also note: the doc-correctness item from the findings is finally fixed — infra PR #125 corrects `docs/minio-storage-migration.md` + the `infrastructure-reality` rule mirrors. Residual tracked in: #529.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
agentic
Actionable surfaced by the agentic architecture & security review
 bug
Something is not working
 duplicate
This issue or pull request already exists
 enhancement
New feature
 help wanted
Need some help
 invalid
Something is wrong
 question
More information is needed
 sonarqube
Code quality issue surfaced by SonarQube (bug / code smell)
 wontfix
This won't be fixed
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
spikerj (Joseph Spiker)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: spikerj/spikersoft-issues#528