Update for the middleware ABI minor 3 vhost_id / KV-scope change - #9
Merged
Merged
Conversation
ePHPm #448 takes the native-middleware ABI to minor 3 and redefines two surfaces these examples teach against. Nothing here failed to compile — none of the four modules called `vhost_id()` — but two of them were teaching the wrong thing once the host changed underneath them. `Request::vhost_id()` now returns `Option<&str>` carrying the router's canonical site key, NULL for a host that matched no vhost, where it used to return the raw `Host` header (ephpm#390). And the host table's `kv_*` callbacks now resolve the serving vhost's keyspace rather than the process-global store, with the global store reachable through the appended `kv_*_global` slots (ephpm#376). api-key: the KV credential lookup moves to `kv_get_global`. On a multi-tenant node the per-site store is writable by that site's own PHP, so a `key -> consumer-id` map living there would let any tenant mint itself a consumer identity. The global store is where operator-owned state belongs, and it is also the pre-minor-3 behaviour, so this is a no-op for existing deployments rather than a migration. api-key also gains an optional `<site>` placeholder in `kv_key_template` so a per-tenant key map is expressible. It resolves from `vhost_id()` and DENIES when there is no tenant — the fail-closed half of the pattern, as against the deliberate `UNMATCHED_VHOST` bucket a rate limiter wants. Three tests cover it: per-tenant isolation, the fail-closed branch, and a template without `<site>` staying node-wide. The api-key test helper seeds through `kv_set_global` and every test now uses key names no sibling touches: the store is process-wide, the tests run in parallel, and `Store::set_local` removes before it inserts, so two tests writing one key left a window where a third read saw a miss. redirect: documents why reading the `Host` header is correct *here* — a canonicalizing redirect exists to rewrite what the client asked for, which is exactly the thing `vhost_id()` is not. Also drops the stale claim that the ABI exposes no request scheme (minor 2 added it). README: a Tenancy section with the two patterns (fail closed vs bucket under `UNMATCHED_VHOST`) and the `kv_*` / `kv_*_global` split, plus the ABI minor history. Refs ephpm/ephpm#449
The previous rev `21a7c8a7` was ephpm#448's PR-branch head, which never landed on main: #448 was squash-merged as `691e6fef`, so the old pin is unreachable once the branch is deleted. Re-pinned to `c2774ab6` — the commit tagged v0.8.9, the first published ePHPm release carrying minor 3. Pinning the tag's commit rather than the tag name keeps the pin immutable; pinning the release rather than the raw merge commit means these examples build against an ABI that shipped in a host binary operators can actually run. No API drift between the two revs: the only changes to the consumed crates are documentation plus a `!Send` marker on the host-side `SiteKvScope`, which nothing here uses. Manifests are byte-identical, so the lockfile needed only the rev rewrite.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Downstream half of ephpm/ephpm#449.
Unblocked and re-pinned. ephpm/ephpm#448 merged to main as
691e6fefandshipped in v0.8.9. The
revhere now points atc2774ab6— the committagged v0.8.9 — not at #448's PR-branch head
21a7c8a7, which never landed onmain (the PR was squash-merged) and would have become unfetchable once the
branch was deleted. Pinning the released tag's commit keeps the pin immutable
while guaranteeing these examples build against an ABI that actually shipped in
a host binary operators can run. No API drift between the two revs: the only
changes to the consumed crates are docs plus a
!Sendmarker on the host-sideSiteKvScope, which nothing here uses.What changed upstream
ePHPm #448 takes the native-middleware ABI to minor 3 and redefines two
surfaces rather than only appending to them:
Request::vhost_id()returnsOption<&str>carrying the router'scanonical site key,
Nonefor a host that matched no vhost — where itused to return the raw
Hostheader, un-normalized and never absent(ephpm#390).
kv_*callbacks resolve the serving vhost's keyspaceinstead of the process-global store, with the global store reachable through
the appended
kv_*_globalslots (ephpm#376).What that meant here
Nothing failed to compile — none of the four examples called
vhost_id(),and I verified that at the new pin before changing anything. But two of them
were teaching the wrong thing once the host changed underneath them.
api-key— the credential map moves to the global storekv_key_templatemapsapikey:<secret>→ consumer id. Under minor 3 a plainkv_getreads the serving tenant's own store — the one that tenant's PHPwrites through
ephpm_kv_set(). On a multi-tenant node any tenant could thenwrite
apikey:<anything>and mint itself a consumer identity this gate wouldhonour. So the lookup moves to
kv_get_global.Worth being clear that this is not a behaviour change: the global store is
exactly where
kv_getwent before minor 3. It is the change that keeps theexample correct, not one that alters it.
api-key— an optional<site>, and the fail-closed patternSince one mount serves every vhost, a multi-tenant deployment usually wants one
key map per tenant.
kv_key_templategains an optional<site>placeholder(
apikey:<site>:<key>) resolved fromvhost_id(), and it denies when thereis no tenant rather than substituting anything — the fail-closed half of the
pattern, as against the deliberate
UNMATCHED_VHOSTbucket thatratelimitandmaintenance-modeuse in-tree. Three tests: per-tenant isolation, thefail-closed branch, and a template without
<site>staying node-wide.Site substitution happens before key substitution, deliberately: the presented
key is client input, so doing it the other way round would let a caller inject a
literal
<site>for the nextreplaceto expand.redirect— the one place readingHostis rightDocumented why: a canonicalizing redirect exists to rewrite what the client
asked for, which is exactly the thing
vhost_id()is not. Also dropped a staleclaim that the ABI exposes no request scheme — minor 2 added
req.scheme()/req.is_secure(), and the example keeps the header-derived form on purpose (italso has to preserve the request's port, which
http_host()strips).README
A
Tenancysection with both patterns and thekv_*/kv_*_globalsplit,plus the ABI minor history.
A test-isolation bug found on the way
Adding
setup_kv_withcallers made three api-key tests fail ~100% of the timein parallel and pass serially. Cause: the store is process-wide, and
Store::set_localremoves the old entry before inserting the new one, sotwo tests seeding the same key name left a window where a third read saw a miss.
Fixed by isolating the key names — the interference is the bug, not the timing.
Verification
Re-verified against the v0.8.9 pin before merge:
abi.rsat v0.8.9 statesboth halves independently: the
kv_*slots resolve the serving vhost'skeyspace and "shares a keyspace with the tenant's
ephpm_kv_*", and theglobal slots are where "operator-owned state lives, out of reach of any
tenant's PHP".
kv_*_globalwrappers the README names really exist(
kv_get_global,kv_set_global,kv_set_nx_global,kv_incr_global,kv_incr_ttl_global) — no phantom API.request_hostis documented port-stripped, soredirect's stated reason forkeeping the raw header holds.
CI green on the re-pinned commit
79ad4fc: fmt, clippy, test, release build.