Skip to content

fix: bind KUKE_CONFIGURATION env so in-process image verbs honor the dev-profile suffix - #1328

Merged
eminwux merged 1 commit into
mainfrom
fix/image-cmds-honor-kuke-config-env
Jun 16, 2026
Merged

fix: bind KUKE_CONFIGURATION env so in-process image verbs honor the dev-profile suffix#1328
eminwux merged 1 commit into
mainfrom
fix/image-cmds-honor-kuke-config-env

Conversation

@eminwux

@eminwux eminwux commented Jun 16, 2026

Copy link
Copy Markdown
Owner

Summary

Under KUKEON_PROFILE=dev, kuke image * / kuke get image* ignored the configured containerdNamespaceSuffix and resolved the default <realm>.kukeon.io namespace, so the daemon's own images (in <realm>.dev.kukeon.io) were invisible and image load/prune/delete landed in a parallel, unused namespace.

Verified root cause — differs from the issue's hypothesis. The issue speculated the image command tree bypasses the root PersistentPreRunE, or that namespace resolution is captured before ConfigureRuntime runs, and explicitly asked the implementer to confirm. Neither holds: the image trees register no PersistentPreRunE, so the root's runs for them, and resolution is per-call. The real cause is that KUKE_CONFIGURATION was never bound to viper (git log -S confirms it never existed). loadClientConfiguration reads the config path via viper.GetString(config.KUKE_CONFIGURATION.ViperKey), which only had a BindPFlag for --configuration (default ~/.kuke/kuke.yaml). So KUKE_CONFIGURATION=<dev config> — the env var scripts/dev-init.sh exports to point every in-process client command at the dev config — was silently dropped, the dev config was never read, and the suffix stayed .kukeon.io. Daemon-routed kinds (get realms) reflected the daemon's own dev config and looked correct, masking the gap exactly as the issue describes.

Empirically confirmed with a viper probe: an unbound key returns the flag default and ignores the env var; BindEnv makes the env win over an unchanged flag's default (correct precedence: explicit flag > env > flag default).

Fix

  • cmd/kuke/kuke.go: call config.KUKE_CONFIGURATION.BindEnv() at the top of loadClientConfiguration, before the path read. It must live here (not in loadConfig, where the other BindEnv calls are) because loadClientConfiguration runs earlier in PersistentPreRunE.
  • cmd/kuke/clientconfig_test.go: regression test driving the path through the env var only (t.Setenv, no viper.Set/flag) — the sibling …SeedsViper test seeded the viper key directly, which is precisely why this slipped through. Asserts both the suffix viper key and consts.RealmNamespace("kuke-system") == "kuke-system.dev.kukeon.io".
  • scripts/dev-init.sh: corrected an inline comment that asserted this binding already existed "in loadConfig" — it pointed at the wrong function and the binding it claimed was in fact absent. Now points at loadClientConfiguration ahead of the path read.

This is a root-cause fix at the env-binding layer rather than threading the suffix into the image controller specifically (the issue's Proposal item 1 offered either). It also makes other in-process client paths (e.g. --no-daemon workloads) honor a KUKE_CONFIGURATION-supplied config, which is the documented dev-init contract. Proposal item 2 (a --containerd-namespace-suffix escape-hatch flag on the image commands) is intentionally not implemented — the issue marked it "if one is intended" and none of the AC items require it; the lower-blast-radius reading is to fix the binding and skip the new flag surface.

Test plan

  • GOWORK=off go test ./cmd/kuke/ -run TestLoadClientConfiguration -v — new + existing pass
  • Confirmed the new test fails without the fix (reproduces the issue: got "kuke-system.kukeon.io", want "kuke-system.dev.kukeon.io") and passes with it
  • GOWORK=off go build ./... (exit 0)
  • GOWORK=off go test $(go list ./... | grep -v /e2e) — full suite, 0 failures, exit 0
  • cd cmd/kukebuild && GOWORK=off go test ./... (exit 0)
  • GOWORK=off go vet ./cmd/kuke/ (exit 0)
  • pre-commit run --files … — all hooks pass (golangci-lint, go fmt, addlicense, …)
  • make dev-init dev-profile smoke — not run; the dev daemon / standalone containerd is not provisioned on this host. The fix's observable (suffix resolution from the env-supplied config) is covered by the failing-without-fix regression test above. Note: this fix also repairs the dev-init --no-daemon get realms parity tail, which depends on the same KUKE_CONFIGURATION env binding.

Note on GOWORK=off

The host's go1.25.5 + repo go.work trips a pre-existing containerd/cgroups/v2 compile break unrelated to this change; GOWORK=off builds against go.mod's pinned graph (the standard local workaround) and is green.

Acceptance criteria

  • With a dev-suffix client config, kuke get image[s] resolves <realm>.dev.kukeon.io (suffix + consts.RealmNamespace asserted in the regression test; the in-process list/get path reads the same package var)
  • kuke image load --realm <r> writes to <realm>.<configured-suffix> — same consts.RealmNamespace resolution (internal/controller/image.go:175)
  • kuke image prune / kuke image delete operate on <realm>.<configured-suffix> — same resolution (:212, :175)
  • No regression to <realm>.kukeon.io when no suffix is configured (unbound/unset env → flag default → existing …AbsentFileNoError + …SeedsViper tests stay green)

Closes #1325

@eminwux eminwux added ready-for-review PR is handed off to the reviewer agent in-review Reviewer agent is actively reading this PR and removed ready-for-review PR is handed off to the reviewer agent labels Jun 16, 2026
@eminwux

eminwux commented Jun 16, 2026

Copy link
Copy Markdown
Owner Author

PR #1328 Review — fix: bind KUKE_CONFIGURATION env so in-process image verbs honor the dev-profile suffix

LGTM — correct root-cause fix: loadClientConfiguration reads the config path (cmd/kuke/kuke.go:300) before loadConfig's BindEnv calls run, so binding KUKE_CONFIGURATION there (:299) is what lets the env-supplied dev config win; viper's changed-flag > env > flag-default precedence keeps an explicit --configuration path unaffected. The _ = …BindEnv() discard matches the sibling pattern in loadConfig, and the regression test drives the env-only path the …SeedsViper test masked. CI fully green incl. make e2e; commits signed.

The make dev-init dev-profile smoke is unchecked (host-gated — no dev daemon/containerd on the sandbox) and non-blocking; worth running pre-merge to confirm the --no-daemon get realms parity tail it also repairs.

@eminwux eminwux added ready-to-merge Reviewed and ready to merge and removed in-review Reviewer agent is actively reading this PR labels Jun 16, 2026
@eminwux
eminwux merged commit 1fd7ef2 into main Jun 16, 2026
5 checks passed
@eminwux
eminwux deleted the fix/image-cmds-honor-kuke-config-env branch June 16, 2026 22:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge Reviewed and ready to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

image: kuke image */kuke get image* ignore containerdNamespaceSuffix — target <realm>.kukeon.io under dev profile, can't see/load the daemon's images

1 participant