stacklok / toolhive / 26239810132

66%

Build Type

push

github

Committed by web-flow

Commit Message

Wire OBO dispatch arms and reconciler branch (#5345)

* Wire OBO dispatch arms and reconciler branch

Implements changes for issue #5328:
- Replace the placeholder OBO arms in AddExternalAuthConfigOptions and
  getExternalAuthConfigSecretEnvVar with calls into the registered OBO
  handler (OBOApplyRunConfig / OBOSecretEnvVars). In upstream-only
  builds the default handler returns obo.ErrEnterpriseRequired, so the
  dispatch never falls through to the default arm's generic
  "unsupported external auth type" error.
- Add an OBO branch to MCPExternalAuthConfigReconciler.Reconcile that
  calls controllerutil.OBOValidate after the existing Validate()
  succeeds. errors.Is(err, obo.ErrEnterpriseRequired) maps to
  Reason=EnterpriseRequired; any other error maps to
  Reason=InvalidConfig. The branch writes status via
  controllerutil.MutateAndPatchStatus per the operator status-write
  convention; the existing r.Status().Update call sites are untouched
  (refactor is out of scope for this task).
- Cover each dispatch arm with a fake-client unit test that asserts
  errors.Is matches obo.ErrEnterpriseRequired and that no
  "unsupported external auth type" string leaks. A table-driven
  setInvalid test verifies the reason-string selection handles the
  sentinel directly, the sentinel through a fmt.Errorf wrap (the
  vMCP converter path), and any other error.

* Address code review feedback on OBO dispatch wiring

Fixed issues from code review:
- CRITICAL: setInvalid now re-fetches the object via r.Get and re-applies
  applyIdentitySynthesizedCondition inside the MutateAndPatchStatus
  closure. Previously the helper diffed against an already-mutated
  snapshot, so a user switching a config from embeddedAuthServer to obo
  would leave a stale IdentitySynthesized=True condition lingering on
  the server. Regression test
  TestMCPExternalAuthConfigReconciler_OBO_ClearsStaleIdentitySynthesized
  constructs that scenario and asserts the stale condition is removed
  ... (continued)

Coverage Stats

59 of 69 new or added lines in 5 files covered. (85.51%)

8 existing lines in 3 files now uncovered.

64919 of 98727 relevant lines covered (65.76%)

63.42 hits per line