From 868ee6beb0554a0388090e607b4daa46f78b240f Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 9 Nov 2026 12:52:54 +0100 Subject: [PATCH] A claim problem or a refused credential holds its module, not the whole pass (issue 1966, S1 and S5) The node-engine refused the whole pass when two modules claimed a seat held once, or when a consumer's credential was refused. Now the composer leaves that module out of the declaration and composes the rest: the module is returned held (Resolved.Held, resolution.ResolveHolding), reaches the declaration as left out through the filter and the declaration #218 already keep (Rendering.NotPlaced), and Declared.LeftOut names it. The second claimant of a seat is the one held; its claim is not this node's holding. The rest-of-mesh view and the network use the same, so a node with a held module stays on the network. Resolve stays strict for callers that cannot leave a module out. Quick fix: hold only the credential refusal (resolve.go planForNow), about 20 minutes. Thorough fix, chosen: every claim problem is attributed to its module (claimsOf) and both kinds go through the one left-out path of ADR 0163 rule 6, in the declaration, the filter and the network, so no per-kind refusal of a pass remains in the composer. Left for the next step: a declared-and-not-made own secret (NotMadeError) and the Foreseeing-mode refusal still refuse the pass; the frombus candidate path holds those by stub already. Claude-Session: https://claude.ai/code/session_01NcbgfFfQ4Z9UE5jK32PmE5 --- internal/compose/credentials_needed.go | 2 +- internal/compose/credentials_test.go | 5 +- internal/compose/network.go | 2 +- internal/compose/network_test.go | 16 ++-- internal/compose/node_test.go | 20 +---- internal/compose/one_machine_alone_test.go | 4 +- internal/compose/render.go | 11 +++ internal/compose/resolution/declaration.go | 5 ++ internal/compose/resolution/holdings_test.go | 5 ++ internal/compose/resolution/resolve.go | 89 +++++++++++++++++--- internal/compose/resolve.go | 35 +++++--- internal/compose/resolve_test.go | 56 ++++++++---- internal/compose/rest.go | 4 +- internal/compose/rest_test.go | 11 +-- 14 files changed, 182 insertions(+), 83 deletions(-) diff --git a/internal/compose/credentials_needed.go b/internal/compose/credentials_needed.go index ec36c71..6937130 100644 --- a/internal/compose/credentials_needed.go +++ b/internal/compose/credentials_needed.go @@ -23,7 +23,7 @@ type CredentialKey struct { // in neither list: the vault mints nothing for it (issue 1126). func CredentialsNeeded(in Input, node string) (asConsumer, asProvider []CredentialKey, err error) { c := newComposer(in) - resolved, err := c.resolution(node) + resolved, _, err := c.resolutionHolding(node) if err != nil { return nil, nil, err } diff --git a/internal/compose/credentials_test.go b/internal/compose/credentials_test.go index 47b5671..1ba5920 100644 --- a/internal/compose/credentials_test.go +++ b/internal/compose/credentials_test.go @@ -201,10 +201,7 @@ func TestAGrantIsCreatedForEachCredentialMadeForAConsumer(t *testing.T) { } // A consumer whose set does not resolve is skipped, not fatal. nodeOf(t, &in, "laptop").Assigned = append(nodeOf(t, &in, "laptop").Assigned, "files") - in.Catalogue["files"] = catalogue.Manifest{Module: "files", Version: "1", Requires: []string{"s3-bucket"}, - Claims: []catalogue.Claim{{Name: "x", Scope: catalogue.ScopeNode}}} - in.Catalogue["twin"] = catalogue.Manifest{Module: "twin", Version: "1", Claims: []catalogue.Claim{{Name: "x", Scope: catalogue.ScopeNode}}} - nodeOf(t, &in, "laptop").Assigned = append(nodeOf(t, &in, "laptop").Assigned, "twin") + in.Catalogue["files"] = catalogue.Manifest{Module: "files", Version: "1", Requires: []string{"s3-bucket", "nothing-gives-this"}} if got, err := Render(in, "anchor", Foreseeing); err != nil || len(got.Rendering.Grants) != 0 { t.Errorf("a consumer that does not resolve: %v %+v", err, got.Rendering.Grants) } diff --git a/internal/compose/network.go b/internal/compose/network.go index 09ab10d..6ebce85 100644 --- a/internal/compose/network.go +++ b/internal/compose/network.go @@ -283,7 +283,7 @@ func (c *composer) onTheNetworkNow() []Node { if len(n.Assigned) == 0 { continue } - got, err := resolution.Resolve(c.in.Catalogue, n.Assigned, + got, _, err := resolution.ResolveHolding(c.in.Catalogue, n.Assigned, catalogue.Node{Name: n.Name, Site: n.Place.Site, Capabilities: n.Capabilities}, catalogue.World{Unchecked: true, Holdings: c.in.Holdings}) if err != nil { diff --git a/internal/compose/network_test.go b/internal/compose/network_test.go index 2cf4657..47a8553 100644 --- a/internal/compose/network_test.go +++ b/internal/compose/network_test.go @@ -164,9 +164,8 @@ func TestANodeNotOnThePrivateNetworkGetsNoAddressAndNoNetwork(t *testing.T) { func TestARefusedNodeIsSaidWhenTheNetworkCannotBeComposed(t *testing.T) { in := aMesh(t) nodeOf(t, &in, "anchor").Place.Key = "" - one := catalogue.Manifest{Module: "rival-one", Version: "1", Claims: []catalogue.Claim{{Name: "the-seat", Scope: catalogue.ScopeNode}}} - two := catalogue.Manifest{Module: "rival-two", Version: "1", Claims: []catalogue.Claim{{Name: "the-seat", Scope: catalogue.ScopeNode}}} - in.Catalogue["rival-one"], in.Catalogue["rival-two"] = one, two + in.Catalogue["rival-one"] = catalogue.Manifest{Module: "rival-one", Version: "1", Resources: []map[string]any{{"id": "r", "type": "directory", "path": "/var/lib/the-same", "mode": "0700"}}} + in.Catalogue["rival-two"] = catalogue.Manifest{Module: "rival-two", Version: "1", Resources: []map[string]any{{"id": "r", "type": "directory", "path": "/var/lib/the-same", "mode": "0700"}}} hub := nodeOf(t, &in, "anchor") hub.Assigned = append(hub.Assigned, "rival-one", "rival-two") // its own set is incoherent: the hub does not resolve _, err := Generators(in) @@ -293,14 +292,13 @@ func TestEachFieldOfTheInputReachesTheGenerator(t *testing.T) { } laptop := nodeOf(t, &in, "laptop") laptop.Assigned = append(laptop.Assigned, "holder-one", "holder-two") - // Without a key: a machine that does not resolve and holds a complete place keeps it on the network (issue 1833). - laptop.Place.Key = "" - if got := addressesOf(in); got["laptop"] != "" { - t.Fatalf("a machine whose set claims one seat twice is on the network: %v", got) + // Two claimants and nobody on record: the second is held and the machine stays on the network (issue 1966). + if got, err := Resolve(in, "laptop"); err != nil || len(got.Held) != 1 || got.Held["holder-two"] == "" || addressesOf(in)["laptop"] != "10.77.0.2" { + t.Fatalf("two claimants, none on record: held %v, %v", got.Held, err) } in.Holdings = []catalogue.Held{{Claim: "mesh-broker", Scope: catalogue.ScopeMesh, Node: "laptop", Module: "holder-one"}} - if got := addressesOf(in); got["laptop"] != "10.77.0.2" { - t.Errorf("a holder on record did not settle the seat: %v", got) + if got, err := Resolve(in, "laptop"); err != nil || len(got.Held) != 0 { + t.Errorf("a holder on record did not settle the seat: held %v, %v", got.Held, err) } }) } diff --git a/internal/compose/node_test.go b/internal/compose/node_test.go index 6735f95..15b97fe 100644 --- a/internal/compose/node_test.go +++ b/internal/compose/node_test.go @@ -112,22 +112,6 @@ func TestAManifestTheNodeEngineRefusesComesBackAsProblemsAndNoError(t *testing.T } } -// 1298/AC3: a node whose own set does not compose returns an error and no declaration, told apart from an unreadable Input. -func TestANodeWhoseSetDoesNotComposeReturnsAnErrorAndNoDeclaration(t *testing.T) { - in := aMesh(t) - in.Catalogue["rival-one"] = catalogue.Manifest{Module: "rival-one", Version: "1", Claims: []catalogue.Claim{{Name: "the-seat", Scope: catalogue.ScopeNode}}} - in.Catalogue["rival-two"] = catalogue.Manifest{Module: "rival-two", Version: "1", Claims: []catalogue.Claim{{Name: "the-seat", Scope: catalogue.ScopeNode}}} - laptop := nodeOf(t, &in, "laptop") - laptop.Assigned = append(laptop.Assigned, "rival-one", "rival-two") - d, problems, err := Compose(in, "laptop") - if err == nil || !IsUnresolvable(err) || d.Resources != nil || problems != nil { - t.Fatalf("a set that does not compose: %v %v %v", d.Resources, problems, err) - } - if _, _, err := Compose(in, "nowhere"); err == nil || IsUnresolvable(err) { - t.Fatalf("a node the Input does not have: %v", err) - } -} - // 1298/AC4: composing the same Input twice gives byte-identical bodies, and the Input is unchanged. func TestComposingTwiceGivesTheSameBytesAndChangesNothing(t *testing.T) { mk := func() Input { @@ -392,8 +376,8 @@ func overlayName() string { return overlay.Name } func TestANodeNotCountedAsOnTheNetworkIsSaidInTheResult(t *testing.T) { in := aMesh(t) nodeOf(t, &in, "laptop").Place.Key = "" - in.Catalogue["rival-one"] = catalogue.Manifest{Module: "rival-one", Version: "1", Claims: []catalogue.Claim{{Name: "the-seat", Scope: catalogue.ScopeNode}}} - in.Catalogue["rival-two"] = catalogue.Manifest{Module: "rival-two", Version: "1", Claims: []catalogue.Claim{{Name: "the-seat", Scope: catalogue.ScopeNode}}} + in.Catalogue["rival-one"] = catalogue.Manifest{Module: "rival-one", Version: "1", Resources: []map[string]any{{"id": "r", "type": "directory", "path": "/var/lib/the-same", "mode": "0700"}}} + in.Catalogue["rival-two"] = catalogue.Manifest{Module: "rival-two", Version: "1", Resources: []map[string]any{{"id": "r", "type": "directory", "path": "/var/lib/the-same", "mode": "0700"}}} laptop := nodeOf(t, &in, "laptop") laptop.Assigned = append(laptop.Assigned, "rival-one", "rival-two") said := func(lines []string) bool { diff --git a/internal/compose/one_machine_alone_test.go b/internal/compose/one_machine_alone_test.go index f05accb..2c8fef9 100644 --- a/internal/compose/one_machine_alone_test.go +++ b/internal/compose/one_machine_alone_test.go @@ -147,8 +147,8 @@ func TestANetworkThatCannotBeComposedFromWhatRemainsStillFails(t *testing.T) { // among the sources "from the mesh" -- and another machine still composes. func TestAMachineThatDoesNotResolveKeepsItsNameAndItsAddressInTheMesh(t *testing.T) { in := fourOnTheNetwork(t) - in.Catalogue["rival-one"] = catalogue.Manifest{Module: "rival-one", Version: "1", Claims: []catalogue.Claim{{Name: "the-seat", Scope: catalogue.ScopeNode}}} - in.Catalogue["rival-two"] = catalogue.Manifest{Module: "rival-two", Version: "1", Claims: []catalogue.Claim{{Name: "the-seat", Scope: catalogue.ScopeNode}}} + in.Catalogue["rival-one"] = catalogue.Manifest{Module: "rival-one", Version: "1", Resources: []map[string]any{{"id": "r", "type": "directory", "path": "/var/lib/the-same", "mode": "0700"}}} + in.Catalogue["rival-two"] = catalogue.Manifest{Module: "rival-two", Version: "1", Resources: []map[string]any{{"id": "r", "type": "directory", "path": "/var/lib/the-same", "mode": "0700"}}} hub := nodeOf(t, &in, "anchor") hub.Assigned = append(hub.Assigned, "rival-one", "rival-two") if _, err := Resolve(in, "anchor"); !IsUnresolvable(err) { diff --git a/internal/compose/render.go b/internal/compose/render.go index 125569d..09d9b7b 100644 --- a/internal/compose/render.go +++ b/internal/compose/render.go @@ -114,6 +114,17 @@ func (c *composer) render(nodeName string, mode Mode) (Rendered, error) { out.CoreBoxesWanted = wanted } + // A module whose claim cannot be held, or whose credential was refused, is left out of the declaration and run as it runs: + // the machine composes the rest (issue 1966, ADR 0636 §3). + for module, why := range resolved.Held { + if r.NotPlaced == nil { + r.NotPlaced = map[string]string{} + } + if _, said := r.NotPlaced[module]; !said { + r.NotPlaced[module] = why + } + } + // And a certificate for this machine's name inside the mesh, when anything on it asks. for _, m := range plan.Modules { if m.Certificate == nil { diff --git a/internal/compose/resolution/declaration.go b/internal/compose/resolution/declaration.go index 3133367..b3f5672 100644 --- a/internal/compose/resolution/declaration.go +++ b/internal/compose/resolution/declaration.go @@ -1253,6 +1253,11 @@ func Rules(r mcat.Resolution, with mcat.Rendering) ([]mcat.Rule, error) { // cannot be read for it, its listens open no wider than the mesh, never at the manifest's // default, which could reopen a port an operator had narrowed. left := LeftOut(r, with.Settings, with.Adopted) + for module, why := range with.NotPlaced { + if _, already := left[module]; !already { + left[module] = why + } + } exposure := map[string]map[int]string{} for _, m := range r.Modules { e, err := exposureOf(m, with.Settings[m.Module]) diff --git a/internal/compose/resolution/holdings_test.go b/internal/compose/resolution/holdings_test.go index 103d133..953111b 100644 --- a/internal/compose/resolution/holdings_test.go +++ b/internal/compose/resolution/holdings_test.go @@ -37,6 +37,11 @@ func TestWithNoHolderOnRecordTheSoleClaimantHoldsAndTwoAreRefused(t *testing.T) if len(problems) != 1 || !strings.Contains(problems[0], "both claim") { t.Fatalf("two claimants with nothing on record were not refused: %v", problems) } + // Held for the module that cannot hold, which then holds nothing; the other still does (issue 1966). + held, holding := holdClaims([]mcat.Manifest{oldBroker(), newBroker()}, node, nil, nil) + if len(holding) != 1 || !strings.Contains(holding["new-broker"], "both claim") || len(held) != 1 || held[0].Module != "old-broker" { + t.Fatalf("the second claimant is not the one held: held=%v holding=%v", held, holding) + } } // With a holder on record, the other eligible assignment is silent: not refused, and not holding. diff --git a/internal/compose/resolution/resolve.go b/internal/compose/resolution/resolve.go index 7a36df8..553c538 100644 --- a/internal/compose/resolution/resolve.go +++ b/internal/compose/resolution/resolve.go @@ -32,6 +32,20 @@ func (r *Refusal) Error() string { // What comes back is the closure — assigned modules plus everything they require — or a refusal // naming every reason it could not be closed. func Resolve(catalogue map[string]mcat.Manifest, assigned []string, node mcat.Node, world mcat.World) (mcat.Resolution, error) { + resolution, _, err := resolve(catalogue, assigned, node, world, true) + return resolution, err +} + +// ResolveHolding is Resolve where a claim problem costs the module that has it, not the machine (issue 1966, ADR 0636 §3: a +// refused claim blocks that claimant only). The modules that cannot hold what they claim come back with why, each still in +// the resolution's modules and none of them in its claims; the caller leaves them out of what it declares (LeftOut), so the +// machine composes everything else. Every other refusal is Resolve's. +func ResolveHolding(catalogue map[string]mcat.Manifest, assigned []string, node mcat.Node, world mcat.World) (mcat.Resolution, map[string]string, error) { + return resolve(catalogue, assigned, node, world, false) +} + +// resolve is Resolve; with strict, a claim problem is a refusal beside every other, and without it a module held. +func resolve(catalogue map[string]mcat.Manifest, assigned []string, node mcat.Node, world mcat.World, strict bool) (mcat.Resolution, map[string]string, error) { elsewhere := world.Held var problems []string @@ -593,8 +607,13 @@ func Resolve(catalogue map[string]mcat.Manifest, assigned []string, node mcat.No } problems = append(problems, checkCapabilities(resolution.Modules, node)...) - claims, claimProblems := checkClaims(resolution.Modules, node, elsewhere, world.Holdings) - problems = append(problems, claimProblems...) + claims, held := holdClaims(resolution.Modules, node, elsewhere, world.Holdings) + if strict { + for _, why := range held { + problems = append(problems, why) + } + held = nil + } problems = append(problems, checkResources(resolution.Modules)...) resolution.Claims = claims @@ -616,9 +635,9 @@ func Resolve(catalogue map[string]mcat.Manifest, assigned []string, node mcat.No if len(problems) > 0 { sort.Strings(problems) - return mcat.Resolution{}, &Refusal{Problems: problems} + return mcat.Resolution{}, nil, &Refusal{Problems: problems} } - return resolution, nil + return resolution, held, nil } // isModule reports whether a name is a module in its own right rather than only something @@ -677,7 +696,37 @@ func checkCapabilities(modules []mcat.Manifest, node mcat.Node) []string { // Within this node's own set, and against what is already held elsewhere for the wider scopes. A // claim at mesh scope is the same idea as the mesh's one hub, said once instead of hard-coded. func checkClaims(modules []mcat.Manifest, node mcat.Node, elsewhere []mcat.Held, holdings []mcat.Held) ([]mcat.Held, []string) { - var problems []string + held, problems := claimsOf(modules, node, elsewhere, holdings) + var out []string + for _, p := range problems { + out = append(out, p.why) + } + return held, out +} + +// holdClaims is checkClaims with each problem put on the module that has it: the claims that hold, and for each module that +// cannot hold what it claims, why (every reason, joined). +func holdClaims(modules []mcat.Manifest, node mcat.Node, elsewhere []mcat.Held, holdings []mcat.Held) ([]mcat.Held, map[string]string) { + held, problems := claimsOf(modules, node, elsewhere, holdings) + var out map[string]string + for _, p := range problems { + if out == nil { + out = map[string]string{} + } + if prior, ok := out[p.module]; ok { + out[p.module] = prior + "; " + p.why + } else { + out[p.module] = p.why + } + } + return held, out +} + +// claimProblem is a refusal of one module's claim. +type claimProblem struct{ module, why string } + +func claimsOf(modules []mcat.Manifest, node mcat.Node, elsewhere []mcat.Held, holdings []mcat.Held) ([]mcat.Held, []claimProblem) { + var problems []claimProblem var held []mcat.Held // onRecord is every recorded holder of a seat, if a handover ever named one: one for most seats, @@ -717,9 +766,9 @@ func checkClaims(modules []mcat.Manifest, node mcat.Node, elsewhere []mcat.Held, // disagrees with the mesh's definition of the role, and it is refused, named, rather // than letting two machines answer for what the mesh has one of. if s, known := mcat.SeatNamed(c.Name); known && !s.Replicated && len(recs) > 1 { - problems = append(problems, fmt.Sprintf( + problems = append(problems, claimProblem{m.Module, fmt.Sprintf( "%q is on record as held by %d assignments, and it is held once per %s — "+ - "`seat %s --to /` records one", c.Name, len(recs), scope, c.Name)) + "`seat %s --to /` records one", c.Name, len(recs), scope, c.Name)}) continue } if !recordedHere(c.Name, scope, m.Module) { @@ -734,9 +783,9 @@ func checkClaims(modules []mcat.Manifest, node mcat.Node, elsewhere []mcat.Held, // one seat, compared by the seat they resolve to and not by how each spelled it. seat := heldKey(c.Name, c.Kind) if other, taken := byScope[scope][seat]; taken { - problems = append(problems, fmt.Sprintf( + problems = append(problems, claimProblem{m.Module, fmt.Sprintf( "%s and %s both claim %q, and only one thing may hold it per %s", - other, m.Module, seat, scope)) + other, m.Module, seat, scope)}) continue } byScope[scope][seat] = m.Module @@ -762,20 +811,34 @@ func checkClaims(modules []mcat.Manifest, node mcat.Node, elsewhere []mcat.Held, // Both claim and nobody is on record, or this refusal could not have happened. // The remedy is the handover that records the holder (novox/hq ADR 0131, // 04-ISSUES/170), so it is named here rather than left to be found. - problems = append(problems, fmt.Sprintf( + problems = append(problems, claimProblem{h.Module, fmt.Sprintf( "%s on %s claims %q, which %s on %s already holds — one per mesh. Nothing is "+ "on record for it; `seat %s --to %s/%s` records the holder, and the other "+ "assignment then stands beside it, eligible and silent", - h.Module, node.Name, h.Claim, e.Module, e.Node, h.Claim, e.Node, e.Module)) + h.Module, node.Name, h.Claim, e.Module, e.Node, h.Claim, e.Node, e.Module)}) case mcat.ScopeSite: if node.Site != "" && node.Site == e.Site { - problems = append(problems, fmt.Sprintf( + problems = append(problems, claimProblem{h.Module, fmt.Sprintf( "%s on %s claims %q, which %s on %s already holds at %s — one per site", - h.Module, node.Name, h.Claim, e.Module, e.Node, node.Site)) + h.Module, node.Name, h.Claim, e.Module, e.Node, node.Site)}) } } } } + // A module refused here holds nothing of what it claims: its claims are not this node's holdings. + refused := map[string]bool{} + for _, p := range problems { + refused[p.module] = true + } + if len(refused) > 0 { + kept := held[:0] + for _, h := range held { + if !refused[h.Module] { + kept = append(kept, h) + } + } + held = kept + } return held, problems } diff --git a/internal/compose/resolve.go b/internal/compose/resolve.go index 117e907..fcd2fc9 100644 --- a/internal/compose/resolve.go +++ b/internal/compose/resolve.go @@ -22,6 +22,10 @@ type Resolved struct { // NotGiven are the words of each credential only a person gives that nobody has: the consumer still resolves without it // (novox/hq ADR 0337 clause 4). NotGiven []string + // Held are the modules this machine leaves out of its declaration and runs as they run, and why: one whose claim cannot be + // held (two claim a seat held once) or whose credential was refused (issue 1966, ADR 0636 §3). The rest of the machine + // composes; the declaration says each as left out (Declared.LeftOut), so a condition names it. + Held map[string]string } // Resolve works out everything a node should run, from what was assigned to it (the controller's planFor, plan.go): the @@ -77,7 +81,7 @@ func (r Resolved) copied() Resolved { // planForNow is planFor worked out. func (c *composer) planForNow(nodeName string) (Resolved, error) { - resolved, err := c.resolution(nodeName) + resolved, held, err := c.resolutionHolding(nodeName) if err != nil { return Resolved{}, err } @@ -87,6 +91,7 @@ func (c *composer) planForNow(nodeName string) (Resolved, error) { } shelf := c.shelfFor(node) var out Resolved + out.Held = held // The credential for each thing this node takes from elsewhere. for i, n := range resolved.Needs { @@ -129,8 +134,13 @@ func (c *composer) planForNow(nodeName string) (Resolved, error) { case cred.State == CredentialRefused: // Said rather than skipped. A machine that resolves cleanly and receives no credential is one that will fail to // authenticate at some later, less obvious moment. - return Resolved{}, fmt.Errorf("%s on %s needs %s from %s and no credential could be made for it: %s", + // Of that module only: it is held, the rest of the machine composes (issue 1966). + if out.Held == nil { + out.Held = map[string]string{} + } + out.Held[n.For] = fmt.Sprintf("%s on %s needs %s from %s and no credential could be made for it: %s", n.For, nodeName, n.Name, n.From, cred.Why) + continue case cred.State == CredentialNotGiven, outside && !(held && cred.State == CredentialHeld): // Not given, and the rest of the consumer runs (ADR 0337 clause 4): no credential file, so the module can tell // it was not given; nothing the mesh made stands in for it. @@ -249,18 +259,19 @@ func movedProvider(m catalogue.Manifest) bool { return m.Moved || slices.ContainsFunc(m.Claims, func(c catalogue.Claim) bool { return c.Name == "mesh-vault" }) } -// resolution works out what one node should run, from what was assigned to it: the controller's planFor up to and -// including the resolution (plan.go planFor), and no further. A failure to compose the node's own modules is wrapped as -// notResolvable; every other failure is returned as it is. -func (c *composer) resolution(nodeName string) (catalogue.Resolution, error) { +// resolutionHolding works out what one node should run, from what was assigned to it: the controller's planFor up to and +// including the resolution (plan.go planFor), and no further. A module whose claim cannot be held is returned beside the +// resolution, with why, and not refused with the machine (issue 1966). A failure to compose the node's own modules is +// wrapped as notResolvable; every other failure is returned as it is. +func (c *composer) resolutionHolding(nodeName string) (catalogue.Resolution, map[string]string, error) { node, err := c.in.node(nodeName) if err != nil { - return catalogue.Resolution{}, err + return catalogue.Resolution{}, nil, err } shelf := c.shelfFor(node) world, _, left, err := c.theRestOfTheMesh(nodeName) if err != nil { - return catalogue.Resolution{}, err + return catalogue.Resolution{}, nil, err } world.Pinned = node.Pins // Where each of its consumers of a provision that keeps data was last sent (novox/hq ADR 0232). @@ -268,14 +279,14 @@ func (c *composer) resolution(nodeName string) (catalogue.Resolution, error) { onNetwork, err := c.whereEveryoneIs() if err != nil { - return catalogue.Resolution{}, err + return catalogue.Resolution{}, nil, err } // What this mesh can answer with a record rather than a machine, and which record each of this node's modules was // put on. world.Licences, world.Using = c.licencesFor(nodeName) - resolved, err := resolution.Resolve(shelf, node.Assigned, + resolved, held, err := resolution.ResolveHolding(shelf, node.Assigned, catalogue.Node{Name: nodeName, Site: node.Place.Site, Capabilities: node.Capabilities, At: onNetwork[nodeName], PublicDomain: node.PublicDomain, Account: node.Account, AccountHome: node.AccountHome, @@ -295,9 +306,9 @@ func (c *composer) resolution(nodeName string) (catalogue.Resolution, error) { err, strings.Join(c.unhostable, "; ")) } // The node's own set does not compose: the only failure a mesh-wide gatherer may pass over. - return catalogue.Resolution{}, notResolvable{err} + return catalogue.Resolution{}, nil, notResolvable{err} } - return resolved, nil + return resolved, held, nil } // shelfFor is the catalogue as one node composes its own set: the Input's, with each recorded or core module the node keeps at the diff --git a/internal/compose/resolve_test.go b/internal/compose/resolve_test.go index d361b8c..09369de 100644 --- a/internal/compose/resolve_test.go +++ b/internal/compose/resolve_test.go @@ -2,6 +2,7 @@ package compose import ( "errors" + "slices" "strings" "testing" @@ -223,21 +224,38 @@ func TestACredentialOnlyAPersonGivesIsNotMadeAndTheConsumerStillResolves(t *test } } -// 1362/AC3: a credential the send would refuse, a given value sealed to a key a machine no longer has, refuses the resolution with -// today's words, and is not a node whose set does not compose. -func TestACredentialTheSendWouldRefuseRefusesTheResolutionInTheSendsWords(t *testing.T) { +// 1362/AC3, issue 1966: a fault of one module's own, a credential the send would refuse (a given value sealed to a key a machine no +// longer has) or a second claimant of a seat held once, costs that module and not the machine: it is held with the send's words, its +// declaration says it left out, and every other module of the machine composes. Not a node whose set does not compose. +func TestAModuleWhoseCredentialOrClaimFailsIsHeldAndTheMachineComposes(t *testing.T) { why := "files's \"s3-bucket\" credential from anchor 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 laptop files s3-bucket --provider anchor`" - in := theObjectsMesh(t, objects()) - in.Credentials = []Credential{{Provision: "s3-bucket", Consumer: "laptop", ConsumerModule: "files", Provider: "anchor", - State: CredentialRefused, Why: why}} - _, err := Resolve(in, "laptop") - want := "files on laptop needs s3-bucket from anchor and no credential could be made for it: " + why - if err == nil || err.Error() != want { - t.Fatalf("got %v, want %q", err, want) - } - if IsUnresolvable(err) { - t.Error("a refused credential is a node whose set does not compose") + for _, c := range []struct{ name, module, said string }{ + {"a refused credential", "files", "files on laptop needs s3-bucket from anchor and no credential could be made for it: " + why}, + {"a second claimant", "holder-two", "holder-one and holder-two both claim"}, + } { + t.Run(c.name, func(t *testing.T) { + withSeat(t) + in := theObjectsMesh(t, objects()) + in.Credentials = []Credential{{Provision: "s3-bucket", Consumer: "laptop", ConsumerModule: "files", Provider: "anchor", + State: CredentialRefused, Why: why}} + if c.module == "holder-two" { + in = aMesh(t) + for _, name := range []string{"holder-one", "holder-two"} { + in.Catalogue[name] = aBrokerClaiming(name) + } + laptop := nodeOf(t, &in, "laptop") + laptop.Assigned = append(laptop.Assigned, "holder-one", "holder-two") + } + got, err := Resolve(in, "laptop") + if err != nil || IsUnresolvable(err) || len(got.Held) != 1 || !strings.Contains(got.Held[c.module], c.said) || len(got.Plan.Modules) == 0 { + t.Fatalf("held %v, %v; want %s held with %q", got.Held, err, c.module, c.said) + } + d, _, err := Compose(in, "laptop") + if err != nil || !slices.Contains(d.LeftOut, c.module) || !strings.Contains(d.LeftOutWhy[c.module], c.said) || len(d.Resources) == 0 { + t.Errorf("declared: left out %v, %d resources, %v", d.LeftOut, len(d.Resources), err) + } + }) } } @@ -277,13 +295,19 @@ func TestARecordAnsweredNeedGetsItsKeyAndTheManagerItsPublicKey(t *testing.T) { // 1362/AC5: a node whose own set does not compose returns an error callers can tell from an unreadable Input. func TestANodeWhoseOwnSetDoesNotComposeIsToldApartFromAnUnreadableInput(t *testing.T) { in := aMesh(t) - in.Catalogue["rival-one"] = catalogue.Manifest{Module: "rival-one", Version: "1", Claims: []catalogue.Claim{{Name: "the-seat", Scope: catalogue.ScopeNode}}} - in.Catalogue["rival-two"] = catalogue.Manifest{Module: "rival-two", Version: "1", Claims: []catalogue.Claim{{Name: "the-seat", Scope: catalogue.ScopeNode}}} + in.Catalogue["rival-one"] = catalogue.Manifest{Module: "rival-one", Version: "1", Requires: []string{"nothing-gives-this"}} laptop := nodeOf(t, &in, "laptop") - laptop.Assigned = append(laptop.Assigned, "rival-one", "rival-two") + laptop.Assigned = append(laptop.Assigned, "rival-one") if _, err := Resolve(in, "laptop"); err == nil || !IsUnresolvable(err) { t.Fatalf("a set that does not compose: %v", err) } + // 1298/AC3: and its declaration is an error and nothing else. + if d, problems, err := Compose(in, "laptop"); err == nil || !IsUnresolvable(err) || d.Resources != nil || problems != nil { + t.Fatalf("a set that does not compose: %v %v %v", d.Resources, problems, err) + } + if _, _, err := Compose(in, "nowhere"); err == nil || IsUnresolvable(err) { + t.Fatalf("a node the Input does not have: %v", err) + } if _, err := Resolve(in, "nowhere"); err == nil || IsUnresolvable(err) || !errors.Is(err, ErrNoSuchNode) { t.Fatalf("a node the mesh does not have: %v", err) } diff --git a/internal/compose/rest.go b/internal/compose/rest.go index 7a073a5..f97ef72 100644 --- a/internal/compose/rest.go +++ b/internal/compose/rest.go @@ -122,7 +122,7 @@ func (c *composer) theRestOfTheMeshNow(exclude string) (catalogue.World, Passes, var firstHeld []catalogue.Held c.unhostable = nil for _, o := range others { - got, err := resolution.Resolve(shelf, o.assigned, o.node, catalogue.World{Unchecked: true, Holdings: holdings}) + got, _, err := resolution.ResolveHolding(shelf, o.assigned, o.node, catalogue.World{Unchecked: true, Holdings: holdings}) if err != nil { // Said, not skipped: a machine dropped here offers nothing and holds nothing. left = append(left, LeftOut{Node: o.node.Name, Err: err}) @@ -162,7 +162,7 @@ func (c *composer) theRestOfTheMeshNow(exclude string) (catalogue.World, Passes, // Each machine is resolved with its own pins, as its plan is. theirs := world theirs.Pinned = o.pins - got, err := resolution.Resolve(shelf, o.assigned, o.node, theirs) + got, _, err := resolution.ResolveHolding(shelf, o.assigned, o.node, theirs) if err != nil { // Said only for the whole-mesh view: with one machine excluded, the view is partial by design. if exclude == "" { diff --git a/internal/compose/rest_test.go b/internal/compose/rest_test.go index 63e6d94..8cb1e61 100644 --- a/internal/compose/rest_test.go +++ b/internal/compose/rest_test.go @@ -89,8 +89,8 @@ func TestTheRestOfTheMeshOffersEveryOtherNodesProvisionsAndNoneOfTheExcluded(t * // 1361/AC2: a node whose assigned set does not resolve is left out and named with today's reason, and the others still offer. func TestANodeWhoseSetDoesNotResolveIsLeftOutAndTheOthersStillOffer(t *testing.T) { in := threeNodes(t) - rivalOne := catalogue.Manifest{Module: "rival-one", Version: "1", Claims: []catalogue.Claim{{Name: "the-seat", Scope: catalogue.ScopeNode}}} - rivalTwo := catalogue.Manifest{Module: "rival-two", Version: "1", Claims: []catalogue.Claim{{Name: "the-seat", Scope: catalogue.ScopeNode}}} + rivalOne := catalogue.Manifest{Module: "rival-one", Version: "1", Resources: []map[string]any{{"id": "r", "type": "directory", "path": "/var/lib/the-same", "mode": "0700"}}} + rivalTwo := catalogue.Manifest{Module: "rival-two", Version: "1", Resources: []map[string]any{{"id": "r", "type": "directory", "path": "/var/lib/the-same", "mode": "0700"}}} in.Catalogue["rival-one"], in.Catalogue["rival-two"] = rivalOne, rivalTwo ace := nodeOf(t, &in, "ace") ace.Assigned = append(ace.Assigned, "rival-one", "rival-two") @@ -208,9 +208,10 @@ func TestAHolderOnRecordSettlesTheSeatInTheRestOfTheMesh(t *testing.T) { t.Fatalf("%v %v %+v", err, left, passes) } in.Holdings = nil - _, _, left, err = TheRestOfTheMesh(in, "ace") - if err != nil || len(left) == 0 { - t.Fatalf("two claimants with nothing on record were not refused: %v %v", err, left) + // Nothing on record: the second claimant is held, not the node left out, and holds nothing in the view (issue 1966). + w, _, left, err := TheRestOfTheMesh(in, "ace") + if err != nil || len(left) != 0 || len(w.Held) != 1 || w.Held[0].Module != "old-broker" { + t.Fatalf("two claimants with nothing on record: %v %v %+v", err, left, w.Held) } }