diff --git a/cmd/mesh-controller/merge_gate.go b/cmd/mesh-controller/merge_gate.go index a1c473b8..ec61888b 100644 --- a/cmd/mesh-controller/merge_gate.go +++ b/cmd/mesh-controller/merge_gate.go @@ -67,6 +67,8 @@ type mergeCheckInput struct { group []groupChange // 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. + beside string // say is where progress goes; the verdict is returned. say io.Writer } @@ -197,7 +199,7 @@ func mergeGateCommand(ctx context.Context, args []string) error { os.Stdout = os.Stderr } v, err := judgeChange(ctx, mergeCheckInput{facts: f, repository: *repository, tree: *tree, changed: splitList(*changed), - group: others, admin: *admin, say: out}) + group: others, admin: *admin, say: out, beside: os.Getenv("MESH_CHECK_BESIDE")}) os.Stdout = stdout if err != nil { return err @@ -274,7 +276,7 @@ func judgeChange(ctx context.Context, in mergeCheckInput) (mergeVerdict, error) // **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) + moves, err := movesOfChange(in, sources) if err != nil { return v, err } @@ -560,6 +562,28 @@ func sourcesOfFacts(f snapshot.Facts) map[string]inventory.Source { return out } +// movesOfChange is the modules the change moves: with a delivery group, those the group's heads move; without +// one, those it moves with the core clones beside the check (MESH_CHECK_BESIDE, mesh-catalog and +// mesh-controller, the repository a module can move between), so a member judged alone sees the move its +// group's composed check sees (issue 1043). The clones beside are used to find moves only, never as a +// composition of what machines get. +func movesOfChange(in mergeCheckInput, sources map[string]inventory.Source) (map[string]moveOfModule, error) { + heads := append([]groupChange{{Repository: in.repository, Tree: in.tree, Changed: in.changed}}, in.group...) + if len(in.group) == 0 && in.beside != "" && in.repository != "" && in.tree != "" { + for _, name := range []string{"mesh-catalog", "mesh-controller"} { + dir := filepath.Join(in.beside, name) + if repoOf(in.repository) == name { + continue + } + if st, err := os.Stat(dir); err != nil || !st.IsDir() { + continue + } + heads = append(heads, groupChange{Repository: ownerOf(in.repository) + "/" + name, Tree: dir}) + } + } + return movesInGroup(heads, sources) +} + // 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 } diff --git a/cmd/mesh-controller/merge_gate_test.go b/cmd/mesh-controller/merge_gate_test.go index 38c3ae17..11c304c7 100644 --- a/cmd/mesh-controller/merge_gate_test.go +++ b/cmd/mesh-controller/merge_gate_test.go @@ -588,3 +588,82 @@ func TestTheGateRefusesAnArtifactContextNamingAnotherRepository(t *testing.T) { } } } + +// **A member of a delivery group judged alone sees a move** through the core clones beside its check +// (novox/hq ADR 0420 §6, issue 1043): the controller's own gate, with the catalogue beside having removed the +// module, is not refused for a second definition (issue 239); the catalogue's own gate, with the controller +// beside defining it, is not refused for a removal (ADR 0236). A clone beside at main finds no move. +func TestAMemberJudgedAloneSeesAMoveThroughTheClonesBeside(t *testing.T) { + const catalogueRepo, controllerRepo = "novox/mesh-catalog", "novox/mesh-controller" + raw := `{"module":"mover","version":"1"}` + sources := map[string]inventory.Source{"mover": {Repository: "http://forge.internal:20000/" + catalogueRepo + ".git", + Path: "modules/mover", BuiltFrom: "c0ffee"}} + // A tree is written into dir: defining mover, or holding only another module. + write := func(dir string, defines bool) string { + name, body := "other", `{"module":"other","version":"1"}` + if defines { + name, body = "mover", raw + } + if err := os.MkdirAll(filepath.Join(dir, "modules", name), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "modules", name, "module.json"), []byte(body), 0o644); err != nil { + t.Fatal(err) + } + return dir + } + defining := func() string { return write(t.TempDir(), true) } + without := func() string { return write(t.TempDir(), false) } + // beside lays out a MESH_CHECK_BESIDE directory holding the named repositories' trees, each defining mover or not. + beside := func(trees map[string]bool) string { + root := t.TempDir() + for name, defines := range trees { + write(filepath.Join(root, name), defines) + } + return root + } + moves := func(repository, tree, besideRoot string) map[string]moveOfModule { + t.Helper() + got, err := movesOfChange(mergeCheckInput{repository: repository, tree: tree, beside: besideRoot}, sources) + if err != nil { + t.Fatal(err) + } + return got + } + // The controller's member: defines mover; the catalogue beside has removed it. + if got := moves(controllerRepo, defining(), beside(map[string]bool{"mesh-catalog": false})); got["mover"].To != controllerRepo { + t.Errorf("the controller's change defining mover, the catalogue beside without it: moves %v", got) + } + // The sources the gate then judges with: built from the controller, the definition is not a second one (issue 239); + // unmoved, it still is. + judge := func(at map[string]inventory.Source) []string { + t.Helper() + _, failures, err := shelfWithChange(map[string]catalogue.Manifest{}, at, snapshot.Facts{}, controllerRepo, defining(), + map[string]string{}, nil) + if err != nil { + t.Fatal(err) + } + return failures + } + mv := moves(controllerRepo, defining(), beside(map[string]bool{"mesh-catalog": false}))["mover"] + moved := map[string]inventory.Source{"mover": {Repository: mv.To, Path: mv.Dir, BuiltFrom: "c0ffee"}} + if f := judge(moved); len(f) != 0 { + t.Errorf("a definition of a module the catalogue beside removed was refused: %v", f) + } + if f := judge(sources); len(f) != 1 || !strings.Contains(f[0], "issue 239") { + t.Errorf("with the catalogue beside at main, the definition: %v, want the issue 239 refusal", f) + } + if got := moves(controllerRepo, defining(), beside(map[string]bool{"mesh-catalog": true})); len(got) != 0 { + t.Errorf("the catalogue beside at main still defines mover, yet moves %v", got) + } + // The catalogue's member: removes mover; the controller beside defines it. + if got := moves(catalogueRepo, without(), beside(map[string]bool{"mesh-controller": true})); got["mover"].To != controllerRepo { + t.Errorf("the catalogue's change removing mover, the controller beside defining it: moves %v", got) + } + if got := moves(catalogueRepo, without(), beside(map[string]bool{"mesh-controller": false})); len(got) != 0 { + t.Errorf("the controller beside at main does not define mover, yet moves %v", got) + } + if got := moves(catalogueRepo, without(), ""); len(got) != 0 { + t.Errorf("nothing beside, yet moves %v", got) + } +}