review: a retired configuration that comes back is retired again on its first original, and only wg-quick's own file is ever removed (hq ADR 0119)

Put back by hand, it was found afresh and its copy became the hold's original, so a machine that kept restoring it kept growing copies and lost which one was first. The first original now stays the record's, content that differs is kept once beside it, and the note says a rollback means unassigning the private network. Nothing is removed unless it is <wireguard dir>/<iface>.conf, not a path the mesh writes, and not a link, which would leave the key-bearing target behind.
2026-09-27 00:55:50 +02:00
parent b462f461c6
commit 50776b8613
4 changed files with 340 additions and 65 deletions
@@ -66,6 +66,10 @@ func aHubInUse(t *testing.T) (dir, config, mesh, keyFile string, m *machine) {
 		"wg-quick@mesh0": {active: "inactive", enabled: "disabled", fragment: "/usr/lib/systemd/system/wg-quick@.service"},
 	}}
 	takeoverRecheck = 0
+	// The found configuration lives in this test's own wireguard directory (novox/hq ADR 0119).
+	was := wireguardDir
+	wireguardDir = dir
+	t.Cleanup(func() { wireguardDir = was })
 	return dir, config, mesh, keyFile, m
 }

@@ -509,3 +513,203 @@ func TestAPlanSaysTheFoundConfigurationIsRetiredWhenTheTakeIsProven(t *testing.T
 		}
 	}
 }
+
+// keptCopies is every copy kept of the found configuration.
+func keptCopies(t *testing.T, dir string) []string {
+	t.Helper()
+	kept, err := filepath.Glob(filepath.Join(dir, "kept", "*-wg0.conf"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	return kept
+}
+
+func TestAConfigurationPutBackIsRetiredAgainOnItsFirstOriginalAndSettles(t *testing.T) {
+	dir, config, mesh, keyFile, m := aHubInUse(t)
+	m.handshakes = handshaken
+	d := aTakeover(t, config, mesh, keyFile, "51900", "192.0.2.1")
+	_, state := applyAdopted(t, d, store.State{}, m, dir)
+	first, _ := state.RetiredAt(config)
+
+	// Put back by hand with the original, while no peer is through yet: held on the first
+	// original, and the account says what a rollback takes.
+	write(t, config, foundConf)
+	m.handshakes = ""
+	report, state := applyAdopted(t, d, state, m, dir)
+	if held, ok := state.HeldAt(takesOverID); !ok || held.Kept != first.Kept {
+		t.Fatalf("what came back is not held on the first original: %+v", held)
+	}
+	if report.Tunnel == nil || !strings.Contains(report.Tunnel.Note, "came back after it was retired") ||
+		!strings.Contains(report.Tunnel.Note, "unassigning the private network first") {
+		t.Errorf("the account does not say a rollback means unassigning the private network: %+v", report.Tunnel)
+	}
+
+	// Proven: retired again, nothing more kept, said once.
+	m.handshakes = handshaken
+	report, state = applyAdopted(t, d, state, m, dir)
+	again, _ := state.RetiredAt(config)
+	if _, err := os.Lstat(config); !os.IsNotExist(err) || again.Kept != first.Kept || again.Extra != "" {
+		t.Fatalf("put back as it was, it was not retired again on the first original: %+v", again)
+	}
+	if o := outcomeOf(report, takesOverID); o.Action != "removed" ||
+		!strings.HasPrefix(o.Detail, "the found configuration came back and was retired again") ||
+		strings.Contains(o.Detail, "differed") {
+		t.Errorf("the second retirement is not said as one: %+v", o)
+	}
+	if !strings.Contains(report.Tunnel.Note, "unassigning the private network first") {
+		t.Errorf("the account does not say what a rollback takes: %q", report.Tunnel.Note)
+	}
+	if n := len(keptCopies(t, dir)); n != 1 {
+		t.Errorf("%d copies kept of one content", n)
+	}
+	if report, _ := applyAdopted(t, d, state, m, dir); report.Changed() {
+		t.Errorf("a steady machine moved after the second retirement: %+v", report.Outcomes)
+	}
+
+	// Put back with something else: that is kept beside the first original, which stays the record's.
+	other := strings.Replace(foundConf, "PEER-B=", "PEER-Z=", 1)
+	write(t, config, other)
+	report, state = applyAdopted(t, d, state, m, dir)
+	third, _ := state.RetiredAt(config)
+	if third.Kept != first.Kept || third.Extra == "" || third.Extra == first.Kept {
+		t.Fatalf("other content was not kept apart from the first original: %+v", third)
+	}
+	if got, _ := os.ReadFile(third.Extra); string(got) != other {
+		t.Errorf("the extra copy does not hold what was put back: %q", got)
+	}
+	if o := outcomeOf(report, takesOverID); !strings.Contains(o.Detail, third.Extra) ||
+		!strings.Contains(report.Tunnel.Note, third.Extra) {
+		t.Errorf("where the extra copy is was not said: %+v / %q", o, report.Tunnel.Note)
+	}
+
+	// And the same other content again: nothing more kept, the record as it was.
+	write(t, config, other)
+	report, state = applyAdopted(t, d, state, m, dir)
+	fourth, _ := state.RetiredAt(config)
+	if fourth.Kept != first.Kept || fourth.Extra != third.Extra || len(keptCopies(t, dir)) != 2 {
+		t.Errorf("the same content put back again grew the copies: %+v, %v", fourth, keptCopies(t, dir))
+	}
+	if o := outcomeOf(report, takesOverID); strings.Contains(o.Detail, "differed") {
+		t.Errorf("a copy already kept was said as new: %+v", o)
+	}
+	if report, _ := applyAdopted(t, d, state, m, dir); report.Changed() {
+		t.Errorf("a steady machine moved: %+v", report.Outcomes)
+	}
+}
+
+func TestOnlyWgQuicksOwnConfigurationIsRetired(t *testing.T) {
+	// Not under the wireguard directory.
+	dir, config, mesh, keyFile, m := aHubInUse(t)
+	wireguardDir = filepath.Join(dir, "elsewhere")
+	m.handshakes = handshaken
+	report, state := applyAdopted(t, aTakeover(t, config, mesh, keyFile, "51900", "192.0.2.1"), store.State{}, m, dir)
+	if got, _ := os.ReadFile(config); string(got) != foundConf {
+		t.Fatal("a configuration outside wg-quick's directory was removed")
+	}
+	if _, held := state.HeldAt(takesOverID); !held || !strings.Contains(report.Tunnel.Note, "is not retired: only ") {
+		t.Errorf("the refusal is not said, or the hold ended: %+v", report.Tunnel)
+	}
+
+	// A path the mesh itself writes.
+	dir, config, mesh, keyFile, m = aHubInUse(t)
+	m.handshakes = handshaken
+	known := store.State{}
+	known.Record(store.Applied{ID: "bundle.wg0", Type: "file", Target: config, Origin: store.OriginCarried})
+	report, _ = applyAdopted(t, aTakeover(t, config, mesh, keyFile, "51900", "192.0.2.1"), known, m, dir)
+	if got, _ := os.ReadFile(config); string(got) != foundConf {
+		t.Fatal("a path the mesh writes was retired")
+	}
+	if !strings.Contains(report.Tunnel.Note, "a path the mesh itself writes") {
+		t.Errorf("the refusal is not said: %+v", report.Tunnel)
+	}
+}
+
+func TestAFoundConfigurationThatIsALinkIsKeptAndLeftToAPerson(t *testing.T) {
+	dir, config, mesh, keyFile, m := aHubInUse(t)
+	target := filepath.Join(dir, "predecessor", "hub.conf")
+	if err := os.MkdirAll(filepath.Dir(target), 0o700); err != nil {
+		t.Fatal(err)
+	}
+	write(t, target, foundConf)
+	if err := os.Remove(config); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.Symlink(target, config); err != nil {
+		t.Fatal(err)
+	}
+	m.handshakes = handshaken
+	report, state := applyAdopted(t, aTakeover(t, config, mesh, keyFile, "51900", "192.0.2.1"), store.State{}, m, dir)
+	if _, err := os.Lstat(config); err != nil {
+		t.Fatal("the link was removed, leaving the key-bearing file it points at")
+	}
+	if got, _ := os.ReadFile(target); string(got) != foundConf {
+		t.Fatal("the file the link points at was touched")
+	}
+	held, ok := state.HeldAt(takesOverID)
+	if !ok {
+		t.Fatal("the hold ended")
+	}
+	if kept, _ := os.ReadFile(held.Kept); string(kept) != foundConf {
+		t.Errorf("what was kept is not what the link points at: %q", kept)
+	}
+	if !strings.Contains(report.Tunnel.Note, "is a link to "+target) || !strings.Contains(report.Tunnel.Note, "by hand") {
+		t.Errorf("the account does not say the link must be retired by hand: %q", report.Tunnel.Note)
+	}
+}
+
+func TestARetirementWhoseRecordWasNeverSavedIsRecordedByTheNextApply(t *testing.T) {
+	dir, config, mesh, keyFile, m := aHubInUse(t)
+	d := aTakeover(t, config, mesh, keyFile, "51900", "192.0.2.1")
+	_, state := applyAdopted(t, d, store.State{}, m, dir)
+	held, _ := state.HeldAt(takesOverID)
+	// An apply removed the file and stopped before its state was saved.
+	if err := os.Remove(config); err != nil {
+		t.Fatal(err)
+	}
+	m.handshakes = handshaken
+	report, state := applyAdopted(t, d, state, m, dir)
+	if r, ok := state.RetiredAt(config); !ok || r.Kept != held.Kept {
+		t.Fatalf("the retirement was not recorded: %+v", state.Retired)
+	}
+	if _, still := state.HeldAt(takesOverID); still {
+		t.Error("the hold did not end")
+	}
+	if o := outcomeOf(report, takesOverID); o.Action != "unchanged" || !strings.Contains(o.Detail, "already gone") {
+		t.Errorf("a file already gone is not said as such: %+v", o)
+	}
+}
+
+func TestARemovalThatFailsKeepsTheFileAndTheHold(t *testing.T) {
+	if os.Geteuid() == 0 {
+		t.Skip("root removes from a directory it may not write to")
+	}
+	dir, _, mesh, keyFile, m := aHubInUse(t)
+	wg := filepath.Join(dir, "wireguard")
+	if err := os.MkdirAll(wg, 0o700); err != nil {
+		t.Fatal(err)
+	}
+	config := filepath.Join(wg, "wg0.conf")
+	write(t, config, foundConf)
+	wireguardDir = wg
+	d := aTakeover(t, config, mesh, keyFile, "51900", "192.0.2.1")
+	_, state := applyAdopted(t, d, store.State{}, m, dir)
+
+	if err := os.Chmod(wg, 0o500); err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(func() { _ = os.Chmod(wg, 0o700) })
+	m.handshakes = handshaken
+	report, state := applyAdopted(t, d, state, m, dir)
+	if got, _ := os.ReadFile(config); string(got) != foundConf {
+		t.Fatal("the found configuration is gone although it could not be removed")
+	}
+	if _, held := state.HeldAt(takesOverID); !held {
+		t.Error("the hold ended although nothing was retired")
+	}
+	if _, retired := state.RetiredAt(config); retired {
+		t.Error("recorded as retired")
+	}
+	if !strings.Contains(report.Tunnel.Note, "removing it failed") || !strings.Contains(report.Tunnel.Note, "permission denied") {
+		t.Errorf("the failed removal is not said: %q", report.Tunnel.Note)
+	}
+}