Skip to content

Support optional typed secret paths and Fastly store mappings - #344

Open
ChristianPavilonis wants to merge 4 commits into
mainfrom
feature/typed-static-secret-paths
Open

Support optional typed secret paths and Fastly store mappings#344
ChristianPavilonis wants to merge 4 commits into
mainfrom
feature/typed-static-secret-paths

Conversation

@ChristianPavilonis

@ChristianPavilonis ChristianPavilonis commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add an explicit optional intermediate segment to typed secret paths so disabled or absent feature blocks do not require unrelated secrets.
  • Make Fastly's runtime environment mapping available to custom entry points.
  • Persist non-default Fastly store mappings under service-scoped keys during provision, and make staged deployments mirror the mappings production actually uses.
  • Read Fastly runtime overrides only from the current service namespace; legacy unscoped entries require migration.

This PR is stacked on #316 because Trusted Server currently pins that branch.

Changes

Crate / File Change
edgezero-core Add SecretPathSegment::OptionalField and teach runtime extraction to skip absent or null optional containers atomically.
edgezero-cli Mirror optional-path behavior in secret-leaf discovery and emit non-default store-name mappings into edgezero_runtime_env.
edgezero-adapter-fastly Expose env_config_from_runtime_dictionary, persist non-default logical-to-physical mappings under EDGEZERO__SERVICES__<SERVICE_ID>__* keys in the remote edgezero_runtime_env Config Store, and mirror the selected service’s persisted mappings into its staged runtime-env twin.

Related

Test plan

  • cargo test --workspace --all-targets
  • cargo clippy --workspace --all-targets --all-features -- -D warnings
  • cargo fmt --all -- --check
  • cargo check --workspace --all-targets --features "fastly cloudflare spin"
  • WASM builds: wasm32-wasip1 (Fastly)
  • examples/app-demo workspace: cd examples/app-demo && cargo test --workspace --all-targets
  • Docs build: cd docs && npm run lint && npm run format && npm run build
  • Manual testing via edgezero serve --adapter axum
  • Other: cargo test -p edgezero-core, cargo test -p edgezero-cli, cargo test -p edgezero-macros, native Fastly adapter tests, Fastly WASM checks, live staged runtime mapping verification, and package-specific Clippy checks with -D warnings

Checklist

  • Changes follow CLAUDE.md conventions
  • No Tokio deps added to core or adapter crates
  • Route params use {id} syntax (not :id) — no route changes
  • Types imported from edgezero_core (not http crate)
  • Store wiring goes through registries — no runtime store wiring added
  • New code has tests
  • No secrets or credentials committed

@ChristianPavilonis ChristianPavilonis changed the title feature/typed static secret paths Support optional typed secret paths and Fastly store mappings Aug 24, 2026
@ChristianPavilonis
ChristianPavilonis force-pushed the feature/typed-static-secret-paths branch 2 times, most recently from 2b24957 to 0d6ebf9 Compare August 24, 2026 22:22
@ChristianPavilonis ChristianPavilonis self-assigned this Aug 24, 2026

@prk-Jr prk-Jr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review

Summary

Three separable changes: an OptionalField secret-path segment threaded through core + CLI, env_config_from_runtime_dictionary made public, and Fastly provision/staged-deploy persistence of non-default logical→physical store-name mappings. The secret-path half is clean and symmetric — CLI and runtime walkers grew matching arms with matching tests. The Fastly store-mapping half introduces a new write into a store the ACTIVE version reads, sourced from ambient process env, with no reconciliation of stale entries; that is where the blocking findings are.

All four CI gates pass locally on 0d6ebf9b.

😃 Praise

  • collect_secret_leaf extraction (crates/edgezero-cli/src/config.rs:1636) avoids a fourth copy of the leaf-resolution block, and threading optional_segment separately from field.optional keeps "this segment is optional" distinct from "this leaf is Option<String>" — which makes the required-Field arm provably unchanged.
  • CLI/runtime symmetry maintained exactly: collect_secret_leaves and resolve_secret_field grew the same two arms in the same order, each with a test on both sides. That lockstep is the hard part of this subsystem.
  • staging_entries_from_production kept pure, with overlay_runtime_store_name_entries as a separate testable function.

Findings

Blocking

  • 🔧 provision rewrites live production store resolution from ambient shell envcrates/edgezero-adapter-fastly/src/cli.rs:598. EnvConfig::from_env() (crates/edgezero-cli/src/provision.rs:96) + resolve_kind (provision.rs:145) mean any EDGEZERO__STORES__*__NAME in the operator's shell is written into edgezero_runtime_env, read by the ACTIVE version. New behavior: before this PR provision created the store but wrote no entries into it.
  • 🔧 Stale __NAME mappings never removedcli.rs:3640. Dropping an override leaves the old entry live; production has no equivalent of the staging mirror's upsert-then-delete reconciliation.
  • 🔧 Override validation weaker than the runtime's, fails opencli.rs:3487. Control characters pass the CLI check but are silently rejected by is_blank_or_control (crates/edgezero-core/src/env_config.rs:144), so the runtime falls back to the logical id — wrong store, no diagnostic.
  • OptionalField has no derive pathcrates/edgezero-core/src/app_config.rs:41. The macro only emits Field/ArrayEach, and nested_child_type (crates/edgezero-macros/src/app_config.rs:453) doesn't unwrap Option<T>, so #[app_config(nested)] x: Option<Inner> fails the AppConfigRoot bound. Reachable only from a hand-written impl AppConfigMeta — is that the intended surface?
  • 🔧 Add #[non_exhaustive] to SecretPathSegmentapp_config.rs:35-42. Public enum, exhaustively matchable; this PR is itself the breaking change.
  • 🔧 Docs now contradict the codedocs/guide/configuration.md:393-401 still states that Option<Inner> is unsupported and that "only the leaf's own Option<String> is skippable. A missing or null intermediate object/array … is a ConfigOutOfDate error, not a silent skip — both config validate and the runtime reject it." Both halves are now false for OptionalField. (No inline comment — the file is untouched by this PR.)

Non-blocking

  • 🤔 env::vars() panics on non-UTF-8 environment — cli.rs:3586.
  • ♻️ Hoist that env read into the caller so mirror_production_to_staging stays pure and end-to-end testable — currently the only untested line in the new staging path.
  • 🤔 is_runtime_store_name_key (cli.rs:3465) accepts undeclared store ids, so a typo'd override silently no-ops.
  • #[inline] on env_config_from_runtime_dictionary (crates/edgezero-adapter-fastly/src/lib.rs:184) buys nothing.
  • ⛏ The ```ignore doc example (lib.rs:177-182) references a non-existent MyApp::stores() and can never be caught rotting.
  • 🌱 Test gaps: no coverage of the non-dry-run provision write path, nor of is_runtime_store_name_key rejecting near-miss keys (…__A__B__NAME, …__NAME__EXTRA, lowercase kind). The dry-run test at cli.rs:6753 covers the happy shape only.

CI Status

Run locally against 0d6ebf9b:

  • cargo fmt --all -- --check: PASS
  • cargo clippy --workspace --all-targets --all-features -- -D warnings: PASS
  • cargo test --workspace --all-targets: PASS
  • cargo check --workspace --all-targets --features "fastly cloudflare spin": PASS

The three unchecked boxes in the PR description's test plan (cargo test --workspace --all-targets, the feature check, and examples/app-demo) — the first two pass; examples/app-demo was not run.

Comment thread crates/edgezero-adapter-fastly/src/cli.rs Outdated
Comment thread crates/edgezero-adapter-fastly/src/cli.rs Outdated
Comment thread crates/edgezero-adapter-fastly/src/cli.rs Outdated
Comment thread crates/edgezero-core/src/app_config.rs
Comment thread crates/edgezero-core/src/app_config.rs
Comment thread crates/edgezero-adapter-fastly/src/cli.rs Outdated
Comment thread crates/edgezero-adapter-fastly/src/lib.rs Outdated
Comment thread crates/edgezero-adapter-fastly/src/lib.rs Outdated
Comment thread crates/edgezero-cli/src/config.rs
Comment thread crates/edgezero-core/src/extractor.rs
Base automatically changed from feature/edgezero-deploy-actions to main August 27, 2026 04:01
@aram356
aram356 force-pushed the feature/typed-static-secret-paths branch 2 times, most recently from d4dc05d to 79628ec Compare August 27, 2026 15:18

@aram356 aram356 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review

Summary

Tightly scoped change adding optional secret-path intermediates and Fastly store-name mapping persistence. The CLI-validate ↔ runtime secret-walk symmetry is preserved with mirrored tests on both sides, and the reconciliation deliberately preserves undeclared-id and unrelated runtime entries for shared-store accounts. Two blocking findings around the new provision reconciliation and the deploy-time env overlay; the rest are non-blocking.

Findings

Blocking

  • 🔧 Provision hard-fails in a previously-safe state: reconciliation errors when edgezero_runtime_env is absent remotely even when there is nothing to write (crates/edgezero-adapter-fastly/src/cli.rs:3747) — see inline comment.
  • Scope and documentation of the deploy-time env overlay: the staging mirror applies all EDGEZERO__STORES__*__NAME process env vars, including undeclared ids, and hard-errors on malformed ones (crates/edgezero-adapter-fastly/src/cli.rs:3619) — see inline comment.

Non-blocking

  • 🤔 Divergent invalid-value semantics between EnvConfig::store_name (silent fallback) and runtime_store_name_entries_from_vars (fail closed) — see inline comment at cli.rs:3528.
  • 🤔 Case-normalization gap in is_runtime_store_name_key — lowercase id segments pass validation but are dead at runtime — see inline comment at cli.rs:3498.
  • ♻️ Wrapper triplication in the foo / foo_in / foo_with_cwd helpers — see inline comment at cli.rs:3945.
  • Dry-run noise: one "would remove … if a stale mapping is present" line per declared default-named store — see inline comment at cli.rs:3739.
  • Newly-public API keeps its internal doc voice: env_config_from_runtime_dictionary — see inline comment at lib.rs:179.
  • 🌱 Derive support for optional intermediates: SecretPathSegment::OptionalField is hand-written-meta-only; the AppConfig derive still emits Field for every intermediate, so derived configs with Option<Struct> nesting cannot express this. The doc comment acknowledges it — worth a tracked follow-up to emit OptionalField for Option-typed nested fields.
  • 📝 #[non_exhaustive] semver note: it correctly forces the fail-closed wildcard arm in edgezero-cli, but adding #[non_exhaustive] to the existing public SecretPathSegment enum is a breaking change for any external code matching it exhaustively.

📌 Out of Scope

  • Cloudflare parity: Fastly now persists non-default logical→physical mappings so its runtime resolves them; the Cloudflare adapter resolves __NAME at provision (binding writeback) but nothing persists the mapping for its runtime EnvConfig. If that gap is real, it deserves its own issue.

CI Status

  • fmt: PASS
  • clippy: PASS
  • tests: PASS (cargo test --workspace --all-targets, including the workspace run the PR checklist left unchecked)

Comment thread crates/edgezero-adapter-fastly/src/cli.rs Outdated
Comment thread crates/edgezero-adapter-fastly/src/cli.rs Outdated
Comment thread crates/edgezero-adapter-fastly/src/cli.rs Outdated
Comment thread crates/edgezero-adapter-fastly/src/cli.rs Outdated
Comment thread crates/edgezero-adapter-fastly/src/cli.rs
Comment thread crates/edgezero-adapter-fastly/src/cli.rs Outdated
Comment thread crates/edgezero-adapter-fastly/src/lib.rs Outdated

@prk-Jr prk-Jr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review

Summary

Reviewed at 79628ecc. Three separable changes: the OptionalField secret-path segment threaded through core + CLI, env_config_from_runtime_dictionary made public, and Fastly provision / staged-deploy persistence of non-default logical-to-physical store-name mappings. The secret-path half stays CLI/runtime symmetric for the intermediate case it was written for. The blocking findings are on the mapping half: the reconciliation added since the last round now deletes production mappings while its desired state still comes from ambient process env, and there is a production/staging asymmetry in where the deploy-time overlay applies.

Findings below are new relative to both existing reviews (#5048264125 and #5027983720); items still open from the earlier round are listed separately at the end rather than re-raised as findings.

All five CI gates pass locally on 79628ecc.

😃 Praise

  • fake_fastly_runtime_mapping records $PWD per shell-out, so the new tests assert service-context resolution and not just call shape - see the inline comment at cli.rs:7583.
  • runtime_store_name_reconciliation's declared-scoped delete filter, with the shared-account rationale spelled out in the doc comment and proved by a negative assertion on unrelated runtime entries, is the right shape for a store multiple services can link.
  • Fail-closed wildcard arm in edgezero-cli for the now-#[non_exhaustive] enum: Some((_unsupported, _)) => Err(...) rather than a silent skip.

Findings

Blocking

  • 🔧 provision deletes live production store-name mappings when the override isn't exported - desired state comes from EnvConfig::from_env(), so an unrelated provision run from a shell without the var exported silently repoints the ACTIVE version at the logical default store (cli.rs:3700, inline).
  • Deploy-time __NAME overlay reaches staging only - the production deploy path is a bare fastly compute deploy passthrough, so the same CI env yields different store resolution for the staged and production versions (cli.rs:3619, inline).
  • 🔧 OptionalField in terminal position silently overrides SecretField::optional, and both new leaf arms are untested - probed: optional: false plus a terminal OptionalField returns Ok(()) where the all-required equivalent returns ConfigOutOfDate (extractor.rs:1006 and config.rs:1667, inline).

Non-blocking

  • 🤔 CLI/runtime divergence on a scalar where an object is expected, immediately before an OptionalField - runtime Ok(()), CLI Err("expected a table at \integrations`")`. Safe direction, but the comment asserting exact lockstep was deleted without note (extractor.rs:1030, inline).
  • resolve_leaf's doc comment is now false - crates/edgezero-core/src/extractor.rs:1077-1080 still says "The leaf's parent is a required intermediate, so a non-object parent is always an error". With OptionalField the leaf's parent may be optional. Every other comment in that function was updated in this PR; this one is outside the diff, hence no inline comment.
  • 🤔 push_entries_with_committer's failure text is config push-specific - a partial-write failure during provision tells the operator to re-run config push and explains content-addressed chunk keys (cli.rs:3754, inline).
  • 🤔 Uppercasing the logical id collides case-distinct declared ids - ids = ["Sessions", "sessions"] passes manifest validation but shares one __NAME key, and EnvConfig case-folds on lookup, so both resolve to one physical store (cli.rs:3635, inline).
  • The create_fastly_store_in cwd change is broader than the feature - all three declared-kind creates move into manifest_dir; only the runtime-env store's cwd is asserted (cli.rs:1480, inline).
  • 📝 PR description says "persist store mappings in local Fastly manifests" - the code persists them into the remote edgezero_runtime_env config store, the one the ACTIVE version reads. Nothing writes [local_server.config_stores.edgezero_runtime_env]. A reviewer skimming the description would badly under-read the blast radius; worth correcting so the remote-write scope is visible up front.

📌 Out of Scope

  • Local / Viceroy parity: nothing populates [local_server.config_stores.edgezero_runtime_env], so fastly compute serve resolves logical ids while production now resolves __NAME overrides. Pre-existing (no __NAME entries existed at all before this PR), but this change widens the practical gap between local and production store resolution. Deserves its own issue.

Still open from the earlier review (#5027983720) - listed, not re-raised

  • docs/guide/configuration.md:395-401 still states that Option<Inner> is unsupported and that a missing or null intermediate is a ConfigOutOfDate error "both config validate and the runtime reject". Both halves are now false for OptionalField.
  • env::vars() (cli.rs:3619) still panics on a non-UTF-8 environment - vars_os() is the non-panicking form - and reading it inline still keeps mirror_production_to_staging out of end-to-end test reach.
  • #[inline] on the newly-pub env_config_from_runtime_dictionary (lib.rs:177) buys nothing on a wasm target; pub + #[must_use] are the parts that serve the stated goal.

CI Status

Run locally against 79628ecc:

  • cargo fmt --all -- --check: PASS
  • cargo clippy --workspace --all-targets --all-features -- -D warnings: PASS
  • cargo test --workspace --all-targets: PASS
  • cargo check --workspace --all-targets --features "fastly cloudflare spin": PASS
  • cargo check -p edgezero-adapter-spin --target wasm32-wasip2 --features spin: PASS

All five gates green, including the boxes the PR checklist leaves unchecked (cargo test --workspace --all-targets and the feature check). examples/app-demo was not run.

Verification note

The behavioral claims above - terminal-OptionalField overriding optional, and the scalar-parent divergence between the CLI and runtime walkers - were confirmed with temporary probe tests against 79628ecc, not inferred from reading. Actual output is quoted in the inline comments; the probes were reverted.

Comment thread crates/edgezero-adapter-fastly/src/cli.rs
Comment thread crates/edgezero-adapter-fastly/src/cli.rs Outdated
Comment thread crates/edgezero-core/src/extractor.rs
Comment thread crates/edgezero-cli/src/config.rs
Comment thread crates/edgezero-core/src/extractor.rs
Comment thread crates/edgezero-adapter-fastly/src/cli.rs Outdated
Comment thread crates/edgezero-adapter-fastly/src/cli.rs Outdated
Comment thread crates/edgezero-adapter-fastly/src/cli.rs
Comment thread crates/edgezero-adapter-fastly/src/cli.rs
@aram356
aram356 force-pushed the feature/typed-static-secret-paths branch from 79628ec to 8773b05 Compare August 29, 2026 16:54

@prk-Jr prk-Jr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review

Summary

Two independent changes ride together here: an OptionalField segment for secret paths in core, and service-scoping of the Fastly edgezero_runtime_env Config Store (plus provision-time persistence of non-default store-name mappings). The Fastly half is the substantial one and is well-defended — fail-fast service-id resolution before any remote mutation, deletes confined to the current service namespace and to declared ids, and a fake-fastly oplog that asserts the exact keys and the cwd of every shell-out. All five CI gates pass locally.

Blocking on three correctness items and one question, all in the new code paths.

😃 Praise

  • The reconciliation is conservative in exactly the right places. runtime_store_name_reconciliation deletes only keys that are both in the current service namespace and in the declared set, and provision_reconciles_runtime_store_name_mappings asserts negatively that another service's mappings, the legacy unscoped mapping, and EDGEZERO__LOGGING__LEVEL all survive. That negative assertion is the one people skip, and it's the one that would catch a regression here.
  • Service-id resolution fails before any remote mutation. provision_runtime_env_service_id_for_stores runs at the top of provision, and provision_non_default_mapping_requires_service_id_before_fastly_mutation pins that ordering rather than just the error text. The manifest/FASTLY_SERVICE_ID mismatch check is a nice touch — refusing is better than picking a winner.
  • Splitting commit_entries_with_committer so each caller owns its recovery text. provision_mapping_failure_recommends_provision_recovery asserts the provision failure contains no config push advice and no chunk/root-pointer language. Factoring shared mechanics while keeping the operator-facing prose caller-specific is the right cut, and testing the absence of the wrong guidance is what makes it stick.
  • deploy_staged_ignores_ambient_store_name_overrides — proving staging mirrors persisted production mappings rather than ambient process env is a subtle property and easy to regress.

Findings

Blocking

  • 🔧 Legacy unscoped runtime-env entries are silently ignored on upgrade (crates/edgezero-adapter-fastly/src/lib.rs:224) — an already-deployed service carrying a hand-written unscoped __KEY/__NAME entry silently falls back to defaults on the next deploy and starts serving different config. Docs cover the manual migration; the runtime gives no signal. Suggested one-time log::warn! inline.
  • 🔧 OptionalField leaf arm skips a malformed parent instead of rejecting it (crates/edgezero-core/src/extractor.rs:1006) — Value::get returns None for any non-object receiver, so a scalar parent passes the walk silently. Asymmetric with the Field leaf arm, with the new OptionalField intermediate arm 23 lines below, and with collect_secret_leaf in the CLI — so config validate now rejects what the runtime accepts.
  • 🔧 Service-prefix match is not on a segment boundary (crates/edgezero-adapter-fastly/src/cli.rs:3595) — validate_service_id bans __ but still accepts a trailing _, so EDGEZERO__SERVICES__SVC1__ prefix-matches service SVC1_'s keys and pulls its overrides into SVC1's staging twin.
  • Viceroy / local dev: panic risk and an undocumented local service id. On wasm32-wasip1 — EdgeZero's Fastly target — fastly::compute_runtime::service_id() is std::env::var("FASTLY_SERVICE_ID").unwrap() (fastly 0.12.1, compute_runtime.rs:186). Locally the ConfigStore::try_open guard succeeds via [local_server.config_stores.edgezero_runtime_env], so service_id() is reached on every request. Two things I could not resolve from the diff:
    1. Does the guest panic under Viceroy if FASTLY_SERVICE_ID is unset? The new path has no guard, and the function's whole documented posture is fail-soft ("logs a warning and returns an empty EnvConfig").
    2. What service id should an operator write into local keys? docs/guide/blob-app-config-migration.md:265 still describes the local block with no mention that its keys now need the EDGEZERO__SERVICES__<id>__ prefix, and there's no documented way to learn Viceroy's id.

Non-blocking

  • 🤔 A failed delete gets none of the recovery guidance the upserts get (crates/edgezero-adapter-fastly/src/cli.rs:3843) — bare ? after the carefully-worded upsert failure path, leaving a stale mapping live with no "re-run provision" hint.
  • 🤔 OptionalField is unreachable from the derive. crates/edgezero-macros/src/app_config.rs:227-252 emits only Field and ArrayEach, and the variant's own doc says so. So "support optional typed secret paths" currently means "a hand-written AppConfigMeta can express one" — the derive-facing half of the feature isn't here. Is that a follow-up PR, or is hand-written meta the intended surface? Worth stating in the PR description either way.
  • 🤔 #[non_exhaustive] on SecretPathSegment (crates/edgezero-core/src/app_config.rs:35) is semver-breaking for downstream exhaustive matches and forces the dead _unsupported arm at crates/edgezero-cli/src/config.rs:1736.
  • ♻️ env_config_from_runtime_dictionary made pub (crates/edgezero-adapter-fastly/src/lib.rs:204) with no in-repo caller beyond run_app_with_request_extensions, documenting a custom-entry-point pattern nothing exercises.
  • ♻️ Stale unscoped-key docs outside this PR's file set. Two places still present the unscoped form as the Fastly override with no scoping note:
    • crates/edgezero-cli/src/templates/root/README.md.hbs:54-59 — this is generated into every new project, and it names Fastly's edgezero_runtime_env store directly while showing EDGEZERO__STORES__CONFIG__APP_CONFIG__KEY=<name>.
    • docs/guide/manifest-store-migration.md:89-91 — the canonical env-var reference table.
  • RUNTIME_ENV_PREFIX used for the strip, then re-hardcoded in the format! (crates/edgezero-adapter-fastly/src/lib.rs:70).
  • Dry-run emits a possible-removal line per declared store (crates/edgezero-adapter-fastly/src/cli.rs:3816) — 4 rows to 7 for a 3-store app, and the extra rows are hypotheticals.

📌 Out of Scope

  • A store that is dropped from the app's declared ids while holding a non-default mapping keeps its scoped entry forever — runtime_store_name_reconciliation only considers declared keys for deletion. Documented as intentional ("undeclared ids ... are preserved"), and the alternative risks deleting a namespace the app no longer knows about, so leaving it. Might deserve a provision --prune escape hatch, or a status line naming the orphans, tracked separately.
  • Provision writes __NAME mappings from the process env at provision time while the runtime reads them from the Config Store. Changing EDGEZERO__STORES__<KIND>__<ID>__NAME and running config push without re-running provision leaves push and the runtime pointed at different stores. Pre-existing, and this PR narrows the gap rather than widening it.

CI Status

Run locally against origin/main merge-base 10263543:

  • fmt: PASS
  • clippy (--workspace --all-targets --all-features -D warnings): PASS
  • tests (--workspace --all-targets): PASS — 0 failed
  • feature check (--features "fastly cloudflare spin"): PASS
  • spin wasm32-wasip2: PASS
  • additionally verified: -p edgezero-adapter-fastly --target wasm32-wasip1 --features fastly PASS, and --features cli without fastly PASS (the new #[cfg(any(feature = "cli", feature = "fastly", test))] gates hold in isolation).

Comment thread crates/edgezero-adapter-fastly/src/lib.rs
Comment thread crates/edgezero-core/src/extractor.rs
Comment thread crates/edgezero-adapter-fastly/src/cli.rs
Comment thread crates/edgezero-adapter-fastly/src/cli.rs
Comment thread crates/edgezero-core/src/app_config.rs
Comment thread crates/edgezero-adapter-fastly/src/lib.rs Outdated
Comment thread crates/edgezero-adapter-fastly/src/lib.rs
Comment thread crates/edgezero-adapter-fastly/src/cli.rs Outdated

@aram356 aram356 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Looks good

@ChristianPavilonis

Copy link
Copy Markdown
Contributor Author

Review-body-only follow-ups landed in c2bf557:

  • The Viceroy panic concern was checked with a Wasm probe against Viceroy 0.17.0. Viceroy supplies FASTLY_SERVICE_ID=0000000000000000000000; the Fastly docs now show that namespace for local edgezero_runtime_env entries.
  • The generated project README now shows Fastly's service-scoped stored key, and the generic manifest migration guide distinguishes canonical EdgeZero names from Fastly's stored representation.
  • OptionalField remains intentionally limited to hand-written AppConfigMeta; the existing #[app_config(nested)] limitations are derive-specific.

Validation completed:

  • cargo test --workspace --all-targets
  • cargo clippy --workspace --all-targets --all-features -- -D warnings
  • cargo check --workspace --all-targets --features "fastly cloudflare spin"
  • Fastly wasm32-wasip1 and Spin wasm32-wasip2 checks
  • app-demo workspace tests
  • docs lint, formatting, and build
  • deploy-action Bash contract suite: 244 passed, 0 failed

@ChristianPavilonis
ChristianPavilonis force-pushed the feature/typed-static-secret-paths branch from c2bf557 to 055f7e9 Compare September 1, 2026 17:16
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.

3 participants