From 4e18f4c6df53a5af433710c41bf650cf12b44687 Mon Sep 17 00:00:00 2001 From: JJ Eaton <58495321+jayleaton@users.noreply.github.com> Date: Fri, 23 Oct 2026 16:22:05 +0700 Subject: [PATCH] feat(storage): rotate the bucket and keys live, probed before the swap STORAGE_S3_ENDPOINT/BUCKET/ACCESS_KEY_ID/SECRET_ACCESS_KEY already come from the secrets manager; now a change takes effect without a restart. config.Secrets.OnStorage calls the hook once per distinct set of the four names (like OnKeyring). cmd/server builds a client for the new set and probes it with one signed HEAD on the bucket through storage.Live.Swap, which switches atomically only if the probe passes and closes the old client's idle connections. A failed probe keeps the old client; the manager logs the rejection with fingerprints and retries after each later fetch. Removing all four upstream keeps the current client. Source: jayleaton/auto-visuals#41 (storage part). Co-Authored-By: Claude Opus 5.5 --- apps/web/cmd/server/storage.go | 31 +++++++++++++- apps/web/internal/config/storage.go | 27 ++++++++++++- apps/web/internal/server/images.go | 7 ++-- docs/secrets.md | 2 +- docs/storage.md | 12 +++++- packages/storage/live.go | 63 +++++++++++++++++++++++++++++ 6 files changed, 134 insertions(+), 8 deletions(-) create mode 100644 packages/storage/live.go diff --git a/apps/web/cmd/server/storage.go b/apps/web/cmd/server/storage.go index c14d98a..dafce2e 100644 --- a/apps/web/cmd/server/storage.go +++ b/apps/web/cmd/server/storage.go @@ -24,7 +24,8 @@ const imageConcurrency = 2 // newImages builds the image endpoints over object storage, or returns nil // (routes not mounted) when STORAGE_S3_BUCKET is unset. A bucket that the // keys cannot reach fails boot, like a database that cannot be reached. -// docs/storage.md. +// The bucket and keys are live secrets: a new set is probed with one real +// call and swapped in only if it works (docs/storage.md). func newImages(ctx context.Context, cfg config.Config, gate *access.Gate, log *slog.Logger) (*api.Images, error) { creds, err := cfg.Secrets.Storage() if err != nil { @@ -32,6 +33,12 @@ func newImages(ctx context.Context, cfg config.Config, gate *access.Gate, log *s } if !creds.Configured() { log.Info("storage: off (STORAGE_S3_BUCKET not set); image routes not mounted") + cfg.Secrets.OnStorage(func(credentials config.StorageCredentials) error { + if credentials.Configured() { + log.Warn("storage: STORAGE_S3_* set upstream but storage was off at boot; restart to mount the image routes", "storage", credentials.Fingerprint()) + } + return nil + }) return nil, nil } client, err := storage.New(storageConfig(cfg, creds)) @@ -44,8 +51,28 @@ func newImages(ctx context.Context, cfg config.Config, gate *access.Gate, log *s return nil, fmt.Errorf("storage: bucket probe failed (%s): %w", creds.Fingerprint(), err) } log.Info("storage: bucket reachable", "storage", creds.Fingerprint()) + live := storage.NewLive(client) + applied := creds.Fingerprint() + cfg.Secrets.OnStorage(func(credentials config.StorageCredentials) error { + next := credentials.Fingerprint() + if next == applied { + return nil + } + if !credentials.Configured() { // removed upstream: keep serving from the bucket we have + log.Warn("storage: STORAGE_S3_* removed upstream; keeping the current bucket client until restart", "storage", applied) + return nil + } + ctx, cancel := context.WithTimeout(ctx, rotationTimeout) // shutdown aborts a rotation in progress + defer cancel() + if err := live.Swap(ctx, storageConfig(cfg, credentials)); err != nil { + return fmt.Errorf("storage: new bucket/keys %s failed their probe, keeping %s: %w", next, applied, err) // retried after the next fetch + } + log.Info("storage: bucket/keys rotated", "from", applied, "to", next) + applied = next + return nil + }) svc := services.NewImageService( - server.NewObjectStore(client), + server.NewObjectStore(live), server.NewImageFetcher(server.NewSafeFetch(cfg.AppName)), server.NewImageCodec(imaging.New(imageConcurrency)), ) diff --git a/apps/web/internal/config/storage.go b/apps/web/internal/config/storage.go index cab3006..3f74116 100644 --- a/apps/web/internal/config/storage.go +++ b/apps/web/internal/config/storage.go @@ -10,7 +10,7 @@ import ( "github.com/jayleaton/go-fast-stack/packages/secrets" ) -// Object storage (README "Object storage") is optional: off unless +// Object storage (docs/storage.md) is optional: off unless // STORAGE_S3_BUCKET is set. Which bucket and which keys are live secrets, so // a bucket move or a key rotation needs no redeploy; the region and // addressing style are plain env. @@ -62,6 +62,31 @@ func (store Secrets) Storage() (StorageCredentials, error) { }, nil } +// OnStorage calls apply now (once a snapshot is published) and whenever any +// of the four storage secrets changes, once per distinct set: a bucket move +// changes several in one save. apply builds and probes a client for the new +// set and swaps it in; an error keeps the old client and is retried after the +// next fetch. +func (store Secrets) OnStorage(apply func(StorageCredentials) error) { + var last string // fingerprint of the last set apply accepted; hooks run one at a time + hook := func(secrets.Secret) error { + credentials, err := store.Storage() + if err != nil { + return err + } + if fingerprint := credentials.Fingerprint(); fingerprint != last { + if err := apply(credentials); err != nil { + return err + } + last = fingerprint + } + return nil + } + for _, name := range storageSecrets { + store.m.OnChange(name, hook) + } +} + func loadStorageOptions(get func(key, fallback string) string, errs *[]error) StorageOptions { raw := get("STORAGE_S3_PATH_STYLE", "false") pathStyle, err := strconv.ParseBool(raw) diff --git a/apps/web/internal/server/images.go b/apps/web/internal/server/images.go index aaaff19..b60e887 100644 --- a/apps/web/internal/server/images.go +++ b/apps/web/internal/server/images.go @@ -11,13 +11,14 @@ import ( "github.com/jayleaton/go-fast-stack/packages/storage/safefetch" ) -// ObjectStore adapts a *storage.Client to services.ObjectStore. -type ObjectStore struct{ bucket *storage.Client } +// ObjectStore adapts a *storage.Live (the bucket client that follows +// rotations) to services.ObjectStore. +type ObjectStore struct{ bucket *storage.Live } var _ services.ObjectStore = ObjectStore{} // NewObjectStore returns the adapter over bucket. -func NewObjectStore(bucket *storage.Client) ObjectStore { return ObjectStore{bucket: bucket} } +func NewObjectStore(bucket *storage.Live) ObjectStore { return ObjectStore{bucket: bucket} } func (store ObjectStore) Put(ctx context.Context, key string, data []byte, contentType string) error { return store.bucket.Put(ctx, key, data, contentType) diff --git a/docs/secrets.md b/docs/secrets.md index efca937..3f330d5 100644 --- a/docs/secrets.md +++ b/docs/secrets.md @@ -14,7 +14,7 @@ Secrets come from one place, `SECRETS_PROVIDER`, through [`packages/secrets`](.. | `DATABASE_URL` | opens a new pool, pings it and checks it is migrated at least as far as this binary, then switches atomically; queries already running finish on the old pool, which closes once they are done (at most 30 s). If the new URL fails, the old pool stays (and `/healthz` only goes red if the old database fails too). Migrations stay a deploy step (`server migrate`). | | `AUTH_SECRET` + `AUTH_SECRET_PREVIOUS` | updates the auth keyring: the new secret signs; previous ones (comma-separated) still verify for `AUTH_SECRET_GRACE`, so no session, email link or trusted device breaks. Cookies signed with an old secret are re-signed on their next request. 2FA secrets are protected by data keys the auth secret only *wraps*, so they are rewrapped, never lost. | | `RESEND_API_KEY` | rebuilds the mailer's Resend client. | -| `STORAGE_S3_ENDPOINT`, `_BUCKET`, `_ACCESS_KEY_ID`, `_SECRET_ACCESS_KEY` (optional, [object storage](storage.md)) | is read when the server starts: restart it to use new values. | +| `STORAGE_S3_ENDPOINT`, `_BUCKET`, `_ACCESS_KEY_ID`, `_SECRET_ACCESS_KEY` (optional, [object storage](storage.md)) | builds a client for the new set and probes it with one signed `HEAD` on the bucket, then switches atomically; if the probe fails, the old client stays and the change is retried after each later fetch. | | `DOPPLER_TOKEN` | — it is the credential that reaches Doppler: update the Railway variable (redeploy), then revoke the old token. | With `env`, all of these are read at boot and change with a redeploy. The admin service does the same with its own Doppler config: `DATABASE_URL` and `ADMIN_DATABASE_URL` each go through a `database.Pool`, the `ADMIN_AUTH_SECRET` keyring (`_PREVIOUS`, `_REVOKED`, grace `ADMIN_AUTH_SECRET_GRACE`) through the admin auth instance's `SetSecrets`, and `RESEND_API_KEY` rebuilds its mailer. Its `/healthz` also reports stale secrets. diff --git a/docs/storage.md b/docs/storage.md index ebc4b4b..a88e81e 100644 --- a/docs/storage.md +++ b/docs/storage.md @@ -67,7 +67,17 @@ The codecs are pure Go: `github.com/gen2brain/webp` (libwebp transpiled to Go) a ## Rotate keys -Change the values in Doppler (or the env and restart, with `SECRETS_PROVIDER=env`). With this version the server reads them at boot, so restart it after a change. +With `SECRETS_PROVIDER=doppler`, change the values in Doppler; no restart, no redeploy. On its next poll (`SECRETS_REFRESH_INTERVAL`) the server builds a client for the new set and makes one real signed call with it (`HEAD` on the bucket). Only if that works does it switch, atomically: uploads already running finish on the old client, new ones use the new bucket and keys. The log shows `storage: bucket/keys rotated from=… to=…` with fingerprints, never values. + +If the probe fails (a typo, a key without access, a bucket that is not there yet), the server keeps the old client, logs `consumer rejected change, keeping its previous client` with the fingerprints and the reason (`access denied: head 403 Forbidden`), and tries again after every later fetch, so fixing the value in Doppler is enough. A partial set (some of the four names missing) is refused before it is published. Removing all four upstream keeps the current client until a restart; turning storage on upstream while it was off at boot needs a restart to mount the routes (it is logged). + +Rotate a key without downtime: + +1. Create a second key for the bucket (Railway, R2 and S3 allow two at once). +2. Put it in `STORAGE_S3_ACCESS_KEY_ID` and `STORAGE_S3_SECRET_ACCESS_KEY` in Doppler, and wait for the `rotated` log line on every instance. +3. Revoke the old key. + +Moving to another bucket works the same way (copy the objects first: the server does not migrate them). With `SECRETS_PROVIDER=env`, change the env and restart. ## Delete it diff --git a/packages/storage/live.go b/packages/storage/live.go new file mode 100644 index 0000000..e543212 --- /dev/null +++ b/packages/storage/live.go @@ -0,0 +1,63 @@ +package storage + +import ( + "context" + "sync" + "sync/atomic" +) + +// Live is a Client whose bucket and credentials can change while the app +// runs (a key rotation, a bucket move). Swap builds the new Client and probes +// it with one real call before switching atomically; until then, and if the +// probe fails, every call goes to the old one. Safe for concurrent use. +type Live struct { + current atomic.Pointer[Client] + swapMu sync.Mutex // serializes Swap +} + +// NewLive starts on client (built and probed by the caller). +func NewLive(client *Client) *Live { + live := &Live{} + live.current.Store(client) + return live +} + +// Swap switches to cfg once a Client for it passes Probe. On any error the +// current Client stays and is returned unchanged; the error names no values. +// Requests already running finish on the old Client; its idle connections +// are closed. +func (live *Live) Swap(ctx context.Context, cfg Config) error { + live.swapMu.Lock() + defer live.swapMu.Unlock() + next, err := New(cfg) + if err != nil { + return err + } + if err := next.Probe(ctx); err != nil { + next.httpClient.CloseIdleConnections() + return err + } + old := live.current.Swap(next) + if old.httpClient != next.httpClient { // a shared Config.HTTPClient stays open + old.httpClient.CloseIdleConnections() + } + return nil +} + +// Put stores data under key on the current Client (see Client.Put). +func (live *Live) Put(ctx context.Context, key string, data []byte, contentType string) error { + return live.current.Load().Put(ctx, key, data, contentType) +} + +// Get opens key on the current Client (see Client.Get). +func (live *Live) Get(ctx context.Context, key string) (*Object, error) { + return live.current.Load().Get(ctx, key) +} + +// Delete removes key on the current Client (see Client.Delete). +func (live *Live) Delete(ctx context.Context, key string) error { + return live.current.Load().Delete(ctx, key) +} + +// Probe checks the current Client (see Client.Probe). +func (live *Live) Probe(ctx context.Context) error { return live.current.Load().Probe(ctx) }