diff --git a/cmd/mesh-controller/merge_gate.go b/cmd/mesh-controller/merge_gate.go index 6e6d0fb9..fb781bda 100644 --- a/cmd/mesh-controller/merge_gate.go +++ b/cmd/mesh-controller/merge_gate.go @@ -271,6 +271,25 @@ func judgeChange(ctx context.Context, in mergeCheckInput) (mergeVerdict, error) // The change, then each other head of its delivery group laid over it in turn (novox/hq ADR 0239): one // future state of the mesh, judged as the group would leave it. heads := append([]groupChange{{Repository: in.repository, Tree: in.tree, Changed: in.changed}}, in.group...) + // **A module one head removes and another defines is a move** (novox/hq ADR 0420 §6, issue 1025): neither + // the removal of a module machines run (ADR 0236) nor a second definition of one module name (issue 239). + // The change is judged with the module built from the repository that now defines it. + moves, err := movesInGroup(heads, sources) + if err != nil { + return v, err + } + changeSources := sources + if len(moves) > 0 { + changeSources = make(map[string]inventory.Source, len(sources)) + for k, o := range sources { + changeSources[k] = o + } + for name, mv := range moves { + o := changeSources[name] + o.Repository, o.Path = mv.To, mv.Dir + changeSources[name] = o + } + } for i, h := range heads { var r *mergeReach if h.Repository != "" && len(h.Changed) > 0 { @@ -289,13 +308,23 @@ func judgeChange(ctx context.Context, in mergeCheckInput) (mergeVerdict, error) continue } var failures []string - change, failures, err = shelfWithChange(change, sources, in.facts, h.Repository, h.Tree, v.Modules, - scopeOfReach(r, h.Repository)) + scope := scopeOfReach(r, h.Repository) + if scope != nil { + for _, mv := range moves { + if mv.To == h.Repository { + scope[mv.Dir] = true + } + } + } + change, failures, err = shelfWithChange(change, changeSources, in.facts, h.Repository, h.Tree, v.Modules, scope) if err != nil { return v, err } v.Failures = append(v.Failures, failures...) } + for name := range moves { + v.Modules[name] = "moved" + } if len(in.group) > 0 { say("a delivery group: %d head(s) composed together", len(heads)) } @@ -332,7 +361,7 @@ func judgeChange(ctx context.Context, in mergeCheckInput) (mergeVerdict, error) } } sort.Strings(added) - after, tried, notes, err := composeEveryMachine(ctx, in, change, sources, added) + after, tried, notes, err := composeEveryMachine(ctx, in, change, changeSources, added) if err != nil { return v, fmt.Errorf("the mesh with the change could not be raised: %w", err) } @@ -531,6 +560,98 @@ func sourcesOfFacts(f snapshot.Facts) map[string]inventory.Source { return out } +// moveOfModule is a module a delivery group moves: the repository the mesh builds it from, the one whose +// head now defines it, and its directory there. +type moveOfModule struct{ From, To, Dir string } + +// movesInGroup is the modules one head of a group removes and another defines (novox/hq ADR 0420 §6): a name +// the mesh builds from a head's repository that the head's tree no longer defines, and some other head's tree +// defines. A name removed by a head with no other head defining it, or defined by a head the mesh builds from +// another repository with none removing it, is no move. +func movesInGroup(heads []groupChange, sources map[string]inventory.Source) (map[string]moveOfModule, error) { + if len(heads) < 2 { + return nil, nil + } + defined := make([]map[string]string, len(heads)) + for i, h := range heads { + if h.Tree == "" { + continue + } + d, err := namesDefinedIn(h.Tree) + if err != nil { + return nil, err + } + defined[i] = d + } + var moves map[string]moveOfModule + for name, s := range sources { + if s.Repository == "" { + continue + } + for i, h := range heads { + if h.Tree == "" || !sameRepository(s.Repository, link.SourceMoved{Owner: ownerOf(h.Repository), Repo: repoOf(h.Repository)}) { + continue + } + if _, still := defined[i][name]; still { + break + } + // Removed by head i: defined by another? + for j, o := range heads { + if dir, ok := defined[j][name]; ok && j != i && o.Repository != h.Repository { + if moves == nil { + moves = map[string]moveOfModule{} + } + moves[name] = moveOfModule{From: h.Repository, To: o.Repository, Dir: dir} + } + } + break + } + } + return moves, nil +} + +// namesDefinedIn is the module names a tree's module.json files define, each with its directory; a manifest +// that does not read is left to shelfWithChange to refuse. +func namesDefinedIn(tree string) (map[string]string, error) { + found := map[string]string{} + err := filepath.WalkDir(tree, func(path string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + if d.IsDir() { + if path != tree && (ignoredInTree[d.Name()] || strings.HasPrefix(d.Name(), ".")) { + return filepath.SkipDir + } + return nil + } + if d.Name() != "module.json" { + return nil + } + raw, err := os.ReadFile(path) + if err != nil { + return err + } + var m struct { + Module string `json:"module"` + } + if json.Unmarshal(raw, &m) != nil || m.Module == "" { + return nil + } + dir, err := filepath.Rel(tree, filepath.Dir(path)) + if err != nil { + return err + } + if dir == "." { + dir = "" + } + if _, twice := found[m.Module]; !twice { + found[m.Module] = dir + } + return nil + }) + return found, err +} + // ignoredInTree are directories a module.json in is not a module of the repository. var ignoredInTree = map[string]bool{".git": true, "vendor": true, "node_modules": true, "testdata": true, "examples": true, "fixtures": true} diff --git a/cmd/mesh-controller/merge_gate_test.go b/cmd/mesh-controller/merge_gate_test.go index 00289ff2..e50cb483 100644 --- a/cmd/mesh-controller/merge_gate_test.go +++ b/cmd/mesh-controller/merge_gate_test.go @@ -434,3 +434,124 @@ func sdkPinnedByGoMod(t *testing.T) string { t.Fatalf("go.mod requires no %s", sdkModule) return "" } + +// **A module one head of a group removes and another defines is a move** (novox/hq ADR 0420 §6, issue 1025): +// neither a second definition of one module name (issue 239) nor the removal of a module machines run +// (ADR 0236). Alone, each head is still refused. This lays the heads over the shelf as judgeChange does, +// which needs no database. +func TestAGroupsMoveOfAModuleIsNeitherTwoDefinitionsNorARemoval(t *testing.T) { + const catalogueRepo, controllerRepo = "novox/mesh-catalog", "novox/mesh-controller" + built := func(repo, path string) inventory.Source { + return inventory.Source{Repository: "http://forge.internal:20000/" + repo + ".git", Path: path, BuiltFrom: "c0ffee"} + } + sources := map[string]inventory.Source{"mover": built(catalogueRepo, "modules/mover")} + raw := `{"module":"mover","version":"1"}` + base := map[string]catalogue.Manifest{} + m, err := catalogue.ParseManifest([]byte(raw)) + if err != nil { + t.Fatal(err) + } + base["mover"] = m + facts := snapshot.Facts{Machines: []snapshot.Machine{{Name: "anchor", Assigned: []string{"mover"}}}} + + catalogueWithout := aTree(t, map[string]string{"other": `{"module":"other","version":"1"}`}) + controllerWith := func() string { + dir := t.TempDir() + if err := os.MkdirAll(filepath.Join(dir, "modules", "mover"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "modules", "mover", "module.json"), []byte(raw), 0o644); err != nil { + t.Fatal(err) + } + return dir + }() + removes := groupChange{Repository: catalogueRepo, Tree: catalogueWithout, Changed: []string{"modules/mover/module.json"}} + defines := groupChange{Repository: controllerRepo, Tree: controllerWith, Changed: []string{"modules/mover/module.json"}} + + lay := func(heads ...groupChange) (map[string]catalogue.Manifest, []string, map[string]string) { + t.Helper() + moves, err := movesInGroup(heads, sources) + if err != nil { + t.Fatal(err) + } + at := sources + if len(moves) > 0 { + at = map[string]inventory.Source{} + for k, v := range sources { + at[k] = v + } + for name, mv := range moves { + o := at[name] + o.Repository, o.Path = mv.To, mv.Dir + at[name] = o + } + } + shelf, said := base, map[string]string{} + var failures []string + for _, h := range heads { + var f []string + if shelf, f, err = shelfWithChange(shelf, at, facts, h.Repository, h.Tree, said, nil); err != nil { + t.Fatal(err) + } + failures = append(failures, f...) + } + for name := range moves { + said[name] = "moved" + } + return shelf, failures, said + } + for _, order := range [][]groupChange{{removes, defines}, {defines, removes}} { + shelf, failures, said := lay(order...) + if len(failures) > 0 { + t.Fatalf("a move of a module machines run, laid %s first, was refused: %v", order[0].Repository, failures) + } + if said["mover"] != "moved" { + t.Errorf("the move reads %v, not moved", said) + } + if _, held := shelf["mover"]; !held { + t.Errorf("the moved module is not on the shelf after laying %s first", order[0].Repository) + } + } + // Alone, a head is still refused: a second definition (issue 239), and a removal of what runs (ADR 0236). + if _, failures, _ := lay(defines); len(failures) != 1 || !strings.Contains(failures[0], "issue 239") { + t.Errorf("a definition of a module another repository builds, alone: %v", failures) + } + if _, failures, _ := lay(removes); len(failures) != 1 || !strings.Contains(failures[0], "ADR 0236") { + t.Errorf("the removal of a module machines run, alone: %v", failures) + } +} + +// A group that moves a module machines run passes the gate whole, in either order of its heads, and says the +// module moved (novox/hq ADR 0420 §6, issue 1025); a head alone is still refused. Needs the test store. +func TestADeliveryGroupMovingAModuleMachinesRunPassesTheGate(t *testing.T) { + f, manifests := catalogueMeshWithAnApp(t) + catTree := aTree(t, withEdit(manifests, "album", "")) + app := t.TempDir() + if err := os.MkdirAll(filepath.Join(app, "modules", "album"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(app, "modules", "album", "module.json"), []byte(manifests["album"]), 0o644); err != nil { + t.Fatal(err) + } + removes := groupChange{Repository: "novox/mesh-catalog", Tree: catTree, Changed: []string{"modules/album/module.json"}} + defines := groupChange{Repository: "novox/app", Tree: app, Changed: []string{"modules/album/module.json"}} + judge := func(first groupChange, rest ...groupChange) mergeVerdict { + v, err := judgeChange(t.Context(), mergeCheckInput{facts: f, admin: os.Getenv("MESH_TEST_POSTGRES"), + repository: first.Repository, tree: first.Tree, changed: first.Changed, group: rest}) + if err != nil { + t.Fatal(err) + } + return v + } + for _, v := range []mergeVerdict{judge(removes, defines), judge(defines, removes)} { + if v.Verdict == "fail" || v.Modules["album"] != "moved" { + t.Fatalf("a group moving a module machines run: %v\n%s", v.Modules, v.Report()) + } + } + if v := judge(defines); v.Verdict != "fail" { + t.Errorf("a definition of a module another repository builds passed alone:\n%s", v.Report()) + } + if v := judge(removes); v.Verdict != "fail" { + t.Errorf("the removal of a module machines run passed alone:\n%s", v.Report()) + } +}