Skip to content

S3 and S3-compatible storage backend #20

Description

@jiashuoz

Let an operator run AgentDrive on Amazon S3 and S3-compatible stores (MinIO, Ceph RGW, Cloudflare R2, Backblaze B2, Wasabi) as a third STORAGE_BACKEND, beside gcs and fs. One bucket per install; per-tenant buckets are out of scope.

This is the first follow-on after the public release (see the README). It was designed before the repository went public; this issue carries that design forward, updated to the code as it stands.

Where the code is today

Already done, so an S3 backend is an adapter plus a GC decision rather than a refactor:

  • src/agentdrive/storage/ is one ObjectStore protocol (base.py), a facade (__init__.py) and two backends: gcs.py and fs.py, chosen by STORAGE_BACKEND in factory.py. Nothing outside the package imports a provider SDK.
  • tests/storage/test_contract.py runs one contract suite against every backend. An S3 backend joins it.
  • Nothing provider-specific is persisted: artifact_versions.storage_object is an opaque key, and the authoritative checksum is an app-computed sha256:. No data migration is needed to add a backend.
  • The GC is live (core/gc.py, python -m agentdrive.jobs.gc). Its mark-sweep only considers objects older than GCSweeper.MARK_SWEEP_AGE (24 h) and pins each delete to the object generation it listed. It refuses to run on a store whose Capabilities.generation_pinned_delete is false.

The hard part: the generation pin does not port to S3

The storage contract (storage/base.py) promises that every object carries a generation that changes on every successful write, including a byte-identical overwrite, and that a delete pinned to a stale generation fails. GCS generations do exactly that; fs.py implements it with a sidecar and a per-key lock. specs/tla/ArtifactGC.tla models the GC against that property.

S3 has no generation. If-Match on DeleteObject compares the ETag, which is derived from content, and the CAS store is content-addressed. So:

unreferenced blob → GC lists it and records ETag E → a client re-uploads the identical bytes and takes a new live reference → GC deletes with If-Match: E → same content, same ETag → 204 → live data destroyed

On GCS the re-upload mints a new generation and the delete fails with 412. On S3 the precondition is ABA-blind precisely because the store is content-addressed. S3 conditional deletes (GA 2025-09-16) are real but do not close this race. The same applies to CAS_REFRESH_AGE's refresh-before-adoption, which relies on a same-content re-upload minting a new generation.

It gets worse on S3-compatible stores:

  • Most lack conditional delete, and some silently ignore the header instead of rejecting it: no 412, no signal, live data gone. MinIO community reportedly ignores it (minio#21677); Ceph RGW only from Tentacle v20.2.1; R2 and B2 unsupported; Wasabi unverified.
  • Versioned buckets defeat the sweep silently. A plain DeleteObject writes a delete marker: 204, the object leaves LIST, zero bytes reclaimed. The GC reports success while storage grows.
  • If-Match delete needs s3:GetObject as well as s3:DeleteObject: a least-privilege GC role gets AccessDenied, not 412.

Setting generation_pinned_delete=True on an S3 adapter would therefore be a lie, and the GC's refusal would keep an S3 install from ever reclaiming space. That is the design decision this issue exists for.

Proposed direction

Age watermark as the primary defense; the pin as per-backend hardening.

  • Keep the listing watermark (only objects older than the sweep age), re-stat immediately before delete and skip anything that moved, and re-check liveness in Postgres inside the delete path.
  • GCS and fs keep the generation pin. AWS S3 may pin to a listed VersionId if the contract requires versioning. Other S3-compatible stores get the watermark only, with an operator-visible warning at startup.
  • Prior art agrees nobody ships a portable conditional delete: JuiceFS uses a 1 h skip window, lakeFS a 24 h uncommitted cutoff.
  • Re-run ArtifactGC.tla with a precondition token that does not change on identical-content overwrite (the S3 reality) before trusting any S3 pinned path.

Replace the boolean with a three-state capability, probed rather than configured:

@dataclass(frozen=True)
class Capabilities:
    conditional_delete: Literal["version_pin", "content_pin", "none"]
    signed_download: bool
    signed_download_response_overrides: bool   # response-content-disposition
    direct_transfer: Literal["gcs_resumable", "s3_multipart", "none"]
    server_checksums: frozenset[str]

Because some backends silently ignore If-Match, a config flag is not enough. At startup, probe: write an object, attempt a conditional delete with a deliberately wrong ETag, and require 412. Never silently downgrade. Model the struct on OpenDAL's Capability (operation+option granularity), remembering that a capability field is not a guarantee (opendal#7889) — hence the probe.

One S3 adapter with provider quirk profiles (MinIO, R2, Ceph, B2 under one adapter with a deviation table, as rclone does), boto3 behind asyncio.to_thread like the GCS adapter.

Bucket contract (decide before writing the adapter)

  • Non-versioned bucket required (check GetBucketVersioning at startup), or implement ListObjectVersions + per-version delete
  • Standard storage class only — early-deletion minimums punish short-lived blobs
  • An AbortIncompleteMultipartUpload lifecycle rule — abandoned multipart uploads bill indefinitely
  • SSE-C / GCS CSEK unsupported: incompatible with browser-pasteable signed URLs. SSE-S3, SSE-KMS and CMEK are fine
  • CORS documented, including ExposeHeaders: ETag (community MinIO has no per-bucket CORS)
  • boto3 ≥ 1.36 sends default checksums that several S3-compatible stores reject: set request_checksum_calculation="when_required" and response_checksum_validation="when_required"

Other work the adapter touches

  • viewer/routes.py hard-codes connect-src … https://storage.googleapis.com in the viewer CSP; on S3 that is a browser CSP violation, not a clean error. Derive it from the store.
  • Direct transfer (DIRECT_TRANSFER_ENABLED, GCS-only today) is a mechanism change on S3, not config: GCS binds CORS to the Origin at resumable-session initiation, while S3 needs bucket CORS plus multipart (5 MiB minimum part, presigned part URLs, CompleteMultipartUpload). The largest implementation delta; it can ship after the basic adapter, with direct transfer refused on S3 until then.
  • Neutral types: ObjectStat.crc32c/md5 → checksums: dict[str, str]; generation: int → an opaque version: str; map time_created explicitly to S3 LastModified (an overwrite resets it and re-ages the object, which matters to the watermark).
  • For large uploads the server never sees the bytes; require a client-supplied SHA-256 rather than recording a backend checksum as authoritative.
  • Signed-URL TTL: a presigned URL dies with its signing credential (role-chained STS credentials cap at 1 h). Sign for min(requested_ttl, remaining_credential_lifetime).
  • Per-backend Limits() (multipart part counts and sizes differ: 10,000 × 5 GiB on AWS, 5 TiB objects on R2).
  • ensure_bucket error codes differ between providers; S3 has no STORAGE_EMULATOR_HOST analogue — use MinIO in CI.
  • Create one boto3 client before threads, with max_pool_connections sized to the executor.

Sequencing

  1. Package split, GCS adapter moved verbatim, fs adapter, contract suite — done.
  2. GC design note (gate before code): watermark first, pin as backend-specific hardening; the TLA+ re-run above.
  3. Three-state capabilities + neutral types + the startup probe + the S3 adapter, with direct transfer refused on S3.
  4. Contract suite over fs / gcs / s3 (MinIO in CI), asserting precondition semantics, not only API shape — e.g. "a stale-precondition delete returns 412" per backend.
  5. An emulator-fidelity tier: fake-gcs-server does not enforce ifGenerationMatch, so precondition tests against it prove nothing. A real-bucket tier (nightly is fine) is needed for any pinned path.
  6. S3 multipart direct transfer.

Open questions (each with the test that settles it)

  1. Do R2, B2 and Ceph reject or silently ignore If-Match on DeleteObject? — delete_object(IfMatch=<stale ETag>) per provider; expect 412.
  2. AWS If-Match delete on multipart (composite-ETag) objects — MPU, then delete with the composite ETag (expect 204) and a wrong one (expect 412).
  3. response-content-disposition on B2 / R2 / MinIO presigned GETs, which the download path depends on.

Non-goals

  • Per-tenant or per-drive buckets.
  • Non-PostgreSQL metadata stores.
  • Replacing the storage layer with obstore / Rust object_store: it has no conditional delete (arrow-rs-object-store#298) and its Python signing lacks response-content-disposition (obstore#746).

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or requesthelp wantedExtra attention is needed

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions