From fcaad7271a8cf0cfcf55f0d9fa79c538697beb5b Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 21 Sep 2026 17:43:24 +0200 Subject: [PATCH 1/4] A failure that repeats is said to be stuck MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The mesh kept one report per machine, replaced, so a resource nothing can ever apply looked like a failure that had just happened, every reconcile interval, for ever. The row now keeps when the current failure began and how many reports in a row have said it — the same outcome, refusal and failed resources; anything different starts again and a clean apply clears it. Three make the machine stuck, and status says so beside the failure, in words and in JSON (novox/hq 04-ISSUES/065, ADR 0090). --- cmd/mesh-controller/readable.go | 6 ++ cmd/mesh-controller/readable_test.go | 28 +++++++ cmd/mesh-controller/status.go | 8 ++ internal/inventory/doing_test.go | 76 +++++++++++++++++++ ...ilure-that-repeats-is-said-to-be-stuck.sql | 15 ++++ internal/inventory/nodes.go | 58 ++++++++++++-- 6 files changed, 183 insertions(+), 8 deletions(-) create mode 100644 internal/inventory/migrations/0025-a-failure-that-repeats-is-said-to-be-stuck.sql diff --git a/cmd/mesh-controller/readable.go b/cmd/mesh-controller/readable.go index 68c8bc6..891bfcd 100644 --- a/cmd/mesh-controller/readable.go +++ b/cmd/mesh-controller/readable.go @@ -75,6 +75,11 @@ type machineDoing struct { } `json:"failed,omitempty"` Applied int `json:"applied"` At time.Time `json:"at"` + // Since is when this same failure was first reported and Times how many reports in a row + // have said it; Stuck is the mesh's word for "enough of them" (novox/hq 04-ISSUES/065). + Since *time.Time `json:"since,omitempty"` + Times int `json:"times"` + Stuck bool `json:"stuck"` } type machineReported struct { @@ -149,6 +154,7 @@ func statusAsJSON(asked answers) ([]byte, error) { for _, d := range wrong { row := machineDoing{ Node: d.Node, Outcome: d.Outcome, Refused: d.Refused, Applied: d.Applied, At: d.At, + Since: d.Since, Times: d.Times, Stuck: d.Stuck(), } for _, f := range d.Failed { row.Failed = append(row.Failed, struct { diff --git a/cmd/mesh-controller/readable_test.go b/cmd/mesh-controller/readable_test.go index fd7dedc..c9d07fa 100644 --- a/cmd/mesh-controller/readable_test.go +++ b/cmd/mesh-controller/readable_test.go @@ -139,3 +139,31 @@ func TestNoSecretIsInWhatABoardReads(t *testing.T) { } } } + +// A failure that has been reported identically enough times is said to be stuck, with when it +// began and how many times — so a board can tell a machine looping on something that will never +// apply from one that failed a minute ago (novox/hq 04-ISSUES/065, ADR 0090). +func TestAMachineFailingTheSameWayIsSaidToBeStuck(t *testing.T) { + began := time.Date(2026, 9, 21, 9, 0, 0, 0, time.UTC) + got := statusOf(t, + []inventory.Doing{ + {Node: "looping", Outcome: inventory.OutcomeFailed, Since: &began, Times: inventory.StuckAfter, + Failed: []inventory.FailedResource{{ID: "img", Error: "no such image"}}}, + {Node: "once", Outcome: inventory.OutcomeFailed, Since: &began, Times: 1, + Failed: []inventory.FailedResource{{ID: "img", Error: "no such image"}}}, + }, + []inventory.Node{{Name: "looping"}, {Name: "once"}}, nil, nil, nil) + + wrong, _ := got["wrong"].([]any) + looping, _ := wrong[0].(map[string]any) + if looping["stuck"] != true || looping["times"] != float64(inventory.StuckAfter) { + t.Fatalf("three identical failures are stuck: %v", looping) + } + if since, _ := looping["since"].(string); !strings.HasPrefix(since, "2026-09-21T09:00:00") { + t.Fatalf("when it began was not carried: %v", looping) + } + once, _ := wrong[1].(map[string]any) + if once["stuck"] != false || once["times"] != float64(1) { + t.Fatalf("one failure is not stuck: %v", once) + } +} diff --git a/cmd/mesh-controller/status.go b/cmd/mesh-controller/status.go index 2ebdd1f..cdc7bc7 100644 --- a/cmd/mesh-controller/status.go +++ b/cmd/mesh-controller/status.go @@ -99,6 +99,14 @@ func statusCommand(ctx context.Context, args []string) error { for _, f := range d.Failed { fmt.Printf(" %-18s %s: %s\n", "", f.ID, firstLine(f.Error)) } + if d.Stuck() { + // Said apart from the failure itself. The host's words say what is wrong; this + // says it is not new — the machine has applied, failed the same way and reported + // so this many times, and will keep doing exactly that until something changes + // (novox/hq 04-ISSUES/065). + fmt.Printf(" %-18s stuck: the same failure %d times since %s — it will not fix itself\n", + "", d.Times, d.Since.Local().Format("2006-01-02 15:04")) + } } fmt.Println() } diff --git a/internal/inventory/doing_test.go b/internal/inventory/doing_test.go index 6ba3f5e..e560fbb 100644 --- a/internal/inventory/doing_test.go +++ b/internal/inventory/doing_test.go @@ -245,3 +245,79 @@ func TestAMachineWithNothingComputedForItIsNotWaiting(t *testing.T) { t.Fatalf("a machine the caller could not work out was reported as waiting: %+v", waiting) } } + +// **A failure that repeats is told apart from one that just happened.** A node re-applies on a +// steady interval and reports each time, so a resource that will never apply arrives as the same +// report over and over — one row, replaced, "failed" at a fresh time — and nothing distinguished +// it from a failure that goes away by itself (novox/hq 04-ISSUES/065). The row now keeps when the +// current failure began and how many reports in a row have said it. +func TestTheSameFailureReportedAgainIsCountedNotRestarted(t *testing.T) { + inv := fresh(t) + ctx := context.Background() + id := nodeNamed(t, inv, "looping") + same := Doing{Outcome: OutcomeFailed, Failed: []FailedResource{{ID: "img", Error: "no such image"}}} + + if err := inv.RecordDoing(ctx, id, same); err != nil { + t.Fatal(err) + } + first, _, err := inv.DoingOf(ctx, "looping") + if err != nil { + t.Fatal(err) + } + if first.Times != 1 || first.Since == nil || first.Stuck() { + t.Fatalf("one failure is one failure, not yet stuck: %+v", first) + } + + for range StuckAfter - 1 { + if err := inv.RecordDoing(ctx, id, same); err != nil { + t.Fatal(err) + } + } + again, _, err := inv.DoingOf(ctx, "looping") + if err != nil { + t.Fatal(err) + } + if again.Times != StuckAfter || !again.Stuck() { + t.Fatalf("the same failure %d times is stuck: %+v", StuckAfter, again) + } + if !again.Since.Equal(*first.Since) { + t.Fatalf("the failure began at %s and the row now says %s", *first.Since, *again.Since) + } + + // A different failure is a new situation, not a longer one. + other := Doing{Outcome: OutcomeFailed, Failed: []FailedResource{{ID: "svc", Error: "unit not found"}}} + if err := inv.RecordDoing(ctx, id, other); err != nil { + t.Fatal(err) + } + changed, _, err := inv.DoingOf(ctx, "looping") + if err != nil { + t.Fatal(err) + } + if changed.Times != 1 || changed.Stuck() || !changed.Since.After(*first.Since) && !changed.Since.Equal(*first.Since) { + t.Fatalf("a new failure starts the count again: %+v", changed) + } + + // And a clean apply clears it: the machine is doing what it was told, since nothing. + if err := inv.RecordDoing(ctx, id, Doing{Outcome: OutcomeApplied, Applied: 2}); err != nil { + t.Fatal(err) + } + fine, _, err := inv.DoingOf(ctx, "looping") + if err != nil { + t.Fatal(err) + } + if fine.Times != 0 || fine.Since != nil || fine.Stuck() { + t.Fatalf("a machine doing what it was told is not stuck: %+v", fine) + } + + // The list of what is wrong carries the count, so `status` can say it. + if err := inv.RecordDoing(ctx, id, same); err != nil { + t.Fatal(err) + } + wrong, err := inv.NotDoingWhatTheyWereTold(ctx) + if err != nil { + t.Fatal(err) + } + if len(wrong) != 1 || wrong[0].Times != 1 || wrong[0].Since == nil { + t.Fatalf("got %+v", wrong) + } +} diff --git a/internal/inventory/migrations/0025-a-failure-that-repeats-is-said-to-be-stuck.sql b/internal/inventory/migrations/0025-a-failure-that-repeats-is-said-to-be-stuck.sql new file mode 100644 index 0000000..266ea5b --- /dev/null +++ b/internal/inventory/migrations/0025-a-failure-that-repeats-is-said-to-be-stuck.sql @@ -0,0 +1,15 @@ +-- A failure that repeats is told apart from one that just happened (novox/hq 04-ISSUES/065). +-- +-- A node re-applies what it holds on a steady interval and reports each time (novox/hq ADR 0010), +-- which is right for a failure that goes away by itself -- the overlay not up yet, a registry +-- briefly unreachable -- and makes a failure that will never go away look exactly the same: one +-- row, replaced, saying "failed" at a fresh time. Nothing distinguished "failed once, will succeed +-- when its dependency arrives" from "failed identically for ever", and nothing escalated the second. +-- +-- Still one row per node. What is added is how long the CURRENT failure has been the same one: +-- when it first appeared, and how many reports in a row have said it. A report that says something +-- different starts the count again; a clean apply clears it. + +alter table node_report + add column failing_since timestamptz, + add column failures int not null default 0; diff --git a/internal/inventory/nodes.go b/internal/inventory/nodes.go index ed6d60b..6729cb5 100644 --- a/internal/inventory/nodes.go +++ b/internal/inventory/nodes.go @@ -466,6 +466,15 @@ type Doing struct { // Declared is the digest of the declaration the report was about; empty when the machine // did not say. Declared string + + // Since is when the machine first reported THIS failure — the same outcome, the same refusal, + // the same failed resources — and Times is how many reports in a row have said it. A node + // re-applies on a steady interval and reports each time (novox/hq ADR 0010), so a failure + // that will never succeed arrives as the same report over and over, indistinguishable from + // one that just happened until somebody counts (novox/hq 04-ISSUES/065). Nil and zero for a + // machine doing what it was told. + Since *time.Time + Times int } // FailedResource is one thing a node could not do. @@ -477,23 +486,54 @@ type FailedResource struct { // Wrong reports whether this machine needs somebody to look at it. func (d Doing) Wrong() bool { return d.Outcome != OutcomeApplied } +// StuckAfter is how many identical reports in a row make a failure one that will not fix itself. +// +// Three, because a node reports after every apply and applies on its reconcile interval: one +// failure is an event, two may be the same event still under way, three separate applies saying +// the same words is a machine looping on something that is not going to change (novox/hq +// 04-ISSUES/065). Not a duration: a laptop that was shut for a week has had one attempt. +const StuckAfter = 3 + +// Stuck reports whether this machine has been failing the same way for long enough that waiting +// is no longer a plan. The failure is still the host's own words; this only says it is not new. +func (d Doing) Stuck() bool { return d.Wrong() && d.Times >= StuckAfter } + // RecordDoing keeps what a node said it did. // // One row per node, replaced. The question is the machine's current state — "this failed an hour // ago and then succeeded" is not something anybody needs to look at, and a table of every report // would bury the ones that matter. +// +// **What the row also keeps is whether this failure is the one before.** The same outcome, the +// same refusal, the same failed resources with the same words: then the failure did not just +// happen, it is still happening, and the row keeps when it began and counts one more report. Any +// difference starts again — a machine failing on a new resource is a new situation, not a longer +// one — and a clean apply clears both (novox/hq 04-ISSUES/065). func (i *Inventory) RecordDoing(ctx context.Context, node string, d Doing) error { failed, err := json.Marshal(d.Failed) if err != nil { return err } _, err = i.store.Pool().Exec(ctx, - `insert into node_report (node, outcome, refused, failed, applied, at, declared) - values ($1, $2, $3, $4, $5, now(), $6) + `insert into node_report (node, outcome, refused, failed, applied, at, declared, + failing_since, failures) + values ($1, $2, $3, $4, $5, now(), $6, + case when $2 <> $7 then now() end, + case when $2 <> $7 then 1 else 0 end) on conflict (node) do update set outcome = excluded.outcome, refused = excluded.refused, failed = excluded.failed, applied = excluded.applied, at = excluded.at, - declared = excluded.declared`, - node, d.Outcome, d.Refused, failed, d.Applied, d.Declared) + declared = excluded.declared, + failing_since = case + when excluded.outcome = $7 then null + when node_report.outcome = excluded.outcome and node_report.refused = excluded.refused + and node_report.failed = excluded.failed then node_report.failing_since + else excluded.at end, + failures = case + when excluded.outcome = $7 then 0 + when node_report.outcome = excluded.outcome and node_report.refused = excluded.refused + and node_report.failed = excluded.failed then node_report.failures + 1 + else 1 end`, + node, d.Outcome, d.Refused, failed, d.Applied, d.Declared, OutcomeApplied) return err } @@ -505,7 +545,7 @@ func (i *Inventory) RecordDoing(ctx context.Context, node string, d Doing) error // from. func (i *Inventory) NotDoingWhatTheyWereTold(ctx context.Context) ([]Doing, error) { rows, err := i.store.Pool().Query(ctx, - `select n.name, r.outcome, r.refused, r.failed, r.applied, r.at + `select n.name, r.outcome, r.refused, r.failed, r.applied, r.at, r.failing_since, r.failures from node_report r join node n on n.id = r.node where r.outcome <> $1 order by r.at desc`, OutcomeApplied) if err != nil { @@ -517,7 +557,8 @@ func (i *Inventory) NotDoingWhatTheyWereTold(ctx context.Context) ([]Doing, erro for rows.Next() { var d Doing var failed []byte - if err := rows.Scan(&d.Node, &d.Outcome, &d.Refused, &failed, &d.Applied, &d.At); err != nil { + if err := rows.Scan(&d.Node, &d.Outcome, &d.Refused, &failed, &d.Applied, &d.At, + &d.Since, &d.Times); err != nil { return nil, err } if err := json.Unmarshal(failed, &d.Failed); err != nil { @@ -537,8 +578,9 @@ func (i *Inventory) DoingOf(ctx context.Context, name string) (Doing, bool, erro var d Doing var failed []byte err = i.store.Pool().QueryRow(ctx, - `select outcome, refused, failed, applied, at from node_report where node = $1`, node.ID). - Scan(&d.Outcome, &d.Refused, &failed, &d.Applied, &d.At) + `select outcome, refused, failed, applied, at, failing_since, failures + from node_report where node = $1`, node.ID). + Scan(&d.Outcome, &d.Refused, &failed, &d.Applied, &d.At, &d.Since, &d.Times) if errors.Is(err, pgx.ErrNoRows) { return Doing{}, false, nil } -- 2.54.0 From 537ad544d3a96a9bfeda4a86cc043857a6e3f26d Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 21 Sep 2026 17:47:44 +0200 Subject: [PATCH 2/4] =?UTF-8?q?A=20container=20may=20not=20mount=20a=20pat?= =?UTF-8?q?h=20the=20module=20never=20declared=20=E2=80=94=20and=20three?= =?UTF-8?q?=20things=20declare=20one?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Brought back from 53eb000, withdrawn because it refused the builder's mount of the container runtime's socket. A path is declared as the module's own (a directory or file resource, or where a secret, grant or contribution lands), as the operator's (an accesses entry, ADR 0051), or as the machine's (a facility a declared capability grants: container-runtime grants its socket). Every catalogue manifest passes, and a test says so (novox/hq 04-ISSUES/026, ADR 0091). --- internal/catalogue/manifest.go | 98 +++++++++++++++++++++++++++++++ internal/catalogue/mounts_test.go | 86 +++++++++++++++++++++++++++ 2 files changed, 184 insertions(+) create mode 100644 internal/catalogue/mounts_test.go diff --git a/internal/catalogue/manifest.go b/internal/catalogue/manifest.go index d2096cb..4c82a6f 100644 --- a/internal/catalogue/manifest.go +++ b/internal/catalogue/manifest.go @@ -980,6 +980,25 @@ func ParseManifest(raw []byte) (Manifest, error) { } } + // **A container may not mount a path the module never declared** (novox/hq 04-ISSUES/026, + // ADR 0091). A bind mount whose source does not exist is created by the container runtime, as + // root, with whatever mode it picks — so `owner` and `mode` never reach the directory holding + // the module's data, and the rule that keeps data when a module goes away (ADR 0030) does not + // cover it, because the mesh has never heard of it. + // + // Three things declare a path, and they are the three kinds of thing a path can be: + // - the module's own: a directory or file resource, or where a secret, a grant or a + // contribution lands — created and owned by the mesh for this module; + // - the operator's: an `accesses` entry (ADR 0051) — pre-existing, shared, granted for use; + // - the machine's: a facility a declared capability grants, such as the container runtime's + // socket. It exists, the machine owns it, and declaring it as the module's own directory + // would be a lie the host would act on — which is why the first version of this check was + // withdrawn: it refused the builder. + // + // Checked here rather than on the machine because the machine cannot tell the difference: by + // the time it sees the mount it is being asked to create the directory, which it can do. + problems = append(problems, m.undeclaredMounts()...) + for i, r := range m.Resources { id, _ := r["id"].(string) if id == "" { @@ -1075,3 +1094,82 @@ func (m Manifest) MachineSide(port int) (at int, mayAssign bool) { } return port, false } + +// facilitiesOf is what each capability lets a container mount: paths the machine owns and a module +// is granted the use of by declaring the capability, never by declaring them as its own. +var facilitiesOf = map[string][]string{ + "container-runtime": {"/var/run/docker.sock"}, +} + +// undeclaredMounts is every bind-mount source no declaration covers — see the check above. +func (m Manifest) undeclaredMounts() []string { + declared := map[string]bool{} + claim := func(p string) { + if strings.HasPrefix(p, "/") { + declared[strings.TrimRight(p, "/")] = true + } + } + for _, r := range m.Resources { + switch fmt.Sprint(r["type"]) { + case "directory", "file": + claim(fmt.Sprint(r["path"])) + } + } + for _, where := range m.OwnSecrets { + claim(where) + } + for _, where := range m.Secrets { + claim(where) + } + for _, where := range m.Receives { + claim(where) + } + for _, where := range m.Grants { + claim(where) + } + for _, a := range m.Accesses { + claim(a.Path) + } + for _, c := range m.Capabilities { + for _, p := range facilitiesOf[c] { + claim(p) + } + } + // Under a declared directory is declared: a module that says where its data lives has said so + // for what it puts inside. + covers := func(path string) bool { + for at := path; strings.HasPrefix(at, "/"); { + if declared[at] { + return true + } + cut := strings.LastIndex(at, "/") + if cut <= 0 { + return false + } + at = at[:cut] + } + return false + } + var problems []string + for _, r := range m.Resources { + if fmt.Sprint(r["type"]) != "container" { + continue + } + mounts, _ := r["volumes"].([]any) + for _, v := range mounts { + from, _, _ := strings.Cut(fmt.Sprint(v), ":") + if !strings.HasPrefix(from, "/") || covers(strings.TrimRight(from, "/")) { + continue + } + problems = append(problems, fmt.Sprintf( + "%s mounts %q into %v, and nothing in %s declares it. A bind mount the module did "+ + "not declare is created by the container runtime as root, so the module's own "+ + "owner and mode do not reach the directory that holds its data, and the rule "+ + "that keeps data when a module goes away (novox/hq ADR 0030) does not cover it. "+ + "Declare it: a directory resource if it is the module's, an `accesses` entry if "+ + "it is the operator's, or the capability that grants it if it is the machine's", + m.Module, from, r["id"], m.Module)) + } + } + return problems +} diff --git a/internal/catalogue/mounts_test.go b/internal/catalogue/mounts_test.go new file mode 100644 index 0000000..0e323c5 --- /dev/null +++ b/internal/catalogue/mounts_test.go @@ -0,0 +1,86 @@ +package catalogue + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// A bind mount the module never declared is refused where it is written (novox/hq 04-ISSUES/026, +// ADR 0091). The container runtime creates a missing bind source itself, as root, with a mode it +// picks, so the module's own owner and mode never reach the directory holding its data, and the +// rule that keeps data when a module goes away (ADR 0030) does not cover it. + +const aContainerMounting = `{"id":"server","type":"container","name":"store","image":"x@sha256:` + + `0000000000000000000000000000000000000000000000000000000000000000","volumes":["%s:/inside"]}` + +func manifestMounting(from string, beside string) []byte { + res := aContainerMounting + if beside != "" { + res = beside + "," + res + } + return []byte(`{"module":"store","resources":[` + strings.Replace(res, "%s", from, 1) + `]}`) +} + +func TestAMountNothingDeclaresIsRefused(t *testing.T) { + _, err := ParseManifest(manifestMounting("/services/store/data", "")) + if err == nil { + t.Fatal("a container mounting a path no resource declares was accepted; the runtime " + + "would create it as root and the module's owner and mode would never apply") + } + if !strings.Contains(err.Error(), "/services/store/data") { + t.Fatalf("refused without naming the path, which leaves the author guessing: %v", err) + } +} + +// Declaring it is enough — including declaring the directory above it. +func TestAMountUnderADeclaredDirectoryIsAccepted(t *testing.T) { + _, err := ParseManifest(manifestMounting("/services/store/data", + `{"id":"state","type":"directory","path":"/services/store","mode":"0700"}`)) + if err != nil { + t.Fatalf("a module that said where its data lives was refused anyway: %v", err) + } +} + +// The operator's path, granted for use (ADR 0051), is declared by `accesses`, not by a directory the +// module would then own. +func TestAMountOfAnAccessedPathIsAccepted(t *testing.T) { + _, err := ParseManifest([]byte(`{"module":"store","accesses":[{"path":"/services/media/movies","mode":"read"}],` + + `"resources":[` + strings.Replace(aContainerMounting, "%s", "/services/media/movies", 1) + `]}`)) + if err != nil { + t.Fatalf("a mount of a path the module declares it accesses was refused: %v", err) + } +} + +// The machine's facility — the container runtime's socket — is granted by the capability that names +// it, and declaring it as the module's own directory would be a lie the host would act on. This is +// the case the first version of this check refused, and was withdrawn for: the builder. +func TestTheRuntimeSocketIsGrantedByTheCapabilityAndNotOtherwise(t *testing.T) { + granted := `{"module":"builder","capabilities":["container-runtime"],"resources":[` + + strings.Replace(aContainerMounting, "%s", "/var/run/docker.sock", 1) + `]}` + if _, err := ParseManifest([]byte(granted)); err != nil { + t.Fatalf("a module with the container-runtime capability may mount its socket: %v", err) + } + if _, err := ParseManifest(manifestMounting("/var/run/docker.sock", "")); err == nil { + t.Fatal("a module mounted the container runtime's socket without declaring the capability, and was accepted") + } +} + +// **Every manifest in the catalogue beside this checkout passes**, so the rule is not one the +// catalogue is already breaking. Skipped, aloud, where the catalogue is not there. +func TestEveryCatalogueManifestDeclaresWhatItMounts(t *testing.T) { + files, _ := filepath.Glob("../../../mesh-catalog/modules/*/module.json") + if len(files) == 0 { + t.Skip("the catalogue is not beside this checkout") + } + for _, file := range files { + raw, err := os.ReadFile(file) + if err != nil { + t.Fatal(err) + } + if _, err := ParseManifest(raw); err != nil { + t.Errorf("%s: %v", filepath.Base(filepath.Dir(file)), err) + } + } +} -- 2.54.0 From 396e05bb6511eb3979bfc5f47ba85e7a2df39ce9 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 21 Sep 2026 17:50:41 +0200 Subject: [PATCH 3/4] An operator delivers a pair credential, and the mesh never replaces it secret accept grows --provider: the value is sealed to the consumer's node, the provider's node and the operator's key, and the pair records origin 'accepted'. An accepted pair is not remade when a key changes (the mesh does not hold the value; the read is refused naming the remedy) and rotate refuses it (accepting a new value is the rotation). The vault's third species has its entry (novox/hq 04-ISSUES/070, ADR 0092). --- cmd/mesh-controller/secret.go | 17 +++- ...air-credential-says-where-it-came-from.sql | 14 +++ internal/inventory/secrets.go | 95 ++++++++++++++++++- internal/inventory/secrets_test.go | 66 +++++++++++++ 4 files changed, 188 insertions(+), 4 deletions(-) create mode 100644 internal/inventory/migrations/0026-a-pair-credential-says-where-it-came-from.sql diff --git a/cmd/mesh-controller/secret.go b/cmd/mesh-controller/secret.go index a8eb726..295baaf 100644 --- a/cmd/mesh-controller/secret.go +++ b/cmd/mesh-controller/secret.go @@ -49,6 +49,9 @@ func secretCommand(ctx context.Context, args []string) error { set := flag.NewFlagSet("secret accept", flag.ContinueOnError) from := set.String("from", "", "read the value from this file instead of asking (use - for standard input)") + provider := set.String("provider", "", + "the node providing : the value becomes the PAIR credential between on "+ + "and that provider, sealed to both — the vault's operator-delivered secret (ADR 0092)") if err := set.Parse(flags); err != nil { return err } @@ -72,6 +75,18 @@ func secretCommand(ctx context.Context, args []string) error { } defer open.Close() + if *provider != "" { + // Into the pair, not into the module's own secrets: what the provider is asked to create + // and what the consumer reads are the same value, and neither end can be told a different + // one later without the other (novox/hq 04-ISSUES/070). + if err := open.inventory.AcceptSecretForPair(ctx, name, node, module, *provider, value); err != nil { + return err + } + fmt.Printf("%s on %s now holds %q from %s, sealed to both machines.\n", module, node, name, *provider) + fmt.Printf(" the mesh cannot read it back, will not replace it with one of its own, and will not rotate it\n") + fmt.Printf(" run `push %s` and `push %s` to send it\n", *provider, node) + return nil + } if err := open.inventory.AcceptSecretForModule(ctx, node, module, name, value); err != nil { return err } @@ -83,7 +98,7 @@ func secretCommand(ctx context.Context, args []string) error { return nil } -const secretUsage = "secret accept [--from ]\n" + +const secretUsage = "secret accept [--from ] [--provider ]\n" + "secret recover --key [--out ] [--from-export ] [--provider ]\n" + "secret export [--out ]" diff --git a/internal/inventory/migrations/0026-a-pair-credential-says-where-it-came-from.sql b/internal/inventory/migrations/0026-a-pair-credential-says-where-it-came-from.sql new file mode 100644 index 0000000..5d75b2b --- /dev/null +++ b/internal/inventory/migrations/0026-a-pair-credential-says-where-it-came-from.sql @@ -0,0 +1,14 @@ +-- A pair credential records whether the mesh made it or a person supplied it +-- (novox/hq 04-ISSUES/070, ADR 0092). +-- +-- Every pair credential so far was made: generated, sealed to both ends, the plaintext discarded, +-- remade whenever either end's key changed and replaced whole by `rotate`. A module's own secret +-- has carried `origin` since the beginning so an accepted one is never replaced by a minted one; +-- a pair could not be accepted at all, so the vault's third species -- a credential for something +-- outside the mesh, which only a person can supply -- had no entry. +-- +-- An accepted pair is not remade when a key changes (the mesh cannot: it does not hold the +-- value) and is not rotated (there is nothing to rotate to); both are refused aloud, and the +-- remedy is to accept it again. + +alter table secret add column origin text not null default 'made'; diff --git a/internal/inventory/secrets.go b/internal/inventory/secrets.go index 9f7caf7..f605070 100644 --- a/internal/inventory/secrets.go +++ b/internal/inventory/secrets.go @@ -29,8 +29,17 @@ type Secret struct { ForProvider string ConsumerKey string ProviderKey string + // Origin is `made` — the mesh generated it — or `accepted` — a person supplied it, for + // something outside the mesh, and the mesh cannot make another (novox/hq 04-ISSUES/070). + Origin string } +// Where a pair credential came from. +const ( + OriginMade = "made" + OriginAccepted = "accepted" +) + // SecretFor is the credential one module uses for one provision, making it the first time. // // **Made once and kept**, rather than regenerated whenever it is asked for. A secret that changed @@ -64,15 +73,26 @@ func (i *Inventory) SecretFor(ctx context.Context, name, consumer, consumerModul var held Secret err = i.store.Pool().QueryRow(ctx, - `select for_consumer, for_provider, consumer_key, provider_key from secret + `select for_consumer, for_provider, consumer_key, provider_key, origin from secret where name = $1 and consumer = $2 and consumer_module = $3 and provider = $4`, name, consumerNode.ID, consumerModule, providerNode.ID). - Scan(&held.ForConsumer, &held.ForProvider, &held.ConsumerKey, &held.ProviderKey) + Scan(&held.ForConsumer, &held.ForProvider, &held.ConsumerKey, &held.ProviderKey, &held.Origin) if err == nil && held.ConsumerKey == consumerKey && held.ProviderKey == providerKey { held.Name, held.Consumer, held.Provider = name, consumer, provider held.ConsumerModule = consumerModule return held, nil } + if err == nil && held.Origin == OriginAccepted { + // A person supplied this, and the mesh does not hold the value: it cannot seal it to the + // new key. Refused aloud rather than replaced by something the mesh made up, which would + // be delivered, reported as applied, and fail to authenticate somewhere else entirely + // (novox/hq 04-ISSUES/070). + return Secret{}, fmt.Errorf( + "%s's %q credential from %s was accepted from a person, and a sealing key at one end "+ + "has changed since. The mesh cannot re-seal a value it does not hold: accept it "+ + "again with `secret accept %s %s %s --provider %s`", + consumerModule, name, provider, consumer, consumerModule, name, provider) + } // And to the operator, when the mesh has one (novox/hq ADR 0085, amended): the third copy that // makes a vault-provided secret recoverable, and nothing the mesh can open. @@ -102,7 +122,60 @@ func (i *Inventory) SecretFor(ctx context.Context, name, consumer, consumerModul return Secret{Name: name, Consumer: consumer, ConsumerModule: consumerModule, Provider: provider, ForConsumer: made.ForConsumer, ForProvider: made.ForProvider, - ConsumerKey: made.ConsumerKey, ProviderKey: made.ProviderKey}, nil + ConsumerKey: made.ConsumerKey, ProviderKey: made.ProviderKey, Origin: OriginMade}, nil +} + +// AcceptSecretForPair takes a value a person supplied into a pair credential — sealed to the +// consumer's node and to the provider's, and to the operator when the mesh has one — where the +// mesh would otherwise have made one (novox/hq 04-ISSUES/070, ADR 0092). +// +// This is the vault's third species: a credential for something outside the mesh, which only a +// person can supply. It is the counterpart to AcceptSecretForModule for a module's own secret; +// what differs is that both ends of the pair are sealed to, and that the record says `accepted` +// so a later read never replaces it with a minted one. The plaintext is discarded here. +func (i *Inventory) AcceptSecretForPair(ctx context.Context, name, consumer, consumerModule, provider, value string) error { + consumerKey, err := i.SealingKeyOf(ctx, consumer) + if err != nil { + return err + } + providerKey, err := i.SealingKeyOf(ctx, provider) + if err != nil { + return err + } + if consumerKey == "" || providerKey == "" { + return fmt.Errorf( + "both %s and %s need a sealing key before a credential can be sealed to them — a "+ + "node joins to get one", consumer, provider) + } + consumerNode, err := i.NodeByName(ctx, consumer) + if err != nil { + return err + } + providerNode, err := i.NodeByName(ctx, provider) + if err != nil { + return err + } + sealed, err := secrets.Accept(value, consumerKey, providerKey) + if err != nil { + return err + } + forOperator, operatorKey, err := i.operatorSeal(ctx, value) + if err != nil { + return err + } + _, err = i.store.Pool().Exec(ctx, + `insert into secret (name, consumer, consumer_module, provider, for_consumer, for_provider, + consumer_key, provider_key, operator_sealed, operator_key, origin) + values ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11) + on conflict (name, consumer, consumer_module, provider) do update set + for_consumer = excluded.for_consumer, for_provider = excluded.for_provider, + consumer_key = excluded.consumer_key, provider_key = excluded.provider_key, + created_at = now(), origin = excluded.origin, + operator_sealed = excluded.operator_sealed, operator_key = excluded.operator_key`, + name, consumerNode.ID, consumerModule, providerNode.ID, + sealed.ForConsumer, sealed.ForProvider, sealed.ConsumerKey, sealed.ProviderKey, + forOperator, operatorKey, OriginAccepted) + return err } // RotateSecret discards what was there, so the next declaration carries a new one. @@ -113,6 +186,10 @@ func (i *Inventory) SecretFor(ctx context.Context, name, consumer, consumerModul // // The new secret then reaches both ends on the same push, together, which is what makes rotation // a single event rather than a fanout with a window where half the mesh holds a dead credential. +// +// **An accepted credential is not rotated.** The mesh did not make it and cannot make its +// replacement; deleting it would have the next read mint one, which is exactly the wrong value +// delivered with the mesh insisting it was (novox/hq 04-ISSUES/070). Refused, and the remedy named. func (i *Inventory) RotateSecret(ctx context.Context, name, consumer, consumerModule, provider string) error { consumerNode, err := i.NodeByName(ctx, consumer) if err != nil { @@ -122,6 +199,18 @@ func (i *Inventory) RotateSecret(ctx context.Context, name, consumer, consumerMo if err != nil { return err } + var origin string + err = i.store.Pool().QueryRow(ctx, + `select origin from secret where name = $1 and consumer = $2 and consumer_module = $3 + and provider = $4`, + name, consumerNode.ID, consumerModule, providerNode.ID).Scan(&origin) + if err == nil && origin == OriginAccepted { + return fmt.Errorf( + "%s's %q credential from %s was accepted from a person, and the mesh cannot make "+ + "its replacement. Accept the new value instead: `secret accept %s %s %s "+ + "--provider %s --from `", + consumerModule, name, provider, consumer, consumerModule, name, provider) + } _, err = i.store.Pool().Exec(ctx, `delete from secret where name = $1 and consumer = $2 and consumer_module = $3 and provider = $4`, diff --git a/internal/inventory/secrets_test.go b/internal/inventory/secrets_test.go index 9a92f07..07c6a46 100644 --- a/internal/inventory/secrets_test.go +++ b/internal/inventory/secrets_test.go @@ -558,3 +558,69 @@ func TestRotatingGivesBothEndsTheSameNewCredential(t *testing.T) { t.Fatal("rotating one machine's credential changed another machine's") } } + +// **A person can deliver a pair credential** (novox/hq 04-ISSUES/070, ADR 0092): the vault's +// third species, a value for something outside the mesh. It is sealed to both ends like a made +// one; what differs is that the mesh will neither replace it with one of its own nor rotate it, +// because it cannot make the replacement. +func TestAnAcceptedPairCredentialIsKeptAndNeverRemade(t *testing.T) { + inv, ctx := twoNodesWithKeys(t) + if err := inv.AcceptSecretForPair(ctx, "secret", "consumer", "gitea", "provider", "hunter2"); err != nil { + t.Fatal(err) + } + got, err := inv.SecretFor(ctx, "secret", "consumer", "gitea", "provider") + if err != nil { + t.Fatal(err) + } + if got.Origin != OriginAccepted { + t.Fatalf("an accepted credential reads back as %q", got.Origin) + } + again, err := inv.SecretFor(ctx, "secret", "consumer", "gitea", "provider") + if err != nil { + t.Fatal(err) + } + if again.ForConsumer != got.ForConsumer || again.ForProvider != got.ForProvider { + t.Fatal("reading an accepted credential twice produced two different values") + } + + // Rotation is refused, and says what to do instead. + err = inv.RotateSecret(ctx, "secret", "consumer", "gitea", "provider") + if err == nil || !strings.Contains(err.Error(), "secret accept") { + t.Fatalf("rotating an accepted credential was not refused with the remedy: %v", err) + } + // And a made one still rotates. + if _, err := inv.SecretFor(ctx, "postgres-database", "consumer", "gitea", "provider"); err != nil { + t.Fatal(err) + } + if err := inv.RotateSecret(ctx, "postgres-database", "consumer", "gitea", "provider"); err != nil { + t.Fatalf("a made credential no longer rotates: %v", err) + } +} + +// A key that changed at either end makes the accepted value unreadable there, and the mesh cannot +// re-seal what it does not hold: refused aloud, never quietly replaced by a minted one. +func TestAnAcceptedPairCredentialIsNotRemadeWhenAKeyChanges(t *testing.T) { + inv, ctx := twoNodesWithKeys(t) + if err := inv.AcceptSecretForPair(ctx, "secret", "consumer", "gitea", "provider", "hunter2"); err != nil { + t.Fatal(err) + } + node, err := inv.NodeByName(ctx, "consumer") + if err != nil { + t.Fatal(err) + } + fresh, _ := aSealingKey(t) + if err := inv.RecordSealingKey(ctx, node.ID, fresh); err != nil { + t.Fatal(err) + } + _, err = inv.SecretFor(ctx, "secret", "consumer", "gitea", "provider") + if err == nil || !strings.Contains(err.Error(), "accept it again") { + t.Fatalf("an accepted credential was remade, or refused without the remedy: %v", err) + } + // Accepting it again is the remedy, and it works. + if err := inv.AcceptSecretForPair(ctx, "secret", "consumer", "gitea", "provider", "hunter3"); err != nil { + t.Fatal(err) + } + if _, err := inv.SecretFor(ctx, "secret", "consumer", "gitea", "provider"); err != nil { + t.Fatal(err) + } +} -- 2.54.0 From 53a79a17a1c73a95ce820a19c4712ff4fd1e8ea3 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 21 Sep 2026 19:23:02 +0200 Subject: [PATCH 4/4] Review: a failure is the same by resource id, not by the host's words; bound files and /run/docker.sock are declared The host's error text may carry a duration or a counter, and a resource looping on it would never have read as stuck. The previous row is read and compared here. Stuck needs a start to say. A container may mount the file a binding lands in; the runtime socket is declared under both of its spellings; the catalogue-wide test takes MESH_CATALOG. --- internal/catalogue/manifest.go | 6 +- internal/catalogue/mounts_test.go | 18 ++++- internal/inventory/doing_test.go | 14 ++++ ...ilure-that-repeats-is-said-to-be-stuck.sql | 5 +- internal/inventory/nodes.go | 80 ++++++++++++++----- 5 files changed, 98 insertions(+), 25 deletions(-) diff --git a/internal/catalogue/manifest.go b/internal/catalogue/manifest.go index 4c82a6f..4976795 100644 --- a/internal/catalogue/manifest.go +++ b/internal/catalogue/manifest.go @@ -1098,7 +1098,8 @@ func (m Manifest) MachineSide(port int) (at int, mayAssign bool) { // facilitiesOf is what each capability lets a container mount: paths the machine owns and a module // is granted the use of by declaring the capability, never by declaring them as its own. var facilitiesOf = map[string][]string{ - "container-runtime": {"/var/run/docker.sock"}, + // Both spellings: /var/run is a link to /run on every machine the mesh runs on. + "container-runtime": {"/var/run/docker.sock", "/run/docker.sock"}, } // undeclaredMounts is every bind-mount source no declaration covers — see the check above. @@ -1127,6 +1128,9 @@ func (m Manifest) undeclaredMounts() []string { for _, where := range m.Grants { claim(where) } + for _, where := range m.Binds { + claim(where) + } for _, a := range m.Accesses { claim(a.Path) } diff --git a/internal/catalogue/mounts_test.go b/internal/catalogue/mounts_test.go index 0e323c5..15125cc 100644 --- a/internal/catalogue/mounts_test.go +++ b/internal/catalogue/mounts_test.go @@ -70,9 +70,13 @@ func TestTheRuntimeSocketIsGrantedByTheCapabilityAndNotOtherwise(t *testing.T) { // **Every manifest in the catalogue beside this checkout passes**, so the rule is not one the // catalogue is already breaking. Skipped, aloud, where the catalogue is not there. func TestEveryCatalogueManifestDeclaresWhatItMounts(t *testing.T) { - files, _ := filepath.Glob("../../../mesh-catalog/modules/*/module.json") + root := os.Getenv("MESH_CATALOG") + if root == "" { + root = "../../../mesh-catalog" + } + files, _ := filepath.Glob(filepath.Join(root, "modules", "*", "module.json")) if len(files) == 0 { - t.Skip("the catalogue is not beside this checkout") + t.Skipf("no catalogue at %s (set MESH_CATALOG to a checkout)", root) } for _, file := range files { raw, err := os.ReadFile(file) @@ -84,3 +88,13 @@ func TestEveryCatalogueManifestDeclaresWhatItMounts(t *testing.T) { } } } + +// Where a bound fact lands is the mesh's file too, and a container may mount it directly. +func TestAMountOfABoundFactIsAccepted(t *testing.T) { + _, err := ParseManifest([]byte(`{"module":"store","requires":["model-access"],` + + `"binds":{"model-access":"/var/lib/store/model.json"},"resources":[` + + strings.Replace(aContainerMounting, "%s", "/var/lib/store/model.json", 1) + `]}`)) + if err != nil { + t.Fatalf("a mount of the file the mesh writes a binding to was refused: %v", err) + } +} diff --git a/internal/inventory/doing_test.go b/internal/inventory/doing_test.go index e560fbb..55a3552 100644 --- a/internal/inventory/doing_test.go +++ b/internal/inventory/doing_test.go @@ -284,6 +284,20 @@ func TestTheSameFailureReportedAgainIsCountedNotRestarted(t *testing.T) { t.Fatalf("the failure began at %s and the row now says %s", *first.Since, *again.Since) } + // The same resource failing with different words — a duration, a counter — is still the same + // failure: it is the resource that loops, not the sentence. + reworded := Doing{Outcome: OutcomeFailed, Failed: []FailedResource{{ID: "img", Error: "no such image (after 31s)"}}} + if err := inv.RecordDoing(ctx, id, reworded); err != nil { + t.Fatal(err) + } + still, _, err := inv.DoingOf(ctx, "looping") + if err != nil { + t.Fatal(err) + } + if still.Times != StuckAfter+1 || !still.Stuck() { + t.Fatalf("the same resource failing in other words restarted the count: %+v", still) + } + // A different failure is a new situation, not a longer one. other := Doing{Outcome: OutcomeFailed, Failed: []FailedResource{{ID: "svc", Error: "unit not found"}}} if err := inv.RecordDoing(ctx, id, other); err != nil { diff --git a/internal/inventory/migrations/0025-a-failure-that-repeats-is-said-to-be-stuck.sql b/internal/inventory/migrations/0025-a-failure-that-repeats-is-said-to-be-stuck.sql index 266ea5b..f034e77 100644 --- a/internal/inventory/migrations/0025-a-failure-that-repeats-is-said-to-be-stuck.sql +++ b/internal/inventory/migrations/0025-a-failure-that-repeats-is-said-to-be-stuck.sql @@ -7,8 +7,9 @@ -- when its dependency arrives" from "failed identically for ever", and nothing escalated the second. -- -- Still one row per node. What is added is how long the CURRENT failure has been the same one: --- when it first appeared, and how many reports in a row have said it. A report that says something --- different starts the count again; a clean apply clears it. +-- when it first appeared, and how many reports in a row have said it -- the same outcome, the same +-- refusal, the same failed resources by id (not by the host's words, which may carry a duration). +-- A report that says something different starts the count again; a clean apply clears it. alter table node_report add column failing_since timestamptz, diff --git a/internal/inventory/nodes.go b/internal/inventory/nodes.go index 6729cb5..8cb1ccb 100644 --- a/internal/inventory/nodes.go +++ b/internal/inventory/nodes.go @@ -9,6 +9,7 @@ import ( "encoding/json" "errors" "fmt" + "sort" "strings" "time" @@ -468,7 +469,7 @@ type Doing struct { Declared string // Since is when the machine first reported THIS failure — the same outcome, the same refusal, - // the same failed resources — and Times is how many reports in a row have said it. A node + // the same failed resources by id — and Times is how many reports in a row have said it. A node // re-applies on a steady interval and reports each time (novox/hq ADR 0010), so a failure // that will never succeed arrives as the same report over and over, indistinguishable from // one that just happened until somebody counts (novox/hq 04-ISSUES/065). Nil and zero for a @@ -496,7 +497,32 @@ const StuckAfter = 3 // Stuck reports whether this machine has been failing the same way for long enough that waiting // is no longer a plan. The failure is still the host's own words; this only says it is not new. -func (d Doing) Stuck() bool { return d.Wrong() && d.Times >= StuckAfter } +func (d Doing) Stuck() bool { return d.Wrong() && d.Times >= StuckAfter && d.Since != nil } + +// sameFailure is whether two reports describe one failure: the same outcome, the same refusal, and +// the same failed resources BY ID. Not by the host's words: an error that carries a duration, a +// counter or a temporary path would read as new on every report, and the resource looping on it — +// which is what stuck is for — would never be said to be (novox/hq 04-ISSUES/065). +func sameFailure(a, b Doing) bool { + if a.Outcome != b.Outcome || a.Refused != b.Refused || len(a.Failed) != len(b.Failed) { + return false + } + ids := func(d Doing) []string { + out := make([]string, 0, len(d.Failed)) + for _, f := range d.Failed { + out = append(out, f.ID) + } + sort.Strings(out) + return out + } + x, y := ids(a), ids(b) + for i := range x { + if x[i] != y[i] { + return false + } + } + return true +} // RecordDoing keeps what a node said it did. // @@ -505,35 +531,49 @@ func (d Doing) Stuck() bool { return d.Wrong() && d.Times >= StuckAfter } // would bury the ones that matter. // // **What the row also keeps is whether this failure is the one before.** The same outcome, the -// same refusal, the same failed resources with the same words: then the failure did not just -// happen, it is still happening, and the row keeps when it began and counts one more report. Any -// difference starts again — a machine failing on a new resource is a new situation, not a longer -// one — and a clean apply clears both (novox/hq 04-ISSUES/065). +// same refusal, the same failed resources by id: then the failure did not just happen, it is +// still happening, and the row keeps when it began and counts one more report. Any difference +// starts again — a machine failing on a new resource is a new situation, not a longer one — and +// a clean apply clears both (novox/hq 04-ISSUES/065). The previous row is read first and the +// comparison made here, so "the same" is a rule this package states rather than a jsonb equality +// that would restart the count on a changed word in an error. func (i *Inventory) RecordDoing(ctx context.Context, node string, d Doing) error { failed, err := json.Marshal(d.Failed) if err != nil { return err } + var since *time.Time + times := 0 + if d.Outcome != OutcomeApplied { + var before Doing + var beforeFailed []byte + err := i.store.Pool().QueryRow(ctx, + `select outcome, refused, failed, failing_since, failures from node_report where node = $1`, + node).Scan(&before.Outcome, &before.Refused, &beforeFailed, &before.Since, &before.Times) + switch { + case errors.Is(err, pgx.ErrNoRows): + case err != nil: + return err + default: + if err := json.Unmarshal(beforeFailed, &before.Failed); err != nil { + return err + } + } + now := time.Now() + since, times = &now, 1 + if err == nil && sameFailure(before, d) && before.Since != nil { + since, times = before.Since, before.Times+1 + } + } _, err = i.store.Pool().Exec(ctx, `insert into node_report (node, outcome, refused, failed, applied, at, declared, failing_since, failures) - values ($1, $2, $3, $4, $5, now(), $6, - case when $2 <> $7 then now() end, - case when $2 <> $7 then 1 else 0 end) + values ($1, $2, $3, $4, $5, now(), $6, $7, $8) on conflict (node) do update set outcome = excluded.outcome, refused = excluded.refused, failed = excluded.failed, applied = excluded.applied, at = excluded.at, declared = excluded.declared, - failing_since = case - when excluded.outcome = $7 then null - when node_report.outcome = excluded.outcome and node_report.refused = excluded.refused - and node_report.failed = excluded.failed then node_report.failing_since - else excluded.at end, - failures = case - when excluded.outcome = $7 then 0 - when node_report.outcome = excluded.outcome and node_report.refused = excluded.refused - and node_report.failed = excluded.failed then node_report.failures + 1 - else 1 end`, - node, d.Outcome, d.Refused, failed, d.Applied, d.Declared, OutcomeApplied) + failing_since = excluded.failing_since, failures = excluded.failures`, + node, d.Outcome, d.Refused, failed, d.Applied, d.Declared, since, times) return err } -- 2.54.0