Merge pull request 'build-agent and route-proxy live whole in mesh-controller (issue 1019, ADR 0420)' (#370) from feat/whole-modules into main

cmd/mesh-controller/check.go

@@ -11,6 +11,7 @@ import (
 	"time"
 
 	"github.com/novox/mesh-controller/internal/catalogue"
+	"github.com/novox/mesh-controller/internal/link"
 )
 
 // moduleCheck judges manifests where they are written, with no mesh (novox/hq ADR 0037, issue 148).
@@ -114,6 +115,20 @@ func moduleCheckFor(paths []string, longestMachine int, out io.Writer) error {
 			failed += len(wrong)
 			faulted[m.Module] = true
 		}
+		// A module lives whole in one repository (novox/hq ADR 0420 §1, issue 1019): when the check is told the
+		// repository it runs for (MESH_CHECK_REPOSITORY, set by the build's check), an artifact building from
+		// another is refused, as the merge gate refuses it. A run by hand names no repository and judges none.
+		if repo := os.Getenv("MESH_CHECK_REPOSITORY"); repo != "" && m.Build != nil {
+			for _, a := range m.Build.Artifacts {
+				if a.Context != nil && a.Context.Repository != "" && !sameRepository(a.Context.Repository,
+					link.SourceMoved{Owner: ownerOf(repo), Repo: repoOf(repo)}) {
+					fmt.Fprintf(out, "%s: %s builds artifact %s from %s: a module lives whole in one repository "+
+						"(novox/hq ADR 0420 §1)\n", path, m.Module, a.Name, a.Context.Repository)
+					failed++
+					faulted[m.Module] = true
+				}
+			}
+		}
 		if named := catalogue.InstallationProblems(m); len(named) > 0 {
 			for _, p := range named {
 				fmt.Fprintf(out, "%s: %s\n", path, p)

cmd/mesh-controller/check_test.go

@@ -122,3 +122,40 @@ func TestASeatAnotherModuleDeclaresIsANoteOverSomeManifests(t *testing.T) {
 		t.Fatalf("the note was not said:\n%s", out.String())
 	}
 }
+
+// **A module lives whole in one repository** (novox/hq ADR 0420 §1, issue 1019): `module check`, told the
+// repository it runs for, refuses an artifact whose build context names another; run by hand, it does not know.
+func TestModuleCheckRefusesAnArtifactContextNamingAnotherRepository(t *testing.T) {
+	dir := t.TempDir()
+	write := func(name, context string) string {
+		path := filepath.Join(dir, name+".json")
+		raw := `{"module":"` + name + `","version":"1","build":{"artifacts":[{"name":"server","kind":"image","from":"Dockerfile"` + context + `}]}}`
+		if err := os.WriteFile(path, []byte(raw), 0o600); err != nil {
+			t.Fatal(err)
+		}
+		return path
+	}
+	other := write("other", `,"context":{"seat":"git","repository":"novox/mesh-controller","ref":"main"}`)
+	own := write("own", `,"context":{"seat":"git","repository":"novox/mesh-catalog","ref":"main"}`)
+	none := write("none", ``)
+
+	t.Setenv("MESH_CHECK_REPOSITORY", "novox/mesh-catalog")
+	var out bytes.Buffer
+	if err := moduleCheck([]string{other}, &out); err == nil {
+		t.Fatalf("a context naming another repository passed:\n%s", out.String())
+	}
+	if !strings.Contains(out.String(), "other builds artifact server from novox/mesh-controller: a module lives whole in one repository (novox/hq ADR 0420 §1)") {
+		t.Errorf("the refusal is not in the words of the gate:\n%s", out.String())
+	}
+	for _, path := range []string{own, none} {
+		out.Reset()
+		if err := moduleCheck([]string{path}, &out); err != nil {
+			t.Errorf("%s was refused: %v\n%s", path, err, out.String())
+		}
+	}
+	t.Setenv("MESH_CHECK_REPOSITORY", "")
+	out.Reset()
+	if err := moduleCheck([]string{other}, &out); err != nil {
+		t.Errorf("a local run, with no repository named, refused it: %v\n%s", err, out.String())
+	}
+}

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 }
@@ -742,6 +766,21 @@ func shelfWithChange(base map[string]catalogue.Manifest, sources map[string]inve
 				"of one module name (issue 239)", orRoot(dir), m.Module, s.Repository))
 			return nil
 		}
+		// A module lives whole in one repository (novox/hq ADR 0420 §1, issue 1019): no artifact builds from another.
+		if m.Build != nil {
+			own := link.SourceMoved{Owner: ownerOf(repository), Repo: repoOf(repository)}
+			other := false
+			for _, a := range m.Build.Artifacts {
+				if a.Context != nil && a.Context.Repository != "" && !sameRepository(a.Context.Repository, own) {
+					failures = append(failures, fmt.Sprintf("%s builds artifact %s from %s: a module lives whole in "+
+						"one repository (novox/hq ADR 0420 §1)", m.Module, a.Name, a.Context.Repository))
+					other = true
+				}
+			}
+			if other {
+				return nil
+			}
+		}
 		resolved, err := standInBuild(written, texts)
 		if err == nil {
 			// As registration reads a built manifest: strictly, again (takeIn).

cmd/mesh-controller/merge_gate_test.go

@@ -559,3 +559,111 @@ func TestADeliveryGroupMovingAModuleMachinesRunPassesTheGate(t *testing.T) {
 		t.Errorf("the removal of a module machines run passed alone:\n%s", v.Report())
 	}
 }
+
+// **A module lives whole in one repository** (novox/hq ADR 0420 §1, issue 1019): an artifact whose build
+// context names another repository is refused by the gate, and one naming its own passes it.
+func TestTheGateRefusesAnArtifactContextNamingAnotherRepository(t *testing.T) {
+	withContext := func(repo string) string {
+		return `{"module":"proxy","version":"1","build":{"artifacts":[{"name":"server","kind":"image","from":"Dockerfile",` +
+			`"context":{"seat":"git","repository":"` + repo + `","ref":"main"}}]}}`
+	}
+	judge := func(repo string) []string {
+		t.Helper()
+		tree := aTree(t, map[string]string{"proxy": withContext(repo)})
+		_, failures, err := shelfWithChange(map[string]catalogue.Manifest{}, map[string]inventory.Source{},
+			snapshot.Facts{}, "novox/mesh-controller", tree, map[string]string{}, nil)
+		if err != nil {
+			t.Fatal(err)
+		}
+		return failures
+	}
+	const want = "proxy builds artifact server from novox/mesh-catalog: a module lives whole in one repository (novox/hq ADR 0420 §1)"
+	failures := judge("novox/mesh-catalog")
+	if len(failures) != 1 || !strings.Contains(failures[0], want) {
+		t.Errorf("a context naming another repository: %v, want it to say %q", failures, want)
+	}
+	for _, f := range judge("novox/mesh-controller") {
+		if strings.Contains(f, "ADR 0420 §1") {
+			t.Errorf("a context naming the module's own repository was refused: %s", f)
+		}
+	}
+}
+
+// **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)
+	}
+}