feat(0124): expose the OpenAPI spec through API Gateway - #169
Conversation
`/api-docs-json` was defined in the axum router but never mapped by API Gateway, so in production the spec was reachable only by running `extract_openapi` locally. Map it as a keyless Lambda proxy onto the existing api-handler, cached 3600s at the stage cache to match the handler's Cache-Control — the document is byte-identical for the life of a deployment. Anonymous by design: an API description is public documentation, and gating it behind a key the reader does not have yet is a self-service dead end. `/health` already set the precedent; the in-app gate already exempted the path. `servers` is now stamped in production. `apiBaseUrl` is new config (validated at synth for the stage-prefix trap) passed to the handler as `API_BASE_URL`, rather than derived from `api.url` — ComputeStack is a dependency of ApiGatewayStack, so reading the gateway's URL there would close a cycle. Linting the document surfaced two real gaps, both fixed: `extract_openapi` emitted a `servers`-less variant of the spec (now shares `stamp_servers` with the served one), and seven key-gated operations documented no 401/403 at all (now documented, with ErrorEnvelope as a published schema). The `x-api-key` scheme is declared and required document-wide, with /health and /api-docs-json opting out explicitly. `npm run openapi:lint` extracts the served document and runs Redocly's recommended ruleset over it; wired into the rust CI job.
Review of the previous commit found two guards that looked like they worked and did not. `redocly lint` exits 0 on warnings, and under `recommended` most checks — including operation-4xx-response, the rule that found seven key-gated operations documenting no 401/403 — are warnings. The CI step would have accepted the exact regression it was added to catch. Extend recommended-strict instead, and pin that rule to error so a switch back to recommended cannot silently demote it. "Route coverage, both directions" was enforced against a hand-written list mirroring the CDK source, so it could not catch a route added to axum with a plain .route() call and never mapped — which is precisely how /api-docs-json went unroutable. Add verify-openapi-routes.mjs, deriving both sides from the synthesized template and the extracted document, and run it in CI after synth. Same reasoning as lambda-assets.sh (task 0077). Both fixes verified by confirming they fail on a seeded regression.
`npm ci` failed on CI with "Missing: yaml@2.9.0 from lock file". Adding @redocly/cli was done with npm 11.17.0 on Node 26, which pruned `node_modules/vitest/node_modules/yaml` — an optional peer entry that the Node 22.22.0 / npm 10 toolchain in .nvmrc still resolves and requires. The lock was regenerated under 22.22.0, so the diff against develop is now purely the @redocly/cli addition. Verified by running `npm ci` on a clean tree under 22.22.0: the previous lock reproduces the CI failure, this one exits 0.
The task asked to note the ordering for the custom-domain change. 0126 already said to update `servers` alongside the docs; now that 0124 has landed it can name the single config value that does it, and the two gotchas around it.
A second validator, IBM's `ibm-openapi-validator`, reported 13 errors on the document Redocly passes cleanly. Seven were real and are fixed here; all seven are cases where the code already had a bound the document did not state. - The five ledger-sequence fields are `u64` in Rust because ClickHouse returns `UInt64`, but a Stellar ledger sequence is `uint32` in the protocol's `LedgerHeader`. The document promised a range four billion times wider than reality; `maximum: 4294967295` is a domain fact, not a limit we impose. - `limit` has been rejecting `0` and anything over 200 with a 400 since it was written, and said so nowhere a client could read. Now declared. - utoipa published the whole `/health` rustdoc as `summary` — 223 characters of maintainer-facing prose where a label belongs. Split into `summary` plus `description`. The remaining six are deliberate: four are OpenAPI 3.1 constructs the validator judges against 3.0's rules, one is a maximum that does not truthfully exist (`Candle.trade_count`), and one is the `/api-docs-json` path, kept rather than renamed. The task file records the full accounting and the reasoning for each.
PR #169 review. Three of the four gates this task shipped had holes, all of the same shape as the two already in Issues Encountered — guards that looked like they worked. `openapi:verify-routes` did not chain `openapi:extract` the way `openapi:lint` does, so it compared the template against whatever target/openapi.json happened to hold. A stale file reads as a pass, or as drift nobody can reproduce. The rust paths filter omitted package.json and package-lock.json while @redocly/cli is a devDependency only this job runs. A PR dropping or bumping it merged green and broke the next author to touch packages/**. HTTP_METHODS included options and head, so 0126's addCorsPreflight would have failed the gate with the remedy "add a #[utoipa::path] for each" for methods OpenAPI does not conventionally describe. Both are now excluded from both sides — excluding one side only manufactures drift. ANY is rejected loudly instead: it can never match an operation key, and skipping it would hide a mapped route from the check. The fourth was not a gate. Seven key-gated operations documented 401/403 but not 429 or 500, the two statuses a partner actually meets — the usage plan throttles, and all seven reach errors::db_error. A generated client fell into its unexpected-response branch for both. The 403 description named the usage plan, which is what returns 429; 403 is the key being missing or unauthorized. Verified by injection, not by reading: an OPTIONS method added to the synthesized template is ignored, an ANY method exits 1, and deleting /v1/prices/batch from the document still reports it undocumented. cargo test --workspace 223 passed; redocly lint 0 errors 0 warnings; verify-routes agrees on all 9 routes.
The ID was claimed twice. `0145_BUG_synth-not-run-on-infra-only-prs` landed on develop with PR #165 (the 0110 won't-do closure), while the unmerged PR #168 branch had already claimed 0145 for the pre-roll `argMax(close_usd, ...)` guard. The pre-roll task keeps the number: it is referenced nine times across four files, gates both the 0088 pass-2 pre-roll and 0136's 07-21 gap pre-roll, and develop's own generated index already resolves 0145 to it. This task is referenced three times and moves. 0152 is deliberately left free for the OpenAPI license task on PR #169, which collides with 0144 the same way and is not ours to renumber. Nothing about the synth work itself changed. lore/README.md is not regenerated here — it already points 0145 at the pre-roll task, and regenerating on this branch would conflict with PR #168's index.
Reverses a decision from this task's own validator accounting, where `Candle.trade_count` was left unbounded as "no truthful maximum exists". The premises were right — a trade count has no protocol bound, `u64::MAX` overflows JSON's safe-integer range, and a domain figure would be invented — but the conclusion did not follow. The ceiling is the safe-integer range itself. `2^53 - 1` is the largest integer an IEEE 754 double represents exactly, and JSON has no integer type, so above it a client's parser silently rounds. Publishing it states a fact about the wire format rather than a limit we impose: values above it cannot be delivered correctly whatever ClickHouse holds. Same kind of claim as the ledger-sequence bound, taken one layer down — that one is a protocol fact, this one is transport. Real Stellar volumes sit ~10 orders of magnitude below it, so it never binds and cannot make a future response contradict the document, which was the actual worry behind leaving it out. It remains a published ceiling rather than a runtime clamp, the same caveat the review raised against the ledger fields. ibm-openapi-validator --errors-only: 6 -> 5, the remainder being the four 3.1-vs-3.0 entries and the deliberate path-casing one. Redocly still 0 errors 0 warnings; cargo test --workspace 223 passed; verify-routes agrees on all 9 routes.
The accounting said what the six remaining errors were but not what to do about them, and named only the utoipa downgrade as the route to zero. Two things learned since are worth not re-deriving. Zero is reachable without touching utoipa: the validator takes a Spectral ruleset, and switching off the three offending rules produces "passed the validator". Measured, not assumed. Declined anyway — the document is already correct 3.1, so the choice is between disabling rules globally (broader than the two path entries in .redocly.lint-ignore.yaml) and down-converting to 3.0 before linting, which breaks decision #9 by making the linted document stop being the served one. More important, errors are not where the ruleset stops. At warning level it demands ErrorEnvelope carry `trace` and an `errors` array — IBM's error-container shape. No toggle removes that honestly, so adopting the tool means redesigning the error body on every endpoint and breaking every client. That is an API redesign, not a lint cleanup, and it is now attached to the open question for Oskar so the cost is visible when the question gets asked.
Why the IBM validator still reports 5 errors — and why that's the intended end state
What remains
All five are the tool applying OpenAPI 3.0 rules to a 3.1 document. utoipa 5 has no 3.0 emit mode ( The distinction being drawn
The remaining five are not gaps. Making them disappear from the document would mean dropping Zero is reachable, and was measured
The part that matters mostErrors are not where IBM's ruleset stops. At warning level it also reports So "adopt IBM's validator" is not a lint cleanup. It is a utoipa downgrade plus an API redesign. Acceptance criteriaThe AC on this task is "valid OpenAPI, passes a linter cleanly" — Redocly One open question for @okarcz: did Full accounting, including every fixed and every left error with its reason, is in |
ReviewMapping The "Issues Encountered" section catching the lint gate for exiting 0 on warnings is the right instinct, and deriving both sides of the route check from artifacts rather than a hand-maintained mirror is the correct lesson from 0077. The vacuous-pass guard and the loud failure on 🔴 Blocking — task ID
|
PR #169 review (okarcz). The task spawned from 0124's future work claimed an ID that PR #168 had already taken for the BE-0199 USD read-surface defects. That branch was unmerged when this one was cut, so the collision was invisible in the tree. The BUG side keeps 0144: it is cited by 0145-0151 and 0154, the BE-facing reply and the phase plan. 0152 (#172), 0153 and 0154 are all claimed, so this moves to the next free ID. 0153's note reserving 0152 for this task has been overtaken by #172. References updated here: redocly.yaml's info-license-strict comment (a fifth site, not in the review's list of four) and the api-endpoints doc. The 0124 task file's two links follow in the review-record commit.
Four of the review's points are the same shape as the bug this task exists to fix: something that looks like a check but is not one. 1. openapi:verify-routes never ran on the PRs most likely to trip it. It lives in the rust job, whose paths filter had no infra/** entry, so an infra-only PR adding a gateway route skipped the only check that sees the gateway->spec direction. Adding a gateway route is a pure-infra edit while adding an axum route touches packages/**, so the uncovered direction was the more likely one. Listing the single stack file rather than infra/** keeps unrelated CDK edits off the ARM Rust build. 2. LEDGER_SEQ_MAX asserted a tautology. The const restated u32::MAX as u64 == 4_294_967_295 and tied the five schema(maximum) literals to nothing: retyping one to 4_294_967_296 left the build green and published a wrong bound. Replaced with a test that reads the bounds back out of the served document, with the field set derived from the document so a later ledger field that forgets the attribute fails as a missing maximum. Mutation-checked both ways. 3. fullPath() truncated silently, despite its comment promising to fail loudly. A partial path still looks like a route, so a broken template surfaced as drift on a path that was almost right rather than as the parse failure it was. Both exits now throw. The root-method check moved above the ANY check so the ANY message always has a resolved path to name. 4. The two route guards compared different method sets: the Rust test matched head/options, the mjs drops both from both sides so 0126's addCorsPreflight does not read as drift. Aligned. Also switches extract-openapi.sh off require()-ing the env JSON as a module. The stated hazard does not reproduce (node -p is still CommonJS under "type": "module", and the root package.json has no type), but the old form depended on both of those staying true and reading bytes depends on neither.
The keyless posture cites /health as precedent, and for the posture it is one. For cost it is not: /health is a MockIntegration and can never invoke anything, while /api-docs-json is proxy([]), so a cache miss reaches the Lambda and the route sits outside the usage plan with only the stage-wide throttle it shares with paying traffic. The residual stays small for reasons already in the stack -- a 3600s TTL with no cache-key parameters, so every caller collapses onto one entry, and API Gateway's default requireAuthorizationForCacheControl blocking anonymous cache-busting -- but none of that was written down, so "matches the /health precedent" read as "same cost profile". States it, and names the lever for a harder bound: a method-level throttle, not a key requirement. Also records the full #169 review response in the task file and completes the 0144 -> 0155 renumber's remaining two links.
All four are cases where a check reads as covering something it does not.
Each is verified by mutating the artifact it reads and confirming it now
fails; three of the four passed that same mutation before.
1. Nothing checked that the deployed handler is configured with the URL
the document advertises. extract-openapi.sh stamps `servers` from
infra/envs/production.json and exports API_BASE_URL itself, so it
never observes ComputeStack putting that variable on the Lambda.
Rename it to API_BASE_URI in an unrelated refactor and synth, lint and
the route gate all pass while production serves a document with no
`servers` block at all. New openapi:verify-servers compares the
synthesized Compute template against the extracted document, and
re-asserts the stage-prefix invariant against the stage the template
actually deploys so deleting the types.ts validation cannot silently
remove it. The handler is identified by carrying API_BASE_URL, not by
name, so a rename fails as "no function declares it". compute-stack.ts
and types.ts join the rust paths filter, which is what makes the new
check run on the PRs that would break it.
2. Dropping head/options from both route guards fixed the disagreement
between them by removing the coverage. A documented HEAD was then
checked by neither guard in either direction — the same unroutable
documented route 0124 exists to close, reopened for two verbs. HEAD is
compared normally now; OPTIONS stays skipped on the gateway side only
(0126's addCorsPreflight), and a documented OPTIONS is refused outright
by both guards instead of ignored.
3. fullPath() still truncated silently. The rewrite threw at two exits,
but the truncation happened earlier: any ParentId that was not `{Ref}`
became null, and null is the walk's "reached the root" signal. An
imported RestApi or a cross-stack split (0126) emits Fn::ImportValue
and would have produced `/status` for `/v1/backfill/status` — drift
reported on a path that is almost right, or worse, genuine drift
passing if the truncation collides with a documented path. ParentId is
now classified into ref/root/unresolved, and PathPart and HttpMethod
are rejected unless they are literal strings.
4. The ledger-ceiling test matched a `_ledger` name suffix, so its own
promise — "a ledger field added later without the attribute fails" —
held only for that name shape, and the count assertion could not see a
field the filter never matched. Replaced with two rules: by type over
the schemas reachable from the /v1/backfill/status response $ref (every
integer there is a ledger sequence), and by name over the whole
document using `contains`, not a suffix.
The lint exception for /health and /api-docs-json claimed they "genuinely
have no 4xx to document". The same branch's own stack comment says
otherwise: neither route is in the usage plan, but both sit under the
stage-wide `/*` `*` throttle, so API Gateway can 429 either one.
/api-docs-json can also 5xx, because unlike /health it is a Lambda proxy
and a cache miss reaches the handler.
Left as it was, a partner generating a client from this document gets no
error branch for either route: a 429 arrives as `{"message": …}` and the
client tries to deserialize it as the OpenAPI document. That is the exact
failure a0b9b29 fixed for the seven key-gated operations; these two were
excepted rather than fixed.
Both responses are documented without a body, because API Gateway
produces them and its shape is not ErrorEnvelope — same asymmetry already
recorded for 403/429 on the data routes.
.redocly.lint-ignore.yaml is now empty and stays in the tree carrying the
reason, so the exceptions cannot quietly come back as the fix.
Both caches on /api-docs-json were 3600s, justified by "the document is
byte-identical for the life of a deployment". True, and beside the point:
the caches outlive the deployment that filled them, and nothing dropped
either one when a build shipped. A partner who fetched the document
minutes before a release kept generating clients from the old one for the
rest of the hour, with no staleness signal — at exactly the moment
integrators go look at it.
Split by who controls the cache:
- Gateway stays 3600s and is now FLUSHED on deploy. `make -C infra
deploy-production` and `deploy-production-compute` both run
flush-production-cache, which reads the REST API id from the SSM
parameter the stack already publishes. API Gateway has no per-route
flush, so it drops the whole stage cache — harmless, every other TTL
there is 10-60s on self-correcting data.
- The handler's Cache-Control drops to 300s, because a partner's HTTP
cache is the one we cannot flush. Revalidating every 5 minutes costs
nothing: those requests land on the gateway cache, not the Lambda.
This is the one place the cache_control tiers and the stage TTLs
deliberately disagree, so both sides say why.
Also replaces the `{}` fallback in lib.rs. A serialization failure served
a syntactically valid EMPTY document as 200 OK — no log, no metric — then
cached it. Every generator run in that window produced a client with zero
endpoints and nothing reported a fault. Failing to start is louder and
shorter, and matches extract_openapi, which already .expects the same
call.
README said openapi:lint runs Redocly's `recommended`; it runs
`recommended-strict`, and the distinction is the whole point of the gate
(plain `redocly lint` exits 0 on warnings). Someone trimming config back
to "the documented ruleset" would have disarmed it.
Every other Lambda-backed route is apiKeyRequired, so it carries two limits from the usage plan: the per-key rate and the daily quota. /api-docs-json is anonymous by design and therefore has neither — its only limiter was the stage-wide bucket it SHARES with paying partners. Throttling is evaluated before the cache, so an anonymous loop on the documentation route draws that bucket down and a partner inside their contracted 100 req/s starts seeing 429s from a route they never called. 10 req/s aggregate, burst 20: far above any legitimate use of a static ~40 KB document cached for an hour at the edge, and 5% of the stage ceiling. A local constant rather than a config key, because it follows from the route's shape (anonymous, cached, static) rather than from an environment's capacity. The methodSettings block moves out of the `if (cacheEnabled)` branch. It was skipped wholesale when the stage cache is off — which is the configuration where an unbounded keyless route costs the most, since every request is then a billed Lambda invocation. Cache TTLs stay conditional; throttles no longer are. The route gets ONE entry carrying both, since method settings are keyed by resourcePath+httpMethod and two entries would collide. Kept as its own commit deliberately. The #169 review looked at this route's posture and accepted it ("the residual is small"), asking only for the cost profile to be written down. This goes further than that, so `git revert` this one commit if you would rather it did not.
Review follow-up, round 2 — plus five findings from a self-review@okarcz — everything from your review landed in One of those last three needs your call, and I have kept it separately revertable. It is at the bottom. The three fixes that were themselves incomplete (
|
| Mutation | Before | After |
|---|---|---|
API_BASE_URL → API_BASE_URI in the template |
not checked | exit 1, points at compute-stack.ts |
servers drifts from the handler's config |
not checked | exit 1, prints both |
documented head /v1/assets/{id} |
green | exit 1, unroutable |
documented options |
green | exit 1, own message |
ParentId: {Fn::ImportValue} |
/assets, phantom drift |
exit 1, names the resource |
gateway-side OPTIONS method |
passes | passes (0126 unblocked) |
tip_seq: u64 with no maximum |
green, count still 5 | fails, names the field |
one ledger literal → 4_294_967_296 |
green (old assert was a tautology) | fails, names the field |
Two candidates were refuted during verification and are not changes: the duplicated resourcePath literal next to the TTL, and the API_KEY_HEADER const.
Still outstanding and unchanged: the two ACs that need a deploy (the live fetch and confirming the advertised servers serves), and the open question above about whether Tranche 3 AC 2's openapi-validator means IBM's package specifically.
…alidated The task file described the branch as it stood three commits ago, and several of its claims were made false by this session's own changes — which is the same failure mode the branch keeps finding in its guards, so it does not get to stay in the file that documents them. Corrected: - AC said the document passes Redocly's `recommended` ruleset. It passes `recommended-strict`, and now with 0 ignored rather than 2 — the two `operation-4xx-response` exceptions were deleted, not preserved. - AC and Implementation Notes said the cache was "3600 s, gateway + handler agreeing". It is 3600 s at the gateway (flushed on deploy) and 300 s at the client, deliberately disagreeing. - Design Decision #3 asserted that agreement as a decision. Struck through rather than rewritten: it was made, shipped, and then found wrong, and that sequence is the part worth keeping. - Verification carried stale counts (223 workspace / 8 openapi, "2 ignored") and said cdk synth had not been re-run. It has: 225 / 9 / 0 ignored, and synth needed a workaround now recorded, since a plain dev checkout has no Lambda bootstrap assets and fails with CannotFindAsset before rendering anything. Added: the self-review round, its mutation-check table, and the finding that matters more than any individual item — three of the four fixes made for the #169 review were incomplete in the same way the originals were. The review caught a class of defect and the fixes reproduced it one layer down; that is what a later reader needs, not the four bugs. Also records the one item still open: the /api-docs-json throttle goes beyond what #169 accepted, so it is isolated in 479548c pending okarcz's call.
The file is ~680 lines against the ~150-line threshold in lore/1-tasks/CLAUDE.md, and larger than any existing task README here. Recording the decision so a later session reads it as deliberate rather than as an oversight to re-litigate: the archive move is a `git mv` anyway, so converting then is one operation instead of two, and doing it now would hand the PR reviewer a large rename on the file he is reading. Carries the proposed split, the target README size, and the two things that break if it is done without care — the current-task symlink and the inbound links from 0128 and 0155.
Both open questions answeredReviewed the self-review round against the branch. The three "my own fixes were incomplete" findings are all genuine, and on the second one you were right and I was wrong — see below. Answers to the two things you left for me: 1. The anonymous-route throttle (
|
PR #169 merged to develop (squash dabdd15) after okarcz's approval, three CI jobs green. Shipped: GET /api-docs-json as a keyless cached proxy route, `servers` stamped from apiBaseUrl, ErrorEnvelope published, a Redocly recommended-strict gate, and three artifact-derived guards. Converted to a directory as the task's own Future Work planned — deferred to archive time so the reviewer never saw a large rename mid-review. The three heavy sections moved to notes/S-*, leaving a ~420-line README. Two ACs stay deployment-verified rather than verified: the live /api-docs-json fetch and the advertised `servers` URL serving a route both need `make -C infra deploy-production`. docs/scf/api-endpoints.md carries the curl. Also quoted the history dates, which failed lore frontmatter validation as bare YAML date scalars.
2 participants
Summary
GET /api-docs-jsonthrough API Gateway as a keyless Lambda proxy onto the existing api-handler. The route was defined in the axum router but never mapped, so in production the spec was reachable only by runningextract_openapilocally. Cached 3600s at the stage cache, matching the handler'sCache-Control— the document is byte-identical for the life of a deployment./healthalready set the precedent, and the in-app gate already exempted the path.serversin production via a newapiBaseUrlconfig →API_BASE_URLon the handler, validated at synth for the stage-prefix trap. It is configured rather than derived fromapi.urlbecause ComputeStack is a dependency of ApiGatewayStack — reading the gateway's URL there closes a cycle.x-api-keysecurity scheme document-wide with explicit opt-outs on the two anonymous routes, and document the 401/403 that seven key-gated operations previously omitted (ErrorEnvelopeis now a published schema).npm run openapi:lint(Redoclyrecommended-strictover the extracted document) andnpm run openapi:verify-routes, which compares the synthesized CloudFormation template against the extracted spec so neither side can drift. Both confirmed to fail on a seeded regression.Notes
Two acceptance criteria are about the deployed API — the live
GET …/production/api-docs-jsonfetch and confirming the advertisedserversURL serves a route. Both need a deploy, so task 0124 staysactiverather than moving toarchive.docs/scf/api-endpoints.mdcarries the verification curl.info.licenseis deliberately left empty and its lint rule turned off: the repo has no LICENSE file or Cargolicensefield, and declaring one in a public API document is a business decision. Spawned as task 0155 (renumbered from 0144, which PR #168 had already claimed).The document is OpenAPI 3.1.0, not 3.0 as the task's AC wording says — utoipa 5 has no 3.0 emit mode, and reaching it would mean downgrading the crate.