feat(codegen/hetzner): vault-wireable token auth and paginated list method #12
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "worktree-471"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Adds vault-wireable auth and discovery to the Hetzner Cloud codegen.
What changed
tokenglobal arg — optional, sensitive (z.meta({ sensitive: true })), takes precedence overHETZNER_API_TOKEN, wireable with${{ vault.get(...) }}. Threaded through the_libauth path (each distinct token validated once via aSetcache); never written into a request body.listmethod — generated for every collection-GET resource. Discovers by an optional Hetzner label selector, followsmeta.pagination.next_pageto exhaustion, and writes onestateresource per item (factory), returning a count. swamp does not enforce required global args at instantiation, solistworks on the existing models with no schema relaxation — CRUD validation is untouched.codegen/hetzner+codegen/shared;model/hetzner-cloudregenerated (manifest + 11 models →2026.05.28.1, upgrade entryAdded: token). Fixed a latent upgrade-diff bug: the pipeline now mirrors the generator'stokeninjection intonewFieldNames, preventing a spuriousRemoved: tokenentry on the next regeneration.Verification
deno check/lint/fmt --check/install --frozenclean acrosscodegen/andmodel/hetzner-cloud/.Note:
z.meta({ sensitive: true })is honored by swamp-core in run logs, reports, and data storage (notswamp model getoutput, but a vault-sourced arg is stored there as thevault.get(...)expression, so no secret leaks on the recommended path).Closes swamp-club #471. Sibling follow-ups for env-var-only auth: #473 (DigitalOcean), #474 (AWS), #475 (GCP), #476 (Cloudflare).
🤖 Generated with Claude Code
Add an optional, sensitive `token` global argument to every Hetzner model. It takes precedence over HETZNER_API_TOKEN so the token can be wired with a ${{ vault.get(...) }} expression instead of a shell env var. The token is threaded through the shared _lib auth path (getToken validates each distinct token once via a Set cache) and is never written into a request body. Add a paginated `list` method for every resource with a collection GET. It discovers resources by an optional Hetzner label selector, follows meta.pagination.next_page to exhaustion, and writes one `state` resource per item (factory), returning a count. swamp does not enforce required global args at instantiation, so list works on the existing models with no schema change. All changes are in codegen/hetzner + codegen/shared; model/hetzner-cloud is regenerated (manifest + 11 models -> 2026.05.28.1, upgrade entry "Added: token"). Closes swamp-club #471. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>Adversarial Review
Medium
Incomplete token collision guard —
extensionModelGenerator.ts:81-83+:171The collision guard correctly skips injecting the auth-specific
tokenfield intoGlobalArgsSchemawhen a resource already has a property named"token":But the generated method bodies unconditionally reference
g.tokenas the auth argument regardless ofhasTokenProp:If a future Hetzner resource had a property named
token(e.g., a webhook token string), the guard would suppress the auth-specific injection, sog.tokenwould resolve to the resource property value. That value would be:body.token = g.token), andtokenauth argument tocreate(), attempting API authentication with the resource's property value.Breaking example: A hypothetical
webhooksresource withcreateProperties: { token: { type: "string" } }would use the webhook's token string for Bearer auth, resulting in a 401 from the Hetzner API.Suggested fix: When
hasTokenPropis true, emitundefinedinstead ofg.tokenin the auth argument position (falling back to the env var), or skip emitting the auth argument entirely. Something like:This is MEDIUM because no current Hetzner resource has a
tokenproperty, and a collision would be caught during regeneration review. But the guard's intent is to handle this case, and it's currently incomplete.listAllpagination does not detect cycles —libGenerator.ts:188-211The loop assigns
page = nextPagefrom the API response on each iteration. If the API returned anext_pagevalue that points to an already-visited page (e.g., a bug where page 3's response saysnext_page: 1), the function would re-fetch pages in a cycle, accumulating duplicate items up tomaxPages * perPage = 50,000entries before the bounded guard breaks out.Breaking example: A Hetzner API bug or proxy misbehavior returns
{ servers: [...], meta: { pagination: { next_page: 1 } } }on every page. Result: 50,000 duplicate items returned instead of the real ~50, consuming memory and writing duplicatestateresources.Suggested fix: Track visited page numbers in a
Set<number>and break with a warning if a page is revisited:This is MEDIUM because the bounded guard prevents an infinite loop, but the duplicate-accumulation behavior could still be surprising.
Low
listmethod always advertises label selector support —extensionModelGenerator.ts:360The generated
listdescription always says "optionally filtered by a Hetzner label selector" and the method always passeslabel_selectoras a query parameter. Not all Hetzner resources support label selectors (e.g., DNS zones use a different filtering model). The Hetzner API would ignore the unknown parameter, so this is functionally harmless, but the description could be misleading.validatedTokensSet grows unboundedly —libGenerator.ts:15The
Set<string>caching validated tokens is never pruned. In a long-running process that rotates through many tokens (e.g., a CI runner using short-lived vault tokens), the Set would grow without bound. In practice, Hetzner API tokens are long-lived and the number of distinct tokens per process is small, so this is unlikely to matter.No test for
listAllwith empty collection response —libGenerator_test.tsThe test suite covers single-page, multi-page, and error responses, but doesn't test the case where the API returns an empty collection (e.g.,
{ servers: [], meta: { pagination: { next_page: null } } }).unwrapListwould return[]correctly via the array check, so this is just a coverage gap, not a bug.Verdict
PASS — The code is well-structured, tests are thorough, and the auth-threading and pagination implementation are solid. The collision guard incompleteness (finding #1) is the most concerning item but is currently theoretical since no Hetzner resource has a
tokenproperty. The pagination cycle detection gap (finding #2) is bounded by the page guard. Neither rises to blocking severity.Code Review
Blocking Issues
None.
Model files under
model/hetzner-cloud/are all auto-generated (confirmed by// Auto-generatedheaders and the presence of correspondingcodegen/hetzner/changes). No hand-edits detected. CLAUDE.md rules are otherwise followed: named exports only, noanyin hand-written code (deno-lint-ignorecomments are correctly scoped to OpenAPI-parsing functions), npm dependencies in the generated import use an exact version (npm:zod@4.3.6). Tests usewithFetchQueue/withFetchRouterlocal stubs (no live cloud calls), and all env vars are restored infinallyblocks.Suggestions
remove()leaves the 404 response body unconsumed (libGenerator.ts, generatedremove()function)When
request()returns a 404,remove()returns immediately without draining the body:Compare with
tryRead(), which correctly callsawait resp.text()before returningnullon 404. For real HTTP connections, an unconsumed body prevents keep-alive reuse. Consider mirroringtryRead():HetznerResource.identifyingFieldis set but never consumed (pipeline.tsline 481,extensionModelGenerator.ts)mergeResourceOperations()setsidentifyingField: postBody.name ? "name" : "id", but the generator never readsresource.identifyingField— it callsresolveNamingField(resource)independently, which also handles thelabelcase thatidentifyingFieldmisses. The field is dead code. The type definition, assignment, and thelabel-naming gap are all harmless right now, but if any future caller readsidentifyingFieldexpecting parity withresolveNamingField, it would get a wrong value for label-named resources (e.g.placement_groups).listAll()silently returns[]on a 404 (libGenerator.ts, generatedlistAll())request()returns 404 responses rather than throwing.listAll()then callsresp.json()on the 404 body,unwrapList()finds no array-valued key, and the function returns an empty list without any error signal. A caller that hits a misconfigured or non-existent endpoint would receive[]and no indication of the problem. Adding an explicit status check afterrequest()(analogous to howupdate()checks for 404) would make the failure visible. This is low-probability against the real Hetzner API (collection endpoints return200 + []for empty lists) but can silently bite during development or if the endpoint path changes.libGenerator_test.tshas no tests forread(),update(), ortryRead()The lib test file covers
remove()(seven cases),getToken()(two cases), andlistAll()(three cases). Theread()404→throw path,update()404→throw path, andtryRead()200→unwrap/404→null semantics have no dedicated tests. The behavior assertions inextensionModelGenerator_test.tstouchreadandtryReadindirectly by checking generated source strings, but those don't execute the runtime logic. Given the depth of coverage elsewhere in this PR, adding a few shortread/update/tryReadtest cases tolibGenerator_test.tswould complete the picture.Nested-object indentation in generated Zod schemas is inconsistent (
extensionModelGenerator.tsgenerateFullFidelityZod/generateSimplifiedZod, visible in snapshot)Both generators hard-code
" "(four-space) for nested object fields and" "(two-space) for the closing}), producing output like:The inconsistency is cosmetic and doesn't affect runtime behaviour, but since these files pass through
deno fmt, it would be cleaned up on format — runningdeno fmton the output directory before committing snapshots would make the snapshots reflect the formatted output.