diff --git a/internal/compose/composed.go b/internal/compose/composed.go index e976e04..5bdc1c0 100644 --- a/internal/compose/composed.go +++ b/internal/compose/composed.go @@ -28,7 +28,7 @@ import ( // writer folds them (issue 1126). Each module's Data says whether its manifest declares data. The compositions are unsigned // (ADR 0593 §6) and not yet by digest (Composed.ByDigest). func Compositions(in Input, node string, fps map[string]string, mode Mode) (out map[string]engcomp.Composed, skipped []string, err error) { - out, skipped, _, err = compositions(in, node, fps, mode, false) + out, skipped, _, _, err = compositions(in, node, fps, mode, false) return out, skipped, err } @@ -45,7 +45,7 @@ const BusUsersStandIn = "the bus's user list" // ErrStoodIn, naming each, so a stand-in is never applied as if it were the value; but for the bus's user list // (BusUsersStandIn), which the bus node holds out of every apply of a composition it made itself. func CompositionsApplied(in Input, node string, fps map[string]string) (map[string]engcomp.Composed, []string, error) { - out, skipped, _, err := compositions(in, node, fps, Reading, true) + out, skipped, _, _, err := compositions(in, node, fps, Reading, true) return out, skipped, err } @@ -56,13 +56,22 @@ func CompositionsApplied(in Input, node string, fps map[string]string) (map[stri // module the candidate adds, or one missing for a module the node runs. A module whose every value is held composes // as CompositionsApplied composes it: the modes differ only in an own secret never issued. func CompositionsShadow(in Input, node string, fps map[string]string) (out map[string]engcomp.Composed, skipped, stood []string, err error) { - return compositions(in, node, fps, Foreseeing, false) + out, skipped, stood, _, err = compositions(in, node, fps, Foreseeing, false) + return out, skipped, stood, err } // CompositionsIn is the node's compositions in mode with what they stand in for answered (stood, sorted, the bus's user // list left out) rather than refused: CompositionsShadow in Foreseeing, and in Reading what CompositionsApplied refuses // before it is refused, so a caller that applies refuses each stand-in to the module it is for alone (issue 1126). func CompositionsIn(in Input, node string, fps map[string]string, mode Mode) (out map[string]engcomp.Composed, skipped, stood []string, err error) { + out, skipped, stood, _, err = compositions(in, node, fps, mode, false) + return out, skipped, stood, err +} + +// CompositionsHolding is CompositionsIn with the modules the node leaves out as they run, each with why: one whose claim cannot be +// held or whose credential was refused (issue 1966). Their compositions are not in out, so a caller that applies keeps what it last +// applied of each and says it held, as for any module refused alone. +func CompositionsHolding(in Input, node string, fps map[string]string, mode Mode) (out map[string]engcomp.Composed, skipped, stood []string, held map[string]string, err error) { return compositions(in, node, fps, mode, false) } @@ -75,7 +84,7 @@ func Folds(inOwn bool, fp string) bool { return inOwn && !strings.HasPrefix(fp, meshcatalogclient.SourcePrefix2) } -func compositions(in Input, node string, fps map[string]string, mode Mode, applied bool) (out map[string]engcomp.Composed, skipped, stood []string, err error) { +func compositions(in Input, node string, fps map[string]string, mode Mode, applied bool) (out map[string]engcomp.Composed, skipped, stood []string, heldBy map[string]string, err error) { reading := in reading.Nodes = append([]Node(nil), in.Nodes...) found := false @@ -99,7 +108,7 @@ func compositions(in Input, node string, fps map[string]string, mode Mode, appli return module != "" && Folds(inOwn[module], fps[module]) } if !found { - return nil, nil, nil, fmt.Errorf("compose: the mesh has no node %q", node) + return nil, nil, nil, nil, fmt.Errorf("compose: the mesh has no node %q", node) } // A folded module built by a build of the old form (Node.RunsOwn) is left to the node: composed from what runs of it // (Carried), so what it provides and requires still shapes the rest and what the others read of it (its published @@ -167,10 +176,10 @@ func compositions(in Input, node string, fps map[string]string, mode Mode, appli } d, problems, err := ComposeIn(reading, node, mode) if err != nil { - return nil, nil, nil, err + return nil, nil, nil, nil, err } if len(problems) > 0 { - return nil, nil, nil, fmt.Errorf("compose: %s's declaration is refused by the node-engine's validator: %v", node, problems) + return nil, nil, nil, nil, fmt.Errorf("compose: %s's declaration is refused by the node-engine's validator: %v", node, problems) } for _, x := range d.StoodIn { // The bus's user list is the bus node's own to follow, written alone and never from a composition the node @@ -186,11 +195,11 @@ func compositions(in Input, node string, fps map[string]string, mode Mode, appli sort.Strings(stood) stood = slices.Compact(stood) // an own secret is both stood in for and foreseen if applied && len(stood) > 0 { - return nil, nil, nil, fmt.Errorf("%w on %s: %s", ErrStoodIn, node, strings.Join(stood, ", ")) + return nil, nil, nil, nil, fmt.Errorf("%w on %s: %s", ErrStoodIn, node, strings.Join(stood, ", ")) } body, err := d.Body() if err != nil { - return nil, nil, nil, err + return nil, nil, nil, nil, err } var envelope struct { Resources []json.RawMessage `json:"resources"` @@ -199,7 +208,7 @@ func compositions(in Input, node string, fps map[string]string, mode Mode, appli Provisioner string `json:"provisioner_account"` } if err := json.Unmarshal(body, &envelope); err != nil { - return nil, nil, nil, err + return nil, nil, nil, nil, err } var kept []json.RawMessage keptPaths := map[string]bool{} @@ -213,7 +222,7 @@ func compositions(in Input, node string, fps map[string]string, mode Mode, appli Path string `json:"path"` } if err := json.Unmarshal(raw, &head); err != nil { - return nil, nil, nil, err + return nil, nil, nil, nil, err } if isCarried(head.Module, head.ID) { continue // what the stub composed of it: the node's own carries what runs of it instead @@ -223,11 +232,11 @@ func compositions(in Input, node string, fps map[string]string, mode Mode, appli // compositionsOf): the node's own composition is then the controller's, byte for byte. var fields map[string]json.RawMessage if err := json.Unmarshal(raw, &fields); err != nil { - return nil, nil, nil, err + return nil, nil, nil, nil, err } delete(fields, "module") if raw, err = json.Marshal(fields); err != nil { - return nil, nil, nil, err + return nil, nil, nil, nil, err } if head.Type == string(declaration.TypeFile) { foldedPaths[head.Path] = true @@ -248,7 +257,7 @@ func compositions(in Input, node string, fps map[string]string, mode Mode, appli if len(carried) > 0 { var err error if kept, foldedPaths, err = carry(kept, foldedPaths, runsOwn, isCarried); err != nil { - return nil, nil, nil, err + return nil, nil, nil, nil, err } for _, p := range runsOwn.Filled { if foldedPaths[p] && !slices.Contains(envelope.Filled, p) { @@ -270,12 +279,12 @@ func compositions(in Input, node string, fps map[string]string, mode Mode, appli out, err = engcomp.Split(node, kept, fps, filled, engcomp.NodeOwn{Adoption: envelope.Adoption, Provisioner: envelope.Provisioner}) if err != nil { - return nil, nil, nil, err + return nil, nil, nil, nil, err } if len(ownFilled) > 0 { own, err := engcomp.NodeKey(node) if err != nil { - return nil, nil, nil, err + return nil, nil, nil, nil, err } c := out[own] c.Filled = append(c.Filled, ownFilled...) @@ -295,7 +304,7 @@ func compositions(in Input, node string, fps map[string]string, mode Mode, appli skipped = append(skipped, m) } sort.Strings(skipped) - return out, skipped, stood, nil + return out, skipped, stood, d.Held, nil } // RunsAnyOf says runs (a composition as it runs) holds a resource of module, by its module or the module its id names. diff --git a/internal/compose/credentials_needed.go b/internal/compose/credentials_needed.go index ec36c71..cd9306c 100644 --- a/internal/compose/credentials_needed.go +++ b/internal/compose/credentials_needed.go @@ -23,12 +23,12 @@ 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, held, err := c.resolutionHolding(node) if err != nil { return nil, nil, err } for _, n := range resolved.Needs { - if n.ByRecord || n.From == "" || credentialless(n) { + if _, isHeld := held[n.For]; isHeld || n.ByRecord || n.From == "" || credentialless(n) { continue } asConsumer = append(asConsumer, CredentialKey{Provision: n.Name, Consumer: node, ConsumerModule: n.For, 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/frombus/answer.go b/internal/compose/frombus/answer.go index 702f1cb..9e0ecb9 100644 --- a/internal/compose/frombus/answer.go +++ b/internal/compose/frombus/answer.go @@ -337,7 +337,8 @@ func composeEach(in compose.Input, node string, fps map[string]string, applied b hold(m, failed[m], "") } for { - out, skipped, stood, err = compose.CompositionsIn(in, node, fps, mode) + var leftOut map[string]string + out, skipped, stood, leftOut, err = compose.CompositionsHolding(in, node, fps, mode) if err != nil { module, leave := culprit(in, node, fps, mode, failed, err) if module == "" { @@ -346,6 +347,18 @@ func composeEach(in compose.Input, node string, fps map[string]string, applied b hold(module, err.Error(), leave) continue } + // A module the node leaves out because its claim cannot be held or its credential was refused (issue 1966) is held with + // those words: its handed composition stays and the check names it refused; the rest composes. + heldNow := false + for _, m := range sortedKeys(leftOut) { + if _, done := failed[m]; !done { + hold(m, leftOut[m], "") + heldNow = true + } + } + if heldNow { + continue + } again := false if applied && len(stood) > 0 { placed := map[string]bool{} diff --git a/internal/compose/frombus/held_rows_test.go b/internal/compose/frombus/held_rows_test.go index e4d5673..5bc01e9 100644 --- a/internal/compose/frombus/held_rows_test.go +++ b/internal/compose/frombus/held_rows_test.go @@ -300,3 +300,35 @@ func aHeldModuleOfItsOwnAccountNeverRefusesTheNode(t *testing.T) { } } } + +// Issue 1966: two modules claiming one seat held once cost the second claimant, not the node: Compose holds it with the claim's +// words, composes the first, and (the node-engine keeping what it handed of a held module) leaves its composition out, whether or +// not it runs. +func TestASecondClaimantOfASeatIsHeldAndTheNodeComposesTheRest(t *testing.T) { + first := `{"module":"alpha","version":"1","claims":[{"name":"the-seat","scope":"node"}]}` + second := `{"module":"beta","version":"1","claims":[{"name":"the-seat","scope":"node"}]}` + for _, running := range []bool{false, true} { + b, fps := aNode(t, first, second) + scope := Scope{Node: "n1"} + if running { + scope.Running = []engcomp.Composed{{Node: "n1", Module: "beta", FP: fps["beta"], Resources: nil}} + } + got, _, held, err := Compose(context.Background(), b, Mesh{}, scope, fps) + if err != nil || len(held) != 1 || !strings.Contains(held["beta"], "alpha and beta both claim") { + t.Fatalf("running %v: err %v, held %v; want beta held with the claim's words", running, err, held) + } + for _, c := range got { + if c.Module == "beta" { + t.Errorf("running %v: the held claimant is composed: %+v", running, c) + } + } + // A check that adds the second claimant is refused for it, in the claim's words, and not judged to compose. + p := b.assignments["n1"] + ans := Answer(context.Background(), b, Mesh{}, Scope{Node: "n1", Assignments: map[string]*meshmoduleassigner.Assignments{"n1": &p}}, + fps, scope.Running, "p-h1", 1) + if ans.Verdict != nodeengine.CandidateAnswerVerdictRefused || len(ans.Refusals) != 1 || ans.Refusals[0].Module != "beta" || + !strings.Contains(ans.Refusals[0].Why, "both claim") { + t.Errorf("running %v: the answer is %+v, want beta refused with the claim's words", running, ans) + } + } +} 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.go b/internal/compose/node.go index bfd0408..1d52253 100644 --- a/internal/compose/node.go +++ b/internal/compose/node.go @@ -37,6 +37,9 @@ type Declared struct { // ADR 0163, rule 6), sorted; LeftOutWhy is why each was. LeftOut []string LeftOutWhy map[string]string + // Held is, of those, each module left out because its claim cannot be held or its credential was refused, and why (issue + // 1966): the node keeps what runs of it and the composer says it held. Not on the wire. + Held map[string]string // Not on the wire: Received map[string]map[string][]catalogue.Contribution @@ -199,7 +202,7 @@ func (c *composer) declaration(nodeName string, mode Mode) (Declared, error) { if err != nil { return Declared{}, err } - out := Declared{Resources: composed.Resources, Adoption: adoptionFor(plan, node, composed), + out := Declared{Held: resolved.Held, Resources: composed.Resources, Adoption: adoptionFor(plan, node, composed), Received: composed.Received, Mesh: with.Mesh, BusUsers: with.BusUsers, LeftOut: sortedKeysOf(composed.LeftOut), LeftOutWhy: composed.LeftOut, Withheld: rendered.Withheld, Unbound: rendered.Unbound, 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..89b6ffb 100644 --- a/internal/compose/resolution/resolve.go +++ b/internal/compose/resolution/resolve.go @@ -26,12 +26,17 @@ func (r *Refusal) Error() string { return "these assignments cannot be applied:\n - " + strings.Join(r.Problems, "\n - ") } -// Resolve works out everything a node runs, from what was assigned to it. -// -// The catalogue is every module the mesh knows about; assigned is what a person put on this node. -// 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) { +// ResolveHolding works out everything a node runs, from what was assigned to it. The catalogue is every module the mesh knows +// about; assigned is what a person put on this node. What comes back is the closure -- assigned modules plus everything they +// require -- or a refusal naming every reason it could not be closed. 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) +} + +func resolve(catalogue map[string]mcat.Manifest, assigned []string, node mcat.Node, world mcat.World) (mcat.Resolution, map[string]string, error) { elsewhere := world.Held var problems []string @@ -593,8 +598,7 @@ 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) problems = append(problems, checkResources(resolution.Modules)...) resolution.Claims = claims @@ -615,10 +619,14 @@ func Resolve(catalogue map[string]mcat.Manifest, assigned []string, node mcat.No } if len(problems) > 0 { + // A pass refused anyway says every reason at once, a held module's among them. + for _, why := range held { + problems = append(problems, why) + } 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 @@ -672,12 +680,29 @@ func checkCapabilities(modules []mcat.Manifest, node mcat.Node) []string { return problems } -// checkClaims refuses two modules holding one singular thing. -// -// 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 +// holdClaims refuses two modules holding one singular thing, within this node's own set and against what is held elsewhere for +// the wider scopes: 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 +742,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 +759,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 +787,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/resolution/strict_test.go b/internal/compose/resolution/strict_test.go new file mode 100644 index 0000000..6c5680e --- /dev/null +++ b/internal/compose/resolution/strict_test.go @@ -0,0 +1,35 @@ +package resolution + +import ( + "sort" + + mcat "git.novox.be/novox/mesh-sdk/go/catalogue" +) + +// Resolve is ResolveHolding for a test that wants a claim problem as a refusal of the whole set, as before issue 1966. +func Resolve(catalogue map[string]mcat.Manifest, assigned []string, node mcat.Node, world mcat.World) (mcat.Resolution, error) { + got, held, err := ResolveHolding(catalogue, assigned, node, world) + if err != nil { + return mcat.Resolution{}, err + } + if len(held) > 0 { + var problems []string + for _, why := range held { + problems = append(problems, why) + } + sort.Strings(problems) + return mcat.Resolution{}, &Refusal{Problems: problems} + } + return got, nil +} + +// checkClaims is holdClaims with the problems as a list. +func checkClaims(modules []mcat.Manifest, node mcat.Node, elsewhere []mcat.Held, holdings []mcat.Held) ([]mcat.Held, []string) { + held, why := holdClaims(modules, node, elsewhere, holdings) + var problems []string + for _, w := range why { + problems = append(problems, w) + } + sort.Strings(problems) + 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) } }