diff --git a/cmd/compute-mcp/docs_test.go b/cmd/compute-mcp/docs_test.go index 3cb6ddf5..b2bd177a 100644 --- a/cmd/compute-mcp/docs_test.go +++ b/cmd/compute-mcp/docs_test.go @@ -113,6 +113,7 @@ func TestSkillsMatchDocumentedSet(t *testing.T) { "/runbooks/referenced-data-triage.md": true, "/runbooks/placement-triage.md": true, "/runbooks/stalled-transient.md": true, + "/runbooks/workload-create.md": true, } got := docs.paths() if len(got) != len(want) { diff --git a/docs/agent/README.md b/docs/agent/README.md index 47399df2..a2b97401 100644 --- a/docs/agent/README.md +++ b/docs/agent/README.md @@ -21,9 +21,25 @@ assistant owns the document schema that carries it. ## Status Landed here: the reason catalog, the diagnosis walk, the knowledge and skills -above, and `cmd/compute-mcp` — the MCP server that publishes the five read-only -tools (`workloads_list`, `workloads_get`, `instances_list`, `workload_diagnose`, -`reason_explain`) over Streamable HTTP. +above, and `cmd/compute-mcp` — the MCP server that publishes compute's tools +over Streamable HTTP: + +| Tools | Names | +|---|---| +| Diagnosis, read-only | `compute_workloads_list`, `compute_workloads_get`, `compute_instances_list`, `compute_workload_diagnose`, `compute_reason_explain` | +| Creation, writes nothing | `compute_instance_types_list` (the sizes a workload may ask for), `compute_workload_render` (inputs to a Workload manifest, pure) | + +Every tool is prefixed `compute_`, so the assistant can compose tools from +several services in one conversation without names colliding; the capability +document must register the prefixed names. + +Compute publishes no mutating tool. Everything else creating a workload needs +comes from the assistant's base tools, which it gives every project turn and +which act as the caller: `locations_list` with service `compute` for where +compute is offered, `quota_get` for what is left, `resources_list` for +Networks and RuntimeClasses, and `resources_validate`, `resources_plan` and +`resources_apply` for the change itself. The plan token and the confirmation +step live there, once, for every service. ## HTTP surface @@ -59,9 +75,6 @@ Three properties of the server are worth knowing before you deploy it: prompt injection away from another tenant's workloads. The caller sets `X-Datum-Project` after authenticating the user. -Compute publishes no mutating tool. Allow-list enforcement is the gateway's job, -but a tool that does not exist cannot be called through any path. - ## Why the knowledge leads with "how to read conditions" Compute's top-level condition reasons are deliberately **pointers, not causes**. @@ -90,6 +103,7 @@ orientation and classification; the procedures live here and nowhere else. | `referenced-data-triage` | Missing, unauthorized, or oversized ConfigMaps/Secrets | | `placement-triage` | `NoMatchingLocation`, `AmbiguousServingLocation`, `LocationMismatch` | | `stalled-transient` | A transient reason that has outlived its expected window | +| `workload-create` | Deploying something new: prerequisites, the choices that are final at create, and render → plan → show → confirm → apply | A skill never grants privileges. It can only direct the model toward tools that are independently on the enforced allow-list, which is why these go through the diff --git a/docs/agent/llms-full.txt b/docs/agent/llms-full.txt index 24729af3..c030db41 100644 --- a/docs/agent/llms-full.txt +++ b/docs/agent/llms-full.txt @@ -78,8 +78,8 @@ Pointer reasons you must read *through*, never report as the answer: PendingQuota -> look at Instance.QuotaGranted SchedulingGatesPresent -> something else is holding it; find what -The `workload_diagnose` tool does this walk for you and returns the leaf cause. -Prefer it over assembling the tree by hand. +The `compute_workload_diagnose` tool does this walk for you and returns the +leaf cause. Prefer it over assembling the tree by hand. ## Kinds of cause @@ -121,8 +121,8 @@ reason, and only the elapsed time tells them apart. Every tool result that names a cause carries how long the state has held: `rootCauseFor` / `rootCauseSince` in the fleet view, `inStateFor` / `lastTransitionTime` on a diagnosis. Every transient reason carries the window -it should clear inside (`expectedWithin` from `reason_explain`). Past that -window the actionability comes back as `stalled`. +it should clear inside (`expectedWithin` from `compute_reason_explain`). Past +that window the actionability comes back as `stalled`. `stalled` is not the same as a platform fault. Nothing reported a cause; the classification has merely been contradicted by the clock. Report the duration @@ -182,21 +182,70 @@ Instance runtime infrastructure that runs the container, not by compute. Load `instance-not-ready`. +## Creating a workload + +A Workload is three things: a name, one instance template, and one or more +placements. The template says what runs — either containers in a sandbox, or a +virtual machine booting a full operating system — at one instance type, on one +network interface, with whatever volumes the containers or the machine attach. +Each placement says where in one of two ways — a list of location names, or a +selector over the topology those locations declare (city code, region), which +places at every location matching it and picks up new ones as they appear — and +a replica count. Where it runs and how many of it there are live in the +placement, not in the template. + +Some of that is settled for good at create. The workload's name, the runtime +kind, and the network interface — its name, the address families it carries, +any extra addresses such as a public IPv4 one, and what becomes of those +addresses afterwards — cannot be changed later. Getting one of them wrong means +a new workload rather than an edit, which is why the create procedure asks +about them explicitly rather than defaulting them quietly. + +Two things are outside your reach entirely. You cannot build or push a +container image: it must already exist in a registry, fully qualified, and be +built for the runtime Datum runs it on — `datumctl compute build` is the step +the customer runs. And you cannot enable Compute for a project or grant it +quota; both are Datum's to give. + +Writing is a sequence, not a call. `compute_workload_render` turns inputs into +a Workload manifest and touches nothing. The rest is the platform's own change +path, the same for every service: `resources_plan` validates the manifests, +settles create versus update, orders a Network ahead of the Workload that names +it, and returns them with a plan token. `resources_apply` takes those manifests +and that token and nothing else, so the only thing that can be created is what +you showed the customer and they agreed to. + +Load `workload-create` before any of this. The prerequisites, the inputs, the +rejections that are worth pre-empting, and what to do at each failure are all +there, and this section deliberately does not restate them. + ## What the tools give you - workloads_list fleet view, worst first, with root-cause reason, - actionability, and how long that cause has held - workloads_get raw condition tree for one workload - instances_list per-instance conditions, to see failure distribution - workload_diagnose the walk, the leaf cause with its age, and next steps - reason_explain any reason, explained, classified, and — when transient — - the window it should clear inside + compute_workloads_list fleet view, worst first, with root-cause reason, + actionability, and how long that cause has held + compute_workloads_get raw condition tree for one workload + compute_instances_list per-instance conditions, to see failure + distribution + compute_workload_diagnose the walk, the leaf cause with its age, and next + steps + compute_reason_explain any reason, explained, classified, and — when + transient — the window it should clear inside + compute_instance_types_list the instance types a workload may ask for + compute_workload_render inputs to a full Workload manifest; writes + nothing, reads nothing + +The platform's base tools fill in the rest of a create. Call locations_list +with service "compute" for the locations this project may place a workload in — +a location absent from it is one compute is not offered in, and placements name +these verbatim or select them by their topology. quota_get says how much +compute is left, resources_list reads Networks and RuntimeClasses, and +resources_plan and resources_apply make the change. Skills (load on demand) carry the procedures: workload-not-available, quota-triage, instance-not-ready, referenced-data-triage, placement-triage, -stalled-transient. This document is orientation; it deliberately does not -restate what a skill covers, so reach for the skill rather than answering a -triage question from what is in the prompt. +stalled-transient, workload-create. This document is orientation; it +deliberately does not restate what a skill covers, so reach for the skill +rather than answering a triage or creation question from what is in the prompt. ## Telling compute what it could not do diff --git a/docs/agent/skills/instance-not-ready.md b/docs/agent/skills/instance-not-ready.md index c57d8e09..ded82177 100644 --- a/docs/agent/skills/instance-not-ready.md +++ b/docs/agent/skills/instance-not-ready.md @@ -46,7 +46,7 @@ it — the logs are there either way. unpacked. Say to wait. Only if it persists well beyond a few minutes should you treat it as Datum's problem. -6. **Check whether every instance fails the same way.** `instances_list` for the +6. **Check whether every instance fails the same way.** `compute_instances_list` for the workload: all of them failing the same way points at the workload or the image; one failing among healthy siblings points at one machine or one location, which is Datum's. @@ -81,7 +81,7 @@ When that happens: "capability": "container log retrieval for a crashing instance", "kind": "UnactionableGuidance", "evidence": { - "tool": "instances_list", + "tool": "compute_instances_list", "observed": "InstanceCrashing; remediation points at the logs", "contradictedBy": "log retrieval fails outright on this instance: the port answers plain HTTP where encrypted diff --git a/docs/agent/skills/placement-triage.md b/docs/agent/skills/placement-triage.md index 1682d75b..c03d1acc 100644 --- a/docs/agent/skills/placement-triage.md +++ b/docs/agent/skills/placement-triage.md @@ -22,17 +22,17 @@ end. - `LocationMismatch` — the workload asked for one location and was sent to another. It was routed to the wrong place. -2. **Confirm the scope.** `workloads_list` shows whether other workloads in the - same placement are also failing. Several failing in one place is a - location-wide problem and is worth reporting as such; a single one may be a - leftover deployment. +2. **Confirm the scope.** `compute_workloads_list` shows whether other + workloads in the same placement are also failing. Several failing in one + place is a location-wide problem and is worth reporting as such; a single one + may be a leftover deployment. 3. **Check whether other placements are serving.** A workload with several placements may be fully available elsewhere. Say so — the customer's service may be up even though this part is broken. 4. **Escalate with specifics.** Datum needs: the WorkloadDeployment name, its - `location`, and the status message. Pull these from `workloads_get`. + `location`, and the status message. Pull these from `compute_workloads_get`. ## Reporting diff --git a/docs/agent/skills/quota-triage.md b/docs/agent/skills/quota-triage.md index ffdc9022..9abe3ff7 100644 --- a/docs/agent/skills/quota-triage.md +++ b/docs/agent/skills/quota-triage.md @@ -11,8 +11,8 @@ service that evaluates it, and not the request compute files against it. ## Procedure 1. **Get the real reason.** `QuotaNotGranted` on the Workload or - WorkloadDeployment is a pointer. Call `workload_diagnose`, or read the - Instance's `QuotaGranted` condition via `instances_list`. Never report + WorkloadDeployment is a pointer. Call `compute_workload_diagnose`, or read the + Instance's `QuotaGranted` condition via `compute_instances_list`. Never report `QuotaNotGranted` as the cause. 2. **Separate the four cases.** They look alike and lead to opposite advice: @@ -34,13 +34,14 @@ service that evaluates it, and not the request compute files against it. 3. **For `QuotaExceeded`, quantify it.** The status message carries the amount requested and the amount left. Quote both. Then give the customer the three real options: fewer replicas, less CPU or memory per instance, or ask Datum - to raise the project's quota. + to raise the project's quota. If the message leaves out what is left, + `quota_get` has it. 4. **For `PendingEvaluation`**, check how long. Minutes is normal. If it stays there, the checking service itself is stuck — treat it as `QuotaBackendUnavailable` and hand it to Datum. -5. **Check the split.** `instances_list` shows how many instances were cleared +5. **Check the split.** `compute_instances_list` shows how many instances were cleared and how many were not. Partial is the common case: the workload is serving at reduced capacity, which is worth saying explicitly. @@ -52,19 +53,22 @@ burn time trying. ## When the numbers are not there -Step 3 rests entirely on the status message. Nothing else in these tools carries -the project's compute quota, how much of it is in use, or how much is left. So -when a `QuotaExceeded` message arrives without figures — or carries what was -requested but not what remains — you cannot tell the customer how much smaller -to go, and "ask for less" without a number is not something they can act on. +When a `QuotaExceeded` message arrives without figures — or carries what was +requested but not what remains — call `quota_get` with service +`compute.datumapis.com`. It reports the project's compute quota per resource +type: the limit, how much is in use, and how much is left. That is the number +the customer needs to know how much smaller to go. -Say which half you have and which is missing, then file `InsufficientDetail` -against the tool you read it from, quoting the message you were given: +If `quota_get` cannot answer either, you cannot tell the customer how much +smaller to go, and "ask for less" without a number is not something they can act +on. Say which half you have and which is missing, then file +`InsufficientDetail` against the tool you read it from, quoting the message you +were given: "capability": "how much of the project's compute quota is left", "kind": "InsufficientDetail", "evidence": { - "tool": "instances_list", + "tool": "compute_instances_list", "observed": "QuotaGranted=False, QuotaExceeded, \"quota exceeded\"", "contradictedBy": "no requested or remaining amount in the response" } diff --git a/docs/agent/skills/stalled-transient.md b/docs/agent/skills/stalled-transient.md index 464fe94b..0285ab11 100644 --- a/docs/agent/skills/stalled-transient.md +++ b/docs/agent/skills/stalled-transient.md @@ -37,7 +37,7 @@ and neither is licence to rule the customer's own workload out. ## Procedure -1. **Quantify it, and use the larger number.** `workload_diagnose` gives two +1. **Quantify it, and use the larger number.** `compute_workload_diagnose` gives two ages on the root cause and they answer different questions: - `inStateFor` — how long the *status* has said this. @@ -54,7 +54,7 @@ and neither is licence to rule the customer's own workload out. an object broken for nine days means something is rewriting the status without ever finishing. Say so. - Then call `reason_explain` for `expectedWithin` — how long this step should + Then call `compute_reason_explain` for `expectedWithin` — how long this step should take. "Nine days, against thirty minutes" is the whole finding. Two things the tools will not give you, on purpose. An age is omitted rather @@ -76,13 +76,13 @@ and neither is licence to rule the customer's own workload out. something is working on this and never saying how it turned out. It does **not** name a culprit — see step 5. -3. **Check whether it is one object or all of them.** `instances_list` for the +3. **Check whether it is one object or all of them.** `compute_instances_list` for the workload. Every instance stuck the same way points at the place they all run; one stuck among healthy siblings points at that object. Say which — it decides who Datum wakes up. 4. **Look underneath before escalating.** Read `contributingConditions` from - `workload_diagnose`. A stalled pointer reason (`InstancesProvisioning`, + `compute_workload_diagnose`. A stalled pointer reason (`InstancesProvisioning`, `PendingQuota`, `SchedulingGatesPresent`) often has a real cause below it that arrived after the stall began. If one is there, that is the answer — follow its skill instead. @@ -166,7 +166,7 @@ copied out of the tool result: "capability": "duration-aware classification of transient reasons", "kind": "MisleadingOutput", "evidence": { - "tool": "workload_diagnose", + "tool": "compute_workload_diagnose", "observed": "actionability: transient, remediation \"Wait.\"", "contradictedBy": "failingFor: 9d, inStateFor: 9h30m, expectedWithin: 30m" } diff --git a/docs/agent/skills/workload-create.md b/docs/agent/skills/workload-create.md new file mode 100644 index 00000000..9bf11ae9 --- /dev/null +++ b/docs/agent/skills/workload-create.md @@ -0,0 +1,291 @@ +# Skill: create a workload + +Use when someone asks to deploy, run, or create something on Datum — a new +Workload, or a change to one that does not exist yet — and whenever you are +about to call `compute_workload_render`, or `resources_plan` or +`resources_apply` with a Workload in the manifests. + +## The one thing to know + +**You never write a workload directly. You render it, plan it, show it, and +apply only what the user agreed to.** `resources_apply` takes the manifests +`resources_plan` returned and that plan's token, and nothing else. Change a +manifest by one character and the token stops matching, so what gets created is +exactly what was shown and agreed to, or nothing at all. + +Two things you cannot do, however the request is phrased: + +- **You cannot build or push an image.** The image has to exist in a registry + before any of this starts. +- **You cannot turn Compute on for a project, and you cannot grant it quota.** + Both are Datum's to grant. Say so and name the step the user takes. + +The project is fixed by the request that reached you. There is no tool argument +for it, so you cannot create a workload in a project other than the one the +conversation is already scoped to. If the user names a different project, say +that this conversation only reaches the current one. + +## 1. Check the prerequisites before gathering anything + +Four things have to be true. Each has a read-only tool, and each failure has a +different answer: + +| Check | Tool | If it fails | +|---|---|---| +| Compute is offered to the project | `locations_list` with service `compute` | Nothing can be placed. Datum's to enable — the user runs `datumctl compute access request`, and approval is a manual step on Datum's side. | +| Somewhere to run it | `locations_list` with service `compute` | The location names it returns are the only ones a placement may name. A location missing from the list is one compute is not offered in. An empty list means nothing is available to this project yet; that is Datum's, not something the user can add. | +| A network | `resources_list` for kind `Network` in `networking.datumapis.com/v1alpha` | `default` by convention. If the one the workload names is missing, add a Network manifest of that name to the same `resources_plan` call — the plan orders it ahead of the Workload. Say so when you show the plan, because it is a second object being created. | +| Quota | `quota_get` with service `compute.datumapis.com` | Quota is granted by Datum and cannot be self-served. A project with none can still create a workload; its instances then sit at `QuotaGranted=False` with `QuotaNoBudget` and never start. | + +Do the quota arithmetic before you apply, not after. Replicas times the instance +type's size from `compute_instance_types_list`, against what `quota_get` says is +left, tells you whether this will start. If it will not, say so *before* asking +for confirmation — a workload that creates cleanly and then sits at +`QuotaExceeded` looks like a success and is not. Load `quota-triage` for the +difference between being over quota and having none. + +## 2. Container or virtual machine + +There are two runtimes and a workload picks exactly one. This is not adjustable +later — switching means a different workload. + +**Container (a sandbox).** The common case. One or more containers, each with a +fully qualified image. Choose this unless the user needs a whole operating +system. + +**Virtual machine.** A full OS booted from a disk image. Choose this only if the +user asks for one, or needs to log into the machine. It carries the extra +requirements in the trap list below. + +### The image is a prerequisite, not an input you can produce + +The image must: + +- **already exist in a registry** the platform can reach. You cannot build one. +- **be fully qualified** — `docker.io/netdata/netdata:latest`, not `netdata`. + A bare name is the most common cause of `ImageUnavailable` afterwards. +- **be built for the runtime Datum runs it on.** An image that runs on a laptop + can still fail here. The user builds it with `datumctl compute build`, which + checks for the known incompatibilities and can fix them. + +If the user has no image yet, stop and say that: the build is theirs to run, and +everything below waits on it. Do not render a manifest around an image name +nobody has pushed. + +## 3. Gather the inputs + +Ask for what is missing rather than inventing it. `compute_workload_render` +takes: + +- **name** — a DNS label (lowercase letters, digits and `-`). It is the object's + name and cannot be changed later. +- **image** — fully qualified, per above. +- **placements** — where the instances run, and how many. A placement says + where in exactly one of two ways: + - **`locations`** — location names, taken verbatim from `locations_list`. Use + this when the user named specific places. A name that is not in that list + can never be satisfied, so never invent one and never pass a city code here. + - **`locationSelector`** — a selector over the topology `locations_list` + reports for each location, such as `topology.datum.net/city-code: DFW`. This + is how you say "every location in Dallas" or "every location in a region" + without naming them, and it picks up locations added later on its own. + + Group locations that scale together into one placement. +- **replicas** — `minReplicas` must be at least 1. There is no scaling from + zero, and the ceiling is 1000. +- **instance type** — from `compute_instance_types_list`. Leave it unset to take + the default. +- **runtime class** — only if the user named an execution tier. The choices are + the RuntimeClass objects `resources_list` returns for + `compute.datumapis.com/v1alpha`. Leave it unset otherwise; the platform picks + its default, and the tier cannot be changed later. +- **network** — the name of a Network from `resources_list`, or leave it unset + for `default`. +- **port** — optional, and named. A port is how anything reaches the workload; + ask whether it serves traffic rather than guessing. +- **environment variables** — literal values, or drawn from a ConfigMap or a + Secret. +- **ConfigMap and Secret references** — mounted as volumes, or read as + environment variables. They must already exist in the project, and the user + must be able to read them, or create is rejected. +- **a public IPv4 address** — only if the workload has to be reachable from the + internet on IPv4. Ask; do not add one by default, and do not leave it out of a + workload that clearly needs one, because it cannot be added afterwards. + +## 4. The traps + +These are the ones that cost a round trip. Check the rendered manifest against +this list before you plan. + +1. **One instance type.** `datumcloud/d1-standard-2` is the only one accepted + today. `compute_instance_types_list` is the check; anything else is rejected + outright. + +2. **Per-container CPU and memory are not accepted.** A `resources` block on a + container is rejected, and so are adjustments to the instance type's own + requests. The size of an instance comes from the instance type and nothing + else. If the user wants a different size, that is a request to Datum. + +3. **ConfigMap volumes use `name`; Secret volumes use `secretName`.** The two + spellings sit next to each other in the same list and are not + interchangeable. Getting it wrong reads as a missing required field. + +4. **Every volume must be attached.** A volume that is declared and never + attached to a container or to the virtual machine is rejected — the create + fails on the volume, not on the attachment. + +5. **The network interface is settled at create.** Its name, the address + families it carries, any extra addresses (a public IPv4 among them), and what + becomes of those addresses when the instance goes away are all immutable. An + instance gets one interface. If any of this turns out to be wrong later, the + fix is a new workload, so ask now: + - IPv6 only is the default. If the workload has to answer on IPv4, that has + to be asked for at create. + - A published address — one in DNS, or allowed through someone's firewall — + wants a reclaim policy that keeps it, and that choice is also final. + +6. **Virtual machines need two extra things.** SSH keys on the template's + metadata, under the annotation `compute.datumapis.com/ssh-keys`, one + `username:key` line per key — a create without them is rejected. And the + first volume attached must be a bootable disk populated by an Ubuntu image. + First, not merely present. + +7. **ConfigMaps and Secrets have size limits**: 256 KiB per object, and 1 MiB + for everything one workload references put together. Over either and the + workload reports `SourceTooLarge` rather than failing at create. + +8. **A port is not a public URL.** `ports` plus the ingress rule the render + emits makes the port reachable on the instance's own address — that is the + whole of what this path does. A managed public HTTPS URL in front of an HTTP + workload is published separately, and today the only way to get one is + `datumctl compute deploy --http-port`, which these tools cannot do. Say that + plainly when the user asks for a URL: they will get an address and a port, + not a hostname, unless they run that command themselves. + +9. **Editing a ConfigMap does not restart anything.** The new contents reach the + machines, but a process that read the file at startup goes on running with + what it read. Say this whenever a config change is the point of the + conversation — the user has to restart the workload themselves, and there is + no tool here that does it. + +## 5. The sequence + +Follow it in order. Each step exists because of a failure the next one cannot +catch. + +1. **`compute_workload_render`** — inputs in, a full Workload manifest out. It + writes nothing and reaches nothing. Read what came back rather than assuming + it matches what you asked for, and read its notes. + +2. **`resources_plan`** — pass the rendered manifest, and a Network manifest + ahead of it if step 1 found the network missing: + + apiVersion: networking.datumapis.com/v1alpha + kind: Network + metadata: + name: default + namespace: default + spec: + ipam: + mode: Auto + + The plan validates everything without creating it, settles create versus + update for each manifest, reports what would change, and returns the + manifests with a plan token. This is where the traps above surface as real + rejections, and where you learn whether a workload of this name already + exists. + +3. **Show the user the planned manifests and the diff.** Whole, not + summarised, and the plan's own manifests rather than your draft. Then say in + plain words what will be created, where, how many, and what it will cost + against their quota. If a Network is in the plan, say that: it is a second + object. + +4. **Get an explicit yes.** A question about the plan is not a yes. "Looks + right" is. If the user asks for any change, go back to step 1 — a token + minted for the old manifests is not valid for new ones, and must not be + applied because it was close. + +5. **`resources_apply`** with the plan's manifests, in the plan's order, and its + token. + +6. **`compute_workload_diagnose`** for the rollout. Creation succeeding means + the request was accepted, not that anything is running. Tell the user what to + expect: instances appear, then start, and the first pull of a large image + takes a while. If it is not serving, that is `workload-not-available`'s + procedure, not this one. + +## What to do when a step fails + +- **Render is missing something** — an input you did not gather. Ask for it by + name. Do not fill it in with a plausible default; a guessed port or location + is a workload that runs in the wrong place. + +- **The plan rejects it** — this is the server's own answer, in its own words, + and it names the exact field. Quote the field path verbatim and translate the + rule beside it: `spec.template.spec.volumes[1].name: volume must be attached + at least 1 time` is "the `config` volume is declared but never mounted". Fix + it, render again, plan again. A rejected plan has no token, so there is + nothing to apply. + +- **The plan says update when you expected create** — a workload of that name is + already there. Stop and say so. Ask whether the user meant to change the + existing one, and check the diff for anything immutable from trap 5 before + going on, because those rejections arrive at apply and not before. + +- **Apply refuses the token** — something changed after the plan. That refusal + is the mechanism working. Re-plan, show the new manifests, and ask again. + Never work around it. + +- **Apply succeeds and nothing starts** — hand it to `compute_workload_diagnose` + and follow the skill it names. Quota and image problems both look like this + and lead to opposite advice. + +## If the user has a shell + +You are usually working without one. When the user is at a terminal, the same +workload is one command, and these are theirs to run, not yours to assume: + + datumctl compute access request + datumctl compute build --push --output ghcr.io/acme/api:1.4.2 . + datumctl compute deploy api --image=ghcr.io/acme/api:1.4.2 --city=DFW --min=1 --port=8080 + datumctl compute deploy api --image=ghcr.io/acme/api:1.4.2 --city=DFW --http-port=8080 + +The last one is the only way to get a public HTTPS URL, per trap 8. Offer them +when a step above has no tool behind it — the access request and the URL have +none at all — and otherwise stay with the tools, which is the path that shows +the user the manifest before anything is created. + +## Reporting + +Say what will exist, where, and how many, in the user's own words first: "one +container running `ghcr.io/acme/api:1.4.2` in Dallas, two replicas, answering on +port 8080". Then the identifiers — the workload name, the image with its tag, +the location names — because those are what they need to check it themselves or +to escalate. + +After apply, say plainly that the workload was created and that it is not +running yet, and what you will look at next. A create reported as a deploy is +the same mistake as reporting a pointer reason: technically true, and it reads +as more than it is. + +## When to file a capability gap + +`report_capability_gap__compute-datumapis-com` is for cases where these tools +could not get a legitimate creation done: + +- A field the user needs that `compute_workload_render` has no input for, where + the API clearly supports it — `InsufficientDetail`, quoting the field and what + you tried. +- A plan rejection whose message does not name what to change, so the user + cannot act on it — `UnactionableGuidance`, quoting the message verbatim. + +Not gaps, however awkward the turn: + +- **No image.** Building one was never in scope here. +- **No quota, or Compute not enabled.** Those are grants, and the tools + reporting them accurately is the tools working. +- **A rejection that was right.** An unsupported instance type or an unattached + volume is the plan doing its job — that is the answer, and it saved a broken + workload. +- **The user declined to confirm.** Not applying is the correct outcome. diff --git a/docs/agent/skills/workload-not-available.md b/docs/agent/skills/workload-not-available.md index 284f7d0b..cebe8927 100644 --- a/docs/agent/skills/workload-not-available.md +++ b/docs/agent/skills/workload-not-available.md @@ -4,7 +4,7 @@ Use when someone asks why a Workload is not running, not available, or stuck. ## Procedure -1. **Diagnose before you read.** Call `workload_diagnose` with the workload +1. **Diagnose before you read.** Call `compute_workload_diagnose` with the workload name. It walks Workload -> WorkloadDeployment -> Instance and returns the leaf cause. Do not assemble the tree by hand first — the top-level reason is usually a pointer, not a cause. @@ -61,7 +61,7 @@ tried. `kind` may be left off — this is the default: "capability": "per-instance CPU and memory usage for a workload", "evidence": { - "tool": "workload_diagnose", + "tool": "compute_workload_diagnose", "observed": "instances.ready 3 of 3; no usage figures on any field" } Describe the need in your own words. Do not copy the customer's message into diff --git a/internal/agent/instancetypes.go b/internal/agent/instancetypes.go new file mode 100644 index 00000000..c70c641f --- /dev/null +++ b/internal/agent/instancetypes.go @@ -0,0 +1,86 @@ +// SPDX-License-Identifier: AGPL-3.0-only + +package agent + +import ( + "context" + + "github.com/modelcontextprotocol/go-sdk/mcp" + + "go.datum.net/compute/pkg/instancetype" +) + +// ToolInstanceTypesList lists the instance types a Workload may ask for. +// +// A model with no catalog invents a plausible size, and the API rejects it the +// moment it is submitted. The catalog is compiled into compute rather than +// published as a resource, so the assistant's generic resource tools cannot +// read it and compute has to. +const ToolInstanceTypesList = "compute_instance_types_list" + +// InstanceTypeView is one instance type a Workload may ask for. +type InstanceTypeView struct { + Name string `json:"name"` + // VCPU is how many virtual CPUs the type provides. Fractional, because the + // size is stored in thousandths and a future type need not be a whole one. + VCPU float64 `json:"vcpu"` + // MemoryMiB is the RAM the type provides, in mebibytes. + MemoryMiB int64 `json:"memoryMiB"` + // Default marks the type to use when the customer expressed no preference. + Default bool `json:"default"` +} + +// InstanceTypesListInput takes no arguments. +type InstanceTypesListInput struct{} + +// InstanceTypesListOutput is the catalog of instance types. +type InstanceTypesListOutput struct { + InstanceTypes []InstanceTypeView `json:"instanceTypes"` +} + +// registerInstanceTypesTool adds compute_instance_types_list to s. +func registerInstanceTypesTool(s *mcp.Server, deps DepsFor) { + mcp.AddTool(s, &mcp.Tool{ + Name: ToolInstanceTypesList, + Title: "List instance types", + Description: "List the instance types a Workload may ask for, with the vCPU and memory each one " + + "provides and which is the default. Only these names are accepted — a Workload naming any " + + "other is rejected the moment it is submitted, so never invent a size. Read-only.", + }, instanceTypesList(deps)) +} + +// instanceTypesList reads only the catalog, but still resolves deps for the +// same reason reasonExplain does: an unauthenticated caller must not be able to +// use it to probe the server. +func instanceTypesList(deps DepsFor) mcp.ToolHandlerFor[InstanceTypesListInput, InstanceTypesListOutput] { + return func( + ctx context.Context, _ *mcp.CallToolRequest, _ InstanceTypesListInput, + ) (*mcp.CallToolResult, InstanceTypesListOutput, error) { + if _, err := deps(ctx); err != nil { + return nil, InstanceTypesListOutput{}, err + } + return nil, InstanceTypesListOutput{InstanceTypes: InstanceTypes()}, nil + } +} + +// InstanceTypes returns the instance types a Workload may ask for, in offer +// order. The first is the default: the order the platform's catalog lists them +// in is the order to prefer them. +// +// Names and sizes both come from pkg/instancetype, the same table the instance +// controller claims quota against. Restating either here would let the tool +// offer a size the API bills differently. +func InstanceTypes() []InstanceTypeView { + names := instancetype.Names() + out := make([]InstanceTypeView, 0, len(names)) + for i, name := range names { + size, _ := instancetype.Lookup(name) + out = append(out, InstanceTypeView{ + Name: name, + VCPU: float64(size.CPUMillicores) / 1000, + MemoryMiB: size.MemoryMiB, + Default: i == 0, + }) + } + return out +} diff --git a/internal/agent/instancetypes_test.go b/internal/agent/instancetypes_test.go new file mode 100644 index 00000000..7813636d --- /dev/null +++ b/internal/agent/instancetypes_test.go @@ -0,0 +1,54 @@ +// SPDX-License-Identifier: AGPL-3.0-only + +package agent + +import ( + "context" + "errors" + "testing" +) + +func TestInstanceTypesListOffersOnlyWhatValidationAccepts(t *testing.T) { + deps := fixtureDeps(fixtureReader()) + + _, out, err := instanceTypesList(deps)(context.Background(), nil, InstanceTypesListInput{}) + if err != nil { + t.Fatalf("compute_instance_types_list: %v", err) + } + if len(out.InstanceTypes) == 0 { + t.Fatal("no instance types offered; a model with no catalog invents one") + } + + var defaults int + for _, it := range out.InstanceTypes { + if it.Default { + defaults++ + } + // A type with no size is worse than useless: it invites a replica count + // chosen against nothing. + if it.VCPU <= 0 || it.MemoryMiB <= 0 { + t.Errorf("%s = %g vCPU / %d MiB, want a real size", it.Name, it.VCPU, it.MemoryMiB) + } + } + if defaults != 1 { + t.Errorf("got %d default instance types, want exactly 1", defaults) + } + + // The one supported type today, with the sizing quota is accounted against. + first := out.InstanceTypes[0] + if first.Name != "datumcloud/d1-standard-2" || first.VCPU != 1 || first.MemoryMiB != 2048 { + t.Errorf("first type = %+v, want datumcloud/d1-standard-2 at 1 vCPU / 2048 MiB", first) + } +} + +// TestInstanceTypesListFailsWhenDepsAreUnavailable: the catalog alone could +// answer, but an unauthenticated caller is still turned away so the tool is not +// a probe. +func TestInstanceTypesListFailsWhenDepsAreUnavailable(t *testing.T) { + wantErr := errors.New("no credentials on this request") + denied := DepsFor(func(context.Context) (ToolDeps, error) { return ToolDeps{}, wantErr }) + + if _, _, err := instanceTypesList(denied)(context.Background(), nil, InstanceTypesListInput{}); !errors.Is(err, wantErr) { + t.Errorf("compute_instance_types_list error = %v, want the deps error to surface unchanged", err) + } +} diff --git a/internal/agent/render.go b/internal/agent/render.go new file mode 100644 index 00000000..75d11a7c --- /dev/null +++ b/internal/agent/render.go @@ -0,0 +1,329 @@ +// SPDX-License-Identifier: AGPL-3.0-only + +package agent + +import ( + "context" + "fmt" + + "github.com/modelcontextprotocol/go-sdk/mcp" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "go.datum.net/compute/internal/workloadspec" +) + +// ToolWorkloadRender turns a short description of a deployment into a complete +// Workload manifest. +// +// Compute publishes no tool that writes. The assistant's own base tools +// validate, plan and apply manifests of any kind as the caller, behind a plan +// token and an explicit confirmation. What they cannot know is how to write a +// Workload the admission webhook accepts, and a model guessing at that invents +// fields plausibly. Rendering is the compute-specific half, so it is the half +// compute publishes. +const ToolWorkloadRender = "compute_workload_render" + +// The assistant's base tools the render output points at. They belong to the +// assistant, not to this server, so nothing here can check they exist; the +// names are the assistant's published contract. +const ( + baseToolLocationsList = "locations_list" + baseToolResourcesList = "resources_list" + baseToolResourcesPlan = "resources_plan" +) + +// ---------------------------------------------------------------- I/O types + +// RenderPlacement is one group of locations scaled together. +type RenderPlacement struct { + Name string `json:"name,omitempty" jsonschema:"Placement name, a DNS label. Defaults to \"default\"."` + // Locations and LocationSelector are the two ways to say where a placement + // runs, and exactly one of them must be given. The schema says so rather + // than leaving a model to discover it from a rejection. + Locations []string `json:"locations,omitempty" jsonschema:"Location names this placement runs in, e.g. [\"us-south-dfw-1\"]. Take the names verbatim from locations_list with service \"compute\" — a name that is not in that list can never be satisfied. Set exactly one of locations or locationSelector."` + LocationSelector *RenderLocationSelector `json:"locationSelector,omitempty" jsonschema:"Place at every location whose topology matches, instead of naming them. Use this for \"every location in Dallas\" or \"every location in a region\": match on the topology keys locations_list reports, such as topology.datum.net/city-code. New locations matching it are picked up automatically. Set exactly one of locations or locationSelector."` + MinReplicas int32 `json:"minReplicas,omitempty" jsonschema:"Instances to run per placement. At least 1 — there is no scaling to zero — and at most 1000. Defaults to 1."` +} + +// RenderLocationSelector is a label selector over location topology, in the +// two forms the API accepts. An empty selector is refused rather than read as +// matching every location. +type RenderLocationSelector struct { + MatchLabels map[string]string `json:"matchLabels,omitempty" jsonschema:"Topology key/value pairs a location must carry, e.g. {\"topology.datum.net/city-code\": \"DFW\"}."` + MatchExpressions []RenderLocationSelectorReq `json:"matchExpressions,omitempty" jsonschema:"Set-based requirements over topology keys, for cases matchLabels cannot express, such as one of several cities."` +} + +// RenderLocationSelectorReq is one set-based requirement. +type RenderLocationSelectorReq struct { + Key string `json:"key" jsonschema:"Topology key, e.g. topology.datum.net/city-code."` + Operator string `json:"operator" jsonschema:"In, NotIn, Exists or DoesNotExist."` + Values []string `json:"values,omitempty" jsonschema:"Values for In and NotIn. Must be empty for Exists and DoesNotExist."` +} + +// RenderPort is a named port the workload serves. +type RenderPort struct { + Name string `json:"name" jsonschema:"Port name, e.g. \"http\". At most 15 characters, and must contain a letter."` + Port int32 `json:"port" jsonschema:"Port number, 1 to 65535."` + Protocol string `json:"protocol,omitempty" jsonschema:"TCP, UDP or SCTP. Defaults to TCP."` +} + +// RenderKeyRef selects one key of a ConfigMap or Secret. +type RenderKeyRef struct { + Name string `json:"name" jsonschema:"Name of the ConfigMap or Secret, which must already exist in the project."` + Key string `json:"key" jsonschema:"Key within it."` +} + +// RenderEnvVar is one environment variable on the container. +type RenderEnvVar struct { + Name string `json:"name" jsonschema:"Variable name."` + Value string `json:"value,omitempty" jsonschema:"Literal value. Set at most one of value, configMapKeyRef, secretKeyRef."` + ConfigMapKeyRef *RenderKeyRef `json:"configMapKeyRef,omitempty" jsonschema:"Read the value from a ConfigMap key instead."` + SecretKeyRef *RenderKeyRef `json:"secretKeyRef,omitempty" jsonschema:"Read the value from a Secret key instead."` +} + +// RenderMount projects a ConfigMap or Secret into the instance's filesystem. +type RenderMount struct { + Name string `json:"name,omitempty" jsonschema:"Volume name. Defaults to the ConfigMap or Secret name."` + ConfigMap string `json:"configMap,omitempty" jsonschema:"Name of the ConfigMap to mount. Set exactly one of configMap or secret."` + Secret string `json:"secret,omitempty" jsonschema:"Name of the Secret to mount. Set exactly one of configMap or secret."` + MountPath string `json:"mountPath" jsonschema:"Absolute path the contents appear at inside the instance."` +} + +// RenderVM asks for a virtual machine rather than a container. +type RenderVM struct { + SSHKeys []string `json:"sshKeys" jsonschema:"Keys authorized to log in, each \"username:ssh-public-key\". At least one — a machine with no key is unreachable and is rejected."` + BootImage string `json:"bootImage,omitempty" jsonschema:"Disk image the machine boots. Defaults to datumcloud/ubuntu-2204-lts, currently the only one accepted."` +} + +// WorkloadRenderInput is the flat description a manifest is rendered from. It +// mirrors workloadspec.Input field for field, so the tool schema can be worded +// for a model without that wording leaking into the renderer. +type WorkloadRenderInput struct { + Name string `json:"name" jsonschema:"Workload name, a DNS label, e.g. \"api-backend\". Cannot be changed later."` + Image string `json:"image,omitempty" jsonschema:"Fully qualified container image, e.g. \"ghcr.io/acme/api:1.4.2\". Required unless vm is set. A bare name is the most common cause of ImageUnavailable afterwards."` + InstanceType string `json:"instanceType,omitempty" jsonschema:"Instance type from compute_instance_types_list. Defaults to the only one accepted today."` + RuntimeClass string `json:"runtimeClass,omitempty" jsonschema:"Execution tier the instances run in, named verbatim from the RuntimeClass objects resources_list returns for compute.datumapis.com/v1alpha. Leave unset unless the person named one: the server picks its default, and the tier cannot be changed after the workload exists."` + Network string `json:"network,omitempty" jsonschema:"Network the instance attaches to. Defaults to \"default\"."` + Placements []RenderPlacement `json:"placements" jsonschema:"Where instances run and how many. At least one is required."` + Ports []RenderPort `json:"ports,omitempty" jsonschema:"Named ports the workload serves. Each is also opened to the internet, since a port nothing can reach is not useful."` + Env []RenderEnvVar `json:"env,omitempty" jsonschema:"Environment variables on the container. Not accepted for a virtual machine."` + ConfigMounts []RenderMount `json:"configMounts,omitempty" jsonschema:"ConfigMaps and Secrets projected into the instance's filesystem."` + PublicIPv4 bool `json:"publicIPv4,omitempty" jsonschema:"Ask for a public IPv4 address. Settled at create: it cannot be added or removed later, so ask before rendering rather than defaulting it."` + Labels map[string]string `json:"labels,omitempty" jsonschema:"Labels applied to the workload and to every instance it creates."` + VM *RenderVM `json:"vm,omitempty" jsonschema:"Render a virtual machine instead of a container. Only when the person needs a whole operating system to log into."` +} + +// WorkloadRenderOutput is the manifest and what rendering it settled. +type WorkloadRenderOutput struct { + // Manifest is the complete Workload, as YAML. + Manifest string `json:"manifest"` + // Notes are the decisions this manifest fixes for the life of the workload + // and the defaults that were filled in. Worth reading out: several of them + // cannot be changed after the first apply. + Notes []string `json:"notes,omitempty"` +} + +// ------------------------------------------------------------ registration + +// registerRenderTool adds compute_workload_render to s. +func registerRenderTool(s *mcp.Server, deps DepsFor) { + mcp.AddTool(s, &mcp.Tool{ + Name: ToolWorkloadRender, + Title: "Render a workload manifest", + Description: "Turn a short description of a deployment — name, image, where, how many — into a " + + "complete Workload manifest, and report what rendering it settled. Nothing is read and nothing " + + "is changed, so render as often as it takes to get the manifest right. Read the manifest that " + + "comes back rather than assuming it says what was asked for, and read the notes: they name the " + + "choices that cannot be changed once the workload exists, the interface's address families and " + + "a public IPv4 address among them. Gather the inputs from the person rather than inventing " + + "them: take location names from locations_list with service \"compute\" and the instance " + + "type from compute_instance_types_list. A placement either names locations or selects them by " + + "topology; use a locationSelector for \"every location in a city or region\", which also picks " + + "up locations added later. The manifest is then passed to resources_plan and, once the person " + + "agrees, resources_apply. Load the workload-create skill before using this. Writes nothing.", + }, workloadRender(deps)) +} + +// ---------------------------------------------------------------- handlers + +func workloadRender(deps DepsFor) mcp.ToolHandlerFor[WorkloadRenderInput, WorkloadRenderOutput] { + return func( + ctx context.Context, _ *mcp.CallToolRequest, in WorkloadRenderInput, + ) (*mcp.CallToolResult, WorkloadRenderOutput, error) { + // Rendering reads nothing, but an unauthenticated caller must not be + // able to use it as a probe, the same rule compute_reason_explain follows. + if _, err := deps(ctx); err != nil { + return nil, WorkloadRenderOutput{}, err + } + + spec := toSpecInput(in) + workload, err := workloadspec.Render(spec) + if err != nil { + return nil, WorkloadRenderOutput{}, err + } + manifest, err := workloadspec.MarshalYAML(workload) + if err != nil { + return nil, WorkloadRenderOutput{}, err + } + + return nil, WorkloadRenderOutput{ + Manifest: string(manifest), + Notes: renderNotes(spec), + }, nil + } +} + +// ---------------------------------------------------------------- rendering + +// toSpecInput converts the tool's input to workloadspec's. A straight mapping, +// kept explicit so the tool schema can be worded for a model without that +// wording leaking into the renderer. +func toSpecInput(in WorkloadRenderInput) workloadspec.Input { + out := workloadspec.Input{ + Name: in.Name, + Image: in.Image, + InstanceType: in.InstanceType, + RuntimeClass: in.RuntimeClass, + Network: in.Network, + PublicIPv4: in.PublicIPv4, + Labels: in.Labels, + } + + for _, p := range in.Placements { + out.Placements = append(out.Placements, workloadspec.Placement{ + Name: p.Name, + Locations: p.Locations, + LocationSelector: toLabelSelector(p.LocationSelector), + MinReplicas: p.MinReplicas, + }) + } + for _, p := range in.Ports { + out.Ports = append(out.Ports, workloadspec.Port{ + Name: p.Name, + Port: p.Port, + Protocol: corev1.Protocol(p.Protocol), + }) + } + for _, e := range in.Env { + out.Env = append(out.Env, workloadspec.EnvVar{ + Name: e.Name, + Value: e.Value, + ConfigMapKeyRef: toKeyRef(e.ConfigMapKeyRef), + SecretKeyRef: toKeyRef(e.SecretKeyRef), + }) + } + for _, m := range in.ConfigMounts { + out.ConfigMounts = append(out.ConfigMounts, workloadspec.Mount{ + Name: m.Name, + ConfigMap: m.ConfigMap, + Secret: m.Secret, + MountPath: m.MountPath, + }) + } + if in.VM != nil { + out.VM = &workloadspec.VMInput{ + SSHKeys: in.VM.SSHKeys, + BootImage: in.VM.BootImage, + } + } + + return out +} + +// toLabelSelector converts the tool's selector to the API's. The operator is +// passed through verbatim: an unrecognized one is refused by the render's own +// validation with the field path, which is more useful than silently dropping +// the requirement here. +func toLabelSelector(sel *RenderLocationSelector) *metav1.LabelSelector { + if sel == nil { + return nil + } + out := &metav1.LabelSelector{MatchLabels: sel.MatchLabels} + for _, req := range sel.MatchExpressions { + out.MatchExpressions = append(out.MatchExpressions, metav1.LabelSelectorRequirement{ + Key: req.Key, + Operator: metav1.LabelSelectorOperator(req.Operator), + Values: req.Values, + }) + } + return out +} + +func toKeyRef(ref *RenderKeyRef) *workloadspec.KeyRef { + if ref == nil { + return nil + } + return &workloadspec.KeyRef{Name: ref.Name, Key: ref.Key} +} + +// renderNotes says what this manifest settled that a later render cannot +// correct, and which values were filled in for a caller who did not name them. +// +// It is written from the input as given, before defaults are applied, so +// "defaulted to" means the person did not choose it — which is the thing they +// need to be asked about while the workload can still be changed. +func renderNotes(in workloadspec.Input) []string { + notes := []string{ + "The instance's single network interface is settled by this manifest and cannot be changed " + + "once the workload exists: its name, the address families it carries, any extra addresses, " + + "and what becomes of those addresses when an instance goes away. Getting one of them wrong " + + "means creating a new workload, not editing this one.", + } + + if in.PublicIPv4 { + notes = append(notes, "A public IPv4 address was asked for, so the interface carries both IPv4 "+ + "and IPv6. Neither the address nor the families can be removed later.") + } else { + notes = append(notes, "The interface carries IPv6 only, which is the default. If this workload "+ + "has to answer on IPv4, say so before it is applied: IPv4 cannot be added afterwards.") + } + + notes = append(notes, "Addresses are given back when an instance goes away. Keeping one — an "+ + "address published in DNS, or allowed through someone's firewall — means editing this manifest "+ + "before the first apply.") + + if in.InstanceType == "" { + notes = append(notes, fmt.Sprintf( + "No instance type was given, so every instance is %s. Per-container CPU and memory are not "+ + "accepted: the instance type is what decides the size.", workloadspec.DefaultInstanceType)) + } + if in.Network == "" { + notes = append(notes, fmt.Sprintf( + "No network was named, so the interface attaches to %q. Check it exists with %s; if it does "+ + "not, plan a Network manifest of that name in the same %s call as this workload.", + workloadspec.DefaultNetwork, baseToolResourcesList, baseToolResourcesPlan)) + } + for _, p := range in.Placements { + if p.Name == "" { + notes = append(notes, fmt.Sprintf("A placement was not named, so it is called %q.", + workloadspec.DefaultPlacementName)) + } + if p.MinReplicas == 0 { + notes = append(notes, fmt.Sprintf( + "Placement %q did not say how many instances to run, so it runs %d. There is no "+ + "scaling to zero.", placementName(p), workloadspec.DefaultMinReplicas)) + } + if p.LocationSelector != nil { + notes = append(notes, fmt.Sprintf( + "Placement %q selects its locations by topology rather than naming them, so it runs "+ + "wherever the selector matches — including locations added later, which will "+ + "start instances without this manifest changing. %s shows which locations match "+ + "today.", placementName(p), baseToolLocationsList)) + } + } + if in.VM != nil && in.VM.BootImage == "" { + notes = append(notes, fmt.Sprintf( + "No boot image was given, so the machine boots %s, currently the only one accepted.", + workloadspec.DefaultBootImage)) + } + + return notes +} + +func placementName(p workloadspec.Placement) string { + if p.Name == "" { + return workloadspec.DefaultPlacementName + } + return p.Name +} diff --git a/internal/agent/render_test.go b/internal/agent/render_test.go new file mode 100644 index 00000000..b67b8843 --- /dev/null +++ b/internal/agent/render_test.go @@ -0,0 +1,237 @@ +// SPDX-License-Identifier: AGPL-3.0-only + +package agent + +import ( + "context" + "encoding/json" + "errors" + "strings" + "testing" + + "github.com/modelcontextprotocol/go-sdk/mcp" + sigsyaml "sigs.k8s.io/yaml" + + computev1alpha "go.datum.net/compute/api/v1alpha" + "go.datum.net/compute/internal/locations" + "go.datum.net/compute/internal/workloadspec" +) + +const testImage = "ghcr.io/acme/api:1.4.2" + +// renderInput is the everyday case: one container, one location, one port. +func renderInput() WorkloadRenderInput { + return WorkloadRenderInput{ + Name: wlAPIBackend, + Image: testImage, + Placements: []RenderPlacement{{Locations: []string{locationDFW}, MinReplicas: 2}}, + Ports: []RenderPort{{Name: "http", Port: 8080}}, + } +} + +// TestWorkloadRenderProducesAManifestAndSaysWhatIsSettled: the manifest is only +// half the answer. The notes carry the decisions that cannot be corrected by a +// later render, and a model that does not read them out lets a person agree to +// something they would have to recreate the workload to change. +func TestWorkloadRenderProducesAManifestAndSaysWhatIsSettled(t *testing.T) { + deps := fixtureDeps(fixtureReader()) + + _, out, err := workloadRender(deps)(context.Background(), nil, renderInput()) + if err != nil { + t.Fatalf("compute_workload_render: %v", err) + } + + for _, want := range []string{ + "kind: Workload", + "name: " + wlAPIBackend, + testImage, + "minReplicas: 2", + "- name: " + locationDFW, + } { + if !strings.Contains(out.Manifest, want) { + t.Errorf("manifest is missing %q:\n%s", want, out.Manifest) + } + } + // The manifest is handed on to the assistant's plan tool as-is, so it has + // to read straight back into a Workload. + var decoded computev1alpha.Workload + if err := sigsyaml.UnmarshalStrict([]byte(out.Manifest), &decoded); err != nil { + t.Errorf("the rendered manifest does not read back: %v", err) + } + + notes := strings.Join(out.Notes, "\n") + // The interface is settled at create, and the instance type and network + // were defaulted rather than chosen — both are things to say out loud + // while the workload can still be changed. A missing network is planned + // alongside the workload, so the note has to say how. + for _, want := range []string{ + "cannot be changed once the workload exists", + "IPv6 only", + workloadspec.DefaultInstanceType, + "\"" + workloadspec.DefaultNetwork + "\"", + baseToolResourcesPlan, + } { + if !strings.Contains(notes, want) { + t.Errorf("notes do not mention %q:\n%s", want, notes) + } + } +} + +// TestWorkloadRenderSelectsLocationsByTopology covers the second way a +// placement says where: a selector over location topology rather than a list of +// names. It is the only way to say "every location in this city", and it keeps +// matching locations added later — which is a standing behaviour the person +// agreeing to the manifest has to be told about, so the notes carry it. +func TestWorkloadRenderSelectsLocationsByTopology(t *testing.T) { + deps := fixtureDeps(fixtureReader()) + in := renderInput() + in.Placements = []RenderPlacement{{ + LocationSelector: &RenderLocationSelector{ + MatchLabels: map[string]string{locations.TopologyCityCodeKey: cityDFW}, + }, + MinReplicas: 2, + }} + + _, out, err := workloadRender(deps)(context.Background(), nil, in) + if err != nil { + t.Fatalf("compute_workload_render: %v", err) + } + for _, want := range []string{"locationSelector:", locations.TopologyCityCodeKey + ": " + cityDFW} { + if !strings.Contains(out.Manifest, want) { + t.Errorf("manifest is missing %q:\n%s", want, out.Manifest) + } + } + if strings.Contains(out.Manifest, "locations:") { + t.Errorf("a selector was given, so no location list may be emitted:\n%s", out.Manifest) + } + if !strings.Contains(strings.Join(out.Notes, "\n"), "locations added later") { + t.Errorf("notes do not say the selector keeps matching new locations:\n%s", out.Notes) + } +} + +// TestWorkloadRenderPassesTheRuntimeClassThrough: the tier is the server's +// catalog to own. Whatever the person named goes through verbatim, and naming +// nothing leaves the field off so the server picks its own default rather than +// this tool settling a choice that cannot be changed afterwards. +func TestWorkloadRenderPassesTheRuntimeClassThrough(t *testing.T) { + deps := fixtureDeps(fixtureReader()) + + _, bare, err := workloadRender(deps)(context.Background(), nil, renderInput()) + if err != nil { + t.Fatalf("compute_workload_render: %v", err) + } + if strings.Contains(bare.Manifest, "class:") { + t.Errorf("no runtime class was asked for, so none may be rendered:\n%s", bare.Manifest) + } + + in := renderInput() + in.RuntimeClass = "datum-sandbox" + _, out, err := workloadRender(deps)(context.Background(), nil, in) + if err != nil { + t.Fatalf("compute_workload_render: %v", err) + } + if !strings.Contains(out.Manifest, "class: datum-sandbox") { + t.Errorf("manifest does not carry the runtime class that was asked for:\n%s", out.Manifest) + } +} + +// TestWorkloadRenderReportsAPublicAddressAsFinal: asking for IPv4 fixes the +// address families for the life of the workload, so the note has to change +// with the input rather than always saying the same thing. +func TestWorkloadRenderReportsAPublicAddressAsFinal(t *testing.T) { + deps := fixtureDeps(fixtureReader()) + in := renderInput() + in.PublicIPv4 = true + + _, out, err := workloadRender(deps)(context.Background(), nil, in) + if err != nil { + t.Fatalf("compute_workload_render: %v", err) + } + notes := strings.Join(out.Notes, "\n") + if !strings.Contains(notes, "public IPv4 address was asked for") { + t.Errorf("notes do not report the public address as settled:\n%s", notes) + } + if strings.Contains(notes, "IPv6 only") { + t.Errorf("notes still claim IPv6 only after IPv4 was asked for:\n%s", notes) + } +} + +// TestWorkloadRenderRefusesAnIncompleteInput: a missing image is the caller's +// to supply, and rendering something plausible around a name nobody pushed is +// the failure this prevents. +func TestWorkloadRenderRefusesAnIncompleteInput(t *testing.T) { + deps := fixtureDeps(fixtureReader()) + in := renderInput() + in.Image = "" + + if _, _, err := workloadRender(deps)(context.Background(), nil, in); err == nil { + t.Error("compute_workload_render accepted an input with no image") + } +} + +// TestWorkloadRenderFailsWhenDepsAreUnavailable: rendering reads nothing, but +// it must not be a probe an unauthenticated caller can use either. +func TestWorkloadRenderFailsWhenDepsAreUnavailable(t *testing.T) { + wantErr := errors.New("no bearer token on the request") + denied := DepsFor(func(context.Context) (ToolDeps, error) { return ToolDeps{}, wantErr }) + + if _, _, err := workloadRender(denied)(context.Background(), nil, renderInput()); !errors.Is(err, wantErr) { + t.Errorf("compute_workload_render error = %v, want the deps error to surface unchanged", err) + } +} + +// TestWorkloadRenderAnswersOverTheWire proves registration and the schemas, +// not just the handler: a tool that is never wired into RegisterTools passes +// every test above and is uncallable in production, and an output the SDK +// cannot encode reaches the model as nothing at all. +func TestWorkloadRenderAnswersOverTheWire(t *testing.T) { + ctx := context.Background() + + server := mcp.NewServer(&mcp.Implementation{Name: testServerName, Version: testImplVersion}, nil) + RegisterTools(server, fixtureDeps(fixtureReader())) + + serverTransport, clientTransport := mcp.NewInMemoryTransports() + serverSession, err := server.Connect(ctx, serverTransport, nil) + if err != nil { + t.Fatalf("connecting server: %v", err) + } + defer func() { _ = serverSession.Close() }() + + client := mcp.NewClient(&mcp.Implementation{Name: testClientName, Version: testImplVersion}, nil) + clientSession, err := client.Connect(ctx, clientTransport, nil) + if err != nil { + t.Fatalf("connecting client: %v", err) + } + defer func() { _ = clientSession.Close() }() + + res, err := clientSession.CallTool(ctx, &mcp.CallToolParams{ + Name: ToolWorkloadRender, + Arguments: map[string]any{ + "name": wlAPIBackend, + "image": testImage, + "placements": []map[string]any{ + {"locations": []string{locationDFW}, "minReplicas": 2}, + }, + }, + }) + if err != nil { + t.Fatalf("calling %s: %v", ToolWorkloadRender, err) + } + if res.IsError { + t.Fatalf("%s returned an error result: %+v", ToolWorkloadRender, res.Content) + } + + // Round-tripped through the wire's JSON, so the output schema is exercised + // as the model would receive it. + raw, err := json.Marshal(res.StructuredContent) + if err != nil { + t.Fatalf("marshalling structured content: %v", err) + } + var out WorkloadRenderOutput + if err := json.Unmarshal(raw, &out); err != nil { + t.Fatalf("decoding %s output: %v", ToolWorkloadRender, err) + } + if !strings.Contains(out.Manifest, "name: "+wlAPIBackend) { + t.Errorf("render over the wire returned no usable manifest: %s", raw) + } +} diff --git a/internal/agent/tools.go b/internal/agent/tools.go index 965baa67..e588776b 100644 --- a/internal/agent/tools.go +++ b/internal/agent/tools.go @@ -12,18 +12,22 @@ import ( computev1alpha "go.datum.net/compute/api/v1alpha" ) -// The tools compute publishes to an assistant. All five are read-only. +// The tools compute publishes to an assistant. These five diagnose what a +// project has deployed; render.go and instancetypes.go add the two that help +// write a new workload. None of them changes anything. // -// There is deliberately no mutating tool — no delete, no scale, no restart. -// The gateway's allow-list is the enforcement point, but a tool that is never +// There is deliberately no mutating tool — no delete, no scale, no restart, +// and no create. Creating a workload goes through the assistant's own plan and +// apply tools, which hold the confirmation step for every service. The +// gateway's allow-list is the enforcement point, but a tool that is never // implemented cannot be called through any path at all. Adding one needs its // own review, not a quiet addition here. const ( - ToolWorkloadsList = "workloads_list" - ToolWorkloadsGet = "workloads_get" - ToolInstancesList = "instances_list" - ToolWorkloadDiagnose = "workload_diagnose" - ToolReasonExplain = "reason_explain" + ToolWorkloadsList = "compute_workloads_list" + ToolWorkloadsGet = "compute_workloads_get" + ToolInstancesList = "compute_instances_list" + ToolWorkloadDiagnose = "compute_workload_diagnose" + ToolReasonExplain = "compute_reason_explain" ) // ToolDeps is what one request's tool calls operate over: where to read from, @@ -167,9 +171,9 @@ type ReasonExplainOutput struct { // ------------------------------------------------------------ registration -// RegisterTools adds compute's read-only diagnostic tools to s. deps is -// consulted per call rather than captured once, so no caller can inherit -// another's identity or project. +// RegisterTools adds every tool compute publishes to s. deps is consulted per +// call rather than captured once, so no caller can inherit another's identity +// or project. func RegisterTools(s *mcp.Server, deps DepsFor) { mcp.AddTool(s, &mcp.Tool{ Name: ToolWorkloadsList, @@ -224,6 +228,9 @@ func RegisterTools(s *mcp.Server, deps DepsFor) { "argument to list the whole catalog. Use when you encounter a reason on a resource the " + "diagnose tool did not cover. Read-only.", }, reasonExplain(deps)) + + registerInstanceTypesTool(s, deps) + registerRenderTool(s, deps) } // ---------------------------------------------------------------- handlers diff --git a/internal/agent/tools_test.go b/internal/agent/tools_test.go index 904b8ceb..dc092adb 100644 --- a/internal/agent/tools_test.go +++ b/internal/agent/tools_test.go @@ -21,6 +21,16 @@ const ( depAPIBackend = "api-backend-a" placementUSCentral = "us-central" locationDFW = "loc-dfw-1" + locationAMS = "loc-ams-1" + cityDFW = "DFW" +) + +// The identities the in-memory MCP transports are exercised under. Shared, so +// the several tests that stand a server up are obviously the same setup. +const ( + testImplVersion = "0.0.1" + testServerName = "test" + testClientName = "test-client" ) // fakeReader serves canned objects so the tools can be exercised without a @@ -109,7 +119,7 @@ func fixtureReader() *fakeReader { computev1alpha.WorkloadDeploymentReasonNoMatchingLocation, "The cell has not been told which location it serves.")) edgeDeployment.Spec.PlacementName = "ams-edge" - edgeDeployment.Spec.LocationRef.Name = "loc-ams-1" + edgeDeployment.Spec.LocationRef.Name = locationAMS apiDeployment := deployment(depAPIBackend, cond(computev1alpha.WorkloadDeploymentAvailable, "False", @@ -141,7 +151,7 @@ func TestWorkloadsListReportsRootCauseAndOrdersWorstFirst(t *testing.T) { _, out, err := workloadsList(deps)(context.Background(), nil, WorkloadsListInput{}) if err != nil { - t.Fatalf("workloads_list: %v", err) + t.Fatalf("compute_workloads_list: %v", err) } if len(out.Workloads) != 3 { t.Fatalf("got %d workloads, want 3", len(out.Workloads)) @@ -194,7 +204,7 @@ func TestWorkloadsGetReturnsFullTree(t *testing.T) { _, out, err := workloadsGet(deps)(context.Background(), nil, WorkloadsGetInput{Name: wlAPIBackend}) if err != nil { - t.Fatalf("workloads_get: %v", err) + t.Fatalf("compute_workloads_get: %v", err) } if out.Workload.Name != wlAPIBackend || out.Workload.Namespace != testNamespace { @@ -230,7 +240,7 @@ func TestInstancesListFilters(t *testing.T) { _, filtered, err := instancesList(deps)(context.Background(), nil, InstancesListInput{Workload: wlAPIBackend}) if err != nil { - t.Fatalf("instances_list filtered: %v", err) + t.Fatalf("compute_instances_list filtered: %v", err) } if len(filtered.Instances) != 3 { t.Errorf("filtered instances = %d, want 3", len(filtered.Instances)) @@ -238,7 +248,7 @@ func TestInstancesListFilters(t *testing.T) { _, all, err := instancesList(deps)(context.Background(), nil, InstancesListInput{}) if err != nil { - t.Fatalf("instances_list unfiltered: %v", err) + t.Fatalf("compute_instances_list unfiltered: %v", err) } // web-frontend has 3, api-backend has 3, edge-cache has none. if len(all.Instances) != 6 { @@ -277,7 +287,7 @@ func TestWorkloadDiagnoseSurfacesLeafCause(t *testing.T) { _, d, err := workloadDiagnose(deps)( context.Background(), nil, WorkloadDiagnoseInput{Name: tc.workload}) if err != nil { - t.Fatalf("workload_diagnose: %v", err) + t.Fatalf("compute_workload_diagnose: %v", err) } if d.RootCause == nil { t.Fatalf("RootCause is nil, want %q", tc.wantReason) @@ -305,7 +315,7 @@ func TestReasonExplain(t *testing.T) { _, one, err := reasonExplain(deps)(ctx, nil, ReasonExplainInput{Reason: "QuotaNoBudget"}) if err != nil { - t.Fatalf("reason_explain: %v", err) + t.Fatalf("compute_reason_explain: %v", err) } if one.Reason == nil { t.Fatal("Reason is nil") @@ -319,7 +329,7 @@ func TestReasonExplain(t *testing.T) { _, all, err := reasonExplain(deps)(ctx, nil, ReasonExplainInput{}) if err != nil { - t.Fatalf("reason_explain (all): %v", err) + t.Fatalf("compute_reason_explain (all): %v", err) } if len(all.Reasons) != len(AllReasons()) { t.Errorf("got %d reasons, want the whole catalog (%d)", len(all.Reasons), len(AllReasons())) @@ -339,19 +349,19 @@ func TestToolsFailWhenDepsUnavailable(t *testing.T) { ctx := context.Background() if _, _, err := workloadsList(denied)(ctx, nil, WorkloadsListInput{}); err == nil { - t.Error("workloads_list should fail without deps") + t.Error("compute_workloads_list should fail without deps") } if _, _, err := workloadsGet(denied)(ctx, nil, WorkloadsGetInput{Name: "x"}); err == nil { - t.Error("workloads_get should fail without deps") + t.Error("compute_workloads_get should fail without deps") } if _, _, err := instancesList(denied)(ctx, nil, InstancesListInput{}); err == nil { - t.Error("instances_list should fail without deps") + t.Error("compute_instances_list should fail without deps") } if _, _, err := workloadDiagnose(denied)(ctx, nil, WorkloadDiagnoseInput{Name: "x"}); err == nil { - t.Error("workload_diagnose should fail without deps") + t.Error("compute_workload_diagnose should fail without deps") } if _, _, err := reasonExplain(denied)(ctx, nil, ReasonExplainInput{}); err == nil { - t.Error("reason_explain should fail without deps: it must not be a probe for unauthenticated callers") + t.Error("compute_reason_explain should fail without deps: it must not be a probe for unauthenticated callers") } } @@ -363,18 +373,20 @@ func TestReaderErrorsPropagate(t *testing.T) { deps := fixtureDeps(r) if _, _, err := workloadsList(deps)(context.Background(), nil, WorkloadsListInput{}); err == nil { - t.Error("workloads_list should surface a reader error") + t.Error("compute_workloads_list should surface a reader error") } } -// TestRegisterToolsPublishesExactlyTheReadOnlySet inspects what a registered -// server actually advertises: five read-only tools and no mutating one, so -// anything extra over the wire is a bug. It also catches a schema that fails to -// infer, since AddTool panics on a bad one. -func TestRegisterToolsPublishesExactlyTheReadOnlySet(t *testing.T) { +// TestRegisterToolsPublishesExactlyTheDocumentedSet inspects what a registered +// server actually advertises: seven named tools, none of them mutating, so +// anything extra over the wire is a bug. A tool's promise that it changes +// nothing is what tells the model it can run it without asking first, so every +// description must make it. It also catches a schema that fails to infer, since +// AddTool panics on a bad one. +func TestRegisterToolsPublishesExactlyTheDocumentedSet(t *testing.T) { ctx := context.Background() - server := mcp.NewServer(&mcp.Implementation{Name: "test", Version: "0.0.1"}, nil) + server := mcp.NewServer(&mcp.Implementation{Name: testServerName, Version: testImplVersion}, nil) RegisterTools(server, fixtureDeps(fixtureReader())) serverTransport, clientTransport := mcp.NewInMemoryTransports() @@ -384,7 +396,7 @@ func TestRegisterToolsPublishesExactlyTheReadOnlySet(t *testing.T) { } defer func() { _ = serverSession.Close() }() - client := mcp.NewClient(&mcp.Implementation{Name: "test-client", Version: "0.0.1"}, nil) + client := mcp.NewClient(&mcp.Implementation{Name: testClientName, Version: testImplVersion}, nil) clientSession, err := client.Connect(ctx, clientTransport, nil) if err != nil { t.Fatalf("connecting client: %v", err) @@ -402,11 +414,15 @@ func TestRegisterToolsPublishesExactlyTheReadOnlySet(t *testing.T) { } want := []string{ + // What the project has deployed. ToolWorkloadsList, ToolWorkloadsGet, ToolInstancesList, ToolWorkloadDiagnose, ToolReasonExplain, + // What a new workload may ask for, and its manifest. + ToolInstanceTypesList, + ToolWorkloadRender, } if len(got) != len(want) { t.Errorf("published %d tools %v, want exactly %d", len(got), keysOf(got), len(want)) @@ -426,12 +442,15 @@ func TestRegisterToolsPublishesExactlyTheReadOnlySet(t *testing.T) { // Compute ships no mutating tool. Enforcement of the allow-list is the // gateway's job, but a tool that does not exist cannot be called at all. - for name := range got { - for _, forbidden := range []string{"delete", "create", "update", "scale", "restart"} { + for name, desc := range got { + for _, forbidden := range []string{"delete", "create", "update", "scale", "restart", "apply", "plan"} { if strings.Contains(name, forbidden) { t.Errorf("tool %q looks mutating; compute publishes read-only tools only", name) } } + if !strings.Contains(desc, "Read-only.") && !strings.Contains(desc, "Writes nothing.") { + t.Errorf("tool %q does not say it is read-only or writes nothing", name) + } } } @@ -468,7 +487,7 @@ func TestWorkloadsListCarriesTheAgeOfTheRootCause(t *testing.T) { _, out, err := workloadsList(deps)(context.Background(), nil, WorkloadsListInput{}) if err != nil { - t.Fatalf("workloads_list: %v", err) + t.Fatalf("compute_workloads_list: %v", err) } if len(out.Workloads) != 1 { t.Fatalf("got %d workloads, want 1", len(out.Workloads)) @@ -490,7 +509,7 @@ func TestWorkloadsListOmitsAgeForHealthyWorkloads(t *testing.T) { _, out, err := workloadsList(deps)(context.Background(), nil, WorkloadsListInput{}) if err != nil { - t.Fatalf("workloads_list: %v", err) + t.Fatalf("compute_workloads_list: %v", err) } for _, row := range out.Workloads { if row.Workload == wlWebFrontend && (row.RootCauseSince != "" || row.RootCauseFor != "") { @@ -510,7 +529,7 @@ func TestWorkloadsListFlagsTheStagingStall(t *testing.T) { _, out, err := workloadsList(deps)(context.Background(), nil, WorkloadsListInput{}) if err != nil { - t.Fatalf("workloads_list: %v", err) + t.Fatalf("compute_workloads_list: %v", err) } row := out.Workloads[0] if row.RootCauseReason != computev1alpha.InstanceProgrammedReasonProgrammingInProgress { @@ -561,7 +580,7 @@ func TestWorkloadsListLeavesFreshTransientStateAlone(t *testing.T) { _, out, err := workloadsList(deps)(context.Background(), nil, WorkloadsListInput{}) if err != nil { - t.Fatalf("workloads_list: %v", err) + t.Fatalf("compute_workloads_list: %v", err) } if got := out.Workloads[0].Actionability; got != ActionabilityTransient { t.Errorf("Actionability = %q, want %q for a two-minute-old ProgrammingInProgress", @@ -592,7 +611,7 @@ func TestWorkloadsListCarriesTheFailureFloor(t *testing.T) { _, out, err := workloadsList(deps)(context.Background(), nil, WorkloadsListInput{}) if err != nil { - t.Fatalf("workloads_list: %v", err) + t.Fatalf("compute_workloads_list: %v", err) } row := out.Workloads[0] if row.RootCauseFor != stagingInState { diff --git a/internal/workloadspec/render.go b/internal/workloadspec/render.go new file mode 100644 index 00000000..6d8eb6e8 --- /dev/null +++ b/internal/workloadspec/render.go @@ -0,0 +1,989 @@ +// SPDX-License-Identifier: AGPL-3.0-only + +// Package workloadspec turns a small, flat description of a deployment into a +// complete compute Workload manifest. +// +// The package is pure: it performs no I/O, reads no configuration, and has no +// dependency on cobra, the datumctl plugin runtime, or anything under +// internal/cmd. Render is a total function of its Input, so the same Input +// always yields the same manifest. That makes it usable both from the CLI's +// flag path and from a tool-call surface that only ever renders and returns +// YAML. +// +// # Relationship to the admission webhook +// +// Render never emits a manifest that the Workload admission webhook +// (internal/validation) would reject for structural reasons: a runtime is +// always present and is exactly one of sandbox or virtualMachine, every +// declared volume is attached at least once, a VM always carries the +// compute.datumapis.com/ssh-keys template annotation and a bootable first +// volume attachment, exactly one network interface is emitted, and scale +// settings stay inside the accepted range. Inputs that cannot satisfy those +// rules are reported as a field.ErrorList rather than rendered. +// +// Render does not police values the platform's catalogs own — the instance +// type and the boot image are passed through and left to the server, which is +// authoritative and whose accepted set changes without this package changing. +// Today the server accepts only DefaultInstanceType and DefaultBootImage. +// +// # Create-time-only decisions +// +// Several fields of a network interface are immutable once the workload +// exists, so Render's choices for them cannot be corrected by a later render +// of a changed Input — the workload has to be recreated instead: +// +// - name: left unset, so the API server defaults it to "eth0". The guest is +// configured against this name and the interface's address claim is named +// after it. +// - ipFamilies: left unset (the API server defaults it to IPv6 only) unless +// PublicIPv4 is requested, in which case [IPv4, IPv6] is emitted so the +// interface also holds an IPv4 address inside its network. Every family +// listed must be satisfiable by the network or the interface is never +// published. +// - addresses: emitted only for PublicIPv4, as a single public-ipv4 class +// request. +// - reclaimPolicy: left unset, so the API server defaults it to Delete and +// addresses are returned to IPAM when the instance slot goes away. Callers +// that publish an address in DNS want Retain, which means editing the +// rendered manifest before the first apply. +package workloadspec + +import ( + "encoding/json" + "fmt" + "strings" + + "golang.org/x/crypto/ssh" + corev1 "k8s.io/api/core/v1" + apimachineryvalidation "k8s.io/apimachinery/pkg/api/validation" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/apimachinery/pkg/util/sets" + utilvalidation "k8s.io/apimachinery/pkg/util/validation" + "k8s.io/apimachinery/pkg/util/validation/field" + sigsyaml "sigs.k8s.io/yaml" + + computev1alpha "go.datum.net/compute/api/v1alpha" + networkingv1alpha "go.datum.net/network-services-operator/api/v1alpha" + locationsv1alpha1 "go.miloapis.com/locations/api/v1alpha1" +) + +const ( + // DefaultInstanceType is the instance type used when Input.InstanceType is + // empty. It is currently the only type the admission webhook accepts. + DefaultInstanceType = "datumcloud/d1-standard-2" + + // DefaultNetwork is the network attached when Input.Network is empty. + DefaultNetwork = "default" + + // DefaultPlacementName is the name given to a placement that does not name + // itself. + DefaultPlacementName = "default" + + // DefaultMinReplicas is the replica floor used when a placement leaves + // MinReplicas at zero. Scale-to-zero is not supported, so zero is read as + // "unset" rather than as a request for no instances. + DefaultMinReplicas int32 = 1 + + // DefaultBootImage is the image a VM's boot disk is populated from when + // VMInput.BootImage is empty. It is currently the only image the admission + // webhook accepts. + DefaultBootImage = "datumcloud/ubuntu-2204-lts" + + // Namespace is the namespace every rendered workload lives in. Project + // control planes serve a single namespace. + Namespace = "default" + + // ContainerName is the name given to the sandbox container. A rendered + // sandbox always has exactly one container. + ContainerName = "app" + + // BootVolumeName is the name of the disk volume a VM boots from. It is + // always the VM's first volume attachment, which is what the webhook + // requires of a bootable volume. + BootVolumeName = "boot" + + // PublicIPv4Class is the IPAM class requested for a public IPv4 address. + PublicIPv4Class = "public-ipv4" + + // diskTypePDStandard is the only disk type the platform currently offers. + diskTypePDStandard = "pd-standard" + + // anyIPv4CIDR is the peer an exposed port is opened to. Exposing a port + // without opening it would leave the port unreachable, so the two travel + // together. + anyIPv4CIDR = "0.0.0.0/0" +) + +// Input is the flat description a manifest is rendered from. Every field +// except Name, Image, and Placements has a usable zero value. +type Input struct { + // Name of the workload. Required. + Name string + + // Image is the fully qualified container image the sandbox runs. Required + // unless VM is set, in which case it must be empty — a VM boots from a + // disk image, not a container image. + Image string + + // InstanceType selects the shape of each instance. Defaults to + // DefaultInstanceType. + InstanceType string + + // RuntimeClass names the execution tier the instances run in. Passed + // through verbatim and left empty when unset, so the server picks the + // class its catalog marks as default. Nothing is defaulted here: the + // catalog is served, this package is pure, and guessing a class would + // settle a choice that cannot be changed after the workload exists. + RuntimeClass string + + // Network is the name of the network the instance's single interface + // attaches to. Defaults to DefaultNetwork. + Network string + + // Placements says where instances run and how many. At least one is + // required. + Placements []Placement + + // Ports are the named ports the workload serves. Each also opens an + // ingress network policy rule for that port from anyIPv4CIDR, because a + // declared port that nothing is allowed to reach is not useful. + Ports []Port + + // Env are environment variables set on the sandbox container. Ignored for + // a VM, which has no container to set them on. + Env []EnvVar + + // ConfigMounts project a ConfigMap or Secret into the instance's + // filesystem. Each becomes a volume plus an attachment on the container + // (sandbox) or on the VM. + ConfigMounts []Mount + + // PublicIPv4 asks for a public IPv4 address in front of the interface's + // private addressing. See the package doc: this also fixes ipFamilies at + // [IPv4, IPv6] for the life of the workload. + PublicIPv4 bool + + // Labels are applied both to the workload and to the instance template, so + // they land on the instances the workload creates. Template labels take + // part in the template hash, so changing them rolls the instances. + Labels map[string]string + + // VM, when set, renders a virtual machine runtime instead of a sandbox. + VM *VMInput +} + +// Placement is one group of locations scaled together. Exactly one of +// Locations or LocationSelector must be set. +type Placement struct { + // Name of the placement. Must be a DNS label. Defaults to + // DefaultPlacementName. + Name string + + // Locations the placement deploys to, by name, such as "us-south-dfw-1". + // Each named location receives the placement's replicas. The set of valid + // names is owned by the platform and is not checked here — only that the + // names are well formed and distinct. + Locations []string + + // LocationSelector places at every location whose topology matches, such + // as every location in a city or a region. It is re-evaluated as locations + // are added and removed, where Locations is a fixed list. An empty + // selector is rejected rather than read as matching everything. + LocationSelector *metav1.LabelSelector + + // MinReplicas is the number of instances per placement. Defaults to + // DefaultMinReplicas; must not exceed 1000. + MinReplicas int32 +} + +// Port is a named port the workload serves. +type Port struct { + // Name of the port, referenced by other platform features. Must be a valid + // IANA service name (a DNS label of at most 15 characters containing a + // letter). Required. + Name string + + // Port number, 1 to 65535. Required. + Port int32 + + // Protocol defaults to TCP. + Protocol corev1.Protocol +} + +// EnvVar is one environment variable. Exactly one of Value, ConfigMapKeyRef, +// or SecretKeyRef may be set; all three unset yields an empty value. +type EnvVar struct { + Name string + Value string + ConfigMapKeyRef *KeyRef + SecretKeyRef *KeyRef +} + +// KeyRef selects one key of a ConfigMap or Secret in the workload's namespace. +type KeyRef struct { + Name string + Key string +} + +// Mount projects a ConfigMap or Secret into the instance's filesystem. +// Exactly one of ConfigMap or Secret must be set. +type Mount struct { + // Name of the generated volume. Must be a DNS label. Defaults to the name + // of the referenced ConfigMap or Secret, so a ConfigMap and a Secret of + // the same name need one of them named explicitly. + Name string + + // ConfigMap is the name of the ConfigMap to project. + ConfigMap string + + // Secret is the name of the Secret to project. + Secret string + + // MountPath is the absolute path the volume appears at inside the guest. + // Required, and unique across mounts. + MountPath string +} + +// VMInput describes a virtual machine runtime. +type VMInput struct { + // SSHKeys are the keys authorized to log in, one per entry, each in + // "username:ssh-public-key" form. At least one is required: a VM with no + // key is unreachable and the webhook rejects it. + SSHKeys []string + + // BootImage the boot disk is populated from. Defaults to + // DefaultBootImage. + BootImage string +} + +// Defaults returns an Input pre-filled with the values the CLI advertises: the +// default instance type and network, and a single placement named "default" +// with one replica. The caller still has to supply Name, Image, and the +// placement's Locations or LocationSelector. +func Defaults() Input { + return Input{ + InstanceType: DefaultInstanceType, + Network: DefaultNetwork, + Placements: []Placement{ + { + Name: DefaultPlacementName, + MinReplicas: DefaultMinReplicas, + }, + }, + } +} + +// Render builds a complete Workload from in. It returns the aggregate of a +// field.ErrorList when the input is missing something required or describes a +// manifest the admission webhook would structurally reject; the returned +// workload is nil in that case. +func Render(in Input) (*computev1alpha.Workload, error) { + in = withDefaults(in) + volumes := plannedVolumes(in) + + if errs := validate(in, volumes); len(errs) > 0 { + return nil, errs.ToAggregate() + } + + workload := &computev1alpha.Workload{ + TypeMeta: metav1.TypeMeta{ + APIVersion: computev1alpha.GroupVersion.String(), + Kind: "Workload", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: in.Name, + Namespace: Namespace, + Labels: copyLabels(in.Labels), + }, + Spec: computev1alpha.WorkloadSpec{ + Template: buildTemplate(in, volumes), + Placements: buildPlacements(in.Placements), + }, + } + + return workload, nil +} + +// plannedVolume pairs a rendered volume with the attachment that carries it +// into the runtime, so the two can never drift apart: every volume the spec +// declares must be attached at least once. +type plannedVolume struct { + volume computev1alpha.InstanceVolume + attachment computev1alpha.VolumeAttachment +} + +// plannedVolumes derives the instance's volumes from the input. The VM boot +// disk, when present, is always first: the webhook requires the first volume +// attachment of a VM to be a bootable one. +func plannedVolumes(in Input) []plannedVolume { + planned := make([]plannedVolume, 0, len(in.ConfigMounts)+1) + + if in.VM != nil { + planned = append(planned, plannedVolume{ + volume: computev1alpha.InstanceVolume{ + Name: BootVolumeName, + VolumeSource: computev1alpha.VolumeSource{ + Disk: &computev1alpha.DiskTemplateVolumeSource{ + Template: &computev1alpha.DiskTemplateVolumeSourceTemplate{ + Spec: computev1alpha.DiskSpec{ + Type: diskTypePDStandard, + Populator: &computev1alpha.DiskPopulator{ + Image: &computev1alpha.ImageDiskPopulator{ + Name: in.VM.BootImage, + }, + }, + }, + }, + }, + }, + }, + // No mount path: the boot disk is attached as the boot device. + attachment: computev1alpha.VolumeAttachment{Name: BootVolumeName}, + }) + } + + for _, m := range in.ConfigMounts { + name := mountVolumeName(m) + + var source computev1alpha.VolumeSource + switch { + case m.ConfigMap != "": + // A configMap volume names its source with `name`, while a secret + // volume names it with `secretName`. + source.ConfigMap = &corev1.ConfigMapVolumeSource{ + LocalObjectReference: corev1.LocalObjectReference{Name: m.ConfigMap}, + } + case m.Secret != "": + source.Secret = &corev1.SecretVolumeSource{SecretName: m.Secret} + default: + // Rejected by validate; skip so rendering stays total. + continue + } + + mountPath := m.MountPath + planned = append(planned, plannedVolume{ + volume: computev1alpha.InstanceVolume{Name: name, VolumeSource: source}, + attachment: computev1alpha.VolumeAttachment{Name: name, MountPath: &mountPath}, + }) + } + + return planned +} + +func mountVolumeName(m Mount) string { + switch { + case m.Name != "": + return m.Name + case m.ConfigMap != "": + return m.ConfigMap + default: + return m.Secret + } +} + +func buildTemplate(in Input, volumes []plannedVolume) computev1alpha.InstanceTemplateSpec { + template := computev1alpha.InstanceTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: copyLabels(in.Labels), + }, + Spec: computev1alpha.InstanceSpec{ + Runtime: computev1alpha.InstanceRuntimeSpec{ + // `requests` is left unset: adjustments to an instance type's + // resources are rejected as not implemented. + Resources: computev1alpha.InstanceRuntimeResources{ + InstanceType: in.InstanceType, + }, + Class: in.RuntimeClass, + }, + NetworkInterfaces: []computev1alpha.InstanceNetworkInterface{ + buildNetworkInterface(in), + }, + }, + } + + if in.VM != nil { + template.Annotations = map[string]string{ + computev1alpha.SSHKeysAnnotation: strings.Join(in.VM.SSHKeys, "\n"), + } + template.Spec.Runtime.VirtualMachine = &computev1alpha.VirtualMachineRuntime{ + VolumeAttachments: attachments(volumes), + Ports: buildPorts(in.Ports), + } + } else { + template.Spec.Runtime.Sandbox = &computev1alpha.SandboxRuntime{ + Containers: []computev1alpha.SandboxContainer{buildContainer(in, volumes)}, + } + } + + for _, v := range volumes { + template.Spec.Volumes = append(template.Spec.Volumes, v.volume) + } + + return template +} + +func buildContainer(in Input, volumes []plannedVolume) computev1alpha.SandboxContainer { + return computev1alpha.SandboxContainer{ + Name: ContainerName, + Image: in.Image, + // `resources` is left unset: per-container resource requirements are + // rejected as not implemented, and the instance type carries the shape. + Env: buildEnv(in.Env), + Ports: buildPorts(in.Ports), + VolumeAttachments: attachments(volumes), + } +} + +func attachments(volumes []plannedVolume) []computev1alpha.VolumeAttachment { + if len(volumes) == 0 { + return nil + } + out := make([]computev1alpha.VolumeAttachment, 0, len(volumes)) + for _, v := range volumes { + out = append(out, v.attachment) + } + return out +} + +func buildNetworkInterface(in Input) computev1alpha.InstanceNetworkInterface { + iface := computev1alpha.InstanceNetworkInterface{ + Network: networkingv1alpha.NetworkRef{Name: in.Network}, + } + + if in.PublicIPv4 { + iface.IPFamilies = []networkingv1alpha.IPFamily{ + networkingv1alpha.IPv4Protocol, + networkingv1alpha.IPv6Protocol, + } + iface.Addresses = []computev1alpha.InstanceNetworkInterfaceAddressRequest{ + {Class: PublicIPv4Class}, + } + } + + if ingress := buildIngressRules(in.Ports); len(ingress) > 0 { + iface.NetworkPolicy = &computev1alpha.InstanceNetworkInterfaceNetworkPolicy{ + Ingress: ingress, + } + } + + return iface +} + +func buildIngressRules(ports []Port) []networkingv1alpha.NetworkPolicyIngressRule { + if len(ports) == 0 { + return nil + } + + rules := make([]networkingv1alpha.NetworkPolicyIngressRule, 0, len(ports)) + for _, p := range ports { + protocol := protocolOrDefault(p.Protocol) + port := intstr.FromInt32(p.Port) + rules = append(rules, networkingv1alpha.NetworkPolicyIngressRule{ + Ports: []networkingv1alpha.NetworkPolicyPort{ + {Protocol: &protocol, Port: &port}, + }, + From: []networkingv1alpha.NetworkPolicyPeer{ + {IPBlock: &networkingv1alpha.IPBlock{CIDR: anyIPv4CIDR}}, + }, + }) + } + return rules +} + +func buildPorts(ports []Port) []computev1alpha.NamedPort { + if len(ports) == 0 { + return nil + } + + out := make([]computev1alpha.NamedPort, 0, len(ports)) + for _, p := range ports { + protocol := protocolOrDefault(p.Protocol) + out = append(out, computev1alpha.NamedPort{ + Name: p.Name, + Port: p.Port, + Protocol: &protocol, + }) + } + return out +} + +func buildEnv(env []EnvVar) []corev1.EnvVar { + if len(env) == 0 { + return nil + } + + out := make([]corev1.EnvVar, 0, len(env)) + for _, e := range env { + v := corev1.EnvVar{Name: e.Name} + switch { + case e.ConfigMapKeyRef != nil: + v.ValueFrom = &corev1.EnvVarSource{ + ConfigMapKeyRef: &corev1.ConfigMapKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: e.ConfigMapKeyRef.Name}, + Key: e.ConfigMapKeyRef.Key, + }, + } + case e.SecretKeyRef != nil: + v.ValueFrom = &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: e.SecretKeyRef.Name}, + Key: e.SecretKeyRef.Key, + }, + } + default: + v.Value = e.Value + } + out = append(out, v) + } + return out +} + +func buildPlacements(placements []Placement) []computev1alpha.WorkloadPlacement { + out := make([]computev1alpha.WorkloadPlacement, 0, len(placements)) + for _, p := range placements { + // Exactly one of the two is emitted: the API rejects a placement + // carrying both, and validate has already refused an input with both. + var refs []locationsv1alpha1.LocationReference + selector := p.LocationSelector + if len(p.Locations) > 0 { + refs = make([]locationsv1alpha1.LocationReference, 0, len(p.Locations)) + for _, name := range p.Locations { + refs = append(refs, locationsv1alpha1.LocationReference{Name: name}) + } + selector = nil + } + + out = append(out, computev1alpha.WorkloadPlacement{ + Name: p.Name, + Locations: refs, + LocationSelector: selector.DeepCopy(), + ScaleSettings: computev1alpha.HorizontalScaleSettings{ + MinReplicas: p.MinReplicas, + // maxReplicas is left unset: it requires scaling metrics, which + // this input does not describe. + InstanceManagementPolicy: computev1alpha.OrderedReadyInstanceManagementPolicyType, + }, + }) + } + return out +} + +func protocolOrDefault(p corev1.Protocol) corev1.Protocol { + if p == "" { + return corev1.ProtocolTCP + } + return p +} + +func copyLabels(labels map[string]string) map[string]string { + if len(labels) == 0 { + return nil + } + out := make(map[string]string, len(labels)) + for k, v := range labels { + out[k] = v + } + return out +} + +// withDefaults returns a copy of in with every defaultable field filled in. It +// is idempotent, so calling it twice is harmless. +func withDefaults(in Input) Input { + if in.InstanceType == "" { + in.InstanceType = DefaultInstanceType + } + if in.Network == "" { + in.Network = DefaultNetwork + } + + placements := make([]Placement, len(in.Placements)) + copy(placements, in.Placements) + for i := range placements { + if placements[i].Name == "" { + placements[i].Name = DefaultPlacementName + } + if placements[i].MinReplicas == 0 { + placements[i].MinReplicas = DefaultMinReplicas + } + } + in.Placements = placements + + if in.VM != nil { + vm := *in.VM + if vm.BootImage == "" { + vm.BootImage = DefaultBootImage + } + in.VM = &vm + } + + return in +} + +// MarshalYAML renders a workload as the YAML a user would commit. Status and +// the null creationTimestamp the object meta always carries are dropped, since +// neither is input to an apply. +func MarshalYAML(w *computev1alpha.Workload) ([]byte, error) { + if w == nil { + return nil, fmt.Errorf("workload is nil") + } + + raw, err := json.Marshal(w) + if err != nil { + return nil, fmt.Errorf("marshalling workload: %w", err) + } + + var doc map[string]any + if err := json.Unmarshal(raw, &doc); err != nil { + return nil, fmt.Errorf("normalizing workload: %w", err) + } + delete(doc, "status") + pruneNulls(doc) + + data, err := sigsyaml.Marshal(doc) + if err != nil { + return nil, fmt.Errorf("marshalling workload: %w", err) + } + return data, nil +} + +// pruneNulls removes explicit nulls, which the Kubernetes object meta emits for +// creationTimestamp at every level of a manifest. +func pruneNulls(node any) { + switch n := node.(type) { + case map[string]any: + for k, v := range n { + if v == nil { + delete(n, k) + continue + } + pruneNulls(v) + } + case []any: + for _, v := range n { + pruneNulls(v) + } + } +} + +func validate(in Input, volumes []plannedVolume) field.ErrorList { + allErrs := validateName(in.Name) + + allErrs = append(allErrs, validateRuntime(in)...) + allErrs = append(allErrs, validateNetwork(in.Network)...) + allErrs = append(allErrs, validatePlacements(in.Placements)...) + allErrs = append(allErrs, validatePorts(in.Ports)...) + allErrs = append(allErrs, validateEnv(in.Env)...) + allErrs = append(allErrs, validateMounts(in.ConfigMounts, volumes)...) + + return allErrs +} + +func validateName(name string) field.ErrorList { + allErrs := field.ErrorList{} + namePath := field.NewPath("name") + + if name == "" { + return append(allErrs, field.Required(namePath, "a workload name is required")) + } + for _, msg := range apimachineryvalidation.NameIsDNSSubdomain(name, false) { + allErrs = append(allErrs, field.Invalid(namePath, name, msg)) + } + return allErrs +} + +// validateRuntime enforces the "exactly one of sandbox or virtualMachine" +// rule at the input level, where the caller can still act on it. +func validateRuntime(in Input) field.ErrorList { + allErrs := field.ErrorList{} + + if in.VM == nil { + if in.Image == "" { + allErrs = append(allErrs, field.Required(field.NewPath("image"), + "a container image is required for a sandbox workload; set vm to render a virtual machine instead")) + } + return allErrs + } + + if in.Image != "" { + allErrs = append(allErrs, field.Forbidden(field.NewPath("image"), + "a virtual machine boots from vm.bootImage, not from a container image")) + } + if len(in.Env) > 0 { + allErrs = append(allErrs, field.Forbidden(field.NewPath("env"), + "a virtual machine has no container to set environment variables on")) + } + + return append(allErrs, validateSSHKeys(in.VM.SSHKeys)...) +} + +// validateSSHKeys mirrors the webhook's parsing of the ssh-keys annotation: +// one "username:key" pair per line, with a parseable public key. +func validateSSHKeys(keys []string) field.ErrorList { + allErrs := field.ErrorList{} + keysPath := field.NewPath("vm", "sshKeys") + + if len(keys) == 0 { + return append(allErrs, field.Required(keysPath, + "a virtual machine requires at least one 'username:ssh-public-key' entry")) + } + + for i, k := range keys { + keyPath := keysPath.Index(i) + + user, key, found := strings.Cut(k, ":") + if !found { + allErrs = append(allErrs, field.Invalid(keyPath, k, "must be in the format 'username:key'")) + continue + } + if user == "" { + allErrs = append(allErrs, field.Required(keyPath, "must provide a username")) + } + if strings.Contains(k, "\n") { + allErrs = append(allErrs, field.Invalid(keyPath, k, "must not contain a newline; provide one entry per key")) + continue + } + if _, _, _, _, err := ssh.ParseAuthorizedKey([]byte(key)); err != nil { + allErrs = append(allErrs, field.Invalid(keyPath, key, "must be a valid SSH public key")) + } + } + + return allErrs +} + +func validateNetwork(network string) field.ErrorList { + networkPath := field.NewPath("network") + + msgs := apimachineryvalidation.NameIsDNSLabel(network, false) + allErrs := make(field.ErrorList, 0, len(msgs)) + for _, msg := range msgs { + allErrs = append(allErrs, field.Invalid(networkPath, network, msg)) + } + return allErrs +} + +func validatePlacements(placements []Placement) field.ErrorList { + allErrs := field.ErrorList{} + placementsPath := field.NewPath("placements") + + if len(placements) == 0 { + return append(allErrs, field.Required(placementsPath, "at least one placement is required")) + } + + names := sets.Set[string]{} + for i, p := range placements { + path := placementsPath.Index(i) + + namePath := path.Child("name") + for _, msg := range apimachineryvalidation.NameIsDNSLabel(p.Name, false) { + allErrs = append(allErrs, field.Invalid(namePath, p.Name, msg)) + } + if names.Has(p.Name) { + allErrs = append(allErrs, field.Duplicate(namePath, p.Name)) + } else { + names.Insert(p.Name) + } + + allErrs = append(allErrs, validatePlacementLocations(p, path)...) + + minPath := path.Child("minReplicas") + if p.MinReplicas < 0 { + allErrs = append(allErrs, field.Invalid(minPath, p.MinReplicas, "must be greater than 0")) + } else if p.MinReplicas > 1000 { + allErrs = append(allErrs, field.Invalid(minPath, p.MinReplicas, "must be less than or equal to 1000")) + } + } + + return allErrs +} + +// validatePlacementLocations enforces the API's "exactly one of locations or +// locationSelector" rule at the input level, where the caller can still act on +// it, and refuses an empty selector for the same reason the API does: it is +// not read as matching every location. +func validatePlacementLocations(p Placement, path *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + locationsPath := path.Child("locations") + selectorPath := path.Child("locationSelector") + + switch { + case len(p.Locations) == 0 && p.LocationSelector == nil: + return append(allErrs, field.Required(locationsPath, + "name at least one location, or set locationSelector to place at every location matching a topology")) + case len(p.Locations) > 0 && p.LocationSelector != nil: + return append(allErrs, field.Forbidden(selectorPath, + "may not be set together with locations; name locations or select them, not both")) + case p.LocationSelector != nil: + if len(p.LocationSelector.MatchLabels) == 0 && len(p.LocationSelector.MatchExpressions) == 0 { + return append(allErrs, field.Required(selectorPath, + "an empty selector is not read as matching every location; select at least one topology key, such as "+ + locationsv1alpha1.TopologyCityCodeKey)) + } + if _, err := metav1.LabelSelectorAsSelector(p.LocationSelector); err != nil { + allErrs = append(allErrs, field.Invalid(selectorPath, p.LocationSelector, err.Error())) + } + return allErrs + } + + seen := sets.Set[string]{} + for i, name := range p.Locations { + namePath := locationsPath.Index(i) + if name == "" { + allErrs = append(allErrs, field.Required(namePath, "a location name is required")) + continue + } + for _, msg := range apimachineryvalidation.NameIsDNSSubdomain(name, false) { + allErrs = append(allErrs, field.Invalid(namePath, name, msg)) + } + if seen.Has(name) { + allErrs = append(allErrs, field.Duplicate(namePath, name)) + } + seen.Insert(name) + } + return allErrs +} + +func validatePorts(ports []Port) field.ErrorList { + allErrs := field.ErrorList{} + portsPath := field.NewPath("ports") + + names := sets.Set[string]{} + for i, p := range ports { + path := portsPath.Index(i) + + namePath := path.Child("name") + if p.Name == "" { + allErrs = append(allErrs, field.Required(namePath, "")) + } else { + for _, msg := range utilvalidation.IsValidPortName(p.Name) { + allErrs = append(allErrs, field.Invalid(namePath, p.Name, msg)) + } + if names.Has(p.Name) { + allErrs = append(allErrs, field.Duplicate(namePath, p.Name)) + } else { + names.Insert(p.Name) + } + } + + for _, msg := range utilvalidation.IsValidPortNum(int(p.Port)) { + allErrs = append(allErrs, field.Invalid(path.Child("port"), p.Port, msg)) + } + + switch p.Protocol { + case "", corev1.ProtocolTCP, corev1.ProtocolUDP, corev1.ProtocolSCTP: + default: + allErrs = append(allErrs, field.NotSupported(path.Child("protocol"), p.Protocol, + []string{string(corev1.ProtocolTCP), string(corev1.ProtocolUDP), string(corev1.ProtocolSCTP)})) + } + } + + return allErrs +} + +func validateEnv(env []EnvVar) field.ErrorList { + allErrs := field.ErrorList{} + envPath := field.NewPath("env") + + names := sets.Set[string]{} + for i, e := range env { + path := envPath.Index(i) + + namePath := path.Child("name") + if e.Name == "" { + allErrs = append(allErrs, field.Required(namePath, "")) + } else { + for _, msg := range utilvalidation.IsCIdentifier(e.Name) { + allErrs = append(allErrs, field.Invalid(namePath, e.Name, msg)) + } + if names.Has(e.Name) { + allErrs = append(allErrs, field.Duplicate(namePath, e.Name)) + } else { + names.Insert(e.Name) + } + } + + sources := 0 + if e.Value != "" { + sources++ + } + if e.ConfigMapKeyRef != nil { + sources++ + allErrs = append(allErrs, validateKeyRef(*e.ConfigMapKeyRef, path.Child("configMapKeyRef"))...) + } + if e.SecretKeyRef != nil { + sources++ + allErrs = append(allErrs, validateKeyRef(*e.SecretKeyRef, path.Child("secretKeyRef"))...) + } + if sources > 1 { + allErrs = append(allErrs, field.Forbidden(path, + "may not specify more than one of value, configMapKeyRef, or secretKeyRef")) + } + } + + return allErrs +} + +func validateKeyRef(ref KeyRef, path *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + + namePath := path.Child("name") + if ref.Name == "" { + allErrs = append(allErrs, field.Required(namePath, "")) + } else { + for _, msg := range apimachineryvalidation.NameIsDNSSubdomain(ref.Name, false) { + allErrs = append(allErrs, field.Invalid(namePath, ref.Name, msg)) + } + } + + if ref.Key == "" { + allErrs = append(allErrs, field.Required(path.Child("key"), "")) + } + + return allErrs +} + +func validateMounts(mounts []Mount, volumes []plannedVolume) field.ErrorList { + allErrs := field.ErrorList{} + mountsPath := field.NewPath("configMounts") + + names := sets.Set[string]{} + // The boot volume claims its name before any mount can. + for _, v := range volumes { + if v.volume.Disk != nil { + names.Insert(v.volume.Name) + } + } + + paths := sets.Set[string]{} + for i, m := range mounts { + path := mountsPath.Index(i) + + if (m.ConfigMap == "") == (m.Secret == "") { + allErrs = append(allErrs, field.Required(path, "must specify exactly one of configMap or secret")) + } + + namePath := path.Child("name") + name := mountVolumeName(m) + if name != "" { + for _, msg := range apimachineryvalidation.NameIsDNSLabel(name, false) { + allErrs = append(allErrs, field.Invalid(namePath, name, msg)) + } + if names.Has(name) { + allErrs = append(allErrs, field.Duplicate(namePath, name)) + } else { + names.Insert(name) + } + } + + mountPath := path.Child("mountPath") + if m.MountPath == "" { + allErrs = append(allErrs, field.Required(mountPath, "")) + } else if paths.Has(m.MountPath) { + allErrs = append(allErrs, field.Duplicate(mountPath, m.MountPath)) + } else { + paths.Insert(m.MountPath) + } + } + + return allErrs +} diff --git a/internal/workloadspec/render_test.go b/internal/workloadspec/render_test.go new file mode 100644 index 00000000..c9f8e40d --- /dev/null +++ b/internal/workloadspec/render_test.go @@ -0,0 +1,685 @@ +// SPDX-License-Identifier: AGPL-3.0-only + +package workloadspec + +import ( + "context" + "strings" + "testing" + + "github.com/google/go-cmp/cmp" + authorizationv1 "k8s.io/api/authorization/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + k8sruntime "k8s.io/apimachinery/pkg/runtime" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" + + computev1alpha "go.datum.net/compute/api/v1alpha" + "go.datum.net/compute/internal/validation" + networkingv1alpha "go.datum.net/network-services-operator/api/v1alpha" + locationsv1alpha1 "go.miloapis.com/locations/api/v1alpha1" +) + +const ( + testSSHKey = "user:ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAILPbDbsv9fgEnam9iJ5b51Na/WieeiKCJRC0+m7fRwPk vscode@42aafaf8293e" + testCityCode = "DFW" + testLocation = "us-south-dfw-1" + testSelectorPath = "placements[0].locationSelector" + testLocationB = "us-south-dfw-2" + testImage = "ghcr.io/acme/api:1.4.2" + testWorkload = "api" + testPlacement = "us" + testMountPath = "/etc/app" + testCredsPath = "/etc/creds" + testConfigMap = "app-config" + testSecretName = "db-creds" + testPortName = "http" + testEnvLiteral = "LOG_LEVEL" + testEnvFromSecre = "DB_PASSWORD" + testSharedName = "shared" + testSSHKeysPath = "vm.sshKeys[0]" + testEnvValue = "debug" + testSecretKey = "password" + testLabelValue = "frontend" +) + +// validInput is the smallest Input that renders: a sandbox in one placement. +func validInput(tweaks ...func(*Input)) Input { + in := Input{ + Name: testWorkload, + Image: testImage, + Placements: []Placement{ + {Name: testPlacement, Locations: []string{testLocation}, MinReplicas: 2}, + }, + } + for _, tweak := range tweaks { + tweak(&in) + } + return in +} + +// cityCodeSelector is the selector that places at every location in a city, +// which is the form the CLI's --city shorthand and the deprecated cityCodes +// field both resolve to. +func cityCodeSelector(cityCode string) *metav1.LabelSelector { + return &metav1.LabelSelector{ + MatchLabels: map[string]string{locationsv1alpha1.TopologyCityCodeKey: cityCode}, + } +} + +func mustRender(t *testing.T, in Input) *computev1alpha.Workload { + t.Helper() + + w, err := Render(in) + if err != nil { + t.Fatalf("Render() error: %v", err) + } + return w +} + +// vmInput turns a sandbox input into the equivalent VM input. +func vmInput(tweaks ...func(*Input)) Input { + return validInput(append([]func(*Input){func(in *Input) { + in.Image = "" + in.VM = &VMInput{SSHKeys: []string{testSSHKey}} + }}, tweaks...)...) +} + +func TestRenderMinimalSandbox(t *testing.T) { + w := mustRender(t, validInput()) + + if got, want := w.APIVersion, computev1alpha.GroupVersion.String(); got != want { + t.Errorf("apiVersion = %q, want %q", got, want) + } + if got, want := w.Kind, "Workload"; got != want { + t.Errorf("kind = %q, want %q", got, want) + } + if got, want := w.Namespace, Namespace; got != want { + t.Errorf("namespace = %q, want %q", got, want) + } + + spec := w.Spec.Template.Spec + if got, want := spec.Runtime.Resources.InstanceType, DefaultInstanceType; got != want { + t.Errorf("instanceType = %q, want %q", got, want) + } + if spec.Runtime.Resources.Requests != nil { + t.Error("runtime.resources.requests must stay unset: the webhook rejects it as not implemented") + } + if spec.Runtime.VirtualMachine != nil { + t.Error("virtualMachine set on a sandbox render") + } + + containers := spec.Runtime.Sandbox.Containers + if len(containers) != 1 { + t.Fatalf("containers = %d, want 1", len(containers)) + } + if got, want := containers[0].Image, testImage; got != want { + t.Errorf("image = %q, want %q", got, want) + } + if containers[0].Resources != nil { + t.Error("containers[0].resources must stay unset: the webhook rejects it as not implemented") + } + + placements := w.Spec.Placements + if len(placements) != 1 { + t.Fatalf("placements = %d, want 1", len(placements)) + } + if got, want := placements[0].ScaleSettings.MinReplicas, int32(2); got != want { + t.Errorf("minReplicas = %d, want %d", got, want) + } + if got, want := placements[0].ScaleSettings.InstanceManagementPolicy, + computev1alpha.OrderedReadyInstanceManagementPolicyType; got != want { + t.Errorf("instanceManagementPolicy = %q, want %q", got, want) + } +} + +func TestRenderNetworkInterfaceLeavesImmutableFieldsDefaulted(t *testing.T) { + spec := mustRender(t, validInput()).Spec.Template.Spec + + if len(spec.NetworkInterfaces) != 1 { + t.Fatalf("networkInterfaces = %d, want exactly 1", len(spec.NetworkInterfaces)) + } + + iface := spec.NetworkInterfaces[0] + if got, want := iface.Network.Name, DefaultNetwork; got != want { + t.Errorf("network = %q, want %q", got, want) + } + if iface.Name != "" || iface.IPFamilies != nil || iface.Addresses != nil || iface.ReclaimPolicy != "" { + t.Errorf("interface should leave create-time-only fields to the API server, got %+v", iface) + } + if iface.NetworkPolicy != nil { + t.Error("no ports were requested, so no network policy should be rendered") + } +} + +func TestRenderDefaults(t *testing.T) { + w := mustRender(t, Input{ + Name: testWorkload, + Image: testImage, + Placements: []Placement{{Locations: []string{testLocation}}}, + }) + + p := w.Spec.Placements[0] + if got, want := p.Name, DefaultPlacementName; got != want { + t.Errorf("placement name = %q, want %q", got, want) + } + if got, want := p.ScaleSettings.MinReplicas, DefaultMinReplicas; got != want { + t.Errorf("minReplicas = %d, want %d", got, want) + } + if got, want := w.Spec.Template.Spec.NetworkInterfaces[0].Network.Name, DefaultNetwork; got != want { + t.Errorf("network = %q, want %q", got, want) + } +} + +func TestRenderPortsOpenIngress(t *testing.T) { + spec := mustRender(t, validInput(func(in *Input) { + in.Ports = []Port{ + {Name: testPortName, Port: 8080}, + {Name: "dns", Port: 53, Protocol: corev1.ProtocolUDP}, + } + })).Spec.Template.Spec + + ports := spec.Runtime.Sandbox.Containers[0].Ports + if len(ports) != 2 { + t.Fatalf("container ports = %d, want 2", len(ports)) + } + if got, want := *ports[0].Protocol, corev1.ProtocolTCP; got != want { + t.Errorf("ports[0].protocol = %q, want %q (the default)", got, want) + } + if got, want := *ports[1].Protocol, corev1.ProtocolUDP; got != want { + t.Errorf("ports[1].protocol = %q, want %q", got, want) + } + + policy := spec.NetworkInterfaces[0].NetworkPolicy + if policy == nil { + t.Fatal("declared ports must open matching ingress rules") + } + if len(policy.Ingress) != 2 { + t.Fatalf("ingress rules = %d, want 2", len(policy.Ingress)) + } + if got, want := policy.Ingress[0].Ports[0].Port.IntValue(), 8080; got != want { + t.Errorf("ingress[0] port = %d, want %d", got, want) + } + if got, want := policy.Ingress[0].From[0].IPBlock.CIDR, anyIPv4CIDR; got != want { + t.Errorf("ingress[0] cidr = %q, want %q", got, want) + } +} + +func TestRenderEnv(t *testing.T) { + spec := mustRender(t, validInput(func(in *Input) { + in.Env = []EnvVar{ + {Name: testEnvLiteral, Value: testEnvValue}, + {Name: testEnvFromSecre, SecretKeyRef: &KeyRef{Name: testSecretName, Key: testSecretKey}}, + {Name: "REGION", ConfigMapKeyRef: &KeyRef{Name: testConfigMap, Key: "region"}}, + } + })).Spec.Template.Spec + + want := []corev1.EnvVar{ + {Name: testEnvLiteral, Value: testEnvValue}, + { + Name: testEnvFromSecre, + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: testSecretName}, + Key: testSecretKey, + }, + }, + }, + { + Name: "REGION", + ValueFrom: &corev1.EnvVarSource{ + ConfigMapKeyRef: &corev1.ConfigMapKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: testConfigMap}, + Key: "region", + }, + }, + }, + } + if delta := cmp.Diff(want, spec.Runtime.Sandbox.Containers[0].Env); delta != "" { + t.Errorf("env mismatch (-want +got):\n%s", delta) + } +} + +func TestRenderConfigMountsAreAttached(t *testing.T) { + spec := mustRender(t, validInput(func(in *Input) { + in.ConfigMounts = []Mount{ + {ConfigMap: testConfigMap, MountPath: testMountPath}, + {Secret: testSecretName, MountPath: testCredsPath}, + } + })).Spec.Template.Spec + + if len(spec.Volumes) != 2 { + t.Fatalf("volumes = %d, want 2", len(spec.Volumes)) + } + // A configMap volume names its source with `name`, a secret volume with + // `secretName`. + if got, want := spec.Volumes[0].ConfigMap.Name, testConfigMap; got != want { + t.Errorf("configMap volume name = %q, want %q", got, want) + } + if got, want := spec.Volumes[1].Secret.SecretName, testSecretName; got != want { + t.Errorf("secret volume secretName = %q, want %q", got, want) + } + + attachments := spec.Runtime.Sandbox.Containers[0].VolumeAttachments + if len(attachments) != 2 { + t.Fatalf("volumeAttachments = %d, want 2", len(attachments)) + } + for i, a := range attachments { + if a.Name != spec.Volumes[i].Name { + t.Errorf("attachment %d = %q, does not match volume %q", i, a.Name, spec.Volumes[i].Name) + } + if a.MountPath == nil { + t.Fatalf("attachment %d has no mount path", i) + } + } + if got, want := *attachments[0].MountPath, testMountPath; got != want { + t.Errorf("mountPath = %q, want %q", got, want) + } +} + +func TestRenderExplicitMountNameResolvesCollision(t *testing.T) { + volumes := mustRender(t, validInput(func(in *Input) { + in.ConfigMounts = []Mount{ + {ConfigMap: testSharedName, MountPath: testMountPath}, + {Name: testSharedName + "-secret", Secret: testSharedName, MountPath: testCredsPath}, + } + })).Spec.Template.Spec.Volumes + + if got, want := volumes[0].Name, testSharedName; got != want { + t.Errorf("volumes[0].name = %q, want %q", got, want) + } + if got, want := volumes[1].Name, testSharedName+"-secret"; got != want { + t.Errorf("volumes[1].name = %q, want %q", got, want) + } +} + +func TestRenderPublicIPv4(t *testing.T) { + iface := mustRender(t, validInput(func(in *Input) { + in.PublicIPv4 = true + })).Spec.Template.Spec.NetworkInterfaces[0] + + wantFamilies := []networkingv1alpha.IPFamily{ + networkingv1alpha.IPv4Protocol, + networkingv1alpha.IPv6Protocol, + } + if delta := cmp.Diff(wantFamilies, iface.IPFamilies); delta != "" { + t.Errorf("ipFamilies mismatch (-want +got):\n%s", delta) + } + + wantAddresses := []computev1alpha.InstanceNetworkInterfaceAddressRequest{{Class: PublicIPv4Class}} + if delta := cmp.Diff(wantAddresses, iface.Addresses); delta != "" { + t.Errorf("addresses mismatch (-want +got):\n%s", delta) + } +} + +func TestRenderLabelsReachTheInstanceTemplate(t *testing.T) { + w := mustRender(t, validInput(func(in *Input) { + in.Labels = map[string]string{"tier": testLabelValue} + })) + + if got, want := w.Labels["tier"], testLabelValue; got != want { + t.Errorf("workload label = %q, want %q", got, want) + } + if got, want := w.Spec.Template.Labels["tier"], testLabelValue; got != want { + t.Errorf("template label = %q, want %q", got, want) + } +} + +func TestRenderVM(t *testing.T) { + template := mustRender(t, vmInput(func(in *Input) { + in.Ports = []Port{{Name: "ssh", Port: 22}} + in.ConfigMounts = []Mount{{Secret: testSecretName, MountPath: testCredsPath}} + })).Spec.Template + + if got, want := template.Annotations[computev1alpha.SSHKeysAnnotation], testSSHKey; got != want { + t.Errorf("ssh-keys annotation = %q, want %q", got, want) + } + + spec := template.Spec + if spec.Runtime.Sandbox != nil { + t.Error("sandbox set on a VM render") + } + vm := spec.Runtime.VirtualMachine + if vm == nil { + t.Fatal("virtualMachine not rendered") + } + if len(vm.Ports) != 1 { + t.Errorf("vm ports = %d, want 1", len(vm.Ports)) + } + + // The webhook requires the first attachment to be a bootable volume: a + // disk with an image populator. + if len(vm.VolumeAttachments) != 2 { + t.Fatalf("volumeAttachments = %d, want 2", len(vm.VolumeAttachments)) + } + boot := vm.VolumeAttachments[0] + if got, want := boot.Name, BootVolumeName; got != want { + t.Errorf("first attachment = %q, want %q", got, want) + } + if boot.MountPath != nil { + t.Error("the boot disk must be attached as a device, not mounted") + } + + bootVolume := spec.Volumes[0] + if bootVolume.Name != BootVolumeName { + t.Fatalf("volumes[0] = %q, want %q", bootVolume.Name, BootVolumeName) + } + if got, want := bootVolume.Disk.Template.Spec.Type, diskTypePDStandard; got != want { + t.Errorf("boot disk type = %q, want %q", got, want) + } + if got, want := bootVolume.Disk.Template.Spec.Populator.Image.Name, DefaultBootImage; got != want { + t.Errorf("boot image = %q, want %q", got, want) + } + // The image populator carries the size, so a storage request would be + // redundant. + if bootVolume.Disk.Template.Spec.Resources != nil { + t.Error("boot disk should take its size from the image populator") + } +} + +func TestRenderErrors(t *testing.T) { + cases := map[string]struct { + input Input + wantPath string + }{ + "no name": { + input: validInput(func(in *Input) { in.Name = "" }), + wantPath: "name", + }, + "invalid name": { + input: validInput(func(in *Input) { in.Name = "Not A Name" }), + wantPath: "name", + }, + "no image for a sandbox": { + input: validInput(func(in *Input) { in.Image = "" }), + wantPath: "image", + }, + "image on a vm": { + input: vmInput(func(in *Input) { in.Image = testImage }), + wantPath: "image", + }, + "no placements": { + input: validInput(func(in *Input) { in.Placements = nil }), + wantPath: "placements", + }, + "placement naming nowhere": { + input: validInput(func(in *Input) { in.Placements = []Placement{{Name: testPlacement}} }), + wantPath: "placements[0].locations", + }, + "placement with both locations and a selector": { + input: validInput(func(in *Input) { + in.Placements[0].LocationSelector = cityCodeSelector(testCityCode) + }), + wantPath: testSelectorPath, + }, + "empty location selector": { + input: validInput(func(in *Input) { + in.Placements[0].Locations = nil + in.Placements[0].LocationSelector = &metav1.LabelSelector{} + }), + wantPath: testSelectorPath, + }, + "malformed location selector": { + input: validInput(func(in *Input) { + in.Placements[0].Locations = nil + in.Placements[0].LocationSelector = &metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{{ + Key: locationsv1alpha1.TopologyCityCodeKey, Operator: metav1.LabelSelectorOpIn, + }}, + } + }), + wantPath: testSelectorPath, + }, + "duplicate locations in one placement": { + input: validInput(func(in *Input) { + in.Placements[0].Locations = []string{testLocation, testLocation} + }), + wantPath: "placements[0].locations[1]", + }, + "location name is not a DNS subdomain": { + input: validInput(func(in *Input) { in.Placements[0].Locations = []string{"Not A Location"} }), + wantPath: "placements[0].locations[0]", + }, + "placement name is not a DNS label": { + input: validInput(func(in *Input) { in.Placements[0].Name = "US East" }), + wantPath: "placements[0].name", + }, + "too many replicas": { + input: validInput(func(in *Input) { in.Placements[0].MinReplicas = 1001 }), + wantPath: "placements[0].minReplicas", + }, + "negative replicas": { + input: validInput(func(in *Input) { in.Placements[0].MinReplicas = -1 }), + wantPath: "placements[0].minReplicas", + }, + "vm without ssh keys": { + input: vmInput(func(in *Input) { in.VM.SSHKeys = nil }), + wantPath: "vm.sshKeys", + }, + "ssh key without a username": { + input: vmInput(func(in *Input) { + _, key, _ := strings.Cut(testSSHKey, ":") + in.VM.SSHKeys = []string{":" + key} + }), + wantPath: testSSHKeysPath, + }, + "ssh key without a username separator": { + input: vmInput(func(in *Input) { in.VM.SSHKeys = []string{"ssh-ed25519 AAAA"} }), + wantPath: testSSHKeysPath, + }, + "unparseable ssh key": { + input: vmInput(func(in *Input) { in.VM.SSHKeys = []string{"user:not-a-key"} }), + wantPath: testSSHKeysPath, + }, + "duplicate port name": { + input: validInput(func(in *Input) { + in.Ports = []Port{{Name: testPortName, Port: 80}, {Name: testPortName, Port: 8080}} + }), + wantPath: "ports[1].name", + }, + "port out of range": { + input: validInput(func(in *Input) { in.Ports = []Port{{Name: testPortName, Port: 70000}} }), + wantPath: "ports[0].port", + }, + "mount with neither configMap nor secret": { + input: validInput(func(in *Input) { + in.ConfigMounts = []Mount{{Name: "cfg", MountPath: testMountPath}} + }), + wantPath: "configMounts[0]", + }, + "mount without a mount path": { + input: validInput(func(in *Input) { in.ConfigMounts = []Mount{{ConfigMap: testConfigMap}} }), + wantPath: "configMounts[0].mountPath", + }, + "colliding volume names": { + input: validInput(func(in *Input) { + in.ConfigMounts = []Mount{ + {ConfigMap: testSharedName, MountPath: testMountPath}, + {Secret: testSharedName, MountPath: testCredsPath}, + } + }), + wantPath: "configMounts[1].name", + }, + "duplicate mount paths": { + input: validInput(func(in *Input) { + in.ConfigMounts = []Mount{ + {ConfigMap: testConfigMap, MountPath: testMountPath}, + {Secret: testSecretName, MountPath: testMountPath}, + } + }), + wantPath: "configMounts[1].mountPath", + }, + "env var with two sources": { + input: validInput(func(in *Input) { + in.Env = []EnvVar{{ + Name: testEnvLiteral, + Value: "literal", + SecretKeyRef: &KeyRef{Name: testSecretName, Key: "k"}, + }} + }), + wantPath: "env[0]", + }, + "env on a vm": { + input: vmInput(func(in *Input) { in.Env = []EnvVar{{Name: testEnvLiteral, Value: "b"}} }), + wantPath: "env", + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + w, err := Render(tc.input) + if err == nil { + t.Fatalf("Render() succeeded, want an error mentioning %q", tc.wantPath) + } + if w != nil { + t.Error("Render() returned a workload alongside an error") + } + if !strings.Contains(err.Error(), tc.wantPath) { + t.Errorf("error %q does not mention %q", err.Error(), tc.wantPath) + } + }) + } +} + +// TestRenderedManifestsPassAdmission runs rendered manifests through the real +// admission validation, so this package is held to the webhook's rules rather +// than to a copy of them. +func TestRenderedManifestsPassAdmission(t *testing.T) { + inputs := map[string]Input{ + "minimal sandbox": validInput(), + "sandbox with ports, env, mounts and a public address": validInput(func(in *Input) { + in.Ports = []Port{{Name: testPortName, Port: 8080}} + in.Env = []EnvVar{ + {Name: testEnvLiteral, Value: testEnvValue}, + {Name: testEnvFromSecre, SecretKeyRef: &KeyRef{Name: testSecretName, Key: testSecretKey}}, + } + in.ConfigMounts = []Mount{ + {ConfigMap: testConfigMap, MountPath: testMountPath}, + {Secret: testSecretName, MountPath: testCredsPath}, + } + in.PublicIPv4 = true + in.Labels = map[string]string{"tier": testLabelValue} + }), + "vm with ssh keys, a boot disk and mounts": vmInput(func(in *Input) { + in.Ports = []Port{{Name: "ssh", Port: 22}} + in.ConfigMounts = []Mount{{Secret: testSecretName, MountPath: testCredsPath}} + }), + "a placement that selects its locations by city": validInput(func(in *Input) { + in.Placements[0].Locations = nil + in.Placements[0].LocationSelector = cityCodeSelector(testCityCode) + }), + "multiple placements at the replica limits": validInput(func(in *Input) { + in.Placements = []Placement{ + {Name: testPlacement, Locations: []string{testLocation}, MinReplicas: 1}, + {Name: testPlacement + "-east", Locations: []string{testLocationB}, MinReplicas: 1000}, + } + }), + } + + for name, in := range inputs { + t.Run(name, func(t *testing.T) { + w := mustRender(t, in) + + opts := validation.WorkloadValidationOptions{ + Context: context.Background(), + Client: allowAllClient(t), + Workload: w, + // The locations the project may place at, and the topology a + // locationSelector is matched against. Both have to be given: + // admission rejects a name that is not entitled and a selector + // that matches nowhere. + ValidLocations: []string{testLocation, testLocationB}, + LocationTopologies: map[string]map[string]string{ + testLocation: {locationsv1alpha1.TopologyCityCodeKey: testCityCode}, + testLocationB: {locationsv1alpha1.TopologyCityCodeKey: testCityCode}, + }, + } + + if errs := validation.ValidateWorkloadCreate(w, opts); len(errs) > 0 { + t.Errorf("rendered manifest rejected by admission validation: %v", errs) + } + }) + } +} + +// allowAllClient returns a client that approves every SubjectAccessReview, the +// way the validation package's own tests stub authorization. +func allowAllClient(t *testing.T) client.Client { + t.Helper() + + scheme := k8sruntime.NewScheme() + utilruntime.Must(computev1alpha.AddToScheme(scheme)) + utilruntime.Must(networkingv1alpha.AddToScheme(scheme)) + utilruntime.Must(clientgoscheme.AddToScheme(scheme)) + + return fake.NewClientBuilder(). + WithScheme(scheme). + WithInterceptorFuncs(interceptor.Funcs{ + Create: func(ctx context.Context, c client.WithWatch, obj client.Object, opts ...client.CreateOption) error { + if sar, ok := obj.(*authorizationv1.SubjectAccessReview); ok { + // The fake client only accepts a create without a name when + // it can generate one. + sar.GenerateName = "sar-" + sar.Status.Allowed = true + } + return c.Create(ctx, obj, opts...) + }, + }). + WithObjects(&networkingv1alpha.Network{ + ObjectMeta: metav1.ObjectMeta{Namespace: Namespace, Name: DefaultNetwork}, + }). + Build() +} + +func TestDefaults(t *testing.T) { + d := Defaults() + if d.InstanceType != DefaultInstanceType || d.Network != DefaultNetwork { + t.Errorf("Defaults() = %+v, want the advertised instance type and network", d) + } + if len(d.Placements) != 1 || d.Placements[0].Name != DefaultPlacementName || + d.Placements[0].MinReplicas != DefaultMinReplicas { + t.Errorf("Defaults().Placements = %+v, want one default placement with one replica", d.Placements) + } + + // Defaults() is a starting point, not a renderable input on its own. + if _, err := Render(d); err == nil { + t.Error("Render(Defaults()) succeeded, want errors for the fields the caller must supply") + } +} + +func TestMarshalYAML(t *testing.T) { + w := mustRender(t, validInput(func(in *Input) { + in.Ports = []Port{{Name: testPortName, Port: 8080}} + })) + + data, err := MarshalYAML(w) + if err != nil { + t.Fatalf("MarshalYAML() error: %v", err) + } + out := string(data) + + for _, want := range []string{ + "apiVersion: compute.datumapis.com/v1alpha", + "kind: Workload", + "name: " + testWorkload, + "instanceType: " + DefaultInstanceType, + } { + if !strings.Contains(out, want) { + t.Errorf("rendered YAML is missing %q:\n%s", want, out) + } + } + + for _, unwanted := range []string{"status:", "creationTimestamp"} { + if strings.Contains(out, unwanted) { + t.Errorf("rendered YAML should not contain %q:\n%s", unwanted, out) + } + } + + if _, err := MarshalYAML(nil); err == nil { + t.Error("MarshalYAML(nil) succeeded, want an error") + } +}