diff --git a/internal/apply/apply.go b/internal/apply/apply.go index 1f20090..d473f3f 100644 --- a/internal/apply/apply.go +++ b/internal/apply/apply.go @@ -290,6 +290,17 @@ func ApplyKeeping( var failures []*Error for i, resource := range ordered { if !orphansRemoved && i == guardFirst { + // **Only a guard that is up may let the filter go.** Removing the derived filter's + // resources stops its unit, whose stop deletes the mesh's table; if a guard resource + // failed, doing that would leave the node with neither, and the store open until some + // later reconcile gets the guard up (novox/hq ADR 0103). + if len(failures) > 0 { + first := failures[0] + first.Done = report + first.Others = len(failures) - 1 + log(" kept " + guardPrefix + "*: the guard is not up, so what it replaces was left in force") + return report, known, first + } if err := removeOrphans(); err != nil { return report, known, err } diff --git a/internal/apply/opening_test.go b/internal/apply/opening_test.go index a81a317..d14fbcf 100644 --- a/internal/apply/opening_test.go +++ b/internal/apply/opening_test.go @@ -371,6 +371,9 @@ func TestReturningToAdoptedLoadsTheGuardBeforeRemovingTheFilter(t *testing.T) { if _, ok := state.Find("adoption.guard"); !ok { t.Error("the guard applied before the failure was not recorded") } + if _, still := state.Find("nftables.load"); !still { + t.Error("the filter that would not stop was forgotten, so nothing would stop it later") + } } else if err != nil { t.Fatal(err) } @@ -419,3 +422,40 @@ func TestARetiredFirewallRetriedStillPutsBackTheForwardPolicy(t *testing.T) { t.Errorf("the retry did not put the forward policy back: %s, %+v", u.forward, state.Firewall) } } + +func TestAGuardThatFailsLeavesTheDerivedFilterInForce(t *testing.T) { + // Returning to adopted: if the guard cannot be raised, the filter it replaces must not be + // stopped — its stop deletes the mesh's table, and the node would have neither (novox/hq ADR 0103). + dir := t.TempDir() + blocked := filepath.Join(dir, "not-a-directory") + if err := os.WriteFile(blocked, []byte("x"), 0o644); err != nil { + t.Fatal(err) + } + guardFile := `{"id":"adoption.guard","type":"file","path":"` + filepath.Join(blocked, "guard.nft") + + `","content":"table inet mesh_guard {}\n"}` + stopped := false + run := func(_ context.Context, name string, args ...string) (string, error) { + if name != "systemctl" { + return "", &exec.Error{Name: name, Err: exec.ErrNotFound} + } + if args[0] == "show" { + return "LoadState=loaded\nActiveState=active\nType=oneshot\nRemainAfterExit=yes\n", nil + } + if args[0] == "stop" { + stopped = true + } + return "", nil + } + converged := store.State{Resources: []store.Applied{ + {ID: "nftables.load", Type: "service", Target: "mesh-filter.service", Origin: store.OriginDeclared}}} + _, state, err := applyWith(t, adopted(t, `{"taken":[]}`, withConf(dir)+","+guardFile), converged, run) + if err == nil { + t.Fatal("a guard that could not be written reported success") + } + if stopped { + t.Error("the derived filter was stopped though the guard is not up") + } + if _, gone := state.Find("nftables.load"); !gone { + t.Error("the filter was forgotten, so nothing would ever stop it") + } +}