Skip to content

feat(file_storage): configurable bucket folder prefix + any S3 provider - #241

Draft
antosubash wants to merge 1 commit into
mainfrom
worktree-s3-provider-prefix
Draft

feat(file_storage): configurable bucket folder prefix + any S3 provider#241
antosubash wants to merge 1 commit into
mainfrom
worktree-s3-provider-prefix

Conversation

@antosubash

Copy link
Copy Markdown
Owner

What

Storage config could not express two things operators need: a folder to confine objects to, and the knobs non-AWS S3 providers require. Both are now DB-backed settings, editable in the admin Settings UI.

Folder prefix

New key_prefix setting (General group). media, /media/, and a//b all canonicalise to a clean media/; .. segments and absolute paths are rejected by a validator at config time, rather than surfacing as a generic backend error at first upload.

The prefix is applied once at upload and baked into the stored key, rather than prepended transparently by the backend on every operation. That is deliberate: it means changing the prefix later steers only new uploads, and every already-stored file keeps resolving. The transparent alternative would orphan every existing object the moment an operator edited the field.

Applies to all backends. Blank (the default) preserves the existing root layout, so current installs see no change.

S3 provider compatibility

Setting Why
s3_addressing_style auto/path/virtual. Reachable only through a real botocore.config.Config, which the factory never built — so path-style was previously unreachable. This is what MinIO, Ceph, and IP/localhost endpoints require.
s3_public_endpoint_url Sign download URLs for the host the browser will actually contact, when it differs from the one the app connects to (app → http://minio:9000, browser → https://files.example.com). An S3 signature is bound to the host in the URL, so this cannot be a rewrite after the fact.
s3_signature_version, s3_verify_ssl Self-hosted gateways and self-signed certificates.
s3_region No longer required (defaults to us-east-1), so R2 and region-less providers boot. Only s3_bucket stays mandatory.

Backend rebuild on settings change

The backend is a singleton built once in on_startup. Without a SettingsReloaded subscription, editing the bucket or endpoint in the UI would swap services.settings while every upload kept talking to the provider built at boot — the config would appear to save and silently do nothing. It now rebuilds in place; no restart needed.

Verification

  • make lint — exit 0 (ruff format, ruff, ty, Biome, per-workspace tsc, file-size cap)
  • make test — 1660 Python passed / 2 skipped, 48 JS passed
  • make doctor — the 3 findings (SM020 dual auth providers, 2× SM003) are byte-identical on the baseline commit; verified by stashing the change and re-running. This adds none.
  • Both new behaviours mutation-tested: dropping the prefix from key generation fails 5 tests; removing the bus.subscribe fails the rebuild test.

New coverage includes the regression that guards the core guarantee — a pre-existing prefix-free key still downloads after a prefix is configured — plus a real moto round-trip proving objects land under the folder, and that path-style addressing still reaches the bucket.

Not included

FilesystemBackend._resolve shards on key[:2], which is the year (20) for every key ever written, so all objects already land in one directory — the shard has never been effective. A prefix makes it me instead of 20, equally degenerate, not worse. Fixing it properly would relocate already-stored files, so this corrects the misleading docstring and leaves behaviour alone.

Design doc: docs/superpowers/specs/2026-08-06-s3-provider-prefix-design.md

https://claude.ai/code/session_01RyvwczjTY8WbiMWEiyPhMx

Storage config could not express two things operators need: a folder to
confine objects to, and the knobs non-AWS S3 providers require.

Folder prefix:
- New `key_prefix` setting (General group), DB-backed and editable in the
  admin Settings UI like every other module setting.
- Normalised by a validator: `media`, `/media/`, `a//b` all canonicalise;
  `..` segments and absolute paths are rejected at config time rather than
  surfacing as a generic backend error at first upload.
- Applied once at upload and baked into the stored key, so changing the
  prefix later steers only new uploads — existing files keep resolving.
  Applies to all backends; blank keeps the previous root layout.

S3 provider compatibility:
- `s3_addressing_style` (auto/path/virtual) — reachable only via a real
  botocore Config, which the factory never built, so path-style was
  previously unreachable. This is what MinIO, Ceph and IP/localhost
  endpoints require.
- `s3_public_endpoint_url` — sign download URLs for the host the browser
  will actually contact, when it differs from the one the app connects to.
- `s3_signature_version`, `s3_verify_ssl` for self-hosted gateways.
- `s3_region` is no longer required (defaults to us-east-1), so R2 and
  region-less providers boot. Only `s3_bucket` stays mandatory.

The backend is now rebuilt on SettingsReloaded. Without it, editing storage
config would swap the settings while every upload kept using the provider
built at startup — saving would appear to work and silently do nothing.

Also corrects the FilesystemBackend docstring, which claimed a sharding
behaviour that has never been effective (every key starts with the year or
the prefix, so all objects land in one directory). Behaviour left alone;
fixing it would relocate already-stored files.

Claude-Session: https://claude.ai/code/session_01RyvwczjTY8WbiMWEiyPhMx
@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying simple-module-python with  Cloudflare Pages  Cloudflare Pages

Latest commit: a0165c6
Status: ✅  Deploy successful!
Preview URL: https://b649dabd.simple-module-python.pages.dev
Branch Preview URL: https://worktree-s3-provider-prefix.simple-module-python.pages.dev

View logs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant