From 3ae999497fa87be9dbeb48a183cd1474abe7c5e6 Mon Sep 17 00:00:00 2001 From: jochen Date: Sat, 26 Sep 2026 23:51:36 +0200 Subject: [PATCH 1/4] 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 2/4] 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 3/4] 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 4/4] 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.