Merge pull request 'A file the host wrote over is given back when undeclared (hq ADR 0102, 0118)' (#92) from fix/a-file-written-over-is-given-back into main

internal/apply/apply.go

@@ -54,8 +54,9 @@ 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
+	// kept is where this apply kept the original of a file it wrote over (novox/hq ADR 0100), and
+	// keptMode and keptOwner how the original was found, in the form a hold records them.
+	kept, keptMode, keptOwner string
 	// stateless is a service whose unit's lifecycle is the machine's (novox/hq ADR 0117).
 	stateless bool
 	// scope and user are, for a service, whose manager it was applied through (novox/hq ADR 0177).
@@ -505,7 +506,8 @@ func ApplyKeeping(
 			// A file this host has no record of, under any id, is the machine's until the mesh
 			// writes over it — on any node, adopted or not: its original is kept first.
 			var keepFound Keep
-			if f, isFile := resource.(*declaration.File); isFile && was.ID == "" &&
+			// A file whose path moved is a file this host has no record of at its new path.
+			if f, isFile := resource.(*declaration.File); isFile && (was.ID == "" || was.Target != f.Path) &&
 				!known.Recorded(string(declaration.TypeFile), f.Path) {
 				keepFound = keep
 			}
@@ -602,13 +604,16 @@ func ApplyKeeping(
 
 		// 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.
+		// Carried with how the original was found, and only for the path it was kept from: a file
+		// whose path moved leaves its original with the record of the old path (a former target),
+		// and must never be given another path's original when it goes (novox/hq ADR 0118).
 		held, wasHeld := known.HeldAt(resource.Identity())
-		kept := outcome.kept
-		if kept == "" && wasHeld {
-			kept = held.Kept
+		kept, keptMode, keptOwner := outcome.kept, outcome.keptMode, outcome.keptOwner
+		if kept == "" && wasHeld && held.Target == outcome.Target {
+			kept, keptMode, keptOwner = held.Kept, held.Mode, held.Owner
 		}
-		if kept == "" {
-			kept = was.Kept
+		if kept == "" && was.Target == outcome.Target {
+			kept, keptMode, keptOwner = was.Kept, was.KeptMode, was.KeptOwner
 		}
 		// Only now. The record follows the fact, never leads it.
 		known.Record(store.Applied{
@@ -618,6 +623,8 @@ func ApplyKeeping(
 			Wrote:     outcome.wrote,
 			Into:      outcome.into,
 			Kept:      kept,
+			KeptMode:  keptMode,
+			KeptOwner: keptOwner,
 			Reads:     outcome.reads,
 			Stateless: outcome.stateless,
 			Scope:     outcome.scope,
@@ -980,9 +987,13 @@ func applyFile(r *declaration.File, previous store.Applied, unseal Unseal, keepF
 	}
 
 	var beforeMode os.FileMode
+	beforeOwner := ""
 	if existed {
 		if info, err := os.Stat(r.Path); err == nil {
 			beforeMode = info.Mode().Perm()
+			if uid, gid, ok := ownerOf(info); ok {
+				beforeOwner = fmt.Sprintf("%d:%d", uid, gid)
+			}
 		}
 	}
 
@@ -1068,6 +1079,7 @@ func applyFile(r *declaration.File, previous store.Applied, unseal Unseal, keepF
 	}
 	out.kept = kept
 	if kept != "" {
+		out.keptMode, out.keptOwner = fmt.Sprintf("%04o", beforeMode), beforeOwner
 		if out.Detail != "" {
 			out.Detail += "; "
 		}
@@ -1467,13 +1479,7 @@ func remove(ctx context.Context, sys system.System, a store.Applied, run Runner,
 		if a.Into != nil {
 			return removeInto(a)
 		}
-		if err := os.RemoveAll(a.Target); err != nil {
-			return "", "", err
-		}
-		if _, err := os.Stat(a.Target); !errors.Is(err, os.ErrNotExist) {
-			return "", "", fmt.Errorf("%s is still there after removing it", a.Target)
-		}
-		return "removed", "no longer declared", nil
+		return removeWhole(a)
 
 	case declaration.TypeService:
 		return removeService(ctx, sys, a, run, made[unitKey(a.Scope, a.User, a.Target)])
@@ -2633,7 +2639,9 @@ func foundAs(ctx context.Context, sys system.System, r *declaration.Service, run
 // in. Such a unit is the mesh's, whatever was found (novox/hq ADR 0118); see removeService.
 //
 // A drop-in is not the unit's own file, and a file the host wrote over is the machine's unit with
-// the mesh's text in it: its original is kept, and put back when the file's record goes.
+// the mesh's text in it: its original is kept, and put back when the file's record goes — unless
+// the file was changed on the machine since the mesh last wrote it, or the kept copy cannot be
+// read, and then it is left as it stands and the outcome says so (removeWhole, novox/hq ADR 0118).
 //
 // **Keyed by manager and name** (novox/hq ADR 0177): an account's unit and the machine's of the same
 // name are two units, and the mesh writing one says nothing about the other. An account's manager

internal/apply/plan.go

@@ -97,6 +97,10 @@ func Plan(d *declaration.Declaration, known store.State, origin string) []Step {
 			step.Verb, step.Why = "forget", "no longer declared; recorded before the host kept what it unpacked, so it is left in place"
 		case orphan.Type == string(declaration.TypeArchive):
 			step.Why = "no longer declared; the files it unpacked go, and the directories the host made for them once empty"
+		case orphan.Type == string(declaration.TypeFile) && orphan.Into == nil && orphan.Kept != "":
+			// In removeWhole's words (novox/hq ADR 0118): written over, so given back, not deleted.
+			step.Verb, step.Why = "restore", "no longer declared; the original the mesh wrote over goes back "+
+				"from "+orphan.Kept+", unless the file was changed since the mesh last wrote it"
 		case orphan.Type == string(declaration.TypeService):
 			// What removal will do, said before it does it (novox/hq ADR 0118), in removeService's
 			// words. "restore" only where it may stop or disable something — the record cannot say

internal/apply/whole.go

@@ -0,0 +1,113 @@
+package apply
+
+import (
+	"errors"
+	"fmt"
+	"os"
+	"strconv"
+
+	"github.com/novox/mesh-host/internal/store"
+)
+
+// A file written whole, undeclared (novox/hq ADR 0118, ADR 0102).
+//
+// **What the mesh made goes; what it wrote over is given back.** Before the host writes a file over
+// one it has no record of making, it keeps the original first (ADR 0102: "whatever the host writes
+// over without a record of it, it keeps first"). Undeclaring gives a thing back the state it was
+// found in (ADR 0118), so a file with a kept original is not deleted when its record goes: the
+// original is put back, with the mode and owner it was found with. Deleting it was the failure —
+// a module that writes the package manager's configuration whole, unassigned, left the machine with
+// no configuration at all.
+//
+// The cases, decided once and in this order:
+//
+//   - **No kept original** — the mesh made the file where there was none (or the record is from
+//     before the host kept originals, which it cannot tell apart): removed, as before.
+//   - **The file is gone** — somebody removed it: nothing is put back, since bringing back a file a
+//     person deleted is not giving back the state the mesh found; the original stays kept.
+//   - **The file was changed since the mesh last wrote it** — it is somebody's again, as a block or
+//     a JSON file the mesh wrote into stays somebody's: left exactly as it stands, never clobbered,
+//     and the outcome names where the original is so a person can choose.
+//   - **The kept copy cannot be read** — the mesh's file is left in place rather than deleted, and
+//     the outcome says the original is missing.
+//   - Otherwise the original is written back atomically, and the outcome is "restored".
+//
+// **Never fatal.** Each case that leaves the file says so and lets the record go; none stops the
+// rest of an unassignment. The kept copy itself is never deleted (novox/hq ADR 0100).
+func removeWhole(a store.Applied) (string, string, error) {
+	if a.Kept == "" {
+		if err := os.RemoveAll(a.Target); err != nil {
+			return "", "", err
+		}
+		if _, err := os.Stat(a.Target); !errors.Is(err, os.ErrNotExist) {
+			return "", "", fmt.Errorf("%s is still there after removing it", a.Target)
+		}
+		return "removed", "no longer declared", nil
+	}
+
+	current, err := os.ReadFile(a.Target)
+	if errors.Is(err, os.ErrNotExist) {
+		return "forgotten", "no longer there; the original the mesh wrote over stays kept at " + a.Kept, nil
+	}
+	if err != nil {
+		return "kept", fmt.Sprintf("no longer declared, and it cannot be read (%v), so it was left as it "+
+			"is; the original the mesh wrote over is kept at %s", err, a.Kept), nil
+	}
+	if a.Wrote == "" || digestOf(string(current)) != a.Wrote {
+		return "kept", "no longer declared, and changed on the machine since the mesh last wrote it, so " +
+			"it was left as it is; the original the mesh wrote over is kept at " + a.Kept, nil
+	}
+	original, err := os.ReadFile(a.Kept)
+	if err != nil {
+		return "kept", fmt.Sprintf("no longer declared, but the original it was written over cannot be "+
+			"read at %s (%v), so the mesh's file was left in place", a.Kept, err), nil
+	}
+
+	info, err := os.Stat(a.Target)
+	if err != nil {
+		return "kept", fmt.Sprintf("no longer declared, and it cannot be seen (%v), so it was left as it "+
+			"is; the original the mesh wrote over is kept at %s", err, a.Kept), nil
+	}
+	mode := info.Mode().Perm()
+	if a.KeptMode != "" {
+		if m, err := strconv.ParseUint(a.KeptMode, 8, 32); err == nil {
+			mode = os.FileMode(m).Perm()
+		}
+	}
+	if err := writeAtomically(a.Target, original, mode); err != nil {
+		return "kept", fmt.Sprintf("no longer declared, and the original kept at %s could not be put "+
+			"back (%v), so the mesh's file was left in place", a.Kept, err), nil
+	}
+	detail := "no longer declared; the original the mesh wrote over was put back from " + a.Kept
+	if err := giveOwnerBack(a.Target, a.KeptOwner, info); err != nil {
+		detail += "; " + err.Error()
+	}
+	if back, err := os.ReadFile(a.Target); err != nil || string(back) != string(original) {
+		return "kept", "no longer declared; putting back the original kept at " + a.Kept +
+			" did not leave it there — check the file by hand", nil
+	}
+	return "restored", detail, nil
+}
+
+// giveOwnerBack gives a file put back the owner its original was found with — "uid:gid" as a hold
+// records it — or, on a record from before the host kept that, the owner of what it replaced.
+func giveOwnerBack(path, owner string, was os.FileInfo) error {
+	if owner == "" {
+		return keepOwner(path, was)
+	}
+	uid, gid, err := idsOf(owner)
+	if err != nil {
+		return fmt.Errorf("its owner %q could not be read: %w", owner, err)
+	}
+	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("its owner %s could not be given back: %w", owner, err)
+	}
+	return nil
+}

internal/apply/whole_test.go

@@ -0,0 +1,173 @@
+package apply
+
+import (
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/novox/mesh-host/internal/store"
+)
+
+// Defends novox/hq ADR 0118 with ADR 0102: a file the host wrote whole over one it found is given
+// its kept original back when it is undeclared — not deleted, which left a machine whose package
+// manager's configuration a module wrote with no configuration at all once that module was
+// unassigned.
+
+const pacmanFound = "[options]\nArchitecture = auto\n\n[core]\nInclude = /etc/pacman.d/mirrorlist\n"
+const pacmanMesh = "# written by the mesh\n[options]\nArchitecture = auto\nParallelDownloads = 5\n"
+
+// writtenOver is a file found at path with content and mode, then written whole by the mesh.
+func writtenOver(t *testing.T, content string, mode os.FileMode) (path string, state store.State) {
+	t.Helper()
+	path = filepath.Join(t.TempDir(), "pacman.conf")
+	if err := os.WriteFile(path, []byte(content), mode); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.Chmod(path, mode); err != nil {
+		t.Fatal(err)
+	}
+	_, state = applyKeepingIn(t, wholeDecl(path, pacmanMesh), store.State{}, t.TempDir())
+	if got := readText(t, path); got != pacmanMesh {
+		t.Fatalf("the mesh's file was not written: %q", got)
+	}
+	return path, state
+}
+
+func TestAFileWrittenOverGetsItsKeptOriginalBackWhenUndeclared(t *testing.T) {
+	path, state := writtenOver(t, pacmanFound, 0o640)
+	rec, _ := state.Find(namesID)
+	if rec.Kept == "" || rec.KeptMode != "0640" {
+		t.Fatalf("the original and how it was found were not recorded: kept %q, mode %q", rec.Kept, rec.KeptMode)
+	}
+	if steps := Plan(somethingElse(t), state, store.OriginDeclared); !strings.Contains(verbs(steps), "restore "+namesID) {
+		t.Errorf("the plan did not say the original goes back: %s", verbs(steps))
+	}
+	report, after := undeclare(t, state)
+	if got := readText(t, path); got != pacmanFound {
+		t.Fatalf("undeclared, the machine did not get its original back: %q", got)
+	}
+	info, err := os.Stat(path)
+	if err != nil || info.Mode().Perm() != 0o640 {
+		t.Errorf("the original came back with mode %o, it was found 640", info.Mode().Perm())
+	}
+	o := outcomeOf(report, namesID)
+	if o.Action != "restored" || !strings.Contains(o.Detail, rec.Kept) {
+		t.Errorf("the give-back was reported as %q: %s", o.Action, o.Detail)
+	}
+	if _, still := after.Find(namesID); still {
+		t.Error("the record outlived its declaration")
+	}
+	if _, err := os.Stat(rec.Kept); err != nil {
+		t.Errorf("the kept copy went with the give-back: %v", err)
+	}
+}
+
+func TestAFileTheMeshMadeIsRemovedWhenUndeclared(t *testing.T) {
+	path := filepath.Join(t.TempDir(), "pacman.conf")
+	_, state := applyKeepingIn(t, wholeDecl(path, pacmanMesh), store.State{}, t.TempDir())
+	if rec, _ := state.Find(namesID); rec.Kept != "" {
+		t.Fatalf("a file that was not there recorded an original at %s", rec.Kept)
+	}
+	report, _ := undeclare(t, state)
+	if _, err := os.Stat(path); !os.IsNotExist(err) {
+		t.Errorf("a file the mesh made outlived its declaration: %v", err)
+	}
+	if got := outcomeOf(report, namesID).Action; got != "removed" {
+		t.Errorf("removal was reported as %q", got)
+	}
+}
+
+func TestAFileWrittenOverAndChangedSinceIsLeftAsItStands(t *testing.T) {
+	path, state := writtenOver(t, pacmanFound, 0o644)
+	edited := pacmanMesh + "IgnorePkg = linux\n"
+	if err := os.WriteFile(path, []byte(edited), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	report, _ := undeclare(t, state)
+	if got := readText(t, path); got != edited {
+		t.Fatalf("the operator's change was clobbered: %q", got)
+	}
+	rec, _ := state.Find(namesID)
+	o := outcomeOf(report, namesID)
+	if o.Action != "kept" || !strings.Contains(o.Detail, "changed on the machine") || !strings.Contains(o.Detail, rec.Kept) {
+		t.Errorf("leaving it was reported as %q: %s", o.Action, o.Detail)
+	}
+}
+
+func TestAFileWhoseKeptOriginalIsMissingIsLeftAndSaysSo(t *testing.T) {
+	path, state := writtenOver(t, pacmanFound, 0o644)
+	rec, _ := state.Find(namesID)
+	if err := os.Remove(rec.Kept); err != nil {
+		t.Fatal(err)
+	}
+	report, _ := undeclare(t, state)
+	if got := readText(t, path); got != pacmanMesh {
+		t.Fatalf("with no original to put back, the file became %q", got)
+	}
+	o := outcomeOf(report, namesID)
+	if o.Action != "kept" || !strings.Contains(o.Detail, "cannot be read at "+rec.Kept) {
+		t.Errorf("leaving it was reported as %q: %s", o.Action, o.Detail)
+	}
+}
+
+func TestAFileWrittenOverAndDeletedSinceIsNotBroughtBack(t *testing.T) {
+	path, state := writtenOver(t, pacmanFound, 0o644)
+	if err := os.Remove(path); err != nil {
+		t.Fatal(err)
+	}
+	report, _ := undeclare(t, state)
+	if _, err := os.Stat(path); !os.IsNotExist(err) {
+		t.Errorf("a file somebody deleted was brought back: %v", err)
+	}
+	if got := outcomeOf(report, namesID).Action; got != "forgotten" {
+		t.Errorf("reported as %q", got)
+	}
+}
+
+func TestAFileWhosePathMovedIsNeverGivenTheOldPathsOriginal(t *testing.T) {
+	// The old path's original stays with the old path's record; the new path keeps its own.
+	oldPath, state := writtenOver(t, pacmanFound, 0o644)
+	newPath := filepath.Join(filepath.Dir(oldPath), "pacman.d.conf")
+	newFound := "# the machine's own at the new path\n"
+	if err := os.WriteFile(newPath, []byte(newFound), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	_, state = applyKeepingIn(t, wholeDecl(newPath, pacmanMesh), state, t.TempDir())
+	rec, _ := state.Find(namesID)
+	if rec.Kept == "" {
+		t.Fatal("the original at the new path was written over without being kept")
+	}
+	if kept := readText(t, rec.Kept); kept != newFound {
+		t.Fatalf("the new path's record names the wrong original: %q", kept)
+	}
+	// The old path is a former target, given back by the next apply; the new one by undeclaring.
+	undeclare(t, state)
+	if got := readText(t, newPath); got != newFound {
+		t.Errorf("undeclared, the new path holds %q", got)
+	}
+	if got := readText(t, oldPath); got != pacmanFound {
+		t.Errorf("the old path did not get its own original back: %q", got)
+	}
+}
+
+func TestARecordFromBeforeTheModeWasKeptPutsTheOriginalBackAsTheFileStands(t *testing.T) {
+	path, state := writtenOver(t, pacmanFound, 0o644)
+	for i := range state.Resources {
+		state.Resources[i].KeptMode, state.Resources[i].KeptOwner = "", ""
+	}
+	if err := os.Chmod(path, 0o600); err != nil {
+		t.Fatal(err)
+	}
+	report, _ := undeclare(t, state)
+	if got := readText(t, path); got != pacmanFound {
+		t.Fatalf("got %q", got)
+	}
+	info, _ := os.Stat(path)
+	if info.Mode().Perm() != 0o600 {
+		t.Errorf("mode %o, the file stood at 600", info.Mode().Perm())
+	}
+	if got := outcomeOf(report, namesID).Action; got != "restored" {
+		t.Errorf("reported as %q", got)
+	}
+}

internal/store/store.go

@@ -77,6 +77,12 @@ type Applied struct {
 	// (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"`
+	// KeptMode and KeptOwner are the original's mode ("0644") and numeric owner ("0:0") as found,
+	// so undeclaring the file puts the original back as the machine had it (novox/hq ADR 0118).
+	// Absent on a record from before the host kept them; the file's mode and owner as it stands
+	// are used then.
+	KeptMode  string `json:"kept_mode,omitempty"`
+	KeptOwner string `json:"kept_owner,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