diff --git a/cmd/mesh-builder/check_spec_test.go b/cmd/mesh-builder/check_spec_test.go index 56f40d21..808c32b9 100644 --- a/cmd/mesh-builder/check_spec_test.go +++ b/cmd/mesh-builder/check_spec_test.go @@ -36,3 +36,17 @@ func TestTheSiblingsAnAskNamesReachTheSpecAndTheOutcome(t *testing.T) { t.Fatalf("a request that names none has siblings %+v", plain.Siblings) } } + +// 1826/AC2: the held values a check request carries reach the spec; a request that carries none leaves it empty. +func TestTheHeldValuesAnAskCarriesReachTheSpec(t *testing.T) { + spec := checkSpecOf(link.BuildRequest{Check: &link.CheckRequest{Owner: "novox", Repo: "mesh-catalog", + HeldConfig: []link.HeldConfig{{Pull: "novox/mesh-catalog#434", Head: "a", Node: "ace", Module: "n8n", Key: "media-gid", + Value: []byte("2000"), Approved: "the operator"}}}}) + if len(spec.Held) != 1 || spec.Held[0].Node != "ace" || spec.Held[0].Key != "media-gid" || string(spec.Held[0].Value) != "2000" || + spec.Held[0].Approved != "the operator" { + t.Fatalf("the spec reads %+v", spec.Held) + } + if plain := checkSpecOf(link.BuildRequest{Check: &link.CheckRequest{Owner: "o", Repo: "r"}}); plain.Held != nil { + t.Fatalf("a request that carries none has %+v", plain.Held) + } +} diff --git a/cmd/mesh-builder/main.go b/cmd/mesh-builder/main.go index 09d57370..3ea5810b 100644 --- a/cmd/mesh-builder/main.go +++ b/cmd/mesh-builder/main.go @@ -505,6 +505,10 @@ func checkSpecOf(request link.BuildRequest) builder.CheckSpec { Modules: c.Modules, New: c.New, Manifests: c.Manifests, Judge: c.Judge, Base: c.Base, Toolchain: builder.ToolchainOf(request.Held), Toolchains: builder.ToolchainsOf(request.Held), Tests: c.Tests, StoreOnly: c.StoreOnly, SiblingsUnknown: c.SiblingsUnknown} + for _, h := range c.HeldConfig { + spec.Held = append(spec.Held, builder.HeldValue{Pull: h.Pull, Head: h.Head, Node: h.Node, Module: h.Module, Key: h.Key, + Value: h.Value, Approved: h.Approved, By: h.By, At: h.At}) + } for _, s := range c.Siblings { spec.Siblings = append(spec.Siblings, builder.Sibling{Repository: s.Repository, Number: s.Number, Head: s.Head, Base: s.Base, Dir: s.Dir}) diff --git a/cmd/mesh-controller/checks.go b/cmd/mesh-controller/checks.go index 048f9a0d..a408e842 100644 --- a/cmd/mesh-controller/checks.go +++ b/cmd/mesh-controller/checks.go @@ -191,6 +191,16 @@ func (f following) pullUpdated(ctx context.Context, p link.PullUpdated, w io.Wri scope.Modules, scope.Dependents, scope.New, scope.Manifests = nil, nil, nil, nil composedGate(&direct) } + // **Mesh-config is told what the head declares at every announced head of a pull request in a repository that holds + // modules** (novox/hq ADR 0502 clauses 2 and 6, issue 1826), also when the head touches no module: a head that reverts + // the change must drop the value held for it. Asked of nothing else here. + this := pullAtOf(p) + if this.Truncated { + this.Paths, this.Truncated = truncatedPaths(entries, p.Owner, p.Repo, scope.Modules), false + } + if scope.gated() || holdsModules(entries, p.Owner, p.Repo) { + tellPullDeclares(ctx, w, []pullAt{this}) + } switch { case !scope.gated() && !scope.Mesh: fmt.Fprintf(w, "%s/%s#%d (%.8s): %s, and the repository is not the mesh's: said, nothing run\n", @@ -209,6 +219,9 @@ func (f following) pullUpdated(ctx context.Context, p link.PullUpdated, w io.Wri } request.Check.Plan = &plan request.Check.Composed = composed + if !composed { + carryHeld(ctx, w, &request, []pullAt{this}) + } // **The group's other open pull requests are read beside it** (novox/hq ADR 0382 §1). siblingsFrom(&request, p, entries, lines, groupsErr) seat := buildSeatHeld(ctx) diff --git a/cmd/mesh-controller/delivery.go b/cmd/mesh-controller/delivery.go index 3d548ffa..e8af7c27 100644 --- a/cmd/mesh-controller/delivery.go +++ b/cmd/mesh-controller/delivery.go @@ -1177,6 +1177,12 @@ func askGroupCheck(ctx context.Context, open *stores, group string, members []or return "", err } request.Check.Group = group + // **The values held for the keys the members introduce are told and carried** (novox/hq ADR 0502, issue 1826). + var pulls []pullAt + for i, s := range changeSourcesOf(members, entries, reach) { + pulls = append(pulls, pullAt{changeSource: s, Number: members[i].pull().Number}) + } + withHeldConfig(ctx, os.Stderr, &request, pulls, true) // A group's check is queued as ready when any member's pull request is marked ready. request.Check.Ready = slices.ContainsFunc(members, func(m orderMember) bool { return m.Ready }) world, err := restOfTheMesh(ctx, open, shelfOf(entries), "") diff --git a/cmd/mesh-controller/merge_gate.go b/cmd/mesh-controller/merge_gate.go index dad7db22..35343ec3 100644 --- a/cmd/mesh-controller/merge_gate.go +++ b/cmd/mesh-controller/merge_gate.go @@ -65,6 +65,9 @@ type mergeCheckInput struct { // group are the other heads of a delivery group composed with this one (novox/hq ADR 0239), each laid // over the mesh after the one before: the group judged as one future state. group []groupChange + // held are the approved config values held for the keys the pull requests introduce (novox/hq ADR 0502 clause 4), + // laid over the composition with the change only. + held []heldValue // admin is a PostgreSQL the gate may create and drop databases in. admin string // beside is the directory holding the core clones beside the check (MESH_CHECK_BESIDE), for finding moves. @@ -189,6 +192,8 @@ func mergeGateCommand(ctx context.Context, args []string) error { "--changed, for a list too long for an argument (novox/hq issue 1247)") group := set.String("group", "", "a delivery group's other heads, as JSON [{repository, tree, changed}], composed "+ "with this one as one future state (novox/hq ADR 0239)") + heldFile := set.String("held", "", "a file of the config values approved for keys the change's pull requests introduce, as JSON "+ + "[{pull, node, module, key, value, approved_by}], laid over the composition with the change (novox/hq ADR 0502)") asJSON := set.Bool("json", false, "print the verdict as JSON") if _, err := parseAround(set, args); err != nil { return err @@ -208,6 +213,10 @@ func mergeGateCommand(ctx context.Context, args []string) error { if err != nil { return err } + held, err := readHeld(*heldFile) + if err != nil { + return err + } out := io.Writer(os.Stdout) stdout := os.Stdout if *asJSON { @@ -223,7 +232,7 @@ func mergeGateCommand(ctx context.Context, args []string) error { return err } v, err := judgeChange(ctx, mergeCheckInput{facts: f, repository: *repository, tree: *tree, changed: changedList, - group: others, admin: *admin, say: out, beside: os.Getenv("MESH_CHECK_BESIDE")}) + group: others, held: held, admin: *admin, say: out, beside: os.Getenv("MESH_CHECK_BESIDE")}) os.Stdout = stdout if err != nil { return err @@ -387,7 +396,16 @@ func judgeChange(ctx context.Context, in mergeCheckInput) (mergeVerdict, error) } } sort.Strings(added) - after, tried, notes, err := composeEveryMachine(ctx, in, change, changeSources, added) + // **The held values are laid in this composition only** (novox/hq ADR 0502 clause 4): the mesh as it is does not + // declare their keys, so the base composition would refuse them. + withHeld := in + var heldNotes []string + withHeld.facts, heldNotes = layHeld(in.facts, in.held, change) + for _, n := range heldNotes { + say("%s", n) + } + v.Notes = append(v.Notes, heldNotes...) + after, tried, notes, err := composeEveryMachine(ctx, withHeld, change, changeSources, added) if err != nil { return v, fmt.Errorf("the mesh with the change could not be raised: %w", err) } diff --git a/cmd/mesh-controller/merge_gate_held.go b/cmd/mesh-controller/merge_gate_held.go new file mode 100644 index 00000000..3974e5ab --- /dev/null +++ b/cmd/mesh-controller/merge_gate_held.go @@ -0,0 +1,135 @@ +package main + +import ( + "encoding/json" + "fmt" + "os" + "sort" + "strings" + + "git.novox.be/novox/mesh-sdk/go/catalogue" + snapshot "git.novox.be/novox/mesh-sdk/go/facts" +) + +// Config values held for the keys an open pull request introduces (novox/hq ADR 0502 clause 4, issue 1826): the gate +// lays each approved one over its module's values on its node in the composition with the change, and in that one +// only. In the mesh as it is the key is not declared, so the value would be refused there. + +// heldValue is one approved held value as the build agent writes it (--held): the value of Key for Module on Node, +// a machine's real name, which the snapshot holds only as a pseudonym. +type heldValue struct { + Pull string `json:"pull"` + Head string `json:"head,omitempty"` + Node string `json:"node"` + Module string `json:"module"` + Key string `json:"key"` + Value json.RawMessage `json:"value"` + Approved string `json:"approved,omitempty"` + By string `json:"by,omitempty"` + At string `json:"at,omitempty"` +} + +// readHeld reads the file --held names; none named, none held. +func readHeld(path string) ([]heldValue, error) { + if path == "" { + return nil, nil + } + raw, err := os.ReadFile(path) + if err != nil { + return nil, err + } + var held []heldValue + if err := json.Unmarshal(raw, &held); err != nil { + return nil, fmt.Errorf("the held values are not readable: %w", err) + } + for _, h := range held { + if h.Node == "" || h.Module == "" || h.Key == "" || len(h.Value) == 0 { + return nil, fmt.Errorf("a held value of %s names no node, module, key or value", h.Pull) + } + } + return held, nil +} + +// layHeld is the snapshot with each held value laid as the value of its key in its node's layer of its module, and what +// it said of them. The snapshot given is not changed. **A held value for a machine the snapshot does not hold, or a +// module the change does not define, is said and not laid**, never dropped silently; one for a module not assigned to +// its machine is laid and said. The notes name machines as the snapshot does (roles and pseudonym), not by their names. +func layHeld(f snapshot.Facts, held []heldValue, shelf map[string]catalogue.Manifest) (snapshot.Facts, []string) { + if len(held) == 0 { + return f, nil + } + out := f + out.Machines = make([]snapshot.Machine, len(f.Machines)) + copy(out.Machines, f.Machines) + var notes, laid []string + pulls := map[string]bool{} + for _, h := range held { + at := -1 + pseudonym := snapshot.Pseudonym("machine", h.Node) + for i, m := range out.Machines { + if m.Name == pseudonym { + at = i + } + } + if at < 0 { + notes = append(notes, fmt.Sprintf("the value held for %s of %s (%s=%s) is not composed: the snapshot holds no machine %s", + h.Module, h.Pull, h.Key, h.Value, pseudonym)) + continue + } + if _, defined := shelf[h.Module]; !defined { + notes = append(notes, fmt.Sprintf("the value held for %s of %s (%s=%s) is not composed: the change defines no module %s", + h.Module, h.Pull, h.Key, h.Value, h.Module)) + continue + } + var value any + if err := json.Unmarshal(h.Value, &value); err != nil { + notes = append(notes, fmt.Sprintf("the value held for %s of %s (%s) is not composed: it is not a value: %v", + h.Module, h.Pull, h.Key, err)) + continue + } + m := &out.Machines[at] + layers := make([]snapshot.Settings, len(m.Settings)) + copy(layers, m.Settings) + found := false + for i := range layers { + if layers[i].Module == h.Module { + values := make(map[string]any, len(layers[i].Values)+1) + for k, v := range layers[i].Values { + values[k] = v + } + values[h.Key] = value + layers[i].Values = values + found = true + } + } + if !found { + layers = append(layers, snapshot.Settings{Module: h.Module, Values: map[string]any{h.Key: value}}) + sort.Slice(layers, func(a, b int) bool { return layers[a].Module < layers[b].Module }) + } + m.Settings = layers + assigned := false + for _, a := range m.Assigned { + assigned = assigned || a == h.Module + } + by := "" + if h.Approved != "" { + by = ", approved by " + h.Approved + } + line := fmt.Sprintf("%s on %s %s=%s%s", h.Module, m.Described(), h.Key, h.Value, by) + if !assigned { + line += " (not assigned there in the snapshot)" + } + laid = append(laid, line) + pulls[h.Pull] = true + } + if len(laid) > 0 { + var names []string + for p := range pulls { + names = append(names, p) + } + sort.Strings(names) + notes = append([]string{fmt.Sprintf("composed with %d value(s) held for %s: %s", len(laid), strings.Join(names, ", "), + strings.Join(laid, "; "))}, notes...) + } + return out, notes +} diff --git a/cmd/mesh-controller/merge_gate_held_test.go b/cmd/mesh-controller/merge_gate_held_test.go new file mode 100644 index 00000000..922a02d0 --- /dev/null +++ b/cmd/mesh-controller/merge_gate_held_test.go @@ -0,0 +1,85 @@ +package main + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "git.novox.be/novox/mesh-sdk/go/catalogue" + snapshot "git.novox.be/novox/mesh-sdk/go/facts" +) + +func heldGateFacts() snapshot.Facts { + return snapshot.Facts{Machines: []snapshot.Machine{ + {Name: snapshot.Pseudonym("machine", "ace"), Roles: []string{"the build node"}, Assigned: []string{"n8n"}, + Settings: []snapshot.Settings{{Module: "n8n", Values: map[string]any{"port": 5678.0}}}}, + {Name: snapshot.Pseudonym("machine", "g14")}, + }} +} + +var heldShelf = map[string]catalogue.Manifest{"n8n": {Module: "n8n"}} + +// 1826/AC3: a held value is laid in its node's layer of its module, beside what the layer holds, and said. +func TestAHeldValueIsLaidOverItsNodesLayerAndSaid(t *testing.T) { + f := heldGateFacts() + got, notes := layHeld(f, []heldValue{{Pull: "novox/mesh-catalog#434", Node: "ace", Module: "n8n", Key: "media-gid", + Value: []byte("2000"), Approved: "the operator"}}, heldShelf) + layer := got.Machines[0].Settings[0].Values + if layer["media-gid"] != 2000.0 || layer["port"] != 5678.0 { + t.Fatalf("the layer holds %v", layer) + } + if _, has := f.Machines[0].Settings[0].Values["media-gid"]; has { + t.Fatal("the snapshot given was changed") + } + if len(notes) != 1 || !strings.Contains(notes[0], "composed with 1 value(s) held for novox/mesh-catalog#434") || + !strings.Contains(notes[0], "media-gid=2000") || !strings.Contains(notes[0], "approved by the operator") || + strings.Contains(notes[0], "ace") { + t.Fatalf("said %q", notes) + } +} + +// 1826/AC3: a machine with no layer of the module gets one. +func TestAHeldValueMakesTheLayerAMachineLacks(t *testing.T) { + got, _ := layHeld(heldGateFacts(), []heldValue{{Pull: "p", Node: "g14", Module: "n8n", Key: "k", Value: []byte(`"x"`)}}, heldShelf) + s := got.Machines[1].Settings + if len(s) != 1 || s[0].Module != "n8n" || s[0].Values["k"] != "x" { + t.Fatalf("layers %+v", s) + } +} + +// 1826/AC3: a held value for a machine or a module the snapshot or the change does not hold is said, never laid. +func TestAHeldValueForWhatIsNotHeldIsSaidNotLaid(t *testing.T) { + f := heldGateFacts() + got, notes := layHeld(f, []heldValue{ + {Pull: "p", Node: "nowhere", Module: "n8n", Key: "k", Value: []byte("1")}, + {Pull: "p", Node: "ace", Module: "ghost", Key: "k", Value: []byte("1")}, + }, heldShelf) + if len(notes) != 2 || !strings.Contains(notes[0], "holds no machine") || !strings.Contains(notes[1], "defines no module ghost") { + t.Fatalf("said %q", notes) + } + if _, has := got.Machines[0].Settings[0].Values["k"]; has { + t.Fatal("laid") + } +} + +// 1826/AC3: none held lays nothing and says nothing; the file is read and refuses a value without a node. +func TestNoHeldValuesLayNothingAndTheFileIsRead(t *testing.T) { + if got, notes := layHeld(heldGateFacts(), nil, heldShelf); len(notes) != 0 || len(got.Machines) != 2 { + t.Fatalf("%v %v", got, notes) + } + dir := t.TempDir() + good := filepath.Join(dir, "good.json") + os.WriteFile(good, []byte(`[{"pull":"p","node":"ace","module":"n8n","key":"k","value":1}]`), 0o644) + if h, err := readHeld(good); err != nil || len(h) != 1 || h[0].Key != "k" { + t.Fatalf("%v %v", h, err) + } + bad := filepath.Join(dir, "bad.json") + os.WriteFile(bad, []byte(`[{"pull":"p","module":"n8n","key":"k","value":1}]`), 0o644) + if _, err := readHeld(bad); err == nil { + t.Fatal("a value without a node was read") + } + if h, err := readHeld(""); h != nil || err != nil { + t.Fatalf("%v %v", h, err) + } +} diff --git a/cmd/mesh-controller/merge_gate_integration_test.go b/cmd/mesh-controller/merge_gate_integration_test.go index 32965523..fca93fd6 100644 --- a/cmd/mesh-controller/merge_gate_integration_test.go +++ b/cmd/mesh-controller/merge_gate_integration_test.go @@ -526,3 +526,57 @@ func TestAMemberJudgedAloneSeesAMoveThroughTheClonesBeside(t *testing.T) { t.Errorf("nothing beside, yet moves %v", got) } } + +// 1826/AC3 (integration level, a throwaway store): a change whose container says ${config:media-gid} fails the gate for +// want of the value, passes it with the value the operator approved held for its pull request, and the mesh as it is +// composes the same either way. +func TestAChangeNeedingAHeldValueFailsWithoutItAndPassesWithIt(t *testing.T) { + open := aMesh(t) + ctx := t.Context() + trunk := `{"module":"n8n","version":"1","resources":[{"id":"server","type":"container","name":"n8n", + "image":"n8nio/n8n@sha256:4846eb2f4b874ab04cde7fc1e249d2ddaec66e9aea64439beb2972cfea88e3c0"}]}` + m, err := catalogue.ParseManifest([]byte(trunk)) + if err != nil { + t.Fatal(err) + } + if err := open.inventory.RegisterModule(ctx, m, inventory.Source{Repository: "novox/mesh-catalog", + Path: "modules/n8n", BuiltFrom: "c0ffee"}); err != nil { + t.Fatal(err) + } + if _, err := assign(ctx, open, "laptop", "n8n"); err != nil { + t.Fatal(err) + } + f, err := gatherFacts(ctx, open, "2.11.17") + if err != nil { + t.Fatal(err) + } + for i := range f.Machines { + f.Machines[i].Capabilities = append(f.Machines[i].Capabilities, snapshot.Capability{Name: "container-groups", Present: true}) + } + tree := aTree(t, map[string]string{"n8n": n8nWithAGroup}) + judge := func(held []heldValue) mergeVerdict { + v, err := judgeChange(t.Context(), mergeCheckInput{facts: f, admin: os.Getenv("MESH_TEST_POSTGRES"), + repository: "novox/mesh-catalog", tree: tree, changed: []string{"modules/n8n/module.json"}, held: held}) + if err != nil { + t.Fatal(err) + } + return v + } + without := judge(nil) + if without.Verdict != "fail" || !strings.Contains(without.Report(), "media-gid") { + t.Fatalf("a required value nothing sets passed, or failed for another reason:\n%s", without.Report()) + } + with := judge([]heldValue{{Pull: "novox/mesh-catalog#434", Node: "laptop", Module: "n8n", Key: "media-gid", + Value: []byte("2000"), Approved: "the operator"}}) + if with.Verdict == "fail" || with.Verdict == "error" { + t.Fatalf("the held value did not make it compose:\n%s", with.Report()) + } + if !strings.Contains(with.Report(), "composed with 1 value(s) held for novox/mesh-catalog#434") { + t.Fatalf("the value laid is not said:\n%s", with.Report()) + } + for i := range with.Machines { + if with.Machines[i].Base.Composes != without.Machines[i].Base.Composes { + t.Fatalf("the mesh as it is composes differently with a held value: %+v / %+v", with.Machines[i].Base, without.Machines[i].Base) + } + } +} diff --git a/cmd/mesh-controller/pull_declares.go b/cmd/mesh-controller/pull_declares.go new file mode 100644 index 00000000..b639a6b3 --- /dev/null +++ b/cmd/mesh-controller/pull_declares.go @@ -0,0 +1,211 @@ +package main + +import ( + "context" + "encoding/json" + "fmt" + "io" + "path" + "reflect" + "slices" + "time" + + "github.com/nats-io/nats.go" + + "github.com/novox/mesh-controller/internal/broker" + "github.com/novox/mesh-controller/internal/inventory" + "github.com/novox/mesh-controller/internal/link" +) + +// What the controller tells mesh-config of an open pull request, and asks it back (novox/hq ADR 0502 clauses 2 and 4, +// issue 1826): a config key a pull request introduces is not declared on the trunk, so a value for it can be judged +// only against the pull request's own declaration, which the core reads from the forge and mesh-config never does +// (ADR 0495 §1). The call is mesh-config's `config` verb in the controller's own name, actions `pull-declares` and +// `held`; the shapes are those of the interface the two sides agreed. + +// meshConfigWait is how long the controller waits for mesh-config before it goes on without: a check is never held up +// by a module that does not answer. +const meshConfigWait = 10 * time.Second + +// meshConfigAsk is one call of mesh-config's `config` verb in the controller's own name, answering its result. A +// variable so a test needs no bus. +var meshConfigAsk = func(ctx context.Context, args map[string]any) ([]byte, error) { + var result []byte + err := onTheBus(func(conn *nats.Conn) error { + answer, err := link.AskMeshSeatToolAs(ctx, conn, broker.ControllerName, "mesh-config", "config", args, meshConfigWait) + if err != nil { + return err + } + if answer.Error != "" { + return fmt.Errorf("%s", answer.Error) + } + result = answer.Result + return nil + }) + return result, err +} + +// pullAt is an open pull request at a head: the change to read, and its number. +type pullAt struct { + changeSource + Number int +} + +// pullAtOf is a pull request announced, as the change it makes over what it merges into. +func pullAtOf(p link.PullUpdated) pullAt { + old := p.BaseSHA + if old == "" { + old = p.Base + } + return pullAt{Number: p.Number, changeSource: changeSource{Owner: p.Owner, Repo: p.Repo, New: p.Commit, Old: old, + Paths: p.Paths, Truncated: p.PathsTruncated, Removed: p.Removed}} +} + +func pullKey(owner, repo string, number int) string { + return fmt.Sprintf("%s/%s#%d", owner, repo, number) +} + +func (p pullAt) key() string { return pullKey(p.Owner, p.Repo, p.Number) } + +// pullDeclarations are the config declarations a pull request's head adds or changes over what it merges into, by +// module, in the shape of the declared-config mirror. The bool says the head could be read: false for a pull request whose +// paths are cut (the caller cuts them to the manifests of the modules it moves) or whose base is unknown, which is then +// told nothing, never an empty set. +func pullDeclarations(ctx context.Context, p pullAt) ([]inventory.DeclaredConfigMirrored, bool, error) { + var out []inventory.DeclaredConfigMirrored + if p.Truncated || p.Old == "" { + return nil, false, nil + } + for _, file := range p.Paths { + if path.Base(file) != moduleManifestFile || slices.Contains(p.Removed, file) { + continue + } + now, exists, err := manifestCached(ctx, p.Owner, p.Repo, file, p.New) + if err != nil { + return nil, false, err + } + if !exists || now.Module == "" { + continue + } + was, _, err := manifestCached(ctx, p.Owner, p.Repo, file, p.Old) + if err != nil { + return nil, false, err + } + have, err := inventory.DeclaredOf(now.Settings) + if err != nil { + return nil, false, fmt.Errorf("%s at %.8s: %w", file, p.New, err) + } + had, err := inventory.DeclaredOf(was.Settings) + if err != nil { + return nil, false, fmt.Errorf("%s at %.8s: %w", file, p.Old, err) + } + changed := map[string]inventory.DeclaredMirrored{} + for key, d := range have { + if before, there := had[key]; !there || !reflect.DeepEqual(before, d) { + changed[key] = d + } + } + if len(changed) > 0 { + out = append(out, inventory.DeclaredConfigMirrored{Module: now.Module, Settings: changed}) + } + } + return out, true, nil +} + +// tellPullDeclares tells mesh-config, for each pull request at every announced head, what the head declares, an empty set +// when it declares nothing new: mesh-config re-judges the values held at every head, so a head that reverts the change +// drops them (ADR 0502 clause 6). A +// failure is said to w and the check goes on: the held values are then none (ADR 0502 clause 2). +func tellPullDeclares(ctx context.Context, w io.Writer, pulls []pullAt) { + for _, p := range pulls { + mods, readable, err := pullDeclarations(ctx, p) + if err != nil { + fmt.Fprintf(w, "%s: what its head declares cannot be read, so mesh-config is told nothing: %v\n", p.key(), err) + continue + } + if !readable { + continue + } + declares := map[string]map[string]inventory.DeclaredMirrored{} + for _, m := range mods { + declares[m.Module] = m.Settings + } + if _, err := meshConfigAsk(ctx, map[string]any{"action": "pull-declares", "pull": p.key(), "head": p.New, + "declares": declares}); err != nil { + fmt.Fprintf(w, "%s: mesh-config cannot be told what its head declares (%v); the check goes on without held values\n", + p.key(), err) + } + } +} + +// heldFor asks mesh-config for the approved held values of these pull requests at their heads (ADR 0502 clause 4). Only +// those of the pull requests asked about are carried. A failure is said to w and answers none. +func heldFor(ctx context.Context, w io.Writer, pulls []pullAt) []link.HeldConfig { + if len(pulls) == 0 { + return nil + } + type pullHead struct { + Pull string `json:"pull"` + Head string `json:"head"` + } + asked := make([]pullHead, 0, len(pulls)) + for _, p := range pulls { + asked = append(asked, pullHead{p.key(), p.New}) + } + raw, err := meshConfigAsk(ctx, map[string]any{"action": "held", "pulls": asked}) + if err != nil { + fmt.Fprintf(w, "mesh-config cannot say what it holds for these pull requests (%v); the check goes on without held values\n", err) + return nil // empty-on-error: said above; a check without held values fails for want of the value and says so, it never passes on a guess + } + var answer struct { + Held []link.HeldConfig `json:"held"` + Skipped []struct { + Pull string `json:"pull"` + Why string `json:"why"` + } `json:"skipped"` + } + if len(raw) == 0 { + return nil + } + if err := json.Unmarshal(raw, &answer); err != nil { + fmt.Fprintf(w, "mesh-config's held values cannot be read (%v); the check goes on without them\n", err) + return nil // empty-on-error: said above; a check without held values fails for want of the value and says so, it never passes on a guess + } + for _, s := range answer.Skipped { + fmt.Fprintf(w, "%s: mesh-config holds a value it does not carry: %s\n", s.Pull, s.Why) + } + var out []link.HeldConfig + for _, h := range answer.Held { + for _, p := range pulls { + if h.Pull == p.key() { + out = append(out, h) + break + } + } + } + return out +} + +// withHeldConfig tells mesh-config what these pull requests declare and puts the approved held values of them on the +// check request (ADR 0502 clause 4). With carry false, a member of a group whose composed check composes, only the +// declarations are told. +func withHeldConfig(ctx context.Context, w io.Writer, request *link.BuildRequest, pulls []pullAt, carry bool) { + tellPullDeclares(ctx, w, pulls) + if carry { + carryHeld(ctx, w, request, pulls) + } +} + +// carryHeld puts the approved held values of these pull requests on the check request. +func carryHeld(ctx context.Context, w io.Writer, request *link.BuildRequest, pulls []pullAt) { + if request.Check != nil { + request.Check.HeldConfig = heldFor(ctx, w, pulls) + } +} + +// holdsModules is whether a module of the mesh is built from this repository. +func holdsModules(entries []inventory.Entry, owner, repo string) bool { + return slices.ContainsFunc(entries, func(e inventory.Entry) bool { + return sameRepository(e.Source.Repository, link.SourceMoved{Owner: owner, Repo: repo}) + }) +} diff --git a/cmd/mesh-controller/pull_declares_test.go b/cmd/mesh-controller/pull_declares_test.go new file mode 100644 index 00000000..c325e79a --- /dev/null +++ b/cmd/mesh-controller/pull_declares_test.go @@ -0,0 +1,209 @@ +package main + +import ( + "context" + "encoding/json" + "io" + "strings" + "testing" + + "git.novox.be/novox/mesh-sdk/go/catalogue" + "github.com/novox/mesh-controller/internal/inventory" + "github.com/novox/mesh-controller/internal/link" +) + +const ( + headSHA = "2222222222222222222222222222222222222222" + baseSHA = "1111111111111111111111111111111111111111" +) + +// fakeManifests serves manifests by ref, and counts what the forge was asked. Issue 1826/AC1 (unit level). +func fakeManifests(t *testing.T, at map[string]*catalogue.Manifest) { + t.Helper() + was := manifestAt + t.Cleanup(func() { manifestAt = was }) + manifestsReadMu.Lock() + clear(manifestsRead) + manifestsReadMu.Unlock() + manifestAt = func(_ context.Context, _, _, _, ref string) (catalogue.Manifest, bool, error) { + if m := at[ref]; m != nil { + return *m, true, nil + } + return catalogue.Manifest{}, false, nil + } +} + +// fakeConfig records the calls to mesh-config and answers each action from answers. +func fakeConfig(t *testing.T, answers map[string]string) *[]map[string]any { + t.Helper() + var calls []map[string]any + was := meshConfigAsk + t.Cleanup(func() { meshConfigAsk = was }) + meshConfigAsk = func(_ context.Context, args map[string]any) ([]byte, error) { + calls = append(calls, args) + return []byte(answers[args["action"].(string)]), nil + } + return &calls +} + +func manifest(module string, keys map[string]catalogue.SettingDeclaration) *catalogue.Manifest { + return &catalogue.Manifest{Module: module, Settings: keys} +} + +func thePull() pullAt { + return pullAt{Number: 434, changeSource: changeSource{Owner: "novox", Repo: "mesh-catalog", New: headSHA, Old: baseSHA, + Paths: []string{"modules/n8n/module.json", "modules/n8n/README.md"}}} +} + +// 1826/AC1: a key the head adds is sent with its declaration, under the pull request and the head. +func TestAKeyThePullRequestAddsIsToldToMeshConfig(t *testing.T) { + fakeManifests(t, map[string]*catalogue.Manifest{ + baseSHA: manifest("n8n", map[string]catalogue.SettingDeclaration{"port": {Type: "number"}}), + headSHA: manifest("n8n", map[string]catalogue.SettingDeclaration{"port": {Type: "number"}, + "media-gid": {Type: "number", Required: true}}), + }) + calls := fakeConfig(t, map[string]string{"pull-declares": `{"kept":1}`}) + tellPullDeclares(context.Background(), io.Discard, []pullAt{thePull()}) + if len(*calls) != 1 { + t.Fatalf("mesh-config was asked %d times: %v", len(*calls), *calls) + } + c := (*calls)[0] + if c["action"] != "pull-declares" || c["pull"] != "novox/mesh-catalog#434" || c["head"] != headSHA { + t.Fatalf("the call is %v", c) + } + body, _ := json.Marshal(c["declares"]) + var declares map[string]map[string]inventory.DeclaredMirrored + if err := json.Unmarshal(body, &declares); err != nil || len(declares) != 1 || declares["n8n"] == nil { + t.Fatalf("declares %s: %v", body, err) + } + mods := []inventory.DeclaredConfigMirrored{{Module: "n8n", Settings: declares["n8n"]}} + if _, has := mods[0].Settings["port"]; has || len(mods[0].Settings) != 1 { + t.Fatalf("only the added key is told, got %v", mods[0].Settings) + } + d := mods[0].Settings["media-gid"] + if d.Type != "number" || !d.Required { + t.Fatalf("the declaration is %+v", d) + } +} + +// 1826/AC1: a key whose declaration the head changes is sent; one it leaves as it was is not. +func TestAnUnchangedKeyIsNotToldAndAChangedOneIs(t *testing.T) { + fakeManifests(t, map[string]*catalogue.Manifest{ + baseSHA: manifest("n8n", map[string]catalogue.SettingDeclaration{"port": {Type: "number"}, "x": {Type: "string"}}), + headSHA: manifest("n8n", map[string]catalogue.SettingDeclaration{"port": {Type: "number"}, "x": {Type: "string", Pattern: "a+"}}), + }) + mods, touched, err := pullDeclarations(context.Background(), thePull()) + if err != nil || !touched || len(mods) != 1 || len(mods[0].Settings) != 1 || mods[0].Settings["x"].Pattern != "a+" { + t.Fatalf("%+v %v %v", mods, touched, err) + } +} + +// 1826/AC1: a head that changes no module.json, or removes it, is still told, with declares {}: mesh-config re-judges at every +// head, so a head that reverts the change drops the value held for it (review round 1, finding 1). +func TestAHeadThatDeclaresNothingIsToldAsEmpty(t *testing.T) { + fakeManifests(t, map[string]*catalogue.Manifest{baseSHA: manifest("n8n", nil), headSHA: manifest("n8n", nil)}) + calls := fakeConfig(t, nil) + p := thePull() + p.Paths = []string{"modules/n8n/README.md"} + tellPullDeclares(context.Background(), io.Discard, []pullAt{p}) + p = thePull() + p.Removed = p.Paths + tellPullDeclares(context.Background(), io.Discard, []pullAt{p}) + if len(*calls) != 2 { + t.Fatalf("mesh-config was told %v", *calls) + } + for _, c := range *calls { + if d, _ := json.Marshal(c["declares"]); string(d) != "{}" || c["head"] != headSHA { + t.Fatalf("told %v", c) + } + } +} + +// 1826/AC1: a head whose paths are cut, or whose base is unknown, cannot be read: nothing is told, never an empty set. +func TestAHeadThatCannotBeReadIsNotToldAsEmpty(t *testing.T) { + calls := fakeConfig(t, nil) + p := thePull() + p.Truncated = true + tellPullDeclares(context.Background(), io.Discard, []pullAt{p}) + if len(*calls) != 0 { + t.Fatalf("told %v", *calls) + } +} + +// 1826/AC1: a module.json changed without a declaration changing is told as an empty set, so mesh-config re-judges what it holds. +func TestAChangedManifestWithNoNewDeclarationIsToldAsEmpty(t *testing.T) { + m := manifest("n8n", map[string]catalogue.SettingDeclaration{"port": {Type: "number"}}) + fakeManifests(t, map[string]*catalogue.Manifest{baseSHA: m, headSHA: m}) + calls := fakeConfig(t, nil) + tellPullDeclares(context.Background(), io.Discard, []pullAt{thePull()}) + if len(*calls) != 1 { + t.Fatalf("calls %v", *calls) + } + if d, _ := json.Marshal((*calls)[0]["declares"]); string(d) != "{}" { + t.Fatalf("declares %s", d) + } +} + +// 1826/AC1: mesh-config not answering is said and goes on; the check is not stopped. +func TestMeshConfigNotAnsweringIsSaidAndGoneOn(t *testing.T) { + fakeManifests(t, map[string]*catalogue.Manifest{baseSHA: manifest("n8n", nil), + headSHA: manifest("n8n", map[string]catalogue.SettingDeclaration{"k": {Type: "string"}})}) + was := meshConfigAsk + t.Cleanup(func() { meshConfigAsk = was }) + meshConfigAsk = func(context.Context, map[string]any) ([]byte, error) { return nil, io.ErrUnexpectedEOF } + var said strings.Builder + tellPullDeclares(context.Background(), &said, []pullAt{thePull()}) + got := heldFor(context.Background(), &said, []pullAt{thePull()}) + if len(got) != 0 || !strings.Contains(said.String(), "mesh-config") { + t.Fatalf("held %v, said %q", got, said.String()) + } +} + +// 1826/AC2: the held values mesh-config answers for the check's pull requests are what the check request carries. +func TestTheHeldValuesOfTheChecksPullRequestsAreAsked(t *testing.T) { + calls := fakeConfig(t, map[string]string{"held": `{"held":[{"pull":"novox/mesh-catalog#434","head":"` + headSHA + + `","node":"ace","module":"n8n","key":"media-gid","value":2000,"approved":"the operator"}]}`}) + other := pullAt{Number: 7, changeSource: changeSource{Owner: "novox", Repo: "mesh-controller", New: baseSHA}} + got := heldFor(context.Background(), io.Discard, []pullAt{thePull(), other}) + if len(got) != 1 || got[0].Node != "ace" || got[0].Key != "media-gid" || string(got[0].Value) != "2000" { + t.Fatalf("got %+v", got) + } + c := (*calls)[0] + pulls, _ := json.Marshal(c["pulls"]) + if c["action"] != "held" || string(pulls) != `[{"pull":"novox/mesh-catalog#434","head":"`+headSHA+ + `"},{"pull":"novox/mesh-controller#7","head":"`+baseSHA+`"}]` { + t.Fatalf("asked %v %s", c, pulls) + } +} + +// 1826/AC2: a value held for a pull request the check is not of is never carried. +func TestAHeldValueOfAnotherPullRequestIsNotCarried(t *testing.T) { + fakeConfig(t, map[string]string{"held": `{"held":[{"pull":"novox/mesh-catalog#999","head":"x","node":"ace","module":"n8n","key":"k","value":1}]}`}) + if got := heldFor(context.Background(), io.Discard, []pullAt{thePull()}); len(got) != 0 { + t.Fatalf("carried %+v", got) + } +} + +// 1826/AC2: the check request carries what mesh-config answered; a member of a composed group asks none. +func TestTheCheckRequestCarriesTheHeldValues(t *testing.T) { + fakeManifests(t, map[string]*catalogue.Manifest{baseSHA: manifest("n8n", nil), headSHA: manifest("n8n", nil)}) + fakeConfig(t, map[string]string{"held": `{"held":[{"pull":"novox/mesh-catalog#434","node":"ace","module":"n8n","key":"media-gid","value":2000}]}`}) + request := link.BuildRequest{Check: &link.CheckRequest{}} + withHeldConfig(context.Background(), io.Discard, &request, []pullAt{thePull()}, true) + if len(request.Check.HeldConfig) != 1 || request.Check.HeldConfig[0].Key != "media-gid" { + t.Fatalf("the request carries %+v", request.Check.HeldConfig) + } + request = link.BuildRequest{Check: &link.CheckRequest{}} + withHeldConfig(context.Background(), io.Discard, &request, []pullAt{thePull()}, false) + if len(request.Check.HeldConfig) != 0 { + t.Fatalf("a composed member carries %+v", request.Check.HeldConfig) + } +} + +// 1826/AC1: a repository that builds a module of the mesh holds modules; another does not. +func TestARepositoryThatBuildsAModuleHoldsModules(t *testing.T) { + entries := []inventory.Entry{{Manifest: catalogue.Manifest{Module: "a"}, Source: inventory.Source{Repository: "novox/mesh-catalog", Path: "modules/a"}}} + if !holdsModules(entries, "novox", "mesh-catalog") || holdsModules(entries, "novox", "hq") { + t.Fatal("wrong") + } +} diff --git a/internal/builder/check.go b/internal/builder/check.go index 075c55cb..9dc3f110 100644 --- a/internal/builder/check.go +++ b/internal/builder/check.go @@ -123,8 +123,41 @@ type CheckSpec struct { // controller could not learn them, and the verdict says so. Siblings []Sibling SiblingsUnknown string + // Held are the approved held values of the check's pull requests (novox/hq ADR 0502 clause 4): written to a file and + // handed to the gate, which lays them over the change's composition only. None: no flag, no file. + Held []HeldValue } +// HeldValue is one approved config value held for a key an open pull request introduces: the value of Key for Module on +// Node (a machine's real name; the gate finds it in the snapshot by its pseudonym). The gate reads this document. +type HeldValue struct { + Pull string `json:"pull"` + Head string `json:"head,omitempty"` + Node string `json:"node"` + Module string `json:"module"` + Key string `json:"key"` + Value json.RawMessage `json:"value"` + Approved string `json:"approved,omitempty"` + By string `json:"by,omitempty"` + At string `json:"at,omitempty"` +} + +// writeHeld writes the held values to held.json under root and answers its path; empty, and nothing written, for none. +func writeHeld(root string, held []HeldValue) (string, error) { + if len(held) == 0 { + return "", nil + } + raw, err := json.Marshal(held) + if err != nil { + return "", err + } + file := filepath.Join(root, "held.json") + return file, os.WriteFile(file, raw, 0o644) +} + +// heldFlag is how the gate is given the held values: only when there are some, so a judge from before --held still runs. +const heldFlag = `${MESH_CHECK_HELD:+--held "$MESH_CHECK_HELD"}` + // Sibling is one other open pull request of a delivery group (novox/hq ADR 0382): the repository it is in as // owner/repo, its number, its whole head, the branch it merges into, and Dir, the directory the check finds that // repository under beside it (an entry of Beside). @@ -272,6 +305,8 @@ const ( // EnvGroup is a delivery group's other heads, as JSON, for the gate (novox/hq ADR 0239); empty for a // pull request checked alone. EnvGroup = "MESH_CHECK_GROUP" + // EnvHeld is the file of the approved held values of the check's pull requests (novox/hq ADR 0502); empty for none. + EnvHeld = "MESH_CHECK_HELD" // EnvTests is a `go test -run` pattern and EnvStoreOnly is 1 when a branch tried before its pull request // asks for only those tests, or only the store tests (novox/hq issue 686). Never set for a pull request's // check. A script that reads neither runs whole. @@ -572,6 +607,15 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry } } + // The approved held values of the check's pull requests, for the gate (novox/hq ADR 0502 clause 4). + heldFile, err := writeHeld(root, spec.Held) + if err != nil { + return CheckVerdict{}, err + } + if heldFile != "" { + say("check", "with %d config value(s) held for its pull request(s)", len(spec.Held)) + } + // Every clone is made: the credential is gone before anything of the check runs (novox/hq issue 462). forget() @@ -733,7 +777,7 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry EnvTestStore + "=" + storeURL, EnvTestBus + "=nats://" + bus, EnvRepository + "=" + spec.Owner + "/" + spec.Repo, EnvBeside + "=" + root, EnvModules + "=" + strings.Join(append(append([]string{}, spec.Modules...), spec.New...), ","), - EnvGroup + "=" + groupFile, + EnvGroup + "=" + groupFile, EnvHeld + "=" + heldFile, "GOCACHE=" + filepath.Join(workspace, "go-cache"), "GOMODCACHE=" + filepath.Join(workspace, "go-modules")} env = append(env, changedEnv...) env = append(env, selection(spec)...) @@ -1100,7 +1144,7 @@ func gateLayer(ctx context.Context, spec CheckSpec, tree, root, gate, verdictFil if err := running(inToolchain(tree, append(env, EnvVerdict+"="+verdictFile), "sh", "-c", `"$MESH_GATE" merge-gate --facts "$MESH_FACTS" --store "$MESH_GATE_POSTGRES" `+ `--repository "$MESH_CHECK_REPOSITORY" --tree . `+gateChangedFlag(env)+` `+ - `${MESH_CHECK_GROUP:+--group "$MESH_CHECK_GROUP"} --json > "$MESH_CHECK_VERDICT"`)); err != nil { + `${MESH_CHECK_GROUP:+--group "$MESH_CHECK_GROUP"} `+heldFlag+` --json > "$MESH_CHECK_VERDICT"`)); err != nil { if ctx.Err() != nil { return "error", "the check was ended during the merge gate" } diff --git a/internal/builder/check_held_test.go b/internal/builder/check_held_test.go new file mode 100644 index 00000000..d098b905 --- /dev/null +++ b/internal/builder/check_held_test.go @@ -0,0 +1,33 @@ +package builder + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// 1826/AC2: the held values are written whole to held.json, and the gate is given the flag only when there are some. +func TestHeldValuesAreWrittenForTheGateAndNoneWriteNothing(t *testing.T) { + root := t.TempDir() + if file, err := writeHeld(root, nil); err != nil || file != "" { + t.Fatalf("none wrote %q %v", file, err) + } + if _, err := os.Stat(filepath.Join(root, "held.json")); err == nil { + t.Fatal("none wrote a file") + } + file, err := writeHeld(root, []HeldValue{{Pull: "novox/mesh-catalog#434", Node: "ace", Module: "n8n", Key: "media-gid", + Value: []byte("2000"), Approved: "the operator"}}) + if err != nil || file != filepath.Join(root, "held.json") { + t.Fatalf("wrote %q %v", file, err) + } + raw, _ := os.ReadFile(file) + for _, want := range []string{`"node":"ace"`, `"module":"n8n"`, `"key":"media-gid"`, `"value":2000`} { + if !strings.Contains(string(raw), want) { + t.Fatalf("%s lacks %s", raw, want) + } + } + if !strings.Contains(heldFlag, `--held "$MESH_CHECK_HELD"`) || !strings.HasPrefix(heldFlag, "${MESH_CHECK_HELD:+") { + t.Fatalf("the flag is %s", heldFlag) + } +} diff --git a/internal/inventory/mirrors.go b/internal/inventory/mirrors.go index 4359b2be..5836e921 100644 --- a/internal/inventory/mirrors.go +++ b/internal/inventory/mirrors.go @@ -6,6 +6,8 @@ import ( "encoding/json" "fmt" "time" + + "git.novox.be/novox/mesh-sdk/go/catalogue" ) // The values of the controller's record mirrors, built from the store (novox/hq ADR 0289, design 54 §1, @@ -179,3 +181,13 @@ func (i *Inventory) PredefinedMirror(ctx context.Context) (map[string][]byte, er } return map[string][]byte{PredefinedMirrorKey: value}, nil } + +// DeclaredOf is a manifest's config declarations as the mirror holds them (DeclaredMirrored), so a pull request's +// declarations are told to mesh-config in the one shape it reads the trunk's in (novox/hq ADR 0502 clause 2). +func DeclaredOf(settings map[string]catalogue.SettingDeclaration) (map[string]DeclaredMirrored, error) { + raw, err := json.Marshal(settings) + if err != nil { + return nil, err + } + return declaredMirrored(raw) +} diff --git a/internal/link/build.go b/internal/link/build.go index f06b86fd..f66fef48 100644 --- a/internal/link/build.go +++ b/internal/link/build.go @@ -218,6 +218,25 @@ type CheckRequest struct { Try bool `json:"try,omitempty"` Tests string `json:"tests,omitempty"` StoreOnly bool `json:"store-only,omitempty"` + // HeldConfig are the approved held values of the pull requests this check is of (novox/hq ADR 0502 clause 4): each + // a config value the operator approved for a key an open pull request introduces. The merge gate lays them over + // their module's values on their node in the composition with the change only, never in the mesh as it is. Empty + // from a controller from before it, or when mesh-config held none or did not answer. + HeldConfig []HeldConfig `json:"held_config,omitempty"` +} + +// HeldConfig is one approved held value (novox/hq ADR 0502), as mesh-config's `held` action answers it. +type HeldConfig struct { + Pull string `json:"pull"` + Head string `json:"head,omitempty"` + Node string `json:"node"` + Module string `json:"module"` + Key string `json:"key"` + Value json.RawMessage `json:"value"` + // Approved says who approved it, in words, as mesh-config states it; By and At are who held it and when. + Approved string `json:"approved,omitempty"` + By string `json:"by,omitempty"` + At string `json:"at,omitempty"` } // StepTried is the step a build seat says a try's outcome under, in the build's log: one line, a Tried as JSON