Admit obo in MCPExternalAuthConfig CRD enum

cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go

@@ -61,12 +61,17 @@ type ExternalAuthType string
 // +kubebuilder:validation:XValidation:rule="self.type == 'embeddedAuthServer' ? has(self.embeddedAuthServer) : !has(self.embeddedAuthServer)",message="embeddedAuthServer configuration must be set if and only if type is 'embeddedAuthServer'"
 // +kubebuilder:validation:XValidation:rule="self.type == 'awsSts' ? has(self.awsSts) : !has(self.awsSts)",message="awsSts configuration must be set if and only if type is 'awsSts'"
 // +kubebuilder:validation:XValidation:rule="self.type == 'upstreamInject' ? has(self.upstreamInject) : !has(self.upstreamInject)",message="upstreamInject configuration must be set if and only if type is 'upstreamInject'"
-// +kubebuilder:validation:XValidation:rule="self.type == 'unauthenticated' ? (!has(self.tokenExchange) && !has(self.headerInjection) && !has(self.bearerToken) && !has(self.embeddedAuthServer) && !has(self.awsSts) && !has(self.upstreamInject)) : true",message="no configuration must be set when type is 'unauthenticated'"
+// +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:XValidation:rule="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",message="no configuration must be set when type is 'unauthenticated'"
 //
 //nolint:lll // CEL validation rules exceed line length limit
 type MCPExternalAuthConfigSpec struct {
-	// Type is the type of external authentication to configure
-	// +kubebuilder:validation:Enum=tokenExchange;headerInjection;bearerToken;unauthenticated;embeddedAuthServer;awsSts;upstreamInject
+	// Type is the type of external authentication to configure.
+	// When set to "obo", the cluster must run a build that has registered an
+	// OBO handler via controllerutil.RegisterOBOHandler; upstream-only builds
+	// surface status.conditions[Valid] = False with Reason: EnterpriseRequired
+	// for obo-typed configs.
+	// +kubebuilder:validation:Enum=tokenExchange;headerInjection;bearerToken;unauthenticated;embeddedAuthServer;awsSts;upstreamInject;obo
 	// +kubebuilder:validation:Required
 	Type ExternalAuthType `json:"type"`
 
@@ -99,8 +104,25 @@ type MCPExternalAuthConfigSpec struct {
 	// Only used when Type is "upstreamInject".
 	// +optional
 	UpstreamInject *UpstreamInjectSpec `json:"upstreamInject,omitempty"`
+
+	// 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"`
 }
 
+// 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 RFC. The struct exists so OBO *OBOConfig compiles and the CRD
+// schema admits `spec.obo: {}` — the CEL rule "obo configuration must be set
+// if and only if type is 'obo'" requires has(self.obo), which evaluates true
+// for an empty object. Stored objects with `obo: {}` will round-trip cleanly
+// when sub-fields land, because Go zero values fill in.
+type OBOConfig struct{}
+
 // TokenExchangeConfig holds configuration for RFC-8693 OAuth 2.0 Token Exchange.
 // This configuration is used to exchange incoming authentication tokens for tokens
 // that can be used with external services.
@@ -1125,7 +1147,8 @@ type MCPExternalAuthConfigList struct {
 // Validate performs validation on the MCPExternalAuthConfig spec.
 // This method is called by the controller during reconciliation.
 //
-// Note: These validations provide defense-in-depth alongside CEL validation rules (lines 44-49).
+// Note: These validations provide defense-in-depth alongside the
+// +kubebuilder:validation:XValidation markers on MCPExternalAuthConfigSpec.
 // CEL catches issues at API admission time, but this method also validates stored objects
 // to catch any that bypassed CEL or were stored before CEL rules were added.
 func (r *MCPExternalAuthConfig) Validate() error {
@@ -1152,14 +1175,20 @@ func (r *MCPExternalAuthConfig) Validate() error {
 		// No complex validation needed for these types
 		return nil
 	case ExternalAuthTypeOBO:
-		// OBO validation is delegated to the registered OBO handler at the
-		// controllerutil layer (via controllerutil.OBOValidate ->
-		// obo.ErrEnterpriseRequired in upstream-only builds), invoked from
-		// the reconcile loop. The CRD-level Validate() stays a no-op for OBO
-		// and exists only to keep the `exhaustive` linter happy now that
-		// ExternalAuthTypeOBO is defined. The CRD enum currently rejects
-		// "obo" at the apiserver layer, so this arm is unreachable in
-		// upstream-only builds.
+		// Structural validation (the OBO field is set iff Type is "obo")
+		// has already run via r.validateTypeConfigConsistency() at the top
+		// of this method, so this arm is reached only when the structural
+		// invariant holds — and the matching CEL rule on the spec catches
+		// it at admission time. The remaining semantic validation
+		// (e.g., whether the cluster has an OBO handler registered) runs
+		// at reconcile time via the controllerutil.OBOValidate
+		// function-pointer hook: upstream-only builds return
+		// obo.ErrEnterpriseRequired, which the reconciler maps to
+		// status.conditions[Valid] = False / Reason: EnterpriseRequired.
+		// Out-of-tree builds that register a handler via
+		// controllerutil.RegisterOBOHandler short-circuit the sentinel and
+		// run their own protocol-level checks. Splitting the tiers this
+		// way keeps the upstream CRD schema stable across builds.
 		return nil
 	default:
 		// Unknown type - should be caught by enum validation, but handle defensively
@@ -1169,13 +1198,20 @@ func (r *MCPExternalAuthConfig) Validate() error {
 
 // validateTypeConfigConsistency validates that the correct config is set for the selected type.
 // This mirrors the CEL validation rules but provides defense-in-depth for stored objects.
+// The per-type `if` shape is intentional: each row reads one-to-one against
+// the corresponding CEL XValidation rule on the spec, so a reviewer can
+// audit the structural-validation contract by skimming this function.
 //
-// TODO(#5329): when OBOConfig is introduced in the CRD admission task, add a
-// matching biconditional row here:
+// The gocyclo suppression is a confirmed false positive: every new
+// ExternalAuthType adds one biconditional row and one disjunct to the
+// unauthenticated guard, which the analyzer counts as branches even though
+// the rows are syntactically uniform. Collapsing the rows into a
+// table-driven loop would reduce the score but obscure the one-to-one
+// correspondence with the CEL rules on MCPExternalAuthConfigSpec, which is
+// the property reviewers rely on to audit the structural-validation
+// contract. See issue #5329 for the broader discussion.
 //
-//	(r.Spec.OBO == nil) == (r.Spec.Type == ExternalAuthTypeOBO)
-//
-// and update the unauthenticated check below to also assert !has(self.obo).
+//nolint:gocyclo // one if per ExternalAuthType mirrors CEL rules; collapsing would obscure parity — see doc comment
 func (r *MCPExternalAuthConfig) validateTypeConfigConsistency() error {
 	// Check that each type has its corresponding config
 	if (r.Spec.TokenExchange == nil) == (r.Spec.Type == ExternalAuthTypeTokenExchange) {
@@ -1196,15 +1232,22 @@ func (r *MCPExternalAuthConfig) validateTypeConfigConsistency() error {
 	if (r.Spec.UpstreamInject == nil) == (r.Spec.Type == ExternalAuthTypeUpstreamInject) {
 		return fmt.Errorf("upstreamInject configuration must be set if and only if type is 'upstreamInject'")
 	}
+	if (r.Spec.OBO == nil) == (r.Spec.Type == ExternalAuthTypeOBO) {
+		return fmt.Errorf("obo configuration must be set if and only if type is 'obo'")
+	}
 
-	// Check that unauthenticated has no config
+	// Redundant with the per-type biconditionals above — each fires first for
+	// Type=Unauthenticated with any non-nil field — but retained as a single
+	// readable invariant so a contributor adding a new ExternalAuthType extends
+	// the "no configuration must be set" check here too.
 	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.UpstreamInject != nil ||
+			r.Spec.OBO != nil {
 			return fmt.Errorf("no configuration must be set when type is 'unauthenticated'")
 		}
 	}

cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types_test.go

@@ -275,6 +275,69 @@ func TestMCPExternalAuthConfig_Validate(t *testing.T) {
 			expectErr: true,
 			errMsg:    "upstreamInject requires a non-empty providerName",
 		},
+		{
+			name: "valid obo type with placeholder OBOConfig",
+			config: &MCPExternalAuthConfig{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-obo",
+					Namespace: "default",
+				},
+				Spec: MCPExternalAuthConfigSpec{
+					Type: ExternalAuthTypeOBO,
+					OBO:  &OBOConfig{},
+				},
+			},
+			expectErr: false,
+		},
+		{
+			name: "invalid obo type with nil obo config",
+			config: &MCPExternalAuthConfig{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-obo-missing",
+					Namespace: "default",
+				},
+				Spec: MCPExternalAuthConfigSpec{
+					Type: ExternalAuthTypeOBO,
+					OBO:  nil,
+				},
+			},
+			expectErr: true,
+			errMsg:    "obo configuration must be set if and only if type is 'obo'",
+		},
+		{
+			name: "invalid obo config set on non-obo type",
+			config: &MCPExternalAuthConfig{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-obo-on-tokenexchange",
+					Namespace: "default",
+				},
+				Spec: MCPExternalAuthConfigSpec{
+					Type:          ExternalAuthTypeTokenExchange,
+					TokenExchange: &TokenExchangeConfig{TokenURL: "https://example.com/token"},
+					OBO:           &OBOConfig{},
+				},
+			},
+			expectErr: true,
+			errMsg:    "obo configuration must be set if and only if type is 'obo'",
+		},
+		{
+			// Also intentional shape-parity coverage for the unauthenticated
+			// guard's OBO != nil disjunct, even though the OBO biconditional
+			// above intercepts first for this input.
+			name: "invalid obo config on unauthenticated type (obo biconditional intercepts first)",
+			config: &MCPExternalAuthConfig{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-obo-on-unauth",
+					Namespace: "default",
+				},
+				Spec: MCPExternalAuthConfigSpec{
+					Type: ExternalAuthTypeUnauthenticated,
+					OBO:  &OBOConfig{},
+				},
+			},
+			expectErr: true,
+			errMsg:    "obo configuration must be set if and only if type is 'obo'",
+		},
 		{
 			name: "invalid OIDC provider with oauth2Config instead",
 			config: &MCPExternalAuthConfig{

cmd/thv-operator/controllers/mcpexternalauthconfig_controller_test.go

@@ -1503,6 +1503,7 @@ func TestMCPExternalAuthConfigReconciler_OBO_DefaultHandler_SetsEnterpriseRequir
 		},
 		Spec: mcpv1beta1.MCPExternalAuthConfigSpec{
 			Type: mcpv1beta1.ExternalAuthTypeOBO,
+			OBO:  &mcpv1beta1.OBOConfig{},
 		},
 	}
 
@@ -1574,6 +1575,7 @@ func TestMCPExternalAuthConfigReconciler_OBO_ClearsStaleIdentitySynthesized(t *t
 		},
 		Spec: mcpv1beta1.MCPExternalAuthConfigSpec{
 			Type: mcpv1beta1.ExternalAuthTypeOBO,
+			OBO:  &mcpv1beta1.OBOConfig{},
 		},
 		Status: mcpv1beta1.MCPExternalAuthConfigStatus{
 			Conditions: []metav1.Condition{
@@ -1722,6 +1724,7 @@ func TestMCPExternalAuthConfigReconciler_OBO_ErrorTriageInReconcile(t *testing.T
 				},
 				Spec: mcpv1beta1.MCPExternalAuthConfigSpec{
 					Type: mcpv1beta1.ExternalAuthTypeOBO,
+					OBO:  &mcpv1beta1.OBOConfig{},
 				},
 			}
 

cmd/thv-operator/controllers/mcpremoteproxy_controller_test.go

@@ -700,6 +700,7 @@ func TestHandleExternalAuthConfig(t *testing.T) {
 				},
 				Spec: mcpv1beta1.MCPExternalAuthConfigSpec{
 					Type: mcpv1beta1.ExternalAuthTypeOBO,
+					OBO:  &mcpv1beta1.OBOConfig{},
 				},
 				Status: mcpv1beta1.MCPExternalAuthConfigStatus{
 					Conditions: []metav1.Condition{{

cmd/thv-operator/controllers/mcpserver_externalauth_test.go

@@ -567,6 +567,7 @@ func TestMCPServerReconciler_handleExternalAuthConfig_MirrorsInvalidCondition(t
 				ObjectMeta: metav1.ObjectMeta{Name: authName, Namespace: namespace},
 				Spec: mcpv1beta1.MCPExternalAuthConfigSpec{
 					Type: mcpv1beta1.ExternalAuthTypeOBO,
+					OBO:  &mcpv1beta1.OBOConfig{},
 				},
 			}
 			if tt.sourceValid != nil {

cmd/thv-operator/controllers/virtualmcpserver_externalauth_test.go

@@ -868,6 +868,7 @@ func TestConvertBackendAuthConfigToVMCP_MirrorsInvalidExternalAuthConfig(t *test
 		ObjectMeta: metav1.ObjectMeta{Name: "obo-source", Namespace: "default"},
 		Spec: mcpv1beta1.MCPExternalAuthConfigSpec{
 			Type: mcpv1beta1.ExternalAuthTypeOBO,
+			OBO:  &mcpv1beta1.OBOConfig{},
 		},
 		Status: mcpv1beta1.MCPExternalAuthConfigStatus{
 			Conditions: []metav1.Condition{{
@@ -1845,6 +1846,7 @@ func TestGetExternalAuthConfigSecretEnvVar_OBO(t *testing.T) {
 		},
 		Spec: mcpv1beta1.MCPExternalAuthConfigSpec{
 			Type: mcpv1beta1.ExternalAuthTypeOBO,
+			OBO:  &mcpv1beta1.OBOConfig{},
 		},
 	}
 

cmd/thv-operator/pkg/controllerutil/tokenexchange_test.go

@@ -259,6 +259,7 @@ func TestAddExternalAuthConfigOptions_OBO(t *testing.T) {
 		},
 		Spec: mcpv1beta1.MCPExternalAuthConfigSpec{
 			Type: mcpv1beta1.ExternalAuthTypeOBO,
+			OBO:  &mcpv1beta1.OBOConfig{},
 		},
 	}