Merge pull request 'An image artifact may name its own build context, apart from the module's repository' (#62) from feat/an-artifact-may-name-its-own-build-context into main

internal/builder/builder.go

@@ -177,7 +177,7 @@ func Build(ctx context.Context, run Runner, publish Publisher,
 		sort.Slice(artifacts, func(i, j int) bool { return artifacts[i].Name < artifacts[j].Name })
 		for _, a := range artifacts {
 			say("artifact", "%s (%s%s) — starting", a.Name, a.Kind, langSuffix(a))
-			made, err := one(ctx, run, publish, manifest.Module, within, commit, a, args, held, npmrcPath, say)
+			made, err := one(ctx, run, publish, manifest.Module, within, workspace, commit, a, args, held, npmrcPath, say)
 			if err != nil {
 				say("artifact", "%s FAILED: %v", a.Name, err)
 				return Result{}, err
@@ -210,6 +210,28 @@ func logging(log Log) func(step, format string, args ...any) {
 	}
 }
 
+// contextFrom clones an image artifact's own build context, when it names one apart from this
+// module's own repository — a fresh tree, the same way the module's own is, keyed by artifact
+// name so two artifacts of one module naming different contexts do not collide.
+func contextFrom(ctx context.Context, run Runner, workspace, artifact string,
+	from catalogue.ArtifactContext, say func(step, format string, args ...any)) (string, error) {
+	say("context", "cloning %s at %s for %s", from.Repository, refOrHead(from.Ref), artifact)
+	dir := filepath.Join(workspace, "context-"+artifact)
+	if err := os.RemoveAll(dir); err != nil {
+		return "", err
+	}
+	if _, err := run(ctx, workspace, "git", "clone", "--quiet", from.Repository, dir); err != nil {
+		return "", fmt.Errorf("cannot clone %s: %w", from.Repository, err)
+	}
+	if from.Ref != "" {
+		if _, err := run(ctx, dir, "git", "checkout", "--quiet", from.Ref); err != nil {
+			return "", fmt.Errorf("%s has no %s: %w", from.Repository, from.Ref, err)
+		}
+	}
+	say("context", "done")
+	return dir, nil
+}
+
 func describePath(path string) string {
 	if path == "" {
 		return ""
@@ -333,7 +355,7 @@ func wantsPackages(manifest catalogue.Manifest, within string) bool {
 }
 
 func one(ctx context.Context, run Runner, publish Publisher,
-	module, tree, commit string, a catalogue.Artifact, args []string,
+	module, tree, workspace, commit string, a catalogue.Artifact, args []string,
 	held map[string]string, npmrc string, say func(step, format string, args ...any)) (catalogue.Built, error) {
 
 	switch a.Kind {
@@ -405,7 +427,27 @@ func one(ctx context.Context, run Runner, publish Publisher,
 					"and start FROM ${<NAME>} (novox/hq ADR 0097)",
 				module, a.From, strings.Join(bases, ", "))
 		}
-		invocation := append([]string{"build", "-f", a.From, "-t", local}, args...)
+		// The recipe is always read from this module's own tree, at this module's own commit — only
+		// the context docker build's final argument names can come from somewhere else, when the
+		// artifact says so.
+		recipePath := a.From
+		buildDir := tree
+		if a.Context != nil {
+			cloned, err := contextFrom(ctx, run, workspace, a.Name, *a.Context, say)
+			if err != nil {
+				return catalogue.Built{}, fmt.Errorf("%s: %s's context: %w", module, a.Name, err)
+			}
+			// docker build accepts -f outside the context it is given; the recipe stays exactly
+			// where it was read from and validated against, absolute so the working directory
+			// switching to the cloned context does not change which file that is.
+			absRecipe, err := filepath.Abs(filepath.Join(tree, a.From))
+			if err != nil {
+				return catalogue.Built{}, fmt.Errorf("%s: %s's recipe: %w", module, a.Name, err)
+			}
+			recipePath = absRecipe
+			buildDir = cloned
+		}
+		invocation := append([]string{"build", "-f", recipePath, "-t", local}, args...)
 		if a.Target != "" {
 			invocation = append(invocation, "--target", a.Target)
 		}
@@ -417,8 +459,8 @@ func one(ctx context.Context, run Runner, publish Publisher,
 			invocation = append(invocation, "--network", "host")
 		}
 		invocation = append(invocation, ".")
-		say("image", "docker build -f %s", a.From)
+		say("image", "docker build -f %s", recipePath)
-		if _, err := run(ctx, tree, "docker", invocation...); err != nil {
+		if _, err := run(ctx, buildDir, "docker", invocation...); err != nil {
 			return catalogue.Built{}, fmt.Errorf("%s: building %s failed: %w", module, a.Name, err)
 		}
 		say("image", "built, publishing")

internal/builder/builder_test.go

@@ -19,12 +19,18 @@ import (
 
 type recorded struct {
 	ran []string
+	// dirs is the directory each entry in ran was run from, same index — so a test can ask not
+	// only what ran but where.
+	dirs     []string
 	images   map[string]string
 	archives map[string]string
 	failPush bool
 	// contents is what a clone of this repository lands, so the fake clone can restore the tree
 	// Build deliberately removes first.
 	contents map[string]string
+	// secondary is what a clone of a repository OTHER than the one under test lands, keyed by
+	// that repository's URL — an artifact's own build context, cloned apart from the module.
+	secondary map[string]map[string]string
 	// stamped is the modification time the clone gives every file. Set differently between two
 	// builds of one commit, because otherwise both land in the same second and a packer that
 	// carried timestamps would still produce one digest — which is a test that passes for a
@@ -35,13 +41,19 @@ type recorded struct {
 func (r *recorded) run(_ context.Context, dir, name string, args ...string) (string, error) {
 	line := name + " " + strings.Join(args, " ")
 	r.ran = append(r.ran, line)
+	r.dirs = append(r.dirs, dir)
 	switch {
 	case name == "git" && len(args) > 0 && args[0] == "clone":
+		repository := args[len(args)-2]
 		tree := args[len(args)-1]
 		if err := os.MkdirAll(tree, 0o755); err != nil {
 			return "", err
 		}
-		for path, body := range r.contents {
+		lands := r.contents
+		if by, is := r.secondary[repository]; is {
+			lands = by
+		}
+		for path, body := range lands {
 			full := filepath.Join(tree, path)
 			if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil {
 				return "", err
@@ -59,7 +71,6 @@ func (r *recorded) run(_ context.Context, dir, name string, args ...string) (str
 	case name == "git" && len(args) > 0 && args[0] == "rev-parse":
 		return "c0ffeec0ffeec0ffeec0ffeec0ffeec0ffeec0ff\n", nil
 	}
-	_ = dir
 	return "", nil
 }
 
@@ -331,3 +342,72 @@ func TestAPathThatLeavesTheRepositoryIsRefused(t *testing.T) {
 		}
 	}
 }
+
+// **Packaging and source are allowed to live apart** — a module that ships only the recipe for
+// source that lives in a second repository (the reference route-proxy, packaged in the catalogue
+// but built from mesh-controller's own repository) names where that source actually is, rather
+// than vendoring a second copy the two could drift from.
+func TestAnArtifactWithItsOwnContextIsBuiltFromThere(t *testing.T) {
+	const withContext = `{"module":"route-proxy","version":"1",
+	  "build":{"artifacts":[
+	    {"name":"server","kind":"image","from":"Dockerfile",
+	     "context":{"repository":"https://forge.invalid/source.git","ref":"main"}}]}}`
+	r := &recorded{
+		contents: map[string]string{
+			ManifestName: withContext,
+			// The recipe lives with the packaging, not the source — read from here regardless of
+			// where the build context comes from. FROM scratch declares no base, so what is under
+			// test — where the context comes from — is not entangled with ADR 0097's own checks.
+			"Dockerfile": "FROM scratch\nCOPY go.mod ./\n",
+		},
+		secondary: map[string]map[string]string{
+			// go.mod exists only in the second repository. A build context taken from the wrong
+			// place would never find it, which a real docker build would refuse on — the fake
+			// does not read files, so what is checked below is that the build was even pointed
+			// at the right place, not that COPY would have succeeded.
+			"https://forge.invalid/source.git": {"go.mod": "module route-proxy\n"},
+		},
+	}
+	_, err := Build(context.Background(), r.run, r,
+		"https://forge.invalid/catalogue.git", "", "", t.TempDir(), nil, Npmrc{}, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	var clonedSource bool
+	for _, line := range r.ran {
+		if strings.HasPrefix(line, "git clone") && strings.Contains(line, "https://forge.invalid/source.git") {
+			clonedSource = true
+		}
+	}
+	if !clonedSource {
+		t.Fatalf("the artifact's own context was never cloned: %v", r.ran)
+	}
+
+	buildIndex := -1
+	for i, line := range r.ran {
+		if strings.HasPrefix(line, "docker build ") {
+			buildIndex = i
+		}
+	}
+	if buildIndex == -1 {
+		t.Fatal("no docker build was run")
+	}
+	build := r.ran[buildIndex]
+	buildDir := r.dirs[buildIndex]
+
+	if !strings.Contains(buildDir, "context-server") {
+		t.Errorf("docker build ran from %q, not the artifact's own cloned context", buildDir)
+	}
+	recipe := strings.SplitN(strings.SplitN(build, "-f ", 2)[1], " ", 2)[0]
+	if !filepath.IsAbs(recipe) {
+		t.Errorf("the recipe %q is not an absolute path, so it is read relative to whatever "+
+			"directory the build context moved to rather than where it actually is", recipe)
+	}
+	if !strings.HasSuffix(recipe, string(filepath.Separator)+"Dockerfile") {
+		t.Errorf("the recipe is not the module's own Dockerfile: %q", recipe)
+	}
+	if !strings.HasSuffix(build, " .") {
+		t.Errorf("the build was not given a context: %s", build)
+	}
+}

internal/catalogue/manifest.go

@@ -447,6 +447,18 @@ type BuildsOn struct {
 	Image string `json:"image,omitempty"`
 }
 
+// ArtifactContext names the repository an image artifact's build context is cloned from, when
+// that is not this module's own repository.
+type ArtifactContext struct {
+	// Repository is cloned fresh, the same way the module's own repository is — a working tree
+	// nothing has touched, so what was built is reproducible from the two commits named rather
+	// than from whatever a previous build happened to leave behind.
+	Repository string `json:"repository"`
+	// Ref is the branch, tag or commit of that repository to build. Empty means its own default
+	// branch — the same meaning an empty module ref already has.
+	Ref string `json:"ref,omitempty"`
+}
+
 // Artifact is one thing built from a module's source.
 type Artifact struct {
 	// Name is how resources refer to it. Local to the module.
@@ -466,6 +478,17 @@ type Artifact struct {
 	// Empty means the whole recipe, which is what a module with one image says by saying nothing.
 	Target string `json:"target,omitempty"`
 
+	// Context names a second repository this image's build reaches into for its own source — the
+	// recipe itself is still read from this module's own directory, at this module's own commit;
+	// only the build context `docker build`'s final argument names comes from here instead.
+	//
+	// **Packaging and source are allowed to live apart.** A module that only ships the recipe for
+	// source that lives elsewhere — the reference route-proxy in mesh-controller's own repository,
+	// packaged as a module in the catalogue rather than vendored a second time the two copies
+	// could drift from — names where that source actually is. Empty means the ordinary case: an
+	// image built from this same module's own repository, the same as every other artifact.
+	Context *ArtifactContext `json:"context,omitempty"`
+
 	// Language is what this module's code is written in, for a bundle.
 	//
 	// **Declared, never guessed.** Inferring it from what files happen to be present makes a