feat(storage): add an S3-compatible object store

[BryanFRD]

First slice of #10, opened as a draft on purpose: nothing constructs this store yet, so it is not mergeable as it stands. What it does is settle the design and prove it against a wire, which is the part that would have been expensive to get wrong later.

Answering the question the issue leaves open first: proxy by default, pre-signed as an option for downloads. The server stays in the byte path so the counters, the ranges, the size limit, the quota and the compression keep working; an operator who would rather spend the bucket's bandwidth than their own can hand out a pre-signed URL instead. presigned_download is here for that, unused until the second slice.

The layout is the local one

The bytes live once under .content/<xx>/<yy>/<oid>, and a repository that holds them owns an empty marker at <org>/<repo>/<xx>/<yy>/<oid>. That marker is the object store's answer to a hard link, and it carries both properties the local store gets from the filesystem:

• deduplication, because content addressing means two projects pushing the same pack write the same key — S3 gives this away for free, and paying twice would be throwing it away
• isolation, because the marker is the proof of possession and the only thing consulted. Without it, guessing a digest would be enough to read another project's assets out of a shared keyspace. There is a test for exactly that.

Uploads stream from the staging file

The obvious shape — read the object, PUT it — puts a three-gigabyte asset in memory. The upload has already been streamed to a staging file, hashed, and checked against everything the server enforces, so that file is what goes up, streamed from disk. The local disk becomes a write buffer rather than the store, and the flat-memory property the whole storage layer is built on survives.

Downloads are ranged GETs against the bucket, so resuming a transfer asks for the bytes it wants rather than the whole object.

Tests

Five, against an in-process stub bucket — PUT, HEAD, ranged GET and a prefix listing, the same shape as the stub forge the authentication tests use. A real MinIO belongs in CI, not in the path of every cargo test.

An object goes up once and comes back whole; a second repository adds a marker and not the bytes; a repository that never pushed an object does not hold it; a range asks the bucket for that range; and what a repository holds is counted from its markers.

Getting the stub honest took three corrections, each of which would have been a wrong assumption about real S3: the listing needs the ListBucketResult namespace, entries need an ETag, and path-style addressing puts the bucket in the path so the stub had to strip it to keep a real keyspace.

What the next slice has to do

The seam. Routes call state.store — a LocalStore — in about ten places, and four of those operations (retain, dedupe, compress, audit) have no S3 equivalent yet. Extracting that seam, wiring LFSX_STORAGE=s3 through configuration, and deciding what those four do on a bucket is the work that makes this reachable, and it deserves its own review rather than being bolted onto this one.

[ferrfleet]

Solid first slice — layout mirrors the local store correctly (dedupe via content key, isolation via marker), streaming upload/download avoids buffering large objects, and the stub-bucket tests cover the right shapes (marker vs content, cross-repo isolation, ranges, listing). No blocking issues; a couple of nits to consider before the seam-extraction slice wires this in:

Nit: content_key/marker_key (server/src/storage/s3.rs:52-63) slice oid[0..2]/oid[2..4] without validating length first. exists() guards with LocalStore::validate_oid before calling into these, but store() and size_of() don't — a malformed/short oid reaching either will panic (byte-index-out-of-bounds) instead of returning Err. Worth validating at the top of store/size_of too, same as exists, before routes start calling this.

Nit: head() (server/src/storage/s3.rs:82) signs the presigned URL via GetObject and then issues an HTTP HEAD against it. SigV4 presigned auth includes the HTTP method in the signature; this happens to work against the in-process stub because it doesn't check signatures at all. Worth confirming this round-trips against a real S3/MinIO endpoint (mentioned as a follow-up in the PR description) before relying on it.

Nit: usage_of (server/src/storage/s3.rs:180) does one HEAD per object after the listing — fine for the sizes involved here, but worth keeping in mind if repositories grow large before this gets cached.

[github-actions]

SonarQube — aucune nouvelle issue

_{Comparaison entre le projet bac à sable de cette PR et la branche par défaut : SonarQube Community n'analyse pas les PR, ce delta est calculé côté CI. Détail}

[BryanFRD]

Second slice: the seam, so this is reachable rather than dead code. Marking it ready.

LFSX_STORAGE=s3 now selects a bucket, and Store is the enum routes talk to. The four maintenance commands answer 501 on a bucket rather than an empty report — collection, compression and verification because they are not implemented against one yet, and deduplication because content addressing already stores each object once, so there is nothing left to fold in. An operator running collection against a bucket has to be told it did nothing.

The refactor that made it possible is worth pointing at: stage() is now the part both backends share — the digest the transfer claims, the size it declared, the object ceiling and the repository budget, all landing on local disk. A bucket cannot be asked to hold bytes that might turn out to be the wrong ones, so local disk stays as a write buffer, and the staging file is removed as soon as the upload lands. There is a test for that: a buffer that is never emptied is a disk that fills.

The seam test found a bug the S3 tests had missed. size_of read the response body length rather than the Content-Length header, and a HEAD has no body — so an object came back one byte long. None of the five store tests caught it because they all passed an explicit length, and the usage test asserted the object count without ever checking the bytes. Both are fixed: the header is read, and the usage test now pins the size it reports.

usage() returns zero on a bucket deliberately. There is no cheap answer for what a whole bucket holds, and building one from a full listing would cost a request per object on every scrape. The per-repository figure is the one this server can afford.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[ferrfleet]

Follow-up review on the new commit (c4115b3) that wires the S3 seam in: Storage config, Store enum dispatching Local/Bucket, staging refactor (stage()/agrees() split out of write()), and the four maintenance ops correctly answering Unsupported/501 on a bucket instead of a fake empty success. The refactor preserves local-store semantics (fresh-tracking, budget, link-or-move all still gated the same way), and the new tests cover the upload/read round-trip, staging cleanup, and the 501 behavior.

No blocking issues. One new nit inline on backend.rs about usage() silently zeroing the /metrics gauges for bucket storage, which is the one place this PR reports empty success instead of saying so like it does everywhere else.

The three nits from the previous review (oid-length validation in s3.rs store/size_of, the HEAD-via-GetObject-signature question, per-object HEAD cost in usage_of) are still open and unchanged — not repeating them here, they still stand for the seam-extraction cleanup.

[BryanFRD]

All three taken, and two of them were worth more than the nit label.

Signing a HEAD as a GET. head() reused a GetObject signature — SigV4 covers the method, so an implementation that checks it is entitled to refuse. AWS happens to be lenient; MinIO and Garage are the targets, and "usually works" is not something to find out in someone's cluster. It uses HeadObject now.

The unvalidated object id. Unreachable today — every caller validates first — but the fanout slices the first four characters, so a short id is an index out of bounds. A panic where the client deserves a 422, one refactor away. Guarded at all four entry points, with a test.

That test earned its place immediately: the guard on store() silently did not apply, because rustfmt had wrapped the signature my patch was matching on. It failed, named the line, and the second attempt landed.

The zeroed gauges. Right, and it contradicted the principle the rest of the PR argues for. usage() is now capacity() -> Option: None on a bucket, and scrape leaves the two gauges alone rather than pinning them to a zero every dashboard would average as an empty store. The startup log now names those gauges specifically, and the README says where to read capacity instead.

Registering them conditionally would be the fuller fix — an absent series beats a stale one — but that reaches into the metrics registry for a mode nobody runs yet, so it can wait for the slice that makes buckets a real deployment.

[ferrfleet]

Follow-up on ef939d6: this commit cleans up all three outstanding nits from the previous reviews.

• oid-length validation added to S3Store::store/size_of/read (they now call LocalStore::validate_oid before slicing, matching exists), with a test asserting ""/"ab"/"abc" are refused rather than panicking.
• head() now signs a HeadObject action instead of a GetObject action for the HTTP HEAD request, so SigV4 covers the right method against a real S3/MinIO endpoint.
• usage() → capacity() -> Option<(u64, u64)>: bucket storage now returns None instead of (0, 0), so scrape() leaves the /metrics gauges untouched rather than pinning them to a misleading zero. Documented in the README and logged at startup.

No new issues in the diff. No blocking findings.