diff --git a/modules/mesh-build-go/README.md b/modules/mesh-build-go/README.md index f2493e35..7b2c14f3 100644 --- a/modules/mesh-build-go/README.md +++ b/modules/mesh-build-go/README.md @@ -102,7 +102,11 @@ integration level, the postgres major and the nats-server version; and, when the workspace (`beside`), each one's repository and commit, which the answer echoes as `beside` (the checker keeps a verdict only for the siblings it was judged with). The text's form is `DigestVersion` 3. The Go version is not in it: the toolchain enters a fingerprint once, as the version the build nodes agree on (ADR 0477 §3), and the standard library's packages are that -version's. A package with no tests at the level has no digest and is named in `without_tests`. A directory that vendors +version's. A package with no tests at the level has no digest and is named in `without_tests`. A catalogue module's packages also hash the module's non-Go files (the nearest ancestor directory holding `module.json`: its manifest, README, `files/` and the rest, so `../../module.json` is covered; digest form 5). A package whose directory holds a file named `reads-outside` (any content; say what the tests read) is never kept (issue 1935): its tests read +files outside it, which a digest from `go list` cannot name, so its digest is a fresh random one on every call, the ask finds no kept +verdict for it and runs it, and the answer names it in `reads_outside` and the module's log says why. The checker still writes a +verdict under that digest, which nothing ever reads again. The marker is the plain way to say it until each verdict keeps the files +its tests opened (Go's test log). A directory that vendors is listed and tested from its vendor directory (`-mod=vendor`). ## A change to this module is judged by its candidate diff --git a/modules/mesh-build-go/cmd/mesh-build-go/beside_digest_test.go b/modules/mesh-build-go/cmd/mesh-build-go/beside_digest_test.go index 3682a61b..22aebaca 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/beside_digest_test.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/beside_digest_test.go @@ -116,7 +116,7 @@ func TestTheIntegrationLevelDigestsUnderBothTags(t *testing.T) { t.Errorf("the integration digest listed %q", s.command) } } - if DigestVersion != "4" { + if DigestVersion != "5" { t.Errorf("the digest's form changed in kind and is still version %s", DigestVersion) } } diff --git a/modules/mesh-build-go/cmd/mesh-build-go/digest.go b/modules/mesh-build-go/cmd/mesh-build-go/digest.go index 1c92d318..bce625ba 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/digest.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/digest.go @@ -7,6 +7,7 @@ package main import ( "bytes" "context" + "crypto/rand" "crypto/sha256" "encoding/hex" "encoding/json" @@ -24,12 +25,20 @@ import ( // 2: no Go version in the text; the toolchain enters a fingerprint as the agreed version (ADR 0477 §3). 3: the siblings' // commits (the lead's decision L5) and the integration level's tags integration,isolation (the checker's agreement). // 4: the siblings' commits enter only the digest of a package whose tests reach a file naming MESH_CHECK_BESIDE (ADR 0536 §1). -const DigestVersion = "4" +// 5: a catalogue module's non-Go files enter the digest of its packages (issue 1935). +const DigestVersion = "5" // besideVariable is the name a file bears when it reads where the siblings lie; a package whose tests reach such a file // depends on the siblings' commits. const besideVariable = "MESH_CHECK_BESIDE" +// ReadsOutsideMarker is the name of a file in a package's directory that says its tests read files outside it (a +// repository's documents, a sibling directory): such a package is never kept (issue 1935). The digest hashes only what +// the Go toolchain lists, so it cannot name what a test opens elsewhere; a marker file, beside the Go files where a +// reader of the package sees it, is the plain way to say so until the checker keeps the files a test opened (the Go +// test log) with each verdict. +const ReadsOutsideMarker = "reads-outside" + // integrationTags are the build tags of the integration level, its run and its digest alike: integration, and isolation // for the bubblewrap proofs (mesh-sdk's and the build modules'), which a build node can run. var integrationTags = []string{"integration", "isolation"} @@ -89,6 +98,9 @@ type DigestResult struct { // only when they are the siblings it gave the fetch. Beside []string `json:"beside,omitempty"` WithoutTests []string `json:"without_tests,omitempty"` + // ReadsOutside names the packages that carry the marker: each has a digest that no kept verdict can have (a fresh + // random one), so the check runs it every time. + ReadsOutside []string `json:"reads_outside,omitempty"` // JudgedBy is the program that ran the digest (ADR 0535 §5): "installed", or "candidate " under the // runner workspace. JudgedBy string `json:"judged_by,omitempty"` @@ -199,6 +211,12 @@ func (m *Module) digestHere(ctx context.Context, workspace, level string) (Diges } } sort.Strings(res.WithoutTests) + sort.Strings(res.ReadsOutside) + for _, p := range res.ReadsOutside { + if m.Say != nil { + m.Say("the %s level of %s always runs: its directory carries %s, so its tests read files outside it that no digest names", level, p, ReadsOutsideMarker) + } + } return res, nil } @@ -256,6 +274,17 @@ func digestModule(res *DigestResult, pkgs map[string]*goPackage, tree string, he if testMain == nil { return fmt.Errorf("go list gave no test binary for %s", path) } + if _, err := os.Stat(filepath.Join(root.Dir, ReadsOutsideMarker)); err == nil { + // The digest of a package that reads outside itself is one nothing can have kept: the key is a fresh random + // hex, so the ask finds no kept verdict and runs it, and what the checker writes under it is never read again. + var nonce [32]byte + if _, err := rand.Read(nonce[:]); err != nil { + return err + } + res.Packages[path] = hex.EncodeToString(nonce[:]) + res.ReadsOutside = append(res.ReadsOutside, path) + continue + } d, err := packageDigest(root, testMain, pkgs, tree, header, beside) if err != nil { return fmt.Errorf("%s: %w", path, err) @@ -303,6 +332,15 @@ func packageDigest(root, testMain *goPackage, pkgs map[string]*goPackage, tree s } } } + // A catalogue module's tests read its manifest, its README and its files from the package's directory (../../module.json): + // every non-Go file of the module's directory is an input (issue 1935). The module is the nearest ancestor with a module.json. + if modDir := enclosingModule(tree, root.Dir); modDir != "" { + more, err := moduleFileLines(tree, root, modDir) + if err != nil { + return "", err + } + lines = append(lines, more...) + } data := filepath.Join(root.Dir, "testdata") if _, err := os.Stat(data); err == nil { err := filepath.WalkDir(data, func(path string, e fs.DirEntry, err error) error { @@ -328,6 +366,41 @@ func packageDigest(root, testMain *goPackage, pkgs map[string]*goPackage, tree s return hex.EncodeToString(h.Sum(nil)), nil } +// enclosingModule is the nearest ancestor of dir, inside tree, that holds a module.json, or "" when there is none. +func enclosingModule(tree, dir string) string { + tree = filepath.Clean(tree) + for d := filepath.Clean(dir); d != tree && strings.HasPrefix(d, tree+string(filepath.Separator)); d = filepath.Dir(d) { + if st, err := os.Stat(filepath.Join(d, "module.json")); err == nil && !st.IsDir() { + return d + } + } + return "" +} + +// moduleFileLines are the content lines of every file of a module's directory that is not Go source (Go source is the +// packages' own, listed by go list), skipping .git and node_modules. +func moduleFileLines(tree string, p *goPackage, modDir string) ([]string, error) { + var lines []string + err := filepath.WalkDir(modDir, func(path string, e fs.DirEntry, err error) error { + if err != nil { + return err + } + if e.IsDir() { + if n := e.Name(); n == ".git" || n == "node_modules" { + return filepath.SkipDir + } + return nil + } + if strings.HasSuffix(path, ".go") { + return nil + } + l, err := contentLine(tree, p, path) + lines = append(lines, l) + return err + }) + return lines, err +} + func fileLine(tree string, p *goPackage, name string) (string, error) { return contentLine(tree, p, filepath.Join(p.Dir, name)) } diff --git a/modules/mesh-build-go/cmd/mesh-build-go/digest_test.go b/modules/mesh-build-go/cmd/mesh-build-go/digest_test.go index 5e2d81d0..9b6fd78c 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/digest_test.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/digest_test.go @@ -189,3 +189,71 @@ func TestAModuleOutsideTheTreeCountsByVersionAndChecksum(t *testing.T) { t.Errorf("a new version left the digest as it was") } } + +// Issue 1935: a package whose directory carries the reads-outside marker is never kept: its digest is one no kept verdict +// can have (it differs on every call, though nothing changed), and the answer names it. A package without the marker +// keeps its digest. +func TestAPackageWithTheReadsOutsideMarkerIsNeverKept(t *testing.T) { + t.Parallel() + m, tree := digestFixture(t) + plain := digests(t, m, "unit") + if len(plain.ReadsOutside) != 0 { + t.Fatalf("a package carries the marker in the fixture: %v", plain.ReadsOutside) + } + edit(t, tree, "c/"+ReadsOutsideMarker, "the tests read ../docs\n") + first, second := digests(t, m, "unit"), digests(t, m, "unit") + c := "fixture/digest/c" + if len(first.Packages[c]) != 64 || first.Packages[c] == second.Packages[c] || first.Packages[c] == plain.Packages[c] { + t.Errorf("c carries the marker and its digest is stable: %s, %s, before %s", first.Packages[c], second.Packages[c], plain.Packages[c]) + } + if strings.Join(first.ReadsOutside, ",") != c { + t.Errorf("the answer does not name the package that reads outside: %v", first.ReadsOutside) + } + a := "fixture/digest/a" + if first.Packages[a] != plain.Packages[a] || first.Packages[a] != second.Packages[a] { + t.Errorf("a has no marker and its digest moved: %s, %s, before %s", first.Packages[a], second.Packages[a], plain.Packages[a]) + } +} + +// Issue 1935: a catalogue module's packages read ../../module.json and the module's files: a change to them changes the +// digest of the module's package, and a change to another module's files does not. +func TestACatalogueModulesFilesChangeTheDigestOfItsPackagesOnly(t *testing.T) { + t.Parallel() + m, tree := digestFixture(t) + for _, mod := range []string{"mod1", "mod2"} { + dir := filepath.Join(tree, mod, "cmd", "p") + if err := os.MkdirAll(filepath.Join(tree, mod, "files"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + edit(t, tree, mod+"/module.json", `{"module":"`+mod+`"}`) + edit(t, tree, mod+"/README.md", "# "+mod+"\n") + edit(t, tree, mod+"/files/f.env", "A=1\n") + edit(t, tree, mod+"/cmd/p/p.go", "package p\n") + edit(t, tree, mod+"/cmd/p/p_test.go", "package p\n\nimport (\n\t\"os\"\n\t\"testing\"\n)\n\nfunc TestReads(t *testing.T) {\n\tif _, err := os.ReadFile(\"../../module.json\"); err != nil {\n\t\tt.Fatal(err)\n\t}\n}\n") + } + p1, p2 := "fixture/digest/mod1/cmd/p", "fixture/digest/mod2/cmd/p" + before := digests(t, m, "unit") + if len(before.Packages[p1]) != 64 || len(before.Packages[p2]) != 64 { + t.Fatalf("no digest for the module packages: %+v", before.Packages) + } + if before.Packages[p1] == before.Packages[p2] { + t.Errorf("two modules, one digest") + } + for _, file := range []string{"mod1/module.json", "mod1/README.md", "mod1/files/f.env"} { + edit(t, tree, file, "changed "+file+"\n") + after := digests(t, m, "unit") + if after.Packages[p1] == before.Packages[p1] { + t.Errorf("a change to %s did not change the module's digest", file) + } + if after.Packages[p2] != before.Packages[p2] { + t.Errorf("a change to %s changed the other module's digest", file) + } + before = after + } + if got := digests(t, m, "unit"); got.Packages["fixture/digest/c"] == "" { + t.Errorf("a package outside any module lost its digest") + } +} diff --git a/modules/mesh-build-go/cmd/mesh-build-go/reads-outside b/modules/mesh-build-go/cmd/mesh-build-go/reads-outside new file mode 100644 index 00000000..c1b869c0 --- /dev/null +++ b/modules/mesh-build-go/cmd/mesh-build-go/reads-outside @@ -0,0 +1 @@ +These tests read other catalogue modules' files, outside this module (issue 1935): never kept. diff --git a/modules/mesh-build-image/cmd/mesh-build-image/reads-outside b/modules/mesh-build-image/cmd/mesh-build-image/reads-outside new file mode 100644 index 00000000..c1b869c0 --- /dev/null +++ b/modules/mesh-build-image/cmd/mesh-build-image/reads-outside @@ -0,0 +1 @@ +These tests read other catalogue modules' files, outside this module (issue 1935): never kept. diff --git a/modules/mesh-build-node/cmd/mesh-build-node/reads-outside b/modules/mesh-build-node/cmd/mesh-build-node/reads-outside new file mode 100644 index 00000000..c1b869c0 --- /dev/null +++ b/modules/mesh-build-node/cmd/mesh-build-node/reads-outside @@ -0,0 +1 @@ +These tests read other catalogue modules' files, outside this module (issue 1935): never kept. diff --git a/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/reads-outside b/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/reads-outside new file mode 100644 index 00000000..c1b869c0 --- /dev/null +++ b/modules/mesh-provisioner-client/cmd/mesh-provisioner-client/reads-outside @@ -0,0 +1 @@ +These tests read other catalogue modules' files, outside this module (issue 1935): never kept. diff --git a/modules/minio/cmd/object-store/reads-outside b/modules/minio/cmd/object-store/reads-outside new file mode 100644 index 00000000..c1b869c0 --- /dev/null +++ b/modules/minio/cmd/object-store/reads-outside @@ -0,0 +1 @@ +These tests read other catalogue modules' files, outside this module (issue 1935): never kept. diff --git a/modules/postgres/cmd/postgres-provider/reads-outside b/modules/postgres/cmd/postgres-provider/reads-outside new file mode 100644 index 00000000..c1b869c0 --- /dev/null +++ b/modules/postgres/cmd/postgres-provider/reads-outside @@ -0,0 +1 @@ +These tests read other catalogue modules' files, outside this module (issue 1935): never kept.