Refuse every file once the mesh has spoken, plan the cutover as one, and let the kept declaration repair the mode

Review of the fix for hq issue 104 found three faults in it. A file applied on an enrolled node — the mesh's own last declaration included — is applied as the bundle is, so its resources are recorded as the machine's own and what the mesh declared reads as undeclared: the plan removed the foundation. `apply FILE` is for a machine the mesh has not spoken to, and is now refused saying so whenever declared.json exists. The plan looked at what is held before what the declaration says is taken, so the one cutover ADR 0100 says must be previewed read as a hold; it now decides in holdOnAdopted's order, models a step run inside a held container, and a test holds the plan's sequence to the apply's outcomes. Genesis wrote the mode on every run, so a re-run after `converge` left the state saying adopted while the kept, signed declaration said converged, and the reconcile loop refused every five minutes with no delivery coming to end it: genesis now writes the mode only when none is recorded, and where the state and the verified kept declaration disagree, the kept declaration wins and the repair is said. Also: a file lock beside the state, taken by the link service, the host's own commands and the installer alike, so a `reconcile` run by hand no longer races the loop's save — chosen over refusing while a named service is active, which would miss a `mesh-host run` started by hand; `--json --dry-run` emits {plan} like an apply emits {plan, report}; the README's duplicate flag line; and the bundle refusal is about the digest, not a claim the carried bytes can never match what genesis applied.
2026-09-23 23:35:49 +02:00
parent 27c4b765b2
commit f08a8ea3f7
9 changed files with 441 additions and 74 deletions
@@ -118,18 +118,54 @@ func Plan(d *declaration.Declaration, known store.State, origin string) []Step {
 }

 // planned is what one declared resource would come to.
+//
+// **In the order holdOnAdopted decides it**, because the one cutover ADR 0100 says must be
+// previewed is the one a plan gets backwards if it looks at the record first: a resource this
+// node holds for a module the declaration now says is taken is not held any longer — it is
+// applied, and what was found is replaced. The declaration's word on which modules are untaken
+// comes first; the record of what is held only says what that replacement replaces.
 func planned(r declaration.Resource, d *declaration.Declaration, known store.State) Step {
 	step := Step{Type: string(r.Kind()), ID: r.Identity(), Target: r.Target()}

 	if d.Adoption != nil {
-		if h, held := known.HeldAt(r.Identity()); held {
-			step.Verb, step.Why = "hold", "found on this machine and kept as it is until "+h.Module+" is taken"
-			return step
+		// Something run inside a held container is held with it, while that container's module
+		// is untaken; once the module is taken the container is replaced before this runs.
+		if in := runsIn(r); in != "" {
+			if container, isHeld := heldContainer(known, in); isHeld {
+				if _, untaken := d.Adoption.Untaken[container.Module]; untaken {
+					step.Verb = "hold"
+					step.Why = "runs in " + in + ", which is held as found; not run until " + container.Module + " is taken"
+					return step
+				}
+			}
 		}
-		if module, untaken := d.Adoption.UntakenModuleOf(r.Identity()); untaken {
+		// 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
+		}
+		h, held := known.HeldAt(r.Identity())
+		module, untaken := d.Adoption.UntakenModuleOf(r.Identity())
+		switch {
+		case into:
+		case untaken && held:
+			step.Verb, step.Why = "hold", "found on this machine and kept as it is until "+module+" is taken"
+			return step
+		case untaken:
 			step.Verb = "create"
 			step.Why = "unless it is found on this machine — then held as it is until " + module + " is taken"
 			return step
+		case held:
+			// The cutover: the module is taken, and what was held for it is replaced.
+			step.Verb = "create"
+			if _, recorded := known.Find(r.Identity()); recorded {
+				step.Verb = "update"
+			}
+			step.Why = h.Module + " is taken: replaces what was found and held"
+			if h.Kept != "" {
+				step.Why += "; the original stays at " + h.Kept
+			}
+			return step
 		}
 	}

@@ -1,6 +1,9 @@
 package apply

 import (
+	"context"
+	"os"
+	"path/filepath"
 	"strings"
 	"testing"
 	"time"
@@ -92,3 +95,133 @@ func TestAPlanHoldsWhatAnAdoptedNodeFound(t *testing.T) {
 		t.Errorf("the plan does not say an untaken module's resource is held if found: %q", steps[1].Why)
 	}
 }
+
+// both is a machine with a container runtime and ufw on it at once.
+type both struct {
+	m *machine
+	u *ufwMachine
+}
+
+func (b *both) run(ctx context.Context, name string, args ...string) (string, error) {
+	switch name {
+	case "nft", "ufw", "iptables", "ip6tables", "firewall-cmd":
+		return b.u.run(ctx, name, args...)
+	}
+	return b.m.run(ctx, name, args...)
+}
+
+func ids(steps []Step) []string {
+	var out []string
+	for _, s := range steps {
+		if s.Type == "firewall" {
+			continue // said, not an outcome
+		}
+		out = append(out, s.ID)
+	}
+	return out
+}
+
+func outcomeIDs(r Report) []string {
+	var out []string
+	for _, o := range r.Outcomes {
+		out = append(out, o.ID)
+	}
+	return out
+}
+
+func write(t *testing.T, path, content string) {
+	t.Helper()
+	if err := os.WriteFile(path, []byte(content), 0o644); err != nil {
+		t.Fatal(err)
+	}
+}
+
+// Defends novox/hq issue 104: the plan is the apply, said first — the same resources in the same
+// order, and the one cutover ADR 0100 says must be previewed said as a cutover, not a hold.
+func TestThePlanIsTheApplyInOrder(t *testing.T) {
+	dir := t.TempDir()
+	guard, page, keep, old := filepath.Join(dir, "guard.nft"), filepath.Join(dir, "index.html"),
+		filepath.Join(dir, "keep.html"), filepath.Join(dir, "old.conf")
+	for _, p := range []string{page, keep, old} {
+		write(t, p, "the predecessor's\n")
+	}
+
+	// Returning to adopted, with a guard to raise, an orphan, a hold that stays, a hold whose
+	// module is now taken, and a hold no longer declared.
+	back := adopted(t, `{"taken":["hello-web"],"untaken":{"keep":["keep.page"]}}`,
+		`{"id":"adoption.guard.table","type":"file","path":"`+guard+`","content":"table inet mesh-guard {}\n"},
+		 {"id":"hello-web.page","type":"file","path":"`+page+`","content":"the mesh's page\n"},
+		 {"id":"keep.page","type":"file","path":"`+keep+`","content":"the mesh's keep\n"}`)
+	known := store.State{Firewall: &store.FoundFirewall{Kind: "ufw", WasActive: true, DisabledByMesh: true, FoundAt: time.Now()}}
+	known.Record(store.Applied{ID: "old.conf", Type: "file", Target: old, Origin: store.OriginDeclared})
+	for id, module := range map[string]string{"hello-web.page": "hello-web", "keep.page": "keep", "gone.page": "gone"} {
+		known.RecordHeld(store.Held{ID: id, Module: module, Kind: "file", Target: filepath.Join(dir, id), Since: time.Now()})
+	}
+	fake := &both{m: &machine{containers: map[string]*fakeContainer{}}, u: &ufwMachine{installed: true}}
+
+	plan := Plan(back, known, store.OriginDeclared)
+	report, state, err := ApplyKeeping(context.Background(), archHost(t), back, known, store.OriginDeclared,
+		fake.run, nil, nil, KeepIn(dir))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if got, want := verbs(plan), "enable ufw, forget gone.page, create adoption.guard.table, remove old.conf, "+
+		"create hello-web.page, hold keep.page"; got != want {
+		t.Errorf("planned %q, want %q", got, want)
+	}
+	if got, want := strings.Join(ids(plan), " "), strings.Join(outcomeIDs(report), " "); got != want {
+		t.Errorf("the plan said %q and the apply did %q", got, want)
+	}
+	taken := outcomeOf(report, "hello-web.page")
+	if !strings.Contains(taken.Detail, "taken") || !strings.Contains(plan[4].Why, "hello-web is taken: replaces what was found") {
+		t.Errorf("the cutover was applied as %q and planned as %q", taken.Detail, plan[4].Why)
+	}
+	if !fake.u.active || state.Firewall.DisabledByMesh {
+		t.Error("returning to adopted did not enable ufw again")
+	}
+
+	// Converged, from the mesh: an orphan, a new file, ufw retired, and what protected the node
+	// removed last.
+	flip := parse(t, `{"declaration":1,"resources":[{"id":"a","type":"file","path":"`+filepath.Join(dir, "a.conf")+`","content":"a\n"}]}`)
+	write(t, old, "again\n")
+	known = store.State{Firewall: &store.FoundFirewall{Kind: "ufw", WasActive: true, FoundAt: time.Now()}}
+	known.Record(store.Applied{ID: "adoption.guard.table", Type: "file", Target: guard, Origin: store.OriginDeclared})
+	known.Record(store.Applied{ID: "old.conf", Type: "file", Target: old, Origin: store.OriginDeclared})
+	fake = &both{m: &machine{containers: map[string]*fakeContainer{}}, u: &ufwMachine{installed: true, active: true,
+		ruleset: "table inet mesh {\n}\n"}}
+
+	plan = Plan(flip, known, store.OriginDeclared)
+	report, state, err = ApplyKeeping(context.Background(), archHost(t), flip, known, store.OriginDeclared,
+		fake.run, nil, nil, KeepIn(dir))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if got, want := verbs(plan), "remove old.conf, create a, disable ufw, remove adoption.guard.table"; got != want {
+		t.Errorf("planned %q, want %q", got, want)
+	}
+	if got, want := strings.Join(ids(plan), " "), strings.Join(outcomeIDs(report), " "); got != want {
+		t.Errorf("the plan said %q and the apply did %q", got, want)
+	}
+	if fake.u.active || !state.Firewall.DisabledByMesh {
+		t.Error("converging did not retire ufw")
+	}
+}
+
+func TestAResourceRunInAHeldContainerIsPlannedAsItIsApplied(t *testing.T) {
+	// A run-once step sharing a container's namespace runs in it, as an action's `in` does.
+	img := "example/x@sha256:0000000000000000000000000000000000000000000000000000000000000000"
+	d := parse(t, `{"declaration":1,"adoption":{"taken":["web"],"untaken":{"db":["db.server"]}},"resources":[
+	  {"id":"web.server","type":"container","name":"web","image":"`+img+`"},
+	  {"id":"db.server","type":"container","name":"db","image":"`+img+`"},
+	  {"id":"db.init","type":"container","name":"db-init","image":"`+img+`","run-once":true,"network":"container:db"},
+	  {"id":"web.warm","type":"container","name":"web-warm","image":"`+img+`","run-once":true,"network":"container:web"}]}`)
+	known := store.State{}
+	known.RecordHeld(store.Held{ID: "db.server", Module: "db", Kind: "container", Target: "db", Container: "predecessor"})
+	known.RecordHeld(store.Held{ID: "web.server", Module: "web", Kind: "container", Target: "web", Container: "predecessor"})
+	got := verbs(Plan(d, known, store.OriginDeclared))
+	// db is untaken, so what runs in it waits; web is taken, so its held container is replaced (the
+	// cutover) and what runs in it runs.
+	if got != "create web.server, hold db.server, hold db.init, create web.warm" {
+		t.Errorf("planned %q", got)
+	}
+}