stacklok / toolhive / 24833483611

67%

Build Type

push

github

Committed by web-flow

Commit Message

Introduce MutateAndPatchSpec and adopt across spec-patch sites (#5004)

* Introduce MutateAndPatchSpec and adopt across spec-patch sites

The inline DeepCopy + Patch(MergeFromWithOptimisticLock) pattern from
#4914 landed at five MCPServer spec-write sites, each carrying the same
four-line rationale comment. Extract it into a MutateAndPatchSpec[T]
generic helper in cmd/thv-operator/pkg/controllerutil, siblinged with
the existing MutateAndPatchStatus.

The helper mirrors its sibling exactly -- same reflection-based
nil-guard, same DeepCopy-then-mutate-then-Patch flow -- with two
deliberate differences:

  - Uses MergeFromWithOptions(original, MergeFromWithOptimisticLock{})
    so concurrent writers get 409-and-requeue instead of silent clobber.
    This is the property that defends spec.authzConfig, which the
    forthcoming authorization controller will own, from being zeroed on
    every reconcile.
  - No no-op short-circuit. MergeFromWithOptimisticLock always emits
    metadata.resourceVersion into the body, so the status helper's
    "body == {}" check never fires; and every current spec call site
    carries a real mutation.

All five inline sites (mcpserver_controller.go finalizer add/remove and
restart-annotation stamp; toolconfig_controller.go and
mcpexternalauthconfig_controller.go config-hash stamps) adopt the
helper. Pure refactor at the call sites -- no behavior change.

Tests mirror the status helper's shape: happy-path + optimistic-lock
wire signal, DeepCopy isolation, 409 Conflict propagation, nil-obj
rejection, and disjoint-spec-field preservation (the regression guard
that would fire if the helper were ever swapped back to r.Update).
Existing TestMCPServerSpecPatchesAreOptimisticLock and the
mcpserver_spec_patch_integration_test.go envtest remain green.

The operator-rules section on spec/metadata patching is updated to
reference the helper as the canonical pattern.

* Address MoE review feedback on MutateAndPatchSpec

Three of five rev... (continued)

Coverage Stats

35 of 39 new or added lines in 4 files covered. (89.74%)

35 existing lines in 4 files now uncovered.

58849 of 88730 relevant lines covered (66.32%)

62.45 hits per line