From 6f6e1244d457a7a6f589c56d6c19255ba55142a5 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 21 Sep 2026 20:45:36 +0200 Subject: [PATCH] A build declares the vendor image it stands on, and a recipe fetches nothing undeclared MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit build.on takes {arg, image@sha256:…} beside {arg, module, artifact}: the image is copied into the mesh's registry before the build (ADR 0096) and the recipe reads the copy from the argument. A FROM or COPY --from naming a registry image the manifest did not declare is refused before the build, naming it and the remedy; stages, declared arguments and scratch are not fetches (novox/hq 04-ISSUES/064, ADR 0097). --- internal/builder/builder.go | 129 ++++++++++++++++++++++++++- internal/builder/standing_on_test.go | 69 +++++++++++++- internal/catalogue/manifest.go | 12 ++- 3 files changed, 201 insertions(+), 9 deletions(-) diff --git a/internal/builder/builder.go b/internal/builder/builder.go index 8dc0dcc..640fbf3 100644 --- a/internal/builder/builder.go +++ b/internal/builder/builder.go @@ -14,6 +14,7 @@ import ( "path/filepath" "regexp" "sort" + "strconv" "strings" "time" @@ -149,7 +150,20 @@ func Build(ctx context.Context, run Runner, publish Publisher, // What this module said it stands on, answered with what this mesh actually holds. Done // before anything is built, so a missing base is refused in front of the person who can // fix it rather than inside a build that stops on its own first line. - args, err := standingOn(manifest, held) + // An image published elsewhere that the build stands on is copied into the mesh's own + // registry first, like an upstream artifact (ADR 0096), and the recipe is handed the copy. + // Genesis has nowhere to copy to and pulls it into this machine's store instead. + mirror := func(ctx context.Context, from, repository string) (string, error) { + if m, can := publish.(Mirrorer); can { + say("bases", "copying %s into the mesh's registry", from) + return m.MirrorImage(ctx, from, repository) + } + if _, err := run(ctx, tree, "docker", "pull", from); err != nil { + return "", fmt.Errorf("cannot fetch %s: %w", from, err) + } + return from, nil + } + args, err := standingOn(ctx, manifest, held, mirror) if err != nil { say("bases", "UNMET: %v", err) return Result{}, err @@ -358,6 +372,29 @@ func one(ctx context.Context, run Runner, publish Publisher, local := fmt.Sprintf("%s-%s:%s", module, a.Name, short(commit)) // The bases this module named, resolved to what this mesh holds. A recipe reads them as // build arguments, so a module says which module it stands on and never which copy. + // **A recipe fetches nothing the manifest did not declare** (novox/hq 04-ISSUES/064). A FROM + // or a COPY --from naming a registry image that is not a declared base is a build that + // reaches a public registry on its own — and works when that registry answers, which is + // sometimes. Refused here, in front of the person who can declare it, not inside a build + // that fails with "pull access denied" for a reason that is not the mesh's. + recipe, err := os.ReadFile(filepath.Join(tree, a.From)) + if err != nil { + return catalogue.Built{}, fmt.Errorf("%s: cannot read the recipe %s: %w", module, a.From, err) + } + declared := map[string]bool{} + for i := 0; i+1 < len(args); i += 2 { + if args[i] == "--build-arg" { + declared[strings.SplitN(args[i+1], "=", 2)[0]] = true + } + } + if fetches := undeclaredFetches(string(recipe), declared); len(fetches) > 0 { + return catalogue.Built{}, fmt.Errorf( + "%s: the recipe %s fetches %s, which the manifest does not declare. A build "+ + "reaching a public registry on its own works only when that registry answers; "+ + "declare it under build.on as {\"arg\": \"\", \"image\": \"@sha256:…\"} "+ + "and read it from that argument (novox/hq ADR 0097)", + module, a.From, strings.Join(fetches, ", ")) + } invocation := append([]string{"build", "-f", a.From, "-t", local}, args...) if a.Target != "" { invocation = append(invocation, "--target", a.Target) @@ -578,7 +615,8 @@ var _ io.Writer = (*stringWriter)(nil) // one a container runtime produces when a recipe's first line refers to an image nobody has. // // The order is fixed so two builds of one commit invoke the same command. -func standingOn(manifest catalogue.Manifest, held map[string]string) ([]string, error) { +func standingOn(ctx context.Context, manifest catalogue.Manifest, held map[string]string, + mirror func(ctx context.Context, from, repository string) (string, error)) ([]string, error) { if manifest.Build == nil || len(manifest.Build.On) == 0 { return nil, nil } @@ -587,6 +625,27 @@ func standingOn(manifest catalogue.Manifest, held map[string]string) ([]string, var args []string for _, base := range on { + if base.Image != "" { + // A vendor's image, declared (novox/hq 04-ISSUES/064, ADR 0097). Pinned, because a tag + // is what somebody else can move; copied into the mesh's registry, because a build + // that reaches a public registry on its own is a build that works sometimes. + if base.Arg == "" || base.Module != "" || base.Artifact != "" { + return nil, fmt.Errorf( + "%s stands on the image %s, and a base is either a module's artifact or an "+ + "image — never both — read from one build argument", manifest.Module, base.Image) + } + if !strings.Contains(base.Image, "@sha256:") { + return nil, fmt.Errorf( + "%s stands on the image %q, which is not pinned by digest. A tag is what "+ + "somebody else can move; name it as @sha256:…", manifest.Module, base.Image) + } + reference, err := mirror(ctx, base.Image, manifest.Module+"/on-"+strings.ToLower(base.Arg)) + if err != nil { + return nil, fmt.Errorf("%s stands on %s: %w", manifest.Module, base.Image, err) + } + args = append(args, "--build-arg", base.Arg+"="+reference) + continue + } if base.Arg == "" || base.Module == "" || base.Artifact == "" { return nil, fmt.Errorf( "%s says its build stands on something, and does not say all of what: a base "+ @@ -727,3 +786,69 @@ func sourcesFor(entrypoints []string, out string) []string { func timeNow() time.Time { return time.Now() } func since(t time.Time) string { return time.Since(t).Round(time.Millisecond).String() } + +// undeclaredFetches is every image a recipe reaches for that is neither a declared build argument +// nor one of its own stages nor `scratch`: a `FROM` or a `COPY --from` naming somebody else's +// registry directly. +func undeclaredFetches(recipe string, declared map[string]bool) []string { + stages := map[string]bool{} + var out []string + seen := map[string]bool{} + note := func(ref string) { + ref = strings.TrimSpace(ref) + switch { + case ref == "" || ref == "scratch" || stages[strings.ToLower(ref)]: + return + case strings.HasPrefix(ref, "$"): + name := strings.Trim(strings.TrimPrefix(ref, "$"), "{}") + if cut := strings.IndexAny(name, ":-"); cut >= 0 { + name = name[:cut] + } + if !declared[name] { + if !seen[ref] { + seen[ref] = true + out = append(out, ref+" (a build argument the manifest does not declare)") + } + } + return + } + // A stage referenced by number (COPY --from=0) is its own recipe's. + if _, err := strconv.Atoi(ref); err == nil { + return + } + if !seen[ref] { + seen[ref] = true + out = append(out, ref) + } + } + for _, raw := range strings.Split(recipe, "\n") { + line := strings.TrimSpace(raw) + if line == "" || strings.HasPrefix(line, "#") { + continue + } + fields := strings.Fields(line) + switch strings.ToUpper(fields[0]) { + case "FROM": + // FROM [--platform=…] [AS ] + var ref string + for i := 1; i < len(fields); i++ { + if strings.HasPrefix(fields[i], "--") { + continue + } + ref = fields[i] + if i+2 < len(fields) && strings.EqualFold(fields[i+1], "AS") { + stages[strings.ToLower(fields[i+2])] = true + } + break + } + note(ref) + case "COPY", "ADD": + for _, f := range fields[1:] { + if strings.HasPrefix(f, "--from=") { + note(strings.TrimPrefix(f, "--from=")) + } + } + } + } + return out +} diff --git a/internal/builder/standing_on_test.go b/internal/builder/standing_on_test.go index 5cee3cd..d3622f9 100644 --- a/internal/builder/standing_on_test.go +++ b/internal/builder/standing_on_test.go @@ -1,6 +1,8 @@ package builder import ( + "context" + "fmt" "strings" "testing" @@ -20,7 +22,7 @@ func TestABaseTheMeshHasNotBuiltIsRefused(t *testing.T) { On: []catalogue.BuildsOn{{Arg: "RUNTIME_BASE", Module: "mesh-tools", Artifact: "runtime"}}, }, } - _, err := standingOn(manifest, map[string]string{}) + _, err := standingOn(context.Background(), manifest, map[string]string{}, noMirror) if err == nil { t.Fatal("a base nothing has built was accepted; the build would have failed on its first line") } @@ -40,7 +42,7 @@ func TestABaseTheMeshHoldsBecomesABuildArgument(t *testing.T) { }, } held := map[string]string{"mesh-tools/runtime": "127.0.0.1:5000/mesh-tools/runtime@sha256:" + strings.Repeat("a", 64)} - args, err := standingOn(manifest, held) + args, err := standingOn(context.Background(), manifest, held, noMirror) if err != nil { t.Fatalf("a base this mesh holds was refused: %v", err) } @@ -52,7 +54,7 @@ func TestABaseTheMeshHoldsBecomesABuildArgument(t *testing.T) { // A module naming no base asks for nothing, which is most modules. func TestAModuleNamingNoBaseAddsNoArguments(t *testing.T) { - args, err := standingOn(catalogue.Manifest{Module: "hello-web", Build: &catalogue.Build{}}, nil) + args, err := standingOn(context.Background(), catalogue.Manifest{Module: "hello-web", Build: &catalogue.Build{}}, nil, noMirror) if err != nil || args != nil { t.Fatalf("a module naming no base produced %v, %v", args, err) } @@ -64,7 +66,66 @@ func TestAnIncompleteBaseIsRefused(t *testing.T) { Module: "postgres", Build: &catalogue.Build{On: []catalogue.BuildsOn{{Module: "mesh-tools", Artifact: "runtime"}}}, } - if _, err := standingOn(manifest, map[string]string{"mesh-tools/runtime": "x"}); err == nil { + if _, err := standingOn(context.Background(), manifest, map[string]string{"mesh-tools/runtime": "x"}, noMirror); err == nil { t.Fatal("a base with no build argument was accepted; nothing would have read it") } } + +// noMirror is a mirror for tests whose bases are all the mesh's own. +func noMirror(context.Context, string, string) (string, error) { + return "", fmt.Errorf("nothing to copy in this test") +} + +// A build may stand on an image published elsewhere, declared and pinned (novox/hq 04-ISSUES/064, +// ADR 0097): it is copied into the mesh's registry first and the recipe is handed the copy. +func TestADeclaredVendorImageIsCopiedInAndHandedToTheRecipe(t *testing.T) { + manifest := catalogue.Manifest{ + Module: "minio", + Build: &catalogue.Build{ + On: []catalogue.BuildsOn{{Arg: "MC_BASE", Image: "quay.io/minio/mc@sha256:" + strings.Repeat("c", 64)}}, + }, + } + var asked []string + args, err := standingOn(context.Background(), manifest, nil, func(_ context.Context, from, repository string) (string, error) { + asked = append(asked, from+" -> "+repository) + return "127.0.0.1:5000/" + repository + "@sha256:" + strings.Repeat("d", 64), nil + }) + if err != nil { + t.Fatal(err) + } + if len(asked) != 1 || asked[0] != "quay.io/minio/mc@sha256:"+strings.Repeat("c", 64)+" -> minio/on-mc_base" { + t.Fatalf("the image was not copied under the module's repository: %v", asked) + } + if strings.Join(args, " ") != "--build-arg MC_BASE=127.0.0.1:5000/minio/on-mc_base@sha256:"+strings.Repeat("d", 64) { + t.Fatalf("the recipe was not handed the copy: %v", args) + } + // Unpinned, it is refused: a tag is what somebody else can move. + manifest.Build.On[0].Image = "quay.io/minio/mc:latest" + if _, err := standingOn(context.Background(), manifest, nil, noMirror); err == nil || !strings.Contains(err.Error(), "not pinned") { + t.Fatalf("an unpinned vendor image was accepted: %v", err) + } +} + +// A recipe reaching for an image the manifest did not declare is named, and its own stages, +// declared arguments and scratch are not. +func TestARecipeFetchingWhatTheManifestDidNotDeclareIsNamed(t *testing.T) { + recipe := ` +ARG RUNTIME_BASE +ARG MC_BASE +FROM ${RUNTIME_BASE} AS build +COPY --from=${MC_BASE} /usr/bin/mc /usr/local/bin/mc +COPY --from=build /out /out +COPY --from=0 /x /x +FROM scratch +COPY --from=vendor/tool:latest /tool /tool +FROM golang:1.25-alpine AS go +` + got := undeclaredFetches(recipe, map[string]bool{"RUNTIME_BASE": true}) + want := []string{"${MC_BASE} (a build argument the manifest does not declare)", "vendor/tool:latest", "golang:1.25-alpine"} + if strings.Join(got, "|") != strings.Join(want, "|") { + t.Fatalf("got %v, want %v", got, want) + } + if got := undeclaredFetches(recipe, map[string]bool{"RUNTIME_BASE": true, "MC_BASE": true}); len(got) != 2 { + t.Fatalf("declared arguments are not fetches: %v", got) + } +} diff --git a/internal/catalogue/manifest.go b/internal/catalogue/manifest.go index 01aaea1..fbf92c6 100644 --- a/internal/catalogue/manifest.go +++ b/internal/catalogue/manifest.go @@ -411,14 +411,20 @@ type Build struct { On []BuildsOn `json:"on,omitempty"` } -// BuildsOn is one base a build needs, and the name the recipe knows it by. +// BuildsOn is one base a build needs, and the name the recipe knows it by: another module's +// artifact, or an image published elsewhere. type BuildsOn struct { // Arg is the build argument the recipe reads it from. Arg string `json:"arg"` // Module is whose artifact it is. - Module string `json:"module"` + Module string `json:"module,omitempty"` // Artifact is which of that module's artifacts, by its own name for it. - Artifact string `json:"artifact"` + Artifact string `json:"artifact,omitempty"` + // Image is an image published elsewhere, pinned by digest, that the build copies out of — a + // vendor's tool, a base nobody in the mesh builds. Declared, the mesh copies it into its own + // registry before the build and hands the recipe the copy (novox/hq 04-ISSUES/064, ADR 0097); + // a recipe fetching from a public registry on its own is refused. + Image string `json:"image,omitempty"` } // Artifact is one thing built from a module's source.