fix: fail when discover of auth creds fail (#2758)

instead of falling back to unauthenticated, make the process fail when there is a problem in discovering auth config Co-authored-by: taskbot <taskbot@users.noreply.github.com>

Author: yrobla

cmd/thv-operator/controllers/virtualmcpserver_discover_backends_test.go

@@ -44,6 +44,8 @@ func TestVirtualMCPServerDiscoverBackends(t *testing.T) {
 vmcp *mcpv1alpha1.VirtualMCPServer
 mcpGroup *mcpv1alpha1.MCPGroup
 mcpServers []mcpv1alpha1.MCPServer
+authConfigs []mcpv1alpha1.MCPExternalAuthConfig // Auth configs referenced by MCPServers
+secrets []corev1.Secret // Secrets referenced by auth configs
 expectedBackends int
 expectedStatuses map[string]string // backend name -> status
 expectedAuthConfigs map[string]string // backend name -> authConfigRef
@@ -153,6 +155,37 @@ func TestVirtualMCPServerDiscoverBackends(t *testing.T) {
 },
 },
 },
+authConfigs: []mcpv1alpha1.MCPExternalAuthConfig{
+{
+ObjectMeta: metav1.ObjectMeta{
+Name: "my-auth-config",
+Namespace: "default",
+},
+Spec: mcpv1alpha1.MCPExternalAuthConfigSpec{
+Type: mcpv1alpha1.ExternalAuthTypeTokenExchange,
+TokenExchange: &mcpv1alpha1.TokenExchangeConfig{
+TokenURL: "https://auth.example.com/token",
+ClientID: "test-client",
+ClientSecretRef: &mcpv1alpha1.SecretKeyRef{
+Name: "my-auth-secret",
+Key: "client-secret",
+},
+Audience: "test-audience",
+},
+},
+},
+},
+secrets: []corev1.Secret{
+{
+ObjectMeta: metav1.ObjectMeta{
+Name: "my-auth-secret",
+Namespace: "default",
+},
+Data: map[string][]byte{
+"client-secret": []byte("test-secret-value"),
+},
+},
+},
 expectedBackends: 1,
 expectedStatuses: map[string]string{
 "backend-1": "ready",
@@ -319,12 +352,19 @@ func TestVirtualMCPServerDiscoverBackends(t *testing.T) {
 
 scheme := runtime.NewScheme()
 _ = mcpv1alpha1.AddToScheme(scheme)
+_ = corev1.AddToScheme(scheme)
 
 // Build objects list for fake client
 objects := []client.Object{tt.vmcp, tt.mcpGroup}
 for i := range tt.mcpServers {
 objects = append(objects, &tt.mcpServers[i])
 }
+for i := range tt.authConfigs {
+objects = append(objects, &tt.authConfigs[i])
+}
+for i := range tt.secrets {
+objects = append(objects, &tt.secrets[i])
+}
 
 fakeClient := fake.NewClientBuilder().
 WithScheme(scheme).

pkg/vmcp/workloads/k8s.go

@@ -98,6 +98,12 @@ func (d *k8sDiscoverer) GetWorkloadAsVMCPBackend(ctx context.Context, workloadNa
 // Convert MCPServer to Backend
 backend := d.mcpServerToBackend(ctx, mcpServer)
 
+// If auth discovery failed, mcpServerToBackend returns nil
+if backend == nil {
+logger.Warnf("Skipping workload %s due to auth discovery failure", workloadName)
+return nil, nil
+}
+
 // Skip workloads without a URL (not accessible)
 if backend.BaseURL == "" {
 logger.Debugf("Skipping workload %s without URL", workloadName)
@@ -182,8 +188,11 @@ func (d *k8sDiscoverer) mcpServerToBackend(ctx context.Context, mcpServer *mcpv1
 
 // Discover and populate authentication configuration from MCPServer
 if err := d.discoverAuthConfig(ctx, mcpServer, backend); err != nil {
-// Log warning but don't fail - backend can still be used without auth
-logger.Warnf("Failed to discover auth config for MCPServer %s: %v", mcpServer.Name, err)
+// If auth discovery fails, we must fail - don't silently allow unauthorized access
+// This is a security-critical operation: if auth is configured but fails to load,
+// we should not proceed without it
+logger.Errorf("Failed to discover auth config for MCPServer %s: %v", mcpServer.Name, err)
+return nil
 }
 
 return backend

pkg/vmcp/workloads/k8s_test.go

@@ -255,11 +255,10 @@ func TestDiscoverAuth_AuthConfigNotFound(t *testing.T) {
 ctx := context.Background()
 backend, err := discoverer.GetWorkloadAsVMCPBackend(ctx, "test-server")
 
-// Should still return the backend but without auth (logs warning)
+// Should return nil backend when auth config is referenced but not found
+// This is security-critical: fail closed rather than allowing unauthorized access
 require.NoError(t, err)
-require.NotNil(t, backend)
-assert.Empty(t, backend.AuthStrategy)
-assert.Nil(t, backend.AuthMetadata)
+require.Nil(t, backend, "Should return nil backend when auth config is missing")
 }
 
 func TestDiscoverAuth_SecretNotFound(t *testing.T) {
@@ -312,11 +311,10 @@ func TestDiscoverAuth_SecretNotFound(t *testing.T) {
 ctx := context.Background()
 backend, err := discoverer.GetWorkloadAsVMCPBackend(ctx, "test-server")
 
-// Should still return the backend but without auth (logs warning)
+// Should return nil backend when secret is missing
+// This is security-critical: fail closed rather than allowing unauthorized access
 require.NoError(t, err)
-require.NotNil(t, backend)
-assert.Empty(t, backend.AuthStrategy)
-assert.Nil(t, backend.AuthMetadata)
+require.Nil(t, backend, "Should return nil backend when secret is missing")
 }
 
 func TestMCPServerToBackend_BasicFields(t *testing.T) {