Add trusted-server-adapter-cloudflare crate (PR 17)#644
Open
prk-Jr wants to merge 20 commits into
Open
Conversation
…ator wasm32-unknown-unknown (Cloudflare Workers) does not support std::time::Instant — it panics at runtime. web_time::Instant is a zero-cost drop-in on native and JS-backed on WASM.
Implements the Cloudflare Workers adapter following the same pattern as trusted-server-adapter-axum: TrustedServerApp implements the Hooks trait, platform services use noop stubs on native (CI-compilable), and the #[event(fetch)] entry point delegates to edgezero_adapter_cloudflare::run_app. Also adds UnavailableHttpClient to trusted-server-core platform module, parallel to the existing UnavailableKvStore.
CI: test-cloudflare job checks native host compile, wasm32-unknown-unknown compile (with cloudflare feature), and runs host-target unit tests. CLAUDE.md: add cloudflare crate to workspace layout and build commands.
…un_app The rev (38198f9) of edgezero used in this workspace requires worker 0.7 (not 0.6) and run_app() takes a manifest_src: &str as first argument. Updated Cargo.toml and lib.rs accordingly.
- Implement CloudflareHttpClient (wasm32 only) using worker::Fetch for real
outbound proxy requests; strip content-encoding/transfer-encoding headers
since the Workers runtime auto-decompresses responses
- Add build.sh with cd-to-SCRIPT_DIR guard so worker-build always runs from
the correct crate root regardless of invocation directory
- Switch wrangler dev task to use --cwd from workspace root (same DX as Fastly)
- Add js-sys to workspace dependencies; reference it via { workspace = true }
- Fix #[ignore] messages on Cloudflare integration tests
- Replace std::time::{SystemTime,UNIX_EPOCH} with web_time in test code for
signing.rs and proxy.rs (consistency with production paths)
- Add NoopConfigStore/NoopSecretStore TODO comment tracking the gap
- Add extract_client_ip unit tests (parses cf-connecting-ip, absent, invalid)
- Remove empty crate_compiles_on_host_target test
- Add CloudflareHttpClient timeout doc noting Workers CPU-budget tradeoff
Replace direct worker::Env store construction with edgezero handles already injected by run_app, reducing #[cfg(target_arch = "wasm32")] blocks from 5 to 2. - ConfigStoreHandleAdapter: bridges ctx.config_store() to PlatformConfigStore — reuses the already-parsed TRUSTED_SERVER_CONFIG JSON handle rather than re-parsing it on every request - KvHandleAdapter: bridges ctx.kv_handle() to PlatformKvStore — reuses the env.kv() handle opened by run_app rather than opening a new one - CloudflareGeo: moved outside #[cfg]; reads cf-ipcountry and related headers via ctx.request().headers() which needs no platform import - CloudflareSecretStoreAdapter: kept under #[cfg] — env.secret() is synchronous at the JS level but PlatformSecretStore::get_bytes is sync while SecretHandle::get_bytes is async; direct env access is required - Add .dev.vars to .gitignore (wrangler convention for local secrets) - Add bytes workspace dep for KvStore impl
…ezero-pr17-cloudflare-adapter
- Implement CloudflareWorkers::spawn() to start wrangler dev; in CI uses wrangler.ci.toml (no build step, uses pre-built bundle); locally uses wrangler.toml (triggers build.sh rebuild) - Add wrangler.ci.toml: no [build] section so wrangler dev skips the worker-build step when the bundle is pre-built as a CI artifact - Add build-cloudflare input to setup-integration-test-env: adds wasm32-unknown-unknown target and runs build.sh with integration test env vars - In prepare-artifacts: enable build-cloudflare and upload build/ dir as artifact - In integration-tests: restore CF build dir, install wrangler, set CLOUDFLARE_WRANGLER_DIR, and remove --skip flags for cloudflare tests
- Add GeoInfo happy-path test: build_geo_lookup_returns_some_with_populated_country verifies country, city, continent, latitude, and longitude are correctly populated when Cloudflare headers are present - Simplify CloudflareSecretStoreAdapter::get_bytes: collapse brittle JsError string-matching guards into a single error arm with contextual message - Document sequential fan-out latency in CloudflareHttpClient: explain sum(DSP_i) vs max(DSP_i) implication and why true parallelism is blocked by the ?Send bound on PlatformHttpClient - Fix stale wrangler.toml comment: update to reflect ConfigStoreHandleAdapter wiring rather than the now-fallback NoopConfigStore - Extend CI triggers to feature branches: format.yml and test.yml now run fmt/clippy/tests on PRs targeting feature/** so stacking PRs are gated - Fix fmt violations caught during pre-review verification: platform.rs two-line Err wrapping and plan doc Prettier formatting
The adapter's tokio dev-dependency uses rt-multi-thread which is not supported on wasm32. It is already tested in the dedicated test-cloudflare job; exclude it from the workspace wasm test to avoid the compile error.
ChristianPavilonis
requested changes
Apr 24, 2026
Collaborator
ChristianPavilonis
left a comment
ChristianPavilonis
left a comment
There was a problem hiding this comment.
Summary
Adds the Cloudflare adapter, runtime wiring, and CI coverage. CI is green, but I found two correctness issues that should be addressed before merge, plus two smaller follow-ups.
Resolved conflicts: - .cargo/config.toml: extend test-fastly alias to also exclude trusted-server-adapter-cloudflare - .github/workflows/test.yml: use cargo test-fastly alias in CI (from PR16) and keep test-cloudflare job (from PR17) - CLAUDE.md: keep Cloudflare workspace entry and cargo test-axum alias; merge both adapters' commands - crates/trusted-server-core/src/proxy.rs: use #[tokio::test] + std::time pattern (consistent with PR16 tests) - Cargo.lock: regenerated to reflect merged workspace state
Brings in the default-members fix: restricts workspace default-members to `trusted-server-adapter-fastly` so Viceroy can locate the binary via `cargo run --bin` during `cargo test-fastly`.
aram356
requested changes
Apr 30, 2026
Collaborator
aram356
left a comment
aram356
left a comment
There was a problem hiding this comment.
Summary
Adds the Cloudflare Workers adapter, with web_time migration in core and a new CI job. The middleware-bypass and backend-naming concerns from the prior review are addressed and have regression tests, but the second prior 🔧 (sequential fan-out + missing per-request deadline) is mitigated only by documentation and a noisy log warning — the underlying behavior is unchanged. CI has three required gates failing.
Blocking
🔧 wrench
- CI is red on three required gates —
cargo fmt --all -- --check,format-docs, andintegration testsare failing on the latest run. The integration-tests failure is especially concerning because this PR is the one adding theCloudflareWorkersruntime to that matrix and the new env-var/wrangler wiring (integration-tests.yml:74-105). Either the new Cloudflare integration test broke, or it isn't actually running and another regression slipped in. Runcargo fmt --all,cd docs && npm run format, and reproduce the integration-tests failure locally withCLOUDFLARE_WRANGLER_DIRset; address whatever surfaces. Don't merge with red required checks. - Per-provider auction timeout is silently dropped on Cloudflare (
crates/trusted-server-adapter-cloudflare/src/platform.rs:289-315) — see inline. select()warning firesn-1times per auction → log noise on every multi-provider request (crates/trusted-server-adapter-cloudflare/src/platform.rs:329-337) — see inline.
Non-blocking
🤔 thinking
send_asyncalways materializes the body, then re-serializes it forselect(platform.rs:289-315) — see inline.Body::Streamsilently dropped in outbound request body (platform.rs:228-235) — see inline.
♻️ refactor
get_fallback/post_fallbacknear-duplicates (app.rs:307-363) — see inline.auth_middleware_rejects_with_401_when_credentials_requireddoesn't actually test 401 (tests/routes.rs:43-75) — see inline.
📌 out of scope
wrangler.tomlships a placeholder KV id (wrangler.toml:11) —id = "REPLACE_WITH_YOUR_KV_NAMESPACE_ID"is fine forwrangler dev --localbut will fail at deploy. Either call this out ingetting-started.mdnext to the deploy instructions, or use akv_namespaces[].preview_idso dev still works while the prod id stays empty/required. Follow-up issue is fine.
📝 note
worker::Env::clone()per request (platform.rs:444-451) — see inline.
Prior-review status
- ✅ Cloudflare routes skip middleware → fixed (new
src/middleware.rs+ regression tests intests/routes.rs:23-75). ⚠️ send_asyncserializes / no deadline → only documentation added; the underlying serialization and missing per-request timeout remain (see blocking findings above).- ✅ Backend names too coarse → fixed (
platform.rs:62-77includes scheme, host, port, timeout_ms, cert flag). - ✅ Stale ignore message → fixed (
integration.rs:139,147).
CI Status
- cargo fmt: FAIL
- format-docs: FAIL
- integration tests: FAIL
- cargo check (cloudflare native + wasm32-unknown-unknown): PASS
- cargo test: PASS
- cargo test (axum native): PASS
- vitest: PASS
- format-typescript: PASS
- browser integration tests: PASS
aram356
requested changes
May 6, 2026
Collaborator
aram356
left a comment
aram356
left a comment
There was a problem hiding this comment.
Summary
Adds the trusted-server-adapter-cloudflare crate (edgezero handle adapters for Config/KV/secrets, CloudflareHttpClient, geo extraction from cf-* headers), an integration test environment, and a web_time migration in core. Crate scaffolding and middleware wiring are sound after the prior two review rounds, but CI is red on three jobs (one introduced by this PR) and several correctness/quality concerns from the earlier reviews remain open.
Note: the PR base is feature/edgezero-pr16-axum-dev-server, not main. Two of the four CI failures listed below are inherited from the base — but they still block merge to main.
Blocking
🔧 wrench
cargo fmtfails oncrates/integration-tests/tests/environments/cloudflare.rs— introduced by this PR. Theintegration-testscrate is in the workspaceexcludelist, socargo fmt --allfrom the repo root passes; the CI rustfmt action checks every Rust file. See inline comment.use worker::*;violatesCLAUDE.md—crates/trusted-server-adapter-cloudflare/src/lib.rs:7. See inline.- Integration tests fail with orphaned
workerdprocesses —crates/integration-tests/tests/environments/cloudflare.rs:104-109only kills thewranglerparent;workerdgrandchildren leak. Likely root cause of thetest_all_combinations/test_nextjs_axumfailures observed in CI. See inline. - Per-provider auction timeout silently dropped on Cloudflare (still open from prior review) —
platform.rs:277. See inline. select()warning logsn-1times per multi-provider auction (still open from prior review) —platform.rs:337. See inline.- CI:
clippy::type_complexityfailure —crates/trusted-server-adapter-axum/src/platform.rs:194. Inherited from PR-16 base; cannot merge tomainuntil fixed in the base chain. Suggested:type ResponseTriplet = (u16, Vec<(String, Vec<u8>)>, Vec<u8>);. - CI:
format-docsfailure —docs/guide/architecture.mdanddocs/guide/getting-started.md. Inherited from PR-15 (6d1184d6). Same merge-to-main concern.
Non-blocking
🤔 thinking
send_asynceagerly awaits → multi-provider auctions degrade tosum(latencies)— see inline atplatform.rs:315.Body::Streamsilently dropped on outbound — see inline atplatform.rs:235.- Top-level
worker dispatch error: {e}may leak internals to clients — see inline atlib.rs:20. - Eager full-response buffering with no cap — see inline at
app.rs:96. - Confirm
worker::Error::Displaydoes not leak the secret value — see inline atplatform.rs:391. - Hardcoded port 8787 conflicts with local
wrangler dev— see inline atcloudflare.rs:23.
♻️ refactor
get_fallbackandpost_fallbackare near-duplicates — see inline atapp.rs:363.auth_middleware_rejects_with_401_when_credentials_requireddoesn't actually test 401 — see inline attests/routes.rs:75.
📝 note
- Stale
#[ignore]reason on Cloudflare integration tests — see inline atintegration.rs:139. - The PR base also has red CI (
format-docs,clippy::type_complexity). Recommend rebasing on the merged base before re-requesting review so CI status reflects only this PR's deltas.
⛏ nitpick
Method::from(... .to_ascii_uppercase())— see inline atplatform.rs:216.wrangler.tomlshipsid = "REPLACE_WITH_YOUR_KV_NAMESPACE_ID"—wrangler devrefuses to start until edited. Worth a one-line note inCLAUDE.mdor atopbuild.sh.- The
cloudflarefeature in this crate'sCargo.tomlis effectively redundant —[target.'cfg(target_arch = "wasm32")'.dependencies]already pullsworkerand the adapter feature unconditionally on wasm32, so--features cloudflareonly matters forcargo checkon native. The doc comment names the use case but a one-line "why it isn't default" note would help future readers.
CI Status
- cargo fmt: FAIL (introduced by this PR —
crates/integration-tests/tests/environments/cloudflare.rs) - clippy: FAIL (inherited from base — axum
type_complexity) - rust tests (fastly, axum, cloudflare): PASS
- vitest: PASS
- format-docs: FAIL (inherited from base)
- format-typescript: PASS
- integration tests: FAIL (
test_all_combinations,test_nextjs_axum— likely the workerd cleanup issue) - browser integration tests: PASS
- cargo check (cloudflare native + wasm32-unknown-unknown): PASS
Blocking: - Fix cargo fmt failure in integration-tests cloudflare.rs (long import, struct init) - Replace `use worker::*` with explicit imports in adapter lib.rs - Return generic 500 body from top-level dispatch error; log detail server-side - Fix workerd process-group leak: spawn wrangler as group leader, killpg on drop - Use find_available_port() for wrangler dev instead of hardcoded 8787 - Reject multi-provider fan-out in select() with PlatformError::HttpClient instead of a noisy warn; per-provider timeout is now enforced by failing loudly rather than silently degrading to sum(latencies) - Fix clippy::type_complexity in axum platform.rs with ResponseTriplet alias - Fix docs prettier formatting Non-blocking: - Return Err from execute() on Body::Stream outbound instead of silent empty body - Assert unreachable! on Body::Stream in send_async (execute always returns Once) - Extract shared dispatch() helper from get_fallback/post_fallback duplicates - Rename auth test to auth_middleware_runs_in_chain_for_protected_routes - Update stale #[ignore] reasons for Cloudflare integration tests
- Merge latest PR16 base (bff1adc, 314575b): removes unused backend.rs and fixes .gitignore comment - Use SpawnedRequestResult type alias from PR16 (includes Result wrapper) - Add check-cloudflare alias for wasm32-unknown-unknown target check - Add clippy-cloudflare alias to complete the per-adapter clippy pattern
…ezero-pr17-cloudflare-adapter
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
3 participants
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
Changes
Closes
Closes #498
Test plan
Checklist
Known deferred items