diff --git a/cmd/mesh-builder/main.go b/cmd/mesh-builder/main.go index fc418367..51dcd1b4 100644 --- a/cmd/mesh-builder/main.go +++ b/cmd/mesh-builder/main.go @@ -24,7 +24,6 @@ import ( "net/url" "os" "os/signal" - "path/filepath" "sort" "strings" "syscall" @@ -314,9 +313,9 @@ func answer(ctx context.Context, publisher builder.Publisher, on, workspace stri registry = r.Address } spec := checkSpecOf(request) - // A release branch's judge is kept beside the workspace, never in it: the check's containers mount the - // workspace, and a pull request's script must not plant the judge of the next check (novox/hq ADR 0358 §2). - spec.JudgeCache = judgeCacheOf(workspace) + // The judge cache the build agent declares as its own data, never in the workspace: only judge builds mount + // it, so no pull request's script can write what a judge is built with or kept as (novox/hq ADR 0358 §6). + spec.JudgeCache = judgeCache() // What the merge would move, built by the check for the walk to take (novox/hq ADR 0350 §1): into the // scratch namespace. **Never with the package registry's credential**: this is code nobody has approved // yet, and a recipe could read the credential it is handed (issue 462). A module that resolves packages @@ -476,11 +475,10 @@ func checkSpecOf(request link.BuildRequest) builder.CheckSpec { return spec } -// judgeCacheOf is where a release branch's judge is kept (novox/hq ADR 0358 §2): beside the workspace, outside what -// a check's containers mount. -func judgeCacheOf(workspace string) string { - return filepath.Clean(workspace) + "-judges" -} +// judgeCache is the judge cache the build agent's manifest declares (MESH_JUDGE_CACHE, novox/hq ADR 0358 §6): a +// directory mounted at the same path on the host, so a judge build's mounts name what the agent sees. Empty when the +// agent declares none, and every judge is then built cold under its check's root. +func judgeCache() string { return os.Getenv("MESH_JUDGE_CACHE") } // triedOf is a try's outcome as its log line says it (novox/hq issue 686): the repository's own check's verdict, // or the check's where that did not run — a head that does not merge — and the lines naming what failed. diff --git a/cmd/mesh-builder/release_test.go b/cmd/mesh-builder/release_test.go index 0b678d16..2eb9c8f3 100644 --- a/cmd/mesh-builder/release_test.go +++ b/cmd/mesh-builder/release_test.go @@ -1,8 +1,6 @@ package main import ( - "path/filepath" - "strings" "testing" "github.com/novox/mesh-controller/internal/link" @@ -10,7 +8,7 @@ import ( // **A pull request into a release branch reaches the check as one** (novox/hq ADR 0358): the build seat hands the // check the release branch and, beside it, the trunk each repository is read at when it has no branch of that name; -// and it keeps the release branch's judge outside the workspace a check's containers mount. +// and it hands the check the judge cache the build agent declares (MESH_JUDGE_CACHE), never a path of its own. func TestTheBuildSeatHandsTheCheckItsReleaseBranch(t *testing.T) { spec := checkSpecOf(link.BuildRequest{ID: "b", Repository: "r", Ref: "abc", Check: &link.CheckRequest{ Owner: "novox", Repo: "mesh-catalog", Base: "feat/57", Release: "feat/57", @@ -18,9 +16,12 @@ func TestTheBuildSeatHandsTheCheckItsReleaseBranch(t *testing.T) { if spec.Release != "feat/57" || spec.Beside["mesh-host"].Ref != "feat/57" || spec.Beside["mesh-host"].Else != "main" { t.Fatalf("the check was handed release %q and mesh-host %+v", spec.Release, spec.Beside["mesh-host"]) } - workspace := filepath.Join(t.TempDir(), "mesh-builder") - cache := judgeCacheOf(workspace) - if rel, err := filepath.Rel(workspace, cache); err != nil || !strings.HasPrefix(rel, "..") { - t.Fatalf("the judge cache %s lies inside the workspace %s, which a check's containers mount", cache, workspace) + t.Setenv("MESH_JUDGE_CACHE", "/var/lib/mesh/build-agent/judge-cache") + if got := judgeCache(); got != "/var/lib/mesh/build-agent/judge-cache" { + t.Fatalf("the judge cache the manifest declares is read as %q", got) + } + t.Setenv("MESH_JUDGE_CACHE", "") + if got := judgeCache(); got != "" { + t.Fatalf("with no judge cache declared, %q is used", got) } } diff --git a/internal/builder/check.go b/internal/builder/check.go index 202a9abf..dcd0f5c2 100644 --- a/internal/builder/check.go +++ b/internal/builder/check.go @@ -94,9 +94,12 @@ type CheckSpec struct { // the release branch too; a change to the controller still judges itself and a change to the node-engine brings // its own validator. The verdict names the branch, the judge's commit and the branch each beside was read at. Release string - // JudgeCache is a directory outside the workspace where a release branch's judge is kept by the commits it is - // built from, so it is built once per release-branch head (ADR 0358 §2). Outside: the check's containers mount - // the workspace and a pull request's own script must not plant a judge for the next check. Empty keeps none. + // JudgeCache is the judge cache (novox/hq ADR 0358 §6): a directory of the build agent's own, outside the + // workspace, that only judge builds mount — never a check's containers, so no pull request's script can write it. + // It holds the Go build cache every judge is built with (judges stay warm), the release-branch judges kept by the + // commits they are built from (judges/, one per release-branch head), and each judge build's fresh home (homes/, + // removed after the build). Empty: each judge is built cold under the check's own root, which every check empties + // before it clones, and nothing is kept. JudgeCache string // Toolchain is the image a check's Go runs in: the mesh's own Go toolchain, as it holds it. // Toolchains is every toolchain the mesh holds, by language, for a script that declares another. @@ -598,20 +601,21 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry // **Every judge is built where no pull request can reach** (novox/hq issue 482, ADR 0358 review): the workspace's // Go cache, HOME and git configuration are written by pull requests' own scripts, and git runs during a Go build // to stamp its version — a planted hook would rewrite the judge's sources before they compile. So a judge is built - // with a HOME, configuration and Go cache of its own, outside the workspace and made fresh for this check, no - // global or system git configuration, no Go environment file, and no version stamping from git. - judgeHome, err := os.MkdirTemp("", "mesh-check-judge-") + // with a HOME and configuration made fresh for this check and a Go cache only judge builds mount (judgeBuildDirs), + // no global or system git configuration, no Go environment file, and no version stamping from git. + judgeHome, judgeGoCache, err := judgeBuildDirs(spec.JudgeCache, workspace, root, spec.ID) if err != nil { return CheckVerdict{}, err } defer os.RemoveAll(judgeHome) - if withinDir(workspace, judgeHome) { - return CheckVerdict{}, fmt.Errorf("the temporary directory %s is inside the workspace %s, which a check's "+ - "containers mount: no judge is built where a pull request could reach it", judgeHome, workspace) - } inJudgeBuild := func(dir string, _ []string, command ...string) []string { - args := in(spec.Toolchain, dir, JudgeBuildEnv(judgeHome), command...) - return append([]string{args[0], "--volume", judgeHome + ":" + judgeHome}, args[1:]...) + args := in(spec.Toolchain, dir, JudgeBuildEnv(judgeHome, judgeGoCache), command...) + if spec.JudgeCache == "" { + return args // under the check's root, in the workspace mounted already + } + // The judge cache's parts this build needs, mounted into judge builds alone (ADR 0358 §6). + return append([]string{args[0], "--volume", judgeHome + ":" + judgeHome, "--volume", + judgeGoCache + ":" + judgeGoCache}, args[1:]...) } var out tail // running runs one container of the check, its output into the report, in a process group of its own. @@ -1153,7 +1157,7 @@ func releaseJudge(ctx context.Context, run, plain Runner, spec CheckSpec, root s with = ", with mesh-host's validator of " + spec.Release + " at " + twelve(host) } bin := filepath.Join(root, "bin", "mesh-controller") - if took := judgeKept(spec.JudgeCache, key, bin); took { + if took := judgeKept(keptJudges(spec.JudgeCache), key, bin); took { say("check", "judged by %s's controller at %s%s, built for an earlier check of that head", spec.Release, twelve(key), with) return bin, dir, nil @@ -1172,21 +1176,63 @@ func releaseJudge(ctx context.Context, run, plain Runner, spec CheckSpec, root s return "", "", fmt.Errorf("the controller of %s does not build%s: %w", spec.Release, with, err) } // Kept before anything of the change runs: nothing a pull request's script writes can reach the cache. - keepJudge(spec.JudgeCache, key, bin, say) + keepJudge(keptJudges(spec.JudgeCache), key, bin, say) say("check", "judged by %s's controller at %s%s", spec.Release, twelve(key), with) return bin, dir, nil } -// JudgeBuildEnv is the environment every judge is built in (novox/hq issue 482): home, a directory outside the -// workspace made fresh for the check, as HOME, configuration and Go cache; no global or system git configuration; no -// Go environment file; and no version stamped from git, so no git runs during the build at all. -func JudgeBuildEnv(home string) []string { +// JudgeBuildEnv is the environment every judge is built in (novox/hq ADR 0358 §6, issue 482): home, made fresh for the +// build, as HOME and configuration; goCache, the judge cache's Go cache that no check's container mounts; no global +// or system git configuration; no Go environment file; and no version stamped from git, so no git runs at all. +func JudgeBuildEnv(home, goCache string) []string { return []string{"HOME=" + home, "XDG_CONFIG_HOME=" + filepath.Join(home, ".config"), "GIT_CONFIG_GLOBAL=/dev/null", "GIT_CONFIG_NOSYSTEM=1", "GOENV=off", - "GOCACHE=" + filepath.Join(home, "go-cache"), "GOPATH=" + filepath.Join(home, "go"), + "GOCACHE=" + goCache, "GOPATH=" + filepath.Join(home, "go"), "CGO_ENABLED=0", "GOFLAGS=-mod=vendor -buildvcs=false", "GOPROXY=off"} } +// judgeBuildDirs are the home and the Go cache a check's judge builds use (ADR 0358 §6). With a judge cache: a home +// made fresh under its homes/ for this check, and its go-cache/, kept warm between checks — both on the host path the +// build agent declares, so the container runtime mounts what the agent sees, and nothing is left in a temporary +// directory of the host. Without one: both under the check's root, which every check empties before it clones. A +// judge cache inside the workspace is refused: a check's containers mount the workspace. +func judgeBuildDirs(cache, workspace, root, id string) (string, string, error) { + if cache == "" { + home, goCache := filepath.Join(root, "judge-home"), filepath.Join(root, "judge-go-cache") + for _, d := range []string{home, goCache} { + if err := os.MkdirAll(d, 0o700); err != nil { + return "", "", err + } + } + return home, goCache, nil + } + if withinDir(workspace, cache) { + return "", "", fmt.Errorf("the judge cache %s is inside the workspace %s, which a check's containers mount: "+ + "no judge is built where a pull request could reach it", cache, workspace) + } + if !safeName.MatchString(id) { + id = "check" + } + home, goCache := filepath.Join(cache, "homes", id), filepath.Join(cache, "go-cache") + if err := os.RemoveAll(home); err != nil { + return "", "", err + } + for _, d := range []string{home, goCache} { + if err := os.MkdirAll(d, 0o700); err != nil { + return "", "", err + } + } + return home, goCache, nil +} + +// keptJudges is where a judge cache keeps the release-branch judges, empty when there is no judge cache. +func keptJudges(cache string) string { + if cache == "" { + return "" + } + return filepath.Join(cache, "judges") +} + // judgesKept is how many release-branch judges a judge cache keeps: a head moved past is not built from again. const judgesKept = 4 diff --git a/internal/builder/release_branch_test.go b/internal/builder/release_branch_test.go index d5509687..123c13c0 100644 --- a/internal/builder/release_branch_test.go +++ b/internal/builder/release_branch_test.go @@ -1,6 +1,7 @@ package builder import ( + "fmt" "net" "os" "os/exec" @@ -130,6 +131,7 @@ case "$cmd" in ps|rm|exec) exit 0 ;; port) echo "` + bus.Addr().String() + `"; exit 0 ;; run) + if [ -n "$RUNS" ]; then echo "$*" >> "$RUNS"; fi workdir=""; detached="" while [ $# -gt 0 ]; do case "$1" in @@ -256,12 +258,16 @@ func TestACataloguePullRequestIntoAReleaseBranchIsJudgedByThatBranchsController( if n := builds(t, built); n != 1 { t.Fatalf("the judge was built %d times for one check", n) } - // **The judge is built with a cache no pull request's script can write** (issue 482): a fresh one under the - // check's own root, never the workspace's shared go-cache, and no Go environment file from the workspace's HOME. + // **The judge is built with the judge cache's Go cache** (ADR 0358 §6), which no check's container mounts, and no + // Go environment file. said, _ := os.ReadFile(built) - if !strings.Contains(string(said), "/mesh-check-judge-") || !strings.Contains(string(said), "GOENV=off go build") || - strings.Contains(string(said), "/check/../go-cache") { - t.Fatalf("the release judge was built with the workspace's cache or Go environment:\n%s", said) + if !strings.Contains(string(said), "GOCACHE="+filepath.Join(cache, "go-cache")+" GOENV=off go build") { + t.Fatalf("the release judge was not built with the judge cache's Go cache:\n%s", said) + } + // **Clause 5**: the head was merged onto the release branch's head, which the verdict names as its trunk commit, + // so the release branch's move puts it out of date as a trunk's move does (ADR 0323 §4, the forge's holder). + if releaseHead := gitInRepo(t, catalogue, "rev-parse", releaseBranch); v.Trunk != releaseHead[:12] { + t.Fatalf("the verdict names trunk %q, not the release branch's head %s", v.Trunk, releaseHead[:12]) } // **Built once per release-branch head**: a second check while that head stands takes the judge kept. @@ -545,3 +551,104 @@ func TestTheBuildSeatNeverReadsMeshLabAtAReleaseBranch(t *testing.T) { } } } + +// **Judges are built warm and where no pull request reaches** (ADR 0358 §6): every judge build uses the judge cache's +// one Go cache, kept between checks; its home is made fresh under the judge cache and removed after; nothing is left in +// a temporary directory of the host; and no container a check runs for the pull request mounts the judge cache. +func TestJudgesAreBuiltWarmFromAJudgeCacheNoCheckContainerMounts(t *testing.T) { + built := aRuntimeThatBuildsTheJudge(t) + runs := filepath.Join(t.TempDir(), "runs") + t.Setenv("RUNS", runs) + tmp := t.TempDir() + t.Setenv("TMPDIR", tmp) + registry := aFactsRegistry(t) + cache := t.TempDir() + controller, controllerMain, _ := aRepositoryWithARelease(t, aJudgeOf("the controller the mesh runs"), nil) + beside := map[string]Beside{"mesh-controller": {Repository: controller, Ref: controllerMain}} + for i := range 2 { + catalogue, head := aPullRequestInto(t, map[string]string{"modules/gitea/module.json": `{"module":"gitea"}`, + CheckScript: "true\n"}, "main", map[string]string{"modules/gitea/index.ts": fmt.Sprint(i)}) + v, err := Check(t.Context(), Command, CheckSpec{ID: fmt.Sprintf("check-704-warm-%d", i), Repository: catalogue, + Ref: head, Owner: "novox", Repo: "mesh-catalog", Number: 1, Base: "main", JudgeCache: cache, + Paths: []string{"modules/gitea/index.ts"}, Modules: []string{"gitea"}, Toolchain: "golang", Beside: beside}, + t.TempDir(), registry, GitCredential{}, nil) + if err != nil { + t.Fatal(err) + } + if v.Gate == nil || v.Gate.Summary != "the controller the mesh runs with the vendored validator" { + t.Fatalf("check %d was not judged: %+v\n%s", i, v.Gate, v.Report) + } + if i == 0 { + if entries, _ := os.ReadDir(filepath.Join(cache, "go-cache")); len(entries) == 0 { + t.Fatal("the judge's build left nothing in the judge cache's Go cache to build the next judge warm") + } + } + } + // Both judges built with the one Go cache of the judge cache. + said, _ := os.ReadFile(built) + if n := strings.Count(string(said), "GOCACHE="+filepath.Join(cache, "go-cache")+" GOENV=off go build"); n != 2 { + t.Fatalf("%d of 2 judge builds used the judge cache's Go cache:\n%s", n, said) + } + // Nothing left behind: no home under the judge cache, nothing in the host's temporary directory. + if left, _ := os.ReadDir(filepath.Join(cache, "homes")); len(left) != 0 { + t.Fatalf("judge builds left %d home(s) under the judge cache", len(left)) + } + if left, _ := os.ReadDir(tmp); len(left) != 0 { + t.Fatalf("a check left %d entr(ies) in the temporary directory, e.g. %s", len(left), left[0].Name()) + } + // Only a judge build mounts the judge cache: no container the check runs for the pull request does. + body, _ := os.ReadFile(runs) + judgeBuilds := 0 + for _, line := range strings.Split(strings.TrimSpace(string(body)), "\n") { + judging := strings.Contains(line, " go build ") + if judging { + judgeBuilds++ + if !strings.Contains(line, "--volume "+filepath.Join(cache, "go-cache")+":") { + t.Fatalf("a judge build did not mount the judge cache's Go cache: %s", line) + } + continue + } + if strings.Contains(line, cache) { + t.Fatalf("a container the check runs for the pull request names the judge cache: %s", line) + } + } + if judgeBuilds != 2 { + t.Fatalf("%d judge builds were run, not 2", judgeBuilds) + } +} + +// Without a judge cache, a judge is built cold under the check's own root — never in a temporary directory of the +// host — and a judge cache inside the workspace is refused. +func TestWithoutAJudgeCacheAJudgeIsBuiltColdUnderTheChecksRoot(t *testing.T) { + built := aRuntimeThatBuildsTheJudge(t) + tmp := t.TempDir() + t.Setenv("TMPDIR", tmp) + registry := aFactsRegistry(t) + controller, controllerMain, _ := aRepositoryWithARelease(t, aJudgeOf("the controller the mesh runs"), nil) + beside := map[string]Beside{"mesh-controller": {Repository: controller, Ref: controllerMain}} + catalogue, head := aPullRequestInto(t, map[string]string{"modules/gitea/module.json": `{"module":"gitea"}`, + CheckScript: "true\n"}, "main", map[string]string{"modules/gitea/index.ts": "x"}) + workspace := t.TempDir() + spec := CheckSpec{ID: "check-704-cold", Repository: catalogue, Ref: head, Owner: "novox", Repo: "mesh-catalog", + Number: 1, Base: "main", Paths: []string{"modules/gitea/index.ts"}, Modules: []string{"gitea"}, + Toolchain: "golang", Beside: beside} + v, err := Check(t.Context(), Command, spec, workspace, registry, GitCredential{}, nil) + if err != nil { + t.Fatal(err) + } + if v.Gate == nil || v.Gate.Verdict != "pass" { + t.Fatalf("the check was not judged: %+v\n%s", v.Gate, v.Report) + } + said, _ := os.ReadFile(built) + if !strings.Contains(string(said), "GOCACHE="+filepath.Join(workspace, "check", "judge-go-cache")+" GOENV=off go build") { + t.Fatalf("without a judge cache the judge was not built under the check's root:\n%s", said) + } + if left, _ := os.ReadDir(tmp); len(left) != 0 { + t.Fatalf("a check left %d entr(ies) in the temporary directory", len(left)) + } + spec.JudgeCache = filepath.Join(workspace, "judge-cache") + if _, err := Check(t.Context(), Command, spec, workspace, registry, GitCredential{}, nil); err == nil || + !strings.Contains(err.Error(), "inside the workspace") { + t.Fatalf("a judge cache inside the workspace was taken: %v", err) + } +}