internal/apply/apply.go

@@ -114,7 +114,9 @@ func (e *Error) Unwrap() error { return e.Err }
 // Removal happens FIRST, and the order is not arbitrary. A resource that leaves a declaration
 // while another arrives at the same path is an ordinary rename: removing afterwards would
 // delete the file that had just been written. Removing first risks losing the old state if the
-// apply then fails — a recovery concern, where the other is a correctness one.
+// apply then fails — a recovery concern, where the other is a correctness one. The one exception
+// is what protects an adopted node, the openings and the guard: that goes last, and only when
+// everything else applied (novox/hq ADR 0103).
 func Apply(
 	ctx context.Context,
 	sys system.System,
@@ -159,7 +161,7 @@ func ApplyKeeping(
 		return report, known, &Error{Resource: "the firewall found on this machine", Err: err, Done: report}
 	}
 
-	for _, orphan := range known.Orphans(declared, origin) {
+	removeOrphan := func(orphan store.Applied) error {
 		var action, detail string
 		var err error
 		if declaration.Type(orphan.Type) == declaration.TypeOpening {
@@ -168,7 +170,7 @@ func ApplyKeeping(
 			action, detail, err = remove(ctx, sys, orphan, run)
 		}
 		if err != nil {
-			return report, known, &Error{Resource: orphan.ID, Err: err, Done: report}
+			return &Error{Resource: orphan.ID, Err: err, Done: report}
 		}
 		known.Forget(orphan.ID)
 		report.Outcomes = append(report.Outcomes, Outcome{
@@ -176,6 +178,24 @@ func ApplyKeeping(
 			Action: action, Detail: detail,
 		})
 		log(fmt.Sprintf("  %s %s (%s)", action, orphan.ID, orphan.Target))
+		return nil
+	}
+
+	// **What protects an adopted node goes last** (novox/hq ADR 0103). The openings and the guard
+	// are what keep the mesh reachable through the found firewall and the store unreachable from
+	// outside. When a node is converged they leave the declaration, and removing them first would
+	// leave the store open from the moment the guard stops until the derived filter loads — and
+	// for ever, if the filter then fails. So they are removed only once everything else applied
+	// and the found firewall is retired; if anything failed, they stay, recorded, for the next try.
+	var protecting []store.Applied
+	for _, orphan := range known.Orphans(declared, origin) {
+		if strings.HasPrefix(orphan.ID, declaration.AdoptionPrefix) {
+			protecting = append(protecting, orphan)
+			continue
+		}
+		if err := removeOrphan(orphan); err != nil {
+			return report, known, err
+		}
 	}
 
 	// What moved in this apply, so a service that must reflect a file can be told the file
@@ -321,6 +341,11 @@ func ApplyKeeping(
 		if err := retireFirewall(ctx, d, origin, &known, run, log); err != nil {
 			return report, known, &Error{Resource: "the firewall found on this machine", Err: err, Done: report}
 		}
+		for _, orphan := range protecting {
+			if err := removeOrphan(orphan); err != nil {
+				return report, known, err
+			}
+		}
 	}
 
 	if len(failures) > 0 {

internal/apply/opening_test.go

@@ -151,8 +151,10 @@ func TestConvergingRetiresTheFoundFirewallAndReturningRestoresIt(t *testing.T) {
 	if len(u.rules) != 1 || u.rules[0] != "allow 22/tcp" {
 		t.Errorf("converged: the operator's rules were touched, or the mesh's left: %v", u.rules)
 	}
-	if del, dis := u.index("ufw delete"), u.index("ufw disable"); del < 0 || dis < del {
+	// What protected the adopted node goes last: after the derived filter applied and ufw was
-		t.Errorf("converged: the opening was not removed before ufw was disabled: %v", u.asked)
+	// retired (novox/hq ADR 0103).
+	if del, dis := u.index("ufw delete"), u.index("ufw disable"); dis < 0 || del < dis {
+		t.Errorf("converged: the opening was removed before ufw was retired: %v", u.asked)
 	}
 	for _, a := range u.asked {
 		if strings.Contains(a, "reset") {
@@ -224,3 +226,64 @@ func TestACarriedApplyOnAnAdoptedNodeLeavesItsFirewallInForce(t *testing.T) {
 			u.active, state.Firewall, u.asked)
 	}
 }
+
+func TestAFlipThatFailsKeepsTheGuardAndTheOpenings(t *testing.T) {
+	// Converging a node removes its openings and its guard only once everything else applied and
+	// the found firewall is retired. A flip that fails part-way keeps them, so the store is never
+	// left unguarded behind a filter that did not load (novox/hq ADR 0103).
+	dir := t.TempDir()
+	guard := filepath.Join(dir, "guard.nft")
+	guardFile := `{"id":"adoption.guard","type":"file","path":"` + guard + `","content":"table inet mesh_guard {}\n"}`
+	u := &ufwMachine{installed: true, active: true, rules: []string{"allow 22/tcp"}}
+	_, state, err := applyWith(t, adopted(t, `{"taken":[]}`, busOpening+","+guardFile+","+withConf(dir)),
+		store.State{}, u.run)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// The derived filter cannot be written: its path is under a file.
+	blocked := filepath.Join(dir, "not-a-directory")
+	if err := os.WriteFile(blocked, []byte("x"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	filter := `{"id":"nftables.config","type":"file","path":"` + filepath.Join(blocked, "nftables.conf") + `","content":"table inet mesh {}\n"}`
+	converged := parse(t, `{"declaration":1,"resources":[`+withConf(dir)+`,`+filter+`]}`)
+	u.asked = nil
+	_, state, err = applyWith(t, converged, state, u.run)
+	if err == nil {
+		t.Fatal("the failing flip reported success")
+	}
+	if !u.active || u.index("ufw disable") >= 0 {
+		t.Errorf("the found firewall was retired by a flip that failed: %v", u.asked)
+	}
+	if len(u.rules) != 2 || u.index("ufw delete") >= 0 {
+		t.Errorf("the opening was removed by a flip that failed: %v", u.rules)
+	}
+	if _, statErr := os.Stat(guard); statErr != nil {
+		t.Errorf("the guard was removed by a flip that failed: %v", statErr)
+	}
+	for _, id := range []string{"adoption.guard", "adoption.opening-tcp-5671-incoming"} {
+		if _, ok := state.Find(id); !ok {
+			t.Errorf("%s was forgotten, so the next flip would never remove it", id)
+		}
+	}
+
+	// Fixed, the next flip completes: filter, retire, and only then the guard and the openings.
+	if err := os.Remove(blocked); err != nil {
+		t.Fatal(err)
+	}
+	u.asked = nil
+	_, state, err = applyWith(t, converged, state, u.run)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if u.active || len(u.rules) != 1 {
+		t.Errorf("the completed flip left ufw active %v, rules %v", u.active, u.rules)
+	}
+	if _, statErr := os.Stat(guard); !errors.Is(statErr, os.ErrNotExist) {
+		t.Errorf("the guard outlived the completed flip: %v", statErr)
+	}
+	if _, ok := state.Find("adoption.guard"); ok {
+		t.Error("the guard is still recorded after the completed flip")
+	}
+}