tools: instances live in OpenStack, so that is where the create goes - #34
Merged
Merged
Conversation
create_instance shipped pointing at POST /api/v1/datacenters/{dc}/instances,
which cube-cos-api does not serve and never has: that API serves the appliance
— nodes, health, events, images, volumes — and VM lifecycle is not among its
resource families. The tool's level gate, approval statement, write ledger and
readable refusal were all correct and destination-independent; what was missing
was somewhere to send the request.
Instances are nova's. The tool now posts to nova's own /servers, confirmed
against nova's router as CubeCOS vendors it (core/nova/caracal_patch/api/
openstack/compute/routes.py.orig, Caracal, cubecos 5a0e4640).
Tools now name a Backend. The zero value is cube-cos-api, so every existing
tool keeps its destination without restating it and a new tool that forgets to
say lands on the read-only management API. It is a small enum rather than an
interface: there are two destinations today and a third is foreseen — k8s
clusters are Rancher's — and an abstraction designed against one real case and
two imagined ones is worse than a second concrete case later. What it avoids is
baking "a write means nova" into anything shared.
The allowlist keeps a flat, canonical body and the backend translates. nova
wants the object nested under "server", with flavorRef and imageRef, and
networks as a list; putting that in the allowlist would mean teaching Body to
express nested JSON, which widens what this file can say for no gain. The
translation is testable without a nova to talk to.
There is no project field, and its absence is the control. A server is created
in whatever project the credential is scoped to, so there is nothing to name
and no way to name another tenant's. internal/openstack refuses outright if
Keystone reports a scope other than the one the operator's configuration names,
rather than creating somewhere nobody chose.
Credentials are a per-cluster OpenStack application credential, read from
openstack-credential.json beside the enrollment identity and the action level —
same directory, same custody, no new distribution mechanism. An application
credential is scoped to one project, carries only the roles the operator
granted, and is revocable on its own. This is the pattern the next backend will
follow, which is part of why it was chosen: Rancher will need its own and the
same argument applies. The file is refused if it is group- or world-readable,
because unlike the action level it holds a secret. Absent is not an error — it
is the ordinary state of every cluster that has not opted in — and the refusal
names the missing configuration and says explicitly that it is not the action
level refusing, because those are different problems with different owners.
The Keystone endpoint comes from configuration; everything else comes from the
catalog, which is why "/servers" is the whole of what the allowlist writes down.
Tokens are cached and renewed five minutes before expiry, and neither the token
nor the credential reaches a log, an error, an audit row or the model — a
Keystone refusal deliberately does not echo the request, because the request
holds the secret.
Idempotency is unchanged and the residue is unchanged: the ledger keys on the
fully resolved path and body, a failed or timed-out write stays retryable, and
a server that ignores a retry can still duplicate. nova offers nothing better
here — it permits duplicate server names and has no create-once semantics on
this path — so the header is sent in case a proxy honours it and the ledger is
what actually holds.
Conformance is per backend now. nova's paths are vendored from its router the
same way cube-cos-api's are vendored from its OpenAPI document, with the same
refusal to skip when the input is missing, since a test that skips reports green
for exactly the case it guards. notInTheSpec is empty: create_instance was its
only entry, and giving the tool a destination paid the debt rather than excusing
it. The body shape has no machine-readable source in this environment and is
therefore pinned by tests rather than by conformance — a weaker guarantee,
stated rather than implied.
TestACallerCannotChooseWhatTheProfileDecides asserted only ErrBadArgument, which
both the executor-context guard and the unknown-argument fallback return — so it
passed with the guard deleted. It now pins the reason, and covers all four
profile fields rather than one.
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
create_instancenow posts to nova's/serversinstead of a cube-cos-api path that does not exist. Tools name aBackend; the executor holds one write client per backend; OpenStack is reached with a per-cluster application credential through the Keystone catalog.Why
#33 established that
POST /api/v1/datacenters/{dc}/instancesnames nothing: cube-cos-api serves the appliance — nodes, health, events, images, volumes — across 102 paths, and VM lifecycle is not among its resource families. The tool's level gate, approval statement, write ledger and readable refusal were all correct and destination-independent. What was missing was somewhere to send the request.Instances are nova's, so that is where it goes.
Reviewer notes
Ground truth, and what is citable.
POST /serversis confirmed from nova's own router as CubeCOS vendors it —core/nova/caracal_patch/api/openstack/compute/routes.py.orig, cubecos5a0e4640. Caracal is confirmed from the tree, not assumed: everycore/*/*.mknames it and the patch directory iscaracal_patch. The request body schema is not vendored anywhere in this environment — nova'sapi/openstack/compute/schemas/is not in the tree, since only patched files are vendored — so the body shape is pinned by tests rather than by conformance. That is a weaker guarantee than the path has, and it is stated rather than implied.Backend is an enum, not an interface. Two destinations today, a third foreseen (k8s clusters are Rancher's). An abstraction designed against one real case and two imagined ones is worse than a second concrete case later. The field is named for the role so nothing bakes in "a write means nova", and the zero value is cube-cos-api so no existing tool restates its destination.
The allowlist stays inexpressive. It can say: a literal path, a flat body of declared fields, finite value sets, and one shaped free parameter. It still cannot say a wildcard, a nested object, or a host. nova wants the body nested under
serverwithflavorRef/imageRefandnetworksas a list — teachingBodyto express that would widen what this file can say for no gain, so the backend translates and the translation is testable without a nova.There is no project field, and that is the control. A server is created in whatever project the credential is scoped to. Nothing to name, so nothing to name wrongly.
internal/openstackrefuses outright if Keystone reports a scope other than the one the operator's configuration names, rather than creating somewhere nobody chose.Credentials — the decision that generalises, and the one worth ratifying. A per-cluster OpenStack application credential in
openstack-credential.json, beside the enrollment identity and the action level: same directory, same custody, no new distribution mechanism. Scoped to one project at creation, carrying only the roles the operator granted, revocable on its own, issued by the operator in their own Keystone — nothing about it depends on Bigstack, which is what makes it work air-gapped. This sets the pattern for Rancher and everything after, which is part of why it was chosen rather than a service user.The file is refused if group- or world-readable, unlike the action level, which is policy anyone may inspect. Absent is not an error: it is the ordinary state of every cluster that has not opted in.
A missing credential does not look like a level refusal. They are different problems with different owners — one is policy the operator chose, the other is configuration they have not finished — and the refusal says so in as many words. A level refusal still comes first, so a cluster at
observeis never told to go and configure a credential for a tool its level does not serve.Keystone and secrets. The auth URL is configuration; the compute endpoint comes from the catalog, which is why
/serversis the whole of what the allowlist writes down. Tokens are cached and renewed five minutes before expiry. Neither the token nor the secret reaches a log, an error, an audit row or the model:Credential.String/GoStringredact so a later%+vcannot leak, a Keystone refusal deliberately does not echo the request, and a malformed credential file is reported without quoting its contents.Idempotency is unchanged, including its residue. The ledger keys on the fully resolved path and body; a failed or timed-out write stays retryable. nova offers nothing better — it permits duplicate server names and has no create-once semantics here — so the
Idempotency-Keyheader is sent in case a proxy honours it, and the ledger is what actually holds. Stated rather than implied to be stronger.Name shape errs strict, deliberately.
ShapeDNSLabelis narrower than nova's server-name rule. A shape looser than the API is a failed write the model cannot interpret; a stricter one is a usability limit an operator can see and complain about. Kept.Conformance is per backend. nova's paths are vendored from its router as cube-cos-api's are from its OpenAPI document, with the same truncation floor and the same refusal to skip when input is missing.
notInTheSpecis now empty —create_instancewas its only entry, and giving the tool a destination paid the debt rather than excusing it. The two honesty guards iterate that map, so an empty map exercises nothing; they earn their keep when someone next adds an entry, and the comment says so.A weak test found by breaking it.
TestACallerCannotChooseWhatTheProfileDecidesasserted onlyErrBadArgument— which both the executor-context guard and the unknown-argument fallback return — so it passed with the guard deleted. It now pins the reason and covers all four profile fields. That is the third time this exercise has found untested behaviour rather than confirming a belief.Breaks proved, each watched to fail a named test and then restored: credential refusal removed (
TestACreateWithoutACredentialIsRefusedAndSaysSoDistinctly); executor-context guard disabled (TestACallerCannotChooseWhatTheProfileDecides, four subcases, after strengthening); ledger suppression disabled (TestARetriedWriteDoesNotCreateASecondInstance).Wired but inert.
run.goloads the credential when one exists and refuses cleanly when it does not. Nothing can act until an operator writes that file and raises the cluster's level, so this ships without changing any deployment's behaviour.No SaaS PR.
pkg/toolcatalogis doing its job:cube-ai-advisorholds no copy of the path — only the tool's name, in two eval cases, which is unchanged.Docs
Handbook: the slice-3 as-built and #33's correction both describe a destination that has now changed.