Mirror MCPExternalAuthConfig Valid=False onto consumer CR conditions

cmd/thv-operator/controllers/external_auth_mirror.go

@@ -0,0 +1,125 @@
+// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc.
+// SPDX-License-Identifier: Apache-2.0
+
+package controllers
+
+import (
+	stderrors "errors"
+	"fmt"
+
+	"k8s.io/apimachinery/pkg/api/meta"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	mcpv1beta1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1beta1"
+)
+
+// Status Condition Parity machinery for #5347. The probe reads a referenced
+// MCPExternalAuthConfig's Valid condition; the per-consumer mirror functions
+// transcribe its reason+message onto the consumer CR's parallel condition.
+// Without this, an obo-typed MCPExternalAuthConfig in an upstream-only build
+// surfaces EnterpriseRequired only on the referenced resource and the consumer
+// status reports only the generic dispatch failure.
+
+// fallbackInvalidReason is substituted when a source surfaces Valid=False with
+// an empty Reason. metav1.Condition requires Reason to be non-empty and the
+// apiserver rejects empty-Reason patches, so the mirror cannot copy an empty
+// Reason verbatim without trapping the consumer in a noisy reconcile loop.
+const fallbackInvalidReason = "InvalidExternalAuthConfig"
+
+// mirroredInvalidExternalAuthConfigError signals that a referenced
+// MCPExternalAuthConfig had Status.Conditions[Valid]=False. Carries the
+// source's reason+message so callers can surface them on the consumer's
+// status without re-fetching the object, and satisfies the error interface
+// so it can travel through error-returning APIs (notably
+// convertBackendAuthConfigToVMCP -> buildOutgoingAuthConfig).
+type mirroredInvalidExternalAuthConfigError struct {
+	Reason  string
+	Message string
+}
+
+func (e *mirroredInvalidExternalAuthConfigError) Error() string {
+	return fmt.Sprintf("invalid (%s): %s", e.Reason, e.Message)
+}
+
+// mirroredExternalAuthConfigInvalid returns a non-nil typed error when the
+// source's Valid condition is False, or nil otherwise. Use
+// [mirroredReasonFromError] to recover the reason from a wrapped chain.
+func mirroredExternalAuthConfigInvalid(
+	externalAuthConfig *mcpv1beta1.MCPExternalAuthConfig,
+) *mirroredInvalidExternalAuthConfigError {
+	validCond := meta.FindStatusCondition(externalAuthConfig.Status.Conditions, mcpv1beta1.ConditionTypeValid)
+	if validCond == nil || validCond.Status != metav1.ConditionFalse {
+		return nil
+	}
+	reason := validCond.Reason
+	if reason == "" {
+		reason = fallbackInvalidReason
+	}
+	return &mirroredInvalidExternalAuthConfigError{Reason: reason, Message: validCond.Message}
+}
+
+// mirroredReasonFromError returns the mirrored source reason embedded in err
+// (via *mirroredInvalidExternalAuthConfigError) or "" if err does not carry
+// one. Walks the wrap chain via errors.As so it remains correct when callers
+// wrap the typed error with fmt.Errorf("...: %w", err) before passing it on
+// (notably buildOutgoingAuthConfig in the VirtualMCPServer pipeline).
+func mirroredReasonFromError(err error) string {
+	var mirrored *mirroredInvalidExternalAuthConfigError
+	if stderrors.As(err, &mirrored) {
+		return mirrored.Reason
+	}
+	return ""
+}
+
+// mirrorInvalidOnMCPServer mirrors the source's Valid=False condition onto the
+// MCPServer's ExternalAuthConfigValidated condition. When the source is healed
+// (Valid=True or absent), it clears any stale mirror so the condition does not
+// outlive its cause. Returns (true, err) when a False mirror was written so
+// the caller can mark Phase=Failed; (false, nil) otherwise.
+//
+// See package-level Status Condition Parity comment for the #5347 motivation.
+func mirrorInvalidOnMCPServer(
+	m *mcpv1beta1.MCPServer,
+	externalAuthConfig *mcpv1beta1.MCPExternalAuthConfig,
+) (bool, error) {
+	mirrored := mirroredExternalAuthConfigInvalid(externalAuthConfig)
+	if mirrored == nil {
+		meta.RemoveStatusCondition(&m.Status.Conditions, mcpv1beta1.ConditionTypeExternalAuthConfigValidated)
+		return false, nil
+	}
+	meta.SetStatusCondition(&m.Status.Conditions, metav1.Condition{
+		Type:               mcpv1beta1.ConditionTypeExternalAuthConfigValidated,
+		Status:             metav1.ConditionFalse,
+		Reason:             mirrored.Reason,
+		Message:            mirrored.Message,
+		ObservedGeneration: m.Generation,
+	})
+	return true, fmt.Errorf("MCPExternalAuthConfig %s/%s: %w", m.Namespace, externalAuthConfig.Name, mirrored)
+}
+
+// mirrorInvalidOnRemoteProxy mirrors the source's Valid=False condition onto
+// the MCPRemoteProxy's ExternalAuthConfigValidated condition. When the source
+// is healed, it clears any stale mirror defensively — the downstream True
+// writer in handleExternalAuthConfig also sets the success reason, but a
+// future early return between this site and that writer would otherwise leak
+// a stale False. Returns (true, err) when a False mirror was written so the
+// caller can short-circuit; (false, nil) otherwise.
+func mirrorInvalidOnRemoteProxy(
+	proxy *mcpv1beta1.MCPRemoteProxy,
+	externalAuthConfig *mcpv1beta1.MCPExternalAuthConfig,
+) (bool, error) {
+	mirrored := mirroredExternalAuthConfigInvalid(externalAuthConfig)
+	if mirrored == nil {
+		meta.RemoveStatusCondition(
+			&proxy.Status.Conditions, mcpv1beta1.ConditionTypeMCPRemoteProxyExternalAuthConfigValidated)
+		return false, nil
+	}
+	meta.SetStatusCondition(&proxy.Status.Conditions, metav1.Condition{
+		Type:               mcpv1beta1.ConditionTypeMCPRemoteProxyExternalAuthConfigValidated,
+		Status:             metav1.ConditionFalse,
+		Reason:             mirrored.Reason,
+		Message:            mirrored.Message,
+		ObservedGeneration: proxy.Generation,
+	})
+	return true, fmt.Errorf("MCPExternalAuthConfig %s/%s: %w", proxy.Namespace, externalAuthConfig.Name, mirrored)
+}

cmd/thv-operator/controllers/mcpremoteproxy_controller.go

@@ -735,6 +735,14 @@ func (r *MCPRemoteProxyReconciler) handleExternalAuthConfig(ctx context.Context,
 		return fmt.Errorf("failed to fetch MCPExternalAuthConfig: %w", err)
 	}
 
+	// Mirror the referenced MCPExternalAuthConfig's Valid=False condition onto
+	// the MCPRemoteProxy so the failure is visible on the consumer CR (e.g.
+	// obo-typed configs surface Valid=False/EnterpriseRequired here without the
+	// user having to inspect the referenced MCPExternalAuthConfig).
+	if mirrored, err := mirrorInvalidOnRemoteProxy(proxy, externalAuthConfig); mirrored {
+		return err
+	}
+
 	// MCPRemoteProxy supports only single-upstream embedded auth server configs.
 	// Multi-upstream requires VirtualMCPServer.
 	if embeddedCfg := externalAuthConfig.Spec.EmbeddedAuthServer; embeddedCfg != nil && len(embeddedCfg.UpstreamProviders) > 1 {

cmd/thv-operator/controllers/mcpremoteproxy_controller_test.go

@@ -499,15 +499,16 @@ func TestHandleExternalAuthConfig(t *testing.T) {
 	t.Parallel()
 
 	tests := []struct {
-		name               string
-		proxy              *mcpv1beta1.MCPRemoteProxy
-		externalAuth       *mcpv1beta1.MCPExternalAuthConfig
-		interceptorFuncs   *interceptor.Funcs
-		expectError        bool
-		errContains        string
-		expectCondition    bool
-		expectedCondStatus metav1.ConditionStatus
-		expectedCondReason string
+		name                string
+		proxy               *mcpv1beta1.MCPRemoteProxy
+		externalAuth        *mcpv1beta1.MCPExternalAuthConfig
+		interceptorFuncs    *interceptor.Funcs
+		expectError         bool
+		errContains         string
+		expectCondition     bool
+		expectedCondStatus  metav1.ConditionStatus
+		expectedCondReason  string
+		expectedCondMessage string // when set, asserts the condition's Message verbatim
 	}{
 		{
 			name: "no external auth reference",
@@ -671,6 +672,51 @@ func TestHandleExternalAuthConfig(t *testing.T) {
 			expectedCondStatus: metav1.ConditionFalse,
 			expectedCondReason: mcpv1beta1.ConditionReasonMCPRemoteProxyExternalAuthConfigFetchError,
 		},
+		{
+			// Mirror added for #5347: when the referenced MCPExternalAuthConfig
+			// has Status.Conditions[Valid]=False (e.g. obo-typed configs that
+			// the default OBO handler rejected with Reason=EnterpriseRequired
+			// in upstream-only builds), the proxy reconciler must surface a
+			// parallel ExternalAuthConfigValidated=False with the same reason
+			// and message.
+			name: "referenced config Valid=False is mirrored onto the proxy",
+			proxy: &mcpv1beta1.MCPRemoteProxy{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:       "obo-mirror-proxy",
+					Namespace:  "default",
+					Generation: 7,
+				},
+				Spec: mcpv1beta1.MCPRemoteProxySpec{
+					RemoteURL: "https://mcp.example.com",
+					ExternalAuthConfigRef: &mcpv1beta1.ExternalAuthConfigRef{
+						Name: "obo-config",
+					},
+				},
+			},
+			externalAuth: &mcpv1beta1.MCPExternalAuthConfig{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "obo-config",
+					Namespace: "default",
+				},
+				Spec: mcpv1beta1.MCPExternalAuthConfigSpec{
+					Type: mcpv1beta1.ExternalAuthTypeOBO,
+				},
+				Status: mcpv1beta1.MCPExternalAuthConfigStatus{
+					Conditions: []metav1.Condition{{
+						Type:    mcpv1beta1.ConditionTypeValid,
+						Status:  metav1.ConditionFalse,
+						Reason:  mcpv1beta1.ConditionReasonEnterpriseRequired,
+						Message: "on-behalf-of (OBO) external auth type requires an enterprise build",
+					}},
+				},
+			},
+			expectError:         true,
+			errContains:         "EnterpriseRequired",
+			expectCondition:     true,
+			expectedCondStatus:  metav1.ConditionFalse,
+			expectedCondReason:  mcpv1beta1.ConditionReasonEnterpriseRequired,
+			expectedCondMessage: "on-behalf-of (OBO) external auth type requires an enterprise build",
+		},
 		{
 			name: "embedded auth server with multiple upstreams rejected",
 			proxy: &mcpv1beta1.MCPRemoteProxy{
@@ -752,6 +798,16 @@ func TestHandleExternalAuthConfig(t *testing.T) {
 							"Condition status should match expected")
 						assert.Equal(t, tt.expectedCondReason, cond.Reason,
 							"Condition reason should match expected")
+						if tt.expectedCondMessage != "" {
+							assert.Equal(t, tt.expectedCondMessage, cond.Message,
+								"Condition message should match expected")
+						}
+						// F9: when the test fixture sets a non-zero Generation,
+						// the mirror must stamp ObservedGeneration with it.
+						if tt.proxy.Generation != 0 {
+							assert.Equal(t, tt.proxy.Generation, cond.ObservedGeneration,
+								"Condition.ObservedGeneration must match proxy.Generation")
+						}
 					}
 				}
 			} else {

cmd/thv-operator/controllers/mcpserver_controller.go

@@ -2016,7 +2016,9 @@ func getToolhiveRunnerImage() string {
 func (r *MCPServerReconciler) handleExternalAuthConfig(ctx context.Context, m *mcpv1beta1.MCPServer) error {
 	ctxLogger := log.FromContext(ctx)
 	if m.Spec.ExternalAuthConfigRef == nil {
-		// No MCPExternalAuthConfig referenced, clear any stored hash
+		// No MCPExternalAuthConfig referenced. Clear any stale mirror written
+		// while the ref was set so the condition doesn't outlive its cause.
+		meta.RemoveStatusCondition(&m.Status.Conditions, mcpv1beta1.ConditionTypeExternalAuthConfigValidated)
 		if m.Status.ExternalAuthConfigHash != "" {
 			m.Status.ExternalAuthConfigHash = ""
 			if err := r.Status().Update(ctx, m); err != nil {
@@ -2029,13 +2031,27 @@ func (r *MCPServerReconciler) handleExternalAuthConfig(ctx context.Context, m *m
 	// Get the referenced MCPExternalAuthConfig
 	externalAuthConfig, err := GetExternalAuthConfigForMCPServer(ctx, r.Client, m)
 	if err != nil {
+		// Source lookup failed (e.g. NotFound). Clear any stale mirror — the
+		// referenced source no longer exists, so the previous mirror is no
+		// longer load-bearing. Pre-existing behavior surfaces the lookup
+		// error through Phase=Failed at the caller.
+		meta.RemoveStatusCondition(&m.Status.Conditions, mcpv1beta1.ConditionTypeExternalAuthConfigValidated)
 		return err
 	}
 
 	if externalAuthConfig == nil {
+		meta.RemoveStatusCondition(&m.Status.Conditions, mcpv1beta1.ConditionTypeExternalAuthConfigValidated)
 		return fmt.Errorf("MCPExternalAuthConfig %s not found", m.Spec.ExternalAuthConfigRef.Name)
 	}
 
+	// Mirror the referenced MCPExternalAuthConfig's Valid=False condition onto
+	// the MCPServer so the failure is visible on the consumer CR (e.g. obo-typed
+	// configs surface Valid=False/EnterpriseRequired here without the user
+	// having to inspect the referenced MCPExternalAuthConfig).
+	if mirrored, err := mirrorInvalidOnMCPServer(m, externalAuthConfig); mirrored {
+		return err
+	}
+
 	// MCPServer supports only single-upstream embedded auth server configs.
 	// Multi-upstream requires VirtualMCPServer.
 	if embeddedCfg := externalAuthConfig.Spec.EmbeddedAuthServer; embeddedCfg != nil && len(embeddedCfg.UpstreamProviders) > 1 {