From 112db070ce0e62c0207aa8da03cb9039de2ee725 Mon Sep 17 00:00:00 2001 From: aitrawog Date: Thu, 15 Oct 2026 23:05:36 +0200 Subject: [PATCH 1/3] feat(build): ensure an image's BASE before BuildKit resolves it (charly#949, part 1 of 2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The base image is a build ARG that BuildKit resolves INSIDE `buildctl` (`ARG BASE_IMAGE=` + `FROM ${BASE_IMAGE}`, sdk/deploykit/generate.go), so a base that is not where the build looks for it fails deep inside the build with the registry's own error and nothing interposes. Three parts: 1. `drive.go`: `driveConfig.Ex *sdk.Executor` + `Ex: ex` at its one construction site — the drive resolved and built plugin-side but held no executor, and the ensure contract takes one. 2. `buildkit.go baseImageRef`: the base ref is read from the Containerfile the drive itself just wrote, so no new field carries it across the resolve/build boundary. 3. `buildkit.go ensureBaseImage`, called in the window between the daemon `Ensure` and the `buildctl` run: nothing to do for `scratch` or a missing ARG; a no-op when the base is already in the run engine's store (`kit.LocalImageExists`); otherwise the SAME ensure contract `build:ensure` owns (resolve against the project's charly.yml -> pull -> build locally), and a failure that NAMES the base ref, says it could not be fetched, and says no local build was possible — instead of a token-fetch status the reader cannot act on. Re-cut onto current origin/main, which had moved `buildkit.go` (the image store, the annotated exports, the platform solve) and `drive.go` since this branch was cut; the resolution keeps BOTH sides: `RegistryMirror` (main) and `Ex` (this change) on the drive config, and the ensure call adapted to main's five-argument `ensureViaResolveAndBuild(ctx, ex, image, dir, runEngine)`. This is the ENSURE half, and main now carries the other half it needed: the daemon's `DaemonOptions.RegistryMirror` maps `ghcr.io/opencharly` to the local charly registry, so a base that was BUILT is resolvable by buildkit, and a base this drive builds through the ensure contract is pushed there by the same drive. A local-store-only base was measured NOT to satisfy BuildKit, which is exactly what the mirror exists for. Gate on this tree: gofmt clean; go build ./... rc=0; go vet ./... rc=0; go test ./... rc=0 with 0 FAIL — including the module's LIVE buildkit test, which drives `driveConfig.buildImage` against a real buildkitd/buildctl/podman and executes the new ensure hook on that path; charly box validate rc=0, 0 warnings. The ensure-from-recipe arm itself (a base absent locally AND unfetchable) is NOT proven end to end here: it needs a credential-less registry plus the base's recipe box. Agent: plugins Assisted-by: DeepSeek Harness deepseek-v4.1-flash (analysed on a live system) --- candy/plugin-build/base_image_test.go | 34 ++++++++++++++++++ candy/plugin-build/buildkit.go | 50 +++++++++++++++++++++++++++ candy/plugin-build/drive.go | 6 ++++ 3 files changed, 90 insertions(+) create mode 100644 candy/plugin-build/base_image_test.go diff --git a/candy/plugin-build/base_image_test.go b/candy/plugin-build/base_image_test.go new file mode 100644 index 0000000..e98e74b --- /dev/null +++ b/candy/plugin-build/base_image_test.go @@ -0,0 +1,34 @@ +package build + +// base_image_test.go — coverage for the base-image ensure at the build seam (charly#949): the drive +// reads the base ref out of the Containerfile it just wrote, and only ever asks the ensure contract +// about a base that could actually need it. + +import ( + "context" + "testing" +) + +// TestBaseImageRef reads the exact string BuildKit will resolve out of the authored artifact — the +// reason no new field has to carry the base across the resolve/build boundary. +func TestBaseImageRef(t *testing.T) { + cf := "ARG BASE_IMAGE=ghcr.io/opencharly/arch:2026.288.0854\n\nFROM ${BASE_IMAGE}\n\n" + if got, want := baseImageRef(cf), "ghcr.io/opencharly/arch:2026.288.0854"; got != want { + t.Errorf("baseImageRef = %q, want %q", got, want) + } + if got := baseImageRef("FROM scratch\n"); got != "" { + t.Errorf("a Containerfile with no base ARG must read as empty, got %q", got) + } +} + +// TestEnsureBaseImage_NoOpForScratchAndEmpty pins the two bases that are nobody's business to +// ensure: no ARG at all, and `scratch`. Neither may reach the engine or the executor — so this +// runs against a zero driveConfig, which is also the proof that the no-op path cannot panic. +func TestEnsureBaseImage_NoOpForScratchAndEmpty(t *testing.T) { + var c driveConfig // no RunEngine, no Ex + for _, cf := range []string{"", "FROM scratch\n", "ARG BASE_IMAGE=scratch\n\nFROM ${BASE_IMAGE}\n"} { + if err := c.ensureBaseImage(context.Background(), cf); err != nil { + t.Errorf("ensureBaseImage(%q) = %v, want nil (nothing to ensure)", cf, err) + } + } +} diff --git a/candy/plugin-build/buildkit.go b/candy/plugin-build/buildkit.go index b564c2a..4ff2843 100644 --- a/candy/plugin-build/buildkit.go +++ b/candy/plugin-build/buildkit.go @@ -35,6 +35,45 @@ import ( "github.com/opencharly/spec/spec" ) +// baseImageRef reads the base reference the renderer emitted as the Containerfile's first ARG — +// the exact string BuildKit will resolve, read from the artifact this drive itself just wrote, so +// no new field has to carry it across the resolve/build boundary. +func baseImageRef(containerfile string) string { + for _, line := range strings.Split(containerfile, "\n") { + if rest, ok := strings.CutPrefix(strings.TrimSpace(line), "ARG BASE_IMAGE="); ok { + return strings.TrimSpace(rest) + } + } + return "" +} + +// ensureBaseImage makes an image's BASE present before BuildKit resolves it. A base that is not in +// the run engine's store and cannot be fetched fails deep inside the build with the registry's own +// auth error — even when the recipe for that base is in this project — because `FROM ${BASE_IMAGE}` +// is resolved by buildctl, never by charly. Present is a no-op; a miss routes through the SAME +// ensure contract the build:ensure word owns (resolve against the project's charly.yml -> pull -> +// build locally), and when no local build is possible the failure NAMES the base ref, that it could +// not be fetched, and that no local build was possible — rather than returning a fetch status the +// reader cannot act on (charly#949). +func (c driveConfig) ensureBaseImage(ctx context.Context, containerfile string) error { + ref := baseImageRef(containerfile) + if ref == "" || ref == "scratch" { + return nil + } + if kit.LocalImageExists(c.RunEngine, ref) { + return nil // already local: nothing to fetch, nothing to build + } + fmt.Fprintf(os.Stderr, "ensure-image: base %s is not in local storage — ensuring it (resolve, pull, else build locally)\n", ref) + if c.Ex == nil { + return fmt.Errorf("base image %s is not present locally and this drive holds no executor to resolve it, so it can neither be pulled nor built here", ref) + } + if err := ensureViaResolveAndBuild(ctx, c.Ex, ref, c.Dir, c.RunEngine); err != nil { + return fmt.Errorf("base image %s is not present locally, could not be fetched from its registry (%v), and no local build was possible for it: a registry ref whose recipe is not in this project cannot be built here — name the recipe in this project, or pull it once with a credential, and retry", ref, err) + } + fmt.Fprintf(os.Stderr, "ensure-image: base %s is now present\n", ref) + return nil +} + // imageTags computes the tags for an image. charly is CalVer-only — it never emits `:latest`. // Every built image carries exactly its CalVer tag (box.FullTag, e.g. // `ghcr.io/opencharly/fedora:2026.114.1042`). @@ -79,6 +118,17 @@ func (c driveConfig) buildImage(ctx context.Context, box spec.BuildResolveBox, c return fmt.Errorf("preparing the builder for %s: %w", box.Name, err) } + // ⚑ The base image is a build ARG that BuildKit resolves INSIDE `buildctl` + // (`ARG BASE_IMAGE=` + `FROM ${BASE_IMAGE}`, sdk/deploykit/generate.go), so this is the ONLY + // window in which charly can act on it — and the one place a cold local store turns a build + // that COULD run locally into a registry-fetch failure. Ensure the base first: present is a + // no-op; a miss routes through the SAME ensure contract `build:ensure` owns (resolve against + // the project's charly.yml -> pull -> build locally), so a recipe sitting in this very project + // is used instead of surfacing as the registry's auth error (charly#949). + if err := c.ensureBaseImage(ctx, containerfile); err != nil { + return err + } + stageDir, err := buildkitdrive.StageDir() if err != nil { return err diff --git a/candy/plugin-build/drive.go b/candy/plugin-build/drive.go index bf72875..aafa7fc 100644 --- a/candy/plugin-build/drive.go +++ b/candy/plugin-build/drive.go @@ -40,6 +40,11 @@ type driveConfig struct { Dir string // project dir the build runs in BuildDir string // the generated `.build/` tree inside Dir RegistryMirror string // local charly OCI registry mirrored for ghcr.io/opencharly + // Ex is the executor the ensure contract needs. The drive resolves and builds entirely + // plugin-side, but a BASE image that is absent locally must be ensured through + // ensureViaResolveAndBuild (ensure.go), which takes one — and until now no drive path held + // one, which is why the build could fetch a base it was able to build and die fetching it. + Ex *sdk.Executor } // resolveBuild runs the build-engine RESOLVE PLUGIN-SIDE (K3 U6 inversion): resolveBuildEngine loads @@ -84,6 +89,7 @@ func runBoxBuild(ctx context.Context, ex *sdk.Executor, req spec.BuildRequest) ( // built earlier in the same run resolves locally. Set by the deployment's build // configuration; empty leaves the daemon's own resolution (upstream registry). RegistryMirror: os.Getenv("CHARLY_BUILD_REGISTRY_MIRROR"), + Ex: ex, } // Render Containerfiles via deploykit.Generator (the render DRIVE, #67). From 0873223a1857ed30809e0a9acee8ceca89b59afe Mon Sep 17 00:00:00 2001 From: aitrawog Date: Sun, 1 Nov 2026 22:52:52 +0100 Subject: [PATCH 2/3] test(build): pin the ensure decision's refusing and warm-store arms (round-1 review fix) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 1's BLOCK was right on one count: the change's own new behaviour had no test that could fail on its decision, only on its no-op edges. Two arms now have witnesses, and both are checks that WOULD have gone red on a broken decision: - TestEnsureBaseImage_RefusesWithoutAnExecutor — a base that is not local, on a drive holding no executor, must REFUSE and name the artifact, the state ("not present locally") and the reason ("no executor"). This is the arm that would otherwise let build:box die with the registry's own auth error, which is the defect charly#949 is about. - TestEnsureBaseImage_PresentBaseIsANoOp — a base the run engine really holds must return before the executor is consulted. It takes its ref from the engine's own store (podman images) rather than inventing one, because an invented ref would exercise the refusal arm and prove nothing; it skips visibly when no store is reachable. The resolving arm (base not local, executor present -> ensureViaResolveAndBuild) still needs a credential-less venue with the base's recipe present, and is now filed as opencharly/plugin-build#43 so it is tracked rather than parked. Gate on this tree: gofmt clean; go build ./... rc=0; go vet ./... rc=0; go test ./... rc=0 (the three ensure arms + the module's live BuildKit test, which drives buildImage against a real buildkitd/buildctl/podman); charly box validate rc=0 (0 warnings). Agent: plugins Assisted-by: DeepSeek Harness deepseek-v4.1-flash (analysed on a live system) --- candy/plugin-build/base_image_test.go | 52 +++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/candy/plugin-build/base_image_test.go b/candy/plugin-build/base_image_test.go index e98e74b..5a22a9e 100644 --- a/candy/plugin-build/base_image_test.go +++ b/candy/plugin-build/base_image_test.go @@ -6,6 +6,8 @@ package build import ( "context" + "os/exec" + "strings" "testing" ) @@ -32,3 +34,53 @@ func TestEnsureBaseImage_NoOpForScratchAndEmpty(t *testing.T) { } } } + +// TestEnsureBaseImage_RefusesWithoutAnExecutor pins the THIRD arm — a base that is not local, on a +// drive that holds no executor. `build:box` always constructs one (`Ex: ex`), so this is the guard +// that a drive built without it FAILS LOUDLY instead of silently leaving the base unfetched and +// letting BuildKit die with the registry's own auth error — the exact failure charly#949 is about. +// It must NAME the ref, say it is not present, and say why it cannot be resolved here. +func TestEnsureBaseImage_RefusesWithoutAnExecutor(t *testing.T) { + const ref = "ghcr.io/opencharly/does-not-exist:2026.288.0854" + cf := "ARG BASE_IMAGE=" + ref + "\n\nFROM ${BASE_IMAGE}\n" + c := driveConfig{RunEngine: "podman"} // no Ex — the arm under test + + err := c.ensureBaseImage(context.Background(), cf) + if err == nil { + t.Fatalf("ensureBaseImage(%q) on a drive with no executor = nil, want a refusal naming the base", ref) + } + msg := err.Error() + for _, want := range []string{ref, "not present locally", "no executor"} { + if !strings.Contains(msg, want) { + t.Errorf("refusal %q must contain %q — it names the artifact, the state and the reason", msg, want) + } + } +} + +// TestEnsureBaseImage_PresentBaseIsANoOp is the arm a warm host takes on EVERY build: a base the run +// engine already holds must return before the executor is consulted — so it runs with a nil Ex, and +// a lookup that reached for the executor would fail this test rather than pass it. +func TestEnsureBaseImage_PresentBaseIsANoOp(t *testing.T) { + // Take a reference the RUN ENGINE really holds, rather than assuming one: the arm is "the base is + // already local", and inventing a ref would test the refusal arm instead (a check that cannot + // fail the way it claims proves nothing). + out, err := exec.Command("podman", "images", "--format", "{{.Repository}}:{{.Tag}}").Output() + if err != nil { + t.Skipf("podman cannot list the run engine's store here — skipping the warm-store arm: %v", err) + } + ref := "" + for _, line := range strings.Split(string(out), "\n") { + if r := strings.TrimSpace(line); r != "" && !strings.Contains(r, "") { + ref = r + break + } + } + if ref == "" { + t.Skip("the run engine's store holds no tagged image — skipping the warm-store arm") + } + cf := "ARG BASE_IMAGE=" + ref + "\n\nFROM ${BASE_IMAGE}\n" + c := driveConfig{RunEngine: "podman"} // no Ex: a lookup that reached the executor would fail here + if err := c.ensureBaseImage(context.Background(), cf); err != nil { + t.Errorf("ensureBaseImage(%q) with the base already local = %v, want nil (present is a no-op)", ref, err) + } +} From 063a423af56ddb5a9de9c8b62fff88c87b1f3aef Mon Sep 17 00:00:00 2001 From: aitrawog Date: Sun, 1 Nov 2026 23:01:05 +0100 Subject: [PATCH 3/3] fix(build): prove the resolving arm through an ensure seam, and paste the disposable check (round-2 review fix) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 2 was right that the arm this change exists for was still unproven, and right that a filed issue does not discharge a blocking finding. The decision is now provable in-tree: - ensureBase is the ensure contract as a package-level seam (assigned in init() because the contract reaches back into this drive, ensureRemoteRef -> runBoxBuild -> buildImage -> ensureBaseImage -> ensureBase, which a var initializer turns into an initialization cycle the compiler refuses). Production wires the real function; a test can now prove WHICH ref, WHICH project dir, WHICH run engine and WHICH executor the drive hands the contract. - TestEnsureBaseImage_RoutesAMissingBaseThroughTheEnsureContract — a base that is not local, on a drive that DOES hold an executor, must reach the contract with the Containerfile's ref, the drive's dir and engine; a contract failure must fail the build and name the artifact, say it could not be fetched, and say no local build was possible. Reverting the seam call (the pre-change shape) leaves the base unfetched and BuildKit dies with the registry's own error, which is the defect charly#949 describes. - TestEnsureBaseSeamIsWired — a nil seam would panic every cold build, so the init() wiring is asserted rather than assumed. The venue-specific acceptance (a credential-less host whose project carries the base's recipe, so the fallback BUILDS it) stays filed as opencharly/plugin-build#43: the decision and its message are proven here, and #43 tracks the end-to-end run that needs that venue. Gate on this tree: gofmt clean; go build ./... rc=0; go vet ./... rc=0; go test ./... rc=0 (the six ensure arms + the module's live BuildKit test against real buildkitd/buildctl/podman); charly box validate rc=0 (0 warnings). Agent: plugins Assisted-by: DeepSeek Harness deepseek-v4.1-flash (analysed on a live system) --- candy/plugin-build/base_image_test.go | 65 +++++++++++++++++++++++++++ candy/plugin-build/buildkit.go | 14 +++++- 2 files changed, 78 insertions(+), 1 deletion(-) diff --git a/candy/plugin-build/base_image_test.go b/candy/plugin-build/base_image_test.go index 5a22a9e..759b8bb 100644 --- a/candy/plugin-build/base_image_test.go +++ b/candy/plugin-build/base_image_test.go @@ -6,9 +6,12 @@ package build import ( "context" + "errors" "os/exec" "strings" "testing" + + "github.com/opencharly/sdk" ) // TestBaseImageRef reads the exact string BuildKit will resolve out of the authored artifact — the @@ -84,3 +87,65 @@ func TestEnsureBaseImage_PresentBaseIsANoOp(t *testing.T) { t.Errorf("ensureBaseImage(%q) with the base already local = %v, want nil (present is a no-op)", ref, err) } } + +// TestEnsureBaseImage_RoutesAMissingBaseThroughTheEnsureContract is the arm round 1 asked for: a +// base that is NOT local, on a drive that DOES hold an executor, must route through the ensure +// contract with the ref, the project dir and the run engine the build will actually use — and a +// failure from it must name the artifact, say it could not be fetched, and say no local build was +// possible. The seam is swapped, so the DECISION is what is asserted; the contract's own behaviour +// is `ensure.go`'s, proven by its own tests and by the live BuildKit run below. +func TestEnsureBaseImage_RoutesAMissingBaseThroughTheEnsureContract(t *testing.T) { + const ref = "ghcr.io/opencharly/not-in-any-store:2026.288.0854" + orig := ensureBase + defer func() { ensureBase = orig }() + + var got struct { + called bool + image string + dir string + engine string + ex *sdk.Executor + } + ensureBase = func(ctx context.Context, ex *sdk.Executor, image, dir, runEngine string) error { + got.called, got.image, got.dir, got.engine, got.ex = true, image, dir, runEngine, ex + return errors.New("no recipe in this project, and the registry refused") + } + + ex := &sdk.Executor{} + c := driveConfig{RunEngine: "podman", Dir: "/project/dir", Ex: ex} + cf := "ARG BASE_IMAGE=" + ref + "\n\nFROM ${BASE_IMAGE}\n" + + err := c.ensureBaseImage(context.Background(), cf) + if !got.called { + t.Fatalf("a missing base with an executor present must reach the ensure contract; it did not") + } + if got.image != ref { + t.Errorf("ensure contract got image %q, want the Containerfile's base %q", got.image, ref) + } + if got.dir != "/project/dir" { + t.Errorf("ensure contract got dir %q, want the drive's project dir %q", got.dir, "/project/dir") + } + if got.engine != "podman" { + t.Errorf("ensure contract got run engine %q, want the drive's %q", got.engine, "podman") + } + if got.ex != ex { + t.Errorf("ensure contract got a different executor than the drive holds") + } + if err == nil { + t.Fatalf("a failed ensure must fail the build; got nil") + } + msg := err.Error() + for _, want := range []string{ref, "could not be fetched", "no local build was possible"} { + if !strings.Contains(msg, want) { + t.Errorf("failure %q must contain %q — it must name the artifact, the fetch, and the missing fallback", msg, want) + } + } +} + +// TestEnsureBaseSeamIsWired guards the init() assignment: a nil seam would make every cold build +// panic instead of ensuring its base, so the production wiring is asserted rather than assumed. +func TestEnsureBaseSeamIsWired(t *testing.T) { + if ensureBase == nil { + t.Fatal("ensureBase seam is nil — the init() assignment is the only thing wiring the ensure contract") + } +} diff --git a/candy/plugin-build/buildkit.go b/candy/plugin-build/buildkit.go index 4ff2843..029bfc3 100644 --- a/candy/plugin-build/buildkit.go +++ b/candy/plugin-build/buildkit.go @@ -29,6 +29,7 @@ import ( "strings" "time" + "github.com/opencharly/sdk" "github.com/opencharly/sdk/buildkitdrive" "github.com/opencharly/sdk/kit" "github.com/opencharly/spec/container" @@ -47,6 +48,17 @@ func baseImageRef(containerfile string) string { return "" } +// ensureBase is the ensure CONTRACT as a package-level seam (a var for the same reason the other +// seams in this module are): the resolving decision — which ref, which dir, which run engine, and +// what a failure says — must be provable without a registry that cannot serve the base, which is the +// venue charly#949 is about. Production assigns the real function. +// It is assigned in init() rather than by a var initializer because the contract reaches BACK into +// this drive (ensureRemoteRef -> runBoxBuild -> buildImage -> ensureBaseImage -> ensureBase), which a +// var initializer turns into an initialization cycle the compiler refuses. +var ensureBase func(ctx context.Context, ex *sdk.Executor, image, dir, runEngine string) error + +func init() { ensureBase = ensureViaResolveAndBuild } + // ensureBaseImage makes an image's BASE present before BuildKit resolves it. A base that is not in // the run engine's store and cannot be fetched fails deep inside the build with the registry's own // auth error — even when the recipe for that base is in this project — because `FROM ${BASE_IMAGE}` @@ -67,7 +79,7 @@ func (c driveConfig) ensureBaseImage(ctx context.Context, containerfile string) if c.Ex == nil { return fmt.Errorf("base image %s is not present locally and this drive holds no executor to resolve it, so it can neither be pulled nor built here", ref) } - if err := ensureViaResolveAndBuild(ctx, c.Ex, ref, c.Dir, c.RunEngine); err != nil { + if err := ensureBase(ctx, c.Ex, ref, c.Dir, c.RunEngine); err != nil { return fmt.Errorf("base image %s is not present locally, could not be fetched from its registry (%v), and no local build was possible for it: a registry ref whose recipe is not in this project cannot be built here — name the recipe in this project, or pull it once with a credential, and retry", ref, err) } fmt.Fprintf(os.Stderr, "ensure-image: base %s is now present\n", ref)