Restart health checks when a backend's auth config changes (#6510)

backendChanged compared only BaseURL and TransportType, so UpdateBackends never restarted a check goroutine when a backend gained or changed outgoing auth.
The stale auth-less copy kept probing tokenless, the 401 classified BackendUnauthenticated instead of the expected-auth-challenge healthy, and ShouldAdvertise dropped the backend from aggregation until a pod restart.
Comparing AuthConfig in backendChanged restarts the loop so probes classify against the current auth config.
Includes the red-green regression test plus backendChanged table cases.

Author: alex-feel

pkg/vmcp/health/monitor.go

@@ -8,6 +8,7 @@ import (
 	"errors"
 	"fmt"
 	"log/slog"
+	"reflect"
 	"sync"
 	"time"
 
@@ -1040,7 +1041,14 @@ func buildConditions(summary Summary, phase vmcp.Phase, configuredBackendCount i
 
 // backendChanged returns true if the backend's health-check-relevant properties have changed.
 // This is used by UpdateBackends to detect when an existing backend needs its monitoring
-// goroutine restarted (e.g., URL updated after operator reconcile).
+// goroutine restarted (e.g., URL updated after operator reconcile, or outgoing auth
+// added, removed, or changed). AuthConfig matters because probe classification depends
+// on it: a 401 from an auth-configured backend is the expected answer to a
+// credential-less probe and counts as healthy, while the same 401 without an auth
+// config means misconfiguration — so a check loop probing with a stale copy inverts
+// the classification and the backend is dropped from aggregation (#6509).
 func backendChanged(old, updated vmcp.Backend) bool {
-	return old.BaseURL != updated.BaseURL || old.TransportType != updated.TransportType
+	return old.BaseURL != updated.BaseURL ||
+		old.TransportType != updated.TransportType ||
+		!reflect.DeepEqual(old.AuthConfig, updated.AuthConfig)
 }

pkg/vmcp/health/monitor_test.go

@@ -15,6 +15,7 @@ import (
 
 	mcpparser "github.com/stacklok/toolhive/pkg/mcp"
 	"github.com/stacklok/toolhive/pkg/vmcp"
+	authtypes "github.com/stacklok/toolhive/pkg/vmcp/auth/types"
 	"github.com/stacklok/toolhive/pkg/vmcp/mocks"
 )
 
@@ -915,6 +916,36 @@ func TestBackendChanged(t *testing.T) {
 			new:      vmcp.Backend{BaseURL: "http://new-svc:9090", TransportType: "streamable-http"},
 			expected: true,
 		},
+		{
+			name: "auth config added",
+			old:  vmcp.Backend{BaseURL: "http://svc:8080", TransportType: "sse"},
+			new: vmcp.Backend{BaseURL: "http://svc:8080", TransportType: "sse",
+				AuthConfig: &authtypes.BackendAuthStrategy{Type: authtypes.StrategyTypeUpstreamInject}},
+			expected: true,
+		},
+		{
+			name: "auth config removed",
+			old: vmcp.Backend{BaseURL: "http://svc:8080", TransportType: "sse",
+				AuthConfig: &authtypes.BackendAuthStrategy{Type: authtypes.StrategyTypeUpstreamInject}},
+			new:      vmcp.Backend{BaseURL: "http://svc:8080", TransportType: "sse"},
+			expected: true,
+		},
+		{
+			name: "auth config type changed",
+			old: vmcp.Backend{BaseURL: "http://svc:8080", TransportType: "sse",
+				AuthConfig: &authtypes.BackendAuthStrategy{Type: authtypes.StrategyTypeUpstreamInject}},
+			new: vmcp.Backend{BaseURL: "http://svc:8080", TransportType: "sse",
+				AuthConfig: &authtypes.BackendAuthStrategy{Type: authtypes.StrategyTypeTokenExchange}},
+			expected: true,
+		},
+		{
+			name: "auth config equal",
+			old: vmcp.Backend{BaseURL: "http://svc:8080", TransportType: "sse",
+				AuthConfig: &authtypes.BackendAuthStrategy{Type: authtypes.StrategyTypeUpstreamInject}},
+			new: vmcp.Backend{BaseURL: "http://svc:8080", TransportType: "sse",
+				AuthConfig: &authtypes.BackendAuthStrategy{Type: authtypes.StrategyTypeUpstreamInject}},
+			expected: false,
+		},
 	}
 
 	for _, tt := range tests {
@@ -987,6 +1018,63 @@ func TestMonitor_UpdateBackends_PropertyChange(t *testing.T) {
 	assert.Equal(t, 1, summary.Healthy, "backend should be healthy")
 }
 
+// TestMonitor_UpdateBackends_AuthConfigChange is the regression test for #6509:
+// a backend that gains outgoing auth after the monitor started must have its
+// check goroutine restarted, so the 401 from its credential-less probe is
+// classified against the current auth config (the expected auth challenge,
+// healthy) instead of the stale auth-less copy (unauthenticated).
+func TestMonitor_UpdateBackends_AuthConfigChange(t *testing.T) {
+	t.Parallel()
+
+	ctrl := gomock.NewController(t)
+	t.Cleanup(ctrl.Finish)
+
+	mockClient := mocks.NewMockBackendClient(ctrl)
+
+	// The backend enforces auth on its MCP endpoint: every probe fails with an
+	// authentication error, because health probes deliberately carry no credentials.
+	mockClient.EXPECT().
+		ListCapabilities(gomock.Any(), gomock.Any()).
+		Return(nil, errors.Join(vmcp.ErrAuthenticationFailed, errors.New("initialize: 401"))).
+		AnyTimes()
+
+	initialBackends := []vmcp.Backend{
+		{ID: "backend-1", Name: "Backend 1", BaseURL: "http://svc:8080", TransportType: "sse"},
+	}
+
+	config := MonitorConfig{
+		CheckInterval:      50 * time.Millisecond,
+		UnhealthyThreshold: 1,
+		Timeout:            10 * time.Millisecond,
+	}
+
+	monitor, err := NewMonitor(mockClient, initialBackends, config)
+	require.NoError(t, err)
+
+	ctx := context.Background()
+	require.NoError(t, monitor.Start(ctx))
+	t.Cleanup(func() { _ = monitor.Stop() })
+
+	// Without an auth config the 401 means misconfiguration: unauthenticated.
+	require.Eventually(t, func() bool {
+		status, ok := monitor.QueryBackendStatus("backend-1")
+		return ok && status == vmcp.BackendUnauthenticated
+	}, 500*time.Millisecond, 10*time.Millisecond, "backend-1 should be classified unauthenticated while auth-less")
+
+	// The backend gains outgoing auth (e.g. an externalAuthConfigRef reconciled
+	// into the registry). URL and transport are unchanged.
+	monitor.UpdateBackends([]vmcp.Backend{
+		{ID: "backend-1", Name: "Backend 1", BaseURL: "http://svc:8080", TransportType: "sse",
+			AuthConfig: &authtypes.BackendAuthStrategy{Type: authtypes.StrategyTypeUpstreamInject}},
+	})
+
+	// The restarted check loop probes with the auth-carrying copy: the same 401
+	// is now the expected auth challenge and classifies healthy.
+	require.Eventually(t, func() bool {
+		return monitor.IsBackendHealthy("backend-1")
+	}, 500*time.Millisecond, 10*time.Millisecond, "backend-1 should classify healthy once its auth config reaches the check loop")
+}
+
 func TestMonitor_CircuitBreakerDisabled(t *testing.T) {
 	t.Parallel()