fix(cloudflare): lazy-init S3 client to support dev mode without R2 creds#489
Open
zhuzhuyule wants to merge 2 commits intomasterfrom
Open
fix(cloudflare): lazy-init S3 client to support dev mode without R2 creds#489zhuzhuyule wants to merge 2 commits intomasterfrom
zhuzhuyule wants to merge 2 commits intomasterfrom
Conversation
…reds createS3Client(c.env) was called unconditionally at the top of the /uploads/presign handler, which throws "accessKeyId is a required option" when R2_ACCESS_KEY_ID / R2_SECRET_ACCESS_KEY are not set. The dev-mode proxy-put branch is supposed to avoid the S3 path entirely, but the upfront construction made dev-mode upload impossible without S3 creds. Move the createS3Client call into the else branch so it only runs in non-dev mode (where presigned direct-to-R2 upload is actually used). This unblocks staging deployments that intentionally run with ENVIRONMENT=development to avoid creating R2 S3 API tokens. Note: multipart upload routes (/uploads/multipart/part-url and /uploads/multipart/status) still construct the S3 client unconditionally. Those paths trigger only for files >= 100MB, so this fix is sufficient for the common single-file upload flow on staging.
….toml These per-environment wrangler config files are local: they contain environment-specific D1/R2 ids, KV namespace ids and service binding targets that differ per Cloudflare account, and should never be committed to the upstream repo. The tracked wrangler.toml stays as the template; operators copy it to wrangler.staging.toml / wrangler.local.toml and fill in real values for their account.
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
accessKeyId is a required optioncrash on/uploads/presignwhen the worker is deployed withENVIRONMENT=developmentbut withoutR2_ACCESS_KEY_ID/R2_SECRET_ACCESS_KEYsecrets setAwsClientonly inside theelsebranch where presigned-direct-to-R2 upload actually needs itwrangler.staging.toml/wrangler.local.tomltocloudflare/.gitignoreso per-environment configs don't get committedBackground
Verified during a fresh CF Workers deploy of media-kit to a staging account that intentionally avoided creating R2 S3 API tokens (using `ENVIRONMENT=development` to keep upload mode = proxy-put). The dev-mode branch in `upload.ts` is supposed to skip the S3 path entirely, but `createS3Client(c.env)` was called unconditionally on line 69, causing `aws4fetch` to throw before reaching the dev/prod branch.
Test plan
Known limitations not addressed by this PR
`/uploads/multipart/part-url` (line 356) and `/uploads/multipart/status` (line 443) still construct `createS3Client` unconditionally. They only trigger for files >= 100MB so dev-mode small-file uploads are unaffected. Suggest a follow-up PR to either lazy-init those too with a clear error, or add a `proxy-put` style fallback for multipart parts.