From a011743c69df02632809cb3d956508d6974fd6f9 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 7 Oct 2026 01:33:18 +0200 Subject: [PATCH] Raise the mesh as it is in the gate, call a baseline that does not compose an error, and let a check run by hand as the seat runs it The gate composed 0 of 4 machines with the change and without, and passed every change: the store it raised held each module's bus credential but no account for it (issue 203's refusal), no outward links (so no filter could be composed), and refused settings the mesh holds. Now the account is minted with its credential, the facts carry each machine's outward links (a stand-in for an older snapshot), the mesh's layers are kept as held, and a withheld path keeps a path's shape. A machine the mesh composes that the gate cannot raise makes the verdict an error, never a pass; the verdict alone is on stdout. A merge-check.sh that passed on an agent's machine failed on the build seat: a newer gofmt, siblings at a feature branch, another user. `mesh-controller check-here` runs builder.Check with the ask the controller would make, from facts that now name the toolchains and the refs cloned beside; a failed script is said by what failed. (novox/hq issues 282, 283) --- cmd/mesh-controller/check_here.go | 200 +++++++++++++++++++++++++ cmd/mesh-controller/checks.go | 31 ++-- cmd/mesh-controller/facts.go | 35 ++++- cmd/mesh-controller/main.go | 3 + cmd/mesh-controller/merge_gate.go | 148 ++++++++++++++++-- cmd/mesh-controller/merge_gate_test.go | 131 ++++++++++++++++ internal/builder/check.go | 52 ++++++- internal/builder/check_test.go | 15 ++ internal/facts/facts.go | 13 ++ internal/facts/facts_test.go | 15 ++ internal/facts/scrub.go | 8 +- internal/inventory/catalogue.go | 20 ++- 12 files changed, 640 insertions(+), 31 deletions(-) create mode 100644 cmd/mesh-controller/check_here.go diff --git a/cmd/mesh-controller/check_here.go b/cmd/mesh-controller/check_here.go new file mode 100644 index 0000000..bb39aad --- /dev/null +++ b/cmd/mesh-controller/check_here.go @@ -0,0 +1,200 @@ +package main + +import ( + "context" + "errors" + "flag" + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + "time" + + "github.com/novox/mesh-controller/internal/builder" + "github.com/novox/mesh-controller/internal/catalogue" + "github.com/novox/mesh-controller/internal/link" +) + +// `check-here` is a pull request's merge check run on the machine at hand **exactly as the build seat +// runs it** (novox/hq issue 283): the same code (builder.Check), the same ask the controller would make of +// the seat — the gate's modules and judge by the planner's own answer over the facts snapshot, the +// repositories beside it at the refs the snapshot says the seat clones them at — in the toolchain image +// the mesh holds, as the user the build seat runs as, against a throwaway store and bus of the versions +// the mesh runs. +// +// **The build seat is the reference.** A merge-check.sh that passed on an agent's machine failed on the +// seat for three reasons that were each the agent's environment, never the change: a newer Go whose gofmt +// lays a file out differently, a sibling checkout at the agent's feature branch where the seat had the +// commit the mesh runs, and a different user. Run here, a check sees what the seat will. +// +// check-here [--tree ] [--base main] [--registry ] [--forge ] +// [--number ] [--user uid:gid] [--facts store|] [--keep] +// +// The checkout's HEAD is what is checked, and it must be committed: the seat checks a commit, never a +// working tree. +func checkHereCommand(ctx context.Context, args []string) error { + set := flag.NewFlagSet("check-here", flag.ContinueOnError) + tree := set.String("tree", ".", "the change's checkout; its HEAD is checked") + base := set.String("base", "main", "the branch the change would merge into") + registry := set.String("registry", os.Getenv("MESH_REGISTRY"), "the artifact store holding the facts and the toolchains") + forge := set.String("forge", "", "where the repositories beside it are cloned from, as /.git; "+ + "the checkout's origin without its own name when not given") + number := set.Int("number", 0, "the pull request's number, when there is one") + // The build seat's service runs as root, and its check containers run as the builder does. + user := set.String("user", "0:0", "the user the check's containers run as: the build seat's") + keep := set.Bool("keep", false, "keep the workspace afterwards") + factsFrom := set.String("facts", "store", "the facts snapshot: `store`, the one the artifact store holds — "+ + "what the seat reads — or a file") + if _, err := parseAround(set, args); err != nil { + return err + } + if *registry == "" { + return errors.New("check-here reads the facts and the toolchains from the artifact store: --registry or MESH_REGISTRY") + } + dir, err := filepath.Abs(*tree) + if err != nil { + return err + } + git := func(args ...string) (string, error) { + cmd := exec.CommandContext(ctx, "git", args...) + cmd.Dir = dir + out, err := cmd.Output() + return strings.TrimSpace(string(out)), err + } + if dirty, err := git("status", "--porcelain", "--untracked-files=no"); err != nil { + return fmt.Errorf("%s is not a checkout: %w", dir, err) + } else if dirty != "" { + return errors.New("the checkout has changes not committed: the build seat checks a commit, so commit first") + } + head, err := git("rev-parse", "HEAD") + if err != nil { + return err + } + origin, err := git("remote", "get-url", "origin") + if err != nil { + return fmt.Errorf("the checkout has no origin to say which repository it is: %w", err) + } + owner, repo, prefix := ownerRepoOf(origin) + if *forge == "" { + *forge = prefix + } + if _, err := git("fetch", "--quiet", "origin", *base); err != nil { + return fmt.Errorf("cannot fetch %s to say what the change touches: %w", *base, err) + } + changedText, err := git("diff", "--name-only", "origin/"+*base+"...HEAD") + if err != nil { + return err + } + var paths, removed []string + for _, p := range strings.Split(changedText, "\n") { + if p = strings.TrimSpace(p); p == "" { + continue + } + paths = append(paths, p) + if _, err := os.Stat(filepath.Join(dir, p)); os.IsNotExist(err) { + removed = append(removed, p) + } + } + + _ = os.Setenv("MESH_REGISTRY", *registry) + f, err := readFacts(ctx, *factsFrom) + if err != nil { + return fmt.Errorf("the facts snapshot cannot be read: %w", err) + } + entries, read, edges, err := graphOfFacts(f) + if err != nil { + return err + } + p := link.PullUpdated{Owner: owner, Repo: repo, Number: *number, Base: *base, Commit: head, Paths: paths, + Removed: removed, ModuleDirs: moduleDirsIn(dir, paths), ModuleDirsSaid: true} + scope := pullScope(p, entries, read, edges) + _, scriptErr := os.Stat(filepath.Join(dir, builder.CheckScript)) + if !scope.gated() && !scope.Mesh { + fmt.Printf("%s: %s, and the repository is not the mesh's: the build seat runs nothing for it\n", + owner+"/"+repo, noModuleTouched) + return nil + } + if !scope.gated() && scriptErr != nil { + fmt.Printf("%s: %s; %s\n", owner+"/"+repo, noModuleTouched, noMergeCheck) + return nil + } + + toolchains := map[string]string{} + for language, reference := range f.Versions.Toolchains { + toolchains[language] = catalogue.Rerouted(reference, *registry) + } + if len(toolchains) == 0 { + return errors.New("the facts snapshot names no toolchain: it was taken by a controller from before " + + "issue 283, and the seat's toolchain cannot be known here") + } + beside := map[string]builder.Beside{} + for d, ref := range f.Beside { + from := d + if d == "mesh-controller-main" { + from = "mesh-controller" + } + beside[d] = builder.Beside{Repository: strings.TrimSuffix(*forge, "/") + "/" + from + ".git", Ref: ref} + } + id := fmt.Sprintf("check-here-%d", time.Now().UnixNano()) + spec := builder.CheckSpec{ID: id, Repository: dir, Ref: head, Owner: owner, Repo: repo, Number: *number, + Paths: paths, Beside: beside, Modules: scope.Modules, New: scope.New, Manifests: scope.Manifests, + Base: *base, Judge: scope.Judge, Toolchain: toolchains["go"], Toolchains: toolchains, User: *user} + + workspace, err := os.MkdirTemp("", "mesh-check-here-") + if err != nil { + return err + } + if !*keep { + defer removeWorkspace(spec.Toolchain, workspace, *user) + } + fmt.Fprintf(os.Stderr, "checking %s/%s at %.8s as the build seat would, against the facts of %s, in %s\n", + owner, repo, head, f.Taken.Format(time.RFC3339), workspace) + v, err := builder.Check(ctx, builder.Command, spec, workspace, *registry, builder.GitCredential{}, + func(step, message string) { + if step != "output" { + fmt.Fprintf(os.Stderr, " [%s] %s\n", step, message) + } + }) + if err != nil { + return fmt.Errorf("the check could not run — on the seat an error, never a pass: %w", err) + } + fmt.Println(v.Report) + fmt.Println() + fmt.Printf("mesh/merge-gate: %s — %s\n", strings.ToUpper(v.Gate.Verdict), v.Gate.Summary) + if v.Repo != nil { + fmt.Printf("mesh/repo-check: %s — %s\n", strings.ToUpper(v.Repo.Verdict), v.Repo.Summary) + } + for _, l := range []*builder.Layer{v.Gate, v.Repo} { + if l != nil && l.Verdict != "pass" && l.Verdict != "warning" { + return errors.New("the build seat would not pass this change") + } + } + return nil +} + +// ownerRepoOf reads owner, repository and the owner's URL from a remote: ssh://git@host:222/novox/mesh-host.git +// → novox, mesh-host, ssh://git@host:222/novox. +func ownerRepoOf(remote string) (string, string, string) { + trimmed := strings.TrimSuffix(strings.TrimSuffix(remote, "/"), ".git") + cut := strings.LastIndexAny(trimmed, "/:") + if cut < 0 { + return "", trimmed, "" + } + repo, prefix := trimmed[cut+1:], trimmed[:cut] + owner := prefix + if at := strings.LastIndexAny(prefix, "/:"); at >= 0 { + owner = prefix[at+1:] + } + return owner, repo, prefix +} + +// removeWorkspace removes what the check left, written as the seat's user: by a container of that user +// when it is not this one. +func removeWorkspace(image, workspace, user string) { + if image != "" && user != fmt.Sprintf("%d:%d", os.Getuid(), os.Getgid()) { + _ = exec.Command("docker", "run", "--rm", "--user", user, "--volume", workspace+":"+workspace, image, + "rm", "-rf", workspace+"/check", workspace+"/go-cache", workspace+"/go-modules", workspace+"/git-credentials").Run() + } + _ = os.RemoveAll(workspace) +} diff --git a/cmd/mesh-controller/checks.go b/cmd/mesh-controller/checks.go index 863625f..478fcea 100644 --- a/cmd/mesh-controller/checks.go +++ b/cmd/mesh-controller/checks.go @@ -241,21 +241,14 @@ func checkRequestFor(ctx context.Context, open *stores, p link.PullUpdated, scop if err != nil { return link.BuildRequest{}, err } - ref := current[e.Manifest.Module].Commit - if dir == "mesh-catalog" { - // The catalogue the mesh runs is in the snapshot, every manifest as it holds it; its checkout - // beside is what tests read its files from, so its main — what the next merge builds from. - ref = "main" - } - beside[dir] = link.CheckedOut{Repository: url, Ref: ref} + refs := besideRefs(dir, current[e.Manifest.Module].Commit) + beside[dir] = link.CheckedOut{Repository: url, Ref: refs[dir]} if dir == "mesh-controller" { - // And its main, for a judge the running controller predates (Phase 5 rolling out). - beside["mesh-controller-main"] = link.CheckedOut{Repository: url, Ref: "main"} - // And the lab, whose replays of what the mesh runs every check runs; on the same forge. + beside["mesh-controller-main"] = link.CheckedOut{Repository: url, Ref: refs["mesh-controller-main"]} if e.Source.Seat != "" { if lab, err := clone(inventory.Source{Seat: e.Source.Seat, Repository: siblingOf(e.Source.Repository, "mesh-lab")}); err == nil { - beside["mesh-lab"] = link.CheckedOut{Repository: lab, Ref: "main"} + beside["mesh-lab"] = link.CheckedOut{Repository: lab, Ref: refs["mesh-lab"]} } } } @@ -273,6 +266,22 @@ func checkRequestFor(ctx context.Context, open *stores, p link.PullUpdated, scop }, nil } +// besideRefs is the ref each repository is cloned at beside a check, by the directory it is found under: +// a core repository at the commit the mesh runs of the module built from it (`running`) — but the +// catalogue, whose checkout beside is what tests read its files from, at its main, what the next merge +// builds from; and beside the controller its main, for a judge the running controller predates, and the +// lab's main, whose replays every check runs. **One rule, read by the check the controller asks for and by +// the facts snapshot** (Facts.Beside), so a check run by hand clones what the build seat clones. +func besideRefs(dir, running string) map[string]string { + switch dir { + case "mesh-catalog": + return map[string]string{dir: "main"} + case "mesh-controller": + return map[string]string{dir: running, "mesh-controller-main": "main", "mesh-lab": "main"} + } + return map[string]string{dir: running} +} + // siblingOf is another repository of the same owner: novox/mesh-controller → novox/mesh-lab. func siblingOf(repository, name string) string { if cut := strings.LastIndex(repository, "/"); cut >= 0 { diff --git a/cmd/mesh-controller/facts.go b/cmd/mesh-controller/facts.go index 4306df1..4bf271f 100644 --- a/cmd/mesh-controller/facts.go +++ b/cmd/mesh-controller/facts.go @@ -18,6 +18,7 @@ import ( "github.com/novox/mesh-controller/internal/artifacts" "github.com/novox/mesh-controller/internal/broker" + "github.com/novox/mesh-controller/internal/builder" "github.com/novox/mesh-controller/internal/catalogue" snapshot "github.com/novox/mesh-controller/internal/facts" "github.com/novox/mesh-controller/internal/inventory" @@ -236,6 +237,28 @@ func gatherFacts(ctx context.Context, open *stores, busVersion string) (snapshot scrub.Site(o.Site) } + // What a merge check runs in and reads beside it, as the build seat would be asked for it. + if held, err := inv.Held(ctx); err == nil { + for language, reference := range builder.ToolchainsOf(held) { + if f.Versions.Toolchains == nil { + f.Versions.Toolchains = map[string]string{} + } + f.Versions.Toolchains[language] = catalogue.Recorded(reference) + } + } else { + return snapshot.Facts{}, err + } + for _, e := range entries { + if dir, core := coreModules[e.Manifest.Module]; core && !e.Provided && e.Source.Repository != "" { + for d, ref := range besideRefs(dir, current[e.Manifest.Module].Commit) { + if f.Beside == nil { + f.Beside = map[string]string{} + } + f.Beside[d] = ref + } + } + } + if c, ok := current["mesh-controller"]; ok { f.Controller.Commit = c.Commit } @@ -263,6 +286,13 @@ func gatherFacts(ctx context.Context, open *stores, busVersion string) (snapshot return snapshot.Facts{}, err } m.Architecture, m.Kernel = reported.Architecture, reported.Kernel + outward, err := inv.OutwardLinksOf(ctx, n.Name) + if err != nil { + return snapshot.Facts{}, err + } + for _, l := range outward { + m.OutwardLinks = append(m.OutwardLinks, scrub.Text(l)) + } capabilities, err := inv.Profile(ctx, n.Name) if err != nil { return snapshot.Facts{}, err @@ -426,7 +456,10 @@ func declarationFacts(ctx context.Context, open *stores, node string, gens map[s if body, err := declared.Body(); err == nil { d.Digest = fmt.Sprintf("sha256:%x", sha256.Sum256(body)) } - d.Resources = resourceNames(declared.Resources) + // Scrubbed like every other word: a resource is named after the machine a grant is for. + for _, r := range resourceNames(declared.Resources) { + d.Resources = append(d.Resources, scrub.Text(r)) + } for module, why := range declared.leftOutWhy { if d.LeftOut == nil { d.LeftOut = map[string]string{} diff --git a/cmd/mesh-controller/main.go b/cmd/mesh-controller/main.go index 223068d..0309fbd 100644 --- a/cmd/mesh-controller/main.go +++ b/cmd/mesh-controller/main.go @@ -123,6 +123,9 @@ func run() error { // The merge gate: every machine of the snapshot composed with a change (novox/hq to-be 45 §9). case "merge-gate": return mergeGateCommand(ctx, args[1:]) + // A pull request's merge check run here exactly as the build seat runs it (novox/hq issue 283). + case "check-here": + return checkHereCommand(ctx, args[1:]) case "upgrade": return upgradeCommand(ctx, args[1:]) // The bus as a planned step (novox/hq to-be 45 §8, ADR 0236). diff --git a/cmd/mesh-controller/merge_gate.go b/cmd/mesh-controller/merge_gate.go index 3100110..9096b11 100644 --- a/cmd/mesh-controller/merge_gate.go +++ b/cmd/mesh-controller/merge_gate.go @@ -22,6 +22,7 @@ import ( "github.com/jackc/pgx/v5" "github.com/novox/mesh-controller/internal/artifacts" + "github.com/novox/mesh-controller/internal/broker" "github.com/novox/mesh-controller/internal/catalogue" snapshot "github.com/novox/mesh-controller/internal/facts" "github.com/novox/mesh-controller/internal/inventory" @@ -103,9 +104,12 @@ type mergeVerdict struct { Verdict string `json:"verdict"` // pass, warning or fail Summary string `json:"summary"` // Judge is the controller build that judged it, and Facts when the snapshot it judged against was taken. - Judge string `json:"judge"` - Facts time.Time `json:"facts"` - Failures []string `json:"failures,omitempty"` + Judge string `json:"judge"` + Facts time.Time `json:"facts"` + Failures []string `json:"failures,omitempty"` + // Errors are what kept the gate from judging: a machine the mesh composes that the gate could not + // raise as it is. Any one makes the verdict an error — never a pass (novox/hq issue 282). + Errors []string `json:"errors,omitempty"` Warnings []string `json:"warnings,omitempty"` Notes []string `json:"notes,omitempty"` Machines []mergeMachine `json:"machines"` @@ -182,11 +186,18 @@ func mergeGateCommand(ctx context.Context, args []string) error { return err } out := io.Writer(os.Stdout) + stdout := os.Stdout if *asJSON { out = os.Stderr + // **The verdict is the only thing on standard output** when it is asked for as JSON: composition + // says what it finds as it goes (a bus user without a credential, a module left out) and prints it, + // and a line of that before the verdict makes the verdict unreadable — the build seat would read no + // verdict at all. What is said goes beside the report. + os.Stdout = os.Stderr } v, err := judgeChange(ctx, mergeCheckInput{facts: f, repository: *repository, tree: *tree, changed: splitList(*changed), group: others, admin: *admin, say: out}) + os.Stdout = stdout if err != nil { return err } @@ -201,12 +212,19 @@ func mergeGateCommand(ctx context.Context, args []string) error { } else { fmt.Print(v.Report()) } - if v.Verdict == "fail" { + switch v.Verdict { + case "fail": return errMergeGateFailed + case "error": + return errMergeGateCouldNotJudge } return nil } +// errMergeGateCouldNotJudge is the gate's own error: the mesh as it is could not be raised, so nothing +// the change does to it can be seen. Never a pass (novox/hq issue 282). +var errMergeGateCouldNotJudge = errors.New("the merge gate could not judge the change") + // errMergeGateFailed is the gate's own failure: the verdict says why, so the error says nothing more. var errMergeGateFailed = errors.New("the change fails the merge gate") @@ -333,10 +351,12 @@ func judgeChange(ctx context.Context, in mergeCheckInput) (mergeVerdict, error) v.Failures = append(v.Failures, fmt.Sprintf("%s: nothing could be sent to it with this change — %s", gm.Described, firstOr(newOnly(gm.Change.Problems, gm.Base.Problems), gm.Change.Problems))) case !gm.Base.Composes && m.Declaration.Composes: - // The mesh composes it and the gate could not raise it as it is: a fact the snapshot does - // not carry. Said, and judged by what the change adds. - v.Notes = append(v.Notes, fmt.Sprintf("%s composes on the mesh and not as the snapshot raised it: %s — "+ - "judged by what the change adds", gm.Described, firstOr(gm.Base.Problems, nil))) + // **The mesh composes it and the gate could not raise it as it is** (novox/hq issue 282): a fact + // the snapshot does not carry, or one the gate does not raise. Then the change is judged against a + // machine that is not the mesh's — broken against broken, which passes whatever the change does — + // so the gate cannot judge, and says so: an error, never a pass. + v.Errors = append(v.Errors, fmt.Sprintf("%s composes on the mesh and not as the gate raised it from the "+ + "snapshot: %s", gm.Described, firstOr(gm.Base.Problems, nil))) if added := newOnly(gm.Change.Problems, gm.Base.Problems); len(added) > 0 { v.Failures = append(v.Failures, fmt.Sprintf("%s: the change adds — %s", gm.Described, added[0])) } @@ -387,10 +407,16 @@ func judgeChange(ctx context.Context, in mergeCheckInput) (mergeVerdict, error) sort.Strings(v.Failures) v.Failures = slices.Compact(v.Failures) + sort.Strings(v.Errors) switch { case len(v.Failures) > 0: v.Verdict = "fail" v.Summary = fmt.Sprintf("%d problem(s) the change brings; the first: %s", len(v.Failures), v.Failures[0]) + case len(v.Errors) > 0: + v.Verdict = "error" + v.Summary = fmt.Sprintf("the mesh as it is could not be raised, so the change cannot be judged against it "+ + "(%d of %d machines compose on the mesh and not in the gate); the first: %s", len(v.Errors), + len(v.Machines), v.Errors[0]) case len(v.Warnings) > 0: v.Verdict = "warning" v.Summary = v.Warnings[0] @@ -423,6 +449,7 @@ func (v mergeVerdict) Report() string { } } list("fails", v.Failures) + list("could not judge", v.Errors) list("warns", v.Warnings) if len(v.Modules) > 0 { var changed []string @@ -961,6 +988,18 @@ func raiseFromFacts(ctx context.Context, open *stores, f snapshot.Facts, shelf m return notes, err } } + // The links it faces outside by, which a filter is written around (ADR 0140). A snapshot from a + // controller that did not carry them stands in one for a machine the mesh composes: composing, it + // had reported them. + outward := m.OutwardLinks + if len(outward) == 0 && m.Declaration.Composes { + outward = []string{"outside0"} + } + if len(outward) > 0 { + if err := inv.RecordOutwardLinks(ctx, node.ID, outward); err != nil { + return notes, err + } + } if m.Account != "" { if err := inv.SetAccount(ctx, m.Name, m.Account, m.AccountHome); err != nil { return notes, err @@ -994,7 +1033,7 @@ func raiseFromFacts(ctx context.Context, open *stores, f snapshot.Facts, shelf m } } for _, s := range f.Settings { - if err := inv.SetSettings(ctx, "", s.Module, s.Values); err != nil { + if err := inv.KeepSettings(ctx, "", s.Module, standInPaths(s.Values)); err != nil { notes = append(notes, fmt.Sprintf("the mesh's settings of %s are not kept: %s", s.Module, oneLine(err.Error()))) } } @@ -1006,13 +1045,31 @@ func raiseFromFacts(ctx context.Context, open *stores, f snapshot.Facts, shelf m } } for _, s := range m.Settings { - if err := inv.SetSettings(ctx, m.Name, s.Module, s.Values); err != nil { + if err := inv.KeepSettings(ctx, m.Name, s.Module, standInPaths(s.Values)); err != nil { notes = append(notes, fmt.Sprintf("%s's settings of %s are not kept: %s", m.Described(), s.Module, oneLine(err.Error()))) } } for _, a := range m.Accepted { standIn := fmt.Sprintf("gate-stand-in-%x", sha256.Sum256([]byte(m.Name+a.Module+a.Name+a.Provider+a.Local)))[:40] + if a.Provider == "" && a.Name == "broker" { + // **The bus credential is an account, not a given value** (novox/hq issue 203): composition asks + // whether one was issued for the module, so the raised store mints the account the mesh issued + // as well as holding the sealed value — otherwise every machine running a module on the bus + // fails to compose, with the change and without, and the gate compares broken to broken. + // A credential the mesh still holds for a module that no longer reads one composes nothing, + // so it is not raised. + if !readsBusCredential(shelf, a.Module) { + continue + } + if _, err := inv.MintBusPassword(ctx, inventory.BusUser{Kind: busKindOf(a.Module), Node: m.Name, + Module: a.Module, Username: broker.Principal{Kind: broker.KindModule, Node: m.Name, + Module: a.Module}.Username()}); err != nil { + notes = append(notes, fmt.Sprintf("%s's bus account for %s is not kept: %s", m.Described(), + a.Module, oneLine(err.Error()))) + continue + } + } var err error if a.Provider == "" { err = inv.AcceptSecretForModule(ctx, m.Name, a.Module, a.Name, standIn) @@ -1028,6 +1085,64 @@ func raiseFromFacts(ctx context.Context, open *stores, f snapshot.Facts, shelf m return notes, nil } +// standInPaths is a settings layer with every value the snapshot withheld where a path stands — an +// access's or a place's — given a path of its own: a path that reads as a key is withheld by the scrub, +// and a bare "withheld" is no absolute path, which composition refuses (a machine the mesh composes +// would not compose as the gate raised it). Each stand-in is distinct, so two withheld places stay two. +func standInPaths(values map[string]any) map[string]any { + out := make(map[string]any, len(values)) + for k, v := range values { + out[k] = v + } + if accesses, ok := values["accesses"].(map[string]any); ok { + kept := make(map[string]any, len(accesses)) + for name, at := range accesses { + if withheldPath(at) { + at = "/" + snapshot.Withheld + "/access/" + name + } + kept[name] = at + } + out["accesses"] = kept + } + if places, ok := values["places"].(map[string]any); ok { + kept := make(map[string]any, len(places)) + for name, p := range places { + if place, ok := p.(map[string]any); ok && withheldPath(place["path"]) { + copied := make(map[string]any, len(place)) + for k, v := range place { + copied[k] = v + } + copied["path"] = "/" + snapshot.Withheld + "/place/" + name + p = copied + } + kept[name] = p + } + out["places"] = kept + } + return out +} + +// withheldPath is a value the scrub withheld where a path stood: bare, or keeping a path's shape. +func withheldPath(v any) bool { + return v == snapshot.Withheld || v == "/"+snapshot.Withheld +} + +// readsBusCredential is whether a module of the shelf, or one that came with the controller, declares the +// own secret its bus account is delivered as. +func readsBusCredential(shelf map[string]catalogue.Manifest, module string) bool { + if m, held := shelf[module]; held { + _, reads := m.OwnSecrets["broker"] + return reads + } + for _, m := range provided { + if m.Module == module { + _, reads := m.OwnSecrets["broker"] + return reads + } + } + return false +} + // standInKey is a key a machine could have reported; its private half is never kept. func standInKey() (string, error) { k, err := ecdh.X25519().GenerateKey(rand.Reader) @@ -1057,12 +1172,21 @@ func reachOfChange(f snapshot.Facts, repository string, paths []string, tree str } } } + entries, read, edges, err := graphOfFacts(f) + if err != nil { + return mergeReach{}, err + } + return reachOfMerge(m, entries, read, edges), nil +} + +// graphOfFacts is the module graph the snapshot carries, as the planner reads it from the store. +func graphOfFacts(f snapshot.Facts) ([]inventory.Entry, map[string][]inventory.ReadRepository, []inventory.Edge, error) { var entries []inventory.Entry read := map[string][]inventory.ReadRepository{} for _, mod := range f.Modules { var manifest catalogue.Manifest if err := json.Unmarshal(mod.Manifest, &manifest); err != nil { - return mergeReach{}, err + return nil, nil, nil, err } entries = append(entries, inventory.Entry{Manifest: manifest, Provided: mod.Provided, Source: inventory.Source{Repository: mod.Repository, Path: mod.Path, BuiltFrom: mod.Commit}}) @@ -1074,7 +1198,7 @@ func reachOfChange(f snapshot.Facts, repository string, paths []string, tree str for _, e := range f.Edges { edges = append(edges, inventory.Edge{From: e.From, To: e.To, Kind: e.Kind}) } - return reachOfMerge(m, entries, read, edges), nil + return entries, read, edges, nil } // widthOf is how wide the rebuild of a reach is. diff --git a/cmd/mesh-controller/merge_gate_test.go b/cmd/mesh-controller/merge_gate_test.go index cb42e03..7874460 100644 --- a/cmd/mesh-controller/merge_gate_test.go +++ b/cmd/mesh-controller/merge_gate_test.go @@ -6,6 +6,7 @@ import ( "strings" "testing" + "github.com/novox/mesh-controller/internal/broker" "github.com/novox/mesh-controller/internal/catalogue" snapshot "github.com/novox/mesh-controller/internal/facts" "github.com/novox/mesh-controller/internal/inventory" @@ -281,3 +282,133 @@ func catalogueMeshWithAnApp(t *testing.T) (snapshot.Facts, map[string]string) { } return f, manifests } + +// busMesh is aMesh with a module on the bus on the laptop, its account issued as `module issue` issues +// one: minted, and its credential held as the module's own secret named broker. +func busMesh(t *testing.T) snapshot.Facts { + t.Helper() + open := aMesh(t) + ctx := t.Context() + m, err := catalogue.ParseManifest([]byte(`{"module":"speaker","version":"1", + "own-secrets":{"broker":"/var/lib/speaker/broker"}}`)) + if err != nil { + t.Fatal(err) + } + if err := open.inventory.RegisterModule(ctx, m, inventory.Source{Repository: "novox/mesh-catalog", + Path: "modules/speaker", BuiltFrom: "c0ffee"}); err != nil { + t.Fatal(err) + } + if _, err := assign(ctx, open, "laptop", "speaker"); err != nil { + t.Fatal(err) + } + user := broker.Principal{Kind: broker.KindModule, Node: "laptop", Module: "speaker"}.Username() + password, err := open.inventory.MintBusPassword(ctx, inventory.BusUser{Username: user, Kind: inventory.BusModule, + Node: "laptop", Module: "speaker"}) + if err != nil { + t.Fatal(err) + } + if err := open.inventory.AcceptSecretForModule(ctx, "laptop", "speaker", "broker", + `{"user":"`+user+`","password":"`+password+`"}`); err != nil { + t.Fatal(err) + } + f, err := gatherFacts(ctx, open, "2.11.17") + if err != nil { + t.Fatal(err) + } + for _, mc := range f.Machines { + if !mc.Declaration.Composes { + t.Fatalf("the mesh itself does not compose: %+v", mc.Declaration) + } + } + return f +} + +// **Issue 282**: every machine running a module on the bus failed to compose in the gate's store, with +// the change and without — the store held the module's credential and no account for it, which +// composition refuses (issue 203) — and the gate passed every change, "0 of 4 compose". The account the +// mesh issued is raised with its credential, so the machine composes in the gate as on the mesh. +func TestIssue282AModuleOnTheBusComposesInTheGate(t *testing.T) { + f := busMesh(t) + v := gateJudged(t, f, "") + if v.Verdict != "pass" { + t.Fatalf("the mesh as it is does not pass:\n%s", v.Report()) + } + for _, m := range v.Machines { + if !m.Base.Composes { + t.Errorf("%s composes on the mesh and not in the gate: %v", m.Described, m.Base.Problems) + } + } + if !strings.Contains(v.Summary, "2 of 2 compose") { + t.Errorf("the summary does not say every machine composes: %s", v.Summary) + } +} + +// **Issue 282**: a machine the mesh composes that the gate cannot raise as it is leaves the change judged +// against a machine that is not the mesh's — broken against broken, which passes whatever the change does. +// That is an error, never a pass. +func TestIssue282AMachineTheGateCannotRaiseIsAnErrorNeverAPass(t *testing.T) { + f := busMesh(t) + for i := range f.Machines { + // A fact the snapshot does not carry: the module's credential, gone from the laptop's. + var kept []snapshot.Accepted + for _, a := range f.Machines[i].Accepted { + if a.Name != "broker" { + kept = append(kept, a) + } + } + f.Machines[i].Accepted = kept + } + v := gateJudged(t, f, "") + if v.Verdict != "error" { + t.Fatalf("a gate whose mesh does not compose said %s:\n%s", v.Verdict, v.Report()) + } + if len(v.Errors) != 1 || !strings.Contains(v.Errors[0], "speaker") { + t.Errorf("the error does not name what could not be raised: %v", v.Errors) + } +} + +// A path the snapshot withheld is given a path of its own where an access or a place needs one: a bare +// "withheld" is no absolute path, and a machine whose setting composes on the mesh would not in the gate. +func TestAWithheldPathIsStoodInForByAPath(t *testing.T) { + got := standInPaths(map[string]any{ + "accesses": map[string]any{"races": "withheld", "films": "/media/films", "shows": "/withheld"}, + "places": map[string]any{"config": map[string]any{"path": "withheld", "owner": "1000:1000"}}, + "puid": 1000, + }) + accesses := got["accesses"].(map[string]any) + if accesses["races"] == accesses["shows"] || !strings.HasPrefix(accesses["races"].(string), "/") || + !strings.HasPrefix(accesses["shows"].(string), "/") || accesses["films"] != "/media/films" { + t.Errorf("accesses: %v", accesses) + } + place := got["places"].(map[string]any)["config"].(map[string]any) + if !strings.HasPrefix(place["path"].(string), "/") || place["owner"] != "1000:1000" || got["puid"] != 1000 { + t.Errorf("places: %v", got) + } +} + +// **Issue 283**: a check run by hand clones beside a change what the seat clones — one rule, read by the +// controller's ask and by the facts — and finds the repository it checks from its origin. +func TestACheckByHandClonesWhatTheSeatClones(t *testing.T) { + for dir, refs := range map[string]map[string]string{ + "mesh-catalog": {"mesh-catalog": "main"}, + "mesh-host": {"mesh-host": "c0ffee"}, + "mesh-controller": {"mesh-controller": "c0ffee", "mesh-controller-main": "main", "mesh-lab": "main"}, + } { + got := besideRefs(dir, "c0ffee") + for d, ref := range refs { + if got[d] != ref { + t.Errorf("beside %s, %s is cloned at %q, not %q", dir, d, got[d], ref) + } + } + } + for owner, want := range map[string][3]string{ + "ssh://git@git.example:222/novox/mesh-host.git": {"novox", "mesh-host", "ssh://git@git.example:222/novox"}, + "git@git.example:novox/mesh-tools.git": {"novox", "mesh-tools", "git@git.example:novox"}, + "http://git.example/novox/hq": {"novox", "hq", "http://git.example/novox"}, + } { + o, r, p := ownerRepoOf(owner) + if [3]string{o, r, p} != want { + t.Errorf("%s reads as %s %s %s", owner, o, r, p) + } + } +} diff --git a/internal/builder/check.go b/internal/builder/check.go index 28fceba..010610d 100644 --- a/internal/builder/check.go +++ b/internal/builder/check.go @@ -81,6 +81,9 @@ type CheckSpec struct { // Toolchains is every toolchain the mesh holds, by language, for a script that declares another. Toolchain string Toolchains map[string]string + // User is who a check's containers run as, uid:gid: the builder's own when empty — on the build seat, + // the user its service runs as. A check run by hand (`mesh-controller check-here`) says the seat's. + User string // Group are a delivery group's other heads (novox/hq ADR 0239), each cloned beside this one at its head // and composed with it by the gate as one future state. The repository's own check is not run for a // group: each member's pull request runs its own. @@ -326,10 +329,18 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry if spec.Toolchain == "" { return CheckVerdict{}, errors.New("the mesh holds no Go toolchain to run a check in") } + user := spec.User + if user == "" { + user = fmt.Sprintf("%d:%d", os.Getuid(), os.Getgid()) + } in := func(image, dir string, env []string, command ...string) []string { // As the builder itself: what a check writes into the workspace is the builder's to remove. args := []string{"run", "--rm", "--network", "host", "--volume", workspace + ":" + workspace, "--workdir", dir, - "--user", fmt.Sprintf("%d:%d", os.Getuid(), os.Getgid()), "--env", "HOME=" + workspace} + "--user", user, "--env", "HOME=" + workspace, + // The check's own checkouts, whoever cloned them: git in a container of another user than the + // one that cloned refuses a repository it does not own ("dubious ownership"), and Go's build + // stamps the version from git — a judge that would not build for want of it. + "--env", "GIT_CONFIG_COUNT=1", "--env", "GIT_CONFIG_KEY_0=safe.directory", "--env", "GIT_CONFIG_VALUE_0=*"} for _, e := range env { args = append(args, "--env", e) } @@ -421,7 +432,7 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry case ctx.Err() != nil: return v, ctx.Err() case err != nil: - v.Repo = &Layer{Verdict: "fail", Summary: "its " + CheckScript + " failed: " + lastLine(own.String())} + v.Repo = &Layer{Verdict: "fail", Summary: "its " + CheckScript + " failed: " + whatFailed(own.String())} default: v.Repo = &Layer{Verdict: "pass", Summary: "its " + CheckScript + " passed"} } @@ -503,8 +514,14 @@ func gateLayer(ctx context.Context, spec CheckSpec, tree, root, gate, verdictFil Verdict string `json:"verdict"` Summary string `json:"summary"` } - if raw, err := os.ReadFile(verdictFile); err == nil && json.Unmarshal(raw, &said) == nil && said.Verdict == "fail" { - return "fail", said.Summary + if raw, err := os.ReadFile(verdictFile); err == nil && json.Unmarshal(raw, &said) == nil { + switch said.Verdict { + case "fail": + return "fail", said.Summary + case "error": + // The gate could not raise the mesh as it is (novox/hq issue 282): said in its own words. + return "error", said.Summary + } } // The gate could not judge: not the change's fault, and never a pass. return "error", "the merge gate could not judge the change: " + lastLine(out.String()) @@ -816,6 +833,33 @@ func (t *tail) String() string { return strings.Join(lines, "\n") } +// whatFailed is the line of a failed script's output that says what failed, for the status a pull request +// shows: the first failing test, the first failing package, the files not formatted — a bare "FAIL" or a +// file's name said nothing a reader could act on (novox/hq issue 283) — and the last line otherwise. +func whatFailed(s string) string { + lines := strings.Split(strings.TrimSpace(s), "\n") + for i, line := range lines { + line = strings.TrimSpace(line) + if strings.HasPrefix(line, "not gofmt'd:") { + var files []string + for _, f := range lines[i+1:] { + if f = strings.TrimSpace(f); f != "" { + files = append(files, f) + } + } + return "not gofmt'd by the toolchain's gofmt: " + strings.Join(files, ", ") + } + } + for _, prefix := range []string{"--- FAIL:", "FAIL\t", "panic:"} { + for _, line := range lines { + if line = strings.TrimSpace(line); strings.HasPrefix(line, prefix) { + return line + } + } + } + return lastLine(s) +} + func lastLine(s string) string { lines := strings.Split(strings.TrimSpace(s), "\n") return strings.TrimSpace(lines[len(lines)-1]) diff --git a/internal/builder/check_test.go b/internal/builder/check_test.go index f35e22f..e278f0d 100644 --- a/internal/builder/check_test.go +++ b/internal/builder/check_test.go @@ -414,3 +414,18 @@ func TestABuildSaysWhetherItsCommitIsOnTheTrunk(t *testing.T) { t.Errorf("a repository with no origin reads as trunk %q, on %v", trunk, on) } } + +// **Issue 283**: a failed merge-check.sh was said by its last line — a bare "FAIL", or the name of a file +// gofmt listed — which named nothing a reader could act on. The status says what failed. +func TestAFailedScriptIsSaidByWhatFailed(t *testing.T) { + for out, want := range map[string]string{ + "not gofmt'd:\ninternal/bootstrap/publish_test.go\n": "not gofmt'd by the toolchain's gofmt: internal/bootstrap/publish_test.go", + "ok \tx/a\t1s\n--- FAIL: TestTheInstallersFirstUserList (0.59s)\n t.go:37: refused\nFAIL\nFAIL\tx/b\t1s\nFAIL\n": "--- FAIL: TestTheInstallersFirstUserList (0.59s)", + "ok \tx/a\t1s\nFAIL\tx/b [build failed]\nFAIL\n": "FAIL\tx/b [build failed]", + "npm ERR! missing script: typecheck\n": "npm ERR! missing script: typecheck", + } { + if got := whatFailed(out); got != want { + t.Errorf("%q is said as %q, not %q", out, got, want) + } + } +} diff --git a/internal/facts/facts.go b/internal/facts/facts.go index f8a8d7c..ecb5a3c 100644 --- a/internal/facts/facts.go +++ b/internal/facts/facts.go @@ -62,6 +62,10 @@ type Facts struct { // Edges are the build dependencies between modules, as the mesh recorded them — what a merge's // rebuild width is computed from. Edges []Edge `json:"edges,omitempty"` + // Beside is the ref each repository is cloned at beside a merge check, by the directory it is found + // under — the commits the mesh runs, the catalogue's main — so a check run by hand reads the siblings + // the build seat reads (novox/hq issue 283). + Beside map[string]string `json:"beside,omitempty"` } // Build names one build of a core component. @@ -78,6 +82,11 @@ type Versions struct { Store string `json:"store,omitempty"` // NodeEngines are the node-engine builds the machines report, each once. NodeEngines []string `json:"node-engines,omitempty"` + // Toolchains are the toolchain images a merge check runs in, by language, as the mesh holds them — + // with no address: the artifact store the snapshot is read from is where they are pulled from. What + // a repository's merge-check.sh runs in on the build seat, and so what it must run in anywhere else + // (novox/hq issue 283): two releases of one compiler disagree, down to how gofmt lays out a file. + Toolchains map[string]string `json:"toolchains,omitempty"` } // Source is a repository the mesh builds from, and the newest commit it built a module of. @@ -112,6 +121,10 @@ type Machine struct { Public bool `json:"public,omitempty"` // OnNetwork is whether it has a place on the private network at all. OnNetwork bool `json:"on-network,omitempty"` + // OutwardLinks are the links it reported as facing outside it, scrubbed: a machine that reported none + // is sent no filter, so a check that raised it without them would compose a machine the mesh does + // not (novox/hq ADR 0140). + OutwardLinks []string `json:"outward-links,omitempty"` // Adopted is whether the mesh adopted it rather than converged it. Adopted bool `json:"adopted,omitempty"` // Account is the operator's login there (a pseudonym of the same length), and AccountHome where its diff --git a/internal/facts/facts_test.go b/internal/facts/facts_test.go index 8f2dc11..56021a9 100644 --- a/internal/facts/facts_test.go +++ b/internal/facts/facts_test.go @@ -157,3 +157,18 @@ func TestANewerSnapshotIsRefusedNotHalfRead(t *testing.T) { t.Errorf("read %+v, %v", f, err) } } + +// A path that reads as a key is withheld, and keeps a path's shape: an access or a place must be an +// absolute path, and a bare word left a machine the mesh composes refused where a check raised it. +func TestAWithheldPathStaysAPath(t *testing.T) { + s := NewScrubber() + if got := s.Text("/storage/media/Formula1-Season-2026"); got != "/"+Withheld { + t.Errorf("a path reading as a key became %q", got) + } + if got := s.Text("/storage/media/movies"); got != "/storage/media/movies" { + t.Errorf("a plain path became %q", got) + } + if got := s.Text("Hunter2Hunter2Hunter2Hunter2xx"); got != Withheld { + t.Errorf("a key became %q", got) + } +} diff --git a/internal/facts/scrub.go b/internal/facts/scrub.go index d2ac346..2f310ed 100644 --- a/internal/facts/scrub.go +++ b/internal/facts/scrub.go @@ -228,7 +228,13 @@ func secretRuns(text string) string { b.WriteString(run) continue } - b.WriteString(judgeRun(run)) + judged := judgeRun(run) + // A path withheld keeps a path's shape — an absolute one stays absolute — so a check composing a + // place or an access still finds a path where one must be. + if judged == Withheld && strings.HasPrefix(run, "/") { + judged = "/" + Withheld + } + b.WriteString(judged) } b.WriteString(text[last:]) return b.String() diff --git a/internal/inventory/catalogue.go b/internal/inventory/catalogue.go index 274201c..fd56782 100644 --- a/internal/inventory/catalogue.go +++ b/internal/inventory/catalogue.go @@ -755,6 +755,18 @@ func profileFrom(raw []byte) ([]Capability, error) { // this is a statement of the whole layer, so removing a key is done by leaving it out, which is // the only way removing one could work at all. func (i *Inventory) SetSettings(ctx context.Context, nodeName, module string, values map[string]any) error { + return i.setSettings(ctx, nodeName, module, values, true) +} + +// KeepSettings records a layer the mesh already holds, as it holds it, without judging it alone: for a +// store raised from the facts snapshot (the merge gate), where the layers arrive one at a time and a +// mesh-wide layer that needs a machine's own value to compose would be refused before that machine's +// layer is there — though the mesh keeps both and composes. Composition still judges every layer. +func (i *Inventory) KeepSettings(ctx context.Context, nodeName, module string, values map[string]any) error { + return i.setSettings(ctx, nodeName, module, values, false) +} + +func (i *Inventory) setSettings(ctx context.Context, nodeName, module string, values map[string]any, judge bool) error { raw, err := json.Marshal(values) if err != nil { return err @@ -762,8 +774,12 @@ func (i *Inventory) SetSettings(ctx context.Context, nodeName, module string, va // Judged here, against the module's current definition, before it is kept (novox/hq ADR 0163, // rule 6): a setting that cannot compose is refused where it is set, naming the node, the // module, the layer and the key — never stored to refuse the whole machine where it is read. - if err := i.judgeSettings(ctx, nodeName, module, values); err != nil { - return err + if judge { + if err := i.judgeSettings(ctx, nodeName, module, values); err != nil { + return err + } + } else if _, err := i.declared(ctx, module); err != nil { + return fmt.Errorf("%w: %s", ErrNoSuchModule, module) } if nodeName == "" { // A port is a fact about one machine (novox/hq ADR 0100). Refused here, in composition's