Skip to content

Admit obo in MCPExternalAuthConfig CRD enum and regenerate manifests #5329

@tgrunnagle

Description

@tgrunnagle

Description

Light up apiserver-level admission of obo-typed MCPExternalAuthConfig resources. Append ;obo to the kubebuilder enum marker, declare an opaque OBOConfig placeholder struct + OBO *OBOConfig spec field, add a CEL rule asserting the field is set iff the type is obo, add the matching defense-in-depth check inside validateTypeConfigConsistency, add a structural-only no-op arm to the Validate() switch, and regenerate both the operator CRD chart YAML and the public CRD reference docs.

This is the moment the feature becomes user-visible. Until this task merges, the apiserver enum on MCPExternalAuthConfig.spec.type does not list obo, so kubectl apply of an obo-typed CR is rejected by admission before any of the previously-landed dispatch wiring runs. After this task merges, the apiserver admits the CR, the reconciler routes it through the function-pointer hook, and an upstream-only build surfaces status.conditions[Valid] = False / Reason: EnterpriseRequired end-to-end.

Context

This is the fourth and final flat technical task under the parent OSS story. The parent establishes the function-pointer override hook design for a new obo external auth type. The earlier tasks landed dark infrastructure: the Go constant ExternalAuthTypeOBO, the default oboHandler returning ErrEnterpriseRequired, the new pkg/auth/obo/ package with RegisterFactory, the OBOConverterStub registered in NewRegistry(), the two operator-side switch arms, and the reconcile-loop branch that maps ErrEnterpriseRequired to Reason: EnterpriseRequired. None of that activated user-facing behavior because the CRD enum did not admit obo.

This task closes the loop with a contained, surgical edit to one Go source file plus regenerated manifests:

  1. Append ;obo to the +kubebuilder:validation:Enum marker on MCPExternalAuthConfigSpec.Type (existing marker lists seven values).
  2. Declare an opaque OBOConfig placeholder struct at the top level of the file (near the other *Config structs). The inner schema is intentionally empty in this revision; sub-fields land in a follow-up.
  3. Add OBO *OBOConfig to MCPExternalAuthConfigSpec with the standard json:"obo,omitempty" tag (alongside the existing seven optional config fields).
  4. Add a CEL XValidation rule asserting self.type == 'obo' ? has(self.obo) : !has(self.obo).
  5. Add the defense-in-depth check inside validateTypeConfigConsistency (file line 1050) so stored objects that bypass CEL still fail Go-level validation. Match the shape of the existing six type-config-consistency checks.
  6. Add a case ExternalAuthTypeOBO no-op arm to the Validate() switch at file line 1019. Structural validation only — semantic validation runs at reconcile time via the OBOValidate function-pointer hook from the dependency tasks.
  7. Regenerate deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml via task operator-manifests.
  8. Regenerate the public CRD reference docs via task crdref-gen (run from cmd/thv-operator/).

Dependencies: #5325 (parent story), #5327 (the Go constant ExternalAuthTypeOBO must already exist; the default oboHandler returns ErrEnterpriseRequired so consumers see a clear status condition the moment the CRD admits obo), #5328 (the dispatch arms and reconciler branch must already be in place; otherwise admitting obo at the apiserver level would produce a generic "unsupported external auth type" error from a default: switch arm).
Blocks: nothing — this is the last OSS task by design. It is what makes the feature user-visible.

Acceptance Criteria

Code surface:

  • In cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go, the +kubebuilder:validation:Enum marker on MCPExternalAuthConfigSpec.Type (currently tokenExchange;headerInjection;bearerToken;unauthenticated;embeddedAuthServer;awsSts;upstreamInject) is extended with ;obo. The other seven enum values are byte-identical to before.
  • A new top-level type OBOConfig struct{} is declared in the same file with a doc comment explaining it is an intentionally-empty placeholder whose sub-fields land in a follow-up.
  • MCPExternalAuthConfigSpec gains a new field OBO *OBOConfig with the standard json:"obo,omitempty" tag and a +optional kubebuilder marker, placed alongside the existing seven optional config-type fields (TokenExchange, HeaderInjection, BearerToken, EmbeddedAuthServer, AWSSts, UpstreamInject). The other seven fields are byte-identical to before.
  • A new +kubebuilder:validation:XValidation rule appears alongside the existing seven rules on MCPExternalAuthConfigSpec: rule="self.type == 'obo' ? has(self.obo) : !has(self.obo)", message="obo configuration must be set if and only if type is 'obo'". The existing seven CEL rules are byte-identical to before.
  • validateTypeConfigConsistency (file line 1050) gains a new check asserting (r.Spec.OBO == nil) == (r.Spec.Type == ExternalAuthTypeOBO) is false (i.e., OBOConfig present iff type is obo), matching the shape of the existing six type/config-pair checks. The unauthenticated guard at the bottom of the function is updated to also reject r.Spec.OBO != nil.
  • The Validate() switch at file line 1019 gains a new no-op arm: case ExternalAuthTypeOBO: return nil. The arm is placed before the default: and is documented with a comment explaining the two-tier validation pattern (see the Technical Approach section for the exact wording).
  • The regenerated deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml (produced by task operator-manifests) contains obo in the enum list on spec.type, the new CEL XValidation rule, and an obo object property under spec.
  • The regenerated public CRD reference (produced by task crdref-gen from cmd/thv-operator/) documents the new obo enum value and the empty OBOConfig placeholder struct, including a note that sub-fields land in a follow-up.

Behavior:

  • kubectl explain mcpexternalauthconfig.spec.type lists obo as a valid enum value with a description noting it requires a build with a registered OBO handler.
  • MCPExternalAuthConfig.spec.type admits obo as a valid enum value at the apiserver level after the regenerated CRD YAML is applied to a cluster.
  • The CEL rule rejects an obo-typed config that omits the obo: field at admission time. The CEL rule rejects a non-obo-typed config that sets the obo: field at admission time.
  • validateTypeConfigConsistency returns an error if Spec.Type == ExternalAuthTypeOBO but Spec.OBO == nil. It also returns an error if Spec.OBO != nil but Spec.Type != ExternalAuthTypeOBO.
  • Validate() returns nil for an obo-typed config that passes validateTypeConfigConsistency (i.e., structural validation succeeds; the new switch arm is a no-op).
  • End-to-end on an upstream-only build: applying an obo-typed MCPExternalAuthConfig to a cluster running the upstream operator binary succeeds at the apiserver level (for the first time), is picked up by the reconciler, routes through controllerutil.OBOValidate, and transitions the resource to status.conditions[Valid] = False / Reason: EnterpriseRequired via the dispatch wiring from the prior task.
  • No regression: every existing test in cmd/thv-operator/test-integration/mcp-external-auth/ passes unchanged. Every existing CEL rule still fires for the seven pre-existing types.

Hygiene:

  • No new lint findings; task lint-fix succeeds; gofmt and goimports are clean.
  • All tests pass via task test.
  • Code reviewed and approved.

Technical Approach

Recommended Implementation

(a) Append ;obo to the kubebuilder enum marker

In cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go, locate the marker on MCPExternalAuthConfigSpec.Type (currently around the lines 17-41 area, on the Type ExternalAuthType field declaration). Today it reads:

// +kubebuilder:validation:Enum=tokenExchange;headerInjection;bearerToken;unauthenticated;embeddedAuthServer;awsSts;upstreamInject
// +kubebuilder:validation:Required
Type ExternalAuthType `json:"type"`

Append ;obo:

// +kubebuilder:validation:Enum=tokenExchange;headerInjection;bearerToken;unauthenticated;embeddedAuthServer;awsSts;upstreamInject;obo
// +kubebuilder:validation:Required
Type ExternalAuthType `json:"type"`

(b) Declare the OBOConfig placeholder struct

Add the type alongside the other *Config structs in the same file:

// OBOConfig is a placeholder for On-Behalf-Of (OBO) external auth configuration.
// The inner schema is intentionally empty in this revision; sub-fields land in a
// follow-up. The struct exists today so that the CRD schema admits `spec.obo: {}`
// (matching the CEL rule "obo field must be set iff type is obo") and so that
// downstream tools that introspect the API surface can see the placeholder
// before the protocol-level fields land.
type OBOConfig struct{}

(c) Add the OBO field on the spec

Add a new optional field on MCPExternalAuthConfigSpec, alongside the existing seven optional config-type fields (TokenExchange, HeaderInjection, BearerToken, EmbeddedAuthServer, AWSSts, UpstreamInject):

// OBO configures On-Behalf-Of (OBO) authentication.
// Only used when Type is "obo". The inner schema is intentionally empty in
// this revision; sub-fields land in a follow-up. Setting this field on an
// upstream-only build will cause the MCPExternalAuthConfig to transition to
// status.conditions[Valid] = False with Reason: EnterpriseRequired.
// +optional
OBO *OBOConfig `json:"obo,omitempty"`

(d) Add the CEL rule

Add one new +kubebuilder:validation:XValidation rule on MCPExternalAuthConfigSpec alongside the existing seven CEL rules. The shape matches the existing per-type "config must be set if and only if type is X" rules:

// +kubebuilder:validation:XValidation:rule="self.type == 'obo' ? has(self.obo) : !has(self.obo)",message="obo configuration must be set if and only if type is 'obo'"

Also extend the existing unauthenticated CEL rule to mention self.obo so the "no configuration set when type is unauthenticated" guard still holds for OBO. Today the rule reads:

self.type == 'unauthenticated' ? (!has(self.tokenExchange) && !has(self.headerInjection) && !has(self.bearerToken) && !has(self.embeddedAuthServer) && !has(self.awsSts) && !has(self.upstreamInject)) : true

Extend it to include && !has(self.obo):

self.type == 'unauthenticated' ? (!has(self.tokenExchange) && !has(self.headerInjection) && !has(self.bearerToken) && !has(self.embeddedAuthServer) && !has(self.awsSts) && !has(self.upstreamInject) && !has(self.obo)) : true

(e) Add the defense-in-depth check to validateTypeConfigConsistency

The existing function (file line 1050) uses a sequence of bivariant if checks of the shape if (r.Spec.X == nil) == (r.Spec.Type == ExternalAuthTypeX) { return error }, one per type. Add a parallel check for OBO matching that shape:

if (r.Spec.OBO == nil) == (r.Spec.Type == ExternalAuthTypeOBO) {
    return fmt.Errorf("obo configuration must be set if and only if type is 'obo'")
}

Also extend the unauthenticated guard at the bottom of the function to reject r.Spec.OBO != nil:

if r.Spec.Type == ExternalAuthTypeUnauthenticated {
    if r.Spec.TokenExchange != nil ||
        r.Spec.HeaderInjection != nil ||
        r.Spec.BearerToken != nil ||
        r.Spec.EmbeddedAuthServer != nil ||
        r.Spec.AWSSts != nil ||
        r.Spec.UpstreamInject != nil ||
        r.Spec.OBO != nil {
        return fmt.Errorf("no configuration must be set when type is 'unauthenticated'")
    }
}

(f) Add the no-op arm to Validate()

The Validate() switch at file line 1019 dispatches on r.Spec.Type and either runs complex per-type validation (validateEmbeddedAuthServer, validateAWSSts, the inline upstreamInject ProviderName check) or returns nil for the four simple types (tokenExchange, headerInjection, bearerToken, unauthenticated). Add a new arm for OBO that returns nil, placed alongside the four-type group:

case ExternalAuthTypeOBO:
    // OBO's Validate() case is a no-op because structural validation
    // (via validateTypeConfigConsistency above) is sufficient at this layer;
    // semantic validation (e.g., does the cluster have an OBO handler
    // registered?) runs at reconcile time via the function-pointer hook
    // controllerutil.OBOValidate. See the two-tier validation note in the
    // accompanying issue.
    return nil

Two-tier validation explanation

OBO's Validate() case is a no-op because structural validation is sufficient — semantic validation (e.g., does the cluster have an OBO handler registered?) runs at reconcile time via the OBOValidate function-pointer hook. This is the first ExternalAuthType where the two tiers diverge; existing types either accept (tokenExchange, headerInjection, bearerToken, unauthenticated) or run complex validation (embeddedAuthServer, awsSts, upstreamInject).

This is the answer to the reviewer question "why no validation here?" before they ask. The semantic check lives in the reconciler (added in the dispatch-wiring task), which calls controllerutil.OBOValidate(cfg). With the default upstream stub, that call returns ErrEnterpriseRequired, and the reconciler maps the sentinel to status.conditions[Valid] = False / Reason: EnterpriseRequired. An out-of-tree build that registers a real handler via controllerutil.RegisterOBOHandler short-circuits the sentinel and runs whatever protocol-level validation it needs (IdP reachability, audience configuration, etc.). Splitting the structural and semantic tiers this way is what keeps the upstream CRD schema stable across out-of-tree builds — anything build-specific lives behind the function-pointer hook, not in the CRD.

(g) Regenerate the operator CRD chart YAML

Run task operator-manifests from the repository root (per the operator conventions). This regenerates deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml. The regenerated YAML must include:

  • obo in the enum: list under spec.type (alongside the existing seven values).
  • A new obo: {description: ..., type: object, properties: {}} entry under spec.properties (empty object schema, matching the empty OBOConfig struct).
  • A new x-kubernetes-validations entry whose rule is self.type == 'obo' ? has(self.obo) : !has(self.obo).
  • The updated unauthenticated CEL rule that also negates self.obo.

(h) Regenerate the public CRD reference

Run task crdref-gen from cmd/thv-operator/ (the task uses relative paths). This regenerates the markdown API reference. The regenerated reference must:

  • Document obo as a valid value for MCPExternalAuthConfig.spec.type.
  • Document the new OBOConfig struct as an empty placeholder with a note that sub-fields land in a follow-up.
  • Document the new spec.obo field on MCPExternalAuthConfigSpec.

Patterns & Frameworks

  • Kubebuilder enum + CEL pair: every CRD type-enum value in this file is paired with (1) an +kubebuilder:validation:Enum entry, (2) a +kubebuilder:validation:XValidation rule asserting the matching *Config field is set iff the type matches, and (3) a defense-in-depth check inside validateTypeConfigConsistency. Follow that pattern exactly. CEL catches issues at admission time; the Go check catches stored objects that predate the CEL rule.
  • Two-tier validation: structural validation in Validate() + CRD-schema/CEL rules; semantic validation behind a function-pointer hook called from the reconciler. The split is what keeps the upstream CRD stable for out-of-tree builds that register a real handler.
  • Operator manifest regeneration: per the operator conventions, task operator-manifests is run after any kubebuilder marker change. The regenerated YAML is committed alongside the Go change in the same PR — never split across PRs.
  • Public CRD reference regeneration: per the operator conventions, task crdref-gen is run from cmd/thv-operator/ after CRD changes. The regenerated markdown is committed alongside the Go change in the same PR.

Code Pointers

(All anchored to upstream toolhive v0.27.2 / 97b0cc3f.)

  • cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go:17-41 — block of existing ExternalAuthType constants. ExternalAuthTypeOBO was added here by Add default OBO handler hooks and vMCP/proxy converter stubs #5327; no change in this task.
  • cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go (around the MCPExternalAuthConfigSpec declaration, ~lines 47-100) — the existing +kubebuilder:validation:XValidation CEL rules and the +kubebuilder:validation:Enum=... marker live here. The new CEL rule and the appended enum value land here; the new OBO *OBOConfig field is appended alongside the seven existing optional *Config fields.
  • cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go:1019Validate() switch. New no-op case ExternalAuthTypeOBO arm added here.
  • cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go:1050validateTypeConfigConsistency. New bivariant if check added here; the bottom unauthenticated guard is extended.
  • deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml — regenerated by task operator-manifests. Commit the diff alongside the Go change.

Component Interfaces

This task does not introduce any new exported functions — only an empty struct and a new spec field:

// In cmd/thv-operator/api/v1beta1:

// New placeholder type (intentionally empty).
type OBOConfig struct{}

// New field on the existing spec (alongside the seven existing optional configs).
type MCPExternalAuthConfigSpec struct {
    // ...existing fields and CEL rules...

    // +kubebuilder:validation:XValidation:rule="self.type == 'obo' ? has(self.obo) : !has(self.obo)",message="obo configuration must be set if and only if type is 'obo'"
    // +kubebuilder:validation:Enum=tokenExchange;headerInjection;bearerToken;unauthenticated;embeddedAuthServer;awsSts;upstreamInject;obo
    Type ExternalAuthType `json:"type"`

    // ...other existing optional fields...

    // OBO configures On-Behalf-Of (OBO) authentication.
    // +optional
    OBO *OBOConfig `json:"obo,omitempty"`
}

Testing Strategy

Unit tests — in cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types_test.go (existing file; extend it alongside the existing per-type cases):

  • MCPExternalAuthConfig{Spec: {Type: ExternalAuthTypeOBO, OBO: &OBOConfig{}}}.Validate() returns nil.
  • MCPExternalAuthConfig{Spec: {Type: ExternalAuthTypeOBO, OBO: nil}}.Validate() returns a non-nil error whose message mentions "obo configuration must be set".
  • MCPExternalAuthConfig{Spec: {Type: ExternalAuthTypeTokenExchange, OBO: &OBOConfig{}}}.Validate() returns a non-nil error (OBO set but type is not obo).
  • MCPExternalAuthConfig{Spec: {Type: ExternalAuthTypeUnauthenticated, OBO: &OBOConfig{}}}.Validate() returns a non-nil error whose message mentions "no configuration must be set when type is 'unauthenticated'".
  • Existing per-type validation tests for the other seven ExternalAuthType values still pass unchanged.

Integration tests — none net new; the previous task added obo-specific integration tests in cmd/thv-operator/test-integration/mcp-external-auth/ and used a workaround (envtest looser validation, in-test CRD mutation, or direct-reconciler invocation) because the apiserver enum did not yet admit obo. After this task lands, those tests can switch to the standard kubectl apply path. The switch itself can be part of this task's test edits OR can be deferred — both are acceptable.

  • Verify every existing integration test in cmd/thv-operator/test-integration/mcp-external-auth/ still passes (regression).

Generated artifacts:

  • After running task operator-manifests, inspect deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml and confirm: (a) obo appears in the spec.type enum: list, (b) a new CEL XValidation rule with rule: self.type == 'obo' ? has(self.obo) : !has(self.obo) is present, (c) the existing unauthenticated CEL rule is updated to negate self.obo, (d) a new obo object property appears under spec.properties with type: object.
  • After running task crdref-gen from cmd/thv-operator/, inspect the regenerated markdown reference and confirm: (a) obo is listed as a valid value for spec.type, (b) the OBOConfig struct is documented as an empty placeholder with a follow-up note, (c) the spec.obo field is documented.

End-to-end (manual sanity, not gated):

  • In a test cluster with the upstream operator running and the regenerated CRD applied: kubectl explain mcpexternalauthconfig.spec.type lists obo. Applying an obo-typed MCPExternalAuthConfig succeeds at the apiserver. The resource transitions to Valid=False / EnterpriseRequired. Applying an obo-typed config without a spec.obo: {} body is rejected by the apiserver with the CEL message. Applying a non-obo typed config with spec.obo: {} is rejected by the apiserver with the CEL message.

Edge cases

  • Stored objects: confirm validateTypeConfigConsistency rejects an obo-typed config with Spec.OBO == nil even if it bypassed CEL (e.g., constructed in Go without going through admission). The Go-level defense-in-depth check is what catches this.
  • Empty struct serialization: OBOConfig struct{} serializes as {} in JSON/YAML. Confirm kubectl apply -f of a manifest containing spec.obo: {} succeeds (CEL has(self.obo) returns true for an empty object). A manifest that omits spec.obo entirely under an obo-typed config must be rejected by CEL.
  • Round-trip through task operator-generate (deepcopy): the empty OBOConfig struct round-trips correctly through deepcopy. Verify task operator-generate succeeds and the regenerated zz_generated.deepcopy.go includes a DeepCopyInto method for OBOConfig.

Out of Scope

  • The OBOConfig inner schema (sub-fields land in a follow-up RFC). This task lands the placeholder only.
  • Adding the Go constant ExternalAuthTypeOBO (already landed in the default-handler-infrastructure task).
  • The two operator-side switch arms in AddExternalAuthConfigOptions and (*VirtualMCPServerReconciler).getExternalAuthConfigSecretEnvVar (already landed in the dispatch-wiring task).
  • The reconcile-loop branch in cmd/thv-operator/controllers/mcpexternalauthconfig_controller.go that maps ErrEnterpriseRequired to Reason: EnterpriseRequired (already landed in the dispatch-wiring task).
  • The default oboHandler global + RegisterOBOHandler setter + OBOValidate / OBOSecretEnvVars wrappers in cmd/thv-operator/pkg/controllerutil/tokenexchange.go (already landed in the default-handler-infrastructure task).
  • The new pkg/auth/obo/ package, the middleware-factory map entry in pkg/runner/middleware.go, and the OBOConverterStub registered in pkg/vmcp/auth/converters/interface.go NewRegistry() (already landed in the default-handler-infrastructure task).
  • Integration tests covering upstream-stub behavior across MCPServer, MCPRemoteProxy, and VirtualMCPServer consumer paths (already landed in the dispatch-wiring task; this task only verifies they still pass).
  • Refactoring the seven existing per-type CEL rules into a registry or a code-generated list. The shape is intentionally one rule per type for reviewability.
  • Refactoring validateTypeConfigConsistency into a table-driven check. Match the existing six-if shape so the diff is one new if (plus the unauthenticated-guard extension).

References

  • cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go — the single Go file modified by this task. Relevant anchors: enum constants near lines 17-41, MCPExternalAuthConfigSpec declaration with the kubebuilder/CEL markers near lines 47-100, Validate() switch at line 1019, validateTypeConfigConsistency at line 1050. (All line numbers anchored to upstream toolhive v0.27.2 / 97b0cc3f.)
  • deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpexternalauthconfigs.yaml — the CRD chart YAML regenerated by task operator-manifests.
  • Kubernetes CEL Validation Rules — reference for the x-kubernetes-validations rule syntax used in the new CEL marker.
  • RFC 8693 — OAuth 2.0 Token Exchange — standards basis for the broader on-behalf-of pattern.

Metadata

Metadata

Assignees

No one assigned

    Labels

    apiItems related to the APIauthkubernetesItems related to Kubernetesoperator
    No fields configured for Task 📋.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions