0035. PatchService: the config-editing backend

Status: Accepted · Date: 2026-10-26 · Design section: CLI and UI, Patch layers

Decided: built PatchService (CRUD for TalosConfigPatch plus PreviewImpact) exactly as ADR 0034's gap report proposed it, with the opaque-JSON PatchDocument shape that avoids re-modeling Talos fields a third time, and with no new RPC for schema-driven forms since the CRD's own committed OpenAPI schema already serves that purpose.

Context

ADR 0034 found phase 6's "config editing by layer ... impact preview before applying ... schema-driven forms" (architecture.md, "CLI and UI") blocked on three missing pieces of API surface, and reported concrete shapes for each rather than building against nothing. This unit builds exactly those shapes.

Decision

PatchService: CRUD mirroring MachineService's own shape

ListPatches/GetPatch/CreatePatch/UpdatePatch/DeletePatch, same pattern MachineService already established (ADR 0016): ListPatches optionally filters to one cluster, exactly like ListMachinesRequest. CreatePatch and UpdatePatch are both full-replace -- no partial-field merge semantics to get wrong -- and both require patch.ref.namespace/ patch.ref.name set by the caller; this service never generates a name. DeletePatch actually deletes, unlike RejectMachine: a rejected machine must never be deleted (ADR 0018, so discovery can't recreate it as fresh), but a patch has no analogous "it would come back" concern, so deleting it is the whole operation "remove this patch" means.

The layer concept is not reduced. Patch.layer reuses ConfigLayer, the same enum GetEffectiveConfig already returns per value (node.proto) -- one enum, not two, for the one concept. PatchTarget mirrors TalosConfigPatchTarget with all three of its forms, including LabelSelector's matchExpressions, not only matchLabels: modeling only the common case would have been a real capability regression against what the CRD already accepts directly. LabelSelector/ LabelSelectorRequirement are new proto messages, but they mirror metav1.LabelSelector -- Kubernetes' own well-known, finite API shape, not a Talos field this project would otherwise catalogue a second time, so modeling it fully costs little and is not the parallel-catalogue problem ADR 0003 exists to avoid.

Every structural rule the CRD's own +kubebuilder:validation:XValidation rules already state -- layer/target consistency, exactly-one-of on target, exactly-one-of on a document's kind-matching field -- is enforced there, on the write itself, via the real Kubernetes API (s.Create/s.Update), not re-implemented in Go. mapWriteError (patch_service.go) turns apierrors.IsInvalid into connect.CodeInvalidArgument, carrying the CRD's own CEL message verbatim -- already human-readable (ADR 0018's own refusal messages set this precedent), so there is nothing to re-word.

PatchDocument.document is opaque JSON, not a third catalogue

patch.proto's own comment states the reasoning ADR 0034 flagged as the one real risk of building this unit at all: modeling every typed-config field a third time (Go mirror type, CRD schema, now protobuf) would be exactly the parallel catalogue ADR 0003's generator exists to avoid. PatchDocument{kind, document} carries document as plain JSON, shaped exactly as TalosConfigPatchDocument's matching Go field would unmarshal it. documentFieldKey (convert.go) is the one place that maps a kind string to its JSON field name -- six entries, the same allow-list render.DocumentFromPatch's own switch already enumerates, not a second one -- and documentFromPB/documentToPB wrap/unwrap the raw JSON under that key so encoding/json fills (or reads) the real typed field itself. No field of any of the six kinds is named, checked or round-tripped by hand anywhere in this unit.

PreviewImpact: every path the patch sets, not a diff

PreviewImpact takes a proposed Patch (new, via CreatePatch's own shape, or an edit of an existing one), never writes anything, and returns pkg/fieldmeta.Assess's three fields unchanged: reboot_required bool, install_time_only []string, unsupported []string. It classifies every field path the patch's own documents set, not a diff against whatever is currently applied: computing a true path-level diff between an existing patch's documents and a proposed edit is real additional work this unit does not attempt, and pkg/fieldmeta's own doc comment already states the safe direction when exact information isn't available -- showing impact for a field even if an edit happens to leave it unchanged over-reports rather than under-reports, which is the direction that can't leave a node running a configuration it doesn't have.

install_time_only and reboot_required are two different response fields, not two severities of one signal, per the task's own instruction: a reboot does nothing for a field Talos only reads at install/imaging time (pkg/fieldmeta's own doc comment), so a caller presenting them identically would let an operator pick "reboot" for a change nothing short of reset-and-reinstall actually applies.

documentLeafPaths (effectiveconfig.go, renamed from an anonymous block inside patchOrigins) is the one place that renders one document in isolation and flattens it to field paths; patchOrigins (origin attribution for GetEffectiveConfig) and patchPaths (patch_service.go, impact classification for PreviewImpact) both call it rather than each re-implementing "what paths does this one document set."

Schema-driven forms: no new RPC, confirmed rather than assumed

ADR 0034 proposed this and nothing here contradicts it: config/crd/ bases/ichor.maesh.dev_talosconfigpatches.yaml's OpenAPI schema is already committed and already generated from the exact mirror types documentFieldKey maps into. A form renderer reads that file as a static asset -- no live cluster state in it, so serving it doesn't cross "browser talks only to the backend" either way -- and submits whatever JSON it produces as PatchDocument.document. Still not built in this unit (the task's own boundary: "UI 側の画面はこのユニットに含めません"); the backend this form would call now exists.

Strict field validation and RBAC

CreatePatch and UpdatePatch both pass strictFieldValidation (machine_service.go's existing package-level var) to s.Create/ s.Update -- the same discipline every other write in this package already has, not a new one invented for this service. internal/uiapi/strictvalidation_test.go's recordingUpdateClient gained a Create override (it already had Update) so a test can assert CreatePatch's own write requests it the same way the existing ApproveMachine/RejectMachine tests do for Update.

internal/uiapi/rbac_markers.go's talosconfigpatches line moved from get;list (read-only, ADR 0018) to get;list;create;update;delete -- the one CRD this package now actually creates and deletes, not only updates in place the way talosmachines is. talosclusters stays get;list only: PreviewImpact reads a TalosCluster for its version contract, it never writes one.

No masking concern

TalosConfigPatch's typed-config mirror types never carry a secret field at all -- ADR 0003's generator drops the eleven meta.Unstructured escape-hatch fields and the nine secret-bearing scalars from the mirror types entirely, not just from what gets rendered. There is nothing for this service to mask, because there is nothing secret-shaped in a TalosConfigPatch to begin with; the task's "don't build an unmask mechanism" instruction has no mechanism to not-build here.

Consequences

• proto/ichor/v1alpha1/patch.proto is new; buf lint and buf breaking both pass clean against it (every addition, no removal or change to an existing file).
• internal/uiapi/patch_service.go (the five CRUD methods plus PreviewImpact), internal/uiapi/convert.go (the wire/CRD conversions), and internal/uiapi/effectiveconfig.go (the documentLeafPaths extraction) are the implementation; internal/uiapi/patch_service_test.go covers CRUD round-tripping, a CRD-rejected write surfacing as CodeInvalidArgument, strict field validation on both writes, and PreviewImpact's three outcomes (none, unsupported, install-time-only kept separate from reboot-required).
• cmd/ichor-ui/main.go registers PatchServiceHandler on the same mux as the other three services.
• ui/ gained only its generated TS client (ui/src/gen/ichor/v1alpha1/ patch_pb.ts, committed per ADR 0021's existing policy); no screen in this unit, per the task's own boundary.
• internal/controller/, internal/cli/, pkg/ (beyond reading pkg/fieldmeta) and proto/'s other files are untouched.