From e9b13378c2cf8d1db70f00109aa880ca753dfc01 Mon Sep 17 00:00:00 2001 From: Travis Wu Date: Thu, 10 Sep 2026 12:11:59 +0800 Subject: [PATCH] toolplane: check every allowlist path against the API that must serve it 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 --- internal/toolplane/allowlist.go | 24 +++- internal/toolplane/conformance_test.go | 130 ++++++++++++++++++ .../toolplane/testdata/cube-cos-api-paths.txt | 109 +++++++++++++++ 3 files changed, 258 insertions(+), 5 deletions(-) create mode 100644 internal/toolplane/conformance_test.go create mode 100644 internal/toolplane/testdata/cube-cos-api-paths.txt diff --git a/internal/toolplane/allowlist.go b/internal/toolplane/allowlist.go index 4925e6f..b504f68 100644 --- a/internal/toolplane/allowlist.go +++ b/internal/toolplane/allowlist.go @@ -371,11 +371,25 @@ var Allowlist = []Tool{ // spends and what code it runs are the customer's decisions, made in // advance and not per call. // - // The path and body field names must be confirmed against the running - // cube-cos-api's OpenAPI document before this is enabled on a real cluster. - // Nothing here can act until an operator both raises a level and wires a - // writer, so shipping the mechanism ahead of that confirmation costs - // nothing; taking it on trust when the level is first raised would not. + // The path was confirmed against cube-cos-api's OpenAPI document, and it + // is wrong: there is no POST /api/v1/datacenters/{dataCenter}/instances, + // in the document, in the copy embedded at build time, or as a handler in + // that API's source. Its resource families are nodes, images, volumes, + // settings, tunings and the rest; VM lifecycle is not among them. So the + // body field names below are unconfirmable too — there is no schema to + // confirm them against. + // + // The entry stays because the mechanism around it is what slice 3 built + // and tested — the level gate, the approval statement, the write ledger, + // the refusal a caller can read — and none of that is wrong. What is + // missing is somewhere to send the request. Whoever supplies that decides + // the shape: an endpoint on cube-cos-api, or a transport that reaches + // whatever owns instances. Until then the write cannot succeed, and it + // cannot be attempted either, since a cluster at observe never offers it. + // + // conformance_test.go holds this as a named debt rather than a comment, + // so it is checked every run and cannot be forgotten the way this + // sentence's predecessor nearly was. { Name: "create_instance", Description: "Create one virtual machine from this cluster's configured instance profile. " + diff --git a/internal/toolplane/conformance_test.go b/internal/toolplane/conformance_test.go new file mode 100644 index 0000000..1d32fc4 --- /dev/null +++ b/internal/toolplane/conformance_test.go @@ -0,0 +1,130 @@ +package toolplane + +import ( + "bufio" + "os" + "strings" + "testing" +) + +// The allowlist names cube-cos-api paths as string literals, and nothing used +// to check them against the API that has to serve them. create_instance +// shipped pointing at /api/v1/datacenters/{dc}/instances, which cube-cos-api +// does not have and has never had: not in its OpenAPI document, not in its +// embedded copy, and not as a handler in its source. It was found by reading +// the spec, which is a thing a person does once. This does it every run. +// +// testdata/cube-cos-api-paths.txt is vendored rather than read from a checkout +// of cube-cos-api, because CI has no such checkout and a test that skips when +// its input is missing reports green for the case it was written to catch. +// The file's header names the revision it came from and the command that +// regenerates it. + +// dcPlaceholder is the one rewrite between the two vocabularies: the allowlist +// writes {dc} because the executor fills it from its own configuration, and +// the OpenAPI document writes {dataCenter} because that is the parameter's +// name. Pinned here so a change to either is a failure rather than a silent +// mismatch in a substitution nobody reads. +const ( + allowlistDC = "{dc}" + specDC = "{dataCenter}" +) + +// notInTheSpec lists allowlist paths knowingly absent from cube-cos-api, +// each with the reason. An entry here is a debt, not a dispensation: it says +// the tool cannot work today, and removing it is what shipping the tool means. +var notInTheSpec = map[string]string{ + "/api/v1/datacenters/{dc}/instances": "cube-cos-api does not create instances — " + + "no such path in its OpenAPI document or source, and VM lifecycle is not " + + "this API's concern. create_instance cannot write until either that endpoint " + + "exists or the tool is given a transport that reaches whatever does.", +} + +func specPaths(t *testing.T) map[string]bool { + t.Helper() + + f, err := os.Open("testdata/cube-cos-api-paths.txt") + if err != nil { + t.Fatalf("open the vendored path list: %v", err) + } + defer f.Close() + + paths := map[string]bool{} + s := bufio.NewScanner(f) + for s.Scan() { + line := strings.TrimSpace(s.Text()) + if line == "" || strings.HasPrefix(line, "#") { + continue + } + paths[line] = true + } + if err := s.Err(); err != nil { + t.Fatalf("read the vendored path list: %v", err) + } + // A truncated or empty file would make every path "absent" and every + // exclusion look justified, which is the failure this test exists to + // prevent in the code it checks. + if len(paths) < 50 { + t.Fatalf("vendored path list holds %d paths, far fewer than cube-cos-api serves; it is truncated", len(paths)) + } + return paths +} + +// TestEveryAllowlistPathIsOneTheAPIServes is the check that would have caught +// create_instance before it shipped. +func TestEveryAllowlistPathIsOneTheAPIServes(t *testing.T) { + spec := specPaths(t) + + for _, tool := range append(append([]Tool{}, Allowlist...), ProbeControls...) { + for _, path := range []string{tool.Get, tool.Post} { + if path == "" { + continue + } + if reason, known := notInTheSpec[path]; known { + t.Logf("%s: %s is knowingly absent from the API: %s", tool.Name, path, reason) + continue + } + if !spec[strings.ReplaceAll(path, allowlistDC, specDC)] { + t.Errorf("%s names %s, which cube-cos-api does not serve; "+ + "fix the path, or list it in notInTheSpec with the reason it cannot work yet", + tool.Name, path) + } + } + } +} + +// TestAnExcludedPathIsOneTheAPIReallyLacks keeps the exclusion list honest in +// the other direction: an entry that the API has since gained is a debt +// someone already paid, and leaving it listed hides a working tool behind a +// note saying it cannot work. +func TestAnExcludedPathIsOneTheAPIReallyLacks(t *testing.T) { + spec := specPaths(t) + + for path, reason := range notInTheSpec { + if spec[strings.ReplaceAll(path, allowlistDC, specDC)] { + t.Errorf("%s is listed as absent from cube-cos-api (%q) but the spec now has it; remove the exclusion", + path, reason) + } + } +} + +// TestEveryExcludedPathIsStillInTheAllowlist stops the list outliving what it +// describes. A stale exclusion reads as a known gap in a tool that no longer +// exists, which is a wrong statement about the state of the world. +func TestEveryExcludedPathIsStillInTheAllowlist(t *testing.T) { + declared := map[string]bool{} + for _, tool := range append(append([]Tool{}, Allowlist...), ProbeControls...) { + if tool.Get != "" { + declared[tool.Get] = true + } + if tool.Post != "" { + declared[tool.Post] = true + } + } + + for path := range notInTheSpec { + if !declared[path] { + t.Errorf("notInTheSpec lists %s, which no tool declares; delete the entry", path) + } + } +} diff --git a/internal/toolplane/testdata/cube-cos-api-paths.txt b/internal/toolplane/testdata/cube-cos-api-paths.txt new file mode 100644 index 0000000..f9da1d2 --- /dev/null +++ b/internal/toolplane/testdata/cube-cos-api-paths.txt @@ -0,0 +1,109 @@ +# cube-cos-api paths, extracted from its OpenAPI document. +# +# Source: bigstack-oss/cube-cos-openapi docs.yaml +# Submodule: 09b7d76b4a8e877b9d2560528faa68b2065fc703 +# Vendored by: cube-cos-api 0458966e6af68840cc183214eba15a6e9f36c393 +# +# Regenerate: grep -oE "^ \"/api/v1/[^\"]*\"" docs.yaml | tr -d " \"" | sort +/api/v1/datacenters +/api/v1/datacenters/{dataCenter} +/api/v1/datacenters/{dataCenter}/events +/api/v1/datacenters/{dataCenter}/events/abstract +/api/v1/datacenters/{dataCenter}/events/filterConditions +/api/v1/datacenters/{dataCenter}/events/predefined +/api/v1/datacenters/{dataCenter}/events/rank +/api/v1/datacenters/{dataCenter}/firmwares +/api/v1/datacenters/{dataCenter}/firmwares/abort +/api/v1/datacenters/{dataCenter}/firmwares/continueAnyway/{nodeName} +/api/v1/datacenters/{dataCenter}/firmwares/md5sum +/api/v1/datacenters/{dataCenter}/firmwares/md5sum/verify +/api/v1/datacenters/{dataCenter}/firmwares/upgradeProgress +/api/v1/datacenters/{dataCenter}/firmwares/{version} +/api/v1/datacenters/{dataCenter}/firmwares/{version}/{nodeName} +/api/v1/datacenters/{dataCenter}/firmwares/{version}/updatableNodes +/api/v1/datacenters/{dataCenter}/fixpacks +/api/v1/datacenters/{dataCenter}/fixpacks/continueAnyway/{nodeName} +/api/v1/datacenters/{dataCenter}/fixpacks/md5sum +/api/v1/datacenters/{dataCenter}/fixpacks/md5sum/verify +/api/v1/datacenters/{dataCenter}/fixpacks/updateProgress/{version} +/api/v1/datacenters/{dataCenter}/fixpacks/{version} +/api/v1/datacenters/{dataCenter}/fixpacks/{version}/rollback +/api/v1/datacenters/{dataCenter}/fixpacks/{version}/rollbackableNodes +/api/v1/datacenters/{dataCenter}/fixpacks/{version}/updatableNodes +/api/v1/datacenters/{dataCenter}/grafana/devices/{hostname}/gpuUtilization +/api/v1/datacenters/{dataCenter}/grafana/devices/{hostname}/gpuVram +/api/v1/datacenters/{dataCenter}/grafana/hosts/{hostname} +/api/v1/datacenters/{dataCenter}/grafana/instances/{instanceId} +/api/v1/datacenters/{dataCenter}/grafana/networkDevices +/api/v1/datacenters/{dataCenter}/grafana/networks +/api/v1/datacenters/{dataCenter}/grafana/storages +/api/v1/datacenters/{dataCenter}/grafana/topHosts +/api/v1/datacenters/{dataCenter}/grafana/topInstances +/api/v1/datacenters/{dataCenter}/healths +/api/v1/datacenters/{dataCenter}/healths/services/{serviceType} +/api/v1/datacenters/{dataCenter}/healths/services/{serviceType}/modules/{moduleType} +/api/v1/datacenters/{dataCenter}/images +/api/v1/datacenters/{dataCenter}/images.csv +/api/v1/datacenters/{dataCenter}/images/{imageId} +/api/v1/datacenters/{dataCenter}/images/materials +/api/v1/datacenters/{dataCenter}/integrations/applications +/api/v1/datacenters/{dataCenter}/integrations/storages +/api/v1/datacenters/{dataCenter}/integrations/storages/models +/api/v1/datacenters/{dataCenter}/integrations/storages/models/{driverName} +/api/v1/datacenters/{dataCenter}/integrations/storages/{storageName} +/api/v1/datacenters/{dataCenter}/integrations/storages/{storageName}/asDefault +/api/v1/datacenters/{dataCenter}/integrations/storages/{storageName}/verify +/api/v1/datacenters/{dataCenter}/integrations/storages/vendors +/api/v1/datacenters/{dataCenter}/licenses +/api/v1/datacenters/{dataCenter}/licenses/attachments +/api/v1/datacenters/{dataCenter}/licenses/hosts/{hostname} +/api/v1/datacenters/{dataCenter}/licenses/verify +/api/v1/datacenters/{dataCenter}/me +/api/v1/datacenters/{dataCenter}/metrics +/api/v1/datacenters/{dataCenter}/metrics/{metricType}/{viewType}/{entityType} +/api/v1/datacenters/{dataCenter}/metrics/{metricType}/{viewType}/{entityType}/{entityIdorName} +/api/v1/datacenters/{dataCenter}/nodes +/api/v1/datacenters/{dataCenter}/nodes/{nodeName} +/api/v1/datacenters/{dataCenter}/nodes/{nodeName}/devices +/api/v1/datacenters/{dataCenter}/nodes/{nodeName}/devices/{deviceName} +/api/v1/datacenters/{dataCenter}/nodes/{nodeName}/gpuCards +/api/v1/datacenters/{dataCenter}/nodes/{nodeName}/gpuCards/{gpuId} +/api/v1/datacenters/{dataCenter}/nodes/{nodeName}/gpuCards/instances/{instanceId}/console +/api/v1/datacenters/{dataCenter}/nodes/{nodeName}/ipmi +/api/v1/datacenters/{dataCenter}/nodes/{nodeName}/ipmi/disconnect +/api/v1/datacenters/{dataCenter}/nodes/{nodeName}/ipmi/{operation} +/api/v1/datacenters/{dataCenter}/nodes/{nodeName}/ipmi/verify +/api/v1/datacenters/{dataCenter}/nodes/{nodeName}/osds/{osdId} +/api/v1/datacenters/{dataCenter}/nodes/{nodeName}/osds/{osdId}/restart +/api/v1/datacenters/{dataCenter}/nodes/{nodeName}/softReboot +/api/v1/datacenters/{dataCenter}/notifications +/api/v1/datacenters/{dataCenter}/notifications/last +/api/v1/datacenters/{dataCenter}/opensearch/requests/{requestId} +/api/v1/datacenters/{dataCenter}/rollingReboot +/api/v1/datacenters/{dataCenter}/services +/api/v1/datacenters/{dataCenter}/settings +/api/v1/datacenters/{dataCenter}/settings/email/recipients +/api/v1/datacenters/{dataCenter}/settings/email/recipients/{recipientEmail} +/api/v1/datacenters/{dataCenter}/settings/email/senders +/api/v1/datacenters/{dataCenter}/settings/email/senders/{senderHost} +/api/v1/datacenters/{dataCenter}/settings/slack/channels +/api/v1/datacenters/{dataCenter}/settings/slack/channels/{channelName} +/api/v1/datacenters/{dataCenter}/settings/titlePrefix +/api/v1/datacenters/{dataCenter}/supportFiles +/api/v1/datacenters/{dataCenter}/supportFiles/hosts/{hostname} +/api/v1/datacenters/{dataCenter}/supportFiles/{supportFileSet} +/api/v1/datacenters/{dataCenter}/tokens +/api/v1/datacenters/{dataCenter}/triggers +/api/v1/datacenters/{dataCenter}/triggers/materials +/api/v1/datacenters/{dataCenter}/triggers/materials/script/verify +/api/v1/datacenters/{dataCenter}/triggers/{triggerName} +/api/v1/datacenters/{dataCenter}/triggers/{triggerName}/enable +/api/v1/datacenters/{dataCenter}/tunings/parameters +/api/v1/datacenters/{dataCenter}/tunings/parameters/{parameterName} +/api/v1/datacenters/{dataCenter}/tunings/parameters/{parameterName}/enable +/api/v1/datacenters/{dataCenter}/tunings/parameters/{parameterName}/reset +/api/v1/datacenters/{dataCenter}/tunings/specs +/api/v1/datacenters/{dataCenter}/volumes +/api/v1/datacenters/{dataCenter}/volumes.csv +/api/v1/datacenters/{dataCenter}/volumes/images +/api/v1/logout