stacklok / toolhive / 24749517883

66%

Build Type

push

github

Committed by web-flow

Commit Message

Introduce MutateAndPatchStatus helper for status writes (#4942)

Relates to #4633. A shared helper that collapses the
"DeepCopy → mutate → r.Status().Patch(MergeFrom(original))" idiom
to a single call so remaining r.Status().Update sites can migrate
without each one re-implementing the DeepCopy-before-mutate
discipline by hand.

Status writes deliberately use a plain merge patch, not an
optimistic-lock one: the operator and the runtime reporter write
disjoint status fields on every reconcile and must coexist without
forcing a 409 on every overlap. Spec and metadata writes still
require optimistic locking — see #4767 (tracking) / #4914
(MCPServer migration).

The helper does not make every multi-writer pattern safe. The
Caller contract in the doc comment spells out two footguns it
cannot defend against:

- JSON merge-patch replaces arrays wholesale for CRDs, so a writer
  to Status.Conditions must be the sole owner of the entire array.
  Any concurrent writer whose Patch lands between this caller's
  Get and Patch — on any condition type, including ones this
  caller does not touch — will be erased. A fresh Get narrows but
  does not eliminate the TOCTOU window.
- A scalar re-computed from a stale snapshot that differs from the
  live value will overwrite a concurrent writer's update.

The codified checklist for new call sites lives in
.claude/rules/operator.md.

Operational safeguards in the helper itself:

- No-op mutations (empty merge-patch body) short-circuit before
  the wire call; the apiserver runs admission and audit for every
  PATCH regardless of body content, so steady-state reconcilers
  must not generate {} traffic.
- A nil obj returns a descriptive error rather than panicking in
  the downstream type assertion.

The helper lives in cmd/thv-operator/pkg/controllerutil alongside
the existing controller helpers. It may move to a shared location
later if a non-operator caller needs it.

Pure addition — no call-site changes in this PR.

Tests... (continued)

Coverage Stats

18 of 20 new or added lines in 1 file covered. (90.0%)

15 existing lines in 3 files now uncovered.

58564 of 88463 relevant lines covered (66.2%)

62.5 hits per line