diff --git a/internal/toolplane/allowlist.go b/internal/toolplane/allowlist.go index 7478139..8e369ae 100644 --- a/internal/toolplane/allowlist.go +++ b/internal/toolplane/allowlist.go @@ -136,6 +136,12 @@ func (s Shape) String() string { // maxDNSLabel is the DNS limit, and doubles as the length bound. const maxDNSLabel = 63 +// catalogArg is the one argument a catalogue read takes: which entry to fetch. +// Named once here rather than written as a literal in the registry and again +// in the SaaS's schema, because the two have to agree and a string typed twice +// is a string that eventually differs. +const catalogArg = "resource" + // admits reports whether v satisfies the shape. // // Written as explicit character classes rather than a compiled regexp, so the @@ -274,9 +280,29 @@ type Tool struct { // and an idempotency key, because a repeated read is free and a repeated // create is not. // - // Exactly one of Argv, Get, Post or Control is set. + // Exactly one of Argv, Get, Post, Catalog or Control is set. Post string + // Catalog is a finite set of cube-cos-api GET paths this one tool may + // fetch: the value the caller supplies, mapped to the path template it + // selects. The caller names a key; anything else is refused. The method is + // GET, always, for the same structural reason Get is — there is no field + // on a catalog entry that could make it anything else. + // + // It exists because Get costs one tool per path, and a diagnosis assistant + // is asked to look at things we did not think of when we shipped. Three + // hand-written reads against an API that declares dozens is not a security + // property, it is a release cycle: every "can you also check X" was a new + // entry, a build and a deploy. + // + // A key is not a Shape and not a pattern — it is a map lookup, which is the + // same finite-value-set rule Params applies, in its strongest form. What a + // caller can express is exactly the set below and nothing else, so widening + // the read surface is still an edit to this file that a reviewer reads. + // + // Exactly one of Argv, Get, Post, Catalog or Control is set. + Catalog map[string]string + // Body is the JSON object sent with Post: a field name to either a literal // or a placeholder declared in Params or Free. There is no free-form body // and no pass-through of caller JSON, so the request this allowlist @@ -321,6 +347,93 @@ type Tool struct { Timeout time.Duration } +// CubeCOSReads is the set of cube-cos-api GET paths cube_cos_read may fetch, +// keyed by the value a caller supplies. +// +// Admission is opt-in. A path is reachable because it is written here, not +// because the API happens to serve it — so an endpoint cube-cos-api gains +// tomorrow is unreachable until someone reads it and adds it. The opposite +// rule, a list of paths to exclude, fails the wrong way: the next sensitive +// endpoint upstream would be reachable the day it shipped, and nobody here +// would know it existed. +// +// What is left out, and why, is in cubeCOSReadsHeldBack. Together the two +// account for every zero-parameter GET the API declares, and a test says so — +// so a new upstream read cannot be quietly unreachable either. It has to be +// classified, one way or the other, by a person. +// +// Three rules decided the split: +// +// - A read that can carry a credential is out. Settings, integrations and +// licenses hold SMTP passwords, storage-vendor logins, webhook URLs with +// tokens in them and license keys. None of it helps diagnose a cluster, +// and a diagnosis assistant reading them puts them in a transcript. +// - A read whose size is unbounded by anything here is out. Support bundles +// are the case: the result cap would truncate one to 32 KiB of an archive, +// which is worse than not offering it. +// - A duplicate is out. The .csv variants return what the JSON reads already +// return; two ways to ask the same question is a worse tool list, not a +// wider one. +// +// Query parameters are not expressible — the key selects a path and nothing +// else — so watch=true, which turns several of these into an event stream, is +// unreachable by construction rather than by exclusion. +var CubeCOSReads = map[string]string{ + "datacenter": "/api/v1/datacenters/{dc}", + "datacenters": "/api/v1/datacenters", + "events": "/api/v1/datacenters/{dc}/events", + "events/abstract": "/api/v1/datacenters/{dc}/events/abstract", + "events/filterConditions": "/api/v1/datacenters/{dc}/events/filterConditions", + "events/predefined": "/api/v1/datacenters/{dc}/events/predefined", + "events/rank": "/api/v1/datacenters/{dc}/events/rank", + "firmwares": "/api/v1/datacenters/{dc}/firmwares", + "firmwares/upgrade": "/api/v1/datacenters/{dc}/firmwares/upgradeProgress", + "fixpacks": "/api/v1/datacenters/{dc}/fixpacks", + "healths": "/api/v1/datacenters/{dc}/healths", + "images": "/api/v1/datacenters/{dc}/images", + "images/materials": "/api/v1/datacenters/{dc}/images/materials", + "metrics": "/api/v1/datacenters/{dc}/metrics", + "nodes": "/api/v1/datacenters/{dc}/nodes", + "services": "/api/v1/datacenters/{dc}/services", + "triggers": "/api/v1/datacenters/{dc}/triggers", + "triggers/materials": "/api/v1/datacenters/{dc}/triggers/materials", + "tunings/parameters": "/api/v1/datacenters/{dc}/tunings/parameters", + "tunings/specs": "/api/v1/datacenters/{dc}/tunings/specs", + "volumes": "/api/v1/datacenters/{dc}/volumes", +} + +// cubeCOSReadsHeldBack is every other zero-parameter GET cube-cos-api declares, +// with the reason it is not in CubeCOSReads. +// +// It is data rather than prose so a test can require the two sets to cover the +// API between them. That is what makes admission opt-in and still visible: a +// read this agent will not perform is a decision written down, not an absence +// somebody has to notice. +var cubeCOSReadsHeldBack = map[string]string{ + "/api/v1/datacenters/{dataCenter}/settings": "configuration, and the delivery settings under it carry credentials", + "/api/v1/datacenters/{dataCenter}/settings/email/recipients": "email delivery configuration", + "/api/v1/datacenters/{dataCenter}/settings/email/senders": "email delivery configuration, sender credentials included", + "/api/v1/datacenters/{dataCenter}/settings/slack/channels": "chat delivery configuration, webhook URLs included", + "/api/v1/datacenters/{dataCenter}/integrations/applications": "external-system integration, credentials included", + "/api/v1/datacenters/{dataCenter}/integrations/storages": "storage-vendor integration, logins included", + "/api/v1/datacenters/{dataCenter}/integrations/storages/models": "integration catalogue; only useful alongside the " + + "integration reads that are held back", + "/api/v1/datacenters/{dataCenter}/integrations/storages/vendors": "integration catalogue; same", + "/api/v1/datacenters/{dataCenter}/licenses": "license keys", + "/api/v1/datacenters/{dataCenter}/licenses/attachments": "license material", + "/api/v1/datacenters/{dataCenter}/me": "the executor's own API identity, not a fact about the cluster", + "/api/v1/datacenters/{dataCenter}/notifications": "notification payloads and their delivery targets", + "/api/v1/datacenters/{dataCenter}/notifications/last": "same", + "/api/v1/datacenters/{dataCenter}/supportFiles": "support bundles; unbounded, and a 32 KiB slice of an archive is not a read", + "/api/v1/datacenters/{dataCenter}/grafana/networkDevices": "dashboard payload, may embed an access token; not diagnostic text", + "/api/v1/datacenters/{dataCenter}/grafana/networks": "same", + "/api/v1/datacenters/{dataCenter}/grafana/storages": "same", + "/api/v1/datacenters/{dataCenter}/grafana/topHosts": "same", + "/api/v1/datacenters/{dataCenter}/grafana/topInstances": "same", + "/api/v1/datacenters/{dataCenter}/images.csv": "CSV duplicate of the images read", + "/api/v1/datacenters/{dataCenter}/volumes.csv": "CSV duplicate of the volumes read", +} + // Allowlist is the complete set of tools the agent will serve. // // Adding an entry here is the security-relevant act. Keep it short, keep every @@ -382,26 +495,21 @@ var Allowlist = []Tool{ }, // cube-cos-api reads. GET only, structurally — the resolved path is the // whole request, and {dc} is filled by the executor, so the SaaS chooses - // which overview to fetch and nothing else. Start with the three - // zero-parameter overviews; per-resource reads (a named node, a service's - // health) are added the same way once their value sets are pinned down. - { - Name: "cube_cos_healths", - Description: "Cluster health summary from cube-cos-api: every service and its state.", - Get: "/api/v1/datacenters/{dc}/healths", - Impact: ImpactRead, - }, - { - Name: "cube_cos_nodes", - Description: "The cluster's nodes and their roles/state from cube-cos-api.", - Get: "/api/v1/datacenters/{dc}/nodes", - Impact: ImpactRead, - }, + // which overview to fetch and nothing else. + // + // One tool over a catalogue rather than one tool per path. Twenty-one + // entries as twenty-one tools would be twenty-one names and descriptions + // in front of the model on every turn, for reads that differ only in which + // noun they return; and because tool specs are what the SaaS fingerprints + // as its prompt stamp, the list would move that stamp every time the + // catalogue grew. One tool with a finite key set costs one name and one + // enum, and adding a read moves nothing but the enum. { - Name: "cube_cos_events", - Description: "Recent cluster events from cube-cos-api — the timeline of what changed.", - Get: "/api/v1/datacenters/{dc}/events", - Impact: ImpactRead, + Name: "cube_cos_read", + Description: "Read one overview from cube-cos-api: health, nodes, events, images, " + + "volumes, services, firmwares, fixpacks, tunings, triggers or metrics.", + Catalog: CubeCOSReads, + Impact: ImpactRead, }, // The first entry that changes the cluster (ADR 0011, slice 3). Served // only where the node's action-level file says operate or internal; every @@ -541,13 +649,13 @@ func (t Tool) validate(probes bool) error { return fmt.Errorf("tool %q has a negative timeout", t.Name) } kinds := 0 - for _, set := range []bool{len(t.Argv) > 0, t.Get != "", t.Post != "", t.Control != 0} { + for _, set := range []bool{len(t.Argv) > 0, t.Get != "", t.Post != "", len(t.Catalog) > 0, t.Control != 0} { if set { kinds++ } } if kinds != 1 { - return fmt.Errorf("tool %q must be exactly one of a command (Argv), a cube-cos-api read (Get), a cube-cos-api write (Post) or a probe-plane control (Control)", t.Name) + return fmt.Errorf("tool %q must be exactly one of a command (Argv), a cube-cos-api read (Get), a cube-cos-api catalogue read (Catalog), a cube-cos-api write (Post) or a probe-plane control (Control)", t.Name) } // A placeholder is either enumerated or shaped, never both: two answers to // "what may this value be" is the same as none. @@ -573,6 +681,8 @@ func (t Tool) validate(probes bool) error { return t.validateControl() case t.Get != "": return t.validateGet() + case len(t.Catalog) > 0: + return t.validateCatalog() case t.Post != "": return t.validatePost() default: @@ -580,6 +690,38 @@ func (t Tool) validate(probes bool) error { } } +// validateCatalog checks a catalogue read entry. +// +// Every rule validateGet applies to one path is applied to each of them, plus +// two the catalogue form makes possible: no parameters, because the key is the +// only thing a caller supplies and a second argument would be a second thing +// to check; and no placeholder other than {dc}, because a key selecting a +// template that still needed an argument would put a value back in the caller's +// hands through the side door. +func (t Tool) validateCatalog() error { + if t.Impact != ImpactRead { + return fmt.Errorf("tool %q is a cube-cos-api catalogue read but declares impact %s; a GET changes nothing", t.Name, t.Impact) + } + if len(t.Params) > 0 || len(t.Free) > 0 { + return fmt.Errorf("tool %q is a catalogue read and declares parameters; the key is the only argument", t.Name) + } + for key, path := range t.Catalog { + if key == "" { + return fmt.Errorf("tool %q has a catalogue entry with an empty key", t.Name) + } + if !strings.HasPrefix(path, "/") { + return fmt.Errorf("tool %q catalogue entry %q has a path that is not absolute", t.Name, key) + } + for _, seg := range pathSegments(path) { + if isPlaceholder(seg) && seg != dcPlaceholder { + return fmt.Errorf("tool %q catalogue entry %q leaves %s unfilled; a catalogue path takes no argument but %s", + t.Name, key, seg, dcPlaceholder) + } + } + } + return nil +} + // validatePost checks a cube-cos-api write entry. // // Everything validateGet requires of a path, plus the body. A write is refused diff --git a/internal/toolplane/catalog_test.go b/internal/toolplane/catalog_test.go new file mode 100644 index 0000000..aa64d38 --- /dev/null +++ b/internal/toolplane/catalog_test.go @@ -0,0 +1,218 @@ +package toolplane + +import ( + "context" + "errors" + "fmt" + "strings" + "testing" +) + +// catalogTool is a small stand-in for the shipped read: two keys, so a miss +// and a hit are both expressible without depending on which overviews the +// product happens to serve this week. +func catalogTool() Tool { + return Tool{ + Name: "cube_cos_read", + Description: "Read one overview from cube-cos-api.", + Catalog: map[string]string{ + "healths": "/api/v1/datacenters/{dc}/healths", + "nodes": "/api/v1/datacenters/{dc}/nodes", + }, + Impact: ImpactRead, + } +} + +func TestACatalogueKeySelectsItsPathAndTheAgentFillsTheDatacenter(t *testing.T) { + r, _, fake := newGetRegistry(t, []Tool{catalogTool()}, "sky-dc") + + out, err := r.Call(context.Background(), "cube_cos_read", map[string]string{"resource": "nodes"}) + if err != nil { + t.Fatalf("Call: %v", err) + } + if fake.lastPath != "/api/v1/datacenters/sky-dc/nodes" { + t.Errorf("path = %q, want the key's template with the agent's own datacenter", fake.lastPath) + } + if string(out) != `{"ok":true}` { + t.Errorf("out = %q", out) + } +} + +// The set is closed. A key the catalogue does not hold is refused before +// anything is fetched, and no part of the caller's string reaches a path. +func TestAKeyOutsideTheCatalogueIsRefused(t *testing.T) { + r, rec, fake := newGetRegistry(t, []Tool{catalogTool()}, "sky-dc") + + _, err := r.Call(context.Background(), "cube_cos_read", map[string]string{"resource": "settings"}) + if !errors.Is(err, ErrBadArgument) { + t.Fatalf("err = %v, want a bad-argument refusal", err) + } + if fake.lastPath != "" { + t.Errorf("a refused key still reached the API at %q", fake.lastPath) + } + if len(rec.calls) != 1 || rec.calls[0].Allowed { + t.Fatal("the refusal was not audited as a refusal") + } +} + +// A path the API serves but the catalogue does not admit is unreachable for +// the same reason any other unlisted string is: it is not a key. Held-back +// reads are the case that matters — settings and licenses are in the API and +// must not be fetchable through this tool. +func TestAHeldBackReadIsNotReachableThroughTheCatalogue(t *testing.T) { + for _, attempt := range []string{ + "settings", + "licenses", + "me", + "supportFiles", + "/api/v1/datacenters/{dc}/settings", + "../settings", + } { + if _, ok := CubeCOSReads[attempt]; ok { + t.Errorf("%q is a catalogue key; a held-back read is reachable", attempt) + } + } +} + +// Nothing on this path can express a write. The catalogue holds paths, the +// method is not a field, and the fetch goes through the read client — so a +// caller has no way to name one. +func TestACatalogueReadCannotExpressAWrite(t *testing.T) { + for key, path := range CubeCOSReads { + if strings.Contains(strings.ToUpper(key), "POST") || strings.Contains(strings.ToUpper(key), "DELETE") { + t.Errorf("catalogue key %q reads like a method, not a resource", key) + } + if !strings.HasPrefix(path, "/api/v1/") { + t.Errorf("catalogue key %q maps to %q, which is not a cube-cos-api path", key, path) + } + } + // The shipped entry declares no write field, and validate would refuse it + // alongside a catalogue anyway — a tool is exactly one kind. + _, err := New([]Tool{{ + Name: "both", + Catalog: map[string]string{"x": "/api/v1/x"}, + Post: "/api/v1/x", + Body: map[string]string{"a": "b"}, + Impact: ImpactRead, + }}, &recorder{}) + if err == nil { + t.Error("a tool that is both a catalogue read and a write registered") + } +} + +// A second argument is not a place to smuggle anything: the key is the only +// thing a caller supplies. +func TestACatalogueReadTakesNoOtherArgument(t *testing.T) { + r, _, fake := newGetRegistry(t, []Tool{catalogTool()}, "sky-dc") + + _, err := r.Call(context.Background(), "cube_cos_read", map[string]string{ + "resource": "nodes", + "watch": "true", + }) + if !errors.Is(err, ErrBadArgument) { + t.Fatalf("err = %v, want a bad-argument refusal", err) + } + if fake.lastPath != "" { + t.Errorf("the call reached the API at %q despite an unexpected argument", fake.lastPath) + } +} + +func TestACatalogueReadWithoutItsKeyIsRefused(t *testing.T) { + r, _, _ := newGetRegistry(t, []Tool{catalogTool()}, "sky-dc") + + if _, err := r.Call(context.Background(), "cube_cos_read", nil); !errors.Is(err, ErrBadArgument) { + t.Fatalf("err = %v, want a bad-argument refusal", err) + } +} + +// The result cap and its marker apply here exactly as they do to a hand-written +// read — both forms share one fetch, and tool-0010 measures whether the model +// reports a cut it was told about. +func TestACatalogueReadIsCappedAndSaysSo(t *testing.T) { + rec := &recorder{} + tool := catalogTool() + tool.MaxOutputBytes = 64 + r, err := New([]Tool{tool}, rec) + if err != nil { + t.Fatalf("New: %v", err) + } + r.SetCubeCOSForTest("sky-dc", &fakeCubeCOS{ + body: strings.Repeat("x", 64), + err: fmt.Errorf("%w at 64 bytes", ErrOutputTruncated), + }) + + out, err := r.Call(context.Background(), "cube_cos_read", map[string]string{"resource": "healths"}) + if err != nil { + t.Fatalf("a truncated read must succeed, got %v", err) + } + if !strings.Contains(string(out), "truncated after") { + t.Errorf("the truncation marker did not reach the caller: %q", out) + } + if len(rec.calls) != 1 || !rec.calls[0].Truncated { + t.Error("the audit record does not say the result was truncated") + } +} + +// An agent with no cube-cos-api client refuses rather than fetching from a +// path built around an empty datacenter. +func TestACatalogueReadWithoutADatacenterIsRefused(t *testing.T) { + r, err := New([]Tool{catalogTool()}, &recorder{}) + if err != nil { + t.Fatalf("New: %v", err) + } + if _, err := r.Call(context.Background(), "cube_cos_read", map[string]string{"resource": "healths"}); err == nil { + t.Fatal("a catalogue read succeeded with no datacenter configured") + } +} + +// Registration rules specific to the catalogue form. +func TestACatalogueEntryMustBeAWellFormedRead(t *testing.T) { + cases := []struct { + name string + tool Tool + }{ + {"configuring class", Tool{ + Name: "c", Catalog: map[string]string{"x": "/api/v1/x"}, Impact: ImpactOperate, + }}, + {"declares parameters", Tool{ + Name: "c", Catalog: map[string]string{"x": "/api/v1/x"}, + Params: map[string][]string{"{y}": {"1"}}, Impact: ImpactRead, + }}, + {"relative path", Tool{ + Name: "c", Catalog: map[string]string{"x": "api/v1/x"}, Impact: ImpactRead, + }}, + {"unfilled placeholder", Tool{ + Name: "c", Catalog: map[string]string{"x": "/api/v1/datacenters/{dc}/nodes/{node}"}, Impact: ImpactRead, + }}, + {"empty key", Tool{ + Name: "c", Catalog: map[string]string{"": "/api/v1/x"}, Impact: ImpactRead, + }}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if _, err := New([]Tool{tc.tool}, &recorder{}); err == nil { + t.Errorf("a catalogue tool with %s registered", tc.name) + } + }) + } +} + +// The shipped catalogue is served at every level, observe included: reading is +// what observe is for, and widening the read surface widens it for the +// clusters that allow nothing else. Stated as a test so the property is a +// decision on the record rather than a consequence nobody wrote down. +func TestTheCatalogueIsServedAtObserve(t *testing.T) { + r, err := New(Allowlist, &recorder{}, WithLevel(LevelObserve)) + if err != nil { + t.Fatalf("New: %v", err) + } + var found bool + for _, n := range r.Names() { + if n == "cube_cos_read" { + found = true + } + } + if !found { + t.Error("cube_cos_read is not offered at observe; the read surface must not need a raised level") + } +} diff --git a/internal/toolplane/conformance_test.go b/internal/toolplane/conformance_test.go index dec8f74..5a6cbbf 100644 --- a/internal/toolplane/conformance_test.go +++ b/internal/toolplane/conformance_test.go @@ -56,9 +56,66 @@ var specFile = map[Backend]struct { // path "absent" and every exclusion look justified — the failure this // test exists to prevent in the code it checks. min int + // methods reports whether the file records "METHOD PATH" rather than + // paths alone. Only a file that does can answer "does this API serve + // this path by GET", which is what the read catalogue's claim rests on. + methods bool }{ - BackendCubeCOS: {"testdata/cube-cos-api-paths.txt", 50}, - BackendOpenStackCompute: {"testdata/nova-compute-paths.txt", 100}, + BackendCubeCOS: {path: "testdata/cube-cos-api-paths.txt", min: 50, methods: true}, + BackendOpenStackCompute: {path: "testdata/nova-compute-paths.txt", min: 100}, +} + +// specOps reads a backend's vendored list as method -> set of paths. +// +// Two file shapes are in use. cube-cos-api's records "METHOD PATH", because a +// catalogue claiming to reach only reads has to be checked against reads +// specifically, and a path-only list cannot tell a GET from a POST sharing a +// URL. nova's records paths alone. Asking for methods from a file that has +// none is a Fatal rather than an empty answer, for the same reason a backend +// with no vendored list is: an unchecked destination is how the first one went +// wrong. +func specOps(t *testing.T, b Backend) map[string]map[string]bool { + t.Helper() + + src, known := specFile[b] + if !known { + t.Fatalf("no vendored path list for backend %s; add one before a tool goes there", b) + } + if !src.methods { + t.Fatalf("the vendored list for %s records paths without methods; it cannot answer a question about GETs", b) + } + + f, err := os.Open(src.path) + if err != nil { + t.Fatalf("open the vendored operation list for %s: %v", b, err) + } + defer f.Close() + + ops := map[string]map[string]bool{} + total := 0 + sc := bufio.NewScanner(f) + for sc.Scan() { + line := strings.TrimSpace(sc.Text()) + if line == "" || strings.HasPrefix(line, "#") { + continue + } + method, path, ok := strings.Cut(line, " ") + if !ok { + t.Fatalf("vendored operation list for %s has a line that is not %q: %q", b, "METHOD PATH", line) + } + if ops[method] == nil { + ops[method] = map[string]bool{} + } + ops[method][path] = true + total++ + } + if err := sc.Err(); err != nil { + t.Fatalf("read the vendored operation list for %s: %v", b, err) + } + if total < src.min { + t.Fatalf("vendored operation list for %s holds %d operations, far fewer than it serves; it is truncated", b, total) + } + return ops } func specPaths(t *testing.T, b Backend) map[string]bool { @@ -76,15 +133,22 @@ func specPaths(t *testing.T, b Backend) map[string]bool { defer f.Close() paths := map[string]bool{} - s := bufio.NewScanner(f) - for s.Scan() { - line := strings.TrimSpace(s.Text()) + sc := bufio.NewScanner(f) + for sc.Scan() { + line := strings.TrimSpace(sc.Text()) if line == "" || strings.HasPrefix(line, "#") { continue } + // A "METHOD PATH" line contributes its path; a path-only line is the + // whole line. One reader for both shapes, so a caller asking only + // "does this API serve this path" need not know which it is reading. + if _, path, ok := strings.Cut(line, " "); ok { + paths[path] = true + continue + } paths[line] = true } - if err := s.Err(); err != nil { + if err := sc.Err(); err != nil { t.Fatalf("read the vendored path list for %s: %v", b, err) } if len(paths) < src.min { @@ -164,6 +228,95 @@ func TestTheCreateGoesToNovaAndNotTheManagementAPI(t *testing.T) { } } +// TestEveryCatalogueReadIsAGetTheAPIServes checks the widened read surface the +// same way the hand-written entries are checked, and one way further: a +// catalogue entry must be a path the API serves *by GET*. A path-only list +// could not tell a read from a write sharing a URL, and the catalogue's whole +// claim is that it cannot reach anything but reads. +func TestEveryCatalogueReadIsAGetTheAPIServes(t *testing.T) { + gets := specOps(t, BackendCubeCOS)["GET"] + if len(gets) == 0 { + t.Fatal("the vendored operation list records no GETs; it is malformed") + } + + for _, tool := range Allowlist { + for key, path := range tool.Catalog { + spec := specName(BackendCubeCOS, path) + if !gets[spec] { + t.Errorf("%s catalogue key %q names %s, which cube-cos-api does not serve by GET", + tool.Name, key, path) + } + } + } +} + +// TestEverySimpleGetIsAdmittedOrHeldBack is what makes admission opt-in and +// still visible. +// +// Opt-in alone would leave a read the API gains tomorrow silently unreachable: +// safe, but nobody would know it existed, and the catalogue would quietly fall +// behind the product. Requiring the two sets to cover the API between them +// turns that into a failing test naming the new path, so somebody classifies +// it. Admitting it stays a deliberate act; ignoring it stops being one. +// +// Scoped to GETs whose only placeholder is the datacenter, because those are +// the reads the catalogue can express — a per-resource read needs a value set +// or a shape for the identifier, which is a wider decision than this slice. +func TestEverySimpleGetIsAdmittedOrHeldBack(t *testing.T) { + admitted := map[string]bool{} + for _, tool := range Allowlist { + for _, path := range tool.Catalog { + admitted[specName(BackendCubeCOS, path)] = true + } + if tool.Get != "" { + admitted[specName(BackendCubeCOS, tool.Get)] = true + } + } + + for path := range specOps(t, BackendCubeCOS)["GET"] { + if strings.Contains(strings.ReplaceAll(path, specDC, ""), "{") { + continue // per-resource read; out of the catalogue's scope for now + } + if admitted[path] { + continue + } + if _, held := cubeCOSReadsHeldBack[path]; held { + continue + } + t.Errorf("cube-cos-api serves GET %s and this agent neither reads it nor says why not; "+ + "add it to CubeCOSReads or to cubeCOSReadsHeldBack with the reason", path) + } +} + +// TestNothingIsBothAdmittedAndHeldBack stops the two sets from disagreeing. +// A path in both reads as refused to anyone scanning the reasons and is in +// fact reachable, which is the worst of the two states to be in. +func TestNothingIsBothAdmittedAndHeldBack(t *testing.T) { + for _, tool := range Allowlist { + for key, path := range tool.Catalog { + spec := specName(BackendCubeCOS, path) + if reason, held := cubeCOSReadsHeldBack[spec]; held { + t.Errorf("%s reads %q (%s) but it is also held back as %q; one of the two is wrong", + tool.Name, key, path, reason) + } + } + } +} + +// TestEveryHeldBackReadIsOneTheAPIStillServes keeps the held-back list from +// outliving the API, the same honesty the exclusion list gets: a reason +// written about a path that no longer exists is a wrong statement, and it +// makes the coverage test above pass for the wrong reason. +func TestEveryHeldBackReadIsOneTheAPIStillServes(t *testing.T) { + gets := specOps(t, BackendCubeCOS)["GET"] + for path, reason := range cubeCOSReadsHeldBack { + if !gets[path] { + t.Errorf("cubeCOSReadsHeldBack lists %s (%q), which cube-cos-api no longer serves by GET; delete the entry", + path, reason) + } + } +} + // 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 diff --git a/internal/toolplane/http_test.go b/internal/toolplane/http_test.go index 1b368dd..45dad6e 100644 --- a/internal/toolplane/http_test.go +++ b/internal/toolplane/http_test.go @@ -177,17 +177,23 @@ func TestAGetTemplateMustDeclareItsModelPlaceholders(t *testing.T) { } } -func TestTheShippedGetToolsRegister(t *testing.T) { - // The three overview reads ship in the allowlist and must be well-formed. - got := map[string]bool{} +func TestTheShippedCatalogueReadKeepsTheOverviewsItReplaced(t *testing.T) { + // The three hand-written overview reads — healths, nodes and events — + // became keys of one catalogue tool. Named here rather than left to the + // coverage test, because widening a surface must not quietly narrow it: + // this is the assertion that the replacement lost nothing. + var catalog map[string]string for _, tool := range Allowlist { - if tool.Get != "" { - got[tool.Name] = true + if len(tool.Catalog) > 0 { + catalog = tool.Catalog } } - for _, want := range []string{"cube_cos_healths", "cube_cos_nodes", "cube_cos_events"} { - if !got[want] { - t.Errorf("%s is not in the shipped allowlist", want) + if catalog == nil { + t.Fatal("no catalogue read ships in the allowlist") + } + for _, want := range []string{"healths", "nodes", "events"} { + if catalog[want] == "" { + t.Errorf("the catalogue no longer reads %q, which shipped as its own tool", want) } } } diff --git a/internal/toolplane/registry.go b/internal/toolplane/registry.go index e97d8fc..72979dd 100644 --- a/internal/toolplane/registry.go +++ b/internal/toolplane/registry.go @@ -373,6 +373,10 @@ func (r *Registry) Call(ctx context.Context, name string, args map[string]string return r.callGet(ctx, tool, args) } + if len(tool.Catalog) > 0 { + return r.callCatalogGet(ctx, tool, args) + } + if tool.Post != "" { return r.callPost(ctx, tool, args) } @@ -495,6 +499,55 @@ func (r *Registry) callGet(ctx context.Context, tool Tool, args map[string]strin return nil, fmt.Errorf("%w: %v", ErrBadArgument, err) } + return r.fetch(ctx, tool, path, args) +} + +// callCatalogGet dispatches a catalogue read: the caller names a key, the +// allowlist owns the path. +// +// The key is checked by map lookup, which is the finite-value-set rule in its +// strongest form — there is no grammar to get wrong and no value outside the +// set to reject, because a value outside the set simply is not a key. A miss +// is refused the same way an out-of-set parameter is, and audited, so a caller +// probing for paths writes a line per attempt in the customer's own log. +func (r *Registry) callCatalogGet(ctx context.Context, tool Tool, args map[string]string) ([]byte, error) { + refuse := func(err error) ([]byte, error) { + r.audit.RecordToolCall(ToolCall{ + Tool: tool.Name, Args: args, Allowed: false, + Reason: err.Error(), At: time.Now().UTC(), + }) + return nil, fmt.Errorf("%w: %v", ErrBadArgument, err) + } + for k := range args { + if k != catalogArg { + return refuse(fmt.Errorf("unexpected argument %q", k)) + } + } + key, given := args[catalogArg] + if !given { + return refuse(fmt.Errorf("missing argument %s", catalogArg)) + } + template, ok := tool.Catalog[key] + if !ok { + // Naming the argument but not the catalogue: which reads exist is in + // the tool's schema, where the model already saw it, and echoing the + // set on every miss would turn a refusal into a directory listing. + return refuse(fmt.Errorf("value for %s is not one this tool reads", catalogArg)) + } + if r.datacenter == "" { + return refuse(fmt.Errorf("cube-cos-api access is not configured on this agent")) + } + path := strings.ReplaceAll(template, dcPlaceholder, r.datacenter) + return r.fetch(ctx, tool, path, args) +} + +// fetch performs a resolved cube-cos-api GET, caps it, and audits the outcome. +// +// Both read forms end here so the cap, the truncation marker and the audit +// record are written once. A second copy of this would be a second place for +// the marker to go missing, and tool-0010 measures whether the model reports a +// cut — which it cannot do if the executor forgot to say there was one. +func (r *Registry) fetch(ctx context.Context, tool Tool, path string, args map[string]string) ([]byte, error) { max := tool.MaxOutputBytes if max <= 0 { max = defaultMaxOutputBytes diff --git a/internal/toolplane/testdata/cube-cos-api-paths.txt b/internal/toolplane/testdata/cube-cos-api-paths.txt index f9da1d2..5345b9f 100644 --- a/internal/toolplane/testdata/cube-cos-api-paths.txt +++ b/internal/toolplane/testdata/cube-cos-api-paths.txt @@ -1,109 +1,148 @@ -# cube-cos-api paths, extracted from its OpenAPI document. +# cube-cos-api operations, extracted from its OpenAPI document. +# +# One "METHOD PATH" line per operation, so a check can ask not only whether the +# API serves a path but whether it serves it the way an allowlist entry expects. +# A Get: entry naming a path the API only POSTs to is a bug this file can catch +# and a path-only list could not. # # 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 +# Regenerate: awk ' +# /^ "\/api\/v1\// { p=$0; gsub(/^ "/,"",p); gsub(/":$/,"",p); next } +# /^ (get|post|put|patch|delete|head|options):$/ && p != "" { +# m=$1; sub(/:$/,"",m); print toupper(m) " " p } +# ' docs.yaml | sort -u +DELETE /api/v1/datacenters/{dataCenter}/firmwares/{version} +DELETE /api/v1/datacenters/{dataCenter}/fixpacks/{version} +DELETE /api/v1/datacenters/{dataCenter}/integrations/storages/models/{driverName} +DELETE /api/v1/datacenters/{dataCenter}/integrations/storages/{storageName} +DELETE /api/v1/datacenters/{dataCenter}/nodes/{nodeName}/devices/{deviceName} +DELETE /api/v1/datacenters/{dataCenter}/nodes/{nodeName}/ipmi/disconnect +DELETE /api/v1/datacenters/{dataCenter}/nodes/{nodeName}/osds/{osdId} +DELETE /api/v1/datacenters/{dataCenter}/settings/email/recipients/{recipientEmail} +DELETE /api/v1/datacenters/{dataCenter}/settings/email/senders/{senderHost} +DELETE /api/v1/datacenters/{dataCenter}/settings/slack/channels/{channelName} +DELETE /api/v1/datacenters/{dataCenter}/supportFiles/{supportFileSet} +DELETE /api/v1/datacenters/{dataCenter}/triggers/{triggerName} +GET /api/v1/datacenters +GET /api/v1/datacenters/{dataCenter} +GET /api/v1/datacenters/{dataCenter}/events +GET /api/v1/datacenters/{dataCenter}/events/abstract +GET /api/v1/datacenters/{dataCenter}/events/filterConditions +GET /api/v1/datacenters/{dataCenter}/events/predefined +GET /api/v1/datacenters/{dataCenter}/events/rank +GET /api/v1/datacenters/{dataCenter}/firmwares +GET /api/v1/datacenters/{dataCenter}/firmwares/upgradeProgress +GET /api/v1/datacenters/{dataCenter}/firmwares/{version}/updatableNodes +GET /api/v1/datacenters/{dataCenter}/fixpacks +GET /api/v1/datacenters/{dataCenter}/fixpacks/updateProgress/{version} +GET /api/v1/datacenters/{dataCenter}/fixpacks/{version}/rollbackableNodes +GET /api/v1/datacenters/{dataCenter}/fixpacks/{version}/updatableNodes +GET /api/v1/datacenters/{dataCenter}/grafana/devices/{hostname}/gpuUtilization +GET /api/v1/datacenters/{dataCenter}/grafana/devices/{hostname}/gpuVram +GET /api/v1/datacenters/{dataCenter}/grafana/hosts/{hostname} +GET /api/v1/datacenters/{dataCenter}/grafana/instances/{instanceId} +GET /api/v1/datacenters/{dataCenter}/grafana/networkDevices +GET /api/v1/datacenters/{dataCenter}/grafana/networks +GET /api/v1/datacenters/{dataCenter}/grafana/storages +GET /api/v1/datacenters/{dataCenter}/grafana/topHosts +GET /api/v1/datacenters/{dataCenter}/grafana/topInstances +GET /api/v1/datacenters/{dataCenter}/healths +GET /api/v1/datacenters/{dataCenter}/healths/services/{serviceType} +GET /api/v1/datacenters/{dataCenter}/healths/services/{serviceType}/modules/{moduleType} +GET /api/v1/datacenters/{dataCenter}/images +GET /api/v1/datacenters/{dataCenter}/images.csv +GET /api/v1/datacenters/{dataCenter}/images/materials +GET /api/v1/datacenters/{dataCenter}/integrations/applications +GET /api/v1/datacenters/{dataCenter}/integrations/storages +GET /api/v1/datacenters/{dataCenter}/integrations/storages/models +GET /api/v1/datacenters/{dataCenter}/integrations/storages/{storageName} +GET /api/v1/datacenters/{dataCenter}/integrations/storages/vendors +GET /api/v1/datacenters/{dataCenter}/licenses +GET /api/v1/datacenters/{dataCenter}/licenses/attachments +GET /api/v1/datacenters/{dataCenter}/me +GET /api/v1/datacenters/{dataCenter}/metrics +GET /api/v1/datacenters/{dataCenter}/metrics/{metricType}/{viewType}/{entityType} +GET /api/v1/datacenters/{dataCenter}/metrics/{metricType}/{viewType}/{entityType}/{entityId or Name} +GET /api/v1/datacenters/{dataCenter}/nodes +GET /api/v1/datacenters/{dataCenter}/nodes/{nodeName} +GET /api/v1/datacenters/{dataCenter}/nodes/{nodeName}/devices +GET /api/v1/datacenters/{dataCenter}/nodes/{nodeName}/gpuCards +GET /api/v1/datacenters/{dataCenter}/nodes/{nodeName}/gpuCards/instances/{instanceId}/console +GET /api/v1/datacenters/{dataCenter}/notifications +GET /api/v1/datacenters/{dataCenter}/notifications/last +GET /api/v1/datacenters/{dataCenter}/opensearch/requests/{requestId} +GET /api/v1/datacenters/{dataCenter}/services +GET /api/v1/datacenters/{dataCenter}/settings +GET /api/v1/datacenters/{dataCenter}/settings/email/recipients +GET /api/v1/datacenters/{dataCenter}/settings/email/senders +GET /api/v1/datacenters/{dataCenter}/settings/slack/channels +GET /api/v1/datacenters/{dataCenter}/supportFiles +GET /api/v1/datacenters/{dataCenter}/supportFiles/hosts/{hostname} +GET /api/v1/datacenters/{dataCenter}/triggers +GET /api/v1/datacenters/{dataCenter}/triggers/materials +GET /api/v1/datacenters/{dataCenter}/triggers/{triggerName} +GET /api/v1/datacenters/{dataCenter}/tunings/parameters +GET /api/v1/datacenters/{dataCenter}/tunings/specs +GET /api/v1/datacenters/{dataCenter}/volumes +GET /api/v1/datacenters/{dataCenter}/volumes.csv +PATCH /api/v1/datacenters/{dataCenter}/firmwares +PATCH /api/v1/datacenters/{dataCenter}/firmwares/{version}/{nodeName} +PATCH /api/v1/datacenters/{dataCenter}/fixpacks +PATCH /api/v1/datacenters/{dataCenter}/healths +PATCH /api/v1/datacenters/{dataCenter}/healths/services/{serviceType}/modules/{moduleType} +PATCH /api/v1/datacenters/{dataCenter}/images/{imageId} +PATCH /api/v1/datacenters/{dataCenter}/integrations/storages/models/{driverName} +PATCH /api/v1/datacenters/{dataCenter}/integrations/storages/{storageName} +PATCH /api/v1/datacenters/{dataCenter}/integrations/storages/{storageName}/asDefault +PATCH /api/v1/datacenters/{dataCenter}/nodes/{nodeName}/devices/{deviceName} +PATCH /api/v1/datacenters/{dataCenter}/nodes/{nodeName}/osds/{osdId} +PATCH /api/v1/datacenters/{dataCenter}/settings/email/senders/{senderHost} +PATCH /api/v1/datacenters/{dataCenter}/triggers/{triggerName} +PATCH /api/v1/datacenters/{dataCenter}/triggers/{triggerName}/enable +PATCH /api/v1/datacenters/{dataCenter}/tunings/parameters/{parameterName} +PATCH /api/v1/datacenters/{dataCenter}/tunings/parameters/{parameterName}/enable +POST /api/v1/datacenters/{dataCenter}/firmwares +POST /api/v1/datacenters/{dataCenter}/firmwares/abort +POST /api/v1/datacenters/{dataCenter}/firmwares/continueAnyway/{nodeName} +POST /api/v1/datacenters/{dataCenter}/firmwares/md5sum +POST /api/v1/datacenters/{dataCenter}/firmwares/md5sum/verify +POST /api/v1/datacenters/{dataCenter}/fixpacks +POST /api/v1/datacenters/{dataCenter}/fixpacks/continueAnyway/{nodeName} +POST /api/v1/datacenters/{dataCenter}/fixpacks/md5sum +POST /api/v1/datacenters/{dataCenter}/fixpacks/md5sum/verify +POST /api/v1/datacenters/{dataCenter}/fixpacks/{version}/rollback +POST /api/v1/datacenters/{dataCenter}/images +POST /api/v1/datacenters/{dataCenter}/integrations/storages +POST /api/v1/datacenters/{dataCenter}/integrations/storages/models +POST /api/v1/datacenters/{dataCenter}/integrations/storages/{storageName}/verify +POST /api/v1/datacenters/{dataCenter}/licenses +POST /api/v1/datacenters/{dataCenter}/licenses/hosts/{hostname} +POST /api/v1/datacenters/{dataCenter}/licenses/verify +POST /api/v1/datacenters/{dataCenter}/nodes/{nodeName}/devices +POST /api/v1/datacenters/{dataCenter}/nodes/{nodeName}/ipmi +POST /api/v1/datacenters/{dataCenter}/nodes/{nodeName}/ipmi/{operation} +POST /api/v1/datacenters/{dataCenter}/nodes/{nodeName}/ipmi/verify +POST /api/v1/datacenters/{dataCenter}/nodes/{nodeName}/osds/{osdId}/restart +POST /api/v1/datacenters/{dataCenter}/nodes/{nodeName}/softReboot +POST /api/v1/datacenters/{dataCenter}/rollingReboot +POST /api/v1/datacenters/{dataCenter}/settings/email/recipients +POST /api/v1/datacenters/{dataCenter}/settings/email/recipients/{recipientEmail} +POST /api/v1/datacenters/{dataCenter}/settings/email/senders +POST /api/v1/datacenters/{dataCenter}/settings/email/senders/{senderHost} +POST /api/v1/datacenters/{dataCenter}/settings/slack/channels +POST /api/v1/datacenters/{dataCenter}/settings/slack/channels/{channelName} +POST /api/v1/datacenters/{dataCenter}/supportFiles +POST /api/v1/datacenters/{dataCenter}/tokens +POST /api/v1/datacenters/{dataCenter}/triggers +POST /api/v1/datacenters/{dataCenter}/triggers/materials/script/verify +POST /api/v1/datacenters/{dataCenter}/tunings/parameters/{parameterName}/reset +POST /api/v1/datacenters/{dataCenter}/volumes/images +POST /api/v1/logout +PUT /api/v1/datacenters/{dataCenter}/integrations/storages/models +PUT /api/v1/datacenters/{dataCenter}/nodes/{nodeName}/gpuCards/{gpuId} +PUT /api/v1/datacenters/{dataCenter}/settings/email/recipients/{recipientEmail} +PUT /api/v1/datacenters/{dataCenter}/settings/slack/channels/{channelName} +PUT /api/v1/datacenters/{dataCenter}/settings/titlePrefix diff --git a/pkg/toolcatalog/toolcatalog.go b/pkg/toolcatalog/toolcatalog.go index 3218dae..f9176fc 100644 --- a/pkg/toolcatalog/toolcatalog.go +++ b/pkg/toolcatalog/toolcatalog.go @@ -38,6 +38,16 @@ type Entry struct { // comparison that ignored the distinction would demand the two lists // match when they legitimately do not. Probe bool + + // Reads is the sorted set of catalogue keys this entry accepts, empty for + // a tool that is not a catalogue read. + // + // Published for the same reason the names are: the SaaS has to tell the + // model which reads exist, and a list of keys typed out over there is a + // copy of this one. The keys remain advice on that side — the executor's + // map is the enforcement, and a stale SaaS must not veto a key a newer + // executor serves. + Reads []string } // Entries returns every tool the executor can serve, sorted by name. @@ -49,7 +59,7 @@ type Entry struct { func Entries() []Entry { out := make([]Entry, 0, len(toolplane.Allowlist)+len(toolplane.ProbeControls)) for _, t := range toolplane.Allowlist { - out = append(out, Entry{Name: t.Name, Impact: t.Impact.String()}) + out = append(out, Entry{Name: t.Name, Impact: t.Impact.String(), Reads: readsOf(t)}) } for _, t := range toolplane.ProbeControls { out = append(out, Entry{Name: t.Name, Impact: t.Impact.String(), Probe: true}) @@ -57,3 +67,18 @@ func Entries() []Entry { sort.Slice(out, func(i, j int) bool { return out[i].Name < out[j].Name }) return out } + +// readsOf returns a catalogue tool's keys, sorted. Sorted because a map's +// order is not one, and a published list whose order changes between calls +// makes a comparison on the other side flap for no reason. +func readsOf(t toolplane.Tool) []string { + if len(t.Catalog) == 0 { + return nil + } + keys := make([]string, 0, len(t.Catalog)) + for k := range t.Catalog { + keys = append(keys, k) + } + sort.Strings(keys) + return keys +}