From 7beb752be04298068b4f4903cfa00675b9df29ac Mon Sep 17 00:00:00 2001 From: jochen Date: Tue, 22 Sep 2026 19:57:15 +0200 Subject: [PATCH] Take a key or list member the host may have written itself as the mesh's, so undeclaring gives the file back (hq ADR 0102) --- internal/apply/into.go | 26 ++++++++++++++++------- internal/apply/into_test.go | 42 +++++++++++++++++++++++++++++++++++-- 2 files changed, 59 insertions(+), 9 deletions(-) diff --git a/internal/apply/into.go b/internal/apply/into.go index 4847a21..eefcbeb 100644 --- a/internal/apply/into.go +++ b/internal/apply/into.go @@ -93,7 +93,7 @@ func applyInto(r *declaration.File, previous store.Applied) (Outcome, error) { if !tracked(k) && !had { rec.Absent = append(rec.Absent, k) } - merged, added, err := addMembers(current, declared[k], rec.Added[k]) + merged, added, err := addMembers(current, declared[k], rec.Added[k], previous.Into == nil) if err != nil { return out, fmt.Errorf("%s: %q: %w", r.Path, k, err) } @@ -102,7 +102,14 @@ func applyInto(r *declaration.File, previous store.Applied) (Outcome, error) { continue } if !tracked(k) { - if v, had := object[k]; had { + v, had := object[k] + // **What the host may have written itself is not the machine's.** With no record of + // this file — the first apply, or a host that wrote and died before saving its state — + // a key already holding exactly what the mesh declares cannot be told from one the + // mesh set a moment ago. Remembered as the machine's, it would never be given back: + // undeclaring would leave the mesh's own value behind for ever. So it is the mesh's, + // and undeclaring takes it out (novox/hq ADR 0102). + if had && !(previous.Into == nil && canonical(v) == canonical(declared[k])) { rec.Before[k] = v } else { rec.Absent = append(rec.Absent, k) @@ -320,8 +327,11 @@ func listOf(members []json.RawMessage) json.RawMessage { // addMembers adds the declared members to the machine's list, dropping only members the mesh // added before and no longer declares. It returns the list and exactly which members the mesh // added — a declared member the machine already had is the machine's, and is never recorded. -func addMembers(current, declared json.RawMessage, addedBefore []json.RawMessage) (json.RawMessage, - []json.RawMessage, error) { +// unrecorded says there is no record of this file yet, in which case a declared member already in +// the list may be one the host itself wrote before it could save its state, and is taken as the +// mesh's. +func addMembers(current, declared json.RawMessage, addedBefore []json.RawMessage, + unrecorded bool) (json.RawMessage, []json.RawMessage, error) { have, err := membersOf(current) if err != nil { return nil, nil, err @@ -341,9 +351,11 @@ func addMembers(current, declared json.RawMessage, addedBefore []json.RawMessage for _, m := range want { if !hasMember(have, m) { have = append(have, m) - if !hasMember(added, m) { - added = append(added, m) - } + } else if !unrecorded { + continue // the machine's own, and never the mesh's to take out + } + if !hasMember(added, m) { + added = append(added, m) } } return listOf(have), added, nil diff --git a/internal/apply/into_test.go b/internal/apply/into_test.go index 58a7962..c5f1362 100644 --- a/internal/apply/into_test.go +++ b/internal/apply/into_test.go @@ -217,9 +217,15 @@ func TestAListIsAddedToNeverReplaced(t *testing.T) { // takes back only what it added (novox/hq ADR 0102). path := filepath.Join(t.TempDir(), "daemon.json") _ = os.WriteFile(path, []byte(`{"insecure-registries":["192.0.2.7:5000","10.42.0.9:5000"]}`), 0o644) - // 10.42.0.9 is declared too, and was already the machine's: it is never the mesh's to remove. + // First the mesh's own member alone, so there is a record of this file. + first := parse(t, intoDecl(t, path, `{"insecure-registries":["10.42.0.1:5000"]}`)) + _, state, err := Apply(context.Background(), archHost(t), first, store.State{}, store.OriginDeclared, nil, nil, nil) + if err != nil { + t.Fatal(err) + } + // Now 10.42.0.9 is declared too, and was already the machine's: it is never the mesh's to remove. d := parse(t, intoDecl(t, path, `{"insecure-registries":["10.42.0.1:5000","10.42.0.9:5000"]}`)) - _, state, err := Apply(context.Background(), archHost(t), d, store.State{}, store.OriginDeclared, nil, nil, nil) + _, state, err = Apply(context.Background(), archHost(t), d, state, store.OriginDeclared, nil, nil, nil) if err != nil { t.Fatal(err) } @@ -302,3 +308,35 @@ func TestAHoldFromAWholeFileDoesNotKeepOutAnIntoWrite(t *testing.T) { t.Errorf("the mesh's member was not written in: %v", readObject(t, path)) } } + +func TestAWriteWithNoRecordOfItIsTheMeshsOwn(t *testing.T) { + // A host that wrote into the file and died before saving its state comes back with no record + // of it. What is there is then exactly what the mesh declares — and remembered as the + // machine's it would never be given back (novox/hq ADR 0102). + path := filepath.Join(t.TempDir(), "daemon.json") + _ = os.WriteFile(path, []byte(`{"data-root":"/srv/docker","insecure-registries":["192.0.2.7:5000"]}`), 0o644) + d := parse(t, intoDecl(t, path, `{"live-restore":true,"insecure-registries":["10.42.0.1:5000"]}`)) + if _, _, err := Apply(context.Background(), archHost(t), d, store.State{}, store.OriginDeclared, nil, nil, nil); err != nil { + t.Fatal(err) + } + + // The crash: the state was never saved, so the next apply knows nothing of this file. + _, state, err := Apply(context.Background(), archHost(t), d, store.State{}, store.OriginDeclared, nil, nil, nil) + if err != nil { + t.Fatal(err) + } + rec, _ := state.Find("networking.registry-trust") + if _, asTheMachines := rec.Into.Before["live-restore"]; asTheMachines { + t.Error("the mesh's own key was remembered as the machine's") + } + if _, _, err := Apply(context.Background(), archHost(t), somethingElse(t), state, store.OriginDeclared, nil, nil, nil); err != nil { + t.Fatal(err) + } + o := readObject(t, path) + if _, still := o["live-restore"]; still { + t.Errorf("undeclaring left the mesh's key behind: %v", o) + } + if fmt.Sprint(o["insecure-registries"]) != "[192.0.2.7:5000]" || o["data-root"] != "/srv/docker" { + t.Errorf("the machine did not get its file back: %v", o) + } +}