From 20e57c3f51185f442da7c97c9eaf31fca35eff0d Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 25 Sep 2026 17:39:27 +0200 Subject: [PATCH] an image artifact may name its own build context, apart from the module's repository MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit route-proxy's own Dockerfile documents the shape it has always needed and never had: 'the proxy source is not vendored here... the build context is the mesh-controller repository root, and this Dockerfile compiles ./examples/route-proxy from it.' Nothing in the mesh could do that — the build command clones one repository and builds every artifact from within it, so route-proxy has never once been built through the pipeline, consistent with it never having been assigned anywhere. Found attempting exactly that build tonight: 'stat go.mod: file does not exist', because the context was mesh-catalog, which does not have one. An image artifact may now carry a context: {repository, ref}, cloned fresh alongside the module's own tree. The recipe (Dockerfile) is still read from the module's own directory, at the module's own commit — only docker build's own context argument moves. Packaging and source stay exactly as separate as route-proxy's own comment already said they were, now for real. --- internal/builder/builder.go | 52 +++++++++++++++++-- internal/builder/builder_test.go | 86 ++++++++++++++++++++++++++++++-- internal/catalogue/manifest.go | 23 +++++++++ 3 files changed, 153 insertions(+), 8 deletions(-) diff --git a/internal/builder/builder.go b/internal/builder/builder.go index f78ef31..222878d 100644 --- a/internal/builder/builder.go +++ b/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 ${} (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) - if _, err := run(ctx, tree, "docker", invocation...); err != nil { + say("image", "docker build -f %s", recipePath) + 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") diff --git a/internal/builder/builder_test.go b/internal/builder/builder_test.go index c04ba87..175295e 100644 --- a/internal/builder/builder_test.go +++ b/internal/builder/builder_test.go @@ -18,13 +18,19 @@ import ( // tree, that two builds of one commit produce one digest. Running docker here would test docker. type recorded struct { - ran []string + 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) + } +} diff --git a/internal/catalogue/manifest.go b/internal/catalogue/manifest.go index 6e42dd9..7ee50bd 100644 --- a/internal/catalogue/manifest.go +++ b/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 -- 2.54.0