From 507e3596c7cbe1709760c6e72f67fd33b8b96400 Mon Sep 17 00:00:00 2001 From: Nicolas CHAUVIN Date: Sun, 13 Sep 2026 18:17:47 +0200 Subject: [PATCH] docs: correct six claims the code had outgrown --- CHANGELOG.md | 2 ++ README.md | 10 ++++++---- docs/design/inventory.md | 11 +++++++++-- docs/dogfood.md | 5 +++++ docs/language.md | 14 +++++++++++--- internal/inventory/inventory.go | 11 ++++++++--- internal/std/postgres/postgres.shellf | 19 +++++++++++++------ 7 files changed, 54 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 649ec57..4eb2ac1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,8 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Fixed +- Six documentation claims the code had outgrown: a spec header short by four chapters, a `when` keyword that never existed, an inventory table missing `local` and `interpreter`, a package comment contradicting its own fields, a def file arguing against a def it contains — and a pinned ssh key now documented as having to be unencrypted (#661, #662). + - `sysctl.set` observes the drop-in it writes as well as the running kernel. A host whose kernel already held the value and whose `/etc/sysctl.d` file was gone reported `already`, and the setting was lost at the next reboot. The def's own comment argued against reading the file, correctly, then concluded "the kernel only" — a false dilemma (#658). - `user.group` matches a group name literally. `grep -qx` read it as a regular expression, and Debian allows `.` in a group name — so a request for `a.b` was satisfied by membership of `axb`, and the def reported `already` over a user it never added. The same class as #598, which fixed it in `htpasswd.entry` (#660). diff --git a/README.md b/README.md index 16a3418..2b55755 100644 --- a/README.md +++ b/README.md @@ -76,7 +76,8 @@ group web = [web1, web2] Authentication uses your **ssh-agent** (`SSH_AUTH_SOCK`) by default, so an encrypted key never leaves the agent. To pin a specific key instead, add `key: "~/.ssh/id_…"` -to `defaults` or a host (it is an optional override). +to `defaults` or a host — it must be **unencrypted**, since shellf parses the file itself and +has no passphrase prompt (ADR-0026). An encrypted key is what the agent path is for. Describe what to do in a **plan** file (`plan.shellf`): @@ -111,9 +112,10 @@ instruction that finds the state it wants reports `ok.already` and does nothing. | Host | `host = { address: "…", user: "…", port: "…" }` | | Group | `group = [, ]` | -Omitted host fields fall back to `defaults`, then to `22` for the port. Only -`address` is required. A host may belong to several groups. `key: "…"` is an -optional field (a pinned ssh key); without it, authentication uses the ssh-agent. +Omitted host fields fall back to `defaults`, then to `22` for the port. `address` is +required unless the host is `local` (below). A host may belong to several groups. `key: "…"` +is an optional field — a pinned ssh key, which must be **unencrypted**; without it, +authentication uses the ssh-agent. Any other field is a per-host variable. A host with `local: "true"` is provisioned on the **control host itself**, with no SSH — `host self = { local: "true" }` (no `address` needed). Same agent, plan, and diff --git a/docs/design/inventory.md b/docs/design/inventory.md index 201fd0a..38fee96 100644 --- a/docs/design/inventory.md +++ b/docs/design/inventory.md @@ -13,10 +13,17 @@ from the network address. | Field | Required | Meaning | |---|---|---| -| `address` | yes | network endpoint (IP or DNS) | +| `address` | unless `local` | network endpoint (IP or DNS) | | `user` | no | ssh user; falls back to `defaults.user` | | `port` | no | ssh port; falls back to `defaults.port`, then `22` | -| `key` | no | ssh identity file; falls back to `defaults.key` | +| `key` | no | ssh identity file, **unencrypted**; falls back to `defaults.key` | +| `local` | no | `"true"` reaches the control host itself, with no SSH (ADR-0027) — no `address` | +| `interpreter` | no | shell for unannotated `shell` blocks: `sh`/`bash`/`dash`/`nu`/`raw` (ADR-0012) | +| any other | no | a free-form per-host variable, read as `${inventory.}` (ADR-0052) | + +`key` must not be passphrase-protected: shellf parses it with `ssh.ParsePrivateKey`, which +fails on an encrypted file. An encrypted key is what the ssh-agent path is for, and that is +the default — ADR-0026 §3 records the passphrase form as deliberately not built. ``` host web1 = { address: "10.0.0.1" } diff --git a/docs/dogfood.md b/docs/dogfood.md index a63e879..2f323dd 100644 --- a/docs/dogfood.md +++ b/docs/dogfood.md @@ -117,6 +117,11 @@ Recorded as #547. Not decided here: ADR-0052 listed real questions this raises ( order between hosts, hosts outside the `on` block), and a measured cost does not by itself answer them. +> **Since answered.** [ADR-0054](adr/0054-cross-host-inventory-reads.md) shipped +> `${inventory..}`. The finding above stays as written — this file is a dated +> report of what a deployment could not express, and history is allowed to describe a world the +> code has left (ADR-0001). + ### What deliberately stays raw - **`su - postgres -c …`** as the way to reach the database as its owner. Any `postgres.*` diff --git a/docs/language.md b/docs/language.md index fa73ed7..3306c60 100644 --- a/docs/language.md +++ b/docs/language.md @@ -1,8 +1,16 @@ # shellf — language spec > Living doc, and incomplete on purpose: what is written here is current, what is missing -> is missing. One shipped construct has no chapter yet — `as ` escalation -> ([ADR-0011](adr/0011-privilege-escalation.md)). +> is missing. Shipped constructs with no chapter yet, named so the gap is not mistaken for +> completeness (#662): +> +> - `as ` escalation ([ADR-0011](adr/0011-privilege-escalation.md)); +> - `on { … }` and `parallel { … }` — the plan's own structure, described in +> [design/orchestration.md](design/orchestration.md); +> - `override def` ([ADR-0014](adr/0014-user-defs-directory-package.md)); +> - the inventory's syntax, described in [design/inventory.md](design/inventory.md). +> +> The header used to name `as ` alone, which understated it by four. ## `Result` — an instruction's outcome @@ -25,7 +33,7 @@ Result = ok.(payload?) | err.(payload?) | would.(payload?) | Shape | `{ exit, stdout, stderr, ok }` | `ok`/`err`/`would` + tag + optional payload | | Produced by | a `shell { }` block | a `def` instruction | -An instruction **reads** the `ShellResult` and **translates** it into a `Result` (via `when`/tags). A `Result` may *carry* a `ShellResult` in its payload; it is not one. Flattening `Result` to exit/stdout/stderr = branching on exit codes = plain bash — the exact regression shellf exists to avoid. +An instruction **reads** the `ShellResult` and **translates** it into a `Result` (it tests the shell and returns a tagged outcome). A `Result` may *carry* a `ShellResult` in its payload; it is not one. Flattening `Result` to exit/stdout/stderr = branching on exit codes = plain bash — the exact regression shellf exists to avoid. ## Phases and modes diff --git a/internal/inventory/inventory.go b/internal/inventory/inventory.go index 94b0dfa..a73a3cf 100644 --- a/internal/inventory/inventory.go +++ b/internal/inventory/inventory.go @@ -1,6 +1,11 @@ -// Package inventory holds the hosts and groups (the orchestration plane's -// targets). Connection coordinates only — no business variables. Built in Go -// for now; a parsed shellf-language inventory comes later. +// Package inventory holds the hosts and groups (the orchestration plane's targets): the +// connection coordinates, and the free-form per-host variables a plan reads as +// `${inventory.}` (ADR-0052). +// +// The types are Go; the file an operator writes is **shellf**, parsed by +// `internal/lang` (`ParseInventory`). This comment said the opposite of both halves — "no +// business variables", beside the `Vars` field, and "a parsed shellf-language inventory comes +// later", which had already shipped (#662). package inventory import ( diff --git a/internal/std/postgres/postgres.shellf b/internal/std/postgres/postgres.shellf index ef6d9ea..acddaa0 100644 --- a/internal/std/postgres/postgres.shellf +++ b/internal/std/postgres/postgres.shellf @@ -1,10 +1,17 @@ -# PostgreSQL roles and databases. +# PostgreSQL roles, databases, and the two config files. # -# Only these two. The two-host dogfood (#542) needed exactly this and nothing else — -# `pg_hba` and `postgresql.conf` were handled by `file.line` and `file.replace`, which -# converged. A `postgres.config` would also inherit the versioned-path problem (#546), -# unresolved; these two go through `psql` and never name a config path, so they dodge it. -# More postgres defs arrive when a deployment shows it needs them, not before. +# Four defs: `role` and `database` go through `psql`; `hba` and `config` write the cluster's +# files. The first two arrived from the two-host dogfood (#542), which needed nothing else — +# `pg_hba` and `postgresql.conf` were handled by `file.line` and `file.replace` and converged. +# +# This header used to read "only these two", and argued that a `postgres.config` *would* +# inherit the versioned-path problem of #546 — unresolved — a hundred lines above the +# `postgres.config` that resolves it by asking `pg_lsclusters` and `pg_conftool` where the +# cluster keeps its files. The prediction was right at the time and the code overtook it +# (#546, #567, #662). +# +# The rule it stated still holds: a postgres def arrives when a deployment shows it is needed, +# not before. # `postgres.role` verifies the password by connecting with it, the way `htpasswd.entry` # verifies against its stored hash rather than trusting that a line exists. PostgreSQL keeps