Remove fastly dependency from trusted-server-core (PR 15)#635
Conversation
BackendConfig uses fastly::backend::Backend directly, making it incompatible with the platform-portability goal. This commit: - Copies backend.rs verbatim into trusted-server-adapter-fastly, updating the one crate-internal import path - Adds url dependency to the adapter Cargo.toml - Rewires platform.rs and management_api.rs to use crate::backend - Removes pub mod backend from trusted-server-core/lib.rs - Migrates aps.rs, adserver_mock.rs, and prebid.rs off the deleted BackendConfig, using context.services.backend().ensure() for registration and a new predict_backend_name_for_url free function in integrations/mod.rs for stateless name prediction cargo check --workspace passes with zero errors.
All callers were migrated to predict_backend_name_for_url in core's integrations module. Remove the now-unused method and update the parse_origin doc comment that referenced it.
Remove crates/trusted-server-core/src/storage/ entirely (config_store.rs, secret_store.rs, mod.rs). All call sites migrated to PlatformConfigStore / PlatformSecretStore traits. Also drop the now-broken intra-doc links in trusted-server-adapter-fastly/src/platform.rs that referenced the deleted types.
Replace direct fastly::kv_store::KVStore usage in consent/kv.rs with a platform-neutral ConsentKvOps trait. The trait has four sync methods (load_entry, save_entry_with_ttl, fingerprint_unchanged, delete_entry) keeping the consent pipeline synchronous. - consent/kv.rs: add ConsentKvOps trait; replace load_consent_from_kv / save_consent_to_kv / delete_consent_from_kv with load_consent / save_consent (trait-based); remove open_store / fingerprint_unchanged private fns; drop ConsentKvMetadata / metadata_from_context (metadata API was Fastly-specific; fingerprint now lives in the body fp field); add StubKvOps + integration tests - consent/mod.rs: add kv_ops field to ConsentPipelineInput; update try_kv_fallback and try_kv_write to consume kv_ops instead of store_name; update all struct literal sites - publisher.rs: add kv_ops: Option<&dyn ConsentKvOps> parameter to handle_publisher_request; wire it into ConsentPipelineInput and use it for the SSC-revocation delete - adapter/platform.rs: add FastlyConsentKvStore wrapping fastly::kv_store::KVStore implementing ConsentKvOps via the sync API - adapter/app.rs + main.rs: open FastlyConsentKvStore from settings.consent.consent_store and pass as kv_ops to handle_publisher_request
Remove `fingerprint_unchanged` from `ConsentKvOps` trait so the common case (consent unchanged) costs one KV read instead of two. `save_consent` now loads the entry once and compares the fingerprint inline. Extract `open_consent_kv` helper in `app.rs` to deduplicate the two identical KV-open blocks. Replace bare `unwrap()` with descriptive `expect` messages in consent KV tests. Run `cargo fmt --all` to fix all whitespace issues.
Resolved five conflicts: - Deleted compat.rs (PR15 goal: remove Fastly from core) - integrations/mod.rs: kept both PR15's ensure_integration_backend_with_timeout / predict_backend_name_for_url and PR14's INTEGRATION_MAX_BODY_BYTES constant - integrations/adserver_mock.rs, aps.rs, prebid.rs: merged collect_body import (PR14) with ensure_integration_backend_with_timeout / predict_backend_name_for_url imports (PR15) into single use statements
aram356
left a comment
aram356
left a comment
There was a problem hiding this comment.
Summary
This PR delivers its stated goal — trusted-server-core is now fully fastly-free and the consent pipeline is abstracted behind a minimal ConsentKvOps trait. Local verification passes (fmt, clippy -D warnings, cargo test --workspace, wasm32-wasip1 release build).
Three concerns merit changes before merge: a diagnostic regression in predict_backend_name_for_url, duplication of the security-adjacent SPOOFABLE_FORWARDED_HEADERS list, and a test-coverage regression in the relocated adapter compat.rs. The remaining items are non-blocking observations and follow-up candidates.
Note: this PR targets
feature/edgezero-pr14-entry-point-dual-path, notmain; review scope is the 31-file PR-15-specific delta (+2230/-2165).
Blocking
🔧 wrench
- Diagnostic regression in
predict_backend_name_for_url— inline comment atcrates/trusted-server-core/src/integrations/mod.rs:135 SPOOFABLE_FORWARDED_HEADERSduplicated across core and adapter — inline comment atcrates/trusted-server-adapter-fastly/src/compat.rs:18- Adapter
compat.rshas zero tests after relocation (14 security-relevant tests dropped) — inline comment atcrates/trusted-server-adapter-fastly/src/compat.rs:82
Non-blocking
🤔 thinking
- KV errors logged at
debuglevel as "lookup miss" — inline comment atcrates/trusted-server-adapter-fastly/src/platform.rs:420 - Backend-name compute logic duplicated between
predict_backend_name_for_url(core) andBackendConfig::compute_name(adapter) — inline comment atcrates/trusted-server-core/src/integrations/mod.rs:130
🌱 seedling
- Redundant KV read on fallback+save path — inline comment at
crates/trusted-server-core/src/consent/kv.rs:267 - Hardcoded
certificate_check: trueinensure_integration_backend(integrations/mod.rs:69) andensure_integration_backend_with_timeout(integrations/mod.rs:114). No regression vs. pre-PR behaviour, but the siblingpredict_backend_name_for_urldoes takecertificate_check: bool— asymmetric. If any integration ever needs a self-signed upstream (test/on-prem), these helpers would need to grow that parameter.
📌 out of scope
migration_guards.rsnot extended — migration_guards.rs:27-46 only covers 12 files and a narrow regex (Request|Response|http::Method|StatusCode|mime::APPLICATION_JSON). With core'sfastlydep now removed, this guard should either broaden to\bfastly::across all core files, be replaced by a Cargo-metadata dependency-presence test (more robust, no false negatives), or be deleted as structurally redundant. Follow-up issue.FastlyConsentKvStore::opensilent failure — inline comment atcrates/trusted-server-adapter-fastly/src/platform.rs:409
📝 note
- Stale "PR 15" forward references:
- adapter-fastly/src/app.rs:150: "will be removed when
legacy_mainis deleted in PR 15". This is PR 15 andlegacy_mainwas not deleted. - adapter-fastly/src/main.rs:113: "
compat.rswill be deleted in PR 15 once this legacy path is retired". This is PR 15;compat.rswas moved (not deleted) and the legacy path was not retired.
Update comments to reference the actual future PR number or drop the promise.
- adapter-fastly/src/app.rs:150: "will be removed when
⛏ nitpick
- Unnecessary
u16type suffix — inline comment atcrates/trusted-server-core/src/integrations/mod.rs:140
CI Status
- fmt: PASS
- clippy (
-D warnings): PASS - rust tests (
cargo test --workspace): PASS (800+ tests) - wasm32-wasip1 release build: PASS
- PR-reported checks (browser integration, integration tests, prepare integration artifacts): PASS
…mpat tests - predict_backend_name_for_url now returns Result<String, Report<TrustedServerError>> instead of Option<String>; call sites use inspect_err(...).ok() to preserve the Option<String> trait interface while logging the actual failure reason - Promote SPOOFABLE_FORWARDED_HEADERS to pub in http_util; replace inline copy in compat.rs with a use import — single source of truth for the strip list - Add sanitize_fastly_forwarded_headers_strips_spoofable test (all 4 entries + Host preserved) and to_fastly_response_with_streaming_body_produces_empty_body test (pins silent-drop behaviour) to compat.rs - Change Consent KV non-ItemNotFound error log from debug "lookup miss" to warn "lookup failed" for operational visibility - Drop stale "will be removed in PR 15" forward references from app.rs and main.rs - Drop unnecessary u16 type suffixes on port literal defaults
ChristianPavilonis
left a comment
ChristianPavilonis
left a comment
There was a problem hiding this comment.
Summary
- Submitted reviewer-approved findings for consent/KV handling, backend naming consistency, and Fastly wiring.
- Verdict: REQUEST_CHANGES (includes a P1 finding).
Findings moved to review body
The following approved findings were included in the review body because GitHub could not resolve their requested inline locations in the current diff:
-
🤔 Consent bootstrap logic is now duplicated across auction and publisher paths (
crates/trusted-server-core/src/auction/endpoints.rs:61)
Both handlers now repeat the same sequence: parse cookies, generate/read synthetic ID, geo lookup with identical error handling, and build ConsentPipelineInput with kv_ops. This duplication is likely to drift as the consent pipeline evolves; this PR already had to touch both call sites just to thread the new abstraction through.
Suggestion: Extract a small shared helper that prepares request-scoped consent state once and returns the ConsentContext plus any reused intermediates such as synthetic_id, geo, and cookie_jar. -
⛏ Public docs/comments still lag the new kv_ops contract (
crates/trusted-server-core/src/auction/endpoints.rs:21)
The new kv_ops parameter materially changes how consent persistence is enabled, but the public docs still describe these handlers mostly in their pre-PR form, and one consent comment still says fallback requires consent_store rather than kv_ops.
Suggestion: Update the affected doc comments to explain that consent KV fallback/write-through occurs only when a concrete kv_ops implementation is supplied, and adjust the stale internal comment in consent/mod.rs.
|
Fixed the two findings GitHub moved into the review body in 3a53f33.
Fresh verification on this commit:
|
…ro-pr15-remove-fastly-core
ChristianPavilonis
left a comment
ChristianPavilonis
left a comment
There was a problem hiding this comment.
Summary
Approving this PR. The main refactor looks solid: trusted-server-core is decoupled from Fastly and the adapter now owns the Fastly-specific backend, KV, and compatibility wiring.
I left 2 non-blocking follow-up comments inline for logging/test coverage.
Incorporates the round-3 review fixes from PR14 (E2E tests, finalize-header coverage for unregistered methods, allowed_domains documentation) and the broader PR14 refactoring (EC ID rename from synthetic_id, kv_ops removal from handle_auction, runtime_services_for_consent_route consent KV approach, async tokio tests in proxy/orchestrator, PublisherResponse streaming). Conflict resolutions: - Take PR14's refactored core files (endpoints, orchestrator, consent, publisher, proxy, platform/mod, testlight) - Keep PR15's deletion of core/compat.rs and storage/mod.rs (Fastly code stays in adapter, not core) - Add from_fastly_response to adapter compat.rs (needed by EdgeZero path for entry-point finalize-header wrap introduced in PR14) - Remove obsolete FastlyConsentKvStore / open_consent_kv / ConsentKvOps references from platform.rs and app.rs (trait removed in PR14) - Fix orchestrator backend_name call to pass services (PR15 trait signature) - Add minimal backend.rs and storage/mod.rs stubs to core for DEFAULT_FIRST_BYTE_TIMEOUT and storage::kv_store module path - Add tokio as dev-dependency to trusted-server-core for async tests
aram356
left a comment
aram356
left a comment
There was a problem hiding this comment.
Summary
Re-review of PR 15 after the most recent commits (Apr 27). The core refactor goal is achieved — trusted-server-core no longer imports fastly, and the migration guard test (migration_guards.rs) durably enforces the invariant. Build, clippy, fmt, and the full Rust test suite (878 tests) pass locally; GitHub CI is green.
However, the merge with PR14 removed the ConsentKvOps / FastlyConsentKvStore abstraction without replacing the wiring on the EdgeZero entry path, leaving a privacy-sensitive gap: when edgezero_enabled = true and consent_store is configured, consent revocation deletes silently fail. The PR description still describes the pre-merge design and is now misleading.
Requesting changes to address (or explicitly accept and document) the EdgeZero consent-KV gap and to update the PR description before merge.
Blocking
❓ question
Consent KV is not opened on the EdgeZero entry path — crates/trusted-server-adapter-fastly/src/app.rs:92
build_state() constructs kv_store = Arc::new(UnavailableKvStore) unconditionally, and build_per_request_services (lines 111–127) hands that same store to every handler. The runtime_services_for_consent_route helper in crates/trusted-server-adapter-fastly/src/main.rs:307-323 (which actually opens the configured consent.consent_store) is only called from the legacy route_request.
Consequence: when edgezero_enabled = true and [consent] consent_store = "..." is configured, every services.kv_store() call in core returns KvError::Unavailable. In particular, the revocation delete in crates/trusted-server-core/src/publisher.rs:765 silently fails (Failed to delete consent KV entry … Unavailable) — the EC cookie expires, but the persisted consent data is not removed. KV-backed consent fallback also never engages on the EdgeZero path. This is a privacy/compliance concern (right-to-erasure).
This appears to be a regression from the merge resolution that removed FastlyConsentKvStore / open_consent_kv after merging PR14. Either (a) the EdgeZero path needs an equivalent of runtime_services_for_consent_route (open the consent store inside build_per_request_services or wrap the auction/publisher handlers), or (b) the EdgeZero path should fail loudly at startup when consent.consent_store is configured but cannot be opened.
Was this intentional, or did it slip through the merge?
🔧 wrench
PR description is stale and contradicts the merged code — The body still claims:
- "Introduces a sync
ConsentKvOpstrait in core" - "
FastlyConsentKvStorein the adapter implements the trait" - "Threads
kv_ops: Option<&dyn ConsentKvOps>throughConsentPipelineInput"
None of these exist after the PR14 merge — the actual code uses PlatformKvStore directly via RuntimeServices, and the consent KV is opened only on the legacy path. The "Changes" table for consent/kv.rs, consent/mod.rs, app.rs, main.rs, and platform.rs describes work that was reverted. Reviewers and git log consumers will be misled. Please update the PR description to reflect the post-PR14-merge reality before merging.
Non-blocking
🤔 thinking
UnavailableKvStore swallows configuration errors silently on the EdgeZero path — Combined with the question above, configuring consent_store = "missing-store" on the EdgeZero path produces no startup error and only per-request warn! lines. The legacy path correctly fails the auction/publisher routes with 503 (verified by route_tests.rs:184). Worth validating consent-store availability at startup so misconfiguration cannot silently disable revocation.
⛏ nitpick
Pre-existing: core is on edition = "2021" — CLAUDE.md mandates 2024 edition. Not introduced by this PR; flagged only for awareness/follow-up.
CI Status
- fmt: PASS (local + GitHub)
- clippy: PASS (local,
-D warnings) - rust tests: PASS (878 tests, local)
- GitHub CI: PASS (browser tests, integration tests, prepare integration artifacts)
aram356
left a comment
aram356
left a comment
There was a problem hiding this comment.
Summary
Re-review of PR 15 against HEAD 725ccdc2. Local verification passes (fmt, clippy -D warnings, 878 + 45 unit tests across the changed crates) and GitHub CI is green.
The core refactor goal is achieved — trusted-server-core no longer imports fastly and the migration guard test enforces the invariant. However, the previously flagged EdgeZero consent-KV regression (Apr 30 review) is still present at the same line, and the PR description still describes the pre-PR14-merge design that was reverted. Requesting changes to address (or explicitly accept and document) those before merge.
Blocking
🔧 wrench
-
EdgeZero path silently disables consent KV (privacy/compliance regression) —
crates/trusted-server-adapter-fastly/src/app.rs:92(line outside this PR's diff hunk, so flagging at body level).build_state()hardcodesArc::new(UnavailableKvStore), andbuild_per_request_services(lines 111–127) hands that exact store to every route. There is no analogue ofruntime_services_for_consent_route(main.rs:307-323) on this path.Concrete effect when
edgezero_enabled = trueAND[consent] consent_store = "..."is set:crates/trusted-server-core/src/publisher.rs:765callsdelete_consent_from_kv(services.kv_store(), cookie_ec_id), which hitsUnavailableKvStore.delete()→KvError::Unavailable.storage/kv_store.rs:323-332swallows the error (warn-log only). The EC cookie expires but the persisted consent is not removed — right-to-erasure broken.crates/trusted-server-core/src/auction/endpoints.rs:83-87andpublisher.rs:508-512passSome(services.kv_store())to the consent pipeline, but it's the unavailable store, so consent persistence and KV-backed consent fallback never engage.
Compare with the legacy path (correct):
route_tests.rs:184-260verifies that a missingconsent.consent_storeproduces a fail-closed 503 on/auctionand the publisher fallback viaroute_request. EdgeZero has no equivalent and silently degrades. Same blocking finding as the Apr 30 review, at the same line, still present at HEAD725ccdc2.Fix options:
- Open
consent.consent_storeinsidebuild_state()(or inbuild_per_request_services) usingplatform::open_kv_store, and either (a) fail startup if configured-but-unavailable or (b) wrap the consent-touching handlers (/auction, fallback) so they return 503 like the legacy path. - Mirror
runtime_services_for_consent_routeat the EdgeZero handler boundaries.
Either way, please add a counterpart of the existing
configured_missing_consent_store_only_breaks_consent_routestest that drivesTrustedServerApp::routes()so the regression cannot slip through another merge. -
PR description is stale and contradicts the merged code — the body still claims the PR introduces a sync
ConsentKvOpstrait, aFastlyConsentKvStoreadapter implementation, and threadskv_ops: Option<&dyn ConsentKvOps>throughConsentPipelineInput. None of those exist after the PR14 merge — the actual code usesPlatformKvStoredirectly viaRuntimeServices. The "Changes" table forconsent/kv.rs,consent/mod.rs,app.rs,main.rs, andplatform.rsdescribes work that was reverted. Please update the description to reflect the post-PR14-merge reality before merging so reviewers andgit logconsumers aren't misled.
Non-blocking
🤔 thinking
- No EdgeZero-side test for consent-store wiring —
route_tests.rs:184 configured_missing_consent_store_only_breaks_consent_routesonly exercises the legacyroute_request. There is no analogous test that constructsTrustedServerApp::routes()withconsent.consent_storeconfigured. Combined with the blocking finding above, the regression slipped through the PR14 merge with no failing test to catch it. - Both crates still on
edition = "2021"— CLAUDE.md mandates 2024 edition. Pre-existing across the repo, not introduced by this PR; flagged for awareness/follow-up.
📌 out of scope
UnavailableKvStoremakes consent-store misconfiguration invisible at startup — even after the blocking finding is addressed, configured-but-unopenable stores will only surface as per-requestwarn!lines unlessbuild_state()validates store availability at startup. Worth a follow-up to fail loudly whenconsent.consent_storeis set but cannot be opened.
CI Status
- fmt: PASS (local)
- clippy: PASS (local,
-D warnings) - rust tests: PASS (878 + 45 in changed crates)
- GitHub CI: PASS (browser tests, integration tests, prepare integration artifacts)
|
Addressed the body-level blocking findings from the Apr 30 and May 6 reviews in 50dc61f.
Fresh verification before commit:
|
aram356
left a comment
aram356
left a comment
There was a problem hiding this comment.
Summary
This PR cleanly removes the fastly crate dependency from trusted-server-core, relocates the compat/backend layer into the adapter, migrates four integrations (aps, prebid, adserver_mock, sourcepoint) off BackendConfig to the platform-trait helpers, and restores the fail-closed consent KV wiring on the EdgeZero path for /auction and publisher fallback only. CI is green and local fmt/clippy/cargo test --workspace/wasm32-wasip1 build all pass.
Requesting changes on the items below — none are correctness-breaking today, but two of them (the select empty-backend-name fallback and the predict/ensure parameter asymmetry) sit on the bid-correlation path and can silently misbehave if upstream conditions shift.
Blocking
🤔 thinking — empty backend name silently propagates from select
crates/trusted-server-adapter-fastly/src/platform.rs:300-307 — when fastly_resp.get_backend_name() returns None, the code logs a warning and falls back to "". The PlatformResponse then carries backend_name == "", and AuctionOrchestrator correlates responses by exact string match against provider.backend_name(...). A bid response with "" will silently fail correlation — only the warning log signals the issue to operators.
Consider returning PlatformError::HttpClient here instead, so the orchestrator gets an explicit error frame for that bidder rather than a phantom non-match:
let ready = match result {
Ok(fastly_resp) => match fastly_resp.get_backend_name() {
Some(backend_name) => fastly_response_to_platform(fastly_resp, backend_name.to_string()),
None => Err(Report::new(PlatformError::HttpClient)
.attach("select: response has no backend name; correlation impossible")),
},
Err(e) => Err(Report::new(PlatformError::HttpClient)
.attach(format!("fastly select error: {e}"))),
};(Section unchanged in this PR, so flagging in body rather than inline — fix in this PR or open a follow-up.)
🌱 seedling — no compile-time guard that services.kv_store() is the consent KV
crates/trusted-server-core/src/publisher.rs:764-766 — correctness relies on the caller having threaded runtime_services_for_consent_route(...) before handle_publisher_request. Today the wiring is right (legacy route_request and EdgeZero dispatch_fallback's publisher branch both do it), but there's no compile-time guard. If a future entry point forgets, services.kv_store() will be UnavailableKvStore and delete will succeed-as-noop, leaving stale consent KV entries after a user revokes.
A typed ConsentKvStore(Arc<dyn PlatformKvStore>) newtype constructed only by runtime_services_for_consent_route would make the wiring mistake a compile error. Worth doing while this surface is fresh.
Non-blocking
♻️ refactor — duplicate enable_ssl().sni_hostname(self.host) in BackendConfig::ensure
crates/trusted-server-adapter-fastly/src/backend.rs:177-188 — enable_ssl().sni_hostname(self.host) is called once unconditionally, then a second time inside the if self.certificate_check branch before .check_certificate(host). Idempotent on the builder, so functionally fine, but easy to simplify on the file's move into the adapter:
builder = builder.enable_ssl().sni_hostname(self.host);
if self.certificate_check {
builder = builder.check_certificate(self.host);
} else {
log::warn!(
"INSECURE: certificate check disabled for backend: {}",
backend_name
);
}
log::info!("enable ssl for backend: {}", backend_name);Pre-existing in the deleted core copy; the relocation is the natural moment to clean it up. (Section unchanged in the rename diff, so flagging in body rather than inline.)
📝 note — AppState.kv_store is now always UnavailableKvStore
crates/trusted-server-adapter-fastly/src/app.rs:103 and the matching build_per_request_services site — per-route consent stores are opened on demand via runtime_services_for_consent_route, and no other consumer reads services.kv_store(). The field is now a placeholder, but its name reads as if it were the primary KV. Consider either inlining Arc::new(UnavailableKvStore) at the two construction sites, or renaming the field to default_kv_store to signal that it's only the fallback when no consent route override applies.
📝 note — three AuctionProvider::backend_name impls replicate identical bodies
aps.rs, prebid.rs, adserver_mock.rs all paste essentially the same body wrapping predict_integration_backend_name(&self.config.{endpoint,server_url}, …). A default method on AuctionProvider parameterised by (endpoint: &str, integration_id: &'static str) would remove the boilerplate; doing so would also make the dead certificate_check parameter (see the inline ♻️ refactor on integrations/mod.rs) impossible to misuse.
⛏ nitpick — from_fastly_request panics on URL parse failure
crates/trusted-server-adapter-fastly/src/compat.rs:11-28 — the deleted crates/trusted-server-core/src/compat.rs parsed the URL with a / fallback and warning log; this version .expects. Fastly delivers validated URLs in practice, and the # Panics doc is honest, so this is defensible — but the previous "log + continue" was strictly more defensive on a path that processes every inbound request. Preference call.
CI Status
- fmt: PASS
- clippy: PASS (no warnings)
- rust tests: PASS (950 + 54 + 21 + 3, doctests included)
- wasm32-wasip1 release build: PASS
- GitHub Actions: PASS (browser-integration, integration, prepare-integration-artifacts)
migration_guards.rsinvariant (nofastly::Request/Response/...in migrated core modules): PASS- Verified zero
use ... fastlyimports remain anywhere undercrates/trusted-server-core/src/
Blocking: - select(): return Err(PlatformError::HttpClient) when get_backend_name() returns None instead of silently propagating "" and losing bid correlation - Remove dead certificate_check: bool from predict_integration_backend_name; hardcode true inside to match ensure_integration_backend_with_timeout so predict/ensure cannot silently diverge and break orchestrator correlation Non-blocking: - Fix duplicate enable_ssl().sni_hostname() in BackendConfig::ensure; call once unconditionally, then add check_certificate() conditionally - Add debug_assert!(false) in to_fastly_response Stream arm to catch caller misuse in debug production builds; gated with #[cfg(not(test))] so the behavior-documentation test still passes - Add integration-route assertion to edgezero_missing_consent_store test: GET /integrations/datadome/tags.js must not return 503 when consent KV is misconfigured, locking in that integration proxy bypasses consent gating - Rename AppState.kv_store -> default_kv_store to signal it is only the fallback when no per-route consent KV override applies
Conflicts resolved: - app.rs: keep PR15's default_kv_store field rename; use PR14's build_state_from_settings in the test helper - compat.rs (modify/delete): accept PR15 deletion — file was relocated to the adapter crate as part of removing fastly from core - adserver_mock.rs, aps.rs, prebid.rs: PR15 wins — use ensure_integration_backend_with_timeout for auction-scoped timeouts and keep predict_integration_backend_name import - sourcepoint.rs: merge both sides — keep ensure_integration_backend (PR15, CDN proxy backend) and collect_response_bounded (PR14, streaming response fix); add missing None timeout arg - proxy.rs: PR15 wins on import; add enforce_max_body_size from PR14 (DEFAULT_FIRST_BYTE_TIMEOUT already available via platform module) - mod.rs: use PR15's integration_backend_spec helper in ensure_integration_backend body; honor PR14's Option<Duration> param by passing it through to the helper
|
All round-1 findings resolved in commit 72ddc42. Blocking — fixed
Non-blocking — addressed
Deferred (acknowledged)
CI: |
3 participants
Summary
fastlycrate imports fromtrusted-server-core, keeping the shared crate platform-agnostic and enforcing that invariant withmigration_guards.rstrusted-server-adapter-fastlyRuntimeServices,PlatformBackendSpec,PlatformKvStore) for core request handling instead of Fastly SDK types/auctionand publisher fallback routes, matching the legacy path's fail-closed503behavior when the store is unavailableChanges
crates/trusted-server-core/src/compat.rscrates/trusted-server-core/src/storage/crates/trusted-server-core/src/geo.rscrates/trusted-server-core/src/storage/kv_store.rsPlatformKvStorerather than Fastly KV typescrates/trusted-server-core/src/consent/mod.rsConsentPipelineInputaccepts an optionalPlatformKvStorereference for KV fallback/write-throughcrates/trusted-server-core/src/platform/crates/trusted-server-core/src/integrations/mod.rscrates/trusted-server-core/src/integrations/{aps,adserver_mock,prebid}.rscrates/trusted-server-core/src/publisher.rsRuntimeServices/PlatformKvStorefor consent KV fallback and revocation deletioncrates/trusted-server-core/Cargo.tomlfastlydependency from corecrates/trusted-server-adapter-fastly/src/compat.rscrates/trusted-server-adapter-fastly/src/backend.rscrates/trusted-server-adapter-fastly/src/platform.rsopen_kv_storeforPlatformKvStorecrates/trusted-server-adapter-fastly/src/app.rscrates/trusted-server-adapter-fastly/src/main.rscrates/trusted-server-adapter-fastly/Cargo.tomlCloses
Closes #496
Test plan
cargo test -p trusted-server-adapter-fastly edgezero_missing_consent_store_breaks_only_consent_routescargo test -p trusted-server-adapter-fastly configured_missing_consent_store_only_breaks_consent_routescargo test -p trusted-server-core --libcargo fmt --all -- --checkcargo clippy --workspace --all-targets --all-features -- -D warningscargo test --workspacecd crates/js/lib && npx vitest runcd crates/js/lib && npm run formatcd docs && npm run formatcargo build --package trusted-server-adapter-fastly --release --target wasm32-wasip1fastly compute serveChecklist
unwrap()in production code; usesexpect("should ...")where neededlogmacros rather thanprintln!