From 8e2a1e762de1e6b22e560bc30047ec2de6c6d42b Mon Sep 17 00:00:00 2001 From: jochen Date: Sat, 26 Sep 2026 22:39:54 +0200 Subject: [PATCH 01/11] A found tunnel carries its MTU to the mesh MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The host parses MTU from the found [Interface] and reports it, so the mesh's interface can come up with the same MTU when it takes the tunnel over. A path tuned to 1380 regresses to the 1420 default otherwise — invisible to ping, fatal to TLS handshakes and transfers over that path (novox/hq: the mesh had no MTU concept). Zero when the config named none, and the mesh writes no MTU line then. --- cmd/mesh-host/main.go | 2 +- internal/link/enrol.go | 1 + internal/tunnel/tunnel.go | 11 +++++++++++ internal/tunnel/tunnel_test.go | 24 ++++++++++++++++++++++++ 4 files changed, 37 insertions(+), 1 deletion(-) diff --git a/cmd/mesh-host/main.go b/cmd/mesh-host/main.go index 3bacc00..4e8aefe 100644 --- a/cmd/mesh-host/main.go +++ b/cmd/mesh-host/main.go @@ -910,7 +910,7 @@ func rekeyOnto(mine identity.Identity, found tunnel.Found) (identity.Identity, l // carried is a found tunnel as it is presented to the mesh: everything but its private key. func carried(t tunnel.Found) *link.Tunnel { out := &link.Tunnel{Interface: t.Interface, Unit: t.Unit, Config: t.Config, Port: t.Port, - Address: t.Address, Range: t.Range, PublicKey: t.PublicKey} + Address: t.Address, Range: t.Range, MTU: t.MTU, PublicKey: t.PublicKey} for _, p := range t.Peers { out.Peers = append(out.Peers, link.TunnelPeer{PublicKey: p.PublicKey, Address: p.Address}) } diff --git a/internal/link/enrol.go b/internal/link/enrol.go index 8f755c8..8f0aa6f 100644 --- a/internal/link/enrol.go +++ b/internal/link/enrol.go @@ -68,6 +68,7 @@ type Tunnel struct { Port int `json:"port"` Address string `json:"address"` Range string `json:"range"` + MTU int `json:"mtu,omitempty"` PublicKey string `json:"public_key"` Peers []TunnelPeer `json:"peers,omitempty"` } diff --git a/internal/tunnel/tunnel.go b/internal/tunnel/tunnel.go index 53c6bce..9e1793d 100644 --- a/internal/tunnel/tunnel.go +++ b/internal/tunnel/tunnel.go @@ -44,6 +44,11 @@ type Found struct { Port int `json:"port"` Address string `json:"address"` Range string `json:"range"` + // MTU is the interface's, when the found config set one. Kept because a tuned tunnel (a path + // that needs 1380, say) breaks silently if the mesh's interface comes up at the 1420 default: + // no ping fails, but TLS handshakes stall and transfers hang (novox/hq: a taken tunnel carries + // its MTU). Zero when the config named none, and the mesh sets no MTU line then. + MTU int `json:"mtu,omitempty"` // PublicKey is what every peer knows this tunnel by — derived here from the private key, so // it is the key the file actually holds and not a comment beside it. PublicKey string `json:"public_key"` @@ -209,6 +214,12 @@ func Parse(raw []byte) (Found, error) { return Found{}, fmt.Errorf("ListenPort %q is not a port", value) } f.Port = port + case "mtu": + mtu, err := strconv.Atoi(value) + if err != nil || mtu < 576 || mtu > 65535 { + return Found{}, fmt.Errorf("MTU %q is not a plausible MTU", value) + } + f.MTU = mtu case "address": // The first address is the interface's; a second family would be a second // tunnel's worth of addressing, which this does not carry. diff --git a/internal/tunnel/tunnel_test.go b/internal/tunnel/tunnel_test.go index 7272a6b..19d73b3 100644 --- a/internal/tunnel/tunnel_test.go +++ b/internal/tunnel/tunnel_test.go @@ -170,3 +170,27 @@ func TestNoneUpIsAnOrdinaryAnswerAndSeveralIsAQuestion(t *testing.T) { t.Errorf("naming a tunnel that is not up was not refused: %v", err) } } + +func TestAFoundTunnelReadsItsMTU(t *testing.T) { + // A tuned path sets MTU in [Interface]; the mesh must carry it or the tunnel regresses to the + // default silently (novox/hq: a taken tunnel carries its MTU). + private, public := aKey(t) + withMTU := "# tuned\n[Interface]\nPrivateKey = " + private + + "\nListenPort = 51820\nAddress = 10.10.0.3/24\nMTU = 1380\n" + + "[Peer]\nPublicKey = " + public + "\nAllowedIPs = 10.10.0.1/32\n" + f, err := Parse([]byte(withMTU)) + if err != nil { + t.Fatal(err) + } + if f.MTU != 1380 { + t.Fatalf("MTU 1380 was not read; got %d", f.MTU) + } + // And a config with none leaves MTU zero, so the mesh writes no MTU line. + f2, err := Parse([]byte(aConfig(private, public))) + if err != nil { + t.Fatal(err) + } + if f2.MTU != 0 { + t.Fatalf("a config with no MTU must leave it zero; got %d", f2.MTU) + } +} From 2722e7b36e2c15f0ee9caa961c4f18f715f55f61 Mon Sep 17 00:00:00 2001 From: jochen Date: Sat, 26 Sep 2026 23:39:32 +0200 Subject: [PATCH 02/11] A host accepts an explicitly-empty declaration (hq 127) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The empty-resources guard refused every empty body as a likely mistake, with no way to say emptiness was meant — so the control plane could never tell a node to drop its last resource. The envelope gains owns_nothing: with it, an empty declaration is applied (the node drops what the mesh owned); without it, empty is still refused, so a truncated or mis-composed body cannot silently strip a machine. One test, both directions. --- internal/declaration/declaration.go | 17 ++++++++++++----- internal/declaration/declaration_test.go | 13 +++++++++++++ 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/internal/declaration/declaration.go b/internal/declaration/declaration.go index 21b3b0c..5fa7f27 100644 --- a/internal/declaration/declaration.go +++ b/internal/declaration/declaration.go @@ -1142,10 +1142,17 @@ func ParseTrusted(raw []byte) (*Declaration, error) { return parse(raw, true) } // first pass takes the envelope and each resource's bytes; the second decodes each one into // the struct for its kind, strictly. type envelope struct { - Version int `json:"declaration"` - For string `json:"for,omitempty"` - Adoption *Adoption `json:"adoption,omitempty"` - Resources []json.RawMessage `json:"resources"` + Version int `json:"declaration"` + For string `json:"for,omitempty"` + Adoption *Adoption `json:"adoption,omitempty"` + // OwnsNothing is the control plane saying, explicitly, that this node's declaration is empty + // on purpose — it owns nothing the mesh put there (novox/hq issue 127). Without it an empty + // resources list is refused as a likely mistake; with it the node applies the empty + // declaration and drops what it last held. The two are distinguished because a truncated or + // mis-composed body arrives as empty too, and a host that could not tell them apart would let + // a bug quietly strip a machine. + OwnsNothing bool `json:"owns_nothing,omitempty"` + Resources []json.RawMessage `json:"resources"` } func parse(raw []byte, allowActions bool) (*Declaration, error) { @@ -1165,7 +1172,7 @@ func parse(raw []byte, allowActions bool) (*Declaration, error) { d := &Declaration{Version: env.Version, For: env.For, Adoption: env.Adoption} var problems []string - if len(env.Resources) == 0 { + if len(env.Resources) == 0 && !env.OwnsNothing { problems = append(problems, "no resources. An empty declaration is a mistake, not a "+ "machine with nothing on it — say so with an explicit empty list if that is meant") } diff --git a/internal/declaration/declaration_test.go b/internal/declaration/declaration_test.go index bb4e7fa..95c8a16 100644 --- a/internal/declaration/declaration_test.go +++ b/internal/declaration/declaration_test.go @@ -451,3 +451,16 @@ func TestAContainersResolverAndAddressAreAddressesOrRefused(t *testing.T) { t.Errorf("a well-formed resolver and address were refused: %v", p) } } + +func TestAnExplicitlyEmptyDeclarationIsAccepted(t *testing.T) { + // A deliberately-empty declaration (novox/hq issue 127) says owns_nothing, and is applied so + // the node drops what it last held — distinct from an accidental empty body, which is refused. + if _, err := Parse([]byte(`{"declaration":1,"owns_nothing":true,"resources":[]}`)); err != nil { + t.Fatalf("an explicitly-empty declaration must be accepted: %v", err) + } + // Without the marker, an empty declaration is still refused as a likely mistake. + _, err := Parse([]byte(`{"declaration":1,"resources":[]}`)) + if err == nil || !strings.Contains(err.Error(), "no resources") { + t.Fatalf("an unmarked empty declaration must still be refused; got %v", err) + } +} From 3ae999497fa87be9dbeb48a183cd1474abe7c5e6 Mon Sep 17 00:00:00 2001 From: jochen Date: Sat, 26 Sep 2026 23:51:36 +0200 Subject: [PATCH 03/11] Format control_test.go so the gate's fmt step passes on main --- internal/bootstrap/control_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/bootstrap/control_test.go b/internal/bootstrap/control_test.go index ff2fed0..7df8e9c 100644 --- a/internal/bootstrap/control_test.go +++ b/internal/bootstrap/control_test.go @@ -254,7 +254,7 @@ func TestAManifestWantingNoStoresIsRefusedWithTheShapeItShouldHave(t *testing.T) // a reply proves the sealed connections it was given are the ones the foundation made. func TestThePermanentControlPlaneIsAskedTheSameQuestion(t *testing.T) { runtime := &asked{answer: aMeshThatAgrees(map[string]string{ - "module list": "", + "module list": "", "exec mesh-controller /mesh-controller": "1 node, 0 waiting\n", })} control := controlPlane{container: "temp-mesh-controller", run: runtime.run, timeout: time.Second} From 1cb895346dc4c150ce10e5bcd7e87e875818dec8 Mon Sep 17 00:00:00 2001 From: jochen Date: Sat, 26 Sep 2026 23:51:36 +0200 Subject: [PATCH 04/11] Write into a marked block of a text file instead of over it, so a shared hosts file keeps every line that is not the mesh's (hq issue 128) --- internal/apply/apply.go | 6 + internal/apply/block.go | 313 ++++++++++++++++++ internal/apply/block_test.go | 471 ++++++++++++++++++++++++++++ internal/declaration/block_test.go | 51 +++ internal/declaration/declaration.go | 83 ++++- internal/store/store.go | 15 +- 6 files changed, 932 insertions(+), 7 deletions(-) create mode 100644 internal/apply/block.go create mode 100644 internal/apply/block_test.go create mode 100644 internal/declaration/block_test.go diff --git a/internal/apply/apply.go b/internal/apply/apply.go index 132fa8c..8ec6c02 100644 --- a/internal/apply/apply.go +++ b/internal/apply/apply.go @@ -669,6 +669,9 @@ func applyAccess(r *declaration.Access) (Outcome, error) { // keepFound, when not nil, is where the original of a file this host has no record of is kept // before it is written over (novox/hq ADR 0100): once, never overwritten, and named in the outcome. func applyFile(r *declaration.File, previous store.Applied, unseal Unseal, keepFound Keep) (Outcome, error) { + if r.Into == declaration.IntoBlock { + return applyBlock(r, previous) + } if r.Into != "" { return applyInto(r, previous) } @@ -1049,6 +1052,9 @@ func remove(ctx context.Context, sys system.System, a store.Applied, run Runner) return "removed", "no longer declared, and empty", nil case declaration.TypeFile: + if a.Into != nil && a.Into.Format == declaration.IntoBlock { + return removeBlock(a) + } if a.Into != nil { return removeInto(a) } diff --git a/internal/apply/block.go b/internal/apply/block.go new file mode 100644 index 0000000..7115cda --- /dev/null +++ b/internal/apply/block.go @@ -0,0 +1,313 @@ +package apply + +import ( + "errors" + "fmt" + "os" + "path/filepath" + "strings" + + "github.com/novox/mesh-host/internal/declaration" + "github.com/novox/mesh-host/internal/store" +) + +// A file written into a marked block, never over (novox/hq issue 128, ADR 0102). +// +// **The file is the machine's; the mesh owns lines in it.** The machine's hosts file is the case +// that needed it. The mesh wrote it whole — its own header, localhost, the machine's name and every +// name in the mesh — and on a workstation that file is shared: the distribution's lines, a local +// development tool's own marked blocks rewritten whenever its projects change, the operator's +// hand-added names. Written whole, all of those went at the next change to the mesh's names, with +// no failure anywhere: the tool believed it had written its block, and the mesh believed it owned +// the file. It is ADR 0102's failure exactly, in a file ADR 0102's JSON verb cannot speak. +// +// So the host finds the lines between `# BEGIN mesh ` and `# END mesh `, rewrites those and +// nothing else, and records what they held before. Every line outside the markers is kept byte for +// byte — including another tool's `# BEGIN …` blocks, which are that tool's. Undeclared, the region +// is given back what it held, or taken out with its markers when it held nothing, and a file the +// mesh created goes only if nothing but whitespace is left. + +// applyBlock writes a file's declared lines into its region of the file already at its path. +func applyBlock(r *declaration.File, previous store.Applied) (Outcome, error) { + out := begin(r) + opening, closing := declaration.BlockMarkers(r.ID) + want := blockBody(r.Content) + + raw, err := os.ReadFile(r.Path) + existed := err == nil + if err != nil && !errors.Is(err, os.ErrNotExist) { + return out, err + } + existing := string(raw) + lines := linesOf(existing) + at, found, err := regionIn(lines, opening, closing) + if err != nil { + // Refused, never guessed at: markers the host cannot pair are markers it cannot write + // between without risking lines that are not the mesh's. + return out, fmt.Errorf("%s: %w; it was left as it is", r.Path, err) + } + + // A record of a block is carried; anything else — no record, a file once written whole, one + // once written into as JSON — is a file the host is seeing for the first time as a block. + rec := store.Into{Format: declaration.IntoBlock} + recorded := previous.Into != nil && previous.Into.Format == declaration.IntoBlock + if recorded && existed { + rec.Created = previous.Into.Created + rec.Region = previous.Into.Region + rec.Separated = previous.Into.Separated + rec.At = previous.Into.At + } else { + // A file gone since the last apply is made again, and made by the mesh: what it held + // before went with it, so there is nothing to give back but the file's absence. + rec.Created = !existed + if found { + // **What the host may have written itself is not the machine's** — the same reasoning + // as a key in a JSON file (novox/hq ADR 0102). With no record, a region already holding + // exactly the declared lines cannot be told from one this host wrote a moment ago and + // died before saving; remembered as the machine's, it would be put back on undeclare + // for ever. So it is the mesh's, and undeclaring takes it out. + if held := at.body(lines); held != want { + rec.Region = &held + } + } + } + + // Drift: the machine no longer holds, between the mesh's markers, what this host last put + // there. Judged only against a record of a block: a digest of a whole file says nothing about + // a region of it. + drifted := recorded && previous.Wrote != "" && existed && + (!found || digestOf(at.body(lines)) != previous.Wrote) + + var next string + switch { + case !existed: + next = regionOf(opening, closing, want) + case found: + // Where it is, whatever At says: the region is never moved, because moving it moves the + // machine's lines around it. + next = strings.Join(lines[:at.begin+1], "") + want + strings.Join(lines[at.end:], "") + case r.At == declaration.AtStart: + // Above everything, and one blank line between the region and the machine's first line + // unless there is one already — a line in some files means what the lines above it say. + rec.At, rec.Separated = declaration.AtStart, false + next = regionOf(opening, closing, want) + if existing != "" && !strings.HasPrefix(existing, "\n") { + next += "\n" + rec.Separated = true + } + next += existing + default: + // At the end, apart from whatever is there: the file's last line is ended if it was not, + // and one blank line separates the region from the machine's lines unless there is one. + rec.At, rec.Separated = "", false + next = existing + if next != "" && !strings.HasSuffix(next, "\n") { + next += "\n" + } + if next != "" && next != "\n" && !strings.HasSuffix(next, "\n\n") { + next += "\n" + rec.Separated = true + } + next += regionOf(opening, closing, want) + } + + same := existed && next == existing + if !same { + var info os.FileInfo + mode := os.FileMode(0o644) + if info, err = os.Stat(r.Path); err == nil { + mode = info.Mode().Perm() // the machine's file keeps the machine's mode + } else if mode, err = modeOf(r.Mode, mode); err != nil { + return out, err + } + if err := os.MkdirAll(filepath.Dir(r.Path), 0o755); err != nil { + return out, err + } + if err := writeAtomically(r.Path, []byte(next), mode); err != nil { + return out, err + } + if existed { + // The write is a new file renamed over the old, so it belongs to whoever wrote it. The + // machine's file keeps the machine's owner, as it keeps its mode. + if err := keepOwner(r.Path, info); err != nil { + return out, err + } + } else if err := own(r.Path, r.Owner); err != nil { + return out, err + } + } + + // Read back: the region holds what was declared, and nothing outside it moved. + written, err := os.ReadFile(r.Path) + if err != nil { + return out, fmt.Errorf("wrote into %s and cannot read it back: %w", r.Path, err) + } + if string(written) != next { + return out, fmt.Errorf("%s does not hold the mesh's region as written after writing into it", r.Path) + } + + out.into = &rec + out.wrote = digestOf(want) + switch { + case !existed: + out.Action = "created" + out.Detail = "written into; the file was not there" + case same: + out.Action = "unchanged" + case drifted: + out.Action = "corrected" + out.Detail = "the mesh's region had been changed on the machine; every line outside it was kept" + case !found: + out.Action = "updated" + where := "end" + if rec.At == declaration.AtStart { + where = "start" + } + out.Detail = "the mesh's region added at the " + where + "; every other line kept as it was" + default: + out.Action = "updated" + out.Detail = "the mesh's region rewritten; every line outside it kept as it was" + } + return out, nil +} + +// removeBlock gives back what a file written into a block held before the mesh's region. +func removeBlock(a store.Applied) (string, string, error) { + raw, err := os.ReadFile(a.Target) + if errors.Is(err, os.ErrNotExist) { + return "forgotten", "no longer there", nil + } + if err != nil { + return "", "", err + } + info, err := os.Stat(a.Target) + if err != nil { + return "", "", err + } + opening, closing := declaration.BlockMarkers(a.ID) + lines := linesOf(string(raw)) + at, found, err := regionIn(lines, opening, closing) + if err != nil { + return "kept", err.Error() + ", so nothing was taken out of it; remove the mesh's region by hand", nil + } + + next, action, detail := string(raw), "forgotten", "the mesh's region was no longer in it" + switch { + case found && a.Into.Region != nil: + next = strings.Join(lines[:at.begin+1], "") + *a.Into.Region + strings.Join(lines[at.end:], "") + action, detail = "restored", "no longer declared; the region was given back what it held" + case found: + from, to := at.begin, at.end+1 + // The blank line the host put beside the region, and only that one: if what stands there + // now is not blank, it is somebody's, and it stays. + if a.Into.Separated { + if a.Into.At == declaration.AtStart { + if to < len(lines) && lines[to] == "\n" { + to++ + } + } else if from > 0 && lines[from-1] == "\n" { + from-- + } + } + next = strings.Join(lines[:from], "") + strings.Join(lines[to:], "") + action, detail = "restored", "no longer declared; the mesh's region was taken out and every other line kept" + } + + if a.Into.Created && strings.TrimSpace(next) == "" { + if err := os.Remove(a.Target); err != nil { + return "", "", err + } + return "removed", "no longer declared; the mesh had created it and nothing else was in it", nil + } + if next == string(raw) { + return action, detail, nil + } + if err := writeAtomically(a.Target, []byte(next), info.Mode().Perm()); err != nil { + return "", "", err + } + if err := keepOwner(a.Target, info); err != nil { + return "", "", err + } + return action, detail, nil +} + +// blockBody is the declared lines as they stand in the region: ending in exactly one line end, or +// nothing at all when there are no lines. +func blockBody(content string) string { + trimmed := strings.TrimRight(content, "\n") + if trimmed == "" { + return "" + } + return trimmed + "\n" +} + +func regionOf(begin, end, body string) string { + return begin + "\n" + body + end + "\n" +} + +// linesOf splits text into lines that keep their line ends, so joining them again gives back +// exactly the bytes that were read — a last line without one included. +func linesOf(text string) []string { + return strings.SplitAfter(text, "\n") +} + +// region is where the mesh's markers stand, as indices into the lines of a file. +type region struct{ begin, end int } + +// body is what stands between the markers. +func (r region) body(lines []string) string { + return strings.Join(lines[r.begin+1:r.end], "") +} + +// regionIn finds the mesh's markers for one resource. A line is a marker only if it is exactly the +// marker, so another tool's block and another resource's region are never it. Markers that do not +// form one pair — a begin with no end, an end before its begin, either twice — are an error rather +// than a best guess, because a guess is how the host would rewrite lines that are not its own. +func regionIn(lines []string, begin, end string) (region, bool, error) { + at := region{begin: -1, end: -1} + for i, line := range lines { + switch strings.TrimSuffix(line, "\n") { + case begin: + if at.begin >= 0 { + return at, false, fmt.Errorf("%q is in it more than once", begin) + } + at.begin = i + case end: + if at.end >= 0 { + return at, false, fmt.Errorf("%q is in it more than once", end) + } + at.end = i + } + } + switch { + case at.begin < 0 && at.end < 0: + return at, false, nil + case at.begin < 0: + return at, false, fmt.Errorf("%q is in it with no %q before it", end, begin) + case at.end < 0: + return at, false, fmt.Errorf("%q is in it with no %q after it", begin, end) + case at.end < at.begin: + return at, false, fmt.Errorf("%q stands before %q", end, begin) + } + return at, true, nil +} + +// keepOwner gives a file rewritten through a new one back to whoever owned what it replaced. +// Changed only where it differs, so a host that is not root can still write a file it owns. +func keepOwner(path string, was os.FileInfo) error { + uid, gid, ok := ownerOf(was) + if !ok { + return nil + } + now, err := os.Stat(path) + if err != nil { + return err + } + if u, g, ok := ownerOf(now); ok && u == uid && g == gid { + return nil + } + if err := os.Chown(path, uid, gid); err != nil { + return fmt.Errorf("cannot give %s back to its owner %d:%d: %w", path, uid, gid, err) + } + return nil +} diff --git a/internal/apply/block_test.go b/internal/apply/block_test.go new file mode 100644 index 0000000..2dfefab --- /dev/null +++ b/internal/apply/block_test.go @@ -0,0 +1,471 @@ +package apply + +import ( + "context" + "encoding/json" + "fmt" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/novox/mesh-host/internal/store" +) + +// Defends novox/hq issue 128 and ADR 0102: a text file the mesh shares with software it did not +// install is written into a marked block, never over — every line outside the mesh's markers is +// the machine's and is kept byte for byte, and undeclaring gives the file back. + +const namesID = "mesh-wireguard.fact-node-names" + +func blockDecl(t *testing.T, path, content string, extra ...string) string { + t.Helper() + more := "" + for _, e := range extra { + more += "," + e + } + return fmt.Sprintf(`{"declaration":1,"resources":[ + {"id":%q,"type":"file","path":%q,"into":"block","content":%q%s} + ]}`, namesID, path, content, more) +} + +func applyBlockDecl(t *testing.T, raw string, known store.State) (Report, store.State) { + t.Helper() + report, state, err := Apply(context.Background(), archHost(t), parse(t, raw), known, store.OriginDeclared, nil, nil, nil) + if err != nil { + t.Fatal(err) + } + return report, state +} + +func undeclare(t *testing.T, known store.State) (Report, store.State) { + t.Helper() + report, state, err := Apply(context.Background(), archHost(t), somethingElse(t), known, store.OriginDeclared, nil, nil, nil) + if err != nil { + t.Fatal(err) + } + return report, state +} + +func readText(t *testing.T, path string) string { + t.Helper() + raw, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + return string(raw) +} + +func marked(id, body string) string { + return "# BEGIN mesh " + id + "\n" + body + "# END mesh " + id + "\n" +} + +// A workstation's hosts file, the way issue 128 found it: the distribution's lines, a development +// tool's own marked blocks, and the operator's hand-added names. +const workstationHosts = "127.0.0.1\tlocalhost\n" + + "127.0.1.1\tg14.localdomain g14\n" + + "\n" + + "# BEGIN devtool project-a\n" + + "127.0.0.1 a.test api.a.test\n" + + "# END devtool project-a\n" + + "# BEGIN devtool project-b\n" + + "127.0.0.1 b.test\n" + + "# END devtool project-b\n" + + "192.168.1.20 printer # the operator's\n" + +const meshNames = "10.42.0.1 ace\n10.42.0.2 novox\n" + +func TestABlockKeepsEveryLineOutsideItsMarkers(t *testing.T) { + path := filepath.Join(t.TempDir(), "hosts") + if err := os.WriteFile(path, []byte(workstationHosts), 0o640); err != nil { + t.Fatal(err) + } + report, state := applyBlockDecl(t, blockDecl(t, path, meshNames), store.State{}) + want := workstationHosts + "\n" + marked(namesID, meshNames) + if got := readText(t, path); got != want { + t.Fatalf("the file after writing into it:\n%q\nwant\n%q", got, want) + } + if got := report.Outcomes[0].Action; got != "updated" { + t.Errorf("adding the region was %q", got) + } + if info, _ := os.Stat(path); info.Mode().Perm() != 0o640 { + t.Errorf("the machine's file mode was changed to %o", info.Mode().Perm()) + } + rec, _ := state.Find(namesID) + if rec.Into == nil || rec.Into.Format != "block" || rec.Into.Region != nil || rec.Into.Created { + t.Fatalf("recorded as %+v", rec.Into) + } + + // Again, with nothing changed: nothing written. + report, state = applyBlockDecl(t, blockDecl(t, path, meshNames), state) + if got := report.Outcomes[0].Action; got != "unchanged" { + t.Errorf("a second apply was %q", got) + } + + // The development tool rewrites its block, and the operator adds a line after the mesh's + // region; the mesh's names change. Only the region moves. + edited := strings.Replace(readText(t, path), "127.0.0.1 b.test\n", "127.0.0.1 b.test c.test\n", 1) + + "10.0.0.5 nas # added after\n" + _ = os.WriteFile(path, []byte(edited), 0o640) + changed := meshNames + "10.42.0.3 shanks\n" + report, state = applyBlockDecl(t, blockDecl(t, path, changed), state) + want = strings.Replace(edited, marked(namesID, meshNames), marked(namesID, changed), 1) + if got := readText(t, path); got != want { + t.Fatalf("rewriting the region moved something else:\n%q\nwant\n%q", got, want) + } + if got := report.Outcomes[0].Action; got != "updated" { + t.Errorf("rewriting the region was %q", got) + } + + // Undeclared: the region, its markers and the blank line the host put before it go; every + // other line is where it was. + report, _ = undeclare(t, state) + want = strings.Replace(edited, "\n"+marked(namesID, meshNames), "", 1) + if got := readText(t, path); got != want { + t.Fatalf("undeclaring left:\n%q\nwant\n%q", got, want) + } + if got := report.Outcomes[0].Action; got != "restored" { + t.Errorf("undeclaring was %q", got) + } +} + +func TestUndeclaringABlockAddedAtTheEndGivesTheFileBackExactly(t *testing.T) { + for name, original := range map[string]string{ + "ending in a line": "127.0.0.1 localhost\n", + "ending in a blank line": "127.0.0.1 localhost\n\n", + "empty": "", + } { + t.Run(name, func(t *testing.T) { + path := filepath.Join(t.TempDir(), "hosts") + _ = os.WriteFile(path, []byte(original), 0o644) + _, state := applyBlockDecl(t, blockDecl(t, path, meshNames), store.State{}) + undeclare(t, state) + if got := readText(t, path); got != original { + t.Errorf("undeclaring left %q, the machine had %q", got, original) + } + }) + } +} + +func TestABlockAddedAtTheEndIsSetApartFromTheMachinesLines(t *testing.T) { + for name, c := range map[string]struct{ before, after string }{ + "no line end": {"127.0.0.1 localhost", "127.0.0.1 localhost\n\n" + marked(namesID, meshNames)}, + "a line end": {"127.0.0.1 localhost\n", "127.0.0.1 localhost\n\n" + marked(namesID, meshNames)}, + "a blank line already": {"127.0.0.1 localhost\n\n", "127.0.0.1 localhost\n\n" + marked(namesID, meshNames)}, + "empty": {"", marked(namesID, meshNames)}, + "only a blank line": {"\n", "\n" + marked(namesID, meshNames)}, + "content without an end": {"x\n\n", "x\n\n" + marked(namesID, meshNames)}, + } { + t.Run(name, func(t *testing.T) { + path := filepath.Join(t.TempDir(), "hosts") + _ = os.WriteFile(path, []byte(c.before), 0o644) + applyBlockDecl(t, blockDecl(t, path, meshNames), store.State{}) + if got := readText(t, path); got != c.after { + t.Errorf("got %q, want %q", got, c.after) + } + }) + } +} + +func TestTheRegionEndsInExactlyOneLineEnd(t *testing.T) { + for content, body := range map[string]string{ + "10.42.0.1 ace": "10.42.0.1 ace\n", + "10.42.0.1 ace\n": "10.42.0.1 ace\n", + "10.42.0.1 ace\n\n\n": "10.42.0.1 ace\n", + "": "", + "\n\n": "", + } { + path := filepath.Join(t.TempDir(), "hosts") + _, state := applyBlockDecl(t, blockDecl(t, path, content), store.State{}) + if got := readText(t, path); got != marked(namesID, body) { + t.Errorf("content %q was written as %q", content, got) + } + // And the same content again is not a change. + report, _ := applyBlockDecl(t, blockDecl(t, path, content), state) + if got := report.Outcomes[0].Action; got != "unchanged" { + t.Errorf("content %q applied twice was %q", content, got) + } + } +} + +func TestARegionAlreadyThereIsRewrittenInPlaceAndGivenBack(t *testing.T) { + path := filepath.Join(t.TempDir(), "hosts") + before := "127.0.0.1 localhost\n" + after := "# BEGIN devtool x\n127.0.0.1 x.test\n# END devtool x\n192.168.1.20 printer\n" + found := "10.42.0.9 old-name\n" + original := before + marked(namesID, found) + after + _ = os.WriteFile(path, []byte(original), 0o644) + + report, state := applyBlockDecl(t, blockDecl(t, path, meshNames), store.State{}) + if got := readText(t, path); got != before+marked(namesID, meshNames)+after { + t.Fatalf("the region was not rewritten in place: %q", got) + } + if got := report.Outcomes[0].Action; got != "updated" { + t.Errorf("rewriting a found region was %q", got) + } + rec, _ := state.Find(namesID) + if rec.Into.Region == nil || *rec.Into.Region != found { + t.Fatalf("what the region held before was recorded as %v", rec.Into.Region) + } + + // Undeclared: what the region held goes back, where it was. + report, _ = undeclare(t, state) + if got := readText(t, path); got != original { + t.Errorf("undeclaring left %q, the machine had %q", got, original) + } + if got := report.Outcomes[0].Action; got != "restored" { + t.Errorf("undeclaring was %q", got) + } +} + +func TestARegionWithNoRecordHoldingExactlyTheDeclaredLinesIsTheMeshs(t *testing.T) { + // A host that wrote the region and died before saving its state: what is between the markers + // is exactly what the mesh declares, and remembered as the machine's it would never go. + path := filepath.Join(t.TempDir(), "hosts") + original := "127.0.0.1 localhost\n" + _ = os.WriteFile(path, []byte(original+"\n"+marked(namesID, meshNames)), 0o644) + _, state := applyBlockDecl(t, blockDecl(t, path, meshNames), store.State{}) + if rec, _ := state.Find(namesID); rec.Into.Region != nil { + t.Fatalf("the mesh's own lines were recorded as the machine's: %q", *rec.Into.Region) + } + undeclare(t, state) + if got := readText(t, path); !strings.HasPrefix(got, original) || strings.Contains(got, "BEGIN mesh") { + t.Errorf("undeclaring left the mesh's region behind: %q", got) + } +} + +func TestADriftedRegionIsCorrected(t *testing.T) { + path := filepath.Join(t.TempDir(), "hosts") + _ = os.WriteFile(path, []byte(workstationHosts), 0o644) + _, state := applyBlockDecl(t, blockDecl(t, path, meshNames), store.State{}) + written := readText(t, path) + + _ = os.WriteFile(path, []byte(strings.Replace(written, "10.42.0.2 novox\n", "10.42.0.2 novox\n6.6.6.6 evil\n", 1)), 0o644) + report, _ := applyBlockDecl(t, blockDecl(t, path, meshNames), state) + if got := report.Outcomes[0].Action; got != "corrected" { + t.Errorf("a region edited on the machine was %q", got) + } + if got := readText(t, path); got != written { + t.Errorf("the region was not put back: %q", got) + } + + // The region taken out by hand is drift too, and it is put back. + _ = os.WriteFile(path, []byte(workstationHosts), 0o644) + report, _ = applyBlockDecl(t, blockDecl(t, path, meshNames), state) + if got := report.Outcomes[0].Action; got != "corrected" { + t.Errorf("a region removed on the machine was %q", got) + } + if got := readText(t, path); got != written { + t.Errorf("the region was not put back: %q", got) + } +} + +func TestTwoRegionsInOneFileAreEachTheirOwn(t *testing.T) { + path := filepath.Join(t.TempDir(), "hosts") + _ = os.WriteFile(path, []byte(workstationHosts), 0o644) + raw := fmt.Sprintf(`{"declaration":1,"resources":[ + {"id":"a.names","type":"file","path":%q,"into":"block","content":"10.42.0.1 ace\n"}, + {"id":"b.names","type":"file","path":%q,"into":"block","content":"10.43.0.1 lab\n"} + ]}`, path, path) + _, state := applyBlockDecl(t, raw, store.State{}) + want := workstationHosts + "\n" + marked("a.names", "10.42.0.1 ace\n") + "\n" + marked("b.names", "10.43.0.1 lab\n") + if got := readText(t, path); got != want { + t.Fatalf("two regions:\n%q\nwant\n%q", got, want) + } + report, state := applyBlockDecl(t, raw, state) + for _, o := range report.Outcomes { + if o.Action != "unchanged" { + t.Errorf("%s applied twice was %q", o.ID, o.Action) + } + } + + // One undeclared: only its region goes. + only := fmt.Sprintf(`{"declaration":1,"resources":[ + {"id":"b.names","type":"file","path":%q,"into":"block","content":"10.43.0.1 lab\n"} + ]}`, path) + applyBlockDecl(t, only, state) + want = workstationHosts + "\n" + marked("b.names", "10.43.0.1 lab\n") + if got := readText(t, path); got != want { + t.Errorf("undeclaring one region:\n%q\nwant\n%q", got, want) + } +} + +func TestABlockInAFileThatWasNotThereIsCreatedAndRemovedWithIt(t *testing.T) { + path := filepath.Join(t.TempDir(), "conf.d", "mesh.conf") + report, state := applyBlockDecl(t, blockDecl(t, path, meshNames, `"mode":"0600"`), store.State{}) + if got := readText(t, path); got != marked(namesID, meshNames) { + t.Fatalf("a created file holds %q", got) + } + if info, _ := os.Stat(path); info.Mode().Perm() != 0o600 { + t.Errorf("a created file is mode %o, declared 0600", info.Mode().Perm()) + } + if got := report.Outcomes[0].Action; got != "created" { + t.Errorf("creating was %q", got) + } + if rec, _ := state.Find(namesID); !rec.Into.Created { + t.Error("the mesh creating the file was not recorded") + } + report, _ = undeclare(t, state) + if _, err := os.Stat(path); !os.IsNotExist(err) { + t.Errorf("a file the mesh created, holding only its region, was left behind") + } + if got := report.Outcomes[0].Action; got != "removed" { + t.Errorf("undeclaring was %q", got) + } + + // Somebody else wrote into it meanwhile: it is no longer only the mesh's, and it stays. + _, state = applyBlockDecl(t, blockDecl(t, path, meshNames), store.State{}) + _ = os.WriteFile(path, []byte(readText(t, path)+"their = line\n"), 0o600) + undeclare(t, state) + if got := readText(t, path); got != "their = line\n" { + t.Errorf("undeclaring a created file somebody wrote into left %q", got) + } +} + +func TestMarkersThatDoNotPairAreRefusedAndLeftAlone(t *testing.T) { + for name, text := range map[string]string{ + "a begin with no end": "a\n# BEGIN mesh " + namesID + "\nb\n", + "an end with no begin": "a\n# END mesh " + namesID + "\n", + "an end before a begin": "# END mesh " + namesID + "\n# BEGIN mesh " + namesID + "\n", + "a begin twice": marked(namesID, "x\n") + "# BEGIN mesh " + namesID + "\n", + } { + t.Run(name, func(t *testing.T) { + path := filepath.Join(t.TempDir(), "hosts") + _ = os.WriteFile(path, []byte(text), 0o644) + if _, _, err := Apply(context.Background(), archHost(t), parse(t, blockDecl(t, path, meshNames)), + store.State{}, store.OriginDeclared, nil, nil, nil); err == nil { + t.Fatal("markers that do not pair were written between") + } + if got := readText(t, path); got != text { + t.Errorf("the file was changed: %q", got) + } + }) + } +} + +func TestAMarkerOfAnotherIDIsNotThisRegion(t *testing.T) { + // The id is part of the marker: a region whose id merely starts with this one is not it. + path := filepath.Join(t.TempDir(), "hosts") + other := marked(namesID+"-extra", "10.9.9.9 other\n") + _ = os.WriteFile(path, []byte(other), 0o644) + _, state := applyBlockDecl(t, blockDecl(t, path, meshNames), store.State{}) + if got := readText(t, path); got != other+"\n"+marked(namesID, meshNames) { + t.Fatalf("got %q", got) + } + undeclare(t, state) + if got := readText(t, path); got != other { + t.Errorf("undeclaring touched the other region: %q", got) + } +} + +func TestABlockAtTheStartStandsAboveEverything(t *testing.T) { + // dhcpcd scopes every line after `interface X` to that interface, so the mesh's global options + // go above all of it. + dhcpcd := "hostname\nduid\n\ninterface enp6s0\nstatic ip_address=192.168.1.5/24\n" + opts := "nohook resolv.conf\ndenyinterfaces mesh0\n" + for name, c := range map[string]struct{ before, after string }{ + "a file with content": {dhcpcd, marked(namesID, opts) + "\n" + dhcpcd}, + "an empty file": {"", marked(namesID, opts)}, + "a file opening blank": {"\n" + dhcpcd, marked(namesID, opts) + "\n" + dhcpcd}, + "a last line with no end": {"interface enp6s0", marked(namesID, opts) + "\ninterface enp6s0"}, + } { + t.Run(name, func(t *testing.T) { + path := filepath.Join(t.TempDir(), "dhcpcd.conf") + _ = os.WriteFile(path, []byte(c.before), 0o644) + report, state := applyBlockDecl(t, blockDecl(t, path, opts, `"at":"start"`), store.State{}) + if got := readText(t, path); got != c.after { + t.Fatalf("got %q, want %q", got, c.after) + } + if got := report.Outcomes[0].Action; got != "updated" { + t.Errorf("adding the region was %q", got) + } + report, state = applyBlockDecl(t, blockDecl(t, path, opts, `"at":"start"`), state) + if got := report.Outcomes[0].Action; got != "unchanged" { + t.Errorf("a second apply was %q", got) + } + undeclare(t, state) + if got := readText(t, path); got != c.before { + t.Errorf("undeclaring left %q, the machine had %q", got, c.before) + } + }) + } + + t.Run("a file that was not there", func(t *testing.T) { + path := filepath.Join(t.TempDir(), "dhcpcd.conf") + applyBlockDecl(t, blockDecl(t, path, opts, `"at":"start"`), store.State{}) + if got := readText(t, path); got != marked(namesID, opts) { + t.Errorf("got %q", got) + } + }) +} + +func TestARegionAlreadyThereIsNotMovedWhereverAtSaysItGoes(t *testing.T) { + for _, at := range []string{`"at":"start"`, `"at":"end"`} { + path := filepath.Join(t.TempDir(), "dhcpcd.conf") + original := "hostname\n" + marked(namesID, "old\n") + "interface enp6s0\n" + _ = os.WriteFile(path, []byte(original), 0o644) + applyBlockDecl(t, blockDecl(t, path, "nohook resolv.conf\n", at), store.State{}) + want := "hostname\n" + marked(namesID, "nohook resolv.conf\n") + "interface enp6s0\n" + if got := readText(t, path); got != want { + t.Errorf("%s: a found region was moved: %q", at, got) + } + } +} + +func TestAFileWrittenIntoABlockIsNeverHeldOnAnAdoptedNode(t *testing.T) { + path := filepath.Join(t.TempDir(), "hosts") + _ = os.WriteFile(path, []byte(workstationHosts), 0o644) + resource := fmt.Sprintf(`{"id":%q,"type":"file","path":%q,"into":"block","content":%q}`, namesID, path, meshNames) + d := adopted(t, `{"taken":[],"untaken":{"mesh-wireguard":["`+namesID+`"]}}`, resource) + report, state := applyAdopted(t, d, store.State{}, &machine{}, t.TempDir()) + if got := outcomeOf(report, namesID).Action; got == "held" { + t.Fatal("a file written into a block was held, though it replaces nothing that was found") + } + if len(state.Held) != 0 { + t.Errorf("something was held: %+v", state.Held) + } + if got := readText(t, path); got != workstationHosts+"\n"+marked(namesID, meshNames) { + t.Errorf("the adopted node's file was not written into: %q", got) + } + + // Held from when it was declared whole, the hold does not keep the region out. + path2 := filepath.Join(t.TempDir(), "hosts") + _ = os.WriteFile(path2, []byte(workstationHosts), 0o644) + known := store.State{Held: []store.Held{{ID: namesID, Module: "mesh-wireguard", Kind: "file", Target: path2}}} + resource2 := fmt.Sprintf(`{"id":%q,"type":"file","path":%q,"into":"block","content":%q}`, namesID, path2, meshNames) + d2 := adopted(t, `{"taken":[],"untaken":{"mesh-wireguard":["`+namesID+`"]}}`, resource2) + report, state = applyAdopted(t, d2, known, &machine{}, t.TempDir()) + if got := outcomeOf(report, namesID).Action; got != "updated" { + t.Errorf("the file was %q, not written into", got) + } + if len(state.Held) != 0 { + t.Errorf("the old hold outlived the block declaration: %+v", state.Held) + } + + // And the preview says the same: written into, not held. + for _, s := range Plan(d2, known, store.OriginDeclared) { + if s.ID == namesID && s.Verb == "hold" { + t.Errorf("the preview holds a file written into a block: %+v", s) + } + } +} + +func TestTheRecordOfABlockSurvivesTheStateFile(t *testing.T) { + // What undeclaring needs is in the state a host saves, not only in memory. + path := filepath.Join(t.TempDir(), "hosts") + original := "a\n" + marked(namesID, "old\n") + _ = os.WriteFile(path, []byte(original), 0o644) + _, state := applyBlockDecl(t, blockDecl(t, path, meshNames, `"at":"start"`), store.State{}) + raw, err := json.Marshal(state) + if err != nil { + t.Fatal(err) + } + var back store.State + if err := json.Unmarshal(raw, &back); err != nil { + t.Fatal(err) + } + undeclare(t, back) + if got := readText(t, path); got != original { + t.Errorf("undeclaring from a saved state left %q", got) + } +} diff --git a/internal/declaration/block_test.go b/internal/declaration/block_test.go new file mode 100644 index 0000000..a9acdec --- /dev/null +++ b/internal/declaration/block_test.go @@ -0,0 +1,51 @@ +package declaration + +import ( + "strings" + "testing" +) + +// Defends novox/hq issue 128: a file written into a block carries only its lines, in content, +// never a line the host keeps as its own marker, and says where a new region goes only as a block. + +func TestAFileWrittenIntoABlockIsRefusedUnlessItIsOnlyItsLines(t *testing.T) { + for name, c := range map[string]struct{ resource, refusal string }{ + "a begin marker in content": {`{"id":"f","type":"file","path":"/etc/hosts","into":"block","content":"a\n# BEGIN mesh f\nb\n"}`, "# BEGIN mesh f"}, + "an end marker in content": {`{"id":"f","type":"file","path":"/etc/hosts","into":"block","content":"a\n# END mesh other\n"}`, "# END mesh other"}, + "a marker with a CR": {`{"id":"f","type":"file","path":"/etc/hosts","into":"block","content":"# BEGIN mesh x\r\n"}`, "marker"}, + "sealed": {`{"id":"f","type":"file","path":"/etc/hosts","into":"block","sealed":"abc"}`, "not sealed"}, + "bytes": {`{"id":"f","type":"file","path":"/etc/hosts","into":"block","bytes":"YQ=="}`, "not sealed, bytes"}, + "secrets": {`{"id":"f","type":"file","path":"/etc/hosts","into":"block","content":"${secret:s}","secrets":{"s":"abc"}}`, "secrets"}, + "create-once": {`{"id":"f","type":"file","path":"/etc/hosts","into":"block","content":"a","create-once":true}`, "create-once"}, + "at on a whole file": {`{"id":"f","type":"file","path":"/etc/hosts","content":"a","at":"start"}`, `at "start"`}, + "at on a JSON file": {`{"id":"f","type":"file","path":"/etc/x.json","into":"json","content":"{}","at":"end"}`, `at "end"`}, + "at somewhere else": {`{"id":"f","type":"file","path":"/etc/hosts","into":"block","content":"a","at":"middle"}`, `"start" or "end"`}, + "an unknown format": {`{"id":"f","type":"file","path":"/etc/hosts","into":"lines","content":"a"}`, `"json" or "block"`}, + } { + _, err := Parse([]byte(`{"declaration":1,"resources":[` + c.resource + `]}`)) + if err == nil || !strings.Contains(err.Error(), c.refusal) { + t.Errorf("%s: want a refusal naming %q, got %v", name, c.refusal, err) + } + } +} + +func TestAFileWrittenIntoABlockIsRead(t *testing.T) { + d, err := Parse([]byte(`{"declaration":1,"resources":[ + {"id":"mesh-wireguard.fact-node-names","type":"file","path":"/etc/hosts","into":"block","content":"10.42.0.1 ace\n# BEGIN devtool x\n"}, + {"id":"dhcpcd.options","type":"file","path":"/etc/dhcpcd.conf","into":"block","content":"nohook resolv.conf\n","at":"start"}, + {"id":"empty","type":"file","path":"/etc/x","into":"block","content":"","at":"end"} + ]}`)) + if err != nil { + t.Fatal(err) + } + if f := d.Resources[0].(*File); f.Into != IntoBlock || f.At != "" { + t.Errorf("read as into %q at %q", f.Into, f.At) + } + if f := d.Resources[1].(*File); f.At != AtStart { + t.Errorf("at was read as %q", f.At) + } + if begin, end := BlockMarkers("mesh-wireguard.fact-node-names"); begin != "# BEGIN mesh mesh-wireguard.fact-node-names" || + end != "# END mesh mesh-wireguard.fact-node-names" { + t.Errorf("the markers are %q and %q", begin, end) + } +} diff --git a/internal/declaration/declaration.go b/internal/declaration/declaration.go index 5fa7f27..65765a8 100644 --- a/internal/declaration/declaration.go +++ b/internal/declaration/declaration.go @@ -161,8 +161,27 @@ type File struct { // "json" is spoken — the content is a JSON object whose keys the host sets in the file's // object, keeping every other key as it found it and recording what each of its keys held // before, so undeclaring the file gives those back. + // + // "block" is the same idea for a file that is not structured (novox/hq issue 128): the + // content is the mesh's lines, and the host owns only the region between `# BEGIN mesh ` + // and `# END mesh `, keeping every line outside it byte for byte. The machine's hosts file + // is the case that needed it — on a workstation the distribution, a local development tool and + // the operator all write into it, and the mesh writing it whole took their lines away at the + // next change to the mesh's names, silently. Marked blocks are the shape the other tools in + // that file already use, and `#` is the comment character of every file this serves. Into string `json:"into,omitempty"` + // At is where a file written into a block has its region added when the file does not hold + // one yet: "end", the default, or "start". A region already there stays where it is, whatever + // this says — moving it would move the lines around it, and those are the machine's. + // + // **Some files give a line its meaning by what stands above it.** dhcpcd's configuration scopes + // every line after `interface X` to that interface, and a real one ends with exactly that — an + // interface and its static address. A region added at the end would make the mesh's global + // options (`nohook resolv.conf`, `denyinterfaces mesh0`) options of one interface, and dhcpcd + // would read them without complaint. At the start, nothing stands above them. + At string `json:"at,omitempty"` + // Sealed is content encrypted to this node's sealing key, for a file the mesh must deliver // without being able to read. // @@ -241,16 +260,45 @@ func (f *File) validate(where string, _ bool) []string { problems = append(problems, where+ ": a file written into JSON carries a JSON object of the keys it sets") } - if f.Sealed != "" || f.Bytes != "" || len(f.Secrets) > 0 || f.CreateOnce { + case IntoBlock: + // The markers are how the host finds its region again. A marker in the content would be + // a second region, or the end of this one, the next time the file is read — and the host + // would then rewrite, or on undeclare take out, lines that were never the mesh's. + for _, line := range strings.Split(f.Content, "\n") { + if strings.HasPrefix(line, BlockBegin) || strings.HasPrefix(line, BlockEnd) { + problems = append(problems, fmt.Sprintf( + "%s: a file written into a block carries the mesh's lines, and %q is a marker the "+ + "host keeps for itself", where, strings.TrimRight(line, "\r"))) + break + } + } + // The id is written into the markers, so it has to stay on one line. + if strings.ContainsAny(f.ID, "\r\n") { problems = append(problems, where+ - ": a file written into says only its keys, in content — not sealed, bytes, "+ - "secrets or create-once") + ": a file written into a block names its region by its id, and this id spans lines") } default: problems = append(problems, fmt.Sprintf( - "%s: into %q; a file is written into \"json\", or omits it to be written whole", + "%s: into %q; a file is written into \"json\" or \"block\", or omits it to be written whole", where, f.Into)) } + switch { + case f.At == "": + case f.Into != IntoBlock: + problems = append(problems, fmt.Sprintf( + "%s: at %q; only a file written into a block has a place its region is added", where, f.At)) + case f.At != AtStart && f.At != AtEnd: + problems = append(problems, fmt.Sprintf( + "%s: at %q; a block is added at \"start\" or \"end\", or omits it to be added at the end", + where, f.At)) + } + if f.Into == IntoJSON || f.Into == IntoBlock { + if f.Sealed != "" || f.Bytes != "" || len(f.Secrets) > 0 || f.CreateOnce { + problems = append(problems, where+ + ": a file written into says only its part, in content — not sealed, bytes, "+ + "secrets or create-once") + } + } var said []string for name, value := range map[string]string{ "content": f.Content, "sealed": f.Sealed, "bytes": f.Bytes, @@ -670,8 +718,31 @@ func (s *Service) validate(where string, _ bool) []string { return problems } -// IntoJSON is the one structured format a file is written into. -const IntoJSON = "json" +// What a file is written into (novox/hq ADR 0102): a JSON object whose keys the mesh sets, or a +// text file in which the mesh owns one marked block of lines (novox/hq issue 128). +const ( + IntoJSON = "json" + IntoBlock = "block" +) + +// The lines that delimit the mesh's region in a file written into a block, each followed by the +// resource's id. Exact lines, never patterns: another tool's `# BEGIN …` block in the same file is +// that tool's, and a marker that merely resembled the mesh's must not be taken for it. +const ( + BlockBegin = "# BEGIN mesh " + BlockEnd = "# END mesh " +) + +// Where a file written into a block has its region added, when it has none yet. +const ( + AtStart = "start" + AtEnd = "end" +) + +// BlockMarkers are the two lines, without their line ends, that delimit a resource's region. +func BlockMarkers(id string) (begin, end string) { + return BlockBegin + id, BlockEnd + id +} // Opening is a port reachable on an adopted node, from where, and on which path. // diff --git a/internal/store/store.go b/internal/store/store.go index eb8264d..a33ba5d 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -84,7 +84,7 @@ type Applied struct { Reads map[string]string `json:"reads,omitempty"` } -// Into is what a file written into held before the mesh's keys. +// Into is what a file written into held before the mesh's keys, or before the mesh's block. type Into struct { Format string `json:"format"` Before map[string]json.RawMessage `json:"before,omitempty"` @@ -94,6 +94,19 @@ type Into struct { // to the machine's list — never a member that was already there. Undeclared, only these go, // and drift is judged on these alone (novox/hq ADR 0102). Added map[string][]json.RawMessage `json:"added,omitempty"` + + // Region is, for a file written into a block (novox/hq issue 128), what the lines between the + // mesh's markers held before the mesh wrote them — nil when there was no region, which is + // different from a region that was there and empty. Undeclared, a recorded region is put back + // and an unrecorded one is taken out, markers and all. It is the block's "what each key held + // before": the one thing the host needs to give the file back. + Region *string `json:"region,omitempty"` + // Separated says the host put a blank line between the region and the machine's lines when it + // added the region — before it at the end, after it at the start, as At says — so taking the + // region out takes that line with it and nothing of the operator's. + Separated bool `json:"separated,omitempty"` + // At is where the host added the region: "start", or empty for the end. + At string `json:"at,omitempty"` } // State is the whole of what a node knows about what it has done. From fdc768c4762020868d201515de38767950e99313 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 27 Sep 2026 00:09:34 +0200 Subject: [PATCH 05/11] review: rebuild a file the mesh once wrote whole, keep links, give back a missing line end, and release a hold only after the write (hq issue 128) --- internal/apply/apply.go | 25 +++- internal/apply/block.go | 172 +++++++++++++++++++++++----- internal/apply/block_test.go | 147 ++++++++++++++++++++++++ internal/apply/hold.go | 7 +- internal/declaration/block_test.go | 1 + internal/declaration/declaration.go | 10 +- internal/store/store.go | 10 ++ 7 files changed, 338 insertions(+), 34 deletions(-) diff --git a/internal/apply/apply.go b/internal/apply/apply.go index 8ec6c02..fa0a83f 100644 --- a/internal/apply/apply.go +++ b/internal/apply/apply.go @@ -52,6 +52,8 @@ type Outcome struct { wrote string // into is what a file written into held before the mesh's keys (novox/hq ADR 0102). into *store.Into + // kept is where this apply kept the original of a file it wrote over (novox/hq ADR 0100). + kept string // reads is, for a container, the digest of each file it was created reading, by path — so // the next apply can say which one changed (novox/hq 04-ISSUES/103). reads map[string]string @@ -441,6 +443,16 @@ func ApplyKeeping( continue } + // Where the original of what this file replaced was kept, carried for as long as the + // resource is recorded: kept by this apply, by a hold its module's cutover ends, or before. + held, wasHeld := known.HeldAt(resource.Identity()) + kept := outcome.kept + if kept == "" && wasHeld { + kept = held.Kept + } + if kept == "" { + kept = was.Kept + } // Only now. The record follows the fact, never leads it. known.Record(store.Applied{ Origin: origin, @@ -448,13 +460,19 @@ func ApplyKeeping( Target: outcome.Target, AppliedAt: time.Now().UTC(), Wrote: outcome.wrote, Into: outcome.into, + Kept: kept, Reads: outcome.reads, Holds: holds(resource), }) - // Its module has been taken, and what was held for it is now the mesh's. - if held, wasHeld := known.HeldAt(resource.Identity()); wasHeld { + // Its module has been taken, and what was held for it is now the mesh's. A file written + // into replaced nothing that was found, so its outcome says what the write did, not that + // a cutover happened; its hold from when it was declared whole goes all the same — here, + // after the write worked, so a failed one keeps the hold and where its original is. + if wasHeld { known.Release(held.ID) - outcome.Detail = takenDetail(held) + if f, isFile := resource.(*declaration.File); !isFile || f.Into == "" { + outcome.Detail = takenDetail(held) + } } if svc, ok := resource.(*declaration.Service); ok && svc.TakesOver != nil && report.Tunnel != nil { // The found interface is down and the mesh's is up in its place: the tunnel changed @@ -840,6 +858,7 @@ func applyFile(r *declaration.File, previous store.Applied, unseal Unseal, keepF default: out.Action = "unchanged" } + out.kept = kept if kept != "" { if out.Detail != "" { out.Detail += "; " diff --git a/internal/apply/block.go b/internal/apply/block.go index 7115cda..0ebca87 100644 --- a/internal/apply/block.go +++ b/internal/apply/block.go @@ -3,6 +3,7 @@ package apply import ( "errors" "fmt" + "net" "os" "path/filepath" "strings" @@ -28,17 +29,71 @@ import ( // mesh created goes only if nothing but whitespace is left. // applyBlock writes a file's declared lines into its region of the file already at its path. +// +// **A link stays a link.** Where the path is a symbolic link — a hosts file some distributions keep +// elsewhere and link into /etc — the file read, written and renamed over is the one it points to, +// so the link and whatever manages it are left as they were. A file written whole, or into JSON, +// still replaces a link with a file; that is unchanged here. func applyBlock(r *declaration.File, previous store.Applied) (Outcome, error) { out := begin(r) opening, closing := declaration.BlockMarkers(r.ID) want := blockBody(r.Content) - raw, err := os.ReadFile(r.Path) + real, err := realPath(r.Path) + if err != nil { + return out, err + } + raw, err := os.ReadFile(real) existed := err == nil if err != nil && !errors.Is(err, os.ErrNotExist) { return out, err } + // What the file is, taken once with what it holds: its mode and owner are the machine's and + // go back onto what is written. A file read and then not there to stat is a failure, never a + // file with no owner. + var info os.FileInfo + if existed { + if info, err = os.Stat(real); err != nil { + return out, fmt.Errorf("read %s and cannot see it: %w", r.Path, err) + } + } existing := string(raw) + + rec := store.Into{Format: declaration.IntoBlock} + var note string + rebuilt := false + + // **A file the mesh once wrote whole** (novox/hq issue 128). The resource keeps its id when its + // module moves from writing the file whole to writing into it, and the file on the machine is + // then the mesh's own old write — its header, its loopback lines, its names. Adding the region + // after that would leave the old names above the new ones, and a resolver takes the first + // line that answers: the region would be shadowed by what it replaced. So the file is rebuilt: + // the original the mesh kept before its first write, with the region in it; or, where the mesh + // made the file itself, the loopback lines every machine needs, kept as the machine's, with the + // region beside them. Changed since the mesh wrote it, the file is somebody's again and is + // written into as it stands, and the outcome says so. + if existed && previous.Into == nil && previous.Wrote != "" { + if digestOf(existing) == previous.Wrote { + if previous.Kept != "" { + original, err := os.ReadFile(previous.Kept) + if err != nil { + return out, fmt.Errorf("%s was written whole by the mesh over an original kept at %s, "+ + "which cannot be read to give it back: %w; it was left as it is", r.Path, previous.Kept, err) + } + existing = string(original) + note = "the mesh's old whole file replaced by the original kept at " + previous.Kept + ", with the region in it" + } else { + existing = loopbackOf(existing) + rec.Created = true + note = "the mesh's old whole file replaced by its loopback lines and the region" + } + // Not what was read: the whole of it was the mesh's, and the file is written afresh. + rebuilt = true + } else { + note = "a file the mesh once wrote whole, changed since; its old lines were kept" + } + } + lines := linesOf(existing) at, found, err := regionIn(lines, opening, closing) if err != nil { @@ -49,14 +104,14 @@ func applyBlock(r *declaration.File, previous store.Applied) (Outcome, error) { // A record of a block is carried; anything else — no record, a file once written whole, one // once written into as JSON — is a file the host is seeing for the first time as a block. - rec := store.Into{Format: declaration.IntoBlock} recorded := previous.Into != nil && previous.Into.Format == declaration.IntoBlock if recorded && existed { rec.Created = previous.Into.Created rec.Region = previous.Into.Region rec.Separated = previous.Into.Separated rec.At = previous.Into.At - } else { + rec.Ended = previous.Into.Ended + } else if !rebuilt { // A file gone since the last apply is made again, and made by the mesh: what it held // before went with it, so there is nothing to give back but the file's absence. rec.Created = !existed @@ -89,7 +144,7 @@ func applyBlock(r *declaration.File, previous store.Applied) (Outcome, error) { case r.At == declaration.AtStart: // Above everything, and one blank line between the region and the machine's first line // unless there is one already — a line in some files means what the lines above it say. - rec.At, rec.Separated = declaration.AtStart, false + rec.At, rec.Separated, rec.Ended = declaration.AtStart, false, false next = regionOf(opening, closing, want) if existing != "" && !strings.HasPrefix(existing, "\n") { next += "\n" @@ -99,10 +154,11 @@ func applyBlock(r *declaration.File, previous store.Applied) (Outcome, error) { default: // At the end, apart from whatever is there: the file's last line is ended if it was not, // and one blank line separates the region from the machine's lines unless there is one. - rec.At, rec.Separated = "", false + rec.At, rec.Separated, rec.Ended = "", false, false next = existing if next != "" && !strings.HasSuffix(next, "\n") { next += "\n" + rec.Ended = true } if next != "" && next != "\n" && !strings.HasSuffix(next, "\n\n") { next += "\n" @@ -110,45 +166,60 @@ func applyBlock(r *declaration.File, previous store.Applied) (Outcome, error) { } next += regionOf(opening, closing, want) } + // What was not the mesh's is what it was. By construction — and checked, because a slip in + // splicing lines is exactly the fault this mode exists to prevent, and it must never be written. + if found { + after := linesOf(next) + if where, ok, err := regionIn(after, opening, closing); err != nil || !ok || outside(after, where) != outside(lines, at) { + return out, fmt.Errorf("%s: writing the region would change lines outside it; it was left as it is", r.Path) + } + } - same := existed && next == existing + same := existed && next == string(raw) if !same { - var info os.FileInfo mode := os.FileMode(0o644) - if info, err = os.Stat(r.Path); err == nil { + if info != nil { mode = info.Mode().Perm() // the machine's file keeps the machine's mode } else if mode, err = modeOf(r.Mode, mode); err != nil { return out, err } - if err := os.MkdirAll(filepath.Dir(r.Path), 0o755); err != nil { + if err := os.MkdirAll(filepath.Dir(real), 0o755); err != nil { return out, err } - if err := writeAtomically(r.Path, []byte(next), mode); err != nil { + if err := writeAtomically(real, []byte(next), mode); err != nil { return out, err } - if existed { + if info != nil { // The write is a new file renamed over the old, so it belongs to whoever wrote it. The // machine's file keeps the machine's owner, as it keeps its mode. - if err := keepOwner(r.Path, info); err != nil { + if err := keepOwner(real, info); err != nil { return out, err } - } else if err := own(r.Path, r.Owner); err != nil { + } else if err := own(real, r.Owner); err != nil { return out, err } } - // Read back: the region holds what was declared, and nothing outside it moved. - written, err := os.ReadFile(r.Path) + // Read back: the region holds what was declared. Only the region — another tool writing its + // own lines in the moment after the rename is not a failed write. What remains is the moment + // between reading the file and renaming over it: a line another tool writes there is lost, and + // found again at its next write. Nothing short of a lock every writer honours closes that, and + // the other writers of a hosts file honour none. + written, err := os.ReadFile(real) if err != nil { return out, fmt.Errorf("wrote into %s and cannot read it back: %w", r.Path, err) } - if string(written) != next { - return out, fmt.Errorf("%s does not hold the mesh's region as written after writing into it", r.Path) + back := linesOf(string(written)) + if where, ok, err := regionIn(back, opening, closing); err != nil || !ok || where.body(back) != want { + return out, fmt.Errorf("%s does not hold the mesh's region after writing into it", r.Path) } out.into = &rec out.wrote = digestOf(want) switch { + case note != "" && !same: + out.Action = "updated" + out.Detail = note case !existed: out.Action = "created" out.Detail = "written into; the file was not there" @@ -173,16 +244,20 @@ func applyBlock(r *declaration.File, previous store.Applied) (Outcome, error) { // removeBlock gives back what a file written into a block held before the mesh's region. func removeBlock(a store.Applied) (string, string, error) { - raw, err := os.ReadFile(a.Target) + real, err := realPath(a.Target) + if err != nil { + return "", "", err + } + raw, err := os.ReadFile(real) if errors.Is(err, os.ErrNotExist) { return "forgotten", "no longer there", nil } if err != nil { return "", "", err } - info, err := os.Stat(a.Target) + info, err := os.Stat(real) if err != nil { - return "", "", err + return "", "", fmt.Errorf("read %s and cannot see it: %w", a.Target, err) } opening, closing := declaration.BlockMarkers(a.ID) lines := linesOf(string(raw)) @@ -198,8 +273,10 @@ func removeBlock(a store.Applied) (string, string, error) { action, detail = "restored", "no longer declared; the region was given back what it held" case found: from, to := at.begin, at.end+1 - // The blank line the host put beside the region, and only that one: if what stands there - // now is not blank, it is somebody's, and it stays. + // The blank line the host added beside the region, when a blank line still stands there. + // Whether it is the same one the host added cannot be known from the file; a blank line + // is the one line whose going changes nothing any program reads, so it is taken. A line + // that is not blank is never taken, whoever put it there. if a.Into.Separated { if a.Into.At == declaration.AtStart { if to < len(lines) && lines[to] == "\n" { @@ -210,11 +287,15 @@ func removeBlock(a store.Applied) (string, string, error) { } } next = strings.Join(lines[:from], "") + strings.Join(lines[to:], "") + // And the line end the host gave the machine's last line, if that line is still last. + if a.Into.Ended && strings.Join(lines[to:], "") == "" { + next = strings.TrimSuffix(next, "\n") + } action, detail = "restored", "no longer declared; the mesh's region was taken out and every other line kept" } - if a.Into.Created && strings.TrimSpace(next) == "" { - if err := os.Remove(a.Target); err != nil { + if a.Into.Created && strings.TrimSpace(next) == "" && real == a.Target { + if err := os.Remove(real); err != nil { return "", "", err } return "removed", "no longer declared; the mesh had created it and nothing else was in it", nil @@ -222,15 +303,54 @@ func removeBlock(a store.Applied) (string, string, error) { if next == string(raw) { return action, detail, nil } - if err := writeAtomically(a.Target, []byte(next), info.Mode().Perm()); err != nil { + if err := writeAtomically(real, []byte(next), info.Mode().Perm()); err != nil { return "", "", err } - if err := keepOwner(a.Target, info); err != nil { + if err := keepOwner(real, info); err != nil { return "", "", err } return action, detail, nil } +// realPath is the file a path names, through any links; a path that is not there yet is itself. +// A link to nothing is refused: writing through it would replace the link with a file. +func realPath(path string) (string, error) { + real, err := filepath.EvalSymlinks(path) + if err == nil { + return real, nil + } + if _, lerr := os.Lstat(path); errors.Is(lerr, os.ErrNotExist) { + return path, nil + } + return "", fmt.Errorf("%s is a link the host cannot follow to a file: %w; it was left as it is", path, err) +} + +// loopbackOf is the lines of a file that answer for the machine itself — localhost, its own name on +// 127.0.1.1, ::1 — and nothing else: what the mesh's old whole hosts file carried that the machine +// needs, without the mesh's header or its names. +func loopbackOf(text string) string { + var b strings.Builder + for _, line := range linesOf(text) { + fields := strings.Fields(line) + if len(fields) < 2 { + continue + } + if ip := net.ParseIP(fields[0]); ip != nil && ip.IsLoopback() { + b.WriteString(strings.TrimSuffix(line, "\n") + "\n") + } + } + return b.String() +} + +// outside is every line of a file but the mesh's region, markers included, as one string. +func outside(lines []string, at region) string { + end := at.end + 1 + if end > len(lines) { + end = len(lines) + } + return strings.Join(lines[:at.begin], "") + "\x00" + strings.Join(lines[end:], "") +} + // blockBody is the declared lines as they stand in the region: ending in exactly one line end, or // nothing at all when there are no lines. func blockBody(content string) string { diff --git a/internal/apply/block_test.go b/internal/apply/block_test.go index 2dfefab..51285f9 100644 --- a/internal/apply/block_test.go +++ b/internal/apply/block_test.go @@ -469,3 +469,150 @@ func TestTheRecordOfABlockSurvivesTheStateFile(t *testing.T) { t.Errorf("undeclaring from a saved state left %q", got) } } + +// The mesh's old whole hosts file, as the controller composed it before issue 128. +const oldWholeHosts = "# Generated by the mesh. Do not edit — this file is replaced whenever a machine\n" + + "# joins or leaves, and an edit would survive until then and vanish.\n\n" + + "127.0.0.1\tlocalhost\n" + + "::1\t\tlocalhost ip6-localhost ip6-loopback\n" + + "127.0.1.1\tg14\n" + + "\n" + + "10.42.0.1\tace.internal\tace\n" + + "10.42.0.9\tg14.internal\tg14\t# this machine\n" + +func wholeDecl(path, content string) string { + return fmt.Sprintf(`{"declaration":1,"resources":[ + {"id":%q,"type":"file","path":%q,"content":%q} + ]}`, namesID, path, content) +} + +func applyKeepingIn(t *testing.T, raw string, known store.State, keepDir string) (Report, store.State) { + t.Helper() + report, state, err := ApplyKeeping(context.Background(), archHost(t), parse(t, raw), known, + store.OriginDeclared, (&machine{}).run, nil, nil, KeepIn(keepDir)) + if err != nil { + t.Fatalf("apply failed: %v", err) + } + return report, state +} + +func TestAFileTheMeshWroteWholeAndMadeItselfKeepsOnlyItsLoopbackLines(t *testing.T) { + // Written whole into a file that was not there, then declared as a block under the same id: + // the old names must not stay above the region, where a resolver would answer from them first. + path := filepath.Join(t.TempDir(), "hosts") + _, state := applyKeepingIn(t, wholeDecl(path, oldWholeHosts), store.State{}, t.TempDir()) + report, state := applyKeepingIn(t, blockDecl(t, path, meshNames), state, t.TempDir()) + floor := "127.0.0.1\tlocalhost\n::1\t\tlocalhost ip6-localhost ip6-loopback\n127.0.1.1\tg14\n" + if got := readText(t, path); got != floor+"\n"+marked(namesID, meshNames) { + t.Fatalf("the old whole file became:\n%q", got) + } + o := outcomeOf(report, namesID) + if o.Action != "updated" || !strings.Contains(o.Detail, "loopback lines") { + t.Errorf("the rebuild was reported as %q: %s", o.Action, o.Detail) + } + if rec, _ := state.Find(namesID); !rec.Into.Created { + t.Error("a file the mesh made itself was not recorded as the mesh's") + } + report, _ = applyKeepingIn(t, blockDecl(t, path, meshNames), state, t.TempDir()) + if got := outcomeOf(report, namesID).Action; got != "unchanged" { + t.Errorf("applied again, the rebuilt file was %q", got) + } + undeclare(t, state) + if got := readText(t, path); got != floor { + t.Errorf("undeclared, the file holds %q", got) + } +} + +func TestAFileTheMeshWroteWholeOverAnOriginalGetsTheOriginalBack(t *testing.T) { + path := filepath.Join(t.TempDir(), "hosts") + _ = os.WriteFile(path, []byte(workstationHosts), 0o644) + keep := t.TempDir() + _, state := applyKeepingIn(t, wholeDecl(path, oldWholeHosts), store.State{}, keep) + if rec, _ := state.Find(namesID); rec.Kept == "" { + t.Fatal("where the original was kept was not recorded") + } + report, state := applyKeepingIn(t, blockDecl(t, path, meshNames), state, keep) + if got := readText(t, path); got != workstationHosts+"\n"+marked(namesID, meshNames) { + t.Fatalf("the old whole file became:\n%q", got) + } + if d := outcomeOf(report, namesID).Detail; !strings.Contains(d, "original kept at") { + t.Errorf("the rebuild was reported as: %s", d) + } + undeclare(t, state) + if got := readText(t, path); got != workstationHosts { + t.Errorf("undeclared, the machine did not get its original back: %q", got) + } +} + +func TestAFileTheMeshWroteWholeAndSomebodyChangedIsWrittenIntoAsItStands(t *testing.T) { + path := filepath.Join(t.TempDir(), "hosts") + _, state := applyKeepingIn(t, wholeDecl(path, oldWholeHosts), store.State{}, t.TempDir()) + edited := oldWholeHosts + "192.168.1.20 printer\n" + _ = os.WriteFile(path, []byte(edited), 0o644) + report, _ := applyKeepingIn(t, blockDecl(t, path, meshNames), state, t.TempDir()) + if got := readText(t, path); got != edited+"\n"+marked(namesID, meshNames) { + t.Fatalf("an edited whole file became:\n%q", got) + } + if d := outcomeOf(report, namesID).Detail; !strings.Contains(d, "changed since; its old lines were kept") { + t.Errorf("the outcome does not say so: %s", d) + } +} + +func TestALinkedFileStaysALink(t *testing.T) { + dir := t.TempDir() + real := filepath.Join(dir, "static", "hosts") + _ = os.MkdirAll(filepath.Dir(real), 0o755) + _ = os.WriteFile(real, []byte(workstationHosts), 0o644) + link := filepath.Join(dir, "hosts") + if err := os.Symlink(real, link); err != nil { + t.Fatal(err) + } + _, state := applyBlockDecl(t, blockDecl(t, link, meshNames), store.State{}) + if info, err := os.Lstat(link); err != nil || info.Mode()&os.ModeSymlink == 0 { + t.Fatalf("the link was replaced by a file") + } + if got := readText(t, real); got != workstationHosts+"\n"+marked(namesID, meshNames) { + t.Errorf("the file the link names holds %q", got) + } + undeclare(t, state) + if info, err := os.Lstat(link); err != nil || info.Mode()&os.ModeSymlink == 0 { + t.Fatalf("undeclaring replaced the link with a file") + } + if got := readText(t, real); got != workstationHosts { + t.Errorf("undeclared, the file the link names holds %q", got) + } +} + +func TestALastLineWithNoEndIsGivenBackWithNone(t *testing.T) { + path := filepath.Join(t.TempDir(), "hosts") + _ = os.WriteFile(path, []byte("x"), 0o644) + _, state := applyBlockDecl(t, blockDecl(t, path, meshNames), store.State{}) + if got := readText(t, path); got != "x\n\n"+marked(namesID, meshNames) { + t.Fatalf("got %q", got) + } + undeclare(t, state) + if got := readText(t, path); got != "x" { + t.Errorf("undeclaring left %q, the machine had %q", got, "x") + } +} + +func TestAFailedBlockWriteKeepsItsHold(t *testing.T) { + // Held from when it was declared whole, then declared as a block into a file whose markers do + // not pair: the write is refused, and the hold — with where its original is — stays. + path := filepath.Join(t.TempDir(), "hosts") + broken := "a\n# BEGIN mesh " + namesID + "\n" + _ = os.WriteFile(path, []byte(broken), 0o644) + known := store.State{Held: []store.Held{{ID: namesID, Module: "mesh-wireguard", Kind: "file", + Target: path, Kept: "/var/lib/mesh/kept/hosts"}}} + resource := fmt.Sprintf(`{"id":%q,"type":"file","path":%q,"into":"block","content":%q}`, namesID, path, meshNames) + d := adopted(t, `{"taken":[],"untaken":{"mesh-wireguard":["`+namesID+`"]}}`, resource) + _, state, err := ApplyKeeping(context.Background(), archHost(t), d, known, + store.OriginDeclared, (&machine{}).run, nil, nil, KeepIn(t.TempDir())) + if err == nil { + t.Fatal("a write into unpaired markers was not refused") + } + h, held := state.HeldAt(namesID) + if !held || h.Kept != "/var/lib/mesh/kept/hosts" { + t.Errorf("a failed write released the hold: %+v", state.Held) + } +} diff --git a/internal/apply/hold.go b/internal/apply/hold.go index b65fad4..1bd5cca 100644 --- a/internal/apply/hold.go +++ b/internal/apply/hold.go @@ -295,11 +295,10 @@ func holdOnAdopted(ctx context.Context, sys system.System, r declaration.Resourc } // A file written into replaces nothing that was found, so it is never held (novox/hq ADR - // 0102) — and a hold from when it was declared whole must not keep the mesh's keys out. + // 0102) — and a hold from when it was declared whole must not keep the mesh's keys out. That + // hold is released by the apply once the write has worked, not here: a write that fails keeps + // it, and with it where the original was kept. if f, ok := r.(*declaration.File); ok && f.Into != "" { - if already { - known.Release(r.Identity()) - } return false, false, out, nil } diff --git a/internal/declaration/block_test.go b/internal/declaration/block_test.go index a9acdec..1512a00 100644 --- a/internal/declaration/block_test.go +++ b/internal/declaration/block_test.go @@ -20,6 +20,7 @@ func TestAFileWrittenIntoABlockIsRefusedUnlessItIsOnlyItsLines(t *testing.T) { "at on a whole file": {`{"id":"f","type":"file","path":"/etc/hosts","content":"a","at":"start"}`, `at "start"`}, "at on a JSON file": {`{"id":"f","type":"file","path":"/etc/x.json","into":"json","content":"{}","at":"end"}`, `at "end"`}, "at somewhere else": {`{"id":"f","type":"file","path":"/etc/hosts","into":"block","content":"a","at":"middle"}`, `"start" or "end"`}, + "an id ending in a space": {`{"id":"f ","type":"file","path":"/etc/hosts","into":"block","content":"a"}`, "whitespace"}, "an unknown format": {`{"id":"f","type":"file","path":"/etc/hosts","into":"lines","content":"a"}`, `"json" or "block"`}, } { _, err := Parse([]byte(`{"declaration":1,"resources":[` + c.resource + `]}`)) diff --git a/internal/declaration/declaration.go b/internal/declaration/declaration.go index 65765a8..0b5d078 100644 --- a/internal/declaration/declaration.go +++ b/internal/declaration/declaration.go @@ -169,6 +169,9 @@ type File struct { // the operator all write into it, and the mesh writing it whole took their lines away at the // next change to the mesh's names, silently. Marked blocks are the shape the other tools in // that file already use, and `#` is the comment character of every file this serves. + // + // Written into, in either format, the file's mode and owner are the machine's: a declared mode + // and owner apply only to a file the host creates, and a file that was there keeps its own. Into string `json:"into,omitempty"` // At is where a file written into a block has its region added when the file does not hold @@ -272,10 +275,15 @@ func (f *File) validate(where string, _ bool) []string { break } } - // The id is written into the markers, so it has to stay on one line. + // The id is written into the markers, so it has to stay on one line — and whitespace at + // either end of it is whitespace the host would have to match exactly in a line some + // editor may trim. if strings.ContainsAny(f.ID, "\r\n") { problems = append(problems, where+ ": a file written into a block names its region by its id, and this id spans lines") + } else if strings.TrimSpace(f.ID) != f.ID { + problems = append(problems, where+ + ": a file written into a block names its region by its id, and this id begins or ends in whitespace") } default: problems = append(problems, fmt.Sprintf( diff --git a/internal/store/store.go b/internal/store/store.go index a33ba5d..9465b27 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -70,6 +70,13 @@ type Applied struct { // anywhere saying why. Wrote string `json:"wrote,omitempty"` + // Kept is where the original of a file this host wrote over was kept (novox/hq ADR 0100): + // by the keep on its first write, or by the hold that was released when its module was taken. + // Recorded rather than only reported, because a file once written whole and now written into + // (novox/hq issue 128) is given back its original with the mesh's region in it — and a path + // said once in a log line is not a path the host can find again. + Kept string `json:"kept,omitempty"` + // Into is set for a file written into rather than over (novox/hq ADR 0102): the format, what // each of the mesh's keys held before it set them, which of them were absent, and whether the // file itself was — so undeclaring it gives the machine back exactly what it had. @@ -107,6 +114,9 @@ type Into struct { Separated bool `json:"separated,omitempty"` // At is where the host added the region: "start", or empty for the end. At string `json:"at,omitempty"` + // Ended says the machine's last line had no line end and the host gave it one to add the + // region after it, so taking the region out takes that line end too. + Ended bool `json:"ended,omitempty"` } // State is the whole of what a node knows about what it has done. From 06aaac0820dd0c9480623fc582867a04ab1a0f63 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 27 Sep 2026 00:11:58 +0200 Subject: [PATCH 06/11] A service may omit its state, so unassigning an uplink module never stops the machine's network manager (hq ADR 0117) --- internal/apply/apply.go | 98 +++++++++++-- internal/apply/hold.go | 20 ++- internal/apply/plan.go | 17 ++- internal/apply/stateless_test.go | 191 +++++++++++++++++++++++++ internal/declaration/declaration.go | 37 ++++- internal/declaration/stateless_test.go | 30 ++++ internal/store/store.go | 5 + 7 files changed, 374 insertions(+), 24 deletions(-) create mode 100644 internal/apply/stateless_test.go create mode 100644 internal/declaration/stateless_test.go diff --git a/internal/apply/apply.go b/internal/apply/apply.go index fa0a83f..fff75e1 100644 --- a/internal/apply/apply.go +++ b/internal/apply/apply.go @@ -54,6 +54,8 @@ type Outcome struct { into *store.Into // kept is where this apply kept the original of a file it wrote over (novox/hq ADR 0100). kept string + // stateless is a service whose unit's lifecycle is the machine's (novox/hq ADR 0117). + stateless bool // reads is, for a container, the digest of each file it was created reading, by path — so // the next apply can say which one changed (novox/hq 04-ISSUES/103). reads map[string]string @@ -458,19 +460,21 @@ func ApplyKeeping( Origin: origin, ID: resource.Identity(), Type: string(resource.Kind()), Target: outcome.Target, AppliedAt: time.Now().UTC(), - Wrote: outcome.wrote, - Into: outcome.into, - Kept: kept, - Reads: outcome.reads, - Holds: holds(resource), + Wrote: outcome.wrote, + Into: outcome.into, + Kept: kept, + Reads: outcome.reads, + Stateless: outcome.stateless, + Holds: holds(resource), }) // Its module has been taken, and what was held for it is now the mesh's. A file written - // into replaced nothing that was found, so its outcome says what the write did, not that - // a cutover happened; its hold from when it was declared whole goes all the same — here, - // after the write worked, so a failed one keeps the hold and where its original is. + // into, or a service whose lifecycle is the machine's, replaced nothing that was found, so + // its outcome says what the apply did, not that a cutover happened; a hold from when it was + // declared otherwise goes all the same — here, after the apply worked, so a failed one + // keeps the hold and where its original is. if wasHeld { known.Release(held.ID) - if f, isFile := resource.(*declaration.File); !isFile || f.Into == "" { + if !replacesNothing(resource) { outcome.Detail = takenDetail(held) } } @@ -919,6 +923,9 @@ type unitReloader interface { func applyService(ctx context.Context, sys system.System, r *declaration.Service, run Runner, changed map[string]bool) (Outcome, error) { + if r.Stateless() { + return reflectOnly(ctx, sys, r, run, changed) + } out := begin(r) var changes []string @@ -1029,6 +1036,73 @@ func applyService(ctx context.Context, sys system.System, r *declaration.Service return out, nil } +// reflectOnly is a service whose unit's lifecycle is the machine's (novox/hq ADR 0117): nothing is +// started, stopped, enabled or disabled, and a changed trigger is acted on only where the unit is +// already running. An inactive unit is left so — started, it would be a second network manager on +// a machine that uses another — and it reads the change when whatever starts it does. +func reflectOnly(ctx context.Context, sys system.System, r *declaration.Service, run Runner, + changed map[string]bool) (Outcome, error) { + out := begin(r) + out.stateless = true + restart := reflected(r, changed) + reload := restartedBy(r.ReloadOn, changed) + if len(restart) == 0 && len(reload) == 0 { + out.Action = "unchanged" + out.Detail = "its lifecycle is the machine's; nothing it reflects changed" + return out, nil + } + state, err := sys.ServiceState(ctx, run, r.Unit) + if err != nil { + return out, err + } + if state != "running" { + out.Action = "unchanged" + out.Detail = "not running; the change applies at its next start" + return out, nil + } + if len(restart) > 0 { + // The same as a stated service: the unit's own file may be what changed, and the manager + // reads that again only when told to. + if u, ok := sys.(unitReloader); ok { + if err := u.ReloadUnits(ctx, run); err != nil { + return out, fmt.Errorf("reloading the service manager's units for %s: %w", r.Unit, err) + } + } + if err := sys.SetServiceState(ctx, run, r.Unit, "stopped"); err != nil { + return out, fmt.Errorf("restarting %s: stopping it: %w", r.Unit, err) + } + if err := sys.SetServiceState(ctx, run, r.Unit, "running"); err != nil { + return out, fmt.Errorf("restarting %s: starting it again: %w", r.Unit, err) + } + } else { + reloader, ok := sys.(serviceReloader) + if !ok { + return out, fmt.Errorf("%s must be reloaded for %s and this machine's service manager "+ + "cannot reload a unit", r.Unit, strings.Join(reload, ", ")) + } + if err := reloader.ReloadService(ctx, run, r.Unit); err != nil { + return out, fmt.Errorf("reloading %s: %w", r.Unit, err) + } + } + // Read back: it was running, and a restart or reload that left it otherwise is a failure — + // the machine's network manager down is not a change to report and move past. + after, err := sys.ServiceState(ctx, run, r.Unit) + if err != nil { + return out, err + } + out.Action = "updated" + if len(restart) > 0 { + out.Detail = "restarted for " + strings.Join(restart, ", ") + } else { + out.Detail = "reloaded for " + strings.Join(reload, ", ") + } + if after != "running" { + return out, fmt.Errorf("%s was %s to pick up a change and is %s", r.Unit, + strings.Fields(out.Detail)[0], after) + } + return out, nil +} + // remove undoes one resource the host applied and the declaration no longer names, and reports // what it actually did. // @@ -1086,6 +1160,12 @@ func remove(ctx context.Context, sys system.System, a store.Applied, run Runner) return "removed", "no longer declared", nil case declaration.TypeService: + // A unit whose lifecycle was the machine's is left exactly as it is (novox/hq ADR 0117): + // stopping it here is how unassigning an uplink module would take down the machine's + // network manager, and with it the channel the mesh reaches the machine on. + if a.Stateless { + return "forgotten", "its state was never the mesh's", nil + } // A unit that is no longer declared is stopped, not deleted. The host did not install // it and does not own the unit file — only the state it put the unit into. // diff --git a/internal/apply/hold.go b/internal/apply/hold.go index 1bd5cca..4525ced 100644 --- a/internal/apply/hold.go +++ b/internal/apply/hold.go @@ -108,7 +108,7 @@ func lookBefore(ctx context.Context, sys system.System, d *declaration.Declarati } } case *declaration.Service: - if known.Recorded(string(declaration.TypeService), res.Unit) { + if res.Stateless() || known.Recorded(string(declaration.TypeService), res.Unit) { continue } // **Found is a unit somebody put on this machine, or one the machine uses.** @@ -261,6 +261,19 @@ func heldContainer(known store.State, name string) (store.Held, bool) { return store.Held{}, false } +// replacesNothing is a resource that takes nothing found on the machine from it, so on an adopted +// node it is never held and never previewed as replacing what was found: a file written into +// (novox/hq ADR 0102), and a service whose unit's lifecycle is the machine's (novox/hq ADR 0117). +func replacesNothing(r declaration.Resource) bool { + switch res := r.(type) { + case *declaration.File: + return res.Into != "" + case *declaration.Service: + return res.Stateless() + } + return false +} + // holdOnAdopted decides whether a resource of an adopted node is held rather than applied, and // holds it (novox/hq ADR 0100, ADR 0103). For a module not yet taken, what is present with no // record is kept as it is: a file or a container under its name, a directory, a service's unit, @@ -297,8 +310,9 @@ func holdOnAdopted(ctx context.Context, sys system.System, r declaration.Resourc // A file written into replaces nothing that was found, so it is never held (novox/hq ADR // 0102) — and a hold from when it was declared whole must not keep the mesh's keys out. That // hold is released by the apply once the write has worked, not here: a write that fails keeps - // it, and with it where the original was kept. - if f, ok := r.(*declaration.File); ok && f.Into != "" { + // it, and with it where the original was kept. A service whose lifecycle is the machine's + // replaces nothing either (novox/hq ADR 0117). + if replacesNothing(r) { return false, false, out, nil } diff --git a/internal/apply/plan.go b/internal/apply/plan.go index 07205bd..c79212b 100644 --- a/internal/apply/plan.go +++ b/internal/apply/plan.go @@ -80,6 +80,9 @@ func Plan(d *declaration.Declaration, known store.State, origin string) []Step { for _, orphan := range known.Orphans(declared, origin) { step := Step{Verb: "remove", Type: orphan.Type, ID: orphan.ID, Target: orphan.Target, Why: "recorded here and no longer declared"} + if orphan.Stateless { + step.Verb, step.Why = "forget", "no longer declared; its unit's state was never the mesh's and is left as it is" + } if d.Adoption == nil && strings.HasPrefix(orphan.ID, declaration.AdoptionPrefix) { step.Why = "what protected this node while adopted; removed last, once everything else applied" protecting = append(protecting, step) @@ -139,11 +142,9 @@ func planned(r declaration.Resource, d *declaration.Declaration, known store.Sta } } } - // A file written into replaces nothing that was found, so it is never held (ADR 0102). - into := false - if f, ok := r.(*declaration.File); ok && f.Into != "" { - into = true - } + // A file written into, or a service whose lifecycle is the machine's, replaces nothing that + // was found, so it is never held (ADR 0102, ADR 0117). + into := replacesNothing(r) h, held := known.HeldAt(r.Identity()) module, untaken := d.Adoption.UntakenModuleOf(r.Identity()) switch { @@ -182,6 +183,12 @@ func planned(r declaration.Resource, d *declaration.Declaration, known store.Sta return step } + if svc, ok := r.(*declaration.Service); ok && svc.Stateless() { + // Nothing is created: the unit and whether it runs are the machine's (novox/hq ADR 0117). + step.Verb, step.Why = "check", "its lifecycle is the machine's; reloaded or restarted only if "+ + "running when what it reflects changes" + return step + } was, recorded := known.Find(r.Identity()) if !recorded { step.Verb, step.Why = "create", "no record of it on this node" diff --git a/internal/apply/stateless_test.go b/internal/apply/stateless_test.go new file mode 100644 index 0000000..7ebdaa1 --- /dev/null +++ b/internal/apply/stateless_test.go @@ -0,0 +1,191 @@ +package apply + +import ( + "context" + "fmt" + "path/filepath" + "strings" + "testing" + + "github.com/novox/mesh-host/internal/store" +) + +// Defends novox/hq ADR 0117: a service that omits its state leaves the unit's lifecycle to the +// machine. The mesh reflects its triggers on a unit already running and does nothing else to it — +// never starts, stops, enables or disables it, and forgets it when undeclared. + +// unitIn is a service manager whose one unit is active or not, recording what it is asked. +func unitIn(active bool, commands *[]string) Runner { + return func(_ context.Context, name string, args ...string) (string, error) { + line := name + " " + strings.Join(args, " ") + *commands = append(*commands, line) + switch { + case strings.Contains(line, "is-enabled"): + return "enabled", nil + case strings.Contains(line, "show") && strings.Contains(line, "ActiveState"): + if active { + return "LoadState=loaded\nActiveState=active\nSubState=running", nil + } + return "LoadState=loaded\nActiveState=inactive\nSubState=dead", nil + } + return "", nil + } +} + +func statelessDecl(path, content, triggers string) string { + return fmt.Sprintf(`{"declaration":1,"resources":[ + {"id":"uplink.conf","type":"file","path":%q,"into":"block","content":%q}, + {"id":"uplink.manager","type":"service","unit":"NetworkManager.service",%s} + ]}`, path, content, triggers) +} + +// touched is whether any command would change the unit's lifecycle. +func touched(commands []string) []string { + var changing []string + for _, c := range commands { + for _, verb := range []string{" start ", " stop ", " restart ", " enable ", " disable ", " reload "} { + if strings.Contains(c+" ", verb) { + changing = append(changing, c) + } + } + } + return changing +} + +func TestAStatelessServiceRunningIsReloadedForItsTrigger(t *testing.T) { + path := filepath.Join(t.TempDir(), "mesh.conf") + var commands []string + report, _, err := Apply(context.Background(), archHost(t), + parse(t, statelessDecl(path, "[main]\ndns=none\n", `"reload-on":["uplink.conf"]`)), + store.State{}, store.OriginDeclared, unitIn(true, &commands), nil, nil) + if err != nil { + t.Fatal(err) + } + joined := strings.Join(commands, "\n") + if !strings.Contains(joined, "systemctl reload NetworkManager.service") { + t.Errorf("the running manager was not reloaded; commands were %v", commands) + } + for _, c := range touched(commands) { + if !strings.Contains(c, "reload") { + t.Errorf("the manager's lifecycle was touched: %s", c) + } + } + if o := outcomeOf(report, "uplink.manager"); o.Action != "updated" || !strings.Contains(o.Detail, "reloaded for uplink.conf") { + t.Errorf("reported as %q: %s", o.Action, o.Detail) + } +} + +func TestAStatelessServiceNotRunningIsLeftSo(t *testing.T) { + path := filepath.Join(t.TempDir(), "mesh.conf") + for _, triggers := range []string{`"reload-on":["uplink.conf"]`, `"restart-on":["uplink.conf"]`} { + var commands []string + report, _, err := Apply(context.Background(), archHost(t), + parse(t, statelessDecl(path, fmt.Sprintf("# %s\n", triggers), triggers)), + store.State{}, store.OriginDeclared, unitIn(false, &commands), nil, nil) + if err != nil { + t.Fatal(err) + } + if changing := touched(commands); len(changing) > 0 { + t.Errorf("%s: an inactive unit was acted on: %v", triggers, changing) + } + if o := outcomeOf(report, "uplink.manager"); o.Action != "unchanged" || + o.Detail != "not running; the change applies at its next start" { + t.Errorf("%s: reported as %q: %s", triggers, o.Action, o.Detail) + } + } +} + +func TestAStatelessServiceIsRestartedOnlyForItsRestartTrigger(t *testing.T) { + path := filepath.Join(t.TempDir(), "mesh.conf") + var commands []string + d := parse(t, statelessDecl(path, "x\n", `"restart-on":["uplink.conf"]`)) + _, state, err := Apply(context.Background(), archHost(t), d, store.State{}, store.OriginDeclared, + unitIn(true, &commands), nil, nil) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(strings.Join(commands, "\n"), "stop NetworkManager.service") { + t.Errorf("not restarted for its restart trigger; commands were %v", commands) + } + // Nothing it reflects changed: nothing is asked of the unit at all. + commands = nil + report, _, err := Apply(context.Background(), archHost(t), d, state, store.OriginDeclared, + unitIn(true, &commands), nil, nil) + if err != nil { + t.Fatal(err) + } + if changing := touched(commands); len(changing) > 0 { + t.Errorf("with nothing changed the unit was acted on: %v", changing) + } + if got := outcomeOf(report, "uplink.manager").Action; got != "unchanged" { + t.Errorf("with nothing changed it was %q", got) + } +} + +func TestAStatelessServiceUndeclaredIsForgottenNotStopped(t *testing.T) { + path := filepath.Join(t.TempDir(), "mesh.conf") + var commands []string + _, state, err := Apply(context.Background(), archHost(t), + parse(t, statelessDecl(path, "x\n", `"reload-on":["uplink.conf"]`)), + store.State{}, store.OriginDeclared, unitIn(true, &commands), nil, nil) + if err != nil { + t.Fatal(err) + } + if rec, _ := state.Find("uplink.manager"); !rec.Stateless { + t.Fatal("the record does not say the service was stateless") + } + if steps := Plan(somethingElse(t), state, store.OriginDeclared); !hasStep(steps, "forget", "uplink.manager") { + t.Errorf("the preview does not forget it: %v", steps) + } + commands = nil + report, _, err := Apply(context.Background(), archHost(t), somethingElse(t), state, store.OriginDeclared, + unitIn(true, &commands), nil, nil) + if err != nil { + t.Fatal(err) + } + if changing := touched(commands); len(changing) > 0 { + t.Errorf("undeclaring acted on the unit: %v", changing) + } + o := outcomeOf(report, "uplink.manager") + if o.Action != "forgotten" || o.Detail != "its state was never the mesh's" { + t.Errorf("undeclaring was %q: %s", o.Action, o.Detail) + } +} + +func TestAStatelessServiceIsNeverHeldOnAnAdoptedNode(t *testing.T) { + path := filepath.Join(t.TempDir(), "mesh.conf") + resources := fmt.Sprintf(`{"id":"uplink.conf","type":"file","path":%q,"into":"block","content":"x\n"}, + {"id":"uplink.manager","type":"service","unit":"NetworkManager.service","reload-on":["uplink.conf"]}`, path) + d := adopted(t, `{"taken":[],"untaken":{"uplink":["uplink.conf","uplink.manager"]}}`, resources) + for _, s := range Plan(d, store.State{}, store.OriginDeclared) { + if s.ID == "uplink.manager" && (s.Verb == "hold" || s.Verb == "create" || strings.Contains(s.Why, "replaces")) { + t.Errorf("the preview holds or replaces a stateless service: %+v", s) + } + } + var commands []string + report, state, err := ApplyKeeping(context.Background(), archHost(t), d, store.State{}, + store.OriginDeclared, unitIn(true, &commands), nil, nil, KeepIn(t.TempDir())) + if err != nil { + t.Fatal(err) + } + if got := outcomeOf(report, "uplink.manager").Action; got == "held" { + t.Fatal("a stateless service was held, though it replaces nothing that was found") + } + if len(state.Held) != 0 { + t.Errorf("something was held: %+v", state.Held) + } + for _, c := range touched(commands) { + if !strings.Contains(c, "reload") { + t.Errorf("the adopted node's manager lifecycle was touched: %s", c) + } + } +} + +func hasStep(steps []Step, verb, id string) bool { + for _, s := range steps { + if s.Verb == verb && s.ID == id { + return true + } + } + return false +} diff --git a/internal/declaration/declaration.go b/internal/declaration/declaration.go index 0b5d078..5ba9091 100644 --- a/internal/declaration/declaration.go +++ b/internal/declaration/declaration.go @@ -645,10 +645,20 @@ func (d *Process) validate(where string, _ bool) []string { // (it will come back at boot), or disabled and running (started by hand, gone after a reboot). // Folding them into one field would make the second expressible only by accident. type Service struct { - ID string `json:"id"` - Type Type `json:"type"` - Unit string `json:"unit"` - State string `json:"state"` + ID string `json:"id"` + Type Type `json:"type"` + Unit string `json:"unit"` + // State is "running" or "stopped" — or absent, and then **the unit's lifecycle is the + // machine's; the mesh only reflects its triggers** (novox/hq ADR 0117). The uplink modules + // declare the machine's own network manager this way: the mesh writes into its configuration + // and needs it to read that again, and nothing more. Stated, the host would start the manager + // on a machine that uses another one — two managers fighting over the same links — and, when + // the module was unassigned, stop it: the machine's network, the channel the mesh itself + // arrives on, gone at the moment of a routine change. So a service without a state is never + // started, stopped, enabled or disabled, is reloaded or restarted only when a trigger changed + // and it is already running, and undeclared is simply forgotten. It says nothing unless it + // names a trigger, and it may not say boot or takes-over, which are both lifecycle. + State string `json:"state,omitempty"` // Boot is "enabled" or "disabled" — whether the unit starts at boot. Optional: absent means // the host asserts nothing about it and leaves whatever is there. // @@ -692,6 +702,10 @@ type TakeOver struct { Config string `json:"config"` } +// Stateless reports whether the unit's lifecycle is the machine's, and the mesh only reflects the +// service's triggers (novox/hq ADR 0117). +func (s *Service) Stateless() bool { return s.State == "" } + func (s *Service) Identity() string { return s.ID } func (s *Service) Kind() Type { return TypeService } func (s *Service) Target() string { return s.Unit } @@ -701,9 +715,18 @@ func (s *Service) validate(where string, _ bool) []string { if s.Unit == "" { problems = append(problems, where+": a service needs a unit") } - if s.State != "running" && s.State != "stopped" { + switch { + case s.State == "running" || s.State == "stopped": + case s.State != "": problems = append(problems, fmt.Sprintf( - "%s: state %q; a service is \"running\" or \"stopped\"", where, s.State)) + "%s: state %q; a service is \"running\" or \"stopped\", or omits state to leave the "+ + "unit's lifecycle to the machine", where, s.State)) + case s.Boot != "" || s.TakesOver != nil: + problems = append(problems, where+": a service that omits state leaves the unit's lifecycle "+ + "to the machine, and boot and takes-over are both its lifecycle") + case len(s.RestartOn) == 0 && len(s.ReloadOn) == 0: + problems = append(problems, where+": a service that omits state leaves the unit's lifecycle "+ + "to the machine, and names no restart-on or reload-on — it declares nothing") } if s.Boot != "" && s.Boot != "enabled" && s.Boot != "disabled" { problems = append(problems, fmt.Sprintf( @@ -718,7 +741,7 @@ func (s *Service) validate(where string, _ bool) []string { case t.Unit == s.Unit: problems = append(problems, fmt.Sprintf("%s: takes-over names %s, which is this service's own unit", where, t.Unit)) - case s.State != "running": + case s.State != "running" && s.State != "": problems = append(problems, where+": a service that takes over a tunnel is running — stopping "+ "the found one for a service that will not run would leave the peers with nothing") } diff --git a/internal/declaration/stateless_test.go b/internal/declaration/stateless_test.go new file mode 100644 index 0000000..3338908 --- /dev/null +++ b/internal/declaration/stateless_test.go @@ -0,0 +1,30 @@ +package declaration + +import ( + "strings" + "testing" +) + +// Defends novox/hq ADR 0117: a service may leave its unit's lifecycle to the machine, and then +// says nothing but its triggers. +func TestAServiceWithoutAStateSaysOnlyItsTriggers(t *testing.T) { + for name, c := range map[string]struct{ resource, refusal string }{ + "no trigger": {`{"id":"s","type":"service","unit":"NetworkManager.service"}`, "declares nothing"}, + "with boot": {`{"id":"s","type":"service","unit":"NetworkManager.service","boot":"enabled","reload-on":["f"]}`, "boot and takes-over"}, + "with takes-over": {`{"id":"s","type":"service","unit":"a.service","reload-on":["f"],"takes-over":{"interface":"wg0","unit":"b.service","config":"/etc/x"}}`, "boot and takes-over"}, + "an unknown state": {`{"id":"s","type":"service","unit":"a.service","state":"paused"}`, "omits state to leave the unit's lifecycle to the machine"}, + } { + _, err := Parse([]byte(`{"declaration":1,"resources":[{"id":"f","type":"file","path":"/etc/x","content":"x"},` + c.resource + `]}`)) + if err == nil || !strings.Contains(err.Error(), c.refusal) { + t.Errorf("%s: want a refusal naming %q, got %v", name, c.refusal, err) + } + } + d, err := Parse([]byte(`{"declaration":1,"resources":[{"id":"f","type":"file","path":"/etc/x","content":"x"}, + {"id":"s","type":"service","unit":"NetworkManager.service","restart-on":["f"]}]}`)) + if err != nil { + t.Fatal(err) + } + if s := d.Resources[1].(*Service); !s.Stateless() { + t.Error("a service without a state was not read as stateless") + } +} diff --git a/internal/store/store.go b/internal/store/store.go index 9465b27..fc1d26b 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -77,6 +77,11 @@ type Applied struct { // said once in a log line is not a path the host can find again. Kept string `json:"kept,omitempty"` + // Stateless is, for a service, that its unit's lifecycle was never the mesh's (novox/hq ADR + // 0117) — kept here because removal happens once the declaration that said so is gone, and a + // service removed as if it had a state is stopped: the machine's network manager, for one. + Stateless bool `json:"stateless,omitempty"` + // Into is set for a file written into rather than over (novox/hq ADR 0102): the format, what // each of the mesh's keys held before it set them, which of them were absent, and whether the // file itself was — so undeclaring it gives the machine back exactly what it had. From 3112c881e41f566d267015c8ff79f09023cf30af Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 27 Sep 2026 00:21:25 +0200 Subject: [PATCH 07/11] Undeclaring gives a unit back the state it was found in, and removes a process the mesh made (hq ADR 0118, issue 130) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A service undeclared used to be stopped: unassigning the private network stopped the container runtime, unassigning sshd would stop ssh, an uplink module would take the machine offline. The host now records the unit's state when it first applies it and restores that on undeclare — found running stays running; started by the mesh (the converge filter) is stopped again; nothing is started on the way out; a pre-existing record leaves the unit alone. An undeclared process had no removal at all and failed every apply on its node; its unit, timer and bundle are now removed. --- internal/apply/apply.go | 111 ++++++++++++++++++++++++-------- internal/apply/apply_test.go | 114 +++++++++++++++++++++++++++------ internal/apply/opening_test.go | 4 +- internal/apply/plan.go | 11 +++- internal/apply/process.go | 52 ++++++++++++++- internal/apply/process_test.go | 61 ++++++++++++++++++ internal/store/store.go | 16 +++++ 7 files changed, 322 insertions(+), 47 deletions(-) diff --git a/internal/apply/apply.go b/internal/apply/apply.go index fff75e1..e5e23f0 100644 --- a/internal/apply/apply.go +++ b/internal/apply/apply.go @@ -56,6 +56,8 @@ type Outcome struct { kept string // stateless is a service whose unit's lifecycle is the machine's (novox/hq ADR 0117). stateless bool + // found is, for a service, its unit as the host first found it (novox/hq ADR 0118). + found *store.FoundUnit // reads is, for a container, the digest of each file it was created reading, by path — so // the next apply can say which one changed (novox/hq 04-ISSUES/103). reads map[string]string @@ -465,6 +467,7 @@ func ApplyKeeping( Kept: kept, Reads: outcome.reads, Stateless: outcome.stateless, + Found: outcome.found, Holds: holds(resource), }) // Its module has been taken, and what was held for it is now the mesh's. A file written @@ -545,7 +548,7 @@ func applyOne(ctx context.Context, sys system.System, r declaration.Resource, ru case *declaration.File: return applyFile(res, previous, unseal, keepFound) case *declaration.Service: - return applyService(ctx, sys, res, run, changed) + return applyService(ctx, sys, res, run, changed, previous) case *declaration.Package: return applyPackage(ctx, sys, res, run) case *declaration.Container: @@ -922,11 +925,34 @@ type unitReloader interface { } func applyService(ctx context.Context, sys system.System, r *declaration.Service, run Runner, - changed map[string]bool) (Outcome, error) { + changed map[string]bool, previous store.Applied) (Outcome, error) { if r.Stateless() { return reflectOnly(ctx, sys, r, run, changed) } out := begin(r) + + // **What the unit was before the mesh touched it**, read once — the first time this host + // applies it — and carried in the record from then on (novox/hq ADR 0118). Undeclared, the + // unit is given back to exactly this: it is the one fact that separates the container runtime, + // running before the mesh arrived and to be left running, from the mesh's packet filter, + // stopped until the mesh started it and to be stopped again. A record that predates this + // field is not read now: the unit's state by then is the mesh's doing, not what was found. + switch { + case previous.Found != nil: + out.found = previous.Found + case previous.ID == "": + state, err := sys.ServiceState(ctx, run, r.Unit) + if err != nil { + return out, err + } + found := &store.FoundUnit{State: state} + if r.Boot != "" { + if found.Boot, err = sys.ServiceBoot(ctx, run, r.Unit); err != nil { + return out, err + } + } + out.found = found + } var changes []string // A file the service reflects changed, and it may be the unit's own file or a drop-in: the @@ -1160,30 +1186,12 @@ func remove(ctx context.Context, sys system.System, a store.Applied, run Runner) return "removed", "no longer declared", nil case declaration.TypeService: - // A unit whose lifecycle was the machine's is left exactly as it is (novox/hq ADR 0117): - // stopping it here is how unassigning an uplink module would take down the machine's - // network manager, and with it the channel the mesh reaches the machine on. - if a.Stateless { - return "forgotten", "its state was never the mesh's", nil - } - // A unit that is no longer declared is stopped, not deleted. The host did not install - // it and does not own the unit file — only the state it put the unit into. - // - // A unit that no longer EXISTS is already in the state removal is trying to reach, and - // saying so matters: stopping it fails, and a failure here fails the whole apply. A - // host holding a record of an uninstalled unit would then be unable to apply anything, - // ever, with no way out but editing its state by hand. Removal is idempotent for the - // same reason `os.RemoveAll` is. - if _, err := sys.ServiceState(ctx, run, a.Target); err != nil { - if strings.Contains(err.Error(), "does not exist on this machine") { - return "forgotten", "the unit no longer exists", nil - } - return "", "", err - } - if err := sys.SetServiceState(ctx, run, a.Target, "stopped"); err != nil { - return "", "", fmt.Errorf("stopping %s: %w", a.Target, err) - } - return "removed", "stopped; the unit file is not the host's to delete", nil + return removeService(ctx, sys, a, run) + + case declaration.TypeProcess: + // The other side of the same line: a process's unit is the host's own — it wrote the unit + // file and unpacked the bundle — so it goes with its declaration (novox/hq ADR 0118). + return removeProcess(ctx, a, run) case declaration.TypeContainer: // The host CREATED this one, so the host removes it. That is the line: it removes what @@ -2019,3 +2027,54 @@ func declaredDigest(r declaration.Resource) string { } return fmt.Sprintf("%x", sha256.Sum256([]byte(material))) } + +// removeService gives a unit back the state the host first found it in, and nothing more. +// +// **Removes what it made, gives back what it changed, leaves what was the machine's** +// (novox/hq ADR 0118). A service resource never installs a unit; it puts one that already existed +// into a state. So undeclaring it cannot mean stopping it — that is how unassigning the private +// network stopped the container runtime and every container with it, how unassigning sshd would +// have stopped ssh, and how an uplink module would have taken a machine off its only link +// (novox/hq issue 130). It means undoing what the mesh did to it: a unit found running is left +// running; a unit the mesh started — the packet filter a converge loaded, which returning to +// adopted must unload — is stopped again, and disabled again if the mesh enabled it. +// +// **Never started on the way out.** A unit the mesh stopped is not started again when its +// declaration goes: starting something is a decision, and the operator makes it. +func removeService(ctx context.Context, sys system.System, a store.Applied, run Runner) (string, string, error) { + if a.Stateless { + return "forgotten", "its state was never the mesh's", nil + } + if a.Found == nil { + // Recorded before the host kept what it found. Not knowing, it leaves the unit as it is: + // a unit left running can be stopped by the operator, and one stopped by mistake may be + // the link the operator would use to do it. + return "forgotten", "the unit is the machine's; left as it is", nil + } + if a.Found.State != "stopped" && (a.Found.Boot == "" || a.Found.Boot == "enabled") { + return "forgotten", "it was running before the mesh; left as it is", nil + } + // Something is to be given back, so the unit must still be there to give it to. One since + // uninstalled is already as far from the mesh as it can be, and saying so keeps a record of it + // from failing every apply. + if _, err := sys.ServiceState(ctx, run, a.Target); err != nil { + if strings.Contains(err.Error(), "does not exist on this machine") { + return "forgotten", "the unit no longer exists", nil + } + return "", "", err + } + var gave []string + if a.Found.State == "stopped" { + if err := sys.SetServiceState(ctx, run, a.Target, "stopped"); err != nil { + return "", "", fmt.Errorf("stopping %s, which the mesh started: %w", a.Target, err) + } + gave = append(gave, "stopped again") + } + if a.Found.Boot == "disabled" { + if err := sys.SetServiceBoot(ctx, run, a.Target, "disabled"); err != nil { + return "", "", fmt.Errorf("disabling %s, which the mesh enabled: %w", a.Target, err) + } + gave = append(gave, "disabled at boot again") + } + return "restored", strings.Join(gave, ", ") + ", as the host found it", nil +} diff --git a/internal/apply/apply_test.go b/internal/apply/apply_test.go index 31c04e5..fdde6c0 100644 --- a/internal/apply/apply_test.go +++ b/internal/apply/apply_test.go @@ -348,33 +348,111 @@ func TestAnUnknownServiceStateIsRefusedNotGuessed(t *testing.T) { } } -func TestADroppedServiceIsStoppedNotDeleted(t *testing.T) { - // The host did not install the unit and does not own the unit file — only the state it put - // the unit into. - var commands []string +func TestADroppedServiceIsGivenBackTheStateItWasFoundIn(t *testing.T) { + // novox/hq ADR 0118: undeclaring removes what the mesh made, gives back what it changed, and + // leaves what was the machine's. A service resource never installs a unit — so undeclaring it + // undoes what the mesh did to the unit, and nothing more. Stopping every undeclared unit is + // how unassigning the private network stopped the container runtime (novox/hq issue 130). + cases := []struct { + name string + found *store.FoundUnit + action string + stop bool + disable bool + }{ + {"recorded before the host kept what it found", nil, "forgotten", false, false}, + {"running before the mesh", &store.FoundUnit{State: "running"}, "forgotten", false, false}, + {"running and enabled before the mesh", &store.FoundUnit{State: "running", Boot: "enabled"}, "forgotten", false, false}, + {"started by the mesh", &store.FoundUnit{State: "stopped"}, "restored", true, false}, + {"started and enabled by the mesh", &store.FoundUnit{State: "stopped", Boot: "disabled"}, "restored", true, true}, + {"enabled by the mesh, running before it", &store.FoundUnit{State: "running", Boot: "disabled"}, "restored", false, true}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + var commands []string + run := func(ctx context.Context, name string, args ...string) (string, error) { + commands = append(commands, strings.Join(args, " ")) + if args[0] == "show" { + return "LoadState=loaded\nActiveState=active\n", nil + } + return "", nil + } + state := store.State{Resources: []store.Applied{ + {ID: "s", Type: "service", Target: "unit.service", Found: c.found}, + }} + d := parse(t, `{"declaration":1,"resources":[ + {"id":"other","type":"file","path":"`+filepath.Join(t.TempDir(), "a")+`","content":"a\n"} + ]}`) + report, after, err := Apply(context.Background(), archHost(t), d, state, store.OriginCarried, run, nil, nil) + if err != nil { + t.Fatal(err) + } + joined := strings.Join(commands, "; ") + if stopped := strings.Contains(joined, "stop unit.service"); stopped != c.stop { + t.Errorf("stopped %v, want %v: %s", stopped, c.stop, joined) + } + if disabled := strings.Contains(joined, "disable unit.service"); disabled != c.disable { + t.Errorf("disabled %v, want %v: %s", disabled, c.disable, joined) + } + if strings.Contains(joined, "start unit.service") || strings.Contains(joined, "mask") { + t.Errorf("the host started or masked a unit on its way out: %s", joined) + } + if o := outcomeOf(report, "s"); o.Action != c.action { + t.Errorf("outcome %+v, want %s", o, c.action) + } + if _, still := after.Find("s"); still { + t.Error("the host still believes it owns the undeclared service") + } + }) + } +} + +func TestAServiceRecordsTheStateItWasFoundInOnceAndCarriesIt(t *testing.T) { + // Read the first time the host applies the unit — before it starts or enables anything — + // and never again: by the next apply, the unit's state is the mesh's doing. + active := "inactive" + enabled := "disabled" run := func(ctx context.Context, name string, args ...string) (string, error) { - commands = append(commands, strings.Join(args, " ")) - if args[0] == "show" { - return "LoadState=loaded\nActiveState=active\n", nil + switch args[0] { + case "show": + return "LoadState=loaded\nActiveState=" + active + "\n", nil + case "is-enabled": + return enabled, nil + case "start": + active = "active" + case "enable": + enabled = "enabled" } return "", nil } - state := store.State{Resources: []store.Applied{ - {ID: "s", Type: "service", Target: "gone.service"}, - }} d := parse(t, `{"declaration":1,"resources":[ - {"id":"other","type":"file","path":"`+filepath.Join(t.TempDir(), "a")+`","content":"a\n"} + {"id":"s","type":"service","unit":"filter.service","state":"running","boot":"enabled"} ]}`) - - if _, _, err := Apply(context.Background(), archHost(t), d, state, store.OriginCarried, run, nil, nil); err != nil { + _, first, err := Apply(context.Background(), archHost(t), d, store.State{}, store.OriginCarried, run, nil, nil) + if err != nil { t.Fatal(err) } - joined := strings.Join(commands, "; ") - if !strings.Contains(joined, "stop gone.service") { - t.Errorf("the dropped service was not stopped: %s", joined) + rec, _ := first.Find("s") + if rec.Found == nil || rec.Found.State != "stopped" || rec.Found.Boot != "disabled" { + t.Fatalf("first apply recorded %+v, want stopped and disabled", rec.Found) } - if strings.Contains(joined, "disable") || strings.Contains(joined, "mask") { - t.Errorf("the host did more than stop a unit it does not own: %s", joined) + _, second, err := Apply(context.Background(), archHost(t), d, first, store.OriginCarried, run, nil, nil) + if err != nil { + t.Fatal(err) + } + rec, _ = second.Find("s") + if rec.Found == nil || rec.Found.State != "stopped" { + t.Errorf("a later apply replaced what was found with what the mesh made: %+v", rec.Found) + } + + // A record from before the host kept what it found is not given one later. + old := store.State{Resources: []store.Applied{{ID: "s", Type: "service", Target: "filter.service"}}} + _, third, err := Apply(context.Background(), archHost(t), d, old, store.OriginCarried, run, nil, nil) + if err != nil { + t.Fatal(err) + } + if rec, _ = third.Find("s"); rec.Found != nil { + t.Errorf("a record that predates the field was given one after the mesh had acted: %+v", rec.Found) } } diff --git a/internal/apply/opening_test.go b/internal/apply/opening_test.go index 5f976ee..1be33f5 100644 --- a/internal/apply/opening_test.go +++ b/internal/apply/opening_test.go @@ -359,7 +359,9 @@ func TestReturningToAdoptedLoadsTheGuardBeforeRemovingTheFilter(t *testing.T) { return "", nil } converged := store.State{Resources: []store.Applied{ - {ID: "nftables.load", Type: "service", Target: "mesh-filter.service", Origin: store.OriginDeclared}}} + {ID: "nftables.load", Type: "service", Target: "mesh-filter.service", Origin: store.OriginDeclared, + // The mesh loaded this filter at converge: it was not running before (novox/hq ADR 0118). + Found: &store.FoundUnit{State: "stopped"}}}} _, state, err := applyWith(t, adopted(t, `{"taken":[]}`, withConf(dir)+","+guardFile), converged, run) if !guardUpAtStop { t.Errorf("stop fails %v: the derived filter was stopped before the guard was written", stopFails) diff --git a/internal/apply/plan.go b/internal/apply/plan.go index c79212b..a142d6e 100644 --- a/internal/apply/plan.go +++ b/internal/apply/plan.go @@ -80,8 +80,17 @@ func Plan(d *declaration.Declaration, known store.State, origin string) []Step { for _, orphan := range known.Orphans(declared, origin) { step := Step{Verb: "remove", Type: orphan.Type, ID: orphan.ID, Target: orphan.Target, Why: "recorded here and no longer declared"} - if orphan.Stateless { + switch { + case orphan.Stateless: step.Verb, step.Why = "forget", "no longer declared; its unit's state was never the mesh's and is left as it is" + case orphan.Type == string(declaration.TypeService): + // What removal will do, said before it does it (novox/hq ADR 0118): a unit is given + // back what the host found, so the preview names which units that stops. + if f := orphan.Found; f != nil && (f.State == "stopped" || f.Boot == "disabled") { + step.Verb, step.Why = "restore", "no longer declared; the mesh started or enabled it, and it goes back as it was found" + } else { + step.Verb, step.Why = "forget", "no longer declared; the unit is the machine's and is left as it is" + } } if d.Adoption == nil && strings.HasPrefix(orphan.ID, declaration.AdoptionPrefix) { step.Why = "what protected this node while adopted; removed last, once everything else applied" diff --git a/internal/apply/process.go b/internal/apply/process.go index d9956e7..e2f37a6 100644 --- a/internal/apply/process.go +++ b/internal/apply/process.go @@ -30,7 +30,7 @@ import ( // Under the mesh's own directory rather than somewhere a distribution owns: these are files the // mesh puts there and replaces, and putting them where a package manager also writes is how two // owners end up disagreeing about one path. -const daemonRoot = "/var/lib/mesh/daemons" +var daemonRoot = "/var/lib/mesh/daemons" // unitDir is where the mesh writes the units it owns. A variable only so a test can point it at a // directory of its own. @@ -290,3 +290,53 @@ func unitValue(key, value string) string { ).Replace(value) return `"` + key + "=" + escaped + `"` } + +// removeProcess takes away a process the mesh no longer declares: its timer and unit stopped and +// disabled, their files removed, the supervisor told, and the unpacked bundle deleted. +// +// **All of it is the host's**, which is why all of it goes (novox/hq ADR 0118). Before this there +// was no way to remove a process at all, and one left undeclared failed every apply on its node +// until someone edited the host's state by hand — unassigning any module that ran its own code +// stranded the machine. +// +// Idempotent, like every removal: a unit already gone is not an error, and a record whose files +// have all vanished is forgotten rather than reported as removed. +func removeProcess(ctx context.Context, a store.Applied, run Runner) (string, string, error) { + name := a.Target + if name == "" || strings.ContainsAny(name, "/ \t") { + // The name is a unit name and a directory under the mesh's own. One that could climb out + // of either is refused rather than acted on, whatever wrote it into the record. + return "", "", fmt.Errorf("a process recorded under %q cannot be removed by name", name) + } + found := false + for _, unit := range []string{name + ".timer", name + ".service"} { + path := filepath.Join(unitDir, unit) + if _, err := os.Stat(path); err != nil { + continue + } + found = true + // The timer first, so a scheduled run cannot start the service between the two. + if _, err := run(ctx, "systemctl", "disable", "--now", unit); err != nil { + return "", "", fmt.Errorf("stopping %s: %w", unit, err) + } + if err := os.Remove(path); err != nil && !os.IsNotExist(err) { + return "", "", err + } + } + if found { + if _, err := run(ctx, "systemctl", "daemon-reload"); err != nil { + return "", "", err + } + } + bundle := filepath.Join(daemonRoot, name) + if _, err := os.Stat(bundle); err == nil { + found = true + if err := os.RemoveAll(bundle); err != nil { + return "", "", err + } + } + if !found { + return "forgotten", "no longer there", nil + } + return "removed", "stopped; its unit and its bundle removed — the mesh's own code", nil +} diff --git a/internal/apply/process_test.go b/internal/apply/process_test.go index 3d38e22..93dca73 100644 --- a/internal/apply/process_test.go +++ b/internal/apply/process_test.go @@ -1,10 +1,14 @@ package apply import ( + "context" + "os" + "path/filepath" "strings" "testing" "github.com/novox/mesh-host/internal/declaration" + "github.com/novox/mesh-host/internal/store" ) func aProcess() *declaration.Process { @@ -163,3 +167,60 @@ func TestAnEnvironmentValueMeansWhatTheDeclarationSaid(t *testing.T) { t.Fatalf("a quote was not escaped, so the value ends early: %s", line) } } + +func TestAnUndeclaredProcessIsRemovedWithItsUnitAndBundle(t *testing.T) { + // novox/hq ADR 0118: a process's unit and bundle are the host's own, so they go with the + // declaration. Before, there was no way to remove a process at all, and one left undeclared + // failed every apply on its node. + units, bundles := t.TempDir(), t.TempDir() + wasUnits, wasBundles := unitDir, daemonRoot + unitDir, daemonRoot = units, bundles + t.Cleanup(func() { unitDir, daemonRoot = wasUnits, wasBundles }) + + for _, f := range []string{"mesh-job.service", "mesh-job.timer"} { + if err := os.WriteFile(filepath.Join(units, f), []byte("[Unit]\n"), 0o644); err != nil { + t.Fatal(err) + } + } + if err := os.MkdirAll(filepath.Join(bundles, "mesh-job", "bin"), 0o755); err != nil { + t.Fatal(err) + } + var commands []string + run := func(ctx context.Context, name string, args ...string) (string, error) { + commands = append(commands, strings.Join(args, " ")) + return "", nil + } + known := store.State{Resources: []store.Applied{{ID: "p", Type: "process", Target: "mesh-job"}}} + d := parse(t, `{"declaration":1,"resources":[ + {"id":"f","type":"file","path":"`+filepath.Join(t.TempDir(), "a")+`","content":"a\n"} + ]}`) + report, after, err := Apply(context.Background(), archHost(t), d, known, store.OriginCarried, run, nil, nil) + if err != nil { + t.Fatalf("an undeclared process failed the apply: %v", err) + } + joined := strings.Join(commands, "; ") + timer, service := strings.Index(joined, "disable --now mesh-job.timer"), strings.Index(joined, "disable --now mesh-job.service") + if timer < 0 || service < 0 || timer > service { + t.Errorf("the timer and the unit were not stopped, timer first: %s", joined) + } + if !strings.Contains(joined, "daemon-reload") { + t.Errorf("the service manager was not told its units changed: %s", joined) + } + for _, gone := range []string{filepath.Join(units, "mesh-job.service"), filepath.Join(units, "mesh-job.timer"), filepath.Join(bundles, "mesh-job")} { + if _, err := os.Stat(gone); !os.IsNotExist(err) { + t.Errorf("%s is still there", gone) + } + } + if o := outcomeOf(report, "p"); o.Action != "removed" { + t.Errorf("outcome %+v, want removed", o) + } + if _, still := after.Find("p"); still { + t.Error("the host still believes it owns the process") + } + + // Again, with everything already gone: forgotten, not an error. + _, _, err = Apply(context.Background(), archHost(t), d, known, store.OriginCarried, run, nil, nil) + if err != nil { + t.Errorf("removing a process that is already gone failed: %v", err) + } +} diff --git a/internal/store/store.go b/internal/store/store.go index fc1d26b..0878b62 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -82,6 +82,13 @@ type Applied struct { // service removed as if it had a state is stopped: the machine's network manager, for one. Stateless bool `json:"stateless,omitempty"` + // Found is, for a service, the state its unit was in when this host first applied it — before + // the mesh started, stopped, enabled or disabled anything. Removal gives that back and nothing + // more (novox/hq ADR 0118): a unit that was running before the mesh arrived keeps running + // when its declaration goes; one the mesh started is stopped again. Absent on a record written + // before the host kept it, and then the unit is left exactly as it is. + Found *FoundUnit `json:"found,omitempty"` + // Into is set for a file written into rather than over (novox/hq ADR 0102): the format, what // each of the mesh's keys held before it set them, which of them were absent, and whether the // file itself was — so undeclaring it gives the machine back exactly what it had. @@ -447,3 +454,12 @@ func originOf(r Applied) string { } return r.Origin } + +// FoundUnit is a service's unit as the host first found it. +type FoundUnit struct { + // State is "running" or "stopped". + State string `json:"state"` + // Boot is "enabled" or "disabled" — or empty when the declaration never set it, and the host + // never touched it. + Boot string `json:"boot,omitempty"` +} From 08c0f40ff0b51630ef0bf96ccee89bf86abed1a8 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 27 Sep 2026 00:41:02 +0200 Subject: [PATCH 08/11] review: refuse a process named ".", ".." or with a leading dash, so removing one cannot delete the mesh's own directory (hq ADR 0118) removeProcess deletes filepath.Join(daemonRoot, name) whole; a process named ".." made that /var/lib/mesh. The declaration and the removal now hold the name to one rule. --- internal/apply/process.go | 9 ++++---- internal/apply/process_test.go | 32 ++++++++++++++++++++++++++ internal/declaration/declaration.go | 34 +++++++++++++++++++++------- internal/declaration/process_test.go | 5 +++- 4 files changed, 67 insertions(+), 13 deletions(-) diff --git a/internal/apply/process.go b/internal/apply/process.go index e2f37a6..b3c5978 100644 --- a/internal/apply/process.go +++ b/internal/apply/process.go @@ -303,10 +303,11 @@ func unitValue(key, value string) string { // have all vanished is forgotten rather than reported as removed. func removeProcess(ctx context.Context, a store.Applied, run Runner) (string, string, error) { name := a.Target - if name == "" || strings.ContainsAny(name, "/ \t") { - // The name is a unit name and a directory under the mesh's own. One that could climb out - // of either is refused rather than acted on, whatever wrote it into the record. - return "", "", fmt.Errorf("a process recorded under %q cannot be removed by name", name) + if problem := declaration.ProcessNameProblem(name); problem != "" { + // The name is a unit name and a directory under the mesh's own, and what goes is that + // directory, whole. One that could climb out of either — ".." is the mesh's own directory's + // parent — is refused rather than acted on, whatever wrote it into the record. + return "", "", fmt.Errorf("a process recorded under %q cannot be removed by name: %s", name, problem) } found := false for _, unit := range []string{name + ".timer", name + ".service"} { diff --git a/internal/apply/process_test.go b/internal/apply/process_test.go index 93dca73..7f055f7 100644 --- a/internal/apply/process_test.go +++ b/internal/apply/process_test.go @@ -224,3 +224,35 @@ func TestAnUndeclaredProcessIsRemovedWithItsUnitAndBundle(t *testing.T) { t.Errorf("removing a process that is already gone failed: %v", err) } } + +func TestAProcessRecordedUnderAPathlikeNameIsRefusedNotRemoved(t *testing.T) { + // filepath.Join(daemonRoot, "..") is the mesh's own directory, and removal deletes what that + // names, whole. A record is what some host wrote, perhaps under looser rules than today's, so + // the removal holds the name to the declaration's rule again (novox/hq ADR 0118). + root := t.TempDir() + bundles := filepath.Join(root, "daemons") + wasUnits, wasBundles := unitDir, daemonRoot + unitDir, daemonRoot = t.TempDir(), bundles + t.Cleanup(func() { unitDir, daemonRoot = wasUnits, wasBundles }) + precious := filepath.Join(root, "state.json") + if err := os.MkdirAll(filepath.Join(bundles, "other"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(precious, []byte("{}"), 0o600); err != nil { + t.Fatal(err) + } + for _, name := range []string{"..", ".", "", "-x"} { + known := store.State{Resources: []store.Applied{{ID: "p", Type: "process", Target: name}}} + d := parse(t, `{"declaration":1,"resources":[ + {"id":"f","type":"file","path":"`+filepath.Join(t.TempDir(), "a")+`","content":"a\n"} + ]}`) + if _, _, err := Apply(context.Background(), archHost(t), d, known, store.OriginCarried, noServices, nil, nil); err == nil { + t.Errorf("a process recorded as %q was removed by name", name) + } + for _, still := range []string{precious, filepath.Join(bundles, "other")} { + if _, err := os.Stat(still); err != nil { + t.Fatalf("removing a process recorded as %q took %s with it", name, still) + } + } + } +} diff --git a/internal/declaration/declaration.go b/internal/declaration/declaration.go index 5ba9091..5cc738f 100644 --- a/internal/declaration/declaration.go +++ b/internal/declaration/declaration.go @@ -575,16 +575,34 @@ func (d *Process) Identity() string { return d.ID } func (d *Process) Kind() Type { return TypeProcess } func (d *Process) Target() string { return d.Name } +// ProcessNameProblem says what is wrong with a process name, or nothing. +// +// The name becomes a unit name, a file under the unit directory and a directory under the mesh's +// own — the one removing the process deletes, whole (novox/hq ADR 0118). So a name that is not one +// plain path element is refused: with a separator it writes somewhere nobody meant, and "." or +// ".." IS the mesh's directory or its parent — removing a process named ".." would delete every +// bundle the mesh has, and more. One with a leading dash is read by the service manager as an +// option, not a unit. Exported because the removal checks the recorded name again: a record is +// what the host wrote, and a host of an older version wrote it under looser rules. +func ProcessNameProblem(name string) string { + switch { + case name == "": + return "a process needs a name, which is what its unit is called" + case strings.ContainsAny(name, "/ \t"): + return "a process name becomes a unit name, so it cannot contain a path separator or a space" + case name == "." || name == "..": + return fmt.Sprintf("a process name becomes a directory under the mesh's own, and %q would be "+ + "that directory or its parent", name) + case strings.HasPrefix(name, "-"): + return "a process name cannot begin with a dash: the service manager would read it as an option" + } + return "" +} + func (d *Process) validate(where string, _ bool) []string { var problems []string - if d.Name == "" { - problems = append(problems, where+": a process needs a name, which is what its unit is called") - } - if strings.ContainsAny(d.Name, "/ \t") { - // It becomes a unit name and a file on disk. A name with a separator in it would write - // somewhere nobody meant. - problems = append(problems, where+": a process name becomes a unit name, so it cannot "+ - "contain a path separator or a space") + if problem := ProcessNameProblem(d.Name); problem != "" { + problems = append(problems, where+": "+problem) } if d.Source == "" { problems = append(problems, where+": a process needs somewhere to fetch its bundle from") diff --git a/internal/declaration/process_test.go b/internal/declaration/process_test.go index c52519f..9a7c7e9 100644 --- a/internal/declaration/process_test.go +++ b/internal/declaration/process_test.go @@ -55,7 +55,10 @@ func TestAProcessMustSayWhatToRun(t *testing.T) { // Its name becomes a unit name and a path, so a separator in it would write somewhere nobody meant. func TestAProcesssNameCannotEscapeItsUnit(t *testing.T) { - for _, bad := range []string{"", "../escape", "two words", "a/b"} { + // "." and ".." are one path element each, and the mesh's own bundle directory and its parent: + // removing a process named ".." would delete every bundle the mesh has, and more (novox/hq ADR + // 0118). A leading dash is an option to the service manager, not a unit. + for _, bad := range []string{"", "../escape", "two words", "a/b", ".", "..", "-", "--now"} { d := aProcess() d.Name = bad if problems := d.validate("a process", false); len(problems) == 0 { From 23a4436499581eaa9470458b521650434bd5069c Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 27 Sep 2026 00:41:02 +0200 Subject: [PATCH 09/11] review: a unit whose file the mesh wrote is stopped when undeclared, and what was found survives a failed first apply (hq ADR 0118) Records written before Found existed left the adoption guard and the converge filter loaded on undeclare, then deleted their unit files from under them; a unit whose own file the mesh created is now the mesh's, whatever its record says. Found is kept apart the moment it is read, so a first apply that enabled and then failed is not read back as the machine's; boot is found the first time the mesh sets it; a service once stateless, or moved to another unit, is found afresh (the old unit given back). The unit is read after the reload that loads a file written in the same apply, and removal reports what it actually did. --- internal/apply/apply.go | 247 +++++++++++++++++---- internal/apply/found_test.go | 387 +++++++++++++++++++++++++++++++++ internal/apply/opening_test.go | 16 +- internal/apply/plan.go | 35 ++- internal/apply/plan_test.go | 39 ++++ internal/store/store.go | 50 +++++ 6 files changed, 718 insertions(+), 56 deletions(-) create mode 100644 internal/apply/found_test.go diff --git a/internal/apply/apply.go b/internal/apply/apply.go index e5e23f0..6bdb549 100644 --- a/internal/apply/apply.go +++ b/internal/apply/apply.go @@ -170,6 +170,14 @@ func ApplyKeeping( // as the service that took it over is (novox/hq ADR 0105). declared[takeOverID(svc)] = true } + // What an unfinished apply found a unit as is kept only while its service is declared: one + // declared again later is read afresh, as any resource with no record is (novox/hq ADR 0118). + known.DropFoundUndeclared(declared, origin) + + // Which units the mesh made, read before any removal can take a unit file's record away — so + // whichever order a module declared a unit and its file in, the unit is known for the mesh's + // when its service goes (novox/hq ADR 0118). + made := meshMadeUnits(known) // Which firewall is found here, before anything else, since an unsupported one refuses the // whole declaration (novox/hq ADR 0100). Nothing for a converged node. @@ -188,7 +196,7 @@ func ApplyKeeping( if declaration.Type(orphan.Type) == declaration.TypeOpening { action, detail, err = removeOpening(ctx, orphan, run, known.Firewall) } else { - action, detail, err = remove(ctx, sys, orphan, run) + action, detail, err = remove(ctx, sys, orphan, run, made) } if err != nil { return &Error{Resource: orphan.ID, Err: err, Done: report} @@ -385,6 +393,14 @@ func ApplyKeeping( } was, _ := known.Find(resource.Identity()) + // What an earlier apply of this service found its unit as and could not yet record is + // newer than anything the record says — the record's own finding with what was read + // since — so it is what this apply goes on from (novox/hq ADR 0118). + previous := was + if p, ok := known.FoundFirst[resource.Identity()]; ok { + f := p.FoundUnit + previous.Found = &f + } var outcome Outcome var err error if o, isOpening := resource.(*declaration.Opening); isOpening { @@ -397,9 +413,15 @@ func ApplyKeeping( !known.Recorded(string(declaration.TypeFile), f.Path) { keepFound = keep } - outcome, err = applyOne(ctx, sys, resource, run, changed, in, was, unseal, keepFound) + outcome, err = applyOne(ctx, sys, resource, run, changed, in, previous, unseal, keepFound) } if err != nil { + // **What was found is kept whatever the apply then did** (novox/hq ADR 0118). The + // failure may have come after the mesh enabled or started the unit, and the next apply + // would otherwise read that as the machine's own. + if outcome.found != nil { + known.KeepFound(resource.Identity(), origin, *outcome.found) + } failed := &Error{Resource: resource.Identity(), Err: err, Done: report} failures = append(failures, failed) log(fmt.Sprintf(" failed %s (%s): %v", resource.Identity(), outcome.Target, err)) @@ -470,6 +492,9 @@ func ApplyKeeping( Found: outcome.found, Holds: holds(resource), }) + if outcome.found != nil { + known.DropFound(resource.Identity()) + } // Its module has been taken, and what was held for it is now the mesh's. A file written // into, or a service whose lifecycle is the machine's, replaced nothing that was found, so // its outcome says what the apply did, not that a cutover happened; a hold from when it was @@ -930,29 +955,6 @@ func applyService(ctx context.Context, sys system.System, r *declaration.Service return reflectOnly(ctx, sys, r, run, changed) } out := begin(r) - - // **What the unit was before the mesh touched it**, read once — the first time this host - // applies it — and carried in the record from then on (novox/hq ADR 0118). Undeclared, the - // unit is given back to exactly this: it is the one fact that separates the container runtime, - // running before the mesh arrived and to be left running, from the mesh's packet filter, - // stopped until the mesh started it and to be stopped again. A record that predates this - // field is not read now: the unit's state by then is the mesh's doing, not what was found. - switch { - case previous.Found != nil: - out.found = previous.Found - case previous.ID == "": - state, err := sys.ServiceState(ctx, run, r.Unit) - if err != nil { - return out, err - } - found := &store.FoundUnit{State: state} - if r.Boot != "" { - if found.Boot, err = sys.ServiceBoot(ctx, run, r.Unit); err != nil { - return out, err - } - } - out.found = found - } var changes []string // A file the service reflects changed, and it may be the unit's own file or a drop-in: the @@ -966,6 +968,21 @@ func applyService(ctx context.Context, sys system.System, r *declaration.Service } } + // **What the unit was before the mesh touched it**, read once and carried in the record from + // then on (novox/hq ADR 0118). Undeclared, the unit is given back to exactly this: it is the one + // fact that separates the container runtime, running before the mesh arrived and to be left + // running, from the mesh's packet filter, stopped until the mesh started it and to be stopped + // again. Read after the reload above, never before it: a unit whose file this same apply wrote + // is not a unit the service manager knows until then, and reading it first would find nothing. + found, gaveBack, err := foundAs(ctx, sys, r, run, previous) + if err != nil { + return out, err + } + out.found = found + if gaveBack != "" { + changes = append(changes, gaveBack) + } + // Boot first. A unit asked to be running and enabled should survive this apply failing // half way in the more useful direction: enabled-and-stopped comes back at the next boot, // where running-and-disabled does not. @@ -974,6 +991,15 @@ func applyService(ctx context.Context, sys system.System, r *declaration.Service if err != nil { return out, err } + // Whether it started at boot is found the first time the mesh is about to change that — + // which is not always the first apply: a declaration that said nothing about boot never + // touched it, so what is there when one first does is still the machine's (novox/hq ADR + // 0118). Kept before the change, so a failure after it still has it. + if out.found != nil && out.found.Boot == "" { + f := *out.found + f.Boot = bootBefore + out.found = &f + } if bootBefore != r.Boot { if err := sys.SetServiceBoot(ctx, run, r.Unit, r.Boot); err != nil { return out, fmt.Errorf("setting %s to %s at boot: %w", r.Unit, r.Boot, err) @@ -1138,7 +1164,10 @@ func reflectOnly(ctx context.Context, sys system.System, r *declaration.Service, // It returns the action rather than assuming "removed", because for half the vocabulary the // honest word is "forgotten". A host that reported a package removed when it left the package // installed would be describing an effect it declined to have. -func remove(ctx context.Context, sys system.System, a store.Applied, run Runner) (string, string, error) { +// +// made is the units whose unit file this host wrote where there was none (meshMadeUnits). +func remove(ctx context.Context, sys system.System, a store.Applied, run Runner, + made map[string]bool) (string, string, error) { switch declaration.Type(a.Type) { case declaration.TypeDirectory: // **A directory with anything left in it is kept, and that is the rule that protects @@ -1186,7 +1215,7 @@ func remove(ctx context.Context, sys system.System, a store.Applied, run Runner) return "removed", "no longer declared", nil case declaration.TypeService: - return removeService(ctx, sys, a, run) + return removeService(ctx, sys, a, run, made[a.Target]) case declaration.TypeProcess: // The other side of the same line: a process's unit is the host's own — it wrote the unit @@ -2036,45 +2065,171 @@ func declaredDigest(r declaration.Resource) string { // network stopped the container runtime and every container with it, how unassigning sshd would // have stopped ssh, and how an uplink module would have taken a machine off its only link // (novox/hq issue 130). It means undoing what the mesh did to it: a unit found running is left -// running; a unit the mesh started — the packet filter a converge loaded, which returning to -// adopted must unload — is stopped again, and disabled again if the mesh enabled it. +// running; a unit the mesh started is stopped again, and disabled again if the mesh enabled it. +// +// **A unit whose file the mesh wrote is the mesh's, whatever was found** — made is that. The +// adoption guard and the converge filter are units of exactly this kind: their unit files are the +// mesh's own `file` resources, written where there was none, and records written before the host +// kept what it found say nothing about them. Forgetting one would leave its table loaded — the +// guard beside a converged node's own filter, or the filter beside the predecessor's firewall +// re-enabled — and its unit file then deleted from under a running unit. So it is stopped and +// disabled at boot, as a process's unit is, before its file goes: orphans are removed newest +// first, and a unit's file is declared before the service that starts it. // // **Never started on the way out.** A unit the mesh stopped is not started again when its -// declaration goes: starting something is a decision, and the operator makes it. -func removeService(ctx context.Context, sys system.System, a store.Applied, run Runner) (string, string, error) { +// declaration goes: starting something is a decision, and the operator makes it. The report says +// what was actually done — a unit already as it was found is forgotten, not "restored". +func removeService(ctx context.Context, sys system.System, a store.Applied, run Runner, + made bool) (string, string, error) { if a.Stateless { + // Declared with no state (novox/hq ADR 0117): its lifecycle was never the mesh's, and the + // declaration that said so is the operator's word to hold to, a file of the mesh's or not. return "forgotten", "its state was never the mesh's", nil } - if a.Found == nil { + if !made && a.Found == nil { // Recorded before the host kept what it found. Not knowing, it leaves the unit as it is: // a unit left running can be stopped by the operator, and one stopped by mistake may be // the link the operator would use to do it. - return "forgotten", "the unit is the machine's; left as it is", nil + return "forgotten", "recorded before the host kept what it found; left as it is", nil } - if a.Found.State != "stopped" && (a.Found.Boot == "" || a.Found.Boot == "enabled") { + stop := made || a.Found.State == "stopped" + disable := made || a.Found.Boot == "disabled" + + if !stop && !disable { + // Found running, and not disabled by anyone but the mesh: nothing to give back. What the + // mesh did since is said, so a unit left stopped is not reported as the machine's doing — + // read as well as the machine answers, since nothing here acts on the answer. + state, err := sys.ServiceState(ctx, run, a.Target) + if err != nil && strings.Contains(err.Error(), "does not exist on this machine") { + return "forgotten", "the unit no longer exists", nil + } + var did []string + if err == nil && state == "stopped" { + did = append(did, "stopped it") + } + if a.Found.Boot == "enabled" { + if boot, err := sys.ServiceBoot(ctx, run, a.Target); err == nil && boot == "disabled" { + did = append(did, "disabled it at boot") + } + } + if len(did) > 0 { + return "forgotten", "left as it is; the mesh " + strings.Join(did, " and ") + + " and does not start anything on the way out", nil + } return "forgotten", "it was running before the mesh; left as it is", nil } - // Something is to be given back, so the unit must still be there to give it to. One since + + // Something may be given back, so the unit must still be there to give it to. One since // uninstalled is already as far from the mesh as it can be, and saying so keeps a record of it // from failing every apply. - if _, err := sys.ServiceState(ctx, run, a.Target); err != nil { + state, err := sys.ServiceState(ctx, run, a.Target) + if err != nil { if strings.Contains(err.Error(), "does not exist on this machine") { return "forgotten", "the unit no longer exists", nil } return "", "", err } - var gave []string - if a.Found.State == "stopped" { - if err := sys.SetServiceState(ctx, run, a.Target, "stopped"); err != nil { - return "", "", fmt.Errorf("stopping %s, which the mesh started: %w", a.Target, err) + var did, already []string + if stop { + if state != "stopped" { + if err := sys.SetServiceState(ctx, run, a.Target, "stopped"); err != nil { + return "", "", fmt.Errorf("stopping %s, which the mesh started: %w", a.Target, err) + } + did = append(did, "stopped") + } else { + already = append(already, "stopped") } - gave = append(gave, "stopped again") } - if a.Found.Boot == "disabled" { - if err := sys.SetServiceBoot(ctx, run, a.Target, "disabled"); err != nil { - return "", "", fmt.Errorf("disabling %s, which the mesh enabled: %w", a.Target, err) + if disable { + // Disabled unless it reads as disabled: a unit the service manager cannot say a boot state + // for — one with no install section — takes a disable as a no-op, and asking is how the + // host stays sure the mesh's enable did not outlive it. + if boot, err := sys.ServiceBoot(ctx, run, a.Target); err == nil && boot == "disabled" { + already = append(already, "disabled at boot") + } else { + if err := sys.SetServiceBoot(ctx, run, a.Target, "disabled"); err != nil { + return "", "", fmt.Errorf("disabling %s, which the mesh enabled: %w", a.Target, err) + } + did = append(did, "disabled at boot") } - gave = append(gave, "disabled at boot again") } - return "restored", strings.Join(gave, ", ") + ", as the host found it", nil + + if made { + if len(did) == 0 { + return "forgotten", "already stopped and disabled at boot; the mesh wrote its unit file", nil + } + return "removed", strings.Join(did, ", ") + "; the mesh wrote its unit file, so the unit is the mesh's own", nil + } + if len(did) == 0 { + return "forgotten", "already as the host found it (" + strings.Join(already, ", ") + ")", nil + } + detail := strings.Join(did, ", ") + ", as the host found it" + if len(already) > 0 { + detail += "; already " + strings.Join(already, ", ") + } + return "restored", detail, nil +} + +// foundAs is what a service's unit was before the mesh touched it, as far as this host can know: +// what an earlier apply recorded, or — the first time the mesh is about to act on the unit — what +// is there now (novox/hq ADR 0118). Nil when it cannot be known: a record written before the host +// kept this has had the mesh acting on the unit since, and a reading now would be the mesh's doing. +// +// Read now as well, besides on a first apply, where the mesh has never acted on this unit's state +// although a record exists: the record is of a service declared with no state (novox/hq ADR 0117), +// which the mesh never starts or stops, or of another unit altogether. What was found about one +// unit says nothing about another, so a service moved to a new unit gives the old one back, just as +// if it had been undeclared, and is read afresh for the new one; gave says what that gave back. +func foundAs(ctx context.Context, sys system.System, r *declaration.Service, run Runner, + previous store.Applied) (found *store.FoundUnit, gave string, err error) { + if f := previous.Found; f != nil { + unit := f.Unit + if unit == "" { + unit = previous.Target + } + if unit == "" || unit == r.Unit { + return f, "", nil + } + // Given back as if undeclared, never as the mesh's own: whether the mesh wrote the old + // unit's file is known to the removal of orphans, and that file's own record goes with it. + action, detail, err := removeService(ctx, sys, + store.Applied{Type: string(declaration.TypeService), Target: unit, Found: f}, run, false) + if err != nil { + return nil, "", fmt.Errorf("giving %s back as the host found it, now that %s is declared instead: %w", + unit, r.Unit, err) + } + if action == "restored" { + gave = unit + " " + detail + } + } else if previous.ID != "" && !previous.Stateless && previous.Target == r.Unit { + return nil, "", nil + } + state, err := sys.ServiceState(ctx, run, r.Unit) + if err != nil { + return nil, gave, err + } + return &store.FoundUnit{Unit: r.Unit, State: state}, gave, nil +} + +// meshMadeUnits is every unit whose unit file this host wrote whole where there was none: a `file` +// record with no kept original and not written into, at a unit's own path under a directory the +// service manager loads administrators' units from — or the one the host writes a process's unit +// in. Such a unit is the mesh's, whatever was found (novox/hq ADR 0118); see removeService. +// +// A drop-in is not the unit's own file, and a file the host wrote over is the machine's unit with +// the mesh's text in it: its original is kept, and put back when the file's record goes. +func meshMadeUnits(known store.State) map[string]bool { + made := map[string]bool{} + dirs := map[string]bool{"/etc/systemd/system": true, "/run/systemd/system": true, + filepath.Clean(unitDir): true} + for _, r := range known.Resources { + if r.Type != string(declaration.TypeFile) || r.Kept != "" || r.Into != nil { + continue + } + path := filepath.Clean(r.Target) + if dirs[filepath.Dir(path)] { + made[filepath.Base(path)] = true + } + } + return made } diff --git a/internal/apply/found_test.go b/internal/apply/found_test.go new file mode 100644 index 0000000..01a5248 --- /dev/null +++ b/internal/apply/found_test.go @@ -0,0 +1,387 @@ +package apply + +import ( + "context" + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/novox/mesh-host/internal/store" +) + +// Defends novox/hq ADR 0118: undeclaring removes what the mesh made, gives back what it changed, +// and leaves what was the machine's — which needs what was found kept exactly, and a unit the mesh +// made known for the mesh's whatever its record says. + +func unitsMachine(units map[string]*fakeUnit) *machine { + return &machine{containers: map[string]*fakeContainer{}, units: units} +} + +// nothingButA is a declaration of one unrelated file, so everything recorded is undeclared. +func nothingButA(t *testing.T) string { + return `{"declaration":1,"resources":[ + {"id":"other","type":"file","path":"` + filepath.Join(t.TempDir(), "a") + `","content":"a\n"}]}` +} + +// copyOf is a state as a later load of it would be: sharing nothing with the one it came from. +func copyOf(t *testing.T, s store.State) store.State { + t.Helper() + raw, err := json.Marshal(s) + if err != nil { + t.Fatal(err) + } + var out store.State + if err := json.Unmarshal(raw, &out); err != nil { + t.Fatal(err) + } + return out +} + +func applyOn(t *testing.T, raw string, known store.State, m *machine) (Report, store.State, error) { + t.Helper() + return Apply(context.Background(), archHost(t), parse(t, raw), known, store.OriginDeclared, m.run, nil, nil) +} + +func TestAUnitWhoseFileTheMeshWroteIsStoppedWhateverItsRecordSays(t *testing.T) { + // The adoption guard's unit file is the mesh's own file resource, written where there was none. + // Its service recorded before the host kept what it found has no Found, and forgetting it left + // the guard's table loaded on a converged node and its unit file deleted from under it. + units := t.TempDir() + was := unitDir + unitDir = units + t.Cleanup(func() { unitDir = was }) + unitFile := filepath.Join(units, "mesh-guard.service") + if err := os.WriteFile(unitFile, []byte("[Unit]\n"), 0o644); err != nil { + t.Fatal(err) + } + m := unitsMachine(map[string]*fakeUnit{"mesh-guard.service": {active: "active", enabled: "enabled"}}) + fileThereAtStop := false + run := func(ctx context.Context, name string, args ...string) (string, error) { + if name == "systemctl" && args[0] == "stop" { + _, err := os.Stat(unitFile) + fileThereAtStop = err == nil + } + return m.run(ctx, name, args...) + } + // As a host before this change recorded them: the unit file first, then the service it + // starts, and no Found on the service. + known := store.State{Resources: []store.Applied{ + {ID: "adoption.guard-unit", Type: "file", Target: unitFile, Origin: store.OriginDeclared}, + {ID: "adoption.guard-running", Type: "service", Target: "mesh-guard.service", Origin: store.OriginDeclared}, + }} + report, after, err := applyWith(t, parse(t, nothingButA(t)), known, run) + if err != nil { + t.Fatal(err) + } + if u := m.units["mesh-guard.service"]; u.active != "inactive" || u.enabled != "disabled" { + t.Errorf("the mesh's own unit was left %s and %s: %v", u.active, u.enabled, m.asked) + } + if !fileThereAtStop { + t.Error("the unit was stopped after its file was deleted, or not at all") + } + if o := outcomeOf(report, "adoption.guard-running"); o.Action != "removed" || !strings.Contains(o.Detail, "unit file") { + t.Errorf("outcome %+v", o) + } + if _, err := os.Stat(unitFile); !os.IsNotExist(err) { + t.Error("the unit file outlived its record") + } + if len(after.Resources) != 1 { + t.Errorf("still recorded: %v", after.IDs()) + } + + // A unit file the host wrote OVER — its original kept — is the machine's unit, and a record + // with no Found leaves it as it is. + if err := os.WriteFile(unitFile, []byte("[Unit]\n"), 0o644); err != nil { + t.Fatal(err) + } + m = unitsMachine(map[string]*fakeUnit{"mesh-guard.service": {active: "active", enabled: "enabled"}}) + known.Resources[0].Kept = filepath.Join(t.TempDir(), "original") + if err := os.WriteFile(known.Resources[0].Kept, []byte("[Unit]\n"), 0o600); err != nil { + t.Fatal(err) + } + if _, _, err := applyWith(t, parse(t, nothingButA(t)), known, m.run); err != nil { + t.Fatal(err) + } + if m.did("systemctl stop") || m.did("systemctl disable") { + t.Errorf("a unit whose file the mesh only wrote over was stopped: %v", m.asked) + } +} + +func TestOnlyAUnitsOwnFileTheMeshCreatedMakesItTheMeshs(t *testing.T) { + known := store.State{Resources: []store.Applied{ + {ID: "a", Type: "file", Target: "/etc/systemd/system/made.service"}, + {ID: "b", Type: "file", Target: "/run/systemd/system/runtime.service"}, + {ID: "c", Type: "file", Target: "/etc/systemd/system/kept.service", Kept: "/var/lib/mesh-host/kept/x"}, + {ID: "d", Type: "file", Target: "/etc/systemd/system/into.service", Into: &store.Into{Format: "block"}}, + {ID: "e", Type: "file", Target: "/etc/systemd/system/docker.service.d/mesh.conf"}, + {ID: "f", Type: "file", Target: "/etc/mesh/elsewhere.service"}, + {ID: "g", Type: "directory", Target: "/etc/systemd/system/dir.service"}, + }} + made := meshMadeUnits(known) + for unit, want := range map[string]bool{"made.service": true, "runtime.service": true, "kept.service": false, + "into.service": false, "mesh.conf": false, "docker.service.d": false, "elsewhere.service": false, "dir.service": false} { + if made[unit] != want { + t.Errorf("%s: made %v, want %v", unit, made[unit], want) + } + } +} + +func TestWhatWasFoundOutlivesAFirstApplyThatFailed(t *testing.T) { + // Enabled, then it would not start: no record. The next apply must not read the enable as + // the machine's — or undeclaring leaves enabled a unit the mesh enabled. + m := unitsMachine(map[string]*fakeUnit{"filter.service": {active: "inactive", enabled: "disabled", wontStart: true}}) + declared := `{"declaration":1,"resources":[ + {"id":"s","type":"service","unit":"filter.service","state":"running","boot":"enabled"}]}` + _, first, err := applyOn(t, declared, store.State{}, m) + if err == nil || !m.did("systemctl enable filter.service") { + t.Fatalf("the fixture did not enable and then fail: %v, %v", err, m.asked) + } + if _, ok := first.Find("s"); ok { + t.Fatal("a failed apply was recorded") + } + if p := first.FoundFirst["s"]; p.State != "stopped" || p.Boot != "disabled" || p.Unit != "filter.service" { + t.Fatalf("what was found was not kept through the failure: %+v", first.FoundFirst) + } + + failed := copyOf(t, first) + + m.units["filter.service"].wontStart = false + _, second, err := applyOn(t, declared, first, m) + if err != nil { + t.Fatal(err) + } + if rec, _ := second.Find("s"); rec.Found == nil || rec.Found.State != "stopped" || rec.Found.Boot != "disabled" { + t.Errorf("the record carries the mesh's own effect as found: %+v", rec.Found) + } + if second.FoundFirst != nil { + t.Errorf("kept apart after the record carried it: %+v", second.FoundFirst) + } + + if _, _, err := applyOn(t, nothingButA(t), second, m); err != nil { + t.Fatal(err) + } + if u := m.units["filter.service"]; u.active != "inactive" || u.enabled != "disabled" { + t.Errorf("undeclared, the unit was left %s and %s", u.active, u.enabled) + } + + // Undeclared before it was ever recorded, what was found goes with it — only for its origin. + if _, kept, _ := Apply(context.Background(), archHost(t), parse(t, nothingButA(t)), copyOf(t, failed), + store.OriginCarried, m.run, nil, nil); kept.FoundFirst["s"].State == "" { + t.Error("a carried apply dropped what the mesh's declaration found") + } + if _, dropped, err := applyOn(t, nothingButA(t), copyOf(t, failed), m); err != nil || dropped.FoundFirst != nil { + t.Errorf("kept for a service no longer declared: %+v, %v", dropped.FoundFirst, err) + } +} + +func TestBootIsFoundTheFirstTimeTheMeshSetsIt(t *testing.T) { + // The declaration said nothing about boot at first, so the mesh never touched it: what is + // there when a declaration first does is still the machine's. + m := unitsMachine(map[string]*fakeUnit{"web.service": {active: "active", enabled: "disabled"}}) + known := store.State{Resources: []store.Applied{{ID: "s", Type: "service", Target: "web.service", + Origin: store.OriginDeclared, Found: &store.FoundUnit{Unit: "web.service", State: "running"}}}} + _, after, err := applyOn(t, `{"declaration":1,"resources":[ + {"id":"s","type":"service","unit":"web.service","state":"running","boot":"enabled"}]}`, known, m) + if err != nil { + t.Fatal(err) + } + rec, _ := after.Find("s") + if rec.Found == nil || rec.Found.State != "running" || rec.Found.Boot != "disabled" { + t.Fatalf("found %+v, want running and disabled", rec.Found) + } + report, _, err := applyOn(t, nothingButA(t), after, m) + if err != nil { + t.Fatal(err) + } + if u := m.units["web.service"]; u.active != "active" || u.enabled != "disabled" { + t.Errorf("undeclared, the unit is %s and %s; want running, and disabled again", u.active, u.enabled) + } + if o := outcomeOf(report, "s"); o.Action != "restored" || o.Detail != "disabled at boot, as the host found it" { + t.Errorf("outcome %+v", o) + } +} + +func TestAServiceOnceDeclaredWithNoStateIsFoundWhenFirstGivenOne(t *testing.T) { + // Declared with no state (novox/hq ADR 0117), the mesh never started or stopped it — so what + // is there when a declaration first gives it one is what the machine had. + m := unitsMachine(map[string]*fakeUnit{"net.service": {active: "inactive", enabled: "disabled"}}) + known := store.State{Resources: []store.Applied{{ID: "s", Type: "service", Target: "net.service", + Origin: store.OriginDeclared, Stateless: true}}} + _, after, err := applyOn(t, `{"declaration":1,"resources":[ + {"id":"s","type":"service","unit":"net.service","state":"running"}]}`, known, m) + if err != nil { + t.Fatal(err) + } + if rec, _ := after.Find("s"); rec.Found == nil || rec.Found.State != "stopped" { + t.Fatalf("found %+v, want stopped", rec.Found) + } + if _, _, err := applyOn(t, nothingButA(t), after, m); err != nil { + t.Fatal(err) + } + if m.units["net.service"].active != "inactive" { + t.Error("the unit the mesh started outlived its declaration") + } +} + +func TestAUnitWrittenInTheSameApplyIsLoadedBeforeItIsRead(t *testing.T) { + // Read before the service manager is told about its new file, the unit is not there to find. + dir := t.TempDir() + m := unitsMachine(map[string]*fakeUnit{"fresh.service": {active: "inactive", enabled: "disabled"}}) + _, _, err := applyOn(t, `{"declaration":1,"resources":[ + {"id":"unit","type":"file","path":"`+filepath.Join(dir, "fresh.service")+`","content":"[Unit]\n"}, + {"id":"s","type":"service","unit":"fresh.service","state":"running","restart-on":["unit"]}]}`, + store.State{}, m) + if err != nil { + t.Fatal(err) + } + reload, show := -1, -1 + for i, a := range m.asked { + if a == "systemctl daemon-reload" && reload < 0 { + reload = i + } + if strings.HasPrefix(a, "systemctl show fresh.service") && show < 0 { + show = i + } + } + if reload < 0 || show < 0 || reload > show { + t.Errorf("the unit was read before its file was loaded: %v", m.asked) + } +} + +func TestAServiceMovedToAnotherUnitGivesTheOldOneBackAndFindsTheNewOne(t *testing.T) { + m := unitsMachine(map[string]*fakeUnit{ + "old.service": {active: "active", enabled: "enabled"}, + "new.service": {active: "inactive", enabled: "disabled"}, + }) + known := store.State{Resources: []store.Applied{{ID: "s", Type: "service", Target: "old.service", + Origin: store.OriginDeclared, Found: &store.FoundUnit{Unit: "old.service", State: "stopped"}}}} + report, after, err := applyOn(t, `{"declaration":1,"resources":[ + {"id":"s","type":"service","unit":"new.service","state":"running"}]}`, known, m) + if err != nil { + t.Fatal(err) + } + if m.units["old.service"].active != "inactive" { + t.Error("the unit the mesh started is still running though nothing declares it") + } + if m.units["new.service"].active != "active" { + t.Error("the unit now declared was not started") + } + rec, _ := after.Find("s") + if rec.Found == nil || rec.Found.Unit != "new.service" || rec.Found.State != "stopped" { + t.Errorf("what was found about the old unit was carried to the new one: %+v", rec.Found) + } + if o := outcomeOf(report, "s"); !strings.Contains(o.Detail, "old.service stopped, as the host found it") { + t.Errorf("giving the old unit back went unsaid: %+v", o) + } +} + +func TestARemovalSaysWhatItDid(t *testing.T) { + cases := []struct { + name string + found *store.FoundUnit + unit *fakeUnit + action, detail string + }{ + {"found stopped and still stopped", &store.FoundUnit{State: "stopped"}, + &fakeUnit{active: "inactive", enabled: "disabled"}, "forgotten", "already as the host found it (stopped)"}, + {"started by the mesh", &store.FoundUnit{State: "stopped"}, + &fakeUnit{active: "active", enabled: "disabled"}, "restored", "stopped, as the host found it"}, + {"started and enabled by the mesh", &store.FoundUnit{State: "stopped", Boot: "disabled"}, + &fakeUnit{active: "active", enabled: "enabled"}, "restored", "stopped, disabled at boot, as the host found it"}, + {"found running, stopped by the mesh", &store.FoundUnit{State: "running"}, + &fakeUnit{active: "inactive", enabled: "disabled"}, "forgotten", + "left as it is; the mesh stopped it and does not start anything on the way out"}, + {"found running and enabled, disabled by the mesh", &store.FoundUnit{State: "running", Boot: "enabled"}, + &fakeUnit{active: "active", enabled: "disabled"}, "forgotten", + "left as it is; the mesh disabled it at boot and does not start anything on the way out"}, + {"found running, still running", &store.FoundUnit{State: "running"}, + &fakeUnit{active: "active", enabled: "enabled"}, "forgotten", "it was running before the mesh; left as it is"}, + // Found says to stop it, so the machine is asked about it — and it is gone. + {"started by the mesh, since uninstalled", &store.FoundUnit{State: "stopped"}, + nil, "forgotten", "the unit no longer exists"}, + {"recorded before the host kept what it found", nil, + &fakeUnit{active: "active", enabled: "enabled"}, "forgotten", "recorded before the host kept what it found; left as it is"}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + units := map[string]*fakeUnit{} + if c.unit != nil { + units["unit.service"] = c.unit + } + m := unitsMachine(units) + known := store.State{Resources: []store.Applied{{ID: "s", Type: "service", Target: "unit.service", + Origin: store.OriginDeclared, Found: c.found}}} + report, after, err := applyOn(t, nothingButA(t), known, m) + if err != nil { + t.Fatal(err) + } + if o := outcomeOf(report, "s"); o.Action != c.action || o.Detail != c.detail { + t.Errorf("said %s: %s; want %s: %s", o.Action, o.Detail, c.action, c.detail) + } + if c.unit == nil && !m.did("systemctl show unit.service") { + t.Errorf("the machine was never asked whether the unit is there: %v", m.asked) + } + if m.did("systemctl start") || m.did("systemctl enable") { + t.Errorf("something was started on the way out: %v", m.asked) + } + if _, still := after.Find("s"); still { + t.Error("still recorded") + } + }) + } +} + +func TestAUnitTheMeshStartedIsStoppedWhenUndeclaredAndOneFoundRunningIsNot(t *testing.T) { + // End to end: found by the first apply, carried, given back. + m := unitsMachine(map[string]*fakeUnit{ + "filter.service": {active: "inactive", enabled: "disabled"}, + "runtime.service": {active: "active", enabled: "enabled"}, + }) + _, state, err := applyOn(t, `{"declaration":1,"resources":[ + {"id":"filter","type":"service","unit":"filter.service","state":"running","boot":"enabled"}, + {"id":"runtime","type":"service","unit":"runtime.service","state":"running","boot":"enabled"}]}`, + store.State{}, m) + if err != nil { + t.Fatal(err) + } + if m.units["filter.service"].active != "active" { + t.Fatal("the fixture did not start the filter") + } + if _, _, err := applyOn(t, nothingButA(t), state, m); err != nil { + t.Fatal(err) + } + if u := m.units["filter.service"]; u.active != "inactive" || u.enabled != "disabled" { + t.Errorf("the filter the mesh started and enabled is %s and %s", u.active, u.enabled) + } + if u := m.units["runtime.service"]; u.active != "active" || u.enabled != "enabled" { + t.Errorf("the runtime that was running before the mesh is %s and %s", u.active, u.enabled) + } +} + +func TestAUnitHeldAndThenTakenIsFoundAsThePredecessorLeftIt(t *testing.T) { + // Held while its module was untaken, nothing was applied and nothing found; taken, the first + // apply finds the predecessor's unit — stopped, but started at boot — before starting it. + dir := t.TempDir() + m := unitsMachine(map[string]*fakeUnit{"hello.service": {active: "inactive", enabled: "enabled"}}) + service := `{"id":"hello-web.unit","type":"service","unit":"hello.service","state":"running","boot":"enabled"}` + _, held := applyAdopted(t, adopted(t, untaken("hello-web.unit"), service), store.State{}, m, dir) + if _, ok := held.HeldAt("hello-web.unit"); !ok { + t.Fatal("the fixture's unit was not held") + } + _, taken := applyAdopted(t, adopted(t, `{"taken":["hello-web"]}`, service), held, m, dir) + rec, _ := taken.Find("hello-web.unit") + if rec.Found == nil || rec.Found.State != "stopped" || rec.Found.Boot != "enabled" { + t.Fatalf("found %+v, want the predecessor's stopped and enabled", rec.Found) + } + if m.units["hello.service"].active != "active" { + t.Fatal("the taken unit was not started") + } + if _, _, err := applyOn(t, nothingButA(t), taken, m); err != nil { + t.Fatal(err) + } + if u := m.units["hello.service"]; u.active != "inactive" || u.enabled != "enabled" { + t.Errorf("given back as %s and %s; the predecessor left it stopped and enabled", u.active, u.enabled) + } +} diff --git a/internal/apply/opening_test.go b/internal/apply/opening_test.go index 1be33f5..15a3a90 100644 --- a/internal/apply/opening_test.go +++ b/internal/apply/opening_test.go @@ -339,8 +339,18 @@ func TestReturningToAdoptedLoadsTheGuardBeforeRemovingTheFilter(t *testing.T) { dir := t.TempDir() guard := filepath.Join(dir, "guard.nft") guardFile := `{"id":"adoption.guard","type":"file","path":"` + guard + `","content":"table inet mesh_guard {}\n"}` + // The filter's unit file is the mesh's own, written where there was none — which is what makes + // its unit the mesh's to stop, with or without a record of what was found (novox/hq ADR 0118). + units := t.TempDir() + was := unitDir + unitDir = units + t.Cleanup(func() { unitDir = was }) + filterUnit := filepath.Join(units, "mesh-filter.service") for _, stopFails := range []bool{false, true} { _ = os.Remove(guard) + if err := os.WriteFile(filterUnit, []byte("[Unit]\n"), 0o644); err != nil { + t.Fatal(err) + } guardUpAtStop := false run := func(_ context.Context, name string, args ...string) (string, error) { if name != "systemctl" { @@ -358,10 +368,10 @@ func TestReturningToAdoptedLoadsTheGuardBeforeRemovingTheFilter(t *testing.T) { } return "", nil } + // As a host recorded them before it kept what it found: no Found on the service. converged := store.State{Resources: []store.Applied{ - {ID: "nftables.load", Type: "service", Target: "mesh-filter.service", Origin: store.OriginDeclared, - // The mesh loaded this filter at converge: it was not running before (novox/hq ADR 0118). - Found: &store.FoundUnit{State: "stopped"}}}} + {ID: "nftables.unit", Type: "file", Target: filterUnit, Origin: store.OriginDeclared}, + {ID: "nftables.load", Type: "service", Target: "mesh-filter.service", Origin: store.OriginDeclared}}} _, state, err := applyWith(t, adopted(t, `{"taken":[]}`, withConf(dir)+","+guardFile), converged, run) if !guardUpAtStop { t.Errorf("stop fails %v: the derived filter was stopped before the guard was written", stopFails) diff --git a/internal/apply/plan.go b/internal/apply/plan.go index a142d6e..1937617 100644 --- a/internal/apply/plan.go +++ b/internal/apply/plan.go @@ -19,7 +19,7 @@ import ( // Step is one thing an apply would do to this machine. type Step struct { - // Verb is create · update · check · hold · run · remove · forget · disable · enable. + // Verb is create · update · check · hold · run · remove · forget · restore · disable · enable. Verb string `json:"verb"` Type string `json:"type,omitempty"` ID string `json:"id,omitempty"` @@ -77,6 +77,7 @@ func Plan(d *declaration.Declaration, known store.State, origin string) []Step { } var protecting, orphans []Step + made := meshMadeUnits(known) for _, orphan := range known.Orphans(declared, origin) { step := Step{Verb: "remove", Type: orphan.Type, ID: orphan.ID, Target: orphan.Target, Why: "recorded here and no longer declared"} @@ -84,12 +85,32 @@ func Plan(d *declaration.Declaration, known store.State, origin string) []Step { case orphan.Stateless: step.Verb, step.Why = "forget", "no longer declared; its unit's state was never the mesh's and is left as it is" case orphan.Type == string(declaration.TypeService): - // What removal will do, said before it does it (novox/hq ADR 0118): a unit is given - // back what the host found, so the preview names which units that stops. - if f := orphan.Found; f != nil && (f.State == "stopped" || f.Boot == "disabled") { - step.Verb, step.Why = "restore", "no longer declared; the mesh started or enabled it, and it goes back as it was found" - } else { - step.Verb, step.Why = "forget", "no longer declared; the unit is the machine's and is left as it is" + // What removal will do, said before it does it (novox/hq ADR 0118), in removeService's + // words. "restore" only where it may stop or disable something — the record cannot say + // whether the unit is still as the mesh left it, so "may" is as far as a preview goes — + // and a unit whose file the mesh wrote is named as the mesh's, since that one is + // stopped whatever was found. + f := orphan.Found + switch { + case made[orphan.Target]: + step.Verb, step.Why = "remove", "no longer declared; the mesh wrote its unit file, so it is "+ + "stopped and disabled at boot before that file goes" + case f == nil: + step.Verb, step.Why = "forget", "no longer declared; recorded before the host kept what it "+ + "found, so it is left as it is" + case f.State == "stopped" || f.Boot == "disabled": + var back []string + if f.State == "stopped" { + back = append(back, "stopped") + } + if f.Boot == "disabled" { + back = append(back, "disabled at boot") + } + step.Verb, step.Why = "restore", "no longer declared; the host found it "+ + strings.Join(back, " and ")+", and it goes back to that if the mesh changed it" + default: + step.Verb, step.Why = "forget", "no longer declared; it was running before the mesh and is "+ + "left as it is — nothing is started or stopped on the way out" } } if d.Adoption == nil && strings.HasPrefix(orphan.ID, declaration.AdoptionPrefix) { diff --git a/internal/apply/plan_test.go b/internal/apply/plan_test.go index 4f8f7ce..dcbeecc 100644 --- a/internal/apply/plan_test.go +++ b/internal/apply/plan_test.go @@ -261,3 +261,42 @@ func TestAPlanSaysAContainerIsRecreatedWhenAFileItReadsChanged(t *testing.T) { t.Errorf("a container with no record of what it read is planned as %q", got) } } + +func TestAPlanSaysWhichUnitsAnUndeclareGivesBackAndWhichItLeaves(t *testing.T) { + // novox/hq ADR 0118, said before it is done: "restore" only where removal may stop or disable + // something, and a unit whose file the mesh wrote named as the mesh's. + known := store.State{} + known.Record(store.Applied{ID: "guard-unit", Type: "file", Target: "/etc/systemd/system/mesh-guard.service"}) + known.Record(store.Applied{ID: "guard", Type: "service", Target: "mesh-guard.service"}) + known.Record(store.Applied{ID: "filter", Type: "service", Target: "filter.service", + Found: &store.FoundUnit{State: "stopped"}}) + known.Record(store.Applied{ID: "boot", Type: "service", Target: "boot.service", + Found: &store.FoundUnit{State: "running", Boot: "disabled"}}) + known.Record(store.Applied{ID: "runtime", Type: "service", Target: "docker.service", + Found: &store.FoundUnit{State: "running", Boot: "enabled"}}) + known.Record(store.Applied{ID: "old", Type: "service", Target: "sshd.service"}) + known.Record(store.Applied{ID: "nm", Type: "service", Target: "NetworkManager.service", Stateless: true}) + + steps := Plan(parse(t, nothingButA(t)), known, store.OriginCarried) + got := verbs(steps) + want := "forget nm, forget old, forget runtime, restore boot, restore filter, remove guard, remove guard-unit, create other" + if got != want { + t.Fatalf("planned %s\nwant %s", got, want) + } + for _, s := range steps { + switch s.ID { + case "guard": + if !strings.Contains(s.Why, "unit file") { + t.Errorf("the mesh's own unit was not named as the mesh's: %q", s.Why) + } + case "filter": + if !strings.Contains(s.Why, "found it stopped") { + t.Errorf("what the unit goes back to went unsaid: %q", s.Why) + } + case "old": + if !strings.Contains(s.Why, "left as it is") { + t.Errorf("a unit with nothing found was not said to be left: %q", s.Why) + } + } + } +} diff --git a/internal/store/store.go b/internal/store/store.go index 0878b62..71485a0 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -154,6 +154,18 @@ type State struct { // other mode can be refused before it is applied (novox/hq issue 104). Mode string `json:"mode,omitempty"` + // FoundFirst is, by resource id, what a service's unit was found as by an apply of it that did + // not finish — kept apart from Resources, because a record follows the fact and this apply's + // fact never came (novox/hq ADR 0118). + // + // **The capture is the one reading that cannot be taken again.** A first apply that enabled a + // unit and then failed to start it leaves no record; without this, the next apply would find + // the unit enabled, take that for what the machine had, and undeclaring would leave enabled a + // unit the mesh enabled. So what is found is written the moment it is read, whatever the apply + // of the resource then does, and a later apply reads it here before it reads the machine. + // Dropped once a record carrying it is written, and when its resource is no longer declared. + FoundFirst map[string]PendingFound `json:"found_first,omitempty"` + // Genesis is the bundle this host consumed raising the foundation, if it has. Once recorded, // the bundle carried in the binary is not applied again: what genesis applied was rewritten // for this machine, and the mesh has said more since (novox/hq issue 104). @@ -457,9 +469,47 @@ func originOf(r Applied) string { // FoundUnit is a service's unit as the host first found it. type FoundUnit struct { + // Unit is which unit this was read from. What was found about one unit says nothing about + // another, so a service whose declaration moves to a different unit is read again for that one + // (novox/hq ADR 0118). Empty on what was kept before this was: the record's Target then says. + Unit string `json:"unit,omitempty"` // State is "running" or "stopped". State string `json:"state"` // Boot is "enabled" or "disabled" — or empty when the declaration never set it, and the host // never touched it. Boot string `json:"boot,omitempty"` } + +// PendingFound is a unit as found by an apply of its service that has not yet been recorded, and +// who asked for that apply — so only a declaration from the same origin can say it is gone. +type PendingFound struct { + FoundUnit + Origin string `json:"origin,omitempty"` +} + +// KeepFound writes down what an unfinished apply found a service's unit as. +func (s *State) KeepFound(id, origin string, f FoundUnit) { + if s.FoundFirst == nil { + s.FoundFirst = map[string]PendingFound{} + } + s.FoundFirst[id] = PendingFound{FoundUnit: f, Origin: origin} +} + +// DropFound forgets what was found for one resource: its record now carries it, or it is gone. +func (s *State) DropFound(id string) { + delete(s.FoundFirst, id) + if len(s.FoundFirst) == 0 { + s.FoundFirst = nil + } +} + +// DropFoundUndeclared forgets what was found for every resource of this origin the declaration no +// longer names. Only this origin's, for the reason Orphans gives: a mesh declaration's silence says +// nothing about what the bundle applies, nor the other way round. +func (s *State) DropFoundUndeclared(declared map[string]bool, origin string) { + for id, p := range s.FoundFirst { + if !declared[id] && originOf(Applied{Origin: p.Origin}) == origin { + s.DropFound(id) + } + } +} From b462f461c6c161dfa8347c944790ff99331e2bf2 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 27 Sep 2026 00:47:57 +0200 Subject: [PATCH 10/11] A taken tunnel's found configuration is retired once the take is proven (hq ADR 0119) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Kept on disk it was the take's fallback; once the mesh's interface is up in its place and a peer has handshaken with it, it is an unmaintained way back onto the network, held for ever. It is now removed from where its unit reads it, its kept original verified first and left as it is, and the hold ends. Until proven — no handshake, or wg not answering — it is kept and the report says why. The retirement is recorded apart from holds, so later applies, an undeclare, and a reassignment find it retired rather than missing, and nothing writes it back. --- internal/apply/apply.go | 16 +- internal/apply/hold_test.go | 7 + internal/apply/plan.go | 38 +++- internal/apply/takeover.go | 168 +++++++++++++++++- internal/apply/takeover_test.go | 257 +++++++++++++++++++++++++++- internal/declaration/declaration.go | 7 +- internal/store/store.go | 58 +++++++ internal/tunnel/tunnel.go | 45 ++++- internal/tunnel/tunnel_test.go | 46 +++++ 9 files changed, 628 insertions(+), 14 deletions(-) diff --git a/internal/apply/apply.go b/internal/apply/apply.go index 6bdb549..af94644 100644 --- a/internal/apply/apply.go +++ b/internal/apply/apply.go @@ -364,6 +364,7 @@ func ApplyKeeping( // flushed. A failure here fails the service too — the mesh's interface is not started on a // port the found one still holds. stoppedFound := false + tookAt := -1 if svc, ok := resource.(*declaration.Service); ok && svc.TakesOver != nil { var outcome Outcome var facts TakenTunnel @@ -388,8 +389,11 @@ func ApplyKeeping( log(fmt.Sprintf(" failed %s (%s): %v", svc.Identity(), svc.Unit, err)) continue } + tookAt = len(report.Outcomes) report.Outcomes = append(report.Outcomes, outcome) - log(fmt.Sprintf(" held %s (%s): %s", outcome.ID, outcome.Target, outcome.Detail)) + if outcome.Action == "held" { + log(fmt.Sprintf(" held %s (%s): %s", outcome.ID, outcome.Target, outcome.Detail)) + } } was, _ := known.Find(resource.Identity()) @@ -510,6 +514,16 @@ func ApplyKeeping( // The found interface is down and the mesh's is up in its place: the tunnel changed // hands (novox/hq ADR 0105). Read from the machine, not assumed. report.Tunnel.State = tunnelState(ctx, sys, svc.TakesOver.Unit, svc.Unit, run) + // And once a peer has handshaken with it, the take is proven and the found + // configuration is retired — here, after the mesh's service applied, so in the apply + // of the take itself only if a peer is already through; otherwise a later apply + // retires it (novox/hq ADR 0119). What it did replaces what the take said of the file. + if o, did := retireFound(ctx, svc, &known, run, keep, report.Tunnel, time.Now().UTC()); did { + if tookAt >= 0 { + report.Outcomes[tookAt] = o + } + log(fmt.Sprintf(" %s %s (%s): %s", o.Action, o.ID, o.Target, o.Detail)) + } } report.Outcomes = append(report.Outcomes, outcome) if outcome.Action != "unchanged" { diff --git a/internal/apply/hold_test.go b/internal/apply/hold_test.go index 850f254..5f67996 100644 --- a/internal/apply/hold_test.go +++ b/internal/apply/hold_test.go @@ -21,6 +21,10 @@ type machine struct { asked []string // wgUp is what `wg show interfaces` answers: the tunnels up on the machine. wgUp string + // handshakes is what `wg show latest-handshakes` answers, and handshakesFail the + // error it fails with instead — a machine with no `wg`, say (novox/hq ADR 0119). + handshakes string + handshakesFail error // units are service units by name, as systemd would report them; volumes are the runtime's // named volumes. @@ -101,6 +105,9 @@ func (m *machine) run(_ context.Context, name string, args ...string) (string, e return m.systemctl(args) } if name == "wg" { + if len(args) > 0 && args[len(args)-1] == "latest-handshakes" { + return m.handshakes, m.handshakesFail + } return m.wgUp, nil } if name == "getent" { diff --git a/internal/apply/plan.go b/internal/apply/plan.go index 1937617..475a34c 100644 --- a/internal/apply/plan.go +++ b/internal/apply/plan.go @@ -56,6 +56,12 @@ func Plan(d *declaration.Declaration, known store.State, origin string) []Step { for _, r := range d.Resources { declared[r.Identity()] = true } + // The found tunnel's configuration is held under an id of its own, declared for as long as the + // service taking it over is — as ApplyKeeping counts it, or a plan would forget a hold the + // apply keeps (novox/hq ADR 0105). + if svc := takesOver(d); svc != nil { + declared[takeOverID(svc)] = true + } rec := known.Firewall ufw := rec != nil && rec.Kind == string(firewall.UFW) @@ -136,7 +142,12 @@ func Plan(d *declaration.Declaration, known store.State, origin string) []Step { } steps = append(steps, orphans...) for _, r := range rest { - steps = append(steps, planned(r, d, known)) + step := planned(r, d, known) + if svc, ok := r.(*declaration.Service); ok && svc.TakesOver != nil && d.Adoption != nil && step.Verb != "hold" { + // The take comes before the service that replaces the tunnel, as it does in the apply. + steps = append(steps, plannedTake(svc, known)) + } + steps = append(steps, step) } // Only a declaration from the mesh converges a node; a bundle or a file never retires the @@ -239,6 +250,31 @@ func planned(r declaration.Resource, d *declaration.Declaration, known store.Sta return step } +// plannedTake is what the take of a found tunnel would do to its configuration (novox/hq ADR 0105, +// ADR 0119): kept as found while the take is not proven, and retired — removed from where its unit +// reads it, its original staying kept — by the first apply that finds the mesh's interface up in +// its place with a peer handshaken. Whether that is this apply is read from the machine, which a +// plan does not do, so it says when rather than whether. One the mesh retired already is said as +// retired: nothing brings it back. +func plannedTake(svc *declaration.Service, known store.State) Step { + t := svc.TakesOver + step := Step{Verb: "hold", Type: string(declaration.TypeFile), ID: takeOverID(svc), Target: t.Config} + if r, ok := known.RetiredAt(t.Config); ok { + step.Verb = "check" + step.Why = "retired once the take of " + t.Interface + " was proven; its original stays at " + r.Kept + + " and the mesh never brings it back" + return step + } + step.Why = "the configuration of the tunnel " + t.Interface + ", kept as found while " + svc.Unit + + " takes it over (" + t.Unit + " stopped and disabled, never flushed); retired — removed from " + + t.Config + ", its original staying kept — once the take is proven by a peer handshaking on " + + strings.TrimPrefix(svc.Unit, "wg-quick@") + if h, ok := known.HeldAt(takeOverID(svc)); ok && h.Kept != "" { + step.Why += "; the original is at " + h.Kept + } + return step +} + // readsChanged is which of the files a container was created reading the apply will hand it // changed — the same comparison applyContainer makes (novox/hq 04-ISSUES/103), settled from the // declaration and the record alone. diff --git a/internal/apply/takeover.go b/internal/apply/takeover.go index 359412c..8bc7130 100644 --- a/internal/apply/takeover.go +++ b/internal/apply/takeover.go @@ -27,6 +27,16 @@ import ( // Every apply, not once: a found unit somebody starts again would take the port back from the // mesh's interface, so it is stopped again and said so. That is the one place an adopted node // undoes something done by hand, and it is because the tunnel is the mesh's now. +// +// **Until the take is proven, and then the found configuration is retired** (novox/hq ADR 0119). +// Keeping it on disk was the caution the take needed: if the mesh's interface does not come up, +// the found unit is started again and the peers never notice. That caution is spent once the +// tunnel is taken — the found unit down and disabled, the mesh's interface up — and a peer has +// handshaken with the mesh's interface. From then on a configuration nothing maintains, one +// command away from raising a second way onto the network, is not a rollback path but a door +// nobody watches. So it is removed from where its unit reads it; its original, kept before +// anything happened to it (ADR 0100), stays kept; and the hold on it ends. A take never proven +// keeps it, and says so — a broken take is visible, not silently retired. // TakenTunnel is what an apply says about a tunnel it took over, for the node's report. type TakenTunnel struct { @@ -36,7 +46,9 @@ type TakenTunnel struct { Peers int // State is "not-taken" (the found interface still up, the mesh's not), "taken" (the found one // down and disabled, the mesh's up with its key) or "down" (the found one down and the mesh's - // not up: the peers reach nothing). Note is what this apply did about it. + // not up: the peers reach nothing). Note is what this apply did about it — and, for a taken + // tunnel, whether the take is proven and its found configuration retired (novox/hq ADR 0119). + // Kept is where the found configuration's original is, retired or not. State string Note string Kept string @@ -84,14 +96,35 @@ func takeOver(ctx context.Context, sys system.System, svc *declaration.Service, // 1. The configuration, kept like any held file. A synthetic file resource stands for it, so // the same code keeps its original, digests it and notices it changing. + // + // **Unless it was retired** (novox/hq ADR 0119): the take was proven and the mesh removed + // it, so there is nothing to hold and nothing missing — only where its original is, which + // the retirement recorded. A hold still standing is let go: that is what retiring it meant. + // One put back at its path by a person is on the machine again, with no hold, and is found + // and kept afresh — the same content kept once, as any original is — and retired again by + // the first apply that finds the take still proven. file := &declaration.File{ID: id, Type: declaration.TypeFile, Path: t.Config} - was, already := known.HeldAt(id) - out, held, err := hold(ctx, sys, file, module, was, already, - "the configuration of the tunnel "+t.Interface+", taken over by "+svc.Unit, run, keep, now) - if err != nil { - return begin(file), facts, false, fmt.Errorf("keeping the found tunnel's configuration: %w", err) + var held store.Held + retired, wasRetired := known.RetiredAt(t.Config) + if wasRetired && !present(t.Config) { + known.Release(id) + held = store.Held{Kept: retired.Kept} + out = begin(file) + out.Action = "unchanged" + facts.Note = "the found configuration " + t.Config + " was retired once the take was proven; " + + "its original is kept at " + retired.Kept + " and the mesh never brings it back" + } else { + if wasRetired { + known.Unretire(t.Config) + } + was, already := known.HeldAt(id) + out, held, err = hold(ctx, sys, file, module, was, already, + "the configuration of the tunnel "+t.Interface+", taken over by "+svc.Unit, run, keep, now) + if err != nil { + return begin(file), facts, false, fmt.Errorf("keeping the found tunnel's configuration: %w", err) + } + known.RecordHeld(held) } - known.RecordHeld(held) facts.Kept = held.Kept // What the file says, for the report: from the machine, or from the kept original when the // machine's copy is gone. The private key stays in the file; nothing here keeps it. @@ -185,6 +218,9 @@ func takeOver(ctx context.Context, sys system.System, svc *declaration.Service, } out.Detail = "the tunnel " + t.Interface + "'s configuration, kept as found" + if wasRetired && out.Action == "unchanged" { + out.Detail = "the tunnel " + t.Interface + "'s configuration, retired once the take was proven" + } if held.Kept != "" { out.Detail += " (original at " + held.Kept + ")" } @@ -335,6 +371,124 @@ func restoreFound(ctx context.Context, sys system.System, unit string, run Runne facts.Note += "; " + unit + " was started again, so the machine has the tunnel it had" } +// retireFound removes the found tunnel's configuration from where its unit reads it, once the take +// is proven, and ends the hold on it (novox/hq ADR 0119). Asked after the mesh's service applied +// and the tunnel reads as taken; retired says whether this apply retired it, and out is then what +// replaces the take's outcome for the configuration. +// +// **Proven is taken and a handshake.** Taken alone — the found unit down and disabled, the mesh's +// interface up — says the mesh's interface exists, not that any peer reaches it: an interface up +// with the wrong key is taken and carries nothing. A peer that has completed a handshake with it +// has checked its key, so that is the proof, asked of the kernel through `wg`. Anything short of +// one — no peer yet, every time zero, `wg` missing or failing — keeps the file, and the account +// says which: a take that never proves itself is visible rather than silently retired. +// +// **The original must still be kept.** It is the record of what the predecessor was and a +// person's only way back (ADR 0100); a kept copy that has gone missing is said, and the file is +// not removed, since removing it then would lose the only copy. What is on disk now, if something +// other than the mesh rewrote it since it was found, is kept too before it goes — by content, so +// the first original is never overwritten. +// +// The found unit is left disabled; without its configuration it cannot raise the interface, so +// every later apply's check of it finds nothing to do. Nothing here ever writes the file back. +func retireFound(ctx context.Context, svc *declaration.Service, known *store.State, run Runner, keep Keep, + facts *TakenTunnel, now time.Time) (out Outcome, retired bool) { + t := svc.TakesOver + id := takeOverID(svc) + if facts.State != Taken { + return out, false + } + held, isHeld := known.HeldAt(id) + if !isHeld { + // Retired already (takeOver let any hold go and said so), or never held: nothing to do. + return out, false + } + say := func(note string) { + if facts.Note != "" { + facts.Note += "; " + } + facts.Note += note + } + mesh := strings.TrimPrefix(svc.Unit, "wg-quick@") + peers, err := tunnel.Handshaken(ctx, tunnel.Runner(run), mesh) + if err != nil { + say("taken, not yet proven: " + err.Error() + "; the found configuration " + t.Config + " is kept") + return out, false + } + if peers == 0 { + say("taken, not yet proven: no peer has handshaken on " + mesh + "; the found configuration " + + t.Config + " is kept") + return out, false + } + proven := fmt.Sprintf("proven: %d peer(s) handshaken on %s", peers, mesh) + + // The kept original, read back — not just named in a record. + if held.Kept == "" { + say(proven + ", and the found configuration " + t.Config + " is not retired: no original of it " + + "was kept, so removing it would leave no record of what the predecessor was") + return out, false + } + original, err := os.ReadFile(held.Kept) + if err != nil || (held.Digest != "" && digestOf(string(original)) != held.Digest) { + why := "is missing" + if err == nil { + why = "no longer holds what was found" + } else if !errors.Is(err, os.ErrNotExist) { + why = "cannot be read (" + err.Error() + ")" + } + say(proven + ", and the found configuration " + t.Config + " is not retired: its kept original " + + held.Kept + " " + why + ", so removing it would lose the only copy") + return out, false + } + + gone := !present(t.Config) + if !gone { + current, err := os.ReadFile(t.Config) + if err != nil { + say(proven + ", and the found configuration " + t.Config + " is not retired: it cannot be read (" + + err.Error() + ")") + return out, false + } + if held.Digest != "" && digestOf(string(current)) != held.Digest { + if keep == nil { + say(proven + ", and the found configuration " + t.Config + " is not retired: it was rewritten " + + "since it was found and this host has nowhere to keep what it holds now") + return out, false + } + if _, err := keep(t.Config, current, 0o600); err != nil { + say(proven + ", and the found configuration " + t.Config + " is not retired: keeping what it " + + "holds now failed (" + err.Error() + ")") + return out, false + } + } + if err := os.Remove(t.Config); err != nil && !errors.Is(err, os.ErrNotExist) { + say(proven + ", and the found configuration " + t.Config + " could not be removed (" + err.Error() + ")") + return out, false + } + if present(t.Config) { + say(proven + ", and the found configuration " + t.Config + " is still there after it was removed") + return out, false + } + } + + known.RecordRetired(store.Retired{ID: id, Path: t.Config, Kept: held.Kept, At: now}) + known.Release(id) + facts.Kept = held.Kept + say(proven + "; the found configuration " + t.Config + " is retired — its original kept at " + + held.Kept + ", " + t.Unit + " left disabled, and the mesh never brings it back") + + out = Outcome{ID: id, Type: string(declaration.TypeFile), Target: t.Config, Action: "removed", + Detail: "retired: the take of " + t.Interface + " is " + proven + "; original kept at " + held.Kept} + if gone { + // Already gone — removed by something other than the mesh, or by an apply whose record was + // never saved. Nothing removed here; the hold ends all the same. + out.Action = "unchanged" + out.Detail = "retired: the take of " + t.Interface + " is " + proven + " and " + t.Config + + " was already gone; original kept at " + held.Kept + } + return out, true +} + // takesOver is the service in a declaration that takes over a tunnel, if any: one per node, since // a machine has one private network. func takesOver(d *declaration.Declaration) *declaration.Service { diff --git a/internal/apply/takeover_test.go b/internal/apply/takeover_test.go index 18fde16..058c1d2 100644 --- a/internal/apply/takeover_test.go +++ b/internal/apply/takeover_test.go @@ -4,6 +4,7 @@ import ( "crypto/ecdh" "crypto/rand" "encoding/base64" + "errors" "os" "path/filepath" "strings" @@ -77,7 +78,10 @@ func TestTheFoundTunnelIsStoppedNeverFlushedAndItsConfigurationKept(t *testing.T t.Fatalf("the found unit was not stopped and disabled: %+v", u) } for _, asked := range m.asked { - if strings.HasPrefix(asked, "wg ") && !strings.HasPrefix(asked, "wg show interfaces") { + // Only ever asked about: which interfaces are up, and whether a peer has handshaken with + // the mesh's own (novox/hq ADR 0119). + if strings.HasPrefix(asked, "wg ") && !strings.HasPrefix(asked, "wg show interfaces") && + asked != "wg show mesh0 latest-handshakes" { t.Errorf("the found interface was touched with %q; it is stopped, never flushed", asked) } if strings.HasPrefix(asked, "wg-quick") || strings.Contains(asked, "peer remove") { @@ -254,3 +258,254 @@ func TestATakeoverIsRefusedOnAConvergedDeclaration(t *testing.T) { t.Fatalf("a takeover on a converged node was accepted: %v", err) } } + +// novox/hq ADR 0119: once the take is proven — taken, and a peer handshaken on the mesh's +// interface — the found configuration is removed from where its unit reads it, its original stays +// kept and the hold on it ends. Never before, and never brought back. + +const takesOverID = "mesh-wireguard.overlay-up.takes-over" + +// handshaken is `wg show mesh0 latest-handshakes` with one of the two peers through. +const handshaken = "PEER-A=\t1790000000\nPEER-B=\t0\n" + +func TestAProvenTakeRetiresTheFoundConfiguration(t *testing.T) { + dir, config, mesh, keyFile, m := aHubInUse(t) + m.handshakes = handshaken + report, state := applyAdopted(t, aTakeover(t, config, mesh, keyFile, "51900", "192.0.2.1"), store.State{}, m, dir) + + if _, err := os.Lstat(config); !os.IsNotExist(err) { + t.Fatalf("a proven take left the found configuration where its unit reads it: %v", err) + } + retired, ok := state.RetiredAt(config) + if !ok || retired.Kept == "" || retired.ID != takesOverID { + t.Fatalf("the retirement was not recorded: %+v", state.Retired) + } + if kept, _ := os.ReadFile(retired.Kept); string(kept) != foundConf { + t.Fatalf("the kept original did not survive the retirement: %q", kept) + } + if _, held := state.HeldAt(takesOverID); held { + t.Error("the hold on the found configuration did not end with its retirement") + } + if u := m.units["wg-quick@wg0"]; u.active != "inactive" || u.enabled != "disabled" { + t.Errorf("the found unit is not left down and disabled: %+v", u) + } + if report.Tunnel == nil || report.Tunnel.State != Taken || report.Tunnel.Kept != retired.Kept || + !strings.Contains(report.Tunnel.Note, "proven: 1 peer(s) handshaken on mesh0") || + !strings.Contains(report.Tunnel.Note, "is retired") { + t.Fatalf("the account does not say the take is proven and the configuration retired: %+v", report.Tunnel) + } + if o := outcomeOf(report, takesOverID); o.Action != "removed" || !strings.Contains(o.Detail, "retired") { + t.Errorf("the retirement is not what the apply says it did to the file: %+v", o) + } + // And the account still carries what was found, read from the kept original. + if report.Tunnel.Port != 51900 || report.Tunnel.Peers != 2 { + t.Errorf("the account lost what the tunnel was: %+v", report.Tunnel) + } +} + +func TestATakeNotProvenKeepsTheFoundConfigurationAndSaysSo(t *testing.T) { + cases := map[string]struct { + handshakes string + fail error + says string + }{ + "no peer at all": {"", nil, "no peer has handshaken on mesh0"}, + "every handshake at zero": {"PEER-A=\t0\nPEER-B=\t0\n", nil, "no peer has handshaken on mesh0"}, + "wg is not there": {"", errors.New(`exec: "wg": executable file not found in $PATH`), "executable file not found"}, + "the answer is nonsense": {"unable to access interface\n", nil, "not a peer and a time"}, + } + for name, c := range cases { + dir, config, mesh, keyFile, m := aHubInUse(t) + m.handshakes, m.handshakesFail = c.handshakes, c.fail + d := aTakeover(t, config, mesh, keyFile, "51900", "192.0.2.1") + report, state := applyAdopted(t, d, store.State{}, m, dir) + + if got, _ := os.ReadFile(config); string(got) != foundConf { + t.Fatalf("%s: a take not proven lost the found configuration", name) + } + if _, held := state.HeldAt(takesOverID); !held { + t.Errorf("%s: the hold ended although the take is not proven", name) + } + if _, retired := state.RetiredAt(config); retired { + t.Errorf("%s: recorded as retired", name) + } + if report.Tunnel == nil || report.Tunnel.State != Taken || + !strings.Contains(report.Tunnel.Note, "taken, not yet proven") || + !strings.Contains(report.Tunnel.Note, c.says) || !strings.Contains(report.Tunnel.Note, "is kept") { + t.Errorf("%s: the account does not say the take is not proven and why: %+v", name, report.Tunnel) + } + + // A later apply that finds a peer through retires it: the take itself need not be the one. + m.handshakes, m.handshakesFail = handshaken, nil + _, state = applyAdopted(t, d, state, m, dir) + if _, err := os.Lstat(config); !os.IsNotExist(err) { + t.Errorf("%s: the apply after the take was proven kept the found configuration", name) + } + if _, retired := state.RetiredAt(config); !retired { + t.Errorf("%s: the later retirement was not recorded", name) + } + } +} + +func TestAFoundConfigurationWhoseKeptOriginalIsMissingIsNotRetired(t *testing.T) { + dir, config, mesh, keyFile, m := aHubInUse(t) + d := aTakeover(t, config, mesh, keyFile, "51900", "192.0.2.1") + _, state := applyAdopted(t, d, store.State{}, m, dir) + held, _ := state.HeldAt(takesOverID) + if err := os.Remove(held.Kept); err != nil { + t.Fatal(err) + } + + m.handshakes = handshaken + report, state := applyAdopted(t, d, state, m, dir) + if got, _ := os.ReadFile(config); string(got) != foundConf { + t.Fatal("the found configuration was removed with no kept original left of it") + } + if _, still := state.HeldAt(takesOverID); !still { + t.Error("the hold ended although nothing was retired") + } + if _, retired := state.RetiredAt(config); retired { + t.Error("recorded as retired") + } + if report.Tunnel == nil || !strings.Contains(report.Tunnel.Note, "is not retired") || + !strings.Contains(report.Tunnel.Note, held.Kept+" is missing") { + t.Errorf("the account does not say the kept original is missing: %+v", report.Tunnel) + } +} + +func TestAFoundConfigurationRewrittenSinceItWasFoundIsKeptAgainBeforeItGoes(t *testing.T) { + dir, config, mesh, keyFile, m := aHubInUse(t) + d := aTakeover(t, config, mesh, keyFile, "51900", "192.0.2.1") + _, state := applyAdopted(t, d, store.State{}, m, dir) + rewritten := foundConf + "\n[Peer]\nPublicKey = PEER-C=\nAllowedIPs = 192.0.2.4/32\n" + if err := os.WriteFile(config, []byte(rewritten), 0o600); err != nil { + t.Fatal(err) + } + + m.handshakes = handshaken + _, state = applyAdopted(t, d, state, m, dir) + if _, err := os.Lstat(config); !os.IsNotExist(err) { + t.Fatal("a proven take kept a rewritten configuration") + } + retired, _ := state.RetiredAt(config) + if first, _ := os.ReadFile(retired.Kept); string(first) != foundConf { + t.Errorf("the first original was overwritten: %q", first) + } + kept, _ := filepath.Glob(filepath.Join(dir, "kept", "*-wg0.conf")) + var found bool + for _, k := range kept { + if got, _ := os.ReadFile(k); string(got) == rewritten { + found = true + } + } + if !found { + t.Errorf("what the file held when it was retired was not kept: %v", kept) + } +} + +func TestARetiredTakeIsSteadyAndItsFoundUnitFindsNothingToDo(t *testing.T) { + dir, config, mesh, keyFile, m := aHubInUse(t) + m.handshakes = handshaken + d := aTakeover(t, config, mesh, keyFile, "51900", "192.0.2.1") + _, state := applyAdopted(t, d, store.State{}, m, dir) + retired, _ := state.RetiredAt(config) + + // wg-quick@wg0 with no configuration: inactive, and disabled — the check of it every apply + // makes must find nothing to do and fail on nothing. + m.asked = nil + report, again := applyAdopted(t, d, state, m, dir) + if report.Changed() { + t.Errorf("an apply after the retirement moved the machine: %+v", report.Outcomes) + } + if m.did("systemctl stop wg-quick@wg0") || m.did("systemctl start wg-quick@wg0") { + t.Errorf("the retired tunnel's unit was acted on: %v", m.asked) + } + if _, err := os.Lstat(config); !os.IsNotExist(err) { + t.Error("the found configuration came back") + } + if _, held := again.HeldAt(takesOverID); held { + t.Error("a retired configuration is held again") + } + if r, ok := again.RetiredAt(config); !ok || r != retired { + t.Errorf("the retirement was not kept as it was: %+v", again.Retired) + } + if o := outcomeOf(report, takesOverID); o.Action != "unchanged" || !strings.Contains(o.Detail, "retired") { + t.Errorf("the retired configuration is not said as retired: %+v", o) + } + if report.Tunnel == nil || report.Tunnel.State != Taken || report.Tunnel.Kept != retired.Kept || + !strings.Contains(report.Tunnel.Note, "retired") || report.Tunnel.Port != 51900 { + t.Errorf("the account of a retired take does not say so: %+v", report.Tunnel) + } + + // Enabled at boot again by a person: disabled again, as any take does, and still no error. + m.units["wg-quick@wg0"].enabled = "enabled" + _, _ = applyAdopted(t, d, again, m, dir) + if m.units["wg-quick@wg0"].enabled != "disabled" { + t.Error("the found unit enabled again by hand was left to start at boot") + } +} + +func TestUndeclaringThePrivateNetworkAfterRetirementBringsNothingBack(t *testing.T) { + dir, config, mesh, keyFile, m := aHubInUse(t) + m.handshakes = handshaken + d := aTakeover(t, config, mesh, keyFile, "51900", "192.0.2.1") + _, state := applyAdopted(t, d, store.State{}, m, dir) + + // The private network unassigned: only something else is declared. + other := adopted(t, `{"taken":[],"untaken":{}}`, + `{"id":"other.file","type":"file","path":"`+filepath.Join(dir, "other.conf")+`","content":"x\n"}`) + m.asked = nil + _, after := applyAdopted(t, other, state, m, dir) + if _, err := os.Lstat(config); !os.IsNotExist(err) { + t.Fatal("undeclaring the private network brought the found configuration back") + } + if m.did("systemctl start wg-quick@wg0") || m.did("systemctl enable wg-quick@wg0") { + t.Errorf("undeclaring the private network started the found tunnel: %v", m.asked) + } + if _, ok := after.RetiredAt(config); !ok { + t.Error("the retirement was forgotten with the private network") + } + + // Assigned again, it finds the configuration retired rather than missing, and raises the + // mesh's interface. + report, _ := applyAdopted(t, d, after, m, dir) + if report.Tunnel == nil || report.Tunnel.State != Taken { + t.Errorf("the private network assigned again did not take the tunnel: %+v", report.Tunnel) + } + if _, err := os.Lstat(config); !os.IsNotExist(err) { + t.Error("assigning the private network again brought the found configuration back") + } +} + +func TestAPlanSaysTheFoundConfigurationIsRetiredWhenTheTakeIsProven(t *testing.T) { + dir, config, mesh, keyFile, m := aHubInUse(t) + d := aTakeover(t, config, mesh, keyFile, "51900", "192.0.2.1") + + plan := Plan(d, store.State{}, store.OriginDeclared) + report, state := applyAdopted(t, d, store.State{}, m, dir) + if got, want := strings.Join(ids(plan), " "), strings.Join(outcomeIDs(report), " "); got != want { + t.Errorf("the plan said %q and the apply did %q", got, want) + } + var take Step + for _, s := range plan { + if s.ID == takesOverID { + take = s + } + } + if take.Verb != "hold" || take.Target != config || !strings.Contains(take.Why, "retired — removed from "+config) || + !strings.Contains(take.Why, "handshaking on mesh0") { + t.Errorf("the plan does not say the found configuration is retired once proven: %+v", take) + } + // Held and still declared: never planned as forgotten. + if strings.Contains(verbs(Plan(d, state, store.OriginDeclared)), "forget "+takesOverID) { + t.Error("the plan forgets a hold the apply keeps") + } + + m.handshakes = handshaken + _, state = applyAdopted(t, d, state, m, dir) + for _, s := range Plan(d, state, store.OriginDeclared) { + if s.ID == takesOverID && (s.Verb != "check" || !strings.Contains(s.Why, "retired once the take")) { + t.Errorf("a retired configuration is planned as %+v", s) + } + } +} diff --git a/internal/declaration/declaration.go b/internal/declaration/declaration.go index 5cc738f..d0c6e63 100644 --- a/internal/declaration/declaration.go +++ b/internal/declaration/declaration.go @@ -706,9 +706,10 @@ type Service struct { // TakesOver names the found tunnel this service replaces (novox/hq ADR 0105): before this unit // is started, the named unit is stopped and disabled — never flushed — and its configuration - // file is kept like any held file. Only on an adopted node, and only said by the controller, - // which knows the found tunnel's key is this node's own: without that, starting this unit on - // the found one's port would drop every peer's packets. + // file is kept like any held file — until the take is proven by a peer's handshake, and then + // retired, its original staying kept (novox/hq ADR 0119). Only on an adopted node, and only + // said by the controller, which knows the found tunnel's key is this node's own: without that, + // starting this unit on the found one's port would drop every peer's packets. TakesOver *TakeOver `json:"takes-over,omitempty"` } diff --git a/internal/store/store.go b/internal/store/store.go index 71485a0..87c7b90 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -170,6 +170,28 @@ type State struct { // the bundle carried in the binary is not applied again: what genesis applied was rewritten // for this machine, and the mesh has said more since (novox/hq issue 104). Genesis *Genesis `json:"genesis,omitempty"` + + // Retired is each found tunnel configuration the mesh removed from where its unit reads it, + // once the private network's take of that tunnel was proven (novox/hq ADR 0119). + // + // **Not a hold, and never released with one.** The hold on the found configuration ends at the + // retirement — what it held for has been replaced, and the node stops reporting it — so without + // this the next apply would find no hold and no file and read the take as one whose + // configuration vanished before it could be kept. It is kept whether or not the private + // network stays declared: undeclaring brings nothing back (ADR 0118), and a private network + // assigned again finds the tunnel's configuration retired rather than missing. + Retired []Retired `json:"retired,omitempty"` +} + +// Retired is a found configuration the mesh removed once what replaced it was proven (novox/hq +// ADR 0119): where it was, under which hold it had been kept, and where its original still is. +type Retired struct { + ID string `json:"id"` + Path string `json:"path"` + // Kept is the original as found (novox/hq ADR 0100) — the record of what the predecessor was, + // and a person's way back if one is ever wanted. The mesh never copies it back. + Kept string `json:"kept"` + At time.Time `json:"at"` } // Modes a node can be in (novox/hq ADR 0100). @@ -312,6 +334,42 @@ func (s *State) Release(id string) { } } +// RetiredAt returns the retirement of the found configuration at a path, if the mesh retired one. +func (s State) RetiredAt(path string) (Retired, bool) { + for _, r := range s.Retired { + if r.Path == path { + return r, true + } + } + return Retired{}, false +} + +// RecordRetired adds or replaces the retirement of the configuration at one path. +func (s *State) RecordRetired(r Retired) { + for i, existing := range s.Retired { + if existing.Path == r.Path { + s.Retired[i] = r + return + } + } + s.Retired = append(s.Retired, r) +} + +// Unretire forgets a retirement: the configuration is at its path again, put back by a person, and +// is found — and kept — afresh. +func (s *State) Unretire(path string) { + kept := s.Retired[:0] + for _, r := range s.Retired { + if r.Path != path { + kept = append(kept, r) + } + } + s.Retired = kept + if len(s.Retired) == 0 { + s.Retired = nil + } +} + // Find returns what was applied under an identity. func (s State) Find(id string) (Applied, bool) { for _, r := range s.Resources { diff --git a/internal/tunnel/tunnel.go b/internal/tunnel/tunnel.go index 9e1793d..5590304 100644 --- a/internal/tunnel/tunnel.go +++ b/internal/tunnel/tunnel.go @@ -3,7 +3,9 @@ // // On an adopted node that is the hub, the mesh's interface is raised with the found interface's // private key, on its port, with its address and range, and every peer it had. The found interface -// is stopped, never flushed; its configuration stays on disk. What this package does is the +// is stopped, never flushed; its configuration stays on disk until the take is proven — a peer +// has handshaken with the mesh's interface — and is then retired (novox/hq ADR 0119). What this +// package does is the // reading: which interface is there, what its file says, and what of that travels to the mesh — // everything but the private key, which becomes the node's own overlay key and is stored the way // that key is stored. @@ -151,6 +153,47 @@ func Find(ctx context.Context, run Runner, named string) (Found, error) { return found, nil } +// Handshaken is how many peers of an interface have completed a handshake with it: the proof that +// the interface carries the tunnel, rather than merely being up (novox/hq ADR 0119). +// +// Asked of the running interface, since a handshake is a fact about the kernel's tunnel that no +// file records. A question that cannot be asked — no `wg` on the machine, no such interface, a +// permission refused — is an error and never a zero: "no peer has handshaken" retires nothing +// either, but it is a different thing to tell a person. +func Handshaken(ctx context.Context, run Runner, iface string) (int, error) { + out, err := run(ctx, "wg", "show", iface, "latest-handshakes") + if err != nil { + return 0, fmt.Errorf("cannot ask %s which peers have handshaken: %w", iface, err) + } + return ParseHandshakes(out) +} + +// ParseHandshakes reads `wg show latest-handshakes`: one line per peer, its public key +// and the Unix time of its latest handshake, tab-separated — zero for a peer that never has. What +// is counted is the peers with a time. A line that is not a key and a time is refused rather than +// skipped: output this does not understand is not evidence of anything. +func ParseHandshakes(out string) (int, error) { + n := 0 + for i, line := range strings.Split(out, "\n") { + line = strings.TrimSpace(line) + if line == "" { + continue + } + fields := strings.Fields(line) + if len(fields) != 2 { + return 0, fmt.Errorf("line %d of the handshakes is not a peer and a time: %q", i+1, line) + } + at, err := strconv.ParseInt(fields[1], 10, 64) + if err != nil || at < 0 { + return 0, fmt.Errorf("line %d of the handshakes does not end in a time: %q", i+1, line) + } + if at > 0 { + n++ + } + } + return n, nil +} + func orNone(names []string) string { if len(names) == 0 { return "none" diff --git a/internal/tunnel/tunnel_test.go b/internal/tunnel/tunnel_test.go index 19d73b3..c35ca4b 100644 --- a/internal/tunnel/tunnel_test.go +++ b/internal/tunnel/tunnel_test.go @@ -194,3 +194,49 @@ func TestAFoundTunnelReadsItsMTU(t *testing.T) { t.Fatalf("a config with no MTU must leave it zero; got %d", f2.MTU) } } + +// novox/hq ADR 0119: a take is proven by a handshake on the mesh's interface, read from `wg show +// latest-handshakes` — as wg prints it, a key and a Unix time per peer, zero for never. +func TestAHandshakeIsAPeerWithATime(t *testing.T) { + cases := map[string]struct { + out string + want int + }{ + "two peers, one handshaken": {"PEER-A=\t1790000000\nPEER-B=\t0\n", 1}, + "every peer handshaken": {"PEER-A=\t1790000000\nPEER-B=\t1790000042\n", 2}, + "no peer ever": {"PEER-A=\t0\nPEER-B=\t0\n", 0}, + "an interface with no peer": {"", 0}, + "spaces, a trailing line": {"PEER-A= 1790000000\n\n", 1}, + } + for name, c := range cases { + got, err := ParseHandshakes(c.out) + if err != nil || got != c.want { + t.Errorf("%s: %d peer(s) handshaken (%v), want %d", name, got, err, c.want) + } + } + // Output that is not a key and a time is not evidence of anything, and not a zero either. + for _, nonsense := range []string{"PEER-A=\n", "PEER-A=\tyesterday\n", "PEER-A=\t-1\n", "a b c\n"} { + if _, err := ParseHandshakes(nonsense); err == nil { + t.Errorf("%q was read as handshakes", nonsense) + } + } +} + +func TestHandshakesThatCannotBeAskedAreAnErrorNotAZero(t *testing.T) { + var asked string + ok := func(_ context.Context, name string, args ...string) (string, error) { + asked = name + " " + strings.Join(args, " ") + return "PEER-A=\t1790000000\n", nil + } + if n, err := Handshaken(context.Background(), ok, "mesh0"); err != nil || n != 1 || + asked != "wg show mesh0 latest-handshakes" { + t.Fatalf("asked %q and read %d (%v)", asked, n, err) + } + missing := func(context.Context, string, ...string) (string, error) { + return "", errors.New(`exec: "wg": executable file not found in $PATH`) + } + if _, err := Handshaken(context.Background(), missing, "mesh0"); err == nil || + !strings.Contains(err.Error(), "mesh0") { + t.Fatalf("a machine with no wg was read as one with no handshake: %v", err) + } +} From 50776b86133c23bc27daf501babd0efca83a6966 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 27 Sep 2026 00:55:50 +0200 Subject: [PATCH 11/11] review: a retired configuration that comes back is retired again on its first original, and only wg-quick's own file is ever removed (hq ADR 0119) Put back by hand, it was found afresh and its copy became the hold's original, so a machine that kept restoring it kept growing copies and lost which one was first. The first original now stays the record's, content that differs is kept once beside it, and the note says a rollback means unassigning the private network. Nothing is removed unless it is /.conf, not a path the mesh writes, and not a link, which would leave the key-bearing target behind. --- internal/apply/apply.go | 2 +- internal/apply/takeover.go | 166 +++++++++++++++++++------- internal/apply/takeover_test.go | 204 ++++++++++++++++++++++++++++++++ internal/store/store.go | 33 +++--- 4 files changed, 340 insertions(+), 65 deletions(-) diff --git a/internal/apply/apply.go b/internal/apply/apply.go index af94644..aced7bd 100644 --- a/internal/apply/apply.go +++ b/internal/apply/apply.go @@ -518,7 +518,7 @@ func ApplyKeeping( // configuration is retired — here, after the mesh's service applied, so in the apply // of the take itself only if a peer is already through; otherwise a later apply // retires it (novox/hq ADR 0119). What it did replaces what the take said of the file. - if o, did := retireFound(ctx, svc, &known, run, keep, report.Tunnel, time.Now().UTC()); did { + if o, did := retireFound(ctx, svc, d, &known, run, keep, report.Tunnel, time.Now().UTC()); did { if tookAt >= 0 { report.Outcomes[tookAt] = o } diff --git a/internal/apply/takeover.go b/internal/apply/takeover.go index 8bc7130..0669746 100644 --- a/internal/apply/takeover.go +++ b/internal/apply/takeover.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "os" + "path/filepath" "strings" "time" @@ -100,30 +101,48 @@ func takeOver(ctx context.Context, sys system.System, svc *declaration.Service, // **Unless it was retired** (novox/hq ADR 0119): the take was proven and the mesh removed // it, so there is nothing to hold and nothing missing — only where its original is, which // the retirement recorded. A hold still standing is let go: that is what retiring it meant. - // One put back at its path by a person is on the machine again, with no hold, and is found - // and kept afresh — the same content kept once, as any original is — and retired again by - // the first apply that finds the take still proven. + // + // **One that comes back is held again on its FIRST original** — the one kept before anything + // happened to it (ADR 0100), never whatever was put back — and retired again by the first + // apply that finds the take still proven, keeping what came back only if it differs from + // what is already kept. Put back by hand while the private network is assigned, it is not a + // rollback: that means unassigning the private network first, and the note says so. file := &declaration.File{ID: id, Type: declaration.TypeFile, Path: t.Config} var held store.Held retired, wasRetired := known.RetiredAt(t.Config) - if wasRetired && !present(t.Config) { + cameBack := wasRetired && present(t.Config) + switch { + case wasRetired && !cameBack: known.Release(id) held = store.Held{Kept: retired.Kept} out = begin(file) out.Action = "unchanged" facts.Note = "the found configuration " + t.Config + " was retired once the take was proven; " + "its original is kept at " + retired.Kept + " and the mesh never brings it back" - } else { - if wasRetired { - known.Unretire(t.Config) - } + default: was, already := known.HeldAt(id) - out, held, err = hold(ctx, sys, file, module, was, already, - "the configuration of the tunnel "+t.Interface+", taken over by "+svc.Unit, run, keep, now) + why := "the configuration of the tunnel " + t.Interface + ", taken over by " + svc.Unit + if cameBack && !already { + digest := retired.Digest + if digest == "" { + if raw, err := os.ReadFile(retired.Kept); err == nil { + digest = digestOf(string(raw)) + } + } + was = store.Held{ID: id, Module: module, Kind: string(declaration.TypeFile), Target: t.Config, + Since: now, Why: why, Kept: retired.Kept, Digest: digest} + already = true + } + out, held, err = hold(ctx, sys, file, module, was, already, why, run, keep, now) if err != nil { return begin(file), facts, false, fmt.Errorf("keeping the found tunnel's configuration: %w", err) } known.RecordHeld(held) + if cameBack { + facts.Note = "the found configuration " + t.Config + " came back after it was retired; while the " + + "private network is assigned the mesh retires it again, so rolling back to the found tunnel " + + "means unassigning the private network first" + } } facts.Kept = held.Kept // What the file says, for the report: from the machine, or from the kept original when the @@ -218,7 +237,11 @@ func takeOver(ctx context.Context, sys system.System, svc *declaration.Service, } out.Detail = "the tunnel " + t.Interface + "'s configuration, kept as found" - if wasRetired && out.Action == "unchanged" { + switch { + case cameBack: + out.Detail = "the tunnel " + t.Interface + "'s configuration, back after it was retired; held until " + + "it is retired again" + case wasRetired: out.Detail = "the tunnel " + t.Interface + "'s configuration, retired once the take was proven" } if held.Kept != "" { @@ -371,6 +394,10 @@ func restoreFound(ctx context.Context, sys system.System, unit string, run Runne facts.Note += "; " + unit + " was started again, so the machine has the tunnel it had" } +// wireguardDir is where a found tunnel's configuration may be retired from: wg-quick's own, and +// nowhere else. A variable so a test can hand in a directory. +var wireguardDir = tunnel.ConfigDir + // retireFound removes the found tunnel's configuration from where its unit reads it, once the take // is proven, and ends the hold on it (novox/hq ADR 0119). Asked after the mesh's service applied // and the tunnel reads as taken; retired says whether this apply retired it, and out is then what @@ -379,20 +406,29 @@ func restoreFound(ctx context.Context, sys system.System, unit string, run Runne // **Proven is taken and a handshake.** Taken alone — the found unit down and disabled, the mesh's // interface up — says the mesh's interface exists, not that any peer reaches it: an interface up // with the wrong key is taken and carries nothing. A peer that has completed a handshake with it -// has checked its key, so that is the proof, asked of the kernel through `wg`. Anything short of -// one — no peer yet, every time zero, `wg` missing or failing — keeps the file, and the account -// says which: a take that never proves itself is visible rather than silently retired. +// has checked its key, so that is the proof, asked of the kernel through `wg`. Any handshake counts, +// however old: a change to the mesh's configuration restarts its unit, which recreates the +// interface and resets its counters, so a time that is there at all was made by this interface. +// Anything short of one — no peer yet, every time zero, `wg` missing or failing — keeps the file, +// and the account says which: a take that never proves itself is visible rather than silently +// retired. +// +// **Only what the take names, and only wg-quick's own file.** Nothing is removed unless the path +// is exactly `/.conf`, is not a path the mesh itself writes, and is +// a file rather than a link: removing a link would leave the key-bearing file it points at where it +// is, a retirement in name only, so that one is said and left to a person. // // **The original must still be kept.** It is the record of what the predecessor was and a // person's only way back (ADR 0100); a kept copy that has gone missing is said, and the file is -// not removed, since removing it then would lose the only copy. What is on disk now, if something -// other than the mesh rewrote it since it was found, is kept too before it goes — by content, so -// the first original is never overwritten. +// not removed, since removing it then would lose the only copy. What is on disk now, if it differs +// from the first original and from what was kept at the last retirement, is kept too before it +// goes — by content, so the first original is never overwritten and a file that keeps coming back +// the same keeps nothing more. // // The found unit is left disabled; without its configuration it cannot raise the interface, so // every later apply's check of it finds nothing to do. Nothing here ever writes the file back. -func retireFound(ctx context.Context, svc *declaration.Service, known *store.State, run Runner, keep Keep, - facts *TakenTunnel, now time.Time) (out Outcome, retired bool) { +func retireFound(ctx context.Context, svc *declaration.Service, d *declaration.Declaration, known *store.State, + run Runner, keep Keep, facts *TakenTunnel, now time.Time) (out Outcome, retired bool) { t := svc.TakesOver id := takeOverID(svc) if facts.State != Taken { @@ -409,6 +445,10 @@ func retireFound(ctx context.Context, svc *declaration.Service, known *store.Sta } facts.Note += note } + notRetired := func(why string) (Outcome, bool) { + say("the found configuration " + t.Config + " is not retired: " + why) + return Outcome{}, false + } mesh := strings.TrimPrefix(svc.Unit, "wg-quick@") peers, err := tunnel.Handshaken(ctx, tunnel.Runner(run), mesh) if err != nil { @@ -421,12 +461,26 @@ func retireFound(ctx context.Context, svc *declaration.Service, known *store.Sta return out, false } proven := fmt.Sprintf("proven: %d peer(s) handshaken on %s", peers, mesh) + say(proven) + + // What may be removed at all. + if want := filepath.Join(wireguardDir, t.Interface+".conf"); t.Config != want { + return notRetired("only " + want + ", the found interface's own wg-quick configuration, is ever " + + "retired by the mesh, and the take names " + t.Config) + } + if known.Recorded(string(declaration.TypeFile), t.Config) || declaresFile(d, t.Config) { + return notRetired("it is a path the mesh itself writes") + } + if info, err := os.Lstat(t.Config); err == nil && info.Mode()&os.ModeSymlink != 0 { + target, _ := os.Readlink(t.Config) + return notRetired("it is a link to " + target + "; removing the link would leave the key-bearing file " + + "it points at, so it must be retired by hand — both are kept") + } // The kept original, read back — not just named in a record. if held.Kept == "" { - say(proven + ", and the found configuration " + t.Config + " is not retired: no original of it " + - "was kept, so removing it would leave no record of what the predecessor was") - return out, false + return notRetired("no original of it was kept, so removing it would leave no record of what the " + + "predecessor was") } original, err := os.ReadFile(held.Kept) if err != nil || (held.Digest != "" && digestOf(string(original)) != held.Digest) { @@ -436,49 +490,59 @@ func retireFound(ctx context.Context, svc *declaration.Service, known *store.Sta } else if !errors.Is(err, os.ErrNotExist) { why = "cannot be read (" + err.Error() + ")" } - say(proven + ", and the found configuration " + t.Config + " is not retired: its kept original " + - held.Kept + " " + why + ", so removing it would lose the only copy") - return out, false + return notRetired("its kept original " + held.Kept + " " + why + ", so removing it would lose the only copy") } + before, cameBack := known.RetiredAt(t.Config) + record := store.Retired{ID: id, Path: t.Config, Kept: held.Kept, Digest: digestOf(string(original)), At: now} + if cameBack { + // The first original stays the record's, and so does what the last retirement kept. + record.Extra, record.ExtraDigest, record.Again = before.Extra, before.ExtraDigest, before.Again+1 + } + newCopy := "" gone := !present(t.Config) if !gone { current, err := os.ReadFile(t.Config) if err != nil { - say(proven + ", and the found configuration " + t.Config + " is not retired: it cannot be read (" + - err.Error() + ")") - return out, false + return notRetired("it cannot be read (" + err.Error() + ")") } - if held.Digest != "" && digestOf(string(current)) != held.Digest { + if sum := digestOf(string(current)); sum != record.Digest && sum != record.ExtraDigest { if keep == nil { - say(proven + ", and the found configuration " + t.Config + " is not retired: it was rewritten " + - "since it was found and this host has nowhere to keep what it holds now") - return out, false + return notRetired("it holds something other than its kept original and this host has nowhere " + + "to keep it") } - if _, err := keep(t.Config, current, 0o600); err != nil { - say(proven + ", and the found configuration " + t.Config + " is not retired: keeping what it " + - "holds now failed (" + err.Error() + ")") - return out, false + where, err := keep(t.Config, current, 0o600) + if err != nil { + return notRetired("keeping what it holds now failed (" + err.Error() + ")") } + record.Extra, record.ExtraDigest, newCopy = where, sum, where } if err := os.Remove(t.Config); err != nil && !errors.Is(err, os.ErrNotExist) { - say(proven + ", and the found configuration " + t.Config + " could not be removed (" + err.Error() + ")") - return out, false + return notRetired("removing it failed (" + err.Error() + ")") } if present(t.Config) { - say(proven + ", and the found configuration " + t.Config + " is still there after it was removed") - return out, false + return notRetired("it is still there after it was removed") } } - known.RecordRetired(store.Retired{ID: id, Path: t.Config, Kept: held.Kept, At: now}) + known.RecordRetired(record) known.Release(id) facts.Kept = held.Kept - say(proven + "; the found configuration " + t.Config + " is retired — its original kept at " + - held.Kept + ", " + t.Unit + " left disabled, and the mesh never brings it back") - - out = Outcome{ID: id, Type: string(declaration.TypeFile), Target: t.Config, Action: "removed", - Detail: "retired: the take of " + t.Interface + " is " + proven + "; original kept at " + held.Kept} + copied := "" + if newCopy != "" { + copied = "; what it held, which differed from the original, is kept at " + newCopy + } + out = Outcome{ID: id, Type: string(declaration.TypeFile), Target: t.Config, Action: "removed"} + switch { + case cameBack: + say("the found configuration came back and was retired again — its original still kept at " + + held.Kept + copied + "; rolling back to the found tunnel means unassigning the private network first") + out.Detail = "the found configuration came back and was retired again; original kept at " + held.Kept + copied + default: + say("the found configuration " + t.Config + " is retired — its original kept at " + held.Kept + copied + + ", " + t.Unit + " left disabled, and the mesh never brings it back") + out.Detail = "retired: the take of " + t.Interface + " is " + proven + "; original kept at " + held.Kept + copied + } if gone { // Already gone — removed by something other than the mesh, or by an apply whose record was // never saved. Nothing removed here; the hold ends all the same. @@ -489,6 +553,16 @@ func retireFound(ctx context.Context, svc *declaration.Service, known *store.Sta return out, true } +// declaresFile is whether a declaration writes a file at a path. +func declaresFile(d *declaration.Declaration, path string) bool { + for _, r := range d.Resources { + if f, ok := r.(*declaration.File); ok && filepath.Clean(f.Path) == filepath.Clean(path) { + return true + } + } + return false +} + // takesOver is the service in a declaration that takes over a tunnel, if any: one per node, since // a machine has one private network. func takesOver(d *declaration.Declaration) *declaration.Service { diff --git a/internal/apply/takeover_test.go b/internal/apply/takeover_test.go index 058c1d2..8b6c24e 100644 --- a/internal/apply/takeover_test.go +++ b/internal/apply/takeover_test.go @@ -66,6 +66,10 @@ func aHubInUse(t *testing.T) (dir, config, mesh, keyFile string, m *machine) { "wg-quick@mesh0": {active: "inactive", enabled: "disabled", fragment: "/usr/lib/systemd/system/wg-quick@.service"}, }} takeoverRecheck = 0 + // The found configuration lives in this test's own wireguard directory (novox/hq ADR 0119). + was := wireguardDir + wireguardDir = dir + t.Cleanup(func() { wireguardDir = was }) return dir, config, mesh, keyFile, m } @@ -509,3 +513,203 @@ func TestAPlanSaysTheFoundConfigurationIsRetiredWhenTheTakeIsProven(t *testing.T } } } + +// keptCopies is every copy kept of the found configuration. +func keptCopies(t *testing.T, dir string) []string { + t.Helper() + kept, err := filepath.Glob(filepath.Join(dir, "kept", "*-wg0.conf")) + if err != nil { + t.Fatal(err) + } + return kept +} + +func TestAConfigurationPutBackIsRetiredAgainOnItsFirstOriginalAndSettles(t *testing.T) { + dir, config, mesh, keyFile, m := aHubInUse(t) + m.handshakes = handshaken + d := aTakeover(t, config, mesh, keyFile, "51900", "192.0.2.1") + _, state := applyAdopted(t, d, store.State{}, m, dir) + first, _ := state.RetiredAt(config) + + // Put back by hand with the original, while no peer is through yet: held on the first + // original, and the account says what a rollback takes. + write(t, config, foundConf) + m.handshakes = "" + report, state := applyAdopted(t, d, state, m, dir) + if held, ok := state.HeldAt(takesOverID); !ok || held.Kept != first.Kept { + t.Fatalf("what came back is not held on the first original: %+v", held) + } + if report.Tunnel == nil || !strings.Contains(report.Tunnel.Note, "came back after it was retired") || + !strings.Contains(report.Tunnel.Note, "unassigning the private network first") { + t.Errorf("the account does not say a rollback means unassigning the private network: %+v", report.Tunnel) + } + + // Proven: retired again, nothing more kept, said once. + m.handshakes = handshaken + report, state = applyAdopted(t, d, state, m, dir) + again, _ := state.RetiredAt(config) + if _, err := os.Lstat(config); !os.IsNotExist(err) || again.Kept != first.Kept || again.Extra != "" { + t.Fatalf("put back as it was, it was not retired again on the first original: %+v", again) + } + if o := outcomeOf(report, takesOverID); o.Action != "removed" || + !strings.HasPrefix(o.Detail, "the found configuration came back and was retired again") || + strings.Contains(o.Detail, "differed") { + t.Errorf("the second retirement is not said as one: %+v", o) + } + if !strings.Contains(report.Tunnel.Note, "unassigning the private network first") { + t.Errorf("the account does not say what a rollback takes: %q", report.Tunnel.Note) + } + if n := len(keptCopies(t, dir)); n != 1 { + t.Errorf("%d copies kept of one content", n) + } + if report, _ := applyAdopted(t, d, state, m, dir); report.Changed() { + t.Errorf("a steady machine moved after the second retirement: %+v", report.Outcomes) + } + + // Put back with something else: that is kept beside the first original, which stays the record's. + other := strings.Replace(foundConf, "PEER-B=", "PEER-Z=", 1) + write(t, config, other) + report, state = applyAdopted(t, d, state, m, dir) + third, _ := state.RetiredAt(config) + if third.Kept != first.Kept || third.Extra == "" || third.Extra == first.Kept { + t.Fatalf("other content was not kept apart from the first original: %+v", third) + } + if got, _ := os.ReadFile(third.Extra); string(got) != other { + t.Errorf("the extra copy does not hold what was put back: %q", got) + } + if o := outcomeOf(report, takesOverID); !strings.Contains(o.Detail, third.Extra) || + !strings.Contains(report.Tunnel.Note, third.Extra) { + t.Errorf("where the extra copy is was not said: %+v / %q", o, report.Tunnel.Note) + } + + // And the same other content again: nothing more kept, the record as it was. + write(t, config, other) + report, state = applyAdopted(t, d, state, m, dir) + fourth, _ := state.RetiredAt(config) + if fourth.Kept != first.Kept || fourth.Extra != third.Extra || len(keptCopies(t, dir)) != 2 { + t.Errorf("the same content put back again grew the copies: %+v, %v", fourth, keptCopies(t, dir)) + } + if o := outcomeOf(report, takesOverID); strings.Contains(o.Detail, "differed") { + t.Errorf("a copy already kept was said as new: %+v", o) + } + if report, _ := applyAdopted(t, d, state, m, dir); report.Changed() { + t.Errorf("a steady machine moved: %+v", report.Outcomes) + } +} + +func TestOnlyWgQuicksOwnConfigurationIsRetired(t *testing.T) { + // Not under the wireguard directory. + dir, config, mesh, keyFile, m := aHubInUse(t) + wireguardDir = filepath.Join(dir, "elsewhere") + m.handshakes = handshaken + report, state := applyAdopted(t, aTakeover(t, config, mesh, keyFile, "51900", "192.0.2.1"), store.State{}, m, dir) + if got, _ := os.ReadFile(config); string(got) != foundConf { + t.Fatal("a configuration outside wg-quick's directory was removed") + } + if _, held := state.HeldAt(takesOverID); !held || !strings.Contains(report.Tunnel.Note, "is not retired: only ") { + t.Errorf("the refusal is not said, or the hold ended: %+v", report.Tunnel) + } + + // A path the mesh itself writes. + dir, config, mesh, keyFile, m = aHubInUse(t) + m.handshakes = handshaken + known := store.State{} + known.Record(store.Applied{ID: "bundle.wg0", Type: "file", Target: config, Origin: store.OriginCarried}) + report, _ = applyAdopted(t, aTakeover(t, config, mesh, keyFile, "51900", "192.0.2.1"), known, m, dir) + if got, _ := os.ReadFile(config); string(got) != foundConf { + t.Fatal("a path the mesh writes was retired") + } + if !strings.Contains(report.Tunnel.Note, "a path the mesh itself writes") { + t.Errorf("the refusal is not said: %+v", report.Tunnel) + } +} + +func TestAFoundConfigurationThatIsALinkIsKeptAndLeftToAPerson(t *testing.T) { + dir, config, mesh, keyFile, m := aHubInUse(t) + target := filepath.Join(dir, "predecessor", "hub.conf") + if err := os.MkdirAll(filepath.Dir(target), 0o700); err != nil { + t.Fatal(err) + } + write(t, target, foundConf) + if err := os.Remove(config); err != nil { + t.Fatal(err) + } + if err := os.Symlink(target, config); err != nil { + t.Fatal(err) + } + m.handshakes = handshaken + report, state := applyAdopted(t, aTakeover(t, config, mesh, keyFile, "51900", "192.0.2.1"), store.State{}, m, dir) + if _, err := os.Lstat(config); err != nil { + t.Fatal("the link was removed, leaving the key-bearing file it points at") + } + if got, _ := os.ReadFile(target); string(got) != foundConf { + t.Fatal("the file the link points at was touched") + } + held, ok := state.HeldAt(takesOverID) + if !ok { + t.Fatal("the hold ended") + } + if kept, _ := os.ReadFile(held.Kept); string(kept) != foundConf { + t.Errorf("what was kept is not what the link points at: %q", kept) + } + if !strings.Contains(report.Tunnel.Note, "is a link to "+target) || !strings.Contains(report.Tunnel.Note, "by hand") { + t.Errorf("the account does not say the link must be retired by hand: %q", report.Tunnel.Note) + } +} + +func TestARetirementWhoseRecordWasNeverSavedIsRecordedByTheNextApply(t *testing.T) { + dir, config, mesh, keyFile, m := aHubInUse(t) + d := aTakeover(t, config, mesh, keyFile, "51900", "192.0.2.1") + _, state := applyAdopted(t, d, store.State{}, m, dir) + held, _ := state.HeldAt(takesOverID) + // An apply removed the file and stopped before its state was saved. + if err := os.Remove(config); err != nil { + t.Fatal(err) + } + m.handshakes = handshaken + report, state := applyAdopted(t, d, state, m, dir) + if r, ok := state.RetiredAt(config); !ok || r.Kept != held.Kept { + t.Fatalf("the retirement was not recorded: %+v", state.Retired) + } + if _, still := state.HeldAt(takesOverID); still { + t.Error("the hold did not end") + } + if o := outcomeOf(report, takesOverID); o.Action != "unchanged" || !strings.Contains(o.Detail, "already gone") { + t.Errorf("a file already gone is not said as such: %+v", o) + } +} + +func TestARemovalThatFailsKeepsTheFileAndTheHold(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("root removes from a directory it may not write to") + } + dir, _, mesh, keyFile, m := aHubInUse(t) + wg := filepath.Join(dir, "wireguard") + if err := os.MkdirAll(wg, 0o700); err != nil { + t.Fatal(err) + } + config := filepath.Join(wg, "wg0.conf") + write(t, config, foundConf) + wireguardDir = wg + d := aTakeover(t, config, mesh, keyFile, "51900", "192.0.2.1") + _, state := applyAdopted(t, d, store.State{}, m, dir) + + if err := os.Chmod(wg, 0o500); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Chmod(wg, 0o700) }) + m.handshakes = handshaken + report, state := applyAdopted(t, d, state, m, dir) + if got, _ := os.ReadFile(config); string(got) != foundConf { + t.Fatal("the found configuration is gone although it could not be removed") + } + if _, held := state.HeldAt(takesOverID); !held { + t.Error("the hold ended although nothing was retired") + } + if _, retired := state.RetiredAt(config); retired { + t.Error("recorded as retired") + } + if !strings.Contains(report.Tunnel.Note, "removing it failed") || !strings.Contains(report.Tunnel.Note, "permission denied") { + t.Errorf("the failed removal is not said: %q", report.Tunnel.Note) + } +} diff --git a/internal/store/store.go b/internal/store/store.go index 87c7b90..7306725 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -189,9 +189,21 @@ type Retired struct { ID string `json:"id"` Path string `json:"path"` // Kept is the original as found (novox/hq ADR 0100) — the record of what the predecessor was, - // and a person's way back if one is ever wanted. The mesh never copies it back. - Kept string `json:"kept"` - At time.Time `json:"at"` + // and a person's way back if one is ever wanted. The mesh never copies it back. It is the FIRST + // original, and stays so however often the file comes back: a retirement repeated never moves + // it. Digest is what it holds. + Kept string `json:"kept"` + Digest string `json:"digest,omitempty"` + // Extra is where what was at the path when it was last retired is kept, when that differed from + // the first original — rewritten since it was found, or put back with other content — and + // ExtraDigest what it holds. One copy per distinct content: a file that comes back as it was + // last retired keeps nothing more. + Extra string `json:"extra,omitempty"` + ExtraDigest string `json:"extra_digest,omitempty"` + At time.Time `json:"at"` + // Again is how many times the configuration came back after it was retired, and was retired + // again (novox/hq ADR 0119). + Again int `json:"again,omitempty"` } // Modes a node can be in (novox/hq ADR 0100). @@ -355,21 +367,6 @@ func (s *State) RecordRetired(r Retired) { s.Retired = append(s.Retired, r) } -// Unretire forgets a retirement: the configuration is at its path again, put back by a person, and -// is found — and kept — afresh. -func (s *State) Unretire(path string) { - kept := s.Retired[:0] - for _, r := range s.Retired { - if r.Path != path { - kept = append(kept, r) - } - } - s.Retired = kept - if len(s.Retired) == 0 { - s.Retired = nil - } -} - // Find returns what was applied under an identity. func (s State) Find(id string) (Applied, bool) { for _, r := range s.Resources {