0027. Private registry auth through a Secret reference on TalosCluster

Status: Accepted, amended once · Date: 2026-10-25 · Design section: "Typed configuration", "Monitoring and registry mirrors"

Decided: Private registry auth through TalosCluster.spec.registries[].authSecretRef, resolved and injected as the system layer by pkg/render; the auth-less Render/BuildConfig entry points were merged into one that always takes the resolved auth, after no production caller had ever been moved onto the auth-aware pair

Context

ADR 0003's secret-fields follow-up dropped RegistryAuthConfig.{password,auth,identityToken} from the mirror types: a TalosConfigPatch is a CR that lives in Git, and letting it carry a registry password directly would put that password in Git -- a plain violation of design principle 2, "secrets are referenced, never embedded". That ADR named the fix it did not build: "a reference form -- a field shaped like TalosCluster.spec.secretsRef already is, pointing at a Kubernetes Secret the controller reads and injects, never a value the patch document carries directly". Phase 10 (air gap) needs exactly this: "registry mirrors pushed to every node through the system layer" (architecture.md, "Monitoring and registry mirrors") only works end to end for a private registry if its pull credentials can reach a node at all, and today they can't -- there is no field anywhere in the API that can carry them without reintroducing the Git-embedding problem ADR 0003 just closed.

Two things had to be settled: where the declaration lives, and what shape the reference takes.

Where it lives. The design's own CRD table defines TalosCluster as "Cluster-wide settings", and a registry mirror is exactly that -- it applies to every node, not one machine or one subset selected by label. TalosMachine (one node) and TalosConfigPatch (a patch with a layer) are both the wrong shape for a cluster-wide, always-on declaration.

Patch layers. "### Patch layers" lists the system layer as "generated by Ichor: CNI settings, kube-proxy, KubePrism, volume configs derived from disk roles, kernel modules and sysctls for storage presets, metrics exposure, control-plane taints" -- registry mirrors are not named there, but "### Monitoring and registry mirrors" is explicit that they are "pushed to every node through the system layer". The two sections describe the same stack from different angles (one lists what's already built, the other names a not-yet-built feature by its layer), not two different answers; rendering registries as the system layer, below every TalosConfigPatch layer, is consistent with both and was the one thing this change had to re-check before proceeding, per the task that produced this ADR. It did not surface a contradiction, so the work went ahead.

Decision

TalosClusterRegistry on TalosCluster.spec.registries[]. One entry per mirror: host (the RegistriesConfig map key machinery itself uses), endpoints (mirror URLs, not secret), username (the one RegistryAuthConfig field ADR 0003 left mirrorable, not secret), and authSecretRef, optional -- unset is an anonymous mirror, not an error.

authSecretRef is a corev1.LocalObjectReference, not a corev1.SecretKeySelector. Machinery's RegistryAuthConfig carries three credential shapes beyond username -- password, auth, identityToken, mirroring docker config.json's own auth forms -- and a given registry's credentials use at most one of them. A SecretKeySelector names exactly one key, so expressing "any of three keys, from one Secret" would need either three separate selector fields (three ways to misconfigure one reference, none of which the CRD can rule out which is set) or a selector plus an implicit convention for which other keys ride along with it, which defeats the point of a selector. A LocalObjectReference plus a fixed, documented set of well-known keys (password, auth, identityToken) is the same shape secretsRef already uses for bundleSecretKey, and it says the same thing: "this Secret holds the thing by convention", with exactly as many fields on the CRD as there are registries, not as many as there are credential shapes.

Resolution and injection live in pkg/render, not internal/controller. The existing precedent, loadBundle, lives in internal/controller and is called before render.Render, passing an already-decoded secrets.Bundle in. This change keeps that same shape -- resolve-then-pass-in, nothing new invented -- but places the resolving function itself (render.ResolveRegistryAuth) in pkg/render so the whole registry-auth surface (resolve, inject, and their leak tests) is one package's responsibility. pkg/render's own doc comment already claimed "touches no cluster"; that claim now has one named exception, documented at the top of the package, the same way the rest of the package's guarantees are documented.

Injection is additive, not a breaking signature change. render.Render and render.BuildConfig are called from internal/controller, internal/cli and internal/uiapi -- none of which this unit of work may touch (a different agent owns internal/controller for TalosOperation concurrently; internal/cli and internal/uiapi are out of scope entirely). Changing either function's signature would break all three without a way to fix them here. The fix: BuildConfig/Render are now one-line wrappers around a private buildConfig that takes an extra map[string]RegistryAuth parameter (nil from the existing wrappers, so their behavior and output are byte-for-byte unchanged -- TestRenderWithRegistryAuthNoRegistries checks this directly); BuildConfigWithRegistryAuth/RenderWithRegistryAuth are the new, fully additive entry points that pass a real map through. Wiring either of ResolveRegistryAuth/RenderWithRegistryAuth into TalosMachineReconciler's actual reconcile loop is internal/controller's call to make and is left for whoever next touches that package; this ADR's scope is the capability, built and tested against its public API, not the call site.

Two entry points for one render, closed with a refusal rather than left as a silent trap. Keeping Render/BuildConfig working unchanged means there are now two ways to render a machine's config, and nothing stopped a caller with cluster.Spec.Registries set from calling the one that never resolves or injects auth: the render would still succeed, the config would still apply, and the resulting cluster simply could not pull from its private registry -- no error, no log, the worst kind of failure to track down because nothing says it happened. Caught in review and closed the same day: BuildConfig (and so Render, which calls it) now refuses outright when cluster.Spec.Registries is non-empty, naming both BuildConfigWithRegistryAuth and RenderWithRegistryAuth in the error so the caller is told which one to use instead of just that it chose wrong. TestRenderRefusesRegistriesWithoutAuthPath covers both BuildConfig and Render. This turns "silently wrong" into "loudly wrong," which is the entire point -- it does not turn "two entry points" into "one"; see Consequences below for why that reduction is internal/controller's to make, not this ADR's.

The injected document is built directly from machinery's types, not through a TalosConfigPatch. The password-shaped fields have no mirror-type representation at all -- that is the entire reason authSecretRef exists -- so there is no typedconfig document kind this could go through even if it wanted to. injectRegistries constructs a *v1alpha1.Config (machinery's own type) directly and merges it via the same configpatcher.Apply strategic merge every TalosConfigPatch already goes through, placed in the pipeline immediately after baseConfig and before SelectPatches/applyPatches -- the system layer, lowest in the stack, exactly where "### Monitoring and registry mirrors" puts it.

A missing Secret or a Secret with none of the three recognized keys fails the render. Silently producing an anonymous mirror for a registry that named a Secret would look like a working configuration until the first pull fails on the node, with nothing in Ichor's own output pointing at why -- the same "fail loudly, not silently" requirement loadBundle already follows for the secrets bundle.

Consequences

• TalosCluster.spec.registries[] uses +listType=map / +listMapKey=host for host uniqueness, not a CEL x-kubernetes-validations rule. A self.exists_one uniqueness rule was tried first and rejected by envtest's real apiserver: with MaxItems=32 its estimated cost exceeded the per-field budget by 32x (the exists_one macro is quadratic in list length) and still contributed enough to blow the whole-schema budget (this CRD already carries CEL rules elsewhere) by 3.3x. The native listType: map structural-schema mechanism -- the same one status.conditions already uses on metav1.Condition's type key -- enforces the same uniqueness at zero estimated CEL cost, confirmed by the same envtest apiserver accepting the CRD and rejecting a duplicate-host Create in api/v1alpha1/validation_test.go.
• Resolved credentials are held only in a short-lived map[string]RegistryAuth in memory, handed from ResolveRegistryAuth to Render/BuildConfig and never written anywhere else -- not a CR's spec or status, not a log line, not an error string, not ichor gitops export's output (which only ever marshals the TalosCluster object itself, and that object never carries a resolved value, only authSecretRef's name). Covered by TestRegistryAuthLeak (every forced error path) following TestRenderBundleLeak's existing discipline for the secrets bundle.
• The rendered config's hash changes when a resolved credential changes, same as any other input -- correct, since the node needs the new config re-applied. The hash cannot be reversed to recover the credential (pkg/render's Hash is a one-way sha256); this was flagged as not needing a test, since it follows from using a cryptographic hash and not from anything this change wrote.
• internal/controller/diff.go's ChangedPaths reports only field-path strings (it walks a decoded YAML tree and appends the prefix on a mismatch, never the leaf values themselves), read but not modified for this change. TestRegistryAuthDriftPathsNeverLeakValue (pkg/render/registry_drift_test.go, an external render_test package so it can import internal/controller without an import cycle through pkg/render) renders the same machine twice with two different resolved passwords and confirms ChangedPaths reports a path naming machine.registries.config.<host>.auth.password and never either password's value -- confirming TalosMachine.status (which only ever carries ChangedPaths's output, never the rendered config) cannot carry a registry credential either.
• Debt (closed, see the amendment below): BuildConfig/Render and the *WithRegistryAuth pair were two entry points.
• Debt: this feature is built on a field machinery itself marks deprecated. injectRegistries nests its output under the legacy v1alpha1.Config.MachineConfig.MachineRegistries (//nolint:staticcheck suppresses the SA1019 warning at the call site), not under the modern standalone RegistryMirrorConfig/ RegistryAuthConfig documents (package config/types/cri) machinery's own doc comment points to instead. That was the right call for this change -- the modern documents are not in Ichor's mirror-type allow-list, and adding them is a mirrorgen-root-sized change (ADR 0012's shape), not a render-time-injection-sized one -- but it means this feature depends on a deprecated field today, not merely declines to use a nicer one. Migrating injectRegistries (and, if the allow-list grows to include them, TalosConfigPatch authors generally) onto the standalone documents is follow-up work this ADR creates, not a closed question; until it happens, a future machinery release that actually removes the deprecated field breaks this feature, not just produces a louder lint warning.
• Out of scope, same as the removal that produced this follow-up: VIPEquinixMetalConfig.apiToken, VIPHCloudConfig.apiToken, WireguardDevice.privateKey stay dropped with no replacement -- nothing in any current phase uses them. ichor images list and mirror-reachability checks (also air-gap, but not this ADR's slice of it) are untouched.

Alternatives considered

A validating admission webhook rejecting a credential-bearing patch. Not applicable here -- the whole point is that the mirror type cannot carry the credential at all, so there is nothing for a patch to smuggle through in the first place. (This is the same reasoning ADR 0003 already gave for not adding a webhook over its nine dropped secret fields.)

Three SecretKeySelector fields (passwordSecretRef, authSecretRef, identityTokenSecretRef) instead of one LocalObjectReference. More flexible -- each credential could live in a different Secret -- but no real use case needs that, it triples the CRD surface for this one struct, and it breaks the parallel with secretsRef's existing one-reference-fixed-keys shape for no benefit this phase needs.

A CEL rule for registries[].host uniqueness. Tried, measured, rejected -- see Consequences above.

Amendment (2026-10-26): one entry point; every production caller was still on the refusing one

The Consequences bullet above tracked the Render/*WithRegistryAuth split as debt to be paid "once TalosMachineReconciler is wired". It never was: all five production call sites (TalosMachineReconciler.Reconcile, internal/uiapi's effective-config view, ichor init, and ichor adopt's two renders) still called the plain pair, and the refusal guard added so a forgotten call would be loud meant that any TalosCluster with spec.registries set, even an anonymous mirror, could not be reconciled at all. ResolveRegistryAuth and the *WithRegistryAuth functions had no non-test caller. A new envtest spec ("reconciles a TalosCluster that declares registries...") failed first with RenderFailed / cluster.Spec.Registries is set; BuildConfig never resolves or injects registry auth, then passed after the fix.

The cause was having two entry points, so the fix removes one. Render and BuildConfig now take auth map[string]RegistryAuth as a required last argument; RenderWithRegistryAuth, BuildConfigWithRegistryAuth and the refusal guard are deleted. A caller that forgets to resolve auth is now a compile error rather than a runtime refusal. ResolveRegistryAuth returns a nil map without reading anything (and without requiring a namespace) when the cluster declares no registries, so the common path costs nothing and needs no extra RBAC.

Each caller resolves the Secret in a fixed namespace, never cluster.Namespace (ADR 0011; the mistake this repository has made twice, see ADR 0018's amendment):

• Controller: r.ControllerNamespace, the namespace loadBundle uses.
• UI backend: Server.ControllerNamespace, the namespace its loadBundle uses.
• ichor adopt: ichorControllerNamespace (ichor-system), where the install manifest puts the controller and so where it reads authSecretRef Secrets, rather than adopt's --namespace (which is where the TalosCluster lives).
• ichor init: builds the TalosCluster itself, has no flag for spec.registries, and has no management-cluster client yet. It passes nil and refuses (rather than rendering an unauthenticated mirror) if the plan ever carries registries.

RBAC needed no change: the controller's Role in ichor-system already has get;list;watch on Secrets and the UI backend's has get (both namespaced, the same grants loadBundle relies on); no cluster-wide Secret read was added.