0014. CLI skeleton under cmd/ichor, and how ichor adopt guarantees zero drift

Status: Accepted · Date: 2026-10-25 · Design section: CLI and UI; Bootstrap (ichor adopt)

Decided: Settled cmd/ichor's package layout and how ichor adopt guarantees zero drift

Context

Phase 4 is the CLI. docs/design/architecture.md's "CLI and UI" section sets the rule — parity: the CLI and the UI share the same Go packages for rendering, diffing and planning, and both write CRs rather than mutating a Talos node directly. It names three exceptions that predate the cluster: init, adopt and full-loss recovery. This unit scaffolds cmd/ichor with cobra and implements the first of those exceptions, ichor adopt:

ichor adopt brings an existing cluster under management by diffing each node's current config against a fresh render and storing the difference as initial patches, so ownership transfers with zero drift.

Two problems surfaced while building this that are worth recording so the next person touching the CLI doesn't rediscover them the hard way.

cobra-cli's default layout collides with kubebuilder's

cobra-cli init cmd/ichor writes cmd/ichor/main.go importing <module>/cmd — its built-in assumption that command definitions live in a cmd package one level up from main.go, resolved against the module root rather than against the path given to init. In this repo that cmd package is already taken: cmd/main.go is the controller manager's own package main, from kubebuilder init. Importing a main package is a compile error ("is a program, not an importable package"), and it broke make manifests repo-wide the moment it landed, because controller-gen's paths="./..." walks into cmd/ichor too.

cobra-cli isn't wrong to assume this; kubebuilder's and cobra-cli's conventions simply both want to own a top-level cmd package, and only one can. The fix is to let cobra-cli scaffold normally and then move its output: the command tree lives in internal/cli (package cli), not cmd/ichor/cmd, and cmd/ichor/main.go is reduced to:

package main

import "github.com/maeshinshin/ichor/internal/cli"

func main() { cli.Execute() }

cobra-cli add <name> has the same issue in reverse: given no way to target a subdirectory (--package was removed from the CLI), it always writes cmd/<name>.go at the module root. Every new subcommand has to be generated there and moved into internal/cli by hand, same as adopt.go was.

goreleaser's default dist/ collides with the committed chart

dist/chart is a committed Helm chart (ADR tooling elsewhere generates it from dist/install.yaml); goreleaser also defaults to writing its own build output to dist/. goreleaser build --clean deletes whatever dist: points at before writing to it — discovered by running it and watching git status show the entire chart staged for deletion. .goreleaser.yaml now sets dist: .goreleaser-dist (ignored in .gitignore) so a release build can never touch the chart.

Decision

Package layout

cmd/ichor/main.go       — package main; nothing but Execute()
internal/cli/root.go    — rootCmd, persistent flags (-o/--output, --yes,
                           --kubeconfig, --talosconfig), Execute()
internal/cli/adopt.go   — the `adopt` cobra.Command: flags, RunE wiring
internal/cli/adopt_plan.go — buildAdoptPlan: the diff/verify/capture logic,
                           no I/O, fully unit-testable
internal/cli/adopt_run.go  — runAdopt: orchestration (fetch, confirm,
                           create, report) against adoptDeps, an interface
                           bundle a test can fake entirely
internal/cli/talosdial.go  — talosConfigReader + dialTalos: the one real
                           Talos API call adopt makes (CurrentConfig)
internal/cli/kube.go       — kubeconfig loading, the controller-runtime
                           client, scoped to Ichor's own CRDs only
internal/cli/bundlefile.go — reads a local secrets bundle file
internal/cli/output.go     — -o yaml|json encoding

Scaffolding commands run (from the repo root, inside nix develop):

cobra-cli init cmd/ichor
cobra-cli add adopt

then moving the four files cobra-cli wrote (cmd/ichor/cmd/{root,adopt}.go from init, cmd/adopt.go from add) into internal/cli, renaming their package cmd to package cli, and rewriting cmd/ichor/main.go's import as above. flake.nix already carried cobra-cli; nothing to add there.

kubeconfig and talosconfig

Both follow the tool whose convention they borrow, rather than inventing a third:

• --kubeconfig (persistent, root-level): client-go's own clientcmd.NewDefaultClientConfigLoadingRules() resolves it — an explicit flag value wins, otherwise $KUBECONFIG, otherwise ~/.kube/config — the same resolution kubectl itself uses. This is adopt's only path to the Kubernetes API: the TalosCluster/TalosMachine objects it reads, and the TalosConfigPatch objects it creates, all live behind it.
• --talosconfig (persistent, root-level): passed through to machinery's client.WithConfigFromFile, whose empty-string case already resolves $TALOSCONFIG, then ~/.talos/config, then the in-cluster service account mount — exactly talosctl's own resolution (pkg/machinery/client/config/path.go). adopt dials each TalosMachine's Talos API through this, using whatever admin identity the operator's talosconfig already carries for the cluster being adopted.

adopt deliberately does not reuse internal/controller.NewMachineryTalosClient (the controller's Talos API adapter): that constructor derives its client certificate from a secrets bundle Ichor already manages. adopt predates that management — there may be no Ichor-issued credential yet — so it dials the same way talosctl would, through the operator's own talosconfig. The secrets bundle still has to come from somewhere, since rendering a baseline needs one: --secrets-bundle takes a local file in the same format talosctl gen secrets writes and internal/controller/bundle.go's Secret convention already documents (secrets.yaml), since adopt runs before Ichor has a Secret of its own to read it from.

How ichor adopt guarantees zero drift

For each TalosMachine already recorded for --cluster:

1. Read the node's live configuration over the Talos API (talosConfigReader.CurrentConfig).
2. Render a baseline from the TalosCluster/TalosMachine/existing TalosConfigPatch objects already in the Kubernetes API (pkg/render.Render — not re-implemented).
3. Diff live vs. baseline with internal/controller.ChangedPaths (exported from this unit's previously-unexported changedPaths, logic unchanged) — not re-implemented either.
4. If nothing changed, there is nothing to adopt for this machine.
5. Otherwise, build a candidate TalosConfigPatch and verify it before ever proposing it: render baseline+candidate and require the result to be byte-identical to the node's live configuration. Only a passing candidate is ever returned from buildAdoptPlan; only a buildAdoptPlan result with a patch is ever shown to the operator or created.

Step 5's self-check, not the diff in step 3, is what the "zero drift" acceptance test actually exercises: it is exactly the comparison the test performs independently (render the created patch again, compare to the current-config fixture).

Step 5 also has to not leak secrets, which drove the scope decision in adopt_plan.go: only SysctlConfig and KernelModuleConfig documents are ever captured automatically. The legacy v1alpha1 Config document is deliberately never captured.

Update, same day: the reason this ADR originally gave no longer holds, and is recorded here rather than silently deleted so the history is honest. The original reason was schema-specific: TalosConfigPatchDocument's config.machine object required type, token and certSANs on any Config document, sparse or not, and token mirrored the secrets bundle — there was no way to patch e.g. cluster.network.cni.name without also restating it. A same-day mirrorgen change (see ADR 0003's update) dropped machine.token, machine.ca, cluster.ca and six other secret-bearing fields from the mirror type and the CRD schema entirely, closing a real hole (configpatcher.Apply's strategic merge meant a Config document could already overwrite a cluster's CA through a TalosConfigPatch). The two fields config.machine still requires — type and certSANs — are not secrets: type is the role already on TalosMachine.Spec.Role, certSANs the machine's own address. A sparse Config document is now representable without touching anything sensitive.

Second update, same day, this one final: it still does not capture it, and now for a design reason rather than a missing-code one. The question was asked directly: now that a sparse Config document is representable without the schema forcing a secret into it, should adopt actually build the sparse-reconstruction path? The answer is no, and the reason is a specific, checked fact about the Config document's own schema, not "nobody got to it yet."

The nine fields the same-day mirrorgen change dropped are not the only secret-shaped fields the Config document can carry. That cleanup removed exactly the fields that mirror secrets.Bundle — the CAs, the join/bootstrap tokens, the etcd encryption key, the service-account signing key — because those are Ichor's own root of trust. It was never meant to, and did not, sweep the rest of Talos's own config surface for every other field that happens to be a credential. Reading the generated mirror type for exactly this (not assumed) turns up several, live in the schema today, that have nothing to do with Ichor's bundle and so were never in scope for that pass:

• RegistryAuthConfig.{password,auth,identityToken} — private registry pull credentials, reachable from machine.registries.* and cluster.* registry config.
• VIPEquinixMetalConfig.apiToken / VIPHCloudConfig.apiToken — cloud provider API tokens, under machine.network's VIP config.
• WireguardDevice.privateKey (machine.network's WireGuard device config) — a WireGuard private key.
• ClusterConfig.secretboxEncryptionSecret — apparently missed by the same-day cleanup entirely: it is the secretbox sibling of aescbcEncryptionSecret, which was dropped, and is exactly the same kind of etcd encryption secret the other one is. Flagged back to whoever owns that cleanup; not something this unit can fix, since pkg/typedconfig is out of scope here.

None of these come from secrets.Bundle, so no amount of extending that one cleanup closes them — they are a different, open-ended category: whatever credential-shaped field Talos's own schema happens to carry this version, for a use case (private registries, cloud VIPs, WireGuard KubeSpan) an adopted cluster may well have configured. A sparse-diff mechanism that captures "whatever changed" has no way to tell "a harmless field changed" from "someone's registry password changed" without a field-by-field allow/deny list over the entire Config document — and that list would need maintaining forever as Talos's own schema grows, which is a standing commitment this command should not make unilaterally just to widen what adopt can automate. SysctlConfig and KernelModuleConfig don't have this problem because their entire field surface is kernel parameters and module names — there is no credential shape anywhere in either one to worry about missing.

The consequence is unchanged in practice, now for the reason that will actually last: a difference confined to the legacy Config document comes back from buildAdoptPlan as UnrepresentablePaths — field names only, safe to print — with no patch. runAdopt reports these and leaves that machine's drift for the operator to patch by hand, through a TalosConfigPatch they write themselves and can see the contents of before it goes anywhere. This is covered by TestBuildAdoptPlan_LegacyDiffIsNeverAutoPatched and is the one TestAdoptNeverLeaksSecrets subtest ("legacy diff reported, not patched") that proves the refusal itself never leaks either.

Confirmation and output

--yes skips the interactive y/N prompt before any TalosConfigPatch is created, matching the design document's "interactive confirmation with --yes" for every other dangerous CLI operation. -o yaml|json prints the patches adopt is about to create (or just created); both encoders only ever see ichorv1alpha1.TalosConfigPatch values, never the secrets bundle or a full rendered machine configuration.

Consequences

• cmd/ichor/main.go stays a two-line shim forever; all real CLI code lives under internal/cli, which also means future subcommands (init, ui, restore --recover-cluster) land there too, not under cmd/ichor/cmd.
• cobra-cli add <name> will keep writing to the wrong place (cmd/<name>.go at the module root) until/unless a future cobra-cli version restores a way to target a subdirectory; every new subcommand needs the same by-hand move this ADR describes for adopt.go.
• adopt's zero-drift guarantee has a known, now-permanent-by-design gap: differences confined to the legacy Config document are reported, not patched. This is not a missing function waiting to be written; it is a deliberate refusal, because the Config document's field surface has no stable boundary between "safe to auto-capture" and "a credential" -- registry passwords, cloud API tokens and a WireGuard private key are live examples found by reading the schema, not hypothetical. ichor adopt is complete for sysctl/kernel-module drift (no such fields exist there) and honest about everything else.
• A second, unrelated finding from the same reading: ClusterConfig's secretboxEncryptionSecret looks like it was missed by the same-day cleanup that dropped its sibling aescbcEncryptionSecret -- both are etcd encryption secrets, only one was removed. Flagged for whoever owns pkg/typedconfig; out of this unit's scope to fix directly.
• .goreleaser.yaml's dist: override means any future tooling that reads goreleaser's own output has to know to look in .goreleaser-dist, not dist/.