Sync EdgeZero PR #257 updates#761
Conversation
f078d3b to
396d270
Compare
41e7268 to
b6c4a79
Compare
b6c4a79 to
159c731
Compare
Point the edgezero dependencies at the upstream main branch and bump fastly/log-fastly to 0.12 to match edgezero's pinned version. Forward-port the body and TLS APIs to the newer surface: - Body::into_bytes() now returns Option<Bytes>; buffered-body call sites use unwrap_or_default() to preserve prior semantics. - fastly 0.12 get_tls_protocol()/get_tls_cipher_openssl_name() return Result<Option<&str>>; adapter call sites use .ok().flatten(). Resync the excluded integration-tests lockfile to the same versions.
ChristianPavilonis
left a comment
There was a problem hiding this comment.
Automated review: I reviewed PR #761 against main, focusing on the EdgeZero/Fastly API update, crate/path renames, build and CI wiring, and the touched runtime code paths. CI is green and I did not find any blocking correctness, security, data-loss, authorization, or severe compatibility issues. I left one low-severity documentation/operational comment.
The wrapper script lives at crates/trusted-server-openrtb/generate.sh, not in the codegen crate. Point the example at the actual script path.
ChristianPavilonis
left a comment
There was a problem hiding this comment.
Automated review: I reviewed PR #761 against main, focusing on the EdgeZero/Fastly API migration, Body/TLS API call sites, crate renames, CI/script path updates, lockfiles, OpenRTB regeneration paths, and touched runtime/security-sensitive code. CI is green, existing CodeQL/README feedback has already been addressed or is non-blocking, and I did not find any blocking correctness, security, data-loss, authorization, or severe compatibility issues in this pass.
ChristianPavilonis
left a comment
There was a problem hiding this comment.
Review Summary
😃 Approved. I reviewed PR #761 against main, with another pass over the rename/tooling/doc fallout after the EdgeZero/Fastly sync. CI is green.
🔧 I left a few non-blocking inline doc suggestions using GitHub suggestion blocks so they can be applied directly.
📝 One additional cleanup is worth doing but could not be attached as an inline suggestion because those files are not part of the PR diff: tracked Claude helper files still reference the deleted crates/js/lib path (for example .claude/commands/check-ci.md, .claude/commands/verify.md, .claude/agents/verify-app.md, .claude/agents/build-validator.md). Please replace remaining crates/js references with crates/trusted-server-js so local/agent verification helpers keep working after the crate rename.
… paths Apply the approving review's inline doc suggestions and the crate-rename cleanup: - integration-guide: describe directory-based entrypoint discovery (src/integrations/<id>/index.ts) instead of arbitrary .ts files, and point the Testlight key file at testlight/index.ts. - creative-processing: fix the generated bundle output directory to crates/trusted-server-js/dist (not lib/dist). - .claude helper commands/agents: replace remaining crates/js references with crates/trusted-server-js so local/agent verification helpers keep working after the crate rename.
|
Addressed the review-summary cleanup in 32f45d5: replaced the remaining |
Reconcile main (Osano CMP mirror #773, fastly extraction out of core, EC/storage refactors) with this branch's EdgeZero/fastly-0.12 bump. Resolution decisions: - Keep this branch's dependency bump: fastly/log-fastly 0.12 and edgezero git deps tracking branch=main (over main's pinned rev + 0.11.12), plus the trusted-server-* crate renames. - Take main's newer runtime code for conflicted files (EC http:: migration, settings, geo, testlight, adapter restructure), then forward-port it to the edgezero-main / fastly-0.12 API surface: - Body::into_bytes() now returns Option<Bytes>; buffered sites use unwrap_or_default() (batch_sync, pull_sync, identify/testlight tests). - fastly 0.12 get_tls_protocol()/get_tls_cipher_openssl_name() return Result<Option>; call sites use .ok().flatten(). - edgezero-main moved adapter symbols into submodules (request::into_core_request, config_store::FastlyConfigStore, context::FastlyRequestContext) and made router oneshot()/IntoResponse into_response() fallible; dispatch and test sites updated accordingly. - Accept main's deletion of core storage/secret_store.rs (fastly removal). - Fix a directory-rename gap: main added Osano JS under the old crates/js path; relocated into crates/trusted-server-js so the bundle is discovered. Verified: cargo fmt/clippy/test --workspace, wasm release build, JS vitest + format. All green.
…r-257 Bring in main #797 (tester cookie clear endpoint) and forward-port its new code to the edgezero-main / fastly-0.12 surface: - Wrap the two new block_on(router.oneshot(...)) test sites with the fallible-oneshot .expect() pattern. Also finish the crates/js -> crates/trusted-server-js rename fallout that main's design docs and a sourcepoint.rs comment still referenced, so no crates/js path remains anywhere in the tree. Verified: cargo fmt/clippy/test --workspace, JS vitest, docs prettier. All green.
The Integration Tests CI job runs scripts/check-integration-dependency-versions.sh, which requires the excluded trusted-server-integration-tests crate to resolve the same shared direct-dependency versions as the workspace. Regenerating its lockfile during the EdgeZero sync picked up log 0.4.33 while the workspace stays on 0.4.32. Downgrade log in the integration lockfile so the parity check passes.
The EdgeZero entry-point step in integration-tests.yml still pointed at the pre-rename crates/integration-tests path for both --manifest-path and VICEROY_CONFIG_PATH, so the job failed with 'manifest path does not exist'. Point them at crates/trusted-server-integration-tests to match the rename (the legacy integration-tests job already used the new path).
The merge resolution kept this branch's looser toml = "1.0" requirement; main uses "1.1". Both resolve to the same locked 1.1.x, so the lockfile is unchanged, but match main to avoid widening the version range.
The integration-tests lockfile was regenerated fresh during the EdgeZero sync and picked up newer patch/minor versions of crates that also resolve through trusted-server-core (http, bytes, uuid, lol_html, config, brotli, chrono, regex, time, etc.), so the tests linked slightly different versions than the production build ships. Pin those shared transitive crates back to the workspace-resolved versions (45 crates aligned). Six remain newer (js-sys, num-conv, wasm-bindgen family) because the integration crate's own dependency tree constrains them to a higher version; those cannot be downgraded without breaking resolution. The CI direct-dependency parity check still passes and the crate compiles --locked on the host target.
Extend check-integration-dependency-versions.sh with a lockfile-based transitive parity check: every (name, version) the workspace lockfile resolves must also be present in the integration lockfile for any shared crate, so the integration tests exercise the same dependency versions the production build ships. This catches accidental drift when the integration lockfile is regenerated and silently bumps shared crates to newer versions than production uses (the failure mode that produced the original log/http drift during the EdgeZero sync). A small documented allowlist exempts crates the integration crate's own dependency tree forces to a different version (js-sys / wasm-bindgen family and num-conv, pulled newer by reqwest's wasm tooling; itertools, whose workspace 0.10.x line the integration tree never resolves). Also align four more shared transitives that were previously skipped as multi-version (bitflags, getrandom, hashbrown, syn) down to the workspace versions, so the enforced allowlist stays minimal. The check is parsed directly from the lockfiles (no cargo invocation), and a negative test confirms it flags an injected http drift and passes after restore.
Resolve conflicts from the EdgeZero PR #257 sync on main (#761): - Cargo.toml: adopt main's crate renames, fastly/log-fastly 0.12, and edgezero tracking the upstream main branch; keep the branch's glob dep. - publisher.rs: keep both sides' new tests; forward-port test body extraction to the Option-returning Body::into_bytes API and drop the now-unused response_body_string helper superseded by the branch tests. - auction/endpoints.rs: unwrap_or_default the Option-returning into_bytes. - Relocate the new GPT SPA tests into the renamed crates/trusted-server-js tree and refresh stale crates/js doc-comment paths. - Take main's CI-validated integration-tests lockfile (deps unchanged on the branch).
The EdgeZero sync (#761) renamed crates/js and crates/integration-tests to crates/trusted-server-*. The old directories still hold local-only build artifacts (node_modules, target, dist) whose gitignore rules moved with the rename, so git now sees them as untracked. Ignore the defunct paths until the directories are removed from disk.
Summary
Syncs this branch with the EdgeZero tooling/dependency updates and standardizes
crate naming, rebased on top of
main. Since this branch was opened,mainlanded its own HTTP-types migration (PR 12 / PR 13 — #624, #626) and additional
features (DataDome server-side protection #769, configurable integration proxy
paths #759, image proxy fixes, and several test-coverage additions). The
overlapping work has been reconciled in favor of
main's newer architecture,so this PR now carries only the changes that are genuinely additive over
main.EdgeZero dependency + toolchain
edgezero-*git dependencies at the upstreammainbranch(rather than a pinned rev), so this app tracks current EdgeZero.
fastly/log-fastlyto 0.12 to match the version EdgeZeromainpins.
edgezero_core::body::Body::into_bytes()now returnsOption<Bytes>(
Noneonly for streaming bodies). Buffered-body call sites use.unwrap_or_default(), preserving priorBytessemantics. Forsize-bounded reads,
Body::into_bytes_bounded(max)is now available.get_tls_protocol()/get_tls_cipher_openssl_name()nowreturn
Result<Option<&str>>; adapter call sites use.ok().flatten().1.91.1 → 1.95.0(rust-toolchain.toml+.tool-versions), Fastly CLI13.3.0 → 15.1.0, Viceroy0.16.4 → 0.17.0,add Wasmtime
44.0.1.Crate naming
trusted-server-*prefix:crates/js→crates/trusted-server-jscrates/openrtb→crates/trusted-server-openrtbcrates/openrtb-codegen→crates/trusted-server-openrtb-codegencrates/integration-tests→crates/trusted-server-integration-testsclippy.tomlto the EdgeZero-style configuration.Reconciliation decisions (vs. the original PR intent)
main's HTTP-types migration supersedes thisbranch's older
fastly::-based integration code, so the conflicted files takemain's versions (thehttp::/ async EdgeZero platform surface).restriction = "deny"workspace posture. That produced large, unrelated churnagainst
main's current code, so the workspace lint levels stay atmain's posture. (clippy.tomlconfig is still refreshed;clippy -D warningsis green.)Cargo.lockand the excludedtrusted-server-integration-tests/Cargo.lockare resynced to EdgeZeromain--locked.Known limitation (tracked separately)
trusted-server-coreis not yet platform-agnostic: it still has anunconditional
fastlydependency, with the coupling incompat.rs(afastly::Request↔http::Requestbridge) andbackend.rs(
fastly::backend::Backend). Onlytrusted-server-adapter-fastlyexists today;the workspace declares (currently unused)
edgezero-adapter-axum/edgezero-adapter-cloudflare, and there is no Spin adapter. Extracting theFastly coupling out of core so it can run on the other EdgeZero adapters
(Cloudflare Workers, Spin, Axum) is intentionally out of scope for this PR
and will be done separately.
Verification
cargo fmt --all -- --checkcargo clippy --workspace --all-targets --all-features -- -D warningscargo test --workspacecargo build --package trusted-server-adapter-fastly --release --target wasm32-wasip1cargo metadata --locked(root)cargo metadata --locked(crates/trusted-server-integration-tests)cargo metadata --locked(crates/trusted-server-openrtb-codegen)cd crates/trusted-server-js/lib && npx vitest runcd crates/trusted-server-js/lib && npm run formatcd docs && npm run formatReferences stackpop/edgezero#257