Skip to content

R2 onboarding: fix depot init's token trap, pin SDK checksums, add live smoketest script#14

Open
adewale wants to merge 1 commit into
mainfrom
claude/scalability-visualization-review-174hr7
Open

R2 onboarding: fix depot init's token trap, pin SDK checksums, add live smoketest script#14
adewale wants to merge 1 commit into
mainfrom
claude/scalability-visualization-review-174hr7

Conversation

@adewale

@adewale adewale commented Jun 12, 2026

Copy link
Copy Markdown
Owner

What

Three changes that came out of checking aha's R2 onboarding against Cloudflare's current best-practices docs:

  1. depot init token trap fixed. The docs recommend an Object Read & Write token scoped to one bucket — correct per Cloudflare — but only Admin tokens can create buckets, while depot init auto-created missing buckets and its 403 hint said "check that the token has Object Read & Write permissions". A user following our own docs hit a denial whose hint told them to re-verify a token that was already correct. ensureBucket now surfaces non-NotFound HeadBucket errors as themselves (previously a credential failure fell through to CreateBucket and reported that error), names the bucket-creation step when CreateBucket is denied, and the hint layer answers that case with "pre-create the bucket" instead.
  2. SDK checksums pinned to Cloudflare's guidance. NewR2 now sets RequestChecksumCalculation/ResponseChecksumValidation to WhenRequired. R2 implements x-amz-checksum-* only partially, and our pinned SDK (service/s3 v1.101.0) is past the version where AWS started attaching checksums to every PutObject by default — the change that broke many S3-compatible stores in early 2025.
  3. scripts/r2-smoketest.sh. Preflights every R2 env var (all missing ones reported at once, with the setup recipe), then runs the live-bucket integration test with -count=1 so a cached pass can't vouch for the live service.

Docs: onboarding.md §8 rewritten in dependency order (bucket → bucket-scoped token → init, with the why: a token can only be scoped to an existing bucket, and a bucket-scoped token can't create one); r2-bucket-settings.md gains concrete bucket-lock guidance (lock blobs/, never machines/ — pushes overwrite the pointer and index with conditional PUTs), Account-vs-User token choice, temporary credentials for CI, and location-hint advice; CHANGELOG entry; drift anchor updated.

Why

No code path or doc told the truth about the most likely first-run failure: init against a fresh bucket with the recommended token. And the checksum default is a silent compatibility risk the fake-S3 suite structurally cannot catch — only a live run can, which is what the script makes routine.

How

  • internal/depot/objectstore_v2.go: ensureBucket creates only on isS3NotFound; CreateBucket failures are wrapped as create r2 bucket "<name>", the marker the CLI hint layer keys on.
  • internal/cli/command_doctor.go: the denied case splits on that marker; everything else keeps the existing hints.
  • internal/depot/r2.go: checksum options on the production client only — test fakes accept any header, so pinning them there would prove nothing.

Testing

  • Red before green for all three changes: TestEnsureBucketSurfacesHeadErrorsWithoutCreating, TestEnsureBucketCreateDenialNamesBucketCreation (plus a happy-path pin), TestNewR2FollowsCloudflareChecksumGuidance, and a TestDepotErrorHints table that pins the whole hint surface (previously untested). Each failed for the documented reason against the old code.
  • make verify-quick green.
  • Script paths exercised: no env → actionable preflight failure (exit 2); dead endpoint → integration test fails and the script reports exit 1 with a aha doctor --depot pointer.

Not verified here: a live-bucket run. This environment has no R2 credentials, so scripts/r2-smoketest.sh against a real bucket — the very check this PR exists to make routine — still needs one run by someone with a test bucket.

Risk

Low. The checksum pin follows Cloudflare's published SDK guidance and depot integrity never depended on those headers (everything is content-addressed and re-hashed on read). The ensureBucket change strictly narrows when CreateBucket is attempted; the only behavior removed is auto-creating a bucket after a non-NotFound HeadBucket error, which only ever "worked" by accident against permissive endpoints.

https://claude.ai/code/session_01PtaKyYUoZtmThtvxFCskYA


Generated by Claude Code

…dd smoketest script

The recommended R2 token (Object Read & Write, scoped to one bucket)
cannot create buckets, but `depot init` auto-created missing buckets and
its 403 hint told users to re-check permissions that were already
correct. ensureBucket now surfaces non-NotFound HeadBucket errors as
themselves, names the bucket-creation step when CreateBucket is denied,
and the hint layer answers that case with "pre-create the bucket"
instead. Onboarding §8 is reordered into dependency order: bucket, then
bucket-scoped token, then init.

The production R2 client pins request/response checksums to
WhenRequired per Cloudflare's SDK guidance — R2 implements
x-amz-checksum-* only partially, the SDK default attaches checksums to
every PutObject, and the fake-S3 suite can never catch that mismatch.

scripts/r2-smoketest.sh preflights the R2 environment with per-variable
guidance and runs the live-bucket integration test (-count=1, so a
cached pass can't vouch for the live service).

Docs: bucket-lock guidance scoped to blobs/ (locking machines/ would
break the conditional pointer/index overwrites every push performs),
Account-vs-User token choice, temporary credentials for CI, location
hints, and a troubleshooting entry for the create-bucket denial.

https://claude.ai/code/session_01PtaKyYUoZtmThtvxFCskYA
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.

2 participants