diff --git a/candy/plugin-deploy-pod/config_setup.go b/candy/plugin-deploy-pod/config_setup.go index 4993a68..864a2cf 100644 --- a/candy/plugin-deploy-pod/config_setup.go +++ b/candy/plugin-deploy-pod/config_setup.go @@ -131,7 +131,12 @@ func resolveDeployRef(ctx context.Context, ex *sdk.Executor, c *spec.PodConfigSe // LoadDeployConfigForRead(caller) swallow): a load error degrades to (nil, nil) + a stderr warning, // so a config setup that finds no overlay proceeds with image-label-driven behavior (the same // graceful degradation the host leg made). -func loadDeploy(ctx context.Context, ex *sdk.Executor, caller string) (*deploykit.DeployConfig, error) { +// +// It is a package var (not a plain func) for the SAME reason saveDeploy / mutateDeploy / +// projectConnect are: a unit test with no reverse-channel executor must be able to stub the +// per-host read instead of driving loaderkit against the operator's real ~/.config/charly, which +// cannot succeed and would print the degradation warning into the test output. +var loadDeploy = func(ctx context.Context, ex *sdk.Executor, caller string) (*deploykit.DeployConfig, error) { dc, err := loaderkit.LoadHostDeployConfigViaExecutor(ctx, ex) if err != nil { fmt.Fprintf(os.Stderr, "Warning: %s: charly.yml unavailable for read: %v\n", caller, err) @@ -252,10 +257,11 @@ func runConfig(ctx context.Context, ex *sdk.Executor, rt *kit.ResolvedRuntime, s volumes, bindMounts := deploykit.ResolveVolumeBacking(c.Box, c.Instance, meta.Volume, deployVolumes, meta.Home, rt.EncryptedStoragePath, rt.VolumesPath) - // resolvedOverlayImage reads the dc already in hand (R3: the SAME gate resolveDeployRefLocal - // consults), so the overlay comparison costs no second resolve. - imageRef = qualifyImageRef(imageRef, meta.Registry, c.ExplicitRef, - resolvedOverlayImage(dc, c.Box, c.Instance), deployBoxName, c.Tag) + // imageRef is ALREADY the ref this deploy resolves to: resolveDeployRefLocal applies the + // box→image naming rule (box registry → namespace defaults.registry → the box LEAF) BEFORE + // ensure-image/ExtractMetadata run. Qualifying it again here from the IMAGE's OCI label would + // be a second naming rule applied after the existence check — the shape that produced + // `image not found in local storage: charly.check-k8s-deploy-app:` (#44). // Reconcile every named volume's ROOT ownership to the deploy's runtime user (volume_root.go). A // volume first populated from an image whose directory was root-owned keeps that root ownership diff --git a/candy/plugin-deploy-pod/lifecycle.go b/candy/plugin-deploy-pod/lifecycle.go index bdc940f..6e473ee 100644 --- a/candy/plugin-deploy-pod/lifecycle.go +++ b/candy/plugin-deploy-pod/lifecycle.go @@ -293,8 +293,16 @@ func podPrepareVenue(ctx context.Context, exec *sdk.Executor, p lifecycleParams) return nil, fmt.Errorf("plugin-deploy-pod prepare-venue: decode opts: %w", err) } } + // The base box name. Resolve it BEFORE the overlay build: the host's `overlay` builder takes + // req.Image as the base ref (falling back to req.DeployName), so a raw deploy KEY reaching it + // fails inside podman with `short-name "charly/check-sidecar-pod" did not resolve to an alias`. + // An explicit image wins untouched; otherwise the deploy KEY is resolved through the plugin's + // own deploy-key→box resolver, because a key is not a box name (a qualified bed is dispatched + // as `ns/name` while its box is the declared `image:`). The SAME resolved name is the key into + // the render-prepped envelope's Boxes (fetched below via the build:generate resolve call). + baseName := resolveBaseBoxName(ctx, exec, p.Name, p.Image, "pod prepare-venue") reqJSON, err := json.Marshal(spec.OverlayBuildRequest{ - Dir: p.Dir, DeployName: p.Name, Image: p.Image, Version: p.Version, + Dir: p.Dir, DeployName: p.Name, Image: baseName, Version: p.Version, DryRun: opts.DryRun, AssumeYes: opts.AssumeYes, AllowRepoChanges: opts.AllowRepoChanges, AllowRootTasks: opts.AllowRootTasks, WithServices: opts.WithServices, }) @@ -313,13 +321,6 @@ func podPrepareVenue(ctx context.Context, exec *sdk.Executor, p lifecycleParams) return nil, fmt.Errorf("plugin-deploy-pod prepare-venue: %s", reply.Error) } - // The base box name — the key into the render-prepped envelope's Boxes (fetched below via the - // build:generate resolve call) — is the SAME base the host prep used (req.Image / req.DeployName). - baseName := p.Image - if baseName == "" { - baseName = p.Name - } - // Base-image metadata (User/Security/Registry) — read PLUGIN-SIDE (K3-W2): the host prep only // resolves+returns reply.BaseImage (the ref); this candy already imports deploykit, so it reads // the ai.opencharly.* labels itself via the SAME spec/container mechanism the host used to run @@ -527,14 +528,14 @@ func podRebuild(ctx context.Context, exec *sdk.Executor, p lifecycleParams) (*pb return nil, fmt.Errorf("plugin-deploy-pod rebuild: decode opts: %w", err) } } - baseRef := p.Image - if baseRef == "" { - baseRef = p.Name - } if ropts.DryRun { return marshalReply(struct{}{}) // dry-run: the host proxy prints nothing for Rebuild } if ropts.RebuildImage { + // Same resolution rule as podPrepareVenue: `box build`/`check box` take a BOX name, and the + // lifecycle params carry the deploy KEY (a qualified bed's key is `ns/name`), never the key + // verbatim — opencharly/plugin-build#46. + baseRef := resolveBaseBoxName(ctx, exec, p.Name, p.Image, "pod rebuild") if _, err := podCli(ctx, exec, false, false, "box", "build", baseRef); err != nil { return nil, err } diff --git a/candy/plugin-deploy-pod/resolve.go b/candy/plugin-deploy-pod/resolve.go index 6dc0495..cf9a401 100644 --- a/candy/plugin-deploy-pod/resolve.go +++ b/candy/plugin-deploy-pod/resolve.go @@ -188,11 +188,9 @@ func resolvePodRuntimeImageWith(ctx context.Context, ex *sdk.Executor, box, inst cliVolumes := parseVolumeFlagsCLI(volumeFlag, bind) volumes, bindMounts := deploykit.ResolveVolumeBacking(box, instance, meta.Volume, mergeVolumeConfigsLocal(deployVolumes, cliVolumes), meta.Home, rt.EncryptedStoragePath, rt.VolumesPath) - if meta.Registry != "" { - if _, ref, e := resolveDeployRefLocal(ctx, ex, box, instance, tag, ""); e == nil { - imageRef = ref - } - } + // imageRef came from resolveDeployRefLocal, which already applies the box→image naming rule + // (registry → leaf) before the existence check — the ONE place the ref is composed. The + // former re-resolve-on-meta.Registry branch was a second naming rule applied after the fact. return &podRuntimeImage{detected: detected, sel: sel, imageRef: imageRef, meta: &meta, dc: dc, volumes: volumes, bindMounts: bindMounts}, nil } diff --git a/candy/plugin-deploy-pod/resolve_box_name_test.go b/candy/plugin-deploy-pod/resolve_box_name_test.go new file mode 100644 index 0000000..aee0bf3 --- /dev/null +++ b/candy/plugin-deploy-pod/resolve_box_name_test.go @@ -0,0 +1,227 @@ +package deploypod + +import ( + "context" + "testing" + + "github.com/opencharly/sdk" + "github.com/opencharly/sdk/deploykit" + "github.com/opencharly/spec/spec" +) + +// TestResolveBaseBoxName_QualifiedDeployKeyResolvesProjectImage is the R7 regression for +// opencharly/plugin-build#46. +// +// A qualified pod bed (`charly.check-sidecar-pod`) is dispatched as the deploy KEY +// `charly/check-sidecar-pod` (spec.DeployKey(ns, rest)). The host could not resolve that slash +// key against the project's namespace-qualified deploy map, so it synthesized a target-only node +// and podPrepareVenue received an EMPTY p.Image. The pre-fix fallback then handed the deploy KEY +// to `build:generate resolve` as a BOX name and failed with +// `unknown box "charly/check-sidecar-pod"`. +// +// The project projection (spec.UnifiedFile.Deploys) keys a namespaced deploy as `ns.name`, so the +// deploy KEY and the project key differ by exactly one separator. resolveBaseBoxName is the ONE +// place that reconciles them, shared by podPrepareVenue and podRebuild. +// +// LAYER PROVEN: the resolver helper. podPrepareVenue itself needs a live reverse-channel executor +// (HostBuild "overlay" + InvokeProvider) and is covered by the live `charly.check-sidecar-pod` +// bed, not by this unit test. +func TestResolveBaseBoxName_QualifiedDeployKeyResolvesProjectImage(t *testing.T) { + // stubNoPerHost makes the per-host overlay read return nothing, so the assertions below + // exercise the PROJECT fallback (a fresh `deploy add` has no per-host entry yet). + stubNoPerHost := func(t *testing.T) { + t.Helper() + origLoadDeploy := loadDeploy + t.Cleanup(func() { loadDeploy = origLoadDeploy }) + loadDeploy = func(_ context.Context, _ *sdk.Executor, _ string) (*deploykit.DeployConfig, error) { + return nil, nil + } + } + stubProject := func(t *testing.T, project *spec.UnifiedFile) { + t.Helper() + origConnect, origLoad := projectConnect, loadUnifiedForBox + t.Cleanup(func() { projectConnect, loadUnifiedForBox = origConnect, origLoad }) + projectConnect = func(_ context.Context, _ *sdk.Executor, _ string, pre *spec.DeployPluginsConnectReply) error { + pre.Dir = "/project" + return nil + } + loadUnifiedForBox = func(_ context.Context, _ *sdk.Executor, _ string) (*spec.UnifiedFile, bool, error) { + return project, true, nil + } + } + + // The umbrella's shape: the `charly` namespace imports the charly repo, whose charly.yml + // declares the bed as `check-sidecar-pod: {pod: {image: check-k8s-deploy-app}}`. + namespaced := &spec.UnifiedFile{ + Deploy: map[string]spec.DeployNode{}, + Namespaces: map[string]*spec.UnifiedFile{ + "charly": { + Deploy: map[string]spec.DeployNode{ + "check-sidecar-pod": {Target: "pod", Image: "check-k8s-deploy-app"}, + }, + }, + }, + } + + t.Run("namespaced deploy key resolves the project image", func(t *testing.T) { + stubNoPerHost(t) + stubProject(t, namespaced) + got := resolveBaseBoxName(context.Background(), nil, "charly/check-sidecar-pod", "", "test") + if got != "check-k8s-deploy-app" { + t.Fatalf("resolveBaseBoxName(charly/check-sidecar-pod) = %q, want %q — the deploy KEY must never reach the box namespace (opencharly/plugin-build#46)", got, "check-k8s-deploy-app") + } + }) + + // The SAME rule drives `charly config`: the bed runner hands config the deploy key + // `charly/check-sidecar-pod`, which canonicalizes to (charly, check-sidecar-pod). Pre-fix, + // resolveDeployRefLocal resolved the bare first segment and built the ref `charly:` — a ref + // nothing ever built (measured). The config leg and the lifecycle leg share one resolver, so a + // fix that covered only prepare-venue would hand the bed a config failure one step later. + t.Run("config ref resolution uses the same rule", func(t *testing.T) { + stubNoPerHost(t) + stubProject(t, namespaced) + box, ref, err := resolveDeployRefLocal(context.Background(), nil, "charly", "check-sidecar-pod", "testtag", "") + if err != nil { + t.Fatalf("resolveDeployRefLocal: %v", err) + } + if box != "check-k8s-deploy-app" { + t.Fatalf("resolveDeployRefLocal box = %q, want %q — the config path must resolve the same deploy entry", box, "check-k8s-deploy-app") + } + if ref != "check-k8s-deploy-app:testtag" { + t.Fatalf("resolveDeployRefLocal ref = %q, want %q", ref, "check-k8s-deploy-app:testtag") + } + }) + + // The supporting mechanism: splitting the key into (base, instance) and handing those to the + // resolver does NOT find the project entry, because the project key is `charly.check-sidecar-pod` + // — not the bare first segment `charly`. This is why the fix must try the namespace-qualified + // spelling rather than merely ParseDeployKey-ing the name. + t.Run("splitting the key at the slash is not enough", func(t *testing.T) { + stubNoPerHost(t) + stubProject(t, namespaced) + if got := deployKeyToBoxLocal(context.Background(), nil, nil, "charly", "check-sidecar-pod"); got != "" { + t.Fatalf("deployKeyToBoxLocal(charly, check-sidecar-pod) = %q, want %q — this assertion documents why the slash split alone does not resolve the project entry", got, "") + } + }) + + t.Run("per-host overlay entry wins without a project read", func(t *testing.T) { + origLoadDeploy := loadDeploy + origConnect := projectConnect + t.Cleanup(func() { loadDeploy, projectConnect = origLoadDeploy, origConnect }) + loadDeploy = func(_ context.Context, _ *sdk.Executor, _ string) (*deploykit.DeployConfig, error) { + return &deploykit.DeployConfig{Deploy: map[string]deploykit.DeployNode{ + "charly/check-sidecar-pod": {Target: "pod", Image: "check-k8s-deploy-app"}, + }}, nil + } + projectConnect = func(_ context.Context, _ *sdk.Executor, _ string, _ *spec.DeployPluginsConnectReply) error { + t.Fatalf("a resolved per-host entry must not fall through to the project read") + return nil + } + got := resolveBaseBoxName(context.Background(), nil, "charly/check-sidecar-pod", "", "test") + if got != "check-k8s-deploy-app" { + t.Fatalf("resolveBaseBoxName from the per-host overlay = %q, want %q", got, "check-k8s-deploy-app") + } + }) + + t.Run("explicit image wins untouched", func(t *testing.T) { + origLoadDeploy := loadDeploy + t.Cleanup(func() { loadDeploy = origLoadDeploy }) + loadDeploy = func(_ context.Context, _ *sdk.Executor, _ string) (*deploykit.DeployConfig, error) { + t.Fatalf("an explicit image must not consult the deploy resolver") + return nil, nil + } + got := resolveBaseBoxName(context.Background(), nil, "charly/check-sidecar-pod", "explicit-ref:tag", "test") + if got != "explicit-ref:tag" { + t.Fatalf("resolveBaseBoxName with explicit image = %q, want %q", got, "explicit-ref:tag") + } + }) + + t.Run("genuinely unresolvable key falls back to the key", func(t *testing.T) { + stubNoPerHost(t) + stubProject(t, &spec.UnifiedFile{Deploy: map[string]spec.DeployNode{}}) + got := resolveBaseBoxName(context.Background(), nil, "no-such-deploy", "", "test") + if got != "no-such-deploy" { + t.Fatalf("resolveBaseBoxName(unresolvable) = %q, want the key itself %q", got, "no-such-deploy") + } + }) + + // A plain Pattern-A instance deploy is authored under its SLASH key in the project map + // (`versa/ecovoyage` stays `versa/ecovoyage`; only namespace imports are dot-qualified), so the + // exact key must be tried before the namespace-qualified spelling. + t.Run("local instance deploy resolves under its slash key", func(t *testing.T) { + stubNoPerHost(t) + stubProject(t, &spec.UnifiedFile{Deploy: map[string]spec.DeployNode{ + "versa/ecovoyage": {Target: "pod", Image: "versa"}, + }}) + got := resolveBaseBoxName(context.Background(), nil, "versa/ecovoyage", "", "test") + if got != "versa" { + t.Fatalf("resolveBaseBoxName(versa/ecovoyage) = %q, want %q", got, "versa") + } + }) +} + +// TestResolveDeployRefLocal_QualifiesTheBoxBuildRef is the R7 regression for the config half of +// the R10 failure measured on run 2026.307.2336 (`charly.check-sidecar-pod`): +// +// ensure-image: building charly.check-k8s-deploy-app locally +// charly: error: ... image not found in local storage: +// charly.check-k8s-deploy-app:charly.check-sidecar-pod-2026.307.2336 +// +// The deploy-add step had persisted the deploy entry with the NAMESPACE-QUALIFIED BOX ADDRESS +// (`charly.check-k8s-deploy-app`) in `image:` — a value that names a box, not an image repo. The +// config resolver composed `:` and handed it to ensure-image. The image the +// build had actually produced is `ghcr.io/opencharly/check-k8s-deploy-app:`: the box LEAF +// under its NAMESPACE's defaults.registry. +// +// This is the plugin side of the ONE box→image naming rule the host build path applies +// (resolveBoxImageRegistryName + ResolveShellImageRef). It FAILS without the fix: the ref comes +// back as the bare namespace-qualified address. +func TestResolveDeployRefLocal_QualifiesTheBoxBuildRef(t *testing.T) { + const ( + registry = "ghcr.io/opencharly" + leaf = "check-k8s-deploy-app" + tag = "charly.check-sidecar-pod-2026.307.2336" + ) + origLoad, origConnect, origLoadUnified := loadDeploy, projectConnect, loadUnifiedForBox + t.Cleanup(func() { + loadDeploy, projectConnect, loadUnifiedForBox = origLoad, origConnect, origLoadUnified + }) + + // The project the build ran against: the box lives in the `charly` namespace, whose + // defaults.registry is the prefix the build applied. + ns := &spec.UnifiedFile{Defaults: spec.BoxConfig{Registry: registry}} + ns.SetBox(leaf, spec.BoxConfig{}) + project := &spec.UnifiedFile{Namespaces: map[string]*spec.UnifiedFile{"charly": ns}} + projectConnect = func(_ context.Context, _ *sdk.Executor, _ string, pre *spec.DeployPluginsConnectReply) error { + pre.Dir = "/project" + return nil + } + loadUnifiedForBox = func(_ context.Context, _ *sdk.Executor, _ string) (*spec.UnifiedFile, bool, error) { + return project, true, nil + } + + // Two spellings reach this resolver, and NEITHER is an image repo: the namespace-qualified + // BOX ADDRESS `deploy add` persists in the per-host `image:`, and the project deploy map's + // bare LEAF. Both must compose the leaf under the namespace's registry. + for _, boxRef := range []string{"charly." + leaf, leaf} { + t.Run(boxRef, func(t *testing.T) { + loadDeploy = func(_ context.Context, _ *sdk.Executor, _ string) (*deploykit.DeployConfig, error) { + return &deploykit.DeployConfig{Deploy: map[string]deploykit.DeployNode{ + "charly.check-sidecar-pod": {Target: "pod", Image: boxRef}, + }}, nil + } + + box, ref, err := resolveDeployRefLocal(context.Background(), nil, "charly.check-sidecar-pod", "", tag, "") + if err != nil { + t.Fatalf("resolveDeployRefLocal: %v", err) + } + if box != boxRef { + t.Fatalf("box = %q, want the deploy's resolved box %q", box, boxRef) + } + want := registry + "/" + leaf + ":" + tag + if ref != want { + t.Fatalf("ref = %q, want %q — the config path must name the ref the box build produced, not the box address/leaf", ref, want) + } + }) + } +} diff --git a/candy/plugin-deploy-pod/resolve_ref.go b/candy/plugin-deploy-pod/resolve_ref.go index fda3d68..8648135 100644 --- a/candy/plugin-deploy-pod/resolve_ref.go +++ b/candy/plugin-deploy-pod/resolve_ref.go @@ -22,10 +22,13 @@ import ( // "deploy-plugins-connect" seam's project dir (the SAME plugin-side loader path // candy/plugin-deploy's resolveTreeViaLoader uses), and // - kit.ResolveShellImageRef / kit.LocalImageExists (pure). -// Byte-for-byte the former host resolvers' two-tier semantics: per-host Image (DeployKey then bare -// key) → project Deploy[box].Image → the key itself; and the add_candy-overlay resolved_image -// preference over the base-name resolution (DeployKey then bare key), gated on the overlay image -// actually existing locally. +// +// The BOX NAME follows the former host resolvers' two-tier semantics: per-host Image (DeployKey +// then bare key) → project Deploy[box].Image → the key itself. The IMAGE REF is composed from that +// box by the ONE build-path rule (resolveDeployImageRef): box registry → namespace +// defaults.registry → box LEAF, so a namespace-qualified box ADDRESS is never handed to the engine +// as an image repo. The add_candy-overlay resolved_image preference still wins over the base-name +// resolution (DeployKey then bare key), gated on the overlay image actually existing locally. const deployPluginsConnectKind = "deploy-plugins-connect" @@ -40,7 +43,8 @@ func resolveDeployRefLocal(ctx context.Context, ex *sdk.Executor, box, instance, if err != nil { return "", "", err } - deployBoxName = resolveDeployBoxNameLocal(ctx, ex, dc, box, instance) + deployKey := spec.DeployKey(box, instance) + deployBoxName, _ = resolveDeployBoxNameForKey(ctx, ex, dc, deployKey) if ov := resolvedOverlayImage(dc, box, instance); ov != "" { rt, rtErr := kit.ResolveRuntime() if rtErr != nil { @@ -54,16 +58,64 @@ func resolveDeployRefLocal(ctx context.Context, ex *sdk.Executor, box, instance, return deployBoxName, ov, nil } } - return deployBoxName, kit.ResolveShellImageRef("", deployBoxName, tag), nil + return deployBoxName, resolveDeployImageRef(ctx, ex, deployKey, deployBoxName, tag), nil +} + +// resolveBaseBoxName returns the BASE BOX NAME a pod lifecycle op must build/deploy from. It is the +// ONE implementation of "a deploy KEY is never a box name" (R2): podPrepareVenue and podRebuild +// call it instead of spelling the image/deploy-name fallback themselves. +// +// An explicit image (the host resolved the deploy entry itself) wins untouched. Otherwise the +// deploy KEY is resolved through the deploy-key→box rule (resolveDeployBoxNameForKey). Handing the +// raw key to the build pipeline is what produced `unknown box "charly/check-sidecar-pod"` +// (opencharly/plugin-build#46). +// +// Only a genuinely unresolvable key falls back to the key itself (the key==image convention). +func resolveBaseBoxName(ctx context.Context, ex *sdk.Executor, name, image, caller string) string { + if image != "" { + return image + } + dc, err := loadDeploy(ctx, ex, caller) + if err != nil { + return name + } + if resolved, ok := resolveDeployBoxNameForKey(ctx, ex, dc, name); ok { + return resolved + } + return name } -// resolveDeployBoxNameLocal mirrors the former host box-name resolver: the deploy entry's declared -// box (per-host then project), falling back to the key itself (the key==image convention). -func resolveDeployBoxNameLocal(ctx context.Context, ex *sdk.Executor, dc *deploykit.DeployConfig, box, instance string) string { +// resolveDeployBoxNameForKey resolves a deploy KEY to the BOX name its deploy entry declares, +// trying every spelling a namespaced deploy can carry. It is the ONE "given a deploy key, which +// box?" rule: the config ref resolution (resolveDeployRefLocal) and the lifecycle paths +// (resolveBaseBoxName) both go through it, so no caller can hand a deploy key to the box +// namespace. +// +// A deploy has TWO spellings: the KEY is `ns/name` (spec.DeployKey), while the PROJECT projection +// keys a namespaced deploy `ns.name` (spec.UnifiedFile.Deploys). This tries, in order: +// - the key's base/instance pair (per-host overlay DeployKey then bare key, then the project's +// base entry — the pre-existing rule, unchanged); +// - the exact deploy key (a local instance deploy keeps its slash spelling in the project map); +// - the namespace-qualified form (`ns.name`), which is how the project stores a namespaced +// deploy — the leg a fresh `deploy add` needs, before any per-host entry exists. +// +// The bool reports whether an entry was found. Callers keep their OWN fallback: the config path +// keeps the bare base name (the key==image convention), the lifecycle paths keep the key. +func resolveDeployBoxNameForKey(ctx context.Context, ex *sdk.Executor, dc *deploykit.DeployConfig, key string) (string, bool) { + box, instance := spec.ParseDeployKey(key) if img := deployKeyToBoxLocal(ctx, ex, dc, box, instance); img != "" { - return img + return img, true + } + if instance == "" { + return box, false + } + if img := deployKeyToBoxLocal(ctx, ex, dc, key, ""); img != "" { + return img, true + } + if img := deployKeyToBoxLocal(ctx, ex, dc, box+"."+instance, ""); img != "" { + return img, true } - return box + return box, false } // projectConnect and loadUnifiedForBox are the two executor-backed legs of the project @@ -120,40 +172,68 @@ func deployKeyToBoxLocal(ctx context.Context, ex *sdk.Executor, dc *deploykit.De return "" } -// qualifyImageRef applies the registry-qualification rule to the ref resolveDeployRef produced, -// returning the ref `charly config` renders the quadlet from. It is a PURE function of the values -// runConfig already holds — deliberately, so the rule below is unit-testable without an executor -// (the pre-fix form computed both its operands from executor-backed calls, which is a large part -// of why it shipped broken). +// resolveDeployImageRef composes the image ref a pod deploy resolves to, from the BOX the deploy +// names. It is the plugin side of the ONE box→image naming rule the host build path applies +// (charly's resolveBoxImageRegistryName + ResolveShellImageRef): the box's OWN registry, falling +// back to its NAMESPACE's defaults.registry, and the box LEAF. // -// The rule: registry-QUALIFY a short ref, UNLESS -// - the operator supplied an explicit ref via the deploy entry's `image:` field (Pattern B — -// arbitrary deploy-key + version-pin, /charly-core:deploy "Two supported deploy patterns"): -// imageRef is already fully-qualified and pinning to that exact tag is the whole point; or -// - imageRef IS this deploy's persisted add_candy overlay: the overlay is a LOCAL-only -// `-overlay:` tag (no registry segment, so LooksLikeFullRef cannot protect -// it), and prefixing the registry would throw the overlay away and deploy the BASE image. +// A namespace-qualified box ADDRESS is not an image repo. `charly.check-k8s-deploy-app` builds as +// `ghcr.io/opencharly/check-k8s-deploy-app:`; composing `
:` hands the engine a +// ref nothing produced (measured, opencharly/plugin-deploy-pod#44 run 2026.307.2336: +// `image not found in local storage: charly.check-k8s-deploy-app:`). // -// Two invariants this rule MUST keep, BOTH of which the pre-fix form violated: +// A FULL ref (a deploy entry's explicit `image:`, Pattern B) is not a box name and keeps the +// pre-existing shell-resolve contract. A project that cannot be loaded degrades to the leaf-only +// bare ref — the resolver's existing best-effort contract, and the same name the build uses with +// no registry. // -// 1. It compares REFS to REFS. The pre-fix form bound resolveDeployRef's FIRST return — the box -// NAME — and compared it to a ref, so the overlay test was always false and the persisted -// overlay was always discarded. -// -// 2. It QUALIFIES the ref it already has; it never RE-RESOLVES through the deploy-key API. The -// pre-fix form re-resolved via resolveDeployRefLocal(deployBoxName, …), and a box NAME is not -// a deploy KEY: that lookup reads whichever SIBLING deployment happens to be named after the -// shared base box, then (tag empty) resolves the base box's label family, where every sibling -// deployment's alias tag competes. -// -// Together those made every add_candy overlay pod deploy run some other deployment's image — -// operator deploys as much as check beds. -func qualifyImageRef(imageRef, registry, explicitRef, resolvedOverlay, boxName, tag string) string { - usingResolvedOverlay := imageRef != "" && imageRef == resolvedOverlay - if registry == "" || explicitRef != "" || usingResolvedOverlay || kit.LooksLikeFullRef(imageRef) { - return imageRef - } - return kit.ResolveShellImageRef(registry, boxName, tag) +// This runs BEFORE any existence check (ensure-image / ExtractMetadata), so the ref handed to the +// engine is the one the build produced. A post-hoc qualification sourced from the IMAGE's OCI +// label is unreachable on this path for exactly that reason, and is deleted. +func resolveDeployImageRef(ctx context.Context, ex *sdk.Executor, deployKey, boxRef, tag string) string { + if kit.LooksLikeFullRef(boxRef) { + return kit.ResolveShellImageRef("", boxRef, tag) + } + return kit.ResolveShellImageRef(projectBoxRegistry(ctx, ex, deployKey, boxRef), spec.LeafName(boxRef), tag) +} + +// projectBoxRegistry returns the registry the BOX BUILD prefixes for boxRef: the box's own +// `registry:`, else its NAMESPACE's `defaults.registry`, else "" (no registry). It reads the SAME +// project the deploy-key→box rule reads, over the SAME reverse-channel loader seams +// (projectConnect + loadUnifiedForBox), and folds the namespace tree through ProjectConfig. +func projectBoxRegistry(ctx context.Context, ex *sdk.Executor, deployKey, boxRef string) string { + var pre spec.DeployPluginsConnectReply + if err := projectConnect(ctx, ex, deployKey, &pre); err != nil || pre.Dir == "" { + return "" + } + uf, ok, err := loadUnifiedForBox(ctx, ex, pre.Dir) + if err != nil || !ok || uf == nil { + return "" + } + cfg := uf.ProjectConfig() + if cfg == nil { + return "" + } + img, nsCfg, ok := cfg.ResolveBoxRef(boxRef) + if !ok { + // The project deploy map's `image:` can carry the box's LEAF (the bed declares + // `image: check-k8s-deploy-app` while the box lives in the `charly` namespace). A bare + // leaf does not resolve from the ROOT Config, so discover its namespaced address first — + // the same discovery the host build path uses (Config.FindBoxByLeaf). + if qualified, found := cfg.FindBoxByLeaf(spec.LeafName(boxRef)); found { + img, nsCfg, ok = cfg.ResolveBoxRef(qualified) + } + } + if !ok { + return "" + } + if img.Registry != "" { + return img.Registry + } + if nsCfg != nil { + return nsCfg.Defaults.Registry + } + return "" } // resolvedOverlayImage mirrors the former host resolved_image lookup: the concrete add_candy-overlay diff --git a/candy/plugin-deploy-pod/resolve_ref_test.go b/candy/plugin-deploy-pod/resolve_ref_test.go index 56f159d..ce3f9ec 100644 --- a/candy/plugin-deploy-pod/resolve_ref_test.go +++ b/candy/plugin-deploy-pod/resolve_ref_test.go @@ -6,7 +6,6 @@ import ( "github.com/opencharly/sdk" "github.com/opencharly/sdk/deploykit" - "github.com/opencharly/sdk/kit" "github.com/opencharly/spec/spec" ) @@ -79,96 +78,6 @@ func TestResolveDeployRefLocal_ExplicitRefShortCircuit(t *testing.T) { } } -// TestQualifyImageRef is the regression witness for the image-crossing defect pair. Each case -// FAILS against the pre-fix rule (which compared the box NAME to a ref and, on the inevitable -// mismatch, re-resolved through the deploy-key API keyed on the base box name). -// -// kit.ResolveShellImageRef is stubbed so the qualify leg is observable without touching local -// podman storage; a nil stub call means the ref was passed through untouched. -func TestQualifyImageRef(t *testing.T) { - const ( - registry = "ghcr.io/opencharly" - boxName = "check-pod" - overlayRef = "check-addcandy-pod-overlay:abc123" - ) - origResolve := kit.ResolveShellImageRef - t.Cleanup(func() { kit.ResolveShellImageRef = origResolve }) - var qualified bool - kit.ResolveShellImageRef = func(reg, name, tag string) string { - qualified = true - if reg == "" { - // The pre-fix path dropped the registry (it re-resolved via the deploy-key API, - // which composes a registry-less ref) — the qualification's whole point is that - // the registry is carried. - t.Errorf("qualification dropped the registry (name=%q tag=%q)", name, tag) - } - if tag == "" { - return reg + "/" + name - } - return reg + "/" + name + ":" + tag - } - - cases := []struct { - name string - imageRef, registry, explicitRef, resolvedOverlay string - tag string - want string - wantQualified bool - }{ - { - // THE DEFECT: the deploy's own persisted add_candy overlay must survive untouched. - // Pre-fix this fell through to a base-box re-resolve and deployed another image. - name: "persisted overlay is preserved, never qualified", - imageRef: overlayRef, registry: registry, resolvedOverlay: overlayRef, - want: overlayRef, wantQualified: false, - }, - { - name: "short base ref is registry-qualified", - imageRef: "check-pod", registry: registry, resolvedOverlay: "", - want: registry + "/check-pod", wantQualified: true, - }, - { - name: "short base ref keeps the requested tag through qualification", - imageRef: "check-pod:2026.216.2119", registry: registry, resolvedOverlay: "", tag: "2026.216.2119", - want: registry + "/check-pod:2026.216.2119", wantQualified: true, - }, - { - // A DIFFERENT deploy's overlay must NOT suppress qualification — the comparison is - // against THIS deploy key's persisted overlay only. - name: "another deploy's overlay does not match this ref", - imageRef: "check-pod", registry: registry, resolvedOverlay: "check-other-pod-overlay:deadbeef", - want: registry + "/check-pod", wantQualified: true, - }, - { - name: "already-full ref passes through", - imageRef: registry + "/check-pod:2026.216.2119", registry: registry, - want: registry + "/check-pod:2026.216.2119", wantQualified: false, - }, - { - name: "Pattern B explicit ref is never recomposed", - imageRef: "check-pod", registry: registry, explicitRef: "ghcr.io/acme/pinned:2026.001.0001", - want: "check-pod", wantQualified: false, - }, - { - name: "no registry on the box → nothing to qualify with", - imageRef: "check-pod", registry: "", - want: "check-pod", wantQualified: false, - }, - } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - qualified = false - got := qualifyImageRef(tc.imageRef, tc.registry, tc.explicitRef, tc.resolvedOverlay, boxName, tc.tag) - if got != tc.want { - t.Fatalf("qualifyImageRef = %q, want %q", got, tc.want) - } - if qualified != tc.wantQualified { - t.Fatalf("qualification fired = %v, want %v", qualified, tc.wantQualified) - } - }) - } -} - // TestDeployKeyToBoxLocal_NamespaceQualifiedProjectFallback is the R7 regression for the // namespaced-deploy resolution defect (RCA-H). On an empty per-host overlay, a namespaced // pod bed (`charly.check-dsh-pod`) resolves its box ONLY through the project fallback