toolplane: check every allowlist path against the API that must serve it - #33
Merged
Merged
Conversation
create_instance shipped naming POST /api/v1/datacenters/{dc}/instances.
cube-cos-api does not serve that path: it is absent from the OpenAPI
document, from the copy embedded at build time, and from that API's
source, whose resource families are nodes, images, volumes, settings,
tunings and the rest. VM lifecycle is not that API's concern, so the
body field names have no schema to be confirmed against either.
The slice-3 entry said the path "must be confirmed before this is
enabled on a real cluster". Confirming it is a thing a person does once
and then stops doing. This does it every run, against a vendored
extract of the spec rather than a checkout CI does not have, because a
test that skips when its input is missing reports green for the case it
was written to catch.
create_instance is listed as a named debt with its reason, and two
further tests keep that list honest: one fails if the API gains a path
the list still calls missing, the other if the list outlives the tool
it describes. The three shipped read paths pass unaided.
Signed-off-by: Travis Wu <travis.wu@bigstack.co>
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.
What
Every allowlist
Get/Postpath is now checked against cube-cos-api's OpenAPI document on each test run, andcreate_instance's path is recorded as a named debt because the endpoint it names does not exist.Why
Slice 3 of ADR 0011 shipped
create_instancewith this in its comment:Confirmed. The answer is worse than "unverified":
Not in
api/cube-cos-openapi/docs.yaml(submodule09b7d76, via cube-cos-api0458966), not in theapi/docs.jsonembedded at build time, and not as a handler in that API's source. Its 102 paths covernodes,images,volumes,settings,tunings,fixpacks,firmwares,grafana,events,healthsand the rest — VM lifecycle is not among them. The only two paths containinginstancesare a GPU-card console and a Grafana dashboard link.Control: the three shipped read paths —
healths,nodes,events— are all present, so the method distinguishes a missing path from a bad search.Because there is no endpoint, there is no request schema, so the
Bodyfield names cannot be confirmed either.CubeCOSPosteris documented as "an authenticated write against the local cube-cos-api", so the tool cannot reach whatever does own instances without a new transport.This was safe only by accident of sequencing: no cluster has a level above
observeand no writer is wired, so nothing could have called it. The moment either changed it would have been a failed write the model could not explain.Reviewer notes
The entry stays. Everything slice 3 built around it — the level gate, the approval statement, the write ledger, the readable refusal — is correct and tested. What is missing is somewhere to send the request. Whoever supplies that decides the shape: an endpoint on cube-cos-api, or a transport reaching whatever owns instances. That is a design call, not a rename, so it is not made here.
Vendored, not read from a checkout.
testdata/cube-cos-api-paths.txtholds the extracted path list with the revision it came from and the command that regenerates it. CI has no cube-cos-api checkout, and a test that skips when its input is missing reports green for exactly the case it was written to catch — the same argument #177 made for the corpus snapshot.specPathsalso refuses a list under 50 paths, since a truncated file would make every path look absent and every exclusion look justified.The exclusion list is kept honest in both directions.
TestAnExcludedPathIsOneTheAPIReallyLacksfails if the API gains a path the list still calls missing — a debt someone paid, still recorded as owing.TestEveryExcludedPathIsStillInTheAllowlistfails if the list outlives the tool it describes.{dc}vs{dataCenter}is the one rewrite between the two vocabularies, pinned as a constant rather than inlined, so a change to either is a failure and not a silent mismatch.Proved failable — each break watched, then restored:
TestEveryAllowlistPathIsOneTheAPIServes"create_instance names …/instances, which cube-cos-api does not serve"TestEveryExcludedPathIsStillInTheAllowlist"no tool declares"TestAnExcludedPathIsOneTheAPIReallyLacks"the spec now has it"go test ./...RC=0, all packages.go vetclean.gofmt -l internal/toolplane/clean — notecmd/agent/main_test.gois gofmt-unclean ondevelopalready and was left alone.Docs
Handbook: the slice-3 as-built records the path as unverified; that sentence is being corrected in a separate handbook PR.