Skip to content

Bind Fastly stores per deployment environment - #381

Merged
aram356 merged 72 commits into
mainfrom
fix/fastly-environment-store-selectors
Oct 8, 2026
Merged

aram356 merged 72 commits into
mainfrom
fix/fastly-environment-store-selectors

Conversation

@aram356

@aram356 aram356 commented Sep 16, 2026 •

Copy link
Copy Markdown
Contributor

Problem and resulting behavior

PR #344 made Fastly store selection depend on a shared edgezero_runtime_env Config Store and service/version-scoped descriptor keys. That added mutable shared state, put Fastly service IDs into configuration names, and made application runtime code participate in deployment-target selection.

This change removes that descriptor architecture. Applications keep stable logical store IDs in edgezero.toml. The selected deployment environment chooses physical Config, KV, and optional Secret Stores through canonical EDGEZERO__STORES__...__NAME variables. Before publication, managed Fastly deployment binds those physical resources to the logical aliases on the unpublished service version.

Production and staging can select the same or different physical stores while deploying the same immutable application release. A logical Config Store always uses its logical ID as the entry key. For example, production and staging can select config-prod and config-stage, while both versions expose the alias and key app_config.

Changes

  • Remove edgezero_runtime_env, runtime descriptors, service/version selector keys, staging selector stores, EDGEZERO_FASTLY_BUILD_SETTINGS, and all compatibility behavior for the Support optional typed secret paths and Fastly store mappings #344 contract.
  • Resolve physical stores from canonical __NAME variables with parent environment > manifest default > logical ID precedence for deploy, push, diff, GC, and provision. Normalize recognized selector casing before merging layers, reject noncanonical action inputs, and fail on present blank or invalid selectors before provider I/O. config gc --no-env still forces the logical fallback.
  • Reconcile declared Fastly resource links by kind and logical ID while preserving undeclared recognized Config, KV, and Secret Store links. Unknown provider resource types fail closed. Parse the live API types config, kv-store, and secret-store.
  • Keep first-service Fastly setup accurate: [setup] is written only when physical and logical names match. A different physical store requires a selected existing service; provision creates the physical resource and reports the logical resource-link operation without writing a misleading setup alias.
  • Invoke Fastly CLI through the supported service resource-link ... and service version ... hierarchy, retaining the reviewed Fastly CLI 15.1.0 pin.
  • Use authoritative Fastly environment records for staging state. A staging record's shadow service ID is accepted; its exact environment name, active version, and global uniqueness determine state.
  • Reject orphan editable drafts beside staged or retired sources because their ownership cannot be proven. Treat an empty scaffold service_id as unselected rather than as an invalid configured ID.
  • Replace the Fastly version self-diff dependency with normalized reads of versioned domains, backends, health checks, logging endpoints, and settings. Canonicalize JSON object-key order recursively before sorting collection snapshots. Logging snapshots use the live REST endpoint kinds, including pubsub, logentries, and s3.
  • Revalidate links, package identity, source state, draft state, and protected Compute configuration immediately before publication, with direct regression coverage for every failure barrier.
  • Keep the application CLI, complete edgezero.toml, package, and selected adapter manifest byte-identical across publishers and targets. A Fastly release includes its adapter manifest at the relative path declared in edgezero.toml.
  • Scope targeted config push/diff --adapter <name> validation to the selected adapter while retaining whole-project config validate.
  • Keep immutable-release packaging and verification in provider-neutral cores. Member digests are streamed through a bounded buffer; release.json is checked as a confined regular file before opening, and lifecycle actions copy the archive once before authenticating, inspecting, and extracting that same copy.
  • Move application CLI invocation into the existing provider-neutral deploy-core/scripts/run-app-cli.sh; thin Fastly wrappers own Fastly arguments, credentials, and output policy.
  • Keep store-free manifest-command compatibility, including its provider-owned --package argument. Managed immutable-release deployment rejects every --package / -p spelling because the release owns the package path, and adapters without managed-release support reject --application-release instead of ignoring it.
  • Rename the archive produced by build-app-cli to app-cli.tar while preserving the configurable artifact name, application binary/package names, and immutable member cli/app-cli.tar.
  • Retain uploaded application CLI and release artifacts for 14 days, document the recovery window, and exercise the public Fastly release-packaging action in the deployment smoke workflow.
  • Provide the provider-neutral setup-rust-build-cache action. Its required app-name scopes Cargo sources, sccache, and optional target artifacts; the target key is stable for an application, runner, architecture, and lockfile, and a two-job smoke proves a real restore.
  • Restore the executable ambiguous-publication recovery path: query the exact live version through the authenticated application release, then run the immutable-release rollback action to the captured previous version.
  • Serialize duplicate deploy-action workflow runs, scan all of .github with Zizmor, restore the required cargo test aggregate check, install every generated-project WASM target in CI, minimize workflow credentials, and keep action tooling free of Python and pip.
  • Limit file-based config-push cleanliness checks to the exact tracked config file, so an independently authenticated untracked release download does not block publication.
  • Document producer/consumer migration, all removed deploy inputs, custom Fastly entrypoint logging setup, edgezero_runtime_env cutover, staging config re-push under the fixed logical key, optional selector exports, cross-repository release retrieval, mutable-config recovery, and the distinction between a real application domain and its GitHub Environment name.
  • Build the application CLI for the runner's explicit native target and package that target's executable, so a restored Cargo cache cannot supply a stale CLI when the app configures build.target.
  • Read Axum dev-server secrets from EDGEZERO__SECRETS__<KEY>, with the key in ASCII uppercase, instead of from a variable named exactly like the key. config validate/push rejects keys outside ASCII letters, digits, and _, and distinct keys that collide once uppercased.
  • Restore the guide corrections from Align guide docs with the code on main #373 that the main merge dropped, and correct the Wrangler, Fastly, and Spin local secret commands.

Fastly does not expose a publication compare-and-swap token. Deployments and other version mutators for one service must share caller-side serialization. The source/clone and final snapshot checks reject observed interference; serialization closes the remaining provider time-of-check/time-of-use window.

Downstream consumers of the build action must consume app-cli.tar.

Local Axum runs must set secrets as EDGEZERO__SECRETS__<KEY>; a variable named exactly like the key is no longer read.

Closes #380

Validation

  • cargo test --workspace --all-targets
  • cargo clippy --workspace --all-targets --all-features -- -D warnings
  • cargo check --workspace --all-targets --features "fastly cloudflare spin"
  • cargo check -p edgezero-adapter-spin --target wasm32-wasip2 --features spin
  • cargo fmt --all -- --check and git diff --check
  • .github/actions/deploy-core/tests/run.sh — 639 passed, 0 failed, 5 expected platform skips
  • .github/actions/setup-rust-build-cache/tests/run.sh
  • .github/actions/package-application-release-fastly/tests/run.sh — exercised on Linux CI; non-Linux runners skip it
  • Actionlint, ShellCheck, and zizmor --offline .github
  • Documentation Prettier, ESLint, and VitePress build
  • rg 'edgezero-cli\\.tar' returns no product-contract references
  • Action and CI paths contain no Python setup, interpreter call, or pip invocation

@aram356 aram356 self-assigned this Sep 16, 2026
@aram356 aram356 changed the title fix(fastly): apply canonical store selectors at deploy feat(fastly): scope runtime configuration to service versions Sep 17, 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.

🔧 Request changes — two reproduced defects

The logical resource-link and immutable-release design has strong publication safeguards, but selector precedence and the documented file-based config-push workflow need correction. The selector issue and a nonblocking cleanup-test gap are commented inline.

🔧 P2 — Downloaded release blocks file-based config push
.github/actions/config-push-fastly/scripts/config-push.sh:123

The documented consumer downloads the release into $GITHUB_WORKSPACE/app-release/. In a publisher checkout where that directory is untracked, assert_committed_source rejects the entire checkout even when the selected config file is committed and unchanged. A nested working-directory: publisher-config does not avoid this because the check climbs to the Git root.

Reproduced with a temporary publisher repository containing committed runtime.toml and an untracked app-release/app-release.tar.gz: the real script exits before invoking the application CLI with committed source is required; the working tree for '.' is dirty.

Suggested fix: Validate that the resolved, confined config file is tracked and unchanged, rather than rejecting unrelated release-download artifacts. Alternatively, consistently download releases outside the publisher checkout and update the documented workflow. Add a regression covering release download followed by file-based config push.

😃 Strong publication and recovery safeguards

Managed deployment revalidates exact resource links, package identity, source/draft state, and protected provider configuration before publication. Publishing independently validated version and package-digest outputs before rejecting the other output contract also preserves useful recovery information.

📝 Validation

  • Rust formatting and all-features Clippy: passed.
  • Workspace tests: passed after rerunning outside the sandbox, which initially denied local HTTP-server port bindings.
  • Workspace feature check (fastly cloudflare spin) and Spin wasm32-wasip2 check: passed.
  • Deploy-core action suite: 623 passed, 0 failed, 5 platform skips.
  • Release-core, build-cache, and GitHub Environment tests: passed.
  • Linux-only package-release test skipped on this macOS runner.
  • Documentation formatting, lint, and build: passed.
  • GitHub PR checks: green at the reviewed head.

Comment thread crates/edgezero-cli/src/config.rs
Comment thread .github/actions/deploy-core/tests/run.sh

@dhruv8sh dhruv8sh left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

PR Review

Summary

The logical resource-link model and the immutable-release pipeline are well designed and defensively tested. I'm requesting changes for three verified regressions (a Fastly scaffold breakage and two CI gating problems) and for docs examples that fail or silently target the wrong store when copied as written. This review leaves out everything already raised in earlier rounds. Where it follows up on an existing thread, it replies there.

😃 Praise

  • prepare-release.sh checks the outer digest before any tar read, uses an exact member allowlist, rejects l/h entries, re-checks confinement after extraction, and only then does an atomic mv.
  • Link deletion keyed on (kind, alias) and re-resolved against the draft's own link IDs is correct whether or not Fastly keeps link IDs across clones.
  • verify_exact_members combines an exact set, a non-following walk, bounded hashing and adversarial tests.
  • Checked selector resolution is consistent across push, diff, gc, provision and deploy, and never echoes the selector value.

Findings

Blocking

  • 🔧 Empty service_id = "" rejected: breaks provision and deploy preflight for every scaffold and for app-demo. Reproduced locally (crates/edgezero-adapter-fastly/src/cli.rs:3195)
  • 🔧 The required check cargo test never reports: the PR is BLOCKED by the ruleset (.github/workflows/test.yml:18)
  • 🔧 Generated-project wasm checks are silently skipped because the rustup target add step was removed (.github/workflows/test.yml:95)
  • 🔧 The --application-release examples fail: the manifest must be inside the release root (docs/guide/cli-reference.md:174, docs/guide/adapters/fastly.md:178)
  • 🔧 "Optional" vars.* selectors expand to blank and are rejected (docs/guide/deploy-action-adoption.md:328)
  • 🔧 A __NAME selector without export silently targets the default store (docs/guide/blob-app-config-migration.md:247)
  • ❓ An orphan draft beside a retired version is adopted as InitialDraft (cli.rs:5884)
  • ❓ Deploy source selection doesn't enforce staging-record uniqueness, although the PR body and rollback both do (cli.rs:5869)
  • ❓ Non-managed adapters silently ignore --application-release (crates/edgezero-cli/src/adapter.rs:175)

Non-blocking

  • 🤔 Non-canonical EDGEZERO__STORES__* names are dropped silently by run-app-cli.sh:86
  • 🤔 The pre-publication barrier has no failing-case Rust tests (cli.rs:1514)
  • 🤔 The test PATH lock is separate from the manifest lock, so env mutation is still unsound (test_support.rs:115)
  • 🤔 release.json is opened before the file-type check, so a FIFO hangs the verifier (release.rs:108)
  • 🤔 The 14-day artifact retention limits rollback of an "immutable" release (deploy-action-adoption.md:15)
  • 🤔 Logging migration for custom entrypoints and the env-var table (adapters/fastly.md:64)
  • 🤔 The deprecated key input's guidance points to __KEY, which Fastly rejects (deploy-github-actions.md:270)
  • 🤔 package-application-release-fastly is never exercised end to end (deploy-action.yml:197)
  • ♻️ Redundant configuration snapshots (cli.rs:976, also 1021 and 1442). Each snapshot is 4 GETs plus 28 logging GETs, about 32 curl calls, and roughly 6 run per CloneActive deploy, so about 190 sequential calls. The snapshot at 976 is fully covered by the barrier, which compares a fresh read against the same plan.source_configuration. The re-reads of the locked active source can't change, because revalidate_active_source already rejects an unlocked source. Dropping both saves about 96 calls with no loss of safety. Also, InitialDraftSnapshot.configuration is always source_configuration.clone() (851), so that field and snapshot_initial_draft (1718) add nothing.
  • ♻️ Dead code left by the removal (cli.rs:5207, cli.rs:4954). The cwd: Option<&Path> parameter of classify_remote_config_store_with_cwd and create_config_store_entry_with_cwd is now always None. The doc at cli.rs:5198 still describes staged config isolation, which no longer exists. Collapse the wrappers and delete that sentence.
  • ⛏ Duplicate branch in select_version_source (cli.rs:5908). The if staging_versions.len() > 1 || drafts.len() > 1 { return Err(X) } is followed by an unconditional Err(X) with the same message. Delete the if.
  • ⛏ Two new provision tests depend on the ambient environment (cli.rs:9280, cli.rs:9319). Unlike the neighbouring tests, they don't clear FASTLY_SERVICE_ID with EnvOverride::remove, so they behave differently on a machine where it is exported.
  • ⛏ Stale comments. crates/edgezero-adapter-fastly/src/lib.rs:4 says chunked_config is only compiled for the CLI push/GC path, but the fastly runtime uses it too (config_store.rs:7). request.rs:328 says the bare-handle path "ignores those selectors", but no selectors are left.
  • ⛏ A contract test depends on locale (.github/actions/deploy-core/tests/run.sh:2008). parse_action_surface … | sort gives a different order under en_US.UTF-8 (retry-delay before retry), so "healthcheck-fastly public surface" fails locally and passes on CI only because runners use C.UTF-8. Use LC_ALL=C sort.
  • ⛏ Two release-core rejection tests pass on any failure (.github/actions/release-core/tests/run.sh:62-67). The wrong-adapter and wrong-protocol cases should capture stderr and assert the "unsupported format, lifecycle protocol, or adapter" reason.
  • ⛏ Broken anchor (docs/guide/deploy-action-adoption.md:134). #migrating-a-custom-entry-point should be #migrating-a-custom-entrypoint, matching the heading at adapters/fastly.md:64.
  • ⛏ The migration mapping only covers deploy-fastly (docs/guide/deploy-action-adoption.md:105). config-push-fastly also lost app-cli-artifact, app-cli-bin and manifest. healthcheck-fastly and rollback-fastly lost app-cli-artifact and app-cli-bin. All four now require app-release-archive, app-release-sha256 and expected-source-revision.
  • ⛏ The recovery helper runs a CLI it hasn't verified (.github/actions/deploy-fastly/tests/recovery-active-version.sh:4-5, 19-31). The header says "already authenticated immutable release", but it extracts and runs cli/app-cli.tar without checking the outer SHA-256. It's a test helper, but it models the operator runbook. Run prepare-release.sh with the expected digest first.
  • ⛏ Duplicate matrix work (.github/workflows/test.yml:61, 85). The workspace leg's cargo test --workspace --all-targets already runs the default-feature edgezero-cli and Axum tests that the cli and axum legs repeat.
  • 🌱 The paginated inventory has no page cap and no unit tests (cli.rs:581-613). collect_paginated_store_inventory only stops on a repeated cursor. A provider or proxy returning ever-new cursors loops forever and grows records without bound. Add a cap that fails closed ("completeness cannot be proven"), plus closure-driven tests: two pages, a repeated cursor, and a cursor with + or /.
  • 🌱 Harden the GitHub Environment lookup's curl (.github/actions/require-github-environment/scripts/require-environment.sh:33). Add -q (so a runner's ~/.curlrc can't enable tracing of the bearer header), --proto =https, --max-time 30 and --path-as-is, and reject ./.. segments in repository.
  • 📝 Public API. AdapterDeployContext and DeployStoreIds are new public structs with public fields and no #[non_exhaustive], so every field added later breaks downstream struct literals. Worth deciding while the version is 0.1.
  • 📝 Breaking change for release notes. Fastly deploys of apps that declare stores now always take the managed path and require --application-release, bypassing [adapters.fastly.commands].deploy. That breaks local edgezero deploy --adapter fastly.

Follow-ups on existing threads (posted as replies):

  • Target-cache fix incomplete. restore-keys can no longer match the new key shape (setup-rust-build-cache/action.yml:48).
  • The selector-case precedence bug has wider scope. It also affects config gc --yes, diff, validate, provision, every adapter, and __KEY (config.rs:1399).
  • Is [setup] still used under managed deploy? (cli.rs:2167)

📌 Out of Scope

  • actions-rust-lang/setup-rust-toolchain@v1 is still unpinned. This was already tracked in the first review round.

CI Status

Run locally on the pinned Rust 1.95.0 at 61e8444:

  • fmt: PASS
  • clippy (--all-features -D warnings): PASS. It fails on rustc 1.98 because of the new inline_trait_bounds lint, which is a toolchain issue, not a PR issue.
  • tests (--workspace --all-targets): PASS
  • feature check (fastly cloudflare spin): PASS
  • spin wasm32-wasip2 check: PASS
  • action suites: release-core, setup-rust-build-cache, require-github-environment and package-application-release-fastly PASS. deploy-core gives 640/640 under C.UTF-8 and 639/640 under en_US.UTF-8 (the locale nit above).
  • actionlint (-S warning), shellcheck and zizmor (repo config): clean
  • GitHub: BLOCKED, because the required cargo test context is missing.

Comment thread crates/edgezero-adapter-fastly/src/cli.rs Outdated
Comment thread .github/workflows/test.yml
Comment thread .github/workflows/test.yml
Comment thread docs/guide/cli-reference.md
Comment thread docs/guide/deploy-action-adoption.md Outdated
Comment thread docs/guide/deploy-github-actions.md
Comment thread .github/workflows/deploy-action.yml
Comment thread .github/actions/release-core/scripts/prepare-release.sh Outdated
Comment thread crates/edgezero-adapter-fastly/src/cli.rs
Comment thread crates/edgezero-adapter/src/release.rs
@aram356

aram356 commented Sep 25, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the remaining review findings in b4cf30c9 and added the exact mixed-case managed-deploy regression in 0a450fdb.

The non-inline config-push blocker is fixed as well: file-based push now validates and confines the selected config file, then requires that exact file to be tracked, committed, and unchanged. An unrelated untracked authenticated application-release download no longer makes the checkout fail the config-push preflight. The action suite includes that layout as an executable regression.

I also merged current main in 9da604f1, updated the PR and issue descriptions, replied to every open thread with its concrete fix, and resolved the addressed threads.

Local validation after the merge and fixes:

  • cargo test --workspace --all-targets
  • cargo clippy --workspace --all-targets --all-features -- -D warnings
  • cargo check --workspace --all-targets --features "fastly cloudflare spin"
  • cargo check -p edgezero-adapter-spin --target wasm32-wasip2 --features spin
  • .github/actions/deploy-core/tests/run.sh — 634 passed, 0 failed, 5 expected platform skips
  • Actionlint, ShellCheck, Zizmor, Rust formatting, documentation formatting/lint/build, and diff checks

The Linux-only release-packaging script test remains platform-gated on this macOS checkout and is running in CI through the public composite-action smoke jobs.

@aram356

aram356 commented Sep 25, 2026

Copy link
Copy Markdown
Contributor Author

Follow-up: the public-packager smoke handoff is fixed in 3c82f338. The fake Fastly provider now receives the packager's authenticated package digest directly rather than looking for the removed sidecar file, and all four consuming jobs have structural contract coverage.

The complete PR check rollup is now green, including production, staging, recovery, store-free deployment, config push, the required cargo test context, Fastly/Cloudflare/Spin WASM tests, static checks, and CodeQL. The local action suite is 638 passed, 0 failed, with 5 expected non-Linux skips.

@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.

🔧 Reviewed PR #381 at 3c82f33. Requesting changes for one reproduced artifact correctness issue: build-app-cli can publish an older cached executable instead of the executable Cargo just built.

😃 The immutable-release verifier has strong adversarial coverage for member digest mismatches, symlinks, extra members, and loaded-manifest identity. The final publication barriers also verify package and resource-link identity before publication.

📝 Verification: formatting, all-features Clippy, workspace tests (1,492 passed; 1 ignored), Fastly/Cloudflare/Spin feature compilation, and Spin wasm32-wasip2 compilation passed locally. The workspace tests required loopback networking permission. The action harness passed 638 tests with 5 platform skips; build-cache tests, Actionlint, ShellCheck, and documentation format/lint/build passed. All GitHub checks were successful on the reviewed head. The stale-artifact reproduction used real Cargo on macOS, with only Linux runner detection and toolchain installation shimmed.

Comment thread .github/actions/build-app-cli/scripts/build-app-cli.sh
A configured Cargo target (CARGO_BUILD_TARGET or build.target) moves the
fresh executable to <target-dir>/<triple>/release, but build-app-cli always
packaged <target-dir>/release. With a restored cache, an older root-level
executable still passed --help and was published as the current CLI.

Pass the toolchain's host triple as --target, which overrides any configured
target, and package the executable from that triple's directory.
@aram356
aram356 requested a review from prk-Jr October 2, 2026 02:06

@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.

📝 Reviewed PR #381 at c2e862f. Two introduced migration-runbook errors prevent the documented Cloudflare and Axum secret setup from working. No additional blocking runtime or action defects were confirmed.

😃 The deployment wrapper preserves independently valid recovery outputs when another output contract fails, and the tests cover provider exit-status preservation. This helps operators reconcile a deployment that may already have mutated provider state.

📝 Validation: all five required Rust gates passed (workspace tests: 1,492 passed, 1 ignored); action contract suite: 639 passed, 0 failed, 5 macOS platform skips; release verification, cache/environment tests, actionlint, ShellCheck, and docs format/lint/build passed. Linux-only packaging and live provider behavior were not exercised locally. Reported GitHub checks passed.

Comment thread docs/guide/blob-app-config-migration.md Outdated
Comment thread docs/guide/blob-app-config-migration.md Outdated
Merging main (9da604f) resolved five guide files to this branch's side
and dropped the corrections #373 made against the code. Restore each one
that still matches the code here: the Fastly manifest version, provision
behavior, build/serve commands, proxy import path, and streaming note;
Cloudflare local push by binding; the serve fallbacks, healthcheck
outputs, push outputs, and manifest/service-id rows in the CLI reference;
and the bare-handle constructors and `wrangler kv key put` spelling in
the store migration guide.

Also correct the local secret commands that never worked: `wrangler
secret put` takes no `--binding`, Fastly CLI 15.1.0 reads secret values
from `--stdin` or `--file`, and Spin reads `SPIN_VARIABLE_<NAME>` rather
than the bare variable name. Note the runner 2.327.1 requirement for the
Node 24 producer actions and drop stale overlay references from the
Fastly request rustdoc.
The example returned `text(...)`, which does not exist, so uncommenting it
in a generated app failed with E0425. Return `Text<&'static str>` like the
template's other handlers.
The Axum dev server looked a secret key up as an environment variable of
the same name, so `api_token = "demo_api_token"` read `demo_api_token`.
That breaks env-var conventions and lets any inherited variable whose
name matches a key answer for it silently.

Secret key `k` now resolves from `EDGEZERO__SECRETS__` plus `k` in ASCII
uppercase, e.g. `EDGEZERO__SECRETS__DEMO_API_TOKEN`. One rule in
`secret_env` serves both the runtime store and `config validate`/`push`:
a key must be ASCII letters, digits, or `_`, and two different keys that
differ only in case are rejected because they would share a variable.
The bare variable is no longer read.

Update the Axum guide, CLI reference, migration guide, the generated
README and app config template, the core rustdoc, and the smoke scripts.
@aram356
aram356 requested a review from prk-Jr October 3, 2026 20:06

@dhruv8sh dhruv8sh left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

PR Review

Summary

The descriptor removal and logical-alias binding are well executed. CI is green, and locally fmt, clippy, workspace tests, and both feature/wasm checks pass on the pinned 1.95.0 toolchain. This review covers blocking items only.

😃 Praise

  • The immutable-release verification in release.rs / prepare-release.sh is rigorous: it rejects path traversal and symlinks, denies unknown fields, verifies per-member digests on a private copy, and has adversarial tests.
  • Resource links are checked for exact map equality at the clone, reconciled-draft, and final stages. Every Fastly CLI call passes argv as an array, with no shell interpolation.

Findings

Blocking

  • 🔧 Broken anchor: the adoption guide links a heading slug that does not exist (docs/guide/deploy-action-adoption.md:139)
  • ❓ 14-day artifact retention vs. rollback window: the hard-coded retention can expire the archive that rollback needs (.github/actions/package-application-release-fastly/action.yml:115, .github/actions/build-app-cli/action.yml:282)
  • ❓ Cross-kind resource-link name collisions: reconciliation keys links by (kind, alias) (crates/edgezero-adapter-fastly/src/cli.rs:670)

CI Status

  • fmt: PASS
  • clippy: PASS
  • tests: PASS

Comment thread docs/guide/deploy-action-adoption.md Outdated
Comment thread .github/actions/package-application-release-fastly/action.yml
Comment thread crates/edgezero-adapter-fastly/src/cli.rs
@aram356
aram356 requested a review from dhruv8sh October 8, 2026 05:27
@aram356
aram356 merged commit f717674 into main Oct 8, 2026
29 checks passed
@aram356
aram356 deleted the fix/fastly-environment-store-selectors branch October 8, 2026 16:29
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.

Bind Fastly stores selected by each deployment environment

4 participants