diff --git a/candy/plugin-build/base_image_test.go b/candy/plugin-build/base_image_test.go new file mode 100644 index 0000000..759b8bb --- /dev/null +++ b/candy/plugin-build/base_image_test.go @@ -0,0 +1,151 @@ +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" + "errors" + "os/exec" + "strings" + "testing" + + "github.com/opencharly/sdk" +) + +// 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) + } + } +} + +// 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) + } +} + +// 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 b564c2a..029bfc3 100644 --- a/candy/plugin-build/buildkit.go +++ b/candy/plugin-build/buildkit.go @@ -29,12 +29,63 @@ import ( "strings" "time" + "github.com/opencharly/sdk" "github.com/opencharly/sdk/buildkitdrive" "github.com/opencharly/sdk/kit" "github.com/opencharly/spec/container" "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 "" +} + +// 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}` +// 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 := 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) + 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 +130,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).