From b8d8427cfea7a07b212e2c30023373a98e6b5289 Mon Sep 17 00:00:00 2001 From: NovusEdge Date: Mon, 10 Aug 2026 21:19:31 +0300 Subject: [PATCH 1/2] docs: update recipe spec with Fable review feedback - Item 1 now includes cloudinit v2 conversion (coupled, not independent) - Item 4 (stage field) reduced to validation-only rejection - Item 2 adds dependency satisfaction rules and apply-time cycle check - Item 3 moves up in priority (fixes broken backend) - Migration section fixed (sweepV1 already runs) - Added missing files-to-modify for ErrAppliedAtBoot removal - dry-run works on stopped VMs - Hash discovery note added --- .../2026-08-10-recipe-system-fixes-design.md | 283 ++++++++++-------- 1 file changed, 152 insertions(+), 131 deletions(-) diff --git a/docs/specs/2026-08-10-recipe-system-fixes-design.md b/docs/specs/2026-08-10-recipe-system-fixes-design.md index 5f8597b..e091a73 100644 --- a/docs/specs/2026-08-10-recipe-system-fixes-design.md +++ b/docs/specs/2026-08-10-recipe-system-fixes-design.md @@ -4,30 +4,32 @@ Status: **draft** ## Summary -This spec addresses six architectural issues in the recipe system. Each fix can be implemented independently, though the order below minimizes rework. +This spec addresses six architectural issues in the recipe system. Items 1 and the cloudinit v2 conversion are coupled; the rest can land independently. ## Priority Order -1. Delete v1 metadata parser (foundational) +1. Delete v1 metadata parser + wire cloudinit to v2 scripts (coupled) 2. Add dependency ordering -3. Add dry-run -4. Implement stage field +3. Cloudinit post-boot path (markers + remove ErrAppliedAtBoot) +4. Add dry-run 5. Document single-reboot behavior -6. Add cloudinit post-boot path +6. Stage field validation (reject until BYO ISO lands) --- -## 1. Delete v1 Metadata Parser +## 1. Delete v1 Metadata Parser + Cloudinit v2 Conversion -**Can implement independently:** Yes +**Can implement independently:** No — cloudinit's `Prepare()` and `clone.go` call `recipes.Read()` on v2 directory names, which fails. These must be fixed together. ### Problem -Two metadata systems coexist: `metadata.go` parses `# stoat: key value` comment front matter from v1 flat files, `manifest.go` parses `recipe.toml` from v2 directories. Both define `OS`, `Requires`, and capability checking with different type shapes and resolution logic. A bugfix to capability resolution must land in two places. +Two metadata systems coexist: `metadata.go` parses `# stoat: key value` comment front matter from v1 flat files, `manifest.go` parses `recipe.toml` from v2 directories. Both define `OS`, `Requires`, and capability checking with different type shapes and resolution logic. + +Additionally, the cloudinit backend is already broken for v2 recipes. `cloudinit.Prepare()` calls `recipes.Read("xfce")`, which does `os.ReadFile` on a directory and errors. The v2-aware `cloudinit.WrapScripts()` exists but has zero non-test callers. ### Solution -Delete the v1 path. Make `Manifest` the single source of truth. +Delete the v1 path, wire cloudinit to v2 scripts, and make `Manifest` the single source of truth. ### Files to Delete @@ -39,31 +41,37 @@ Delete the v1 path. Make `Manifest` the single source of truth. **`internal/core/apply.go`:** - Remove `ReadMetadata` calls - Use `ManifestFor` exclusively +- Remove cloud-fragment branches in `recipeIssueReason` (no longer applicable) **`internal/recipes/recipes.go`:** - Remove v1 fallback paths in `List`, `ScriptBody` -- `Read(name)` becomes unused; delete or deprecate +- Delete `Read(name)` — no longer used + +**`internal/backend/cloudinit/cloudinit.go`:** +- Replace `recipes.Read()` calls with `recipes.ManifestFor()` + `ScriptContent()` +- Wire `WrapScripts()` into `Prepare()` + +**`internal/core/clone.go`:** +- Same conversion: `ManifestFor()` + `ScriptContent()` instead of `Read()` ### Migration -User-created `.sh` files in `~/.stoat/recipes/` become unrecognized after this change. +`sweepV1()` already moves flat files into `.v1-removed/` at Install time. User-created flat files are swept before any warning could fire. -Add a startup warning if flat files exist outside `.v1-removed/`: +To catch user recipes being swept: log a warning *when sweepV1 moves a file* that the manifest doesn't recognize as stoat's own copy: ``` -Warning: Found legacy recipe files in ~/.stoat/recipes/. Convert to v2 format. See docs/writing-recipes.md. +Warning: Moved legacy recipe to .v1-removed/. Convert to v2 format. See docs/writing-recipes.md. ``` -The `.v1-removed/` attic already holds swept files from the v1→v2 migration. This change completes the deprecation period. - ### Result -`MatchesVM` becomes the only capability checker. `UnsupportedReason` (v1 version) goes away. +`MatchesVM` becomes the only capability checker. `UnsupportedReason` (v1 version) goes away. Cloudinit backend works with v2 recipes. --- ## 2. Dependency Ordering -**Can implement independently:** Yes (after v1 deletion for cleaner code, but not required) +**Can implement independently:** Yes ### Problem @@ -82,13 +90,13 @@ depends = ["docker"] **In `ParseManifest`:** - Validate `depends` entries are strings -- Unknown recipe names are caught later at add-recipe time +- Unknown recipe names are caught at add-recipe time **In `CheckRecipes` / TUI add-recipe flow:** - Build a dependency graph from all manifests in `v.Recipes` + the new recipe - Detect cycles via DFS - Error with the cycle path if found: `"cycle detected: devtools -> docker -> devtools"` -- Cycle detection runs when recipes are added to a VM +- Auto-added dependencies must pass `MatchesVM` for the target VM's OS ### Topo-Sort @@ -96,12 +104,23 @@ depends = ["docker"] - Before filtering by run mode, topo-sort `targets` so dependencies come first - A recipe with `depends = ["docker"]` runs after docker regardless of array order in `v.Recipes` - Kahn's algorithm or DFS-based sort; either works for small N +- **Cycle detection here too**: user can edit manifests on disk after add-time. Error if a cycle appears; do not silently fall back to array order. + +### Dependency Satisfaction Rules + +A dependency is satisfied when: +- The dependency recipe ran earlier in this apply run, OR +- The dependency recipe was already applied (`v.Applied` has an entry for it) + +A dependency is **not** satisfied when: +- The dependency is `run = "manual"` and was never applied — error: `"devtools depends on docker, which has never been applied (run = manual)"` +- The dependency is filtered out by `ApplyOpts.Only` and was never applied — error: `"devtools depends on docker; add it to --recipe or apply it first"` ### TUI Behavior User adds "devtools" which depends on "docker": - If docker is already in `v.Recipes`: proceed -- If docker is missing: auto-add it, show message: `"Added docker (required by devtools)"` +- If docker is missing: run `CheckRecipes` on docker for this VM's OS. If it passes, auto-add and show: `"Added docker (required by devtools)"`. If it fails, error with the reason. ### CLI Behavior @@ -114,13 +133,81 @@ The CLI does not auto-add. Scripts must be explicit. --- -## 3. Dry-Run +## 3. Cloudinit Post-Boot Path + +**Can implement independently:** Yes (after item 1) + +### Problem + +After item 1, cloudinit generates seeds with v2 scripts. `Apply()` still returns `ErrAppliedAtBoot` and refuses to run. This means: + +- `v.Applied` is never populated for cloudinit VMs +- `run = "once"` has no effect +- `run = "always"` cannot re-run recipes post-boot +- No way to add recipes to a running cloudinit VM + +### Solution + +Remove `ErrAppliedAtBoot`. Allow `Apply()` to work on cloudinit VMs via SSH after first boot. Track what cloud-init ran via marker files. + +### Marker Files + +**At VM creation (in cloudinit backend's seed generation):** + +Wrap each recipe's runcmd entry: +```yaml +runcmd: + - /var/lib/stoat/recipes/xfce.sh && mkdir -p /var/lib/stoat/.applied && touch /var/lib/stoat/.applied/xfce +``` + +Each recipe writes a marker file on success. + +### Apply Behavior + +**In `applyLocked` (after removing the `ErrAppliedAtBoot` check):** + +1. If backend is cloudinit and `v.Applied` is empty, SSH in and read `/var/lib/stoat/.applied/` +2. Populate `v.Applied` with entries for each marker file found +3. Save `v` to persist the discovered state +4. Continue with normal filtering via `filterByRunMode` + +**Hash note:** The hash stored in `v.Applied` comes from the *current* script on disk, which may differ from what cloud-init ran at creation time. This is benign: recipes are idempotent, and a changed script triggers a re-run on the next `Apply()` anyway. + +**Fallback:** If `/var/lib/stoat/.applied/` doesn't exist (old VM created before this feature, or cloud-init failed entirely), treat all recipes as pending. The first `Apply()` re-runs everything once; after that, state is tracked correctly. + +### Files to Modify + +**`internal/core/apply.go`:** +- Remove the `backend.For(v).Name() == "cloudinit"` check that returns `ErrAppliedAtBoot` +- Add `discoverCloudInitApplied(ctx, v)` call before `filterByRunMode` + +**`internal/cli/wire/errors.go`:** +- Remove `CodeAppliedAtBoot` mapping + +**`internal/tui/provision.go`:** +- Remove `ErrAppliedAtBoot` checks (two locations) + +**`internal/backend/cloudinit/cloudinit.go`:** +- Add marker-writing suffix to each recipe's runcmd entry + +### Result + +Cloudinit VMs behave like other VMs after first boot: +- `Apply()` works over SSH +- `v.Applied` tracks what ran +- `run = "once"` skips already-applied recipes +- `run = "always"` re-runs every time +- New recipes can be added and applied post-creation + +--- + +## 4. Dry-Run **Can implement independently:** Yes ### Problem -`Apply()` runs scripts directly. There's no way to preview what would run without running it. The filtering logic in `filterByRunMode` returns a trimmed list; a caller that wants to preview must duplicate that logic. +`Apply()` runs scripts directly. There's no way to preview what would run without running it. ### Solution @@ -146,7 +233,7 @@ tailscale (skip, already applied at v1.0.0) ### Implementation -**New function in `internal/recipes/` or `internal/core/`:** +**New function in `internal/core/`:** ```go type ApplyPlan struct { Name string @@ -160,55 +247,13 @@ func PlanApply(v *config.VM, opts ApplyOpts) ([]ApplyPlan, error) `PlanApply` calls the same filtering logic as `applyLocked` but returns the plan instead of executing. +**Works on stopped VMs:** The plan is computed host-side from manifests and `v.Applied`. The VM does not need to be running. + **CLI flag:** - `stoat apply --dry-run` calls `PlanApply`, prints result, exits - `stoat apply --dry-run --json` prints JSON -**TUI:** -- Before confirming apply, show the plan in a preview pane -- Use the same `PlanApply` function - ---- - -## 4. Implement Stage Field - -**Can implement independently:** Yes - -### Problem - -`stage = "install"` parses and validates, but install-stage recipe bodies are never executed. The Alpine disk-mode answerfile is generated from VM config, not from recipes. The field exists in the schema with no effect. - -### Use Case - -BYO ISO support needs install-stage hooks for custom partitioning, bootloader config, and installer automation. - -### Solution - -Execute install-stage recipes before first boot. - -### Execution by Backend - -**apkovl backend (disk mode):** -- Install-stage recipes bake into `/etc/local.d/` alongside `stoat-install.start` -- They run in dependency order before `setup-alpine` -- Naming: `00-.start`, `01-.start`, etc. - -**cloudinit backend:** -- Install-stage recipes go into `bootcmd` with a guard file check -- Pattern: `[ -f /var/lib/stoat/.installed/ ] || { /var/lib/stoat/recipes/.sh && touch /var/lib/stoat/.installed/; }` -- This runs once even though `bootcmd` executes every boot - -**ssh backend:** -- Install stage is not applicable; SSH implies the system is already installed -- Validate at add-recipe time: error if an install-stage recipe is added to an ssh-backend VM - -### Manifest - -No schema change needed. `stage` already accepts `"install"` | `"provision"`. - -### State Tracking - -Install-stage recipes are tracked in `v.Applied` like provision-stage recipes. The "already applied" check uses the same hash comparison. +**TUI preview pane:** Out of scope for first cut. CLI-only delivers the value. --- @@ -220,6 +265,8 @@ Install-stage recipes are tracked in `v.Applied` like provision-stage recipes. T `reboot = true` in the manifest triggers one reboot after all recipes finish. The first recipe that declares `reboot = true` names the reboot in the log. Subsequent `reboot = true` recipes don't trigger additional reboots. +Reboot only fires for disk-mode VMs (`apply.go:185` checks `v.Mode == "disk"`). + ### Scope Keep this behavior. Document it clearly. @@ -236,83 +283,54 @@ for changes to take effect (e.g., switching init systems, loading new kernel modules). When one or more recipes in a run declare `reboot = true`, stoat reboots -the guest once after all recipes complete. The reboot is not per-recipe; -it happens at the end of the apply run. +the guest once after all recipes complete. The reboot happens at the end +of the apply run, after every recipe has finished. -For disk-mode VMs, the reboot persists changes. For live-mode VMs, root -is tmpfs and the reboot wipes everything; live recipes that need a -"reboot" should restart their session in place instead. +Reboot applies to disk-mode VMs only. For live-mode VMs, root is tmpfs +and a reboot wipes everything. Live recipes that need a session restart +should restart in place instead (e.g., `kill -HUP 1`). ``` ### Future Extension -Per-recipe reboots (reboot after recipe A, then run recipe B) could be added later. This would require a `reboot = "after"` vs `reboot = "end"` distinction. Out of scope for this spec. +Per-recipe reboots (reboot after recipe A, then run recipe B) could be added later. Out of scope for this spec. --- -## 6. Cloudinit Post-Boot Path +## 6. Stage Field Validation -**Can implement independently:** Yes (largest scope item) +**Can implement independently:** Yes ### Problem -The cloudinit backend bakes provision-stage recipes into `write_files + runcmd` at VM creation time. `Apply()` returns `ErrAppliedAtBoot` and refuses to run. This means: - -- `v.Applied` is never populated for cloudinit VMs -- `run = "once"` has no effect; cloud-init runs once by definition -- `run = "always"` cannot re-run recipes post-boot -- There's no way to add recipes to a running cloudinit VM +`stage = "install"` parses and validates, but install-stage recipe bodies are never executed. The field exists in the schema with no effect. -### Solution +### Use Case -Remove `ErrAppliedAtBoot`. Allow `Apply()` to work on cloudinit VMs via SSH after first boot. Track what cloud-init ran via marker files. +BYO ISO support will need install-stage hooks for custom partitioning, bootloader config, and installer automation. That feature does not exist yet. -### Marker Files +### Solution -**At VM creation (in cloudinit backend's seed generation):** +Reject `stage = "install"` at add-recipe time until BYO ISO lands. -Change the runcmd wrapper from: -```yaml -runcmd: - - /var/lib/stoat/recipes/xfce.sh -``` +### Implementation -To: -```yaml -runcmd: - - /var/lib/stoat/recipes/xfce.sh && mkdir -p /var/lib/stoat/.applied && touch /var/lib/stoat/.applied/xfce +**In `CheckRecipes` / TUI add-recipe flow:** +```go +if m.Stage == "install" { + return fmt.Errorf("install-stage recipes are not yet supported") +} ``` -Each recipe writes a marker file on success. - -### Apply Behavior +### Why Not Implement Now -**In `applyLocked` (after removing the `ErrAppliedAtBoot` check):** +The cloudinit design has a bug: cloud-init's `bootcmd` runs *before* the `write_files` module, so the script file doesn't exist yet. `bootcmd` also runs before networking, so package installs fail. "Install stage" on a cloud image is conceptually unclear anyway — the system is already installed. -1. If backend is cloudinit and `v.Applied` is empty, SSH in and read `/var/lib/stoat/.applied/` -2. Populate `v.Applied` with entries for each marker file found (version from manifest, hash from current script, timestamp now) -3. Save `v` to persist the discovered state -4. Continue with normal filtering via `filterByRunMode` +Building install-stage execution paths for three backends, each with different mechanisms, for a feature with no consumer yet is premature. When BYO ISO lands, the actual hooks needed will be clear. -**Fallback:** If `/var/lib/stoat/.applied/` doesn't exist (old VM created before this feature, or cloud-init failed entirely), treat all recipes as pending. The first `Apply()` re-runs everything once; after that, state is tracked correctly. Recipes are idempotent, so re-running is safe. +### Future -### Files to Modify - -**`internal/core/apply.go`:** -- Remove the `backend.For(v).Name() == "cloudinit"` check that returns `ErrAppliedAtBoot` -- Add `discoverCloudInitApplied(ctx, v)` call before `filterByRunMode` - -**`internal/backend/cloudinit/cloudinit.go`:** -- Modify `Prepare()` to wrap each recipe with the marker-writing suffix - -### Result - -Cloudinit VMs behave like other VMs after first boot: -- `Apply()` works over SSH -- `v.Applied` tracks what ran -- `run = "once"` skips already-applied recipes -- `run = "always"` re-runs every time -- New recipes can be added and applied post-creation +When BYO ISO support is added, revisit this section. The implementation will likely differ from what we'd design today. --- @@ -321,9 +339,12 @@ Cloudinit VMs behave like other VMs after first boot: ### Unit Tests - `TestParseManifestDependsField` — validates depends parsing -- `TestCycleDetection` — catches A→B→A cycles +- `TestCycleDetection` — catches A→B→A cycles at add-time +- `TestCycleDetectionAtApplyTime` — catches cycles from edited manifests - `TestTopoSort` — orders recipes by dependencies +- `TestDependencySatisfaction` — manual/never-applied deps error correctly - `TestPlanApply` — returns correct plan without executing +- `TestPlanApplyStoppedVM` — works when VM is not running - `TestCloudInitMarkerDiscovery` — reads marker files, populates Applied ### Integration Tests @@ -331,6 +352,7 @@ Cloudinit VMs behave like other VMs after first boot: - Create a VM with depends, verify execution order - `--dry-run` outputs correct plan - cloudinit VM: apply after first boot, verify state tracking +- TUI: add recipe with missing dep that fails OS check, verify error ### Manual Tests @@ -344,17 +366,16 @@ Cloudinit VMs behave like other VMs after first boot: ### Order Matters -The fixes can land independently, but this order minimizes conflicts: - -1. **Delete v1 metadata** first — every other change touches fewer code paths -2. **Depends** — adds a manifest field, touches `filterByRunMode` -3. **Dry-run** — extracts logic from `applyLocked`, no schema change -4. **Stage field** — touches backend Prepare functions -5. **Reboot docs** — pure documentation -6. **Cloudinit post-boot** — largest change, benefits from cleaner codebase +1. **Item 1** first — deletes v1, fixes broken cloudinit backend +2. **Item 2** — dependency ordering, no blockers +3. **Item 3** — cloudinit markers, depends on item 1's v2 conversion +4. **Item 4** — dry-run, independent +5. **Item 5** — docs only +6. **Item 6** — validation only, can land anytime ### Backwards Compatibility -- v1 flat-file recipes stop working; users see a warning and must migrate +- v1 flat-file recipes are swept to `.v1-removed/` with a warning; users must migrate - Existing v2 recipes without `depends` keep working (empty deps = no ordering constraint) - Existing cloudinit VMs get their `v.Applied` populated on first post-boot `Apply()` +- `stage = "install"` recipes error at add-time until the feature is built From bed392239f80b17506efe7bb98ad1ee8493f548e Mon Sep 17 00:00:00 2001 From: NovusEdge Date: Mon, 10 Aug 2026 22:11:16 +0300 Subject: [PATCH 2/2] feat(recipes): dependency ordering, dry-run, and cloudinit post-boot apply Implements docs/specs/2026-08-10-recipe-system-fixes-design.md. - Delete the v1 metadata parser and wire cloudinit to v2 scripts: MatchesVM (via MatchReason) is the sole capability checker; Prepare and clone build seeds through ManifestFor+ScriptContent+WrapScripts; sweepV1 warns when it moves a recipe it does not recognise as stoat's. - Add recipe dependency ordering: a `depends` manifest field, TopoSort with cycle detection, apply-time satisfaction checks, create/CLI validation, and TUI auto-add of missing dependencies. - Cloudinit post-boot path: drop ErrAppliedAtBoot, write a marker per recipe from runcmd, and rebuild v.Applied over ssh after first boot. - Add `stoat apply --dry-run [--json]`: PlanApply reports the run/skip plan host-side, without starting the VM. - Document the single-reboot behaviour in docs/recipe-spec-v2.md. - Reject `stage = "install"` at add time until BYO ISO support lands. --- docs/recipe-spec-v2.md | 8 + internal/backend/cloudinit.go | 45 ++-- internal/cli/cli.go | 4 + internal/cli/grammar.go | 9 +- internal/cli/run_apply.go | 28 ++- internal/cli/subcommands_test.go | 21 +- internal/cli/wire/errors.go | 2 - internal/cli/wire/errors_test.go | 1 - internal/cloudinit/scripts.go | 13 +- internal/cloudinit/scripts_test.go | 9 +- internal/core/apply.go | 326 +++++++++++++++++++---------- internal/core/apply_test.go | 232 ++++++++++++++++---- internal/core/clone.go | 17 +- internal/core/core.go | 23 +- internal/core/deps.go | 116 ++++++++++ internal/core/deps_test.go | 89 ++++++++ internal/recipes/deps.go | 72 +++++++ internal/recipes/deps_test.go | 80 +++++++ internal/recipes/manifest.go | 29 ++- internal/recipes/manifest_test.go | 27 +++ internal/recipes/metadata.go | 185 ---------------- internal/recipes/metadata_test.go | 188 ----------------- internal/recipes/recipes.go | 51 +++-- internal/recipes/recipes_test.go | 37 ---- internal/sshx/sshx_test.go | 10 +- internal/tui/autoprov_test.go | 30 ++- internal/tui/deps.go | 47 +++++ internal/tui/edit.go | 18 +- internal/tui/edit_test.go | 73 +++++++ internal/tui/form.go | 20 +- internal/tui/labels.go | 12 +- internal/tui/provision.go | 21 -- internal/tui/provision_test.go | 23 -- 33 files changed, 1160 insertions(+), 706 deletions(-) create mode 100644 internal/core/deps.go create mode 100644 internal/core/deps_test.go create mode 100644 internal/recipes/deps.go create mode 100644 internal/recipes/deps_test.go delete mode 100644 internal/recipes/metadata.go delete mode 100644 internal/recipes/metadata_test.go create mode 100644 internal/tui/deps.go diff --git a/docs/recipe-spec-v2.md b/docs/recipe-spec-v2.md index 9922a25..47594da 100644 --- a/docs/recipe-spec-v2.md +++ b/docs/recipe-spec-v2.md @@ -137,6 +137,14 @@ VM creation (disk mode) -> provision-stage recipes run ``` +## Reboot Behavior + +A recipe declares `reboot = true` in its manifest to mark that the guest needs a reboot before its changes take effect. Examples: switching init systems, loading a new kernel module. + +Stoat reboots the guest once, after every recipe in the apply run finishes. The reboot happens at the end of the run, not after the individual recipe that requested it. If several recipes in the run declare `reboot = true`, stoat still reboots only once. + +This reboot applies to disk-mode VMs only. Live-mode VMs run their root filesystem on tmpfs, so a reboot wipes it. A live recipe that needs to restart a session restarts it in place instead, for example with `kill -HUP 1`. + ## State Tracking Add to `vm.toml`: diff --git a/internal/backend/cloudinit.go b/internal/backend/cloudinit.go index a93790c..cb5b18d 100644 --- a/internal/backend/cloudinit.go +++ b/internal/backend/cloudinit.go @@ -68,24 +68,45 @@ func (cloudinitBackend) Prepare(v *config.VM) error { if err != nil { return err } - // v.Recipes only holds names the form offered for this VM's os/backend. - // recipes.List already filters by backend at selection time, so every - // entry here is already a cloud fragment. No extra backend check is - // needed before reading them. - var recipeBodies []string - for _, name := range v.Recipes { - body, err := recipes.Read(name) - if err != nil { - return fmt.Errorf("reading recipe %s: %w", name, err) - } - recipeBodies = append(recipeBodies, body) + scripts, err := recipeScripts(v) + if err != nil { + return err + } + // WrapScripts renders every recipe into one write_files+runcmd fragment + // cloud-init runs in order at first boot. An empty selection wraps to "", + // which Seed must not carry as a document; pass no bodies instead. + var bodies []string + if frag := cloudinit.WrapScripts(scripts); frag != "" { + bodies = []string{frag} } - if _, err := cloudinit.Seed(v, pub, recipeBodies); err != nil { + if _, err := cloudinit.Seed(v, pub, bodies); err != nil { return err } return nil } +// recipeScripts resolves v.Recipes to the cloud-init scripts WrapScripts +// renders: each recipe's manifest, then the script body for v.OS. A recipe +// with no recipe.toml went missing since create time and errors here. +func recipeScripts(v *config.VM) ([]cloudinit.Script, error) { + var scripts []cloudinit.Script + for _, name := range v.Recipes { + m, ok, err := recipes.ManifestFor(name) + if err != nil { + return nil, fmt.Errorf("reading recipe %s: %w", name, err) + } + if !ok { + return nil, fmt.Errorf("reading recipe %s: no recipe.toml", name) + } + body, err := m.ScriptContent(v.OS) + if err != nil { + return nil, fmt.Errorf("reading recipe %s: %w", name, err) + } + scripts = append(scripts, cloudinit.Script{Name: name, Content: body}) + } + return scripts, nil +} + // Args attaches the seed cdrom only in cloud mode; see the mode-guard note // on cloudinitBackend. func (cloudinitBackend) Args(v *config.VM) []string { diff --git a/internal/cli/cli.go b/internal/cli/cli.go index 773dc42..e66b8fc 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -51,6 +51,10 @@ type Args struct { // behavior existed. NoApply bool + // DryRun belongs to "apply": it prints the plan (core.PlanApply) and runs + // nothing. + DryRun bool + // JSON is set by Main from the pre-parse argv scan, never by Parse: the // flag has to be recognized before any parser exists so a usage error // can still produce an envelope. It implies Quiet, so every prose line diff --git a/internal/cli/grammar.go b/internal/cli/grammar.go index 8a8743b..31d758c 100644 --- a/internal/cli/grammar.go +++ b/internal/cli/grammar.go @@ -198,8 +198,9 @@ type waitCmd struct { } type applyCmd struct { - VM string `arg:"" help:"vm name"` - Only []string `help:"subset of the VM's own recipes"` + VM string `arg:"" help:"vm name"` + Only []string `help:"subset of the VM's own recipes"` + DryRun bool `name:"dry-run" help:"print what would run without running it"` } type recipesCmd struct { @@ -383,10 +384,10 @@ func (g *grammar) toArgs(path string) (*Args, error) { a.VM, a.Until, a.Timeout = w.VM, core.Until(w.Until), w.Timeout case "apply": - a.VM, a.Only = g.Apply.VM, trimList(g.Apply.Only) + a.VM, a.Only, a.DryRun = g.Apply.VM, trimList(g.Apply.Only), g.Apply.DryRun case "provision": - a.VM, a.Only = g.Provision.VM, trimList(g.Provision.Only) + a.VM, a.Only, a.DryRun = g.Provision.VM, trimList(g.Provision.Only), g.Provision.DryRun case "recipes": a.OS, a.Backend = g.Recipes.OS, g.Recipes.Backend diff --git a/internal/cli/run_apply.go b/internal/cli/run_apply.go index 408383a..5e8bdfd 100644 --- a/internal/cli/run_apply.go +++ b/internal/cli/run_apply.go @@ -24,6 +24,10 @@ func runApply(a *Args, stdout, stderr io.Writer) int { return a.fail(stdout, stderr, err) } + if a.DryRun { + return runApplyDryRun(a, stdout, stderr) + } + applied := a.Only if len(applied) == 0 { applied = v.Recipes @@ -60,9 +64,6 @@ func runApply(a *Args, stdout, stderr io.Writer) int { return ExitOK } if aerr != nil { - // core.ErrAppliedAtBoot is a real outcome for a cloud VM, mapped to - // applied_at_boot by wire's error table; it is not special-cased into - // a success here. return a.fail(stdout, stderr, aerr) } @@ -72,3 +73,24 @@ func runApply(a *Args, stdout, stderr io.Writer) int { fmt.Fprintf(stdout, "%s: recipes applied\n", a.VM) return ExitOK } + +// runApplyDryRun prints core.PlanApply's plan and runs nothing. Human output +// is one line per recipe ("xfce (run, never applied)"); --json emits the plan +// array. The plan is computed host-side, so the VM need not be running. +func runApplyDryRun(a *Args, stdout, stderr io.Writer) int { + plan, err := core.PlanApply(a.VM, core.ApplyOpts{Only: a.Only}) + if err != nil { + return a.fail(stdout, stderr, err) + } + if a.JSON { + return a.ok(stdout, plan) + } + for _, p := range plan { + reason := p.Reason + if p.Version != "" { + reason = fmt.Sprintf("%s at %s", p.Reason, p.Version) + } + fmt.Fprintf(stdout, "%s (%s, %s)\n", p.Name, p.Action, reason) + } + return ExitOK +} diff --git a/internal/cli/subcommands_test.go b/internal/cli/subcommands_test.go index bff458c..f9f37b3 100644 --- a/internal/cli/subcommands_test.go +++ b/internal/cli/subcommands_test.go @@ -283,29 +283,22 @@ func TestApplyMissingVMIsNotFound(t *testing.T) { } } -// A cloud VM's recipes ran at first boot, so apply has nothing to do and must -// say which of the two it is, not report a generic failure. -// -// The VM has to look RUNNING to reach that check: core.Apply refuses a stopped -// VM first, so a stopped cloud VM answers not_running, which is also true and -// is not the answer this is about. -func TestApplyOnACloudVMIsAppliedAtBoot(t *testing.T) { +// A cloudinit VM applies over ssh after first boot (item 3), so apply no +// longer refuses it. With no recipes there is nothing to run, so apply is a +// clean no-op rather than the old applied_at_boot refusal. +func TestApplyOnACloudVMIsNoLongerRefused(t *testing.T) { dir := cliRoot(t) v := saveVM(t, &config.VM{ Name: "cloudy", OS: "ubuntu", Mode: "cloud", Backend: "cloudinit", - RAM: 2048, CPUs: 2, SSHPort: 2201, Recipes: []string{"xfce"}, + RAM: 2048, CPUs: 2, SSHPort: 2201, }) v.Dir = filepath.Join(dir, "cloudy") stop := fakeRunning(t, v) defer stop() code, objs := runJSON(t, "apply", "cloudy") - if code != ExitFail { - t.Fatalf("exit = %d, want %d: %v", code, ExitFail, objs) - } - errObj, _ := result(t, objs)["error"].(map[string]any) - if errObj["code"] != wire.CodeAppliedAtBoot { - t.Errorf("code = %v, want %q", errObj["code"], wire.CodeAppliedAtBoot) + if code != ExitOK { + t.Fatalf("exit = %d, want %d: %v", code, ExitOK, objs) } } diff --git a/internal/cli/wire/errors.go b/internal/cli/wire/errors.go index a8b06da..992ee98 100644 --- a/internal/cli/wire/errors.go +++ b/internal/cli/wire/errors.go @@ -25,7 +25,6 @@ const ( CodeImmutableField = "immutable_field" CodeDiskShrink = "disk_shrink" CodeCannotReach = "cannot_reach" - CodeAppliedAtBoot = "applied_at_boot" CodeUnknownLog = "unknown_log" CodeTimeout = "timeout" CodeCanceled = "canceled" @@ -65,7 +64,6 @@ var codeTable = []struct { {CodeImmutableField, core.ErrImmutableField}, {CodeDiskShrink, core.ErrDiskShrink}, {CodeCannotReach, core.ErrCannotReach}, - {CodeAppliedAtBoot, core.ErrAppliedAtBoot}, {CodeUnknownLog, core.ErrUnknownWhich}, {CodeTimeout, context.DeadlineExceeded}, {CodeCanceled, context.Canceled}, diff --git a/internal/cli/wire/errors_test.go b/internal/cli/wire/errors_test.go index 6cb2c41..4d1b49a 100644 --- a/internal/cli/wire/errors_test.go +++ b/internal/cli/wire/errors_test.go @@ -26,7 +26,6 @@ func TestMapErrorEveryCoreSentinel(t *testing.T) { {fmt.Errorf("%w: os", core.ErrImmutableField), CodeImmutableField}, {fmt.Errorf("%w: 8G -> 4G", core.ErrDiskShrink), CodeDiskShrink}, {fmt.Errorf("%w: work: not running", core.ErrCannotReach), CodeCannotReach}, - {fmt.Errorf("%w: work", core.ErrAppliedAtBoot), CodeAppliedAtBoot}, {fmt.Errorf("%w: %q", core.ErrUnknownWhich, "bogus"), CodeUnknownLog}, {context.DeadlineExceeded, CodeTimeout}, {context.Canceled, CodeCanceled}, diff --git a/internal/cloudinit/scripts.go b/internal/cloudinit/scripts.go index 6dedd96..7615e72 100644 --- a/internal/cloudinit/scripts.go +++ b/internal/cloudinit/scripts.go @@ -9,6 +9,12 @@ import ( // matching docs/recipe-spec-v2.md's cloudinit execution model. const scriptDir = "/var/lib/stoat/recipes" +// MarkerDir holds one empty file per recipe that ran successfully at first +// boot. core.Apply reads it over ssh to rebuild v.Applied for a cloudinit VM, +// which never populated it at create time. The name is the recipe's, with no +// extension. +const MarkerDir = "/var/lib/stoat/.applied" + // Script pairs a recipe's Name with the body WrapScripts should run for it, // i.e. the manifest's Name and manifest.ScriptContent(osName) for the guest // being provisioned. @@ -41,7 +47,12 @@ func WrapScripts(scripts []Script) string { wf.WriteString(" permissions: '0755'\n") wf.WriteString(" content: |\n") wf.WriteString(indentBlock(s.Content)) - rc.WriteString(fmt.Sprintf(" - %s\n", path)) + // The script runs, then drops a marker on success. core.Apply reads + // MarkerDir over ssh to rebuild v.Applied post-boot. The && chain + // leaves no marker for a script that failed, so a failed recipe stays + // pending instead of being recorded as applied. + marker := fmt.Sprintf("%s/%s", MarkerDir, s.Name) + rc.WriteString(fmt.Sprintf(" - %s && mkdir -p %s && touch %s\n", path, MarkerDir, marker)) } return "#cloud-config\n" + wf.String() + rc.String() diff --git a/internal/cloudinit/scripts_test.go b/internal/cloudinit/scripts_test.go index fa64b9a..85987ec 100644 --- a/internal/cloudinit/scripts_test.go +++ b/internal/cloudinit/scripts_test.go @@ -65,8 +65,9 @@ func TestWrapScriptsSingleScript(t *testing.T) { if len(f.Runcmd) != 1 { t.Fatalf("runcmd has %d entries, want 1:\n%s", len(f.Runcmd), body) } - if f.Runcmd[0] != "/var/lib/stoat/recipes/xfce.sh" { - t.Errorf("runcmd[0] = %q, want /var/lib/stoat/recipes/xfce.sh", f.Runcmd[0]) + wantCmd := "/var/lib/stoat/recipes/xfce.sh && mkdir -p /var/lib/stoat/.applied && touch /var/lib/stoat/.applied/xfce" + if f.Runcmd[0] != wantCmd { + t.Errorf("runcmd[0] = %q, want %q", f.Runcmd[0], wantCmd) } } @@ -100,7 +101,9 @@ func TestWrapScriptsPreservesOrder(t *testing.T) { if len(f.Runcmd) != len(wantPaths) { t.Fatalf("runcmd has %d entries, want %d:\n%s", len(f.Runcmd), len(wantPaths), body) } - for i, want := range wantPaths { + names := []string{"base", "docker", "xfce"} + for i, path := range wantPaths { + want := path + " && mkdir -p /var/lib/stoat/.applied && touch /var/lib/stoat/.applied/" + names[i] if f.Runcmd[i] != want { t.Errorf("runcmd[%d] = %q, want %q", i, f.Runcmd[i], want) } diff --git a/internal/core/apply.go b/internal/core/apply.go index eceb4db..55179d3 100644 --- a/internal/core/apply.go +++ b/internal/core/apply.go @@ -10,23 +10,13 @@ import ( "time" "github.com/novusedge/stoat/internal/backend" + "github.com/novusedge/stoat/internal/cloudinit" "github.com/novusedge/stoat/internal/config" - "github.com/novusedge/stoat/internal/guest" "github.com/novusedge/stoat/internal/qemu" "github.com/novusedge/stoat/internal/recipes" "github.com/novusedge/stoat/internal/sshx" ) -// ErrAppliedAtBoot is returned by Apply for a VM whose backend already ran -// its recipes at first boot: the cloudinit backend. Its recipes merge into -// the cloud-init seed and run through the guest's own cloud-init service, -// before sshd is even reachable (internal/backend/cloudinit.go's Prepare has -// no matching Provision). Calling sshx.Provision on such a VM is not a -// harmless no-op: v.Recipes holds "*.cloud.yaml" fragment names, and -// Provision pipes whatever it's given to `sh -s` over ssh, so a cloud -// fragment would run as a shell script. Apply refuses up front instead. -var ErrAppliedAtBoot = errors.New("recipes for this backend already ran at first boot") - // ApplyOpts controls one Apply call. type ApplyOpts struct { // Only restricts the run to a subset of the VM's OWN recipe list, @@ -82,14 +72,68 @@ func Apply(ctx context.Context, name string, opts ApplyOpts) error { return err } +// ApplyPlan is one recipe's entry in a dry-run: what Apply would do with it, +// and why. Version is the recipe's applied version from v.Applied, empty when +// it was never applied. +type ApplyPlan struct { + Name string `json:"name"` + Action string `json:"action"` // "run" | "skip" + Reason string `json:"reason"` + Version string `json:"version,omitempty"` +} + +// PlanApply reports what Apply would do for VM name, without running anything. +// It uses the same filtering as applyLocked (run mode, dependency order and +// satisfaction), so a plan matches the run that would follow. +// +// The plan is host-side: it reads manifests and v.Applied, so the VM does not +// need to be running. A cloudinit VM whose markers were never discovered still +// has an empty v.Applied here, so its recipes read as "never applied" until a +// real Apply populates it (docs/specs recipe-system-fixes §3 fallback). +func PlanApply(name string, opts ApplyOpts) ([]ApplyPlan, error) { + v, err := load(name) + if err != nil { + return nil, err + } + + targets := v.Recipes + if len(opts.Only) > 0 { + have := make(map[string]bool, len(v.Recipes)) + for _, r := range v.Recipes { + have[r] = true + } + for _, o := range opts.Only { + if !have[o] { + return nil, fmt.Errorf("%w: recipe %q is not one of %s's recipes", ErrRecipeNotApplicable, o, v.Name) + } + } + targets = opts.Only + } + explicit := make(map[string]bool, len(opts.Only)) + for _, o := range opts.Only { + explicit[o] = true + } + + decisions, _, err := planRecipes(v, targets, explicit) + if err != nil { + return nil, err + } + plan := make([]ApplyPlan, 0, len(decisions)) + for _, d := range decisions { + action := "skip" + if d.run { + action = "run" + } + plan = append(plan, ApplyPlan{Name: d.name, Action: action, Reason: d.reason, Version: v.Applied[d.name].Version}) + } + return plan, nil +} + // applyLocked is Apply's body, run while Apply holds name's provision lock. func applyLocked(ctx context.Context, v *config.VM, opts ApplyOpts) error { if !qemu.Running(v) { return fmt.Errorf("%w: %s", ErrNotRunning, v.Name) } - if backend.For(v).Name() == "cloudinit" { - return fmt.Errorf("%w: %s", ErrAppliedAtBoot, v.Name) - } targets := v.Recipes if len(opts.Only) > 0 { @@ -122,6 +166,14 @@ func applyLocked(ctx context.Context, v *config.VM, opts ApplyOpts) error { for _, o := range opts.Only { explicit[o] = true } + + // A cloudinit VM ran its recipes from the seed at first boot and never + // populated v.Applied. Read the marker files cloud-init left behind, so + // filterByRunMode can skip a "once" recipe instead of re-running it. + if err := discoverCloudInitApplied(ctx, v); err != nil { + return err + } + runTargets, manifests, err := filterByRunMode(v, targets, explicit) if err != nil { return err @@ -241,6 +293,50 @@ func appendProvisionLog(v *config.VM, s string) { f.WriteString(s) } +// discoverCloudInitApplied rebuilds v.Applied for a cloudinit VM from the +// marker files cloud-init left after first boot. It runs over ssh, so the VM +// must be reachable; applyLocked calls it only after the qemu.Running check. +// +// It no-ops unless the backend is cloudinit and v.Applied is still empty: once +// a post-boot Apply has recorded state, that state is authoritative and this +// must not overwrite it. A missing marker directory (an old VM, or cloud-init +// that failed entirely) leaves v.Applied empty, so the next Apply re-runs +// every recipe once, then tracks state from then on. +// +// The recorded Hash comes from the current script on disk, not from whatever +// cloud-init ran at creation. That is benign: recipes are idempotent, and a +// script that has since changed reruns on this same Apply anyway. +func discoverCloudInitApplied(ctx context.Context, v *config.VM) error { + if backend.For(v).Name() != "cloudinit" || len(v.Applied) > 0 { + return nil + } + out, err := exec.CommandContext(ctx, "ssh", sshx.Args(v, "ls -1 "+cloudinit.MarkerDir+" 2>/dev/null")...).Output() + if err != nil { + return nil // marker dir missing or a transient ssh error; discover nothing + } + + var applied map[string]config.AppliedRecipe + for _, name := range strings.Fields(string(out)) { + hash, err := recipes.ScriptHash(name, v.OS) + if err != nil { + continue // a marker for a recipe no longer on disk + } + ver := "" + if m, ok, _ := recipes.ManifestFor(name); ok { + ver = m.Version + } + if applied == nil { + applied = make(map[string]config.AppliedRecipe) + } + applied[name] = config.AppliedRecipe{Version: ver, Hash: hash, At: time.Now()} + } + if applied == nil { + return nil + } + v.Applied = applied + return v.Save() +} + // filterByRunMode narrows targets to the recipes that should actually run, // given each recipe's declared run mode (recipes.Manifest.Run) and what v // has already recorded in Applied. @@ -256,13 +352,47 @@ func appendProvisionLog(v *config.VM, s string) { // That never equals a real script hash, so an existing VM reruns its // "once" recipes exactly once, then carries a real hash from then on. // -// A target with no recipe.toml (ManifestFor's ok=false) is a v1 flat-file -// recipe. It has no run-mode concept and always stays in the result, -// matching Apply's behavior before v2 recipes existed. The returned map -// holds the manifest for every v2 recipe kept, so the caller does not have +// A target with no recipe.toml (ManifestFor's ok=false) is a recipe that +// went missing since the VM was created. It has no script to run, so this +// errors rather than passing a name Provision cannot resolve. The returned +// map holds the manifest for every recipe kept, so the caller does not have // to re-resolve it after the run succeeds. +// +// Targets run in dependency order, not v.Recipes order: a recipe with +// depends = ["docker"] runs after docker regardless of array position. A +// dependency cycle among the targets errors here, since a manifest edited on +// disk after add-time can introduce one (docs/specs recipe-system-fixes §2). +// A kept recipe's dependency must be satisfied: it ran earlier in this run, +// or is already recorded in v.Applied. An unsatisfiable dependency errors +// rather than run the dependent against an unmet one. func filterByRunMode(v *config.VM, targets []string, explicit map[string]bool) ([]string, map[string]recipes.Manifest, error) { - kept := make([]string, 0, len(targets)) + decisions, manifests, err := planRecipes(v, targets, explicit) + if err != nil { + return nil, nil, err + } + kept := make([]string, 0, len(decisions)) + for _, d := range decisions { + if d.run { + kept = append(kept, d.name) + } + } + return kept, manifests, nil +} + +// recipeDecision is planRecipes' verdict for one target: whether it runs, and +// the reason a reader (dry-run) can print. run==false is a skip. +type recipeDecision struct { + name string + run bool + reason string +} + +// planRecipes resolves, dependency-orders, and classifies targets. It returns +// one decision per target in run order. It is the shared body behind both +// filterByRunMode (which keeps the run==true names) and PlanApply (which +// reports every decision). It raises the same errors either way: a missing +// recipe.toml, a dependency cycle, or an unsatisfiable dependency. +func planRecipes(v *config.VM, targets []string, explicit map[string]bool) ([]recipeDecision, map[string]recipes.Manifest, error) { manifests := make(map[string]recipes.Manifest, len(targets)) for _, name := range targets { m, ok, err := recipes.ManifestFor(name) @@ -270,15 +400,28 @@ func filterByRunMode(v *config.VM, targets []string, explicit map[string]bool) ( return nil, nil, err } if !ok { - kept = append(kept, name) - continue + return nil, nil, fmt.Errorf("%w: recipe %q has no recipe.toml", ErrRecipeNotApplicable, name) } manifests[name] = m + } + + ordered, err := recipes.TopoSort(targets, manifests) + if err != nil { + return nil, nil, fmt.Errorf("%w: %v", ErrRecipeNotApplicable, err) + } + decisions := make([]recipeDecision, 0, len(ordered)) + keptSet := make(map[string]bool, len(ordered)) + for _, name := range ordered { + m := manifests[name] + + run, reason := true, "runs every time" switch m.Run { case "manual": - if !explicit[name] { - continue + if explicit[name] { + reason = "named explicitly" + } else { + run, reason = false, "manual, not selected" } case "once": if applied, done := v.Applied[name]; done { @@ -287,13 +430,41 @@ func filterByRunMode(v *config.VM, targets []string, explicit map[string]bool) ( return nil, nil, err } if applied.Hash == hash { - continue + run, reason = false, "already applied" + } else { + reason = "script changed" + } + } else { + reason = "never applied" + } + } + + if run { + for _, dep := range m.Depends { + if keptSet[dep] { + continue // ran earlier in this run + } + if _, done := v.Applied[dep]; done { + continue // already applied on this VM } + return nil, nil, dependencyError(name, dep, manifests, explicit) } + keptSet[name] = true } - kept = append(kept, name) + decisions = append(decisions, recipeDecision{name: name, run: run, reason: reason}) } - return kept, manifests, nil + return decisions, manifests, nil +} + +// dependencyError explains why dependent cannot run: its dependency dep is +// neither running this pass nor already applied. A dep that is a configured +// "manual" recipe was skipped because nobody named it; anything else is a dep +// left out of this run (excluded by ApplyOpts.Only, or not on the VM). +func dependencyError(dependent, dep string, manifests map[string]recipes.Manifest, explicit map[string]bool) error { + if m, ok := manifests[dep]; ok && m.Run == "manual" && !explicit[dep] { + return fmt.Errorf("%w: %s depends on %s, which has never been applied (run = manual)", ErrRecipeNotApplicable, dependent, dep) + } + return fmt.Errorf("%w: %s depends on %s; add it to --recipe or apply it first", ErrRecipeNotApplicable, dependent, dep) } // Recipe is one recipe resolved for a specific OS. In v2, recipes are @@ -347,17 +518,10 @@ type RecipeIssue struct { // reports nothing, so a caller checking "will these work" reads an empty // result as a clean answer. // -// The reasons below prefer a recipe's own declared front matter -// (internal/recipes.ParseMetadata/UnsupportedReason) when it has any: "xfce -// requires systemd, alpine uses openrc" is docs/design/core-api.md §4's -// example. Where a recipe declares no metadata (every *.cloud.yaml fragment -// today, by design; see recipeIssueReason) the reason falls back to a -// structural one, derived from the requested file's name, from -// guest.Lookup(osName), and from what else exists on disk (an OS-specific -// override suppressing a shared fragment; see recipes.List's doc comment on -// "overridden"). That still gives a true reason like "xfce.cloud.yaml is -// not offered to alpine because alpine has its own xfce.alpine.cloud.yaml" -// for recipes with no better reason to give. +// backendName is ignored in v2: every recipe is a shell script, and the +// backend determines how it runs, not whether it applies (see recipes.List). +// The reason for an inapplicable recipe comes from recipes.MatchReason +// against the recipe's manifest: "docker: built for alpine, not debian". func CheckRecipes(osName, backendName string, names []string) ([]RecipeIssue, error) { available, err := recipes.List(osName, backendName) if err != nil { @@ -370,82 +534,32 @@ func CheckRecipes(osName, backendName string, names []string) ([]RecipeIssue, er var issues []RecipeIssue for _, n := range names { - if ok[n] { + if !ok[n] { + issues = append(issues, RecipeIssue{Name: n, Reason: recipeIssueReason(n, osName)}) continue } - issues = append(issues, RecipeIssue{Name: n, Reason: recipeIssueReason(n, osName, backendName)}) + // Applicable, but an install-stage recipe cannot run yet. + if m, mok, err := recipes.ManifestFor(n); err == nil && mok && m.Stage == "install" { + issues = append(issues, RecipeIssue{Name: n, Reason: installStageUnsupported}) + } } return issues, nil } -// recipeIssueReason explains why name is not offered to osName/backendName. -// It prefers a capability-based reason drawn from the recipe's own declared -// front matter (internal/recipes.ParseMetadata/UnsupportedReason). When the -// recipe declares no metadata, it falls back to one derived structurally, -// from the filename, guest.Lookup, and whether other files exist on disk; -// see CheckRecipes' doc comment for why that fallback exists. -func recipeIssueReason(name, osName, backendName string) string { - if _, err := os.Stat(recipes.Path(name)); err != nil { - return fmt.Sprintf("no such recipe %q", name) +// recipeIssueReason explains why name is not offered to osName. A name with no +// recipe.toml is not a recipe. A recipe.toml that fails to parse reports the +// parse error. Otherwise recipes.MatchReason explains the OS or capability +// mismatch that MatchesVM (and so recipes.List) rejected it for. +func recipeIssueReason(name, osName string) string { + m, ok, err := recipes.ManifestFor(name) + if err != nil { + return fmt.Sprintf("%s: %v", name, err) } - - isCloudFragment := strings.HasSuffix(name, ".cloud.yaml") - isShellRecipe := strings.HasSuffix(name, ".sh") && !isCloudFragment - - // The backend mismatches below are checked before metadata and win - // outright: a shell recipe pushed to a cloudinit VM, or a cloud - // fragment offered to a backend with no cloud-init seed to merge it - // into. Both are about HOW a recipe gets applied, which no front-matter - // tag declares, so UnsupportedReason has nothing better to say. - switch { - case isCloudFragment && backendName != "cloudinit": - return fmt.Sprintf("%s is a cloud-init fragment; the %s backend applies recipes over ssh after boot, not from a cloud-init seed", name, backendName) - case isShellRecipe && backendName == "cloudinit": - return fmt.Sprintf("%s is a shell recipe pushed over ssh; the cloudinit backend applies its recipes from the seed at first boot instead", name) - } - - // Backend is applicable. Whether name matches osName is answered by the - // recipe's own declared front matter, when present, with a real reason - // ("requires systemd, alpine uses openrc") instead of one guessed from - // the filename; that reason wins when both exist. A parse error or a - // recipe with no "# stoat:" block (every *.cloud.yaml fragment today; - // see docs/design/guest-subsystem.md §5's phase split) leaves m at its - // zero value. UnsupportedReason then returns "", and the structural - // switch below answers instead. - if m, err := recipes.ReadMetadata(name); err == nil { - if reason := recipes.UnsupportedReason(osName, m); reason != "" { - return fmt.Sprintf("%s: %s", name, reason) - } + if !ok { + return fmt.Sprintf("no such recipe %q", name) } - - switch { - case isCloudFragment: - base := strings.TrimSuffix(name, ".cloud.yaml") - if i := strings.LastIndex(base, "."); i >= 0 { - fileOS := base[i+1:] - return fmt.Sprintf("%s is built for %s, not %s", name, fileOS, osName) - } - g, isKnown := guest.Lookup(osName) - if !isKnown { - return fmt.Sprintf("%s: %q is not a recognised OS", name, osName) - } - if !g.CloudRecipes { - return fmt.Sprintf("%s: %s is not in the shared cloud-recipe set", name, osName) - } - if override := base + "." + osName + ".cloud.yaml"; fileExists(recipes.Path(override)) { - return fmt.Sprintf("%s: %s has its own override, %s; use that instead of the shared fragment", name, osName, override) - } - return fmt.Sprintf("%s is not offered to %s/%s", name, osName, backendName) - case isShellRecipe: - fields := strings.Split(strings.TrimSuffix(name, ".sh"), ".") - fileOS := fields[len(fields)-1] - return fmt.Sprintf("%s is built for %s, not %s", name, fileOS, osName) - default: - return fmt.Sprintf("%s is not offered to %s/%s", name, osName, backendName) + if reason := recipes.MatchReason(&m, osName); reason != "" { + return fmt.Sprintf("%s: %s", name, reason) } -} - -func fileExists(path string) bool { - _, err := os.Stat(path) - return err == nil + return fmt.Sprintf("%s is not offered to %s", name, osName) } diff --git a/internal/core/apply_test.go b/internal/core/apply_test.go index 0362653..7d65ce7 100644 --- a/internal/core/apply_test.go +++ b/internal/core/apply_test.go @@ -62,15 +62,14 @@ func TestApplyRefusesWhileLockIsHeld(t *testing.T) { } } -// A cloudinit VM's recipes ran from the cloud-init seed at first boot; -// there is no post-boot ssh step for Apply to drive, and running one would -// mean piping a YAML fragment to `sh -s`. Apply must refuse this outright -// rather than attempt it. -func TestApplyRefusesCloudinitBackend(t *testing.T) { +// A cloudinit VM no longer refuses Apply: item 3 lets it apply over ssh after +// first boot. A cloudinit VM with no recipes is a plain no-op, which proves +// the old ErrAppliedAtBoot refusal is gone and reaches no ssh step. +func TestApplyCloudinitNoRecipesIsANoop(t *testing.T) { dir := root(t) v := &config.VM{ Name: "cl", Mode: "cloud", OS: "debian", Backend: "cloudinit", - RAM: 1024, CPUs: 1, SSHPort: 2200, Recipes: []string{"xfce"}, + RAM: 1024, CPUs: 1, SSHPort: 2200, } if err := v.Save(); err != nil { t.Fatal(err) @@ -79,8 +78,8 @@ func TestApplyRefusesCloudinitBackend(t *testing.T) { stop := fakeRunning(t, v) defer stop() - if err := Apply(context.Background(), "cl", ApplyOpts{}); !errors.Is(err, ErrAppliedAtBoot) { - t.Fatalf("err = %v, want ErrAppliedAtBoot", err) + if err := Apply(context.Background(), "cl", ApplyOpts{}); err != nil { + t.Fatalf("err = %v, want nil (no recipes, no refusal)", err) } } @@ -135,6 +134,8 @@ func TestApplyOnlyRejectsANameNotOnTheVM(t *testing.T) { // ctx-aware cancellation. func TestApplyOnlyAcceptsAValidSubset(t *testing.T) { dir := root(t) + writeV2Recipe(t, dir, "a", "always", "1.0", "#!/bin/sh\necho a\n") + writeV2Recipe(t, dir, "b", "always", "1.0", "#!/bin/sh\necho b\n") ln, err := net.Listen("tcp", "127.0.0.1:0") if err != nil { @@ -245,11 +246,8 @@ func TestCheckRecipesOKRecipeReportsNoIssue(t *testing.T) { } // A recipe requested for an OS its recipe.toml doesn't declare (docker is -// alpine-only) falls back to the structural reason: ReadMetadata cannot -// read a v2 recipe directory as a flat file, so it has nothing to say, and -// CheckRecipes must still produce a real, useful reason rather than an -// empty one. -func TestCheckRecipesFallsBackToStructuralReason(t *testing.T) { +// alpine-only) reports the OS mismatch from recipes.MatchReason. +func TestCheckRecipesReportsOSMismatch(t *testing.T) { root(t) if err := recipes.Install(); err != nil { t.Fatal(err) @@ -262,23 +260,9 @@ func TestCheckRecipesFallsBackToStructuralReason(t *testing.T) { if len(issues) != 1 { t.Fatalf("issues = %+v, want exactly 1", issues) } - want := "docker is not offered to debian/apkovl" + want := "docker: built for alpine, not debian" if !strings.Contains(issues[0].Reason, want) { - t.Errorf("Reason = %q, want it to contain %q (the structural fallback)", issues[0].Reason, want) - } -} - -// writeRecipe drops a recipe file straight into root's recipes/ dir, -// bypassing recipes.Install/the bundled set, so a test can pin exact front -// matter without editing a shipped recipe. -func writeRecipe(t *testing.T, dir, name, body string) { - t.Helper() - recipesDir := filepath.Join(dir, "recipes") - if err := os.MkdirAll(recipesDir, 0o755); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(filepath.Join(recipesDir, name), []byte(body), 0o755); err != nil { - t.Fatal(err) + t.Errorf("Reason = %q, want it to contain %q", issues[0].Reason, want) } } @@ -303,6 +287,161 @@ func writeV2Recipe(t *testing.T, rootDir, name, run, version, script string) { } } +// writeDepRecipe writes a v2 recipe declaring depends, for the dependency +// ordering tests. run sets the run mode; depends is written verbatim as a +// TOML string array. +func writeDepRecipe(t *testing.T, rootDir, name, run string, depends []string) { + t.Helper() + recipeDir := filepath.Join(rootDir, "recipes", name) + if err := os.MkdirAll(recipeDir, 0o755); err != nil { + t.Fatal(err) + } + quoted := make([]string, len(depends)) + for i, d := range depends { + quoted[i] = "\"" + d + "\"" + } + toml := "name = \"" + name + "\"\n" + + "script = \"install.sh\"\n" + + "run = \"" + run + "\"\n" + + "depends = [" + strings.Join(quoted, ", ") + "]\n" + if err := os.WriteFile(filepath.Join(recipeDir, "recipe.toml"), []byte(toml), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(recipeDir, "install.sh"), []byte("#!/bin/sh\necho "+name+"\n"), 0o755); err != nil { + t.Fatal(err) + } +} + +// TestPlanApplyOnStoppedVM pins the dry-run plan: it is computed host-side, so +// a stopped VM (never started here) still returns a correct run/skip plan. An +// applied "once" recipe with a matching hash skips; an unapplied one runs. +func TestPlanApplyOnStoppedVM(t *testing.T) { + dir := root(t) + writeV2Recipe(t, dir, "xfce", "once", "1.0", "#!/bin/sh\necho xfce\n") + writeV2Recipe(t, dir, "docker", "once", "2.0", "#!/bin/sh\necho docker\n") + xfceHash, err := recipes.ScriptHash("xfce", "alpine") + if err != nil { + t.Fatal(err) + } + v := &config.VM{ + Name: "work", Mode: "live", OS: "alpine", Backend: "apkovl", + RAM: 512, CPUs: 1, SSHPort: 2200, + Recipes: []string{"xfce", "docker"}, + Applied: map[string]config.AppliedRecipe{"xfce": {Version: "1.0", Hash: xfceHash}}, + } + if err := v.Save(); err != nil { + t.Fatal(err) + } + + plan, err := PlanApply("work", ApplyOpts{}) + if err != nil { + t.Fatal(err) + } + if len(plan) != 2 { + t.Fatalf("plan = %+v, want 2 entries", plan) + } + byName := map[string]ApplyPlan{} + for _, p := range plan { + byName[p.Name] = p + } + if byName["xfce"].Action != "skip" || byName["xfce"].Reason != "already applied" || byName["xfce"].Version != "1.0" { + t.Errorf("xfce = %+v, want skip/already applied at 1.0", byName["xfce"]) + } + if byName["docker"].Action != "run" || byName["docker"].Reason != "never applied" { + t.Errorf("docker = %+v, want run/never applied", byName["docker"]) + } +} + +// TestCheckRecipesRejectsInstallStage pins item 6: an install-stage recipe is +// applicable but refused at add time, since no backend runs one yet. +func TestCheckRecipesRejectsInstallStage(t *testing.T) { + dir := root(t) + rd := filepath.Join(dir, "recipes", "partitioner") + if err := os.MkdirAll(rd, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(rd, "recipe.toml"), []byte("name = \"partitioner\"\nstage = \"install\"\nscript = \"install.sh\"\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(rd, "install.sh"), []byte("#!/bin/sh\n"), 0o755); err != nil { + t.Fatal(err) + } + + issues, err := CheckRecipes("alpine", "apkovl", []string{"partitioner"}) + if err != nil { + t.Fatal(err) + } + if len(issues) != 1 || !strings.Contains(issues[0].Reason, "install-stage recipes are not yet supported") { + t.Fatalf("issues = %+v, want one install-stage rejection", issues) + } +} + +// TestFilterByRunModeOrdersDependenciesFirst pins that a dependency runs +// before its dependent regardless of the order in v.Recipes. +func TestFilterByRunModeOrdersDependenciesFirst(t *testing.T) { + dir := root(t) + writeDepRecipe(t, dir, "docker", "always", nil) + writeDepRecipe(t, dir, "devtools", "always", []string{"docker"}) + + v := &config.VM{OS: "alpine"} + // devtools listed first, but docker must still run first. + kept, _, err := filterByRunMode(v, []string{"devtools", "docker"}, nil) + if err != nil { + t.Fatal(err) + } + if len(kept) != 2 || kept[0] != "docker" || kept[1] != "devtools" { + t.Errorf("kept = %v, want [docker devtools]", kept) + } +} + +// TestCycleDetectionAtApplyTime catches a cycle introduced by manifests edited +// on disk after add-time: filterByRunMode must error, not fall back to array +// order. +func TestCycleDetectionAtApplyTime(t *testing.T) { + dir := root(t) + writeDepRecipe(t, dir, "a", "always", []string{"b"}) + writeDepRecipe(t, dir, "b", "always", []string{"a"}) + + v := &config.VM{OS: "alpine"} + _, _, err := filterByRunMode(v, []string{"a", "b"}, nil) + if err == nil || !strings.Contains(err.Error(), "cycle detected") { + t.Fatalf("err = %v, want a cycle error", err) + } +} + +// TestDependencySatisfaction covers the three rules: a manual, never-applied +// dependency errors; an already-applied dependency satisfies; a dependency +// left out of the run (not on the VM here) errors with the "add it" message. +func TestDependencySatisfaction(t *testing.T) { + dir := root(t) + writeDepRecipe(t, dir, "docker", "manual", nil) + writeDepRecipe(t, dir, "devtools", "always", []string{"docker"}) + + // docker is manual and never named, so it does not run; devtools cannot. + v := &config.VM{OS: "alpine"} + _, _, err := filterByRunMode(v, []string{"devtools", "docker"}, nil) + if err == nil || !strings.Contains(err.Error(), "run = manual") { + t.Fatalf("err = %v, want the manual-dependency error", err) + } + + // docker already applied: devtools' dependency is satisfied. + v.Applied = map[string]config.AppliedRecipe{"docker": {Version: "1.0"}} + kept, _, err := filterByRunMode(v, []string{"devtools", "docker"}, nil) + if err != nil { + t.Fatal(err) + } + if len(kept) != 1 || kept[0] != "devtools" { + t.Errorf("kept = %v, want [devtools] (docker manual+applied stays skipped)", kept) + } + + // docker is not on the VM at all and not applied: the "add it" error. + v2 := &config.VM{OS: "alpine"} + _, _, err = filterByRunMode(v2, []string{"devtools"}, nil) + if err == nil || !strings.Contains(err.Error(), "add it to --recipe or apply it first") { + t.Fatalf("err = %v, want the missing-dependency error", err) + } +} + // TestFilterByRunModeSkipsOnceWithMatchingHash pins the base case: a "once" // recipe whose stored hash still matches its current script stays skipped, // even though nothing here checks Version at all. @@ -504,30 +643,33 @@ func TestApplyRecordsHashAndSkipsOnRerun(t *testing.T) { } } -// TestCheckRecipesUsesDeclaredCapabilityReason pins that a recipe declaring -// "requires: systemd" with no "os" restriction produces -// docs/design/core-api.md §4's exact example, "requires systemd, alpine -// uses openrc", from recipes.UnsupportedReason against the recipe's OWN -// declared metadata, not the generic "not offered to alpine/apkovl" the -// structural fallback would give for the same file. -// -// The filename still pins gizmo.debian.sh to debian, so recipes.List -// excludes it for alpine like any other recipe. The front matter omits -// "stoat:os" deliberately, isolating the capability check from the OS -// check UnsupportedReason also makes: this proves the reason came from -// "requires", not from the filename. -func TestCheckRecipesUsesDeclaredCapabilityReason(t *testing.T) { +// TestCheckRecipesReportsCapabilityMismatch pins that a recipe declaring +// "requires = [systemd]" with no "os" restriction reports the capability that +// alpine lacks, from recipes.MatchReason. The manifest omits "os" +// deliberately, isolating the capability check from the OS check MatchReason +// makes first: this proves the reason came from "requires", not from "os". +func TestCheckRecipesReportsCapabilityMismatch(t *testing.T) { dir := root(t) - writeRecipe(t, dir, "gizmo.debian.sh", "#!/bin/sh\n# stoat:name gizmo\n# stoat:requires systemd\nset -e\necho hi\n") + rd := filepath.Join(dir, "recipes", "gizmo") + if err := os.MkdirAll(rd, 0o755); err != nil { + t.Fatal(err) + } + toml := "name = \"gizmo\"\nrequires = [\"systemd\"]\nscript = \"install.sh\"\n" + if err := os.WriteFile(filepath.Join(rd, "recipe.toml"), []byte(toml), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(rd, "install.sh"), []byte("#!/bin/sh\necho hi\n"), 0o755); err != nil { + t.Fatal(err) + } - issues, err := CheckRecipes("alpine", "apkovl", []string{"gizmo.debian.sh"}) + issues, err := CheckRecipes("alpine", "apkovl", []string{"gizmo"}) if err != nil { t.Fatal(err) } if len(issues) != 1 { t.Fatalf("issues = %+v, want exactly 1", issues) } - want := "requires systemd, alpine uses openrc" + want := "gizmo: requires systemd, which alpine does not have" if !strings.Contains(issues[0].Reason, want) { t.Errorf("Reason = %q, want it to contain %q", issues[0].Reason, want) } diff --git a/internal/core/clone.go b/internal/core/clone.go index 1cba4fa..0114d12 100644 --- a/internal/core/clone.go +++ b/internal/core/clone.go @@ -200,13 +200,24 @@ func cloneCloud(src, clone *config.VM) error { if err != nil { return err } - var bodies []string + var scripts []cloudinit.Script for _, name := range clone.Recipes { - body, err := recipes.Read(name) + m, ok, err := recipes.ManifestFor(name) if err != nil { return fmt.Errorf("clone: reading recipe %s: %w", name, err) } - bodies = append(bodies, body) + if !ok { + return fmt.Errorf("clone: reading recipe %s: no recipe.toml", name) + } + body, err := m.ScriptContent(clone.OS) + if err != nil { + return fmt.Errorf("clone: reading recipe %s: %w", name, err) + } + scripts = append(scripts, cloudinit.Script{Name: name, Content: body}) + } + var bodies []string + if frag := cloudinit.WrapScripts(scripts); frag != "" { + bodies = []string{frag} } _, err = cloudinit.Seed(clone, pub, bodies) return err diff --git a/internal/core/core.go b/internal/core/core.go index b16194d..f2aa64b 100644 --- a/internal/core/core.go +++ b/internal/core/core.go @@ -210,6 +210,9 @@ func plan(s Spec) (*config.VM, error) { if err := checkRecipes(img.osName, img.backend, s.Recipes); err != nil { return nil, err } + if err := CheckDependencies(s.Recipes); err != nil { + return nil, err + } if err := validateDisplay(s.Display); err != nil { return nil, err @@ -333,12 +336,11 @@ func ParseSize(s string) (int64, error) { // checkRecipes refuses a Spec naming a recipe this VM cannot run, at CREATE // time rather than at first start. // -// recipes.List returns full filenames ("xfce.cloud.yaml"); recipes.Read -// expects the same. The suffix separates ssh-pushed shell recipes from -// cloud-init seed fragments, and a per-OS fragment from a shared one. Before -// this check, `stoat create x --recipes xfce` was accepted and written to -// vm.toml, then failed only on `stoat up` with "open .../recipes/xfce: no -// such file or directory". +// recipes.List returns the names offered for osName (recipe directory names +// like "xfce"), filtered by each manifest's OS and Requires. Before this +// check, `stoat create x --recipes xfce` was accepted and written to vm.toml, +// then failed only on `stoat up` with "open .../recipes/xfce: no such file or +// directory". // // The error names the recipes that are available. The failure is usually a // name that is close but not exact, and a caller cannot always read the @@ -369,6 +371,15 @@ func checkRecipes(osName, backend string, names []string) error { return fmt.Errorf("%w: recipe %q is not available for %s/%s; available: %s", ErrRecipeNotApplicable, n, osName, backend, strings.Join(available, ", ")) } + if m, mok, err := recipes.ManifestFor(n); err == nil && mok && m.Stage == "install" { + return fmt.Errorf("%w: recipe %q: %s", ErrRecipeNotApplicable, n, installStageUnsupported) + } } return nil } + +// installStageUnsupported is the reason install-stage recipes are refused at +// add time. The stage field parses and validates, but no backend runs an +// install-stage body yet; BYO ISO support will add the hooks (docs/specs +// recipe-system-fixes §6). +const installStageUnsupported = "install-stage recipes are not yet supported" diff --git a/internal/core/deps.go b/internal/core/deps.go new file mode 100644 index 0000000..6dc759a --- /dev/null +++ b/internal/core/deps.go @@ -0,0 +1,116 @@ +package core + +import ( + "fmt" + + "github.com/novusedge/stoat/internal/recipes" +) + +// loadManifests resolves each name to its manifest. A name with no recipe.toml +// errors; callers pass names recipes.List already vetted. +func loadManifests(names []string) (map[string]recipes.Manifest, error) { + manifests := make(map[string]recipes.Manifest, len(names)) + for _, name := range names { + m, ok, err := recipes.ManifestFor(name) + if err != nil { + return nil, err + } + if !ok { + return nil, fmt.Errorf("%w: recipe %q has no recipe.toml", ErrRecipeNotApplicable, name) + } + manifests[name] = m + } + return manifests, nil +} + +// CheckDependencies validates the dependency edges among names: every recipe's +// declared dependency must be present in names, and the set must have no cycle. +// It is the create-time and CLI check. Neither Create nor the CLI auto-adds a +// missing dependency; the caller lists it explicitly (docs/specs +// recipe-system-fixes §2). The TUI uses ResolveDependencies instead. +func CheckDependencies(names []string) error { + manifests, err := loadManifests(names) + if err != nil { + return err + } + present := make(map[string]bool, len(names)) + for _, n := range names { + present[n] = true + } + for _, n := range names { + for _, dep := range manifests[n].Depends { + if !present[dep] { + return fmt.Errorf("%w: %s depends on %s; add it to the recipe list", ErrRecipeNotApplicable, n, dep) + } + } + } + if _, err := recipes.TopoSort(names, manifests); err != nil { + return fmt.Errorf("%w: %v", ErrRecipeNotApplicable, err) + } + return nil +} + +// DepAddition is one dependency ResolveDependencies pulled in: the recipe to +// add, and the recipe whose depends list named it. +type DepAddition struct { + Recipe string + RequiredBy string +} + +// ResolveDependencies returns the recipes to add to names so every declared +// dependency is present, in add order. It follows depends edges transitively. +// Each added dependency must apply to osName (recipes.MatchesVM); one that does +// not errors with the reason. A cycle in the resulting set errors. The TUI +// auto-adds the returned recipes and reports each as "Added X (required by Y)". +func ResolveDependencies(osName string, names []string) ([]DepAddition, error) { + have := make(map[string]bool, len(names)) + for _, n := range names { + have[n] = true + } + manifests := make(map[string]recipes.Manifest, len(names)) + var added []DepAddition + + queue := append([]string{}, names...) + for len(queue) > 0 { + name := queue[0] + queue = queue[1:] + if _, done := manifests[name]; done { + continue + } + m, ok, err := recipes.ManifestFor(name) + if err != nil { + return nil, err + } + if !ok { + return nil, fmt.Errorf("%w: recipe %q has no recipe.toml", ErrRecipeNotApplicable, name) + } + manifests[name] = m + + for _, dep := range m.Depends { + if !have[dep] { + dm, ok, err := recipes.ManifestFor(dep) + if err != nil { + return nil, err + } + if !ok { + return nil, fmt.Errorf("%w: %s depends on %s, which is not a recipe", ErrRecipeNotApplicable, name, dep) + } + if reason := recipes.MatchReason(&dm, osName); reason != "" { + return nil, fmt.Errorf("%w: %s depends on %s: %s", ErrRecipeNotApplicable, name, dep, reason) + } + have[dep] = true + added = append(added, DepAddition{Recipe: dep, RequiredBy: name}) + } + queue = append(queue, dep) + } + } + + all := append([]string{}, names...) + for _, a := range added { + all = append(all, a.Recipe) + } + if _, err := recipes.TopoSort(all, manifests); err != nil { + return nil, fmt.Errorf("%w: %v", ErrRecipeNotApplicable, err) + } + return added, nil +} diff --git a/internal/core/deps_test.go b/internal/core/deps_test.go new file mode 100644 index 0000000..d57bf6f --- /dev/null +++ b/internal/core/deps_test.go @@ -0,0 +1,89 @@ +package core + +import ( + "errors" + "os" + "path/filepath" + "strings" + "testing" +) + +// TestCheckDependenciesMissingDep pins the CLI/create rule: a recipe whose +// dependency is not in the list errors, naming what to add. +func TestCheckDependenciesMissingDep(t *testing.T) { + dir := root(t) + writeDepRecipe(t, dir, "docker", "always", nil) + writeDepRecipe(t, dir, "devtools", "always", []string{"docker"}) + + err := CheckDependencies([]string{"devtools"}) + if err == nil || !strings.Contains(err.Error(), "add it to the recipe list") { + t.Fatalf("err = %v, want the missing-dependency error", err) + } + if !errors.Is(err, ErrRecipeNotApplicable) { + t.Errorf("err = %v, want ErrRecipeNotApplicable", err) + } + + // Both listed: no error. + if err := CheckDependencies([]string{"devtools", "docker"}); err != nil { + t.Errorf("CheckDependencies with both listed: %v", err) + } +} + +// TestCheckDependenciesCycle catches a cycle at create time. +func TestCheckDependenciesCycle(t *testing.T) { + dir := root(t) + writeDepRecipe(t, dir, "a", "always", []string{"b"}) + writeDepRecipe(t, dir, "b", "always", []string{"a"}) + + err := CheckDependencies([]string{"a", "b"}) + if err == nil || !strings.Contains(err.Error(), "cycle detected") { + t.Fatalf("err = %v, want a cycle error", err) + } +} + +// TestResolveDependenciesAutoAdds pins the TUI helper: a missing dependency +// that applies to the VM's OS comes back to be auto-added. +func TestResolveDependenciesAutoAdds(t *testing.T) { + dir := root(t) + writeDepRecipe(t, dir, "docker", "always", nil) + writeDepRecipe(t, dir, "devtools", "always", []string{"docker"}) + + added, err := ResolveDependencies("alpine", []string{"devtools"}) + if err != nil { + t.Fatal(err) + } + if len(added) != 1 || added[0].Recipe != "docker" || added[0].RequiredBy != "devtools" { + t.Errorf("added = %v, want [{docker devtools}]", added) + } + + // docker already present: nothing to add. + added, err = ResolveDependencies("alpine", []string{"devtools", "docker"}) + if err != nil { + t.Fatal(err) + } + if len(added) != 0 { + t.Errorf("added = %v, want none", added) + } +} + +// TestResolveDependenciesRejectsOSMismatch pins that a dependency that does not +// apply to the VM's OS is not silently auto-added; it errors with the reason. +func TestResolveDependenciesRejectsOSMismatch(t *testing.T) { + dir := root(t) + rd := filepath.Join(dir, "recipes", "docker") + if err := os.MkdirAll(rd, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(rd, "recipe.toml"), []byte("name = \"docker\"\nos = [\"alpine\"]\nscript = \"install.sh\"\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(rd, "install.sh"), []byte("#!/bin/sh\n"), 0o755); err != nil { + t.Fatal(err) + } + writeDepRecipe(t, dir, "devtools", "always", []string{"docker"}) + + _, err := ResolveDependencies("debian", []string{"devtools"}) + if err == nil || !strings.Contains(err.Error(), "built for alpine, not debian") { + t.Fatalf("err = %v, want an OS-mismatch error", err) + } +} diff --git a/internal/recipes/deps.go b/internal/recipes/deps.go new file mode 100644 index 0000000..7eea0d6 --- /dev/null +++ b/internal/recipes/deps.go @@ -0,0 +1,72 @@ +package recipes + +import ( + "fmt" + "strings" +) + +// TopoSort orders names so each recipe follows every recipe it depends on. +// manifests maps each name to its Manifest. A Depends edge to a name absent +// from manifests is ignored: that dependency is outside this set, and the +// caller checks it for "already applied" separately. A dependency cycle +// returns an error naming the path, e.g. +// "cycle detected: devtools -> docker -> devtools". +// +// The sort is a depth-first post-order over names in the given order, so the +// result is deterministic for a given input order. +func TopoSort(names []string, manifests map[string]Manifest) ([]string, error) { + const ( + white = iota + gray + black + ) + color := make(map[string]int, len(names)) + var order, stack []string + + var visit func(string) error + visit = func(name string) error { + color[name] = gray + stack = append(stack, name) + for _, dep := range manifests[name].Depends { + if _, ok := manifests[dep]; !ok { + continue // dependency outside this set; not ordered here + } + switch color[dep] { + case gray: + return cycleError(stack, dep) + case white: + if err := visit(dep); err != nil { + return err + } + } + } + stack = stack[:len(stack)-1] + color[name] = black + order = append(order, name) + return nil + } + + for _, name := range names { + if color[name] == white { + if err := visit(name); err != nil { + return nil, err + } + } + } + return order, nil +} + +// cycleError builds the "a -> b -> a" message from the DFS stack and the gray +// node the back edge points to. back appears once in stack; the slice from +// there to the end, plus back again, is the cycle. +func cycleError(stack []string, back string) error { + start := 0 + for i, n := range stack { + if n == back { + start = i + break + } + } + cycle := append(append([]string{}, stack[start:]...), back) + return fmt.Errorf("cycle detected: %s", strings.Join(cycle, " -> ")) +} diff --git a/internal/recipes/deps_test.go b/internal/recipes/deps_test.go new file mode 100644 index 0000000..4c796a0 --- /dev/null +++ b/internal/recipes/deps_test.go @@ -0,0 +1,80 @@ +package recipes + +import ( + "strings" + "testing" +) + +func mans(deps map[string][]string) map[string]Manifest { + m := make(map[string]Manifest, len(deps)) + for name, d := range deps { + m[name] = Manifest{Name: name, Depends: d} + } + return m +} + +// TestTopoSort pins that a dependency lands before its dependent regardless of +// input order. +func TestTopoSort(t *testing.T) { + manifests := mans(map[string][]string{ + "devtools": {"docker"}, + "docker": nil, + "xfce": nil, + }) + order, err := TopoSort([]string{"devtools", "docker", "xfce"}, manifests) + if err != nil { + t.Fatal(err) + } + pos := map[string]int{} + for i, n := range order { + pos[n] = i + } + if pos["docker"] > pos["devtools"] { + t.Errorf("order = %v, want docker before devtools", order) + } + if len(order) != 3 { + t.Errorf("order = %v, want all three recipes", order) + } +} + +// TestTopoSortIgnoresOutsideDep pins that a Depends edge to a name not in the +// set is not an error: it is a dependency checked for "already applied" +// elsewhere, not ordered here. +func TestTopoSortIgnoresOutsideDep(t *testing.T) { + manifests := mans(map[string][]string{"devtools": {"docker"}}) + order, err := TopoSort([]string{"devtools"}, manifests) + if err != nil { + t.Fatal(err) + } + if len(order) != 1 || order[0] != "devtools" { + t.Errorf("order = %v, want [devtools]", order) + } +} + +// TestCycleDetection catches a direct A->B->A cycle and names the path. +func TestCycleDetection(t *testing.T) { + manifests := mans(map[string][]string{ + "devtools": {"docker"}, + "docker": {"devtools"}, + }) + _, err := TopoSort([]string{"devtools", "docker"}, manifests) + if err == nil { + t.Fatal("want a cycle error, got nil") + } + if !strings.Contains(err.Error(), "cycle detected") { + t.Errorf("err = %q, want a \"cycle detected\" message", err) + } + // The path names both recipes and closes the loop. + if !strings.Contains(err.Error(), "devtools") || !strings.Contains(err.Error(), "docker") { + t.Errorf("err = %q, want it to name both recipes in the cycle", err) + } +} + +// TestCycleDetectionSelfLoop catches a recipe that depends on itself. +func TestCycleDetectionSelfLoop(t *testing.T) { + manifests := mans(map[string][]string{"loop": {"loop"}}) + _, err := TopoSort([]string{"loop"}, manifests) + if err == nil || !strings.Contains(err.Error(), "cycle detected") { + t.Fatalf("err = %v, want a cycle error", err) + } +} diff --git a/internal/recipes/manifest.go b/internal/recipes/manifest.go index 65916da..080e20b 100644 --- a/internal/recipes/manifest.go +++ b/internal/recipes/manifest.go @@ -4,6 +4,7 @@ import ( "fmt" "os" "path/filepath" + "strings" "github.com/BurntSushi/toml" ) @@ -24,6 +25,7 @@ type Manifest struct { Run string `toml:"run"` // "once" | "always" | "manual" Reboot bool `toml:"reboot"` // guest needs a reboot after this recipe to take effect Runtime string `toml:"runtime"` // "sh" | "python3", the interpreter the script runs under + Depends []string `toml:"depends"` // recipe names that must run before this one dir string // recipe directory, set by ParseManifest; scripts resolve against it } @@ -74,14 +76,12 @@ func ParseManifest(path string) (Manifest, error) { } // ManifestFor resolves name (an entry in the recipes root, the same -// identifier VM.Recipes/ApplyOpts.Only use) to its recipe.toml manifest, v2's -// replacement for the old flat "..sh" files (docs/recipe-spec-v2.md). +// identifier VM.Recipes/ApplyOpts.Only use) to its recipe.toml manifest +// (docs/recipe-spec-v2.md). // -// ok is false with a nil error when name has no recipe.toml at all. That is -// not a failure: name is a v1 flat-file recipe, or an unrelated, nonexistent -// name that CheckRecipes/List already reject elsewhere. A caller uses ok to -// fall back to the old "always run it, no version tracking" behaviour, -// instead of treating absence as broken. A recipe.toml that exists but +// ok is false with a nil error when name has no recipe.toml at all: an +// unrelated or nonexistent name that CheckRecipes/List reject elsewhere. A +// caller decides what absence means for it. A recipe.toml that exists but // fails to parse is a real problem, and comes back as err instead. func ManifestFor(name string) (m Manifest, ok bool, err error) { path := filepath.Join(dir(), name, "recipe.toml") @@ -141,6 +141,15 @@ func hasCapability(cap, vmOS string) bool { // must either be empty (no restriction) or list vmOS, and every capability // in m.Requires must resolve against vmOS per capabilityOSes. func MatchesVM(m *Manifest, vmOS string) bool { + return MatchReason(m, vmOS) == "" +} + +// MatchReason explains why m's recipe does not apply to a VM running vmOS, or +// returns "" if it does. It is the reason-string form of MatchesVM: the OS +// restriction is checked first, then each capability in Requires in order, +// stopping at the first failure. CheckRecipes turns the returned reason into +// the message a caller reads. +func MatchReason(m *Manifest, vmOS string) string { if len(m.OS) > 0 { ok := false for _, o := range m.OS { @@ -150,15 +159,15 @@ func MatchesVM(m *Manifest, vmOS string) bool { } } if !ok { - return false + return fmt.Sprintf("built for %s, not %s", strings.Join(m.OS, ", "), vmOS) } } for _, cap := range m.Requires { if !hasCapability(cap, vmOS) { - return false + return fmt.Sprintf("requires %s, which %s does not have", cap, vmOS) } } - return true + return "" } diff --git a/internal/recipes/manifest_test.go b/internal/recipes/manifest_test.go index a2acaa0..c564bff 100644 --- a/internal/recipes/manifest_test.go +++ b/internal/recipes/manifest_test.go @@ -74,6 +74,33 @@ alpine = "install-alpine.sh" } } +// TestParseManifestDependsField pins that depends parses into a string slice, +// and defaults to empty when absent. +func TestParseManifestDependsField(t *testing.T) { + dir := t.TempDir() + path := writeManifestFile(t, dir, ` +name = "devtools" +script = "install.sh" +depends = ["docker", "tailscale"] +`) + m, err := ParseManifest(path) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !slices.Equal(m.Depends, []string{"docker", "tailscale"}) { + t.Errorf("Depends = %v, want [docker tailscale]", m.Depends) + } + + bare := writeManifestFile(t, t.TempDir(), "name = \"solo\"\nscript = \"install.sh\"\n") + m, err = ParseManifest(bare) + if err != nil { + t.Fatal(err) + } + if len(m.Depends) != 0 { + t.Errorf("Depends = %v, want empty when the field is absent", m.Depends) + } +} + func TestParseManifestDefaults(t *testing.T) { dir := t.TempDir() path := writeManifestFile(t, dir, ` diff --git a/internal/recipes/metadata.go b/internal/recipes/metadata.go deleted file mode 100644 index 1cb0620..0000000 --- a/internal/recipes/metadata.go +++ /dev/null @@ -1,185 +0,0 @@ -package recipes - -import ( - "fmt" - "regexp" - "strings" - - "github.com/novusedge/stoat/internal/guest" -) - -// Metadata is a phase-2 shell recipe's declared front matter -// (docs/design/guest-subsystem.md §5): who it is, which OSes it applies to, -// what guest capabilities it needs, and what stages it runs in. A recipe -// with no front-matter block at all parses to the zero Metadata: that is -// not an error, it just declares nothing (see List, which already treats an -// unmetadata'd recipe as always offered). -type Metadata struct { - Name string - Description string - OS []string // empty means "no OS restriction declared" - Requires []string // capability names, resolved against guest.OS - Stages []string -} - -// stoatTag matches one front-matter line: "#", optional whitespace, -// "stoat:", then the rest of the line, key and value still combined. -var stoatTag = regexp.MustCompile(`^#\s*stoat:(.*)$`) - -// metadataKeys are the front-matter keys ParseMetadata understands. An -// unrecognised key is reported, not ignored: a recipe that declares -// something wrong needs to be visible, not silently treated as declaring -// nothing (docs/design/guest-subsystem.md §5). -var metadataKeys = map[string]bool{ - "name": true, "description": true, "os": true, "requires": true, "stages": true, -} - -// ParseMetadata reads the front-matter block from a recipe body: "# stoat: -// key value" comment lines starting right after an optional shebang and -// ending at the first line that is neither blank nor a comment. A -// "# stoat:" tag past that point is not front matter. Front matter means -// front: it is positional, not "anywhere in the file". A late tag is left -// as an ordinary comment, not honoured and not reported. -// -// Every other problem in the block is reported, not swallowed: an unknown -// key, a key declared twice, a tag with no key at all, and a key with no -// value. These are all parse errors, collected and returned together rather -// than stopping at the first one. -func ParseMetadata(body string) (Metadata, error) { - var m Metadata - var errs []string - seen := map[string]bool{} - - for i, line := range strings.Split(body, "\n") { - trimmed := strings.TrimSpace(line) - if i == 0 && strings.HasPrefix(trimmed, "#!") { - continue // shebang: not front matter, doesn't end it either - } - if trimmed == "" { - continue // blank lines don't end the block - } - match := stoatTag.FindStringSubmatch(trimmed) - if match == nil { - if !strings.HasPrefix(trimmed, "#") { - break // first non-comment line ends front matter - } - continue // an ordinary comment, still inside the block - } - - rest := strings.TrimSpace(match[1]) - if rest == "" { - errs = append(errs, fmt.Sprintf("malformed line %q: no key after \"stoat:\"", trimmed)) - continue - } - key, value := rest, "" - if idx := strings.IndexAny(rest, " \t"); idx >= 0 { - key, value = rest[:idx], strings.TrimSpace(rest[idx:]) - } - - if !metadataKeys[key] { - errs = append(errs, fmt.Sprintf("unknown key %q", key)) - continue - } - if seen[key] { - errs = append(errs, fmt.Sprintf("duplicate key %q", key)) - continue - } - seen[key] = true - if value == "" { - errs = append(errs, fmt.Sprintf("key %q has no value", key)) - continue - } - - switch key { - case "name": - m.Name = value - case "description": - m.Description = value - case "os": - m.OS = splitMetadataList(value) - case "requires": - m.Requires = splitMetadataList(value) - case "stages": - m.Stages = splitMetadataList(value) - } - } - - if len(errs) > 0 { - return Metadata{}, fmt.Errorf("recipe front matter: %s", strings.Join(errs, "; ")) - } - return m, nil -} - -// splitMetadataList parses a comma-separated front-matter value ("alpine, -// ubuntu, debian"). Empty items from stray commas are dropped rather than -// reported: a formatting slip here isn't the class of error §5 asks to be -// caught, unlike an unknown key or a missing value. -func splitMetadataList(s string) []string { - var out []string - for _, part := range strings.Split(s, ",") { - if p := strings.TrimSpace(part); p != "" { - out = append(out, p) - } - } - return out -} - -// ReadMetadata reads and parses name's front matter. name is the full -// filename, exactly as Read and List expect it. -func ReadMetadata(name string) (Metadata, error) { - body, err := Read(name) - if err != nil { - return Metadata{}, err - } - return ParseMetadata(body) -} - -// capabilities are the guest capabilities a recipe's "requires" tag can -// name, and how each resolves against a guest.OS. Kept to exactly what a -// bundled recipe declares (see the annotated .sh files); add an entry here -// when a recipe actually needs a new one. -var capabilities = map[string]func(guest.OS) bool{ - "systemd": func(g guest.OS) bool { return g.Init == guest.InitSystemd }, -} - -// UnsupportedReason explains why osName cannot run a recipe declaring m, or -// returns "" if it can. os is checked first (a named mismatch), then each -// capability in Requires in order, stopping at the first failure: one -// reason per recipe, matching core.RecipeIssue's shape. -func UnsupportedReason(osName string, m Metadata) string { - if len(m.OS) > 0 { - ok := false - for _, o := range m.OS { - if o == osName { - ok = true - break - } - } - if !ok { - return fmt.Sprintf("built for %s, not %s", strings.Join(m.OS, ", "), osName) - } - } - - g, known := guest.Lookup(osName) - if !known { - if len(m.Requires) > 0 { - return fmt.Sprintf("%q is not a recognised OS", osName) - } - return "" - } - for _, cap := range m.Requires { - check, ok := capabilities[cap] - if !ok { - return fmt.Sprintf("requires unknown capability %q", cap) - } - if !check(g) { - switch cap { - case "systemd": - return fmt.Sprintf("requires systemd, %s uses %s", g.Name, g.Init) - default: - return fmt.Sprintf("requires %s, %s does not have it", cap, g.Name) - } - } - } - return "" -} diff --git a/internal/recipes/metadata_test.go b/internal/recipes/metadata_test.go deleted file mode 100644 index cea2a0a..0000000 --- a/internal/recipes/metadata_test.go +++ /dev/null @@ -1,188 +0,0 @@ -package recipes - -import ( - "slices" - "strings" - "testing" -) - -func TestParseMetadataMissingBlock(t *testing.T) { - // No "# stoat:" tags at all: a recipe predating the contract, or one - // that simply declares nothing. Zero Metadata, no error. - m, err := ParseMetadata("#!/bin/sh\n# just a plain comment\nset -e\necho hi\n") - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if m.Name != "" || m.Description != "" || m.OS != nil || m.Requires != nil || m.Stages != nil { - t.Errorf("m = %+v, want zero value", m) - } -} - -func TestParseMetadataFullBlock(t *testing.T) { - body := "#!/bin/sh\n" + - "# stoat:name xfce\n" + - "# stoat:description XFCE desktop with a graphical login\n" + - "# stoat:os alpine, ubuntu, debian, arch\n" + - "# stoat:requires systemd\n" + - "# stoat:stages install, configure, enable\n" + - "set -e\n" - m, err := ParseMetadata(body) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if m.Name != "xfce" { - t.Errorf("Name = %q, want xfce", m.Name) - } - if m.Description != "XFCE desktop with a graphical login" { - t.Errorf("Description = %q", m.Description) - } - if want := []string{"alpine", "ubuntu", "debian", "arch"}; !slices.Equal(m.OS, want) { - t.Errorf("OS = %v, want %v", m.OS, want) - } - if want := []string{"systemd"}; !slices.Equal(m.Requires, want) { - t.Errorf("Requires = %v, want %v", m.Requires, want) - } - if want := []string{"install", "configure", "enable"}; !slices.Equal(m.Stages, want) { - t.Errorf("Stages = %v, want %v", m.Stages, want) - } -} - -func TestParseMetadataUnknownKey(t *testing.T) { - _, err := ParseMetadata("#!/bin/sh\n# stoat:flavor spicy\nset -e\n") - if err == nil || !strings.Contains(err.Error(), `unknown key "flavor"`) { - t.Errorf("err = %v, want an unknown-key error", err) - } -} - -func TestParseMetadataDuplicateKey(t *testing.T) { - body := "#!/bin/sh\n# stoat:name a\n# stoat:name b\nset -e\n" - _, err := ParseMetadata(body) - if err == nil || !strings.Contains(err.Error(), `duplicate key "name"`) { - t.Errorf("err = %v, want a duplicate-key error", err) - } -} - -func TestParseMetadataMalformedLine(t *testing.T) { - // "# stoat:" with nothing after it at all, no key and not even an empty - // one, is malformed. Distinct from a known key with an empty value. - _, err := ParseMetadata("#!/bin/sh\n# stoat:\nset -e\n") - if err == nil || !strings.Contains(err.Error(), "no key") { - t.Errorf("err = %v, want a malformed-line error", err) - } -} - -func TestParseMetadataEmptyValue(t *testing.T) { - _, err := ParseMetadata("#!/bin/sh\n# stoat:name\nset -e\n") - if err == nil || !strings.Contains(err.Error(), `"name" has no value`) { - t.Errorf("err = %v, want an empty-value error", err) - } -} - -func TestParseMetadataTagsAfterFrontMatterAreNotHonoured(t *testing.T) { - // A tag appearing after the first non-comment, non-blank line is not - // front matter. It is left as an ordinary comment, not parsed and not - // reported as an error either. - body := "#!/bin/sh\nset -e\n# stoat:name too-late\n" - m, err := ParseMetadata(body) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if m.Name != "" { - t.Errorf("Name = %q, want empty: the tag came after front matter", m.Name) - } -} - -func TestParseMetadataCollectsMultipleErrors(t *testing.T) { - body := "#!/bin/sh\n# stoat:flavor spicy\n# stoat:name\nset -e\n" - _, err := ParseMetadata(body) - if err == nil { - t.Fatal("expected an error") - } - if !strings.Contains(err.Error(), `unknown key "flavor"`) || !strings.Contains(err.Error(), `"name" has no value`) { - t.Errorf("err = %v, want both problems reported", err) - } -} - -func TestParseMetadataBlankLinesDoNotEndFrontMatter(t *testing.T) { - body := "#!/bin/sh\n\n# stoat:name xfce\n\nset -e\n" - m, err := ParseMetadata(body) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if m.Name != "xfce" { - t.Errorf("Name = %q, want xfce", m.Name) - } -} - -func TestUnsupportedReasonOS(t *testing.T) { - m := Metadata{OS: []string{"ubuntu", "debian"}} - if r := UnsupportedReason("alpine", m); r == "" { - t.Error("want a reason, alpine is not in the declared os list") - } - if r := UnsupportedReason("ubuntu", m); r != "" { - t.Errorf("want no reason, got %q", r) - } -} - -func TestUnsupportedReasonRequiresSystemd(t *testing.T) { - m := Metadata{Requires: []string{"systemd"}} - // The exact wording from docs/design/guest-subsystem.md §5. - if got, want := UnsupportedReason("alpine", m), "requires systemd, alpine uses openrc"; got != want { - t.Errorf("UnsupportedReason(alpine) = %q, want %q", got, want) - } - if r := UnsupportedReason("ubuntu", m); r != "" { - t.Errorf("want no reason for ubuntu, got %q", r) - } -} - -// TestUnsupportedReasonSystemdPerOS covers every registry OS against the -// systemd capability: only alpine (OpenRC) must be rejected. -func TestUnsupportedReasonSystemdPerOS(t *testing.T) { - m := Metadata{Requires: []string{"systemd"}} - for _, tc := range []struct { - os string - rejects bool - }{ - {"alpine", true}, - {"ubuntu", false}, - {"debian", false}, - {"fedora", false}, - {"arch", false}, - } { - r := UnsupportedReason(tc.os, m) - if tc.rejects && r == "" { - t.Errorf("%s: want a reason, has no systemd", tc.os) - } - if !tc.rejects && r != "" { - t.Errorf("%s: want no reason, got %q", tc.os, r) - } - } -} - -func TestUnsupportedReasonUnknownOS(t *testing.T) { - // A BYO image whose OS string isn't in the registry: a "requires" - // capability can't be verified, so it's rejected rather than assumed. - if r := UnsupportedReason("plan9", Metadata{Requires: []string{"systemd"}}); r == "" { - t.Error("want a reason, plan9 is not a recognised OS") - } - // No requires at all: nothing to verify, so an unknown OS is not blocked. - if r := UnsupportedReason("plan9", Metadata{}); r != "" { - t.Errorf("want no reason with no requires, got %q", r) - } -} - -func TestUnsupportedReasonUnknownCapability(t *testing.T) { - m := Metadata{Requires: []string{"warp-drive"}} - if r := UnsupportedReason("ubuntu", m); !strings.Contains(r, `unknown capability "warp-drive"`) { - t.Errorf("r = %q, want an unknown-capability reason", r) - } -} - -func TestUnsupportedReasonNone(t *testing.T) { - if r := UnsupportedReason("ubuntu", Metadata{}); r != "" { - t.Errorf("want no reason for empty metadata, got %q", r) - } -} - -// v2 recipes use recipe.toml for metadata, not in-file front-matter. -// See manifest_test.go for v2 capability/OS filtering tests. diff --git a/internal/recipes/recipes.go b/internal/recipes/recipes.go index 7d8fafd..4a7a2e9 100644 --- a/internal/recipes/recipes.go +++ b/internal/recipes/recipes.go @@ -22,10 +22,6 @@ var bundled embed.FS func dir() string { return filepath.Join(config.Root(), "recipes") } -// Path is the on-disk location of a recipe. name is the full filename -// (including its .sh or .yaml extension) as returned by List. -func Path(name string) string { return filepath.Join(dir(), name) } - // ManifestName is the file in the recipes directory recording the checksum // of every recipe stoat itself wrote there. It lets Install tell "this is // stoat's copy, from an older release" from "the user edited this", which a @@ -176,11 +172,20 @@ func sweepV1() error { return nil } + man := readManifest() attic := filepath.Join(dir(), v1AtticName) if err := os.MkdirAll(attic, 0o755); err != nil { return err } for _, name := range stale { + // A flat file whose contents still match what stoat recorded writing is + // stoat's own v1 copy, swept quietly. Anything else is a recipe the + // user wrote or edited; warn, so a swept recipe does not just vanish + // from the picker with no explanation. + if !sweptIsStoats(name, man) { + logx.L().Warn("moved a legacy recipe out of the recipes directory; convert it to the v2 format to keep using it", + "recipe", name, "moved_to", v1AtticName, "docs", "docs/writing-recipes.md") + } // An existing attic entry from an earlier sweep wins: it is the older // copy, and overwriting it with a file the user has since re-created // would lose the thing worth keeping. @@ -198,6 +203,21 @@ func sweepV1() error { return nil } +// sweptIsStoats reports whether the flat file name is stoat's own v1 copy: its +// contents still match the checksum man recorded. A file man does not list, or +// one whose contents changed, is treated as the user's. +func sweptIsStoats(name string, man map[string]string) bool { + want, ok := man[name] + if !ok { + return false + } + b, err := os.ReadFile(filepath.Join(dir(), name)) + if err != nil { + return false + } + return sum(b) == want +} + // installDir installs every file under a v2 recipe directory (name), keyed // by its path relative to the recipes root ("xfce/recipe.toml"), mirroring // that directory structure into dir(). @@ -281,8 +301,7 @@ func List(osName, _ string) ([]string, error) { // ListManifests scans dir() for v2 recipes: subdirectories holding a // recipe.toml (docs/recipe-spec-v2.md). Unlike List, it does not filter by // OS or backend. A caller that needs that filters against the parsed -// Manifest's OS/Requires fields, the way UnsupportedReason already does for -// v1's front-matter Metadata. +// Manifest's OS/Requires fields (see MatchesVM). // // A subdirectory with no recipe.toml, a stray directory or leftover .bak // territory, is silently skipped. A directory that is a recipe but fails to @@ -317,38 +336,30 @@ func ListManifests() ([]Manifest, error) { return out, nil } -// Read returns a v1 flat-file recipe's body. name is a filename directly -// under the recipes root. A v2 recipe is a directory, not a file, so callers -// with a guest OS in hand use ScriptBody instead. -func Read(name string) (string, error) { - b, err := os.ReadFile(Path(name)) - return string(b), err -} - -// ScriptBody returns the script a recipe runs on osName. A v2 recipe resolves +// ScriptBody returns the script a recipe runs on osName. The recipe resolves // through its manifest to install.sh, or the per-OS override the manifest -// declares. A name with no recipe.toml is a v1 flat file, read as-is. +// declares. A name with no recipe.toml is not a recipe and returns an error. func ScriptBody(name, osName string) (string, error) { m, ok, err := ManifestFor(name) if err != nil { return "", err } if !ok { - return Read(name) + return "", fmt.Errorf("no such recipe %q", name) } return m.ScriptContent(osName) } // RuntimeFor returns the interpreter that runs name's script: the manifest's -// Runtime field, or "sh" for a v1 flat file, which has no manifest to -// declare one. +// Runtime field. A name with no recipe.toml is not a recipe and returns an +// error. func RuntimeFor(name, osName string) (string, error) { m, ok, err := ManifestFor(name) if err != nil { return "", err } if !ok { - return "sh", nil + return "", fmt.Errorf("no such recipe %q", name) } return m.Runtime, nil } diff --git a/internal/recipes/recipes_test.go b/internal/recipes/recipes_test.go index 1a50a85..7add22c 100644 --- a/internal/recipes/recipes_test.go +++ b/internal/recipes/recipes_test.go @@ -353,43 +353,6 @@ func TestScriptBodyResolvesV2Directory(t *testing.T) { } } -// TestScriptBodyReadsV1FlatFile keeps the old format working: a name with no -// recipe.toml is a v1 flat file, read as-is. -func TestScriptBodyReadsV1FlatFile(t *testing.T) { - t.Setenv("STOAT_HOME", t.TempDir()) - if err := os.MkdirAll(dir(), 0o755); err != nil { - t.Fatal(err) - } - want := "# v1 legacy\n" - if err := os.WriteFile(filepath.Join(dir(), "legacy.sh"), []byte(want), 0o644); err != nil { - t.Fatal(err) - } - got, err := ScriptBody("legacy.sh", "alpine") - if err != nil { - t.Fatalf("ScriptBody: %v", err) - } - if got != want { - t.Errorf("ScriptBody = %q, want %q", got, want) - } -} - -func TestRuntimeForV1FlatFile(t *testing.T) { - t.Setenv("STOAT_HOME", t.TempDir()) - if err := os.MkdirAll(dir(), 0o755); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(filepath.Join(dir(), "legacy.sh"), []byte("echo hi\n"), 0o644); err != nil { - t.Fatal(err) - } - got, err := RuntimeFor("legacy.sh", "alpine") - if err != nil { - t.Fatalf("RuntimeFor: %v", err) - } - if got != "sh" { - t.Errorf("RuntimeFor(legacy.sh) = %q, want sh", got) - } -} - func TestRuntimeForManifestPython3(t *testing.T) { t.Setenv("STOAT_HOME", t.TempDir()) rd := filepath.Join(dir(), "pyrecipe") diff --git a/internal/sshx/sshx_test.go b/internal/sshx/sshx_test.go index 3501e87..390317a 100644 --- a/internal/sshx/sshx_test.go +++ b/internal/sshx/sshx_test.go @@ -473,10 +473,14 @@ func processAlive(pid int) bool { func TestProvisionCancelKillsTheSSHProcess(t *testing.T) { root := t.TempDir() t.Setenv("STOAT_HOME", root) - if err := os.MkdirAll(filepath.Join(root, "recipes"), 0o755); err != nil { + rd := filepath.Join(root, "recipes", "long") + if err := os.MkdirAll(rd, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(rd, "recipe.toml"), []byte("name = \"long\"\nscript = \"install.sh\"\n"), 0o644); err != nil { t.Fatal(err) } - if err := os.WriteFile(filepath.Join(root, "recipes", "long.sh"), []byte("sleep 30\n"), 0o644); err != nil { + if err := os.WriteFile(filepath.Join(rd, "install.sh"), []byte("sleep 30\n"), 0o644); err != nil { t.Fatal(err) } @@ -488,7 +492,7 @@ func TestProvisionCancelKillsTheSSHProcess(t *testing.T) { // sshd, so Wait clears at once and Provision moves on to the recipe. port := acceptOnly(t, "SSH-2.0-OpenSSH_9.6\r\n") - v := &config.VM{Name: "x", SSHPort: port, Dir: vmDir, Recipes: []string{"long.sh"}} + v := &config.VM{Name: "x", SSHPort: port, Dir: vmDir, OS: "alpine", Recipes: []string{"long"}} ctx, cancel := context.WithCancel(context.Background()) done := make(chan struct{}) diff --git a/internal/tui/autoprov_test.go b/internal/tui/autoprov_test.go index b560ab0..486844e 100644 --- a/internal/tui/autoprov_test.go +++ b/internal/tui/autoprov_test.go @@ -1,6 +1,7 @@ package tui import ( + "os" "path/filepath" "testing" @@ -9,8 +10,23 @@ import ( "github.com/novusedge/stoat/internal/core" ) +// autoVM builds a VM fixture and installs a resolvable v2 "xfce" recipe under +// a fresh STOAT_HOME, so core.NeedsProvision's filterByRunMode finds a real +// recipe.toml for the "xfce" entry the fixtures use. func autoVM(t *testing.T, mode string, recipes []string) core.VM { t.Helper() + home := t.TempDir() + t.Setenv("STOAT_HOME", home) + rd := filepath.Join(home, "recipes", "xfce") + if err := os.MkdirAll(rd, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(rd, "recipe.toml"), []byte("name = \"xfce\"\nscript = \"install.sh\"\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(rd, "install.sh"), []byte("#!/bin/sh\necho xfce\n"), 0o755); err != nil { + t.Fatal(err) + } dir := t.TempDir() return core.VM{ Name: "vm", Mode: mode, OS: "alpine", Installed: true, @@ -25,7 +41,7 @@ func autoVM(t *testing.T, mode string, recipes []string) core.VM { // is a tmpfs overlay, so a previous run is genuinely gone after the reboot, // while a disk VM's packages persist and its Applied record can be trusted. func TestNeedsAutoProvision(t *testing.T) { - recipes := []string{"xfce.alpine.sh"} + recipes := []string{"xfce"} cases := []struct { name string @@ -58,7 +74,7 @@ func TestNeedsAutoProvision(t *testing.T) { // the host, survives, but it describes a filesystem that is gone; a live // VM auto-applies every boot regardless of what Applied says. live := autoVM(t, "live", recipes) - live.Applied = map[string]core.AppliedRecipe{"xfce.alpine.sh": {Version: "1.0", Hash: "whatever"}} + live.Applied = map[string]core.AppliedRecipe{"xfce": {Version: "1.0", Hash: "whatever"}} if !needsAutoProvision(live) { t.Error("a live VM must auto-provision every boot even with a stale Applied record") } @@ -75,7 +91,7 @@ func TestNeedsAutoProvision(t *testing.T) { // TestSSHReadyAutoProvisions is the sshReadyMsg handler's main path: it // starts a provision run itself, with no confirmation state in between. func TestSSHReadyAutoProvisions(t *testing.T) { - v := autoVM(t, "live", []string{"xfce.alpine.sh"}) + v := autoVM(t, "live", []string{"xfce"}) m := model{screen: screenList, list: newVMList(), spin: newSpinner(), provisioning: map[string]provState{}, vms: []core.VM{v}} @@ -134,7 +150,7 @@ func batchLen(t *testing.T, cmd tea.Cmd) int { // already excludes this exact case, so a regression here would fall // through to the plain started+loadVMs batch instead). func TestVMStartedForUninstalledDiskVMAwaitsInstall(t *testing.T) { - v := autoVM(t, "disk", []string{"xfce.alpine.sh"}) + v := autoVM(t, "disk", []string{"xfce"}) v.Installed = false m := model{screen: screenList, list: newVMList(), spin: newSpinner(), provisioning: map[string]provState{}, vms: []core.VM{v}} @@ -173,7 +189,7 @@ func TestVMStartedForInstalledVMSkipsInstallWatch(t *testing.T) { // back up and installed, the handler must hand off into the same awaitSSH // watch a directly-installed VM gets from vmStartedMsg. func TestInstallRestartedChainsToAwaitSSH(t *testing.T) { - v := autoVM(t, "disk", []string{"xfce.alpine.sh"}) + v := autoVM(t, "disk", []string{"xfce"}) m := model{screen: screenList, list: newVMList(), spin: newSpinner(), provisioning: map[string]provState{}, vms: []core.VM{v}} @@ -199,7 +215,7 @@ func TestInstallRestartedIgnoresStaleVM(t *testing.T) { // TestSSHReadyNeverPreemptsADeletePrompt: a pending delete confirmation is a // more consequential question than a background watch resuming. func TestInstallRestartedRespectsPendingDelete(t *testing.T) { - v := autoVM(t, "disk", []string{"xfce.alpine.sh"}) + v := autoVM(t, "disk", []string{"xfce"}) m := model{screen: screenList, list: newVMList(), spin: newSpinner(), provisioning: map[string]provState{}, vms: []core.VM{v}, pendingDelete: &v, status: "delete vm? y/N"} @@ -219,7 +235,7 @@ func TestInstallRestartedRespectsPendingDelete(t *testing.T) { // timer must not clear the status line out from under a pending delete // confirmation, a more consequential question the user is mid-answer. func TestSSHReadyNeverPreemptsADeletePrompt(t *testing.T) { - v := autoVM(t, "live", []string{"xfce.alpine.sh"}) + v := autoVM(t, "live", []string{"xfce"}) m := model{screen: screenList, list: newVMList(), spin: newSpinner(), provisioning: map[string]provState{}, vms: []core.VM{v}, pendingDelete: &v, status: "delete vm? y/N"} diff --git a/internal/tui/deps.go b/internal/tui/deps.go new file mode 100644 index 0000000..b6e8d57 --- /dev/null +++ b/internal/tui/deps.go @@ -0,0 +1,47 @@ +package tui + +import ( + "errors" + "fmt" + "strings" + + "github.com/novusedge/stoat/internal/core" +) + +// resolveDeps wraps core.ResolveDependencies for the create form and the +// edit screen's recipe checkboxes. The "recipe not applicable: " prefix +// core.ErrRecipeNotApplicable carries is meant for CLI output stacked with +// other errors; the TUI's toast already frames the message as an error, so +// the prefix is redundant here. +func resolveDeps(osName string, selected []string) ([]core.DepAddition, error) { + added, err := core.ResolveDependencies(osName, selected) + if err != nil { + return nil, errors.New(strings.TrimPrefix(err.Error(), "recipe not applicable: ")) + } + return added, nil +} + +// depMessage renders ResolveDependencies' additions as the toast text a +// recipe checkbox toggle shows. Checking a recipe can pull in more than one +// dependency transitively, so every addition is reported, not just the +// first. +func depMessage(added []core.DepAddition) string { + parts := make([]string, len(added)) + for i, a := range added { + parts[i] = fmt.Sprintf("Added %s (required by %s)", a.Recipe, a.RequiredBy) + } + return strings.Join(parts, "; ") +} + +// selectedNames is the recipe names currently checked in sel, in names' +// order. Both formModel and editModel key their selection off the recipe +// name rather than index, so this helper works for either. +func selectedNames(names []string, sel map[string]bool) []string { + out := make([]string, 0, len(names)) + for _, n := range names { + if sel[n] { + out = append(out, n) + } + } + return out +} diff --git a/internal/tui/edit.go b/internal/tui/edit.go index f5ff6a9..0e586d8 100644 --- a/internal/tui/edit.go +++ b/internal/tui/edit.go @@ -402,8 +402,22 @@ func (m model) updateEdit(msg tea.Msg) (tea.Model, tea.Cmd) { case keySpace: if m.edit.focus == eRecipes && len(m.edit.recipeNames) > 0 { n := m.edit.recipeNames[m.edit.recipeIdx] - m.edit.recipeSel[n] = !m.edit.recipeSel[n] - return m, nil + if m.edit.recipeSel[n] { + m.edit.recipeSel[n] = false + return m, nil + } + // Same auto-add rule as the create form: see form.go's + // fRecipes handler. + pending := append(selectedNames(m.edit.recipeNames, m.edit.recipeSel), n) + added, err := resolveDeps(m.edit.vm.OS, pending) + if err != nil { + return m, m.showToast(err.Error(), true) + } + m.edit.recipeSel[n] = true + for _, a := range added { + m.edit.recipeSel[a.Recipe] = true + } + return m, m.showToast(depMessage(added), false) } case "enter": p, err := m.edit.buildPatch() diff --git a/internal/tui/edit_test.go b/internal/tui/edit_test.go index 03fdff1..d8d01d0 100644 --- a/internal/tui/edit_test.go +++ b/internal/tui/edit_test.go @@ -1,7 +1,9 @@ package tui import ( + "os" "os/exec" + "path/filepath" "strings" "testing" @@ -427,6 +429,77 @@ func TestEditChangeMarkers(t *testing.T) { } } +// writeEditTestRecipe writes a v2 recipe under dir/recipes/name. depends is +// written verbatim as a TOML string array, matching core's writeDepRecipe +// fixture (internal/core/apply_test.go) for the same manifest shape. +func writeEditTestRecipe(t *testing.T, dir, name string, depends []string) { + t.Helper() + recipeDir := filepath.Join(dir, "recipes", name) + if err := os.MkdirAll(recipeDir, 0o755); err != nil { + t.Fatal(err) + } + quoted := make([]string, len(depends)) + for i, d := range depends { + quoted[i] = "\"" + d + "\"" + } + toml := "name = \"" + name + "\"\n" + + "script = \"install.sh\"\n" + + "depends = [" + strings.Join(quoted, ", ") + "]\n" + if err := os.WriteFile(filepath.Join(recipeDir, "recipe.toml"), []byte(toml), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(recipeDir, "install.sh"), []byte("#!/bin/sh\n"), 0o755); err != nil { + t.Fatal(err) + } +} + +// TestEditRecipeToggleAutoAddsDependency pins the auto-add rule from +// docs/specs/2026-08-10-recipe-system-fixes-design.md §2 "TUI Behavior": +// checking a recipe that depends on one not yet selected pulls the +// dependency in and reports it as a toast. +func TestEditRecipeToggleAutoAddsDependency(t *testing.T) { + dir := t.TempDir() + t.Setenv("STOAT_HOME", dir) + if err := os.MkdirAll(filepath.Join(dir, "isos"), 0o755); err != nil { + t.Fatal(err) + } + writeEditTestRecipe(t, dir, "docker", nil) + writeEditTestRecipe(t, dir, "devtools", []string{"docker"}) + + v := &config.VM{ + Name: "work", Mode: "disk", OS: "alpine", Backend: "apkovl", + ISO: "isos/alpine-standard-3.24.1-x86_64.iso", + RAM: 4096, CPUs: 4, Disk: "8G", Share: "~/vms", SSHPort: 2200, + } + if err := v.Save(); err != nil { + t.Fatalf("save fixture vm: %v", err) + } + e := newEdit(v) + e.focus = eRecipes + for i, n := range e.recipeNames { + if n == "devtools" { + e.recipeIdx = i + } + } + + m := model{screen: screenEdit, edit: e} + mm, cmd := m.updateEdit(keyMsg(keySpace)) + m = mm.(model) + + if !m.edit.recipeSel["devtools"] { + t.Error("devtools was not checked") + } + if !m.edit.recipeSel["docker"] { + t.Error("docker was not auto-added") + } + if cmd == nil { + t.Fatal("no toast Cmd returned") + } + if !strings.Contains(m.toast.text, "Added docker (required by devtools)") { + t.Errorf("toast = %q, want it to report the auto-added dependency", m.toast.text) + } +} + // TestEditModeRowRemoved pins the feature removal (D3): mode is immutable in // core.Update, so the edit pane must no longer offer a live/disk/cloud row. func TestEditModeRowRemoved(t *testing.T) { diff --git a/internal/tui/form.go b/internal/tui/form.go index 125d99a..26163e6 100644 --- a/internal/tui/form.go +++ b/internal/tui/form.go @@ -675,8 +675,24 @@ func (m model) updateForm(msg tea.Msg) (tea.Model, tea.Cmd) { if m.form.recipeSel == nil { m.form.recipeSel = map[string]bool{} } - m.form.recipeSel[name] = !m.form.recipeSel[name] - return m, nil + if m.form.recipeSel[name] { + m.form.recipeSel[name] = false + return m, nil + } + // Checking a box can pull in a recipe it depends on. A + // dependency that doesn't apply to this image's OS refuses + // the check entirely rather than leaving a selection + // core.Plan would reject anyway. + pending := append(selectedNames(m.form.recipeNames, m.form.recipeSel), name) + added, err := resolveDeps(m.form.resolvedOS(), pending) + if err != nil { + return m, m.showToast(err.Error(), true) + } + m.form.recipeSel[name] = true + for _, a := range added { + m.form.recipeSel[a.Recipe] = true + } + return m, m.showToast(depMessage(added), false) } // space on the image row downloads the selected catalog entry. // On an image that is already local it re-verifies the file and diff --git a/internal/tui/labels.go b/internal/tui/labels.go index a1e0e5b..d32eaef 100644 --- a/internal/tui/labels.go +++ b/internal/tui/labels.go @@ -11,14 +11,10 @@ import ( // Neither is something the user picked. Showing them raw made the UI read // like a config file. -// recipeLabel is the display name for a recipe file. Recipes live on disk as -// "..sh" or ".cloud.yaml". The suffix names the OS and -// backend the file targets. The picker has already filtered on that suffix, -// so repeating it in every row is noise. "xfce.alpine.sh" reads as "xfce". -// -// Only the display changes. VM.Recipes still stores the filename, since -// recipes.Read opens it by that name. See the recipe-naming decision in -// CHECKPOINT.md, where storing bare names was considered and rejected. +// recipeLabel is the display name for a recipe. v2 recipes are bare directory +// names ("xfce"), so the label is usually the name itself. The suffix +// stripping stays for any legacy "..sh" or ".cloud.yaml" name +// still recorded on an old VM, which reads as "xfce". func recipeLabel(file string) string { name := strings.TrimSuffix(file, ".yaml") name = strings.TrimSuffix(name, ".sh") diff --git a/internal/tui/provision.go b/internal/tui/provision.go index 6ddf041..894829f 100644 --- a/internal/tui/provision.go +++ b/internal/tui/provision.go @@ -7,7 +7,6 @@ import ( tea "charm.land/bubbletea/v2" - "github.com/novusedge/stoat/internal/backend" "github.com/novusedge/stoat/internal/core" "github.com/novusedge/stoat/internal/guest" ) @@ -46,11 +45,6 @@ func provision(v core.VM) tea.Cmd { } // No cancellation source reaches here: the TUI has no "abort // provision" key. - // - // core.Apply refuses a cloudinit-backed VM with ErrAppliedAtBoot. - // startProvision's refusal below should catch that case first, but if - // it reaches here anyway, app.go's generic err.Error() toast still - // reports it. return provisionDoneMsg{v.Name, core.Apply(context.Background(), v.Name, core.ApplyOpts{})} } } @@ -65,21 +59,6 @@ func provision(v core.VM) tea.Cmd { // touched, so it does not block a second real provision from starting right // after. func (m *model) startProvision(v core.VM) tea.Cmd { - if backend.For(cfgVM(v)).Name() == "cloudinit" { - // Keyed on the BACKEND, not v.Mode == "cloud". The edit screen's mode - // switch can produce mode="disk" with backend="cloudinit" (D9a), and - // this refusal must still catch that state. - // - // cloud-init's packages: list is baked into the seed and runs only at - // first boot. ssh-based provisioning has nothing to do there, and a - // cloud recipe is #cloud-config YAML, not a shell script, so piping - // it into `sh -s` fails. - // - // core.Apply refuses the same state with ErrAppliedAtBoot. This check - // shows the user the refusal before anything starts, instead of - // after a failed attempt. - return m.showToast(v.Name+": cloud VMs apply recipes at first boot via cloud-init. Recreate the VM to change them", true) - } // A disk VM still boots its installer ISO until its OS is on disk. sshd // there belongs to the installer, not the system being built, so // provisioning it would run recipes against a tmpfs about to be thrown diff --git a/internal/tui/provision_test.go b/internal/tui/provision_test.go index b30bb30..196d6bd 100644 --- a/internal/tui/provision_test.go +++ b/internal/tui/provision_test.go @@ -27,29 +27,6 @@ func TestInstallerName(t *testing.T) { } } -// TestStartProvisionRefusesByBackendNotMode is the regression test for D9a: -// the edit screen's mode switch can leave a VM with mode="disk" and -// backend="cloudinit", a state core.Apply refuses with ErrAppliedAtBoot. -// Keying the refusal on v.Mode == "cloud" would miss it entirely and let a -// cloud-init fragment be piped into `sh -s` over ssh as a shell script. -func TestStartProvisionRefusesByBackendNotMode(t *testing.T) { - m := model{provisioning: map[string]provState{}, spin: newSpinner()} - v := core.VM{ - Name: "mode-switched", Mode: "disk", Backend: "cloudinit", Installed: true, - SSHPort: 2203, Recipes: []string{"xfce.alpine.cloud.yaml"}, - Paths: core.Paths{Dir: t.TempDir()}, - } - - m.startProvision(v) - - if len(m.provisioning) != 0 { - t.Error("a cloudinit-backed VM was marked as provisioning anyway") - } - if !strings.Contains(m.toast.text, "cloud-init") { - t.Errorf("toast = %q, expected the cloud-init refusal", m.toast.text) - } -} - // TestStartProvisionRefusesZeroRecipes: core.Apply on zero recipes returns // nil (a legitimate no-op), which used to surface here as a false // "provisioned" success. startProvision must still short-circuit before