Skip to content

Commit 8989d46

Browse files
yroblataskbot
andauthored
include missing auth confgured in status (#2493)
Co-authored-by: taskbot <taskbot@users.noreply.github.com>
1 parent b91f31a commit 8989d46

File tree

5 files changed

+136
-2
lines changed

5 files changed

+136
-2
lines changed

cmd/thv-operator/controllers/virtualmcpserver_controller.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,12 @@ func (r *VirtualMCPServerReconciler) ensureAllResources(
228228
// This catches configuration errors early, providing faster feedback than waiting for pod startup failures
229229
if err := r.validateSecretReferences(ctx, vmcp); err != nil {
230230
ctxLogger.Error(err, "Secret validation failed")
231+
// Set AuthConfigured condition to False
232+
statusManager.SetAuthConfiguredCondition(
233+
mcpv1alpha1.ConditionReasonAuthInvalid,
234+
fmt.Sprintf("Authentication configuration is invalid: %v", err),
235+
metav1.ConditionFalse,
236+
)
231237
// Record event for secret validation failure
232238
if r.Recorder != nil {
233239
r.Recorder.Eventf(vmcp, corev1.EventTypeWarning, "SecretValidationFailed",
@@ -236,6 +242,13 @@ func (r *VirtualMCPServerReconciler) ensureAllResources(
236242
return err
237243
}
238244

245+
// Authentication secrets validated successfully
246+
statusManager.SetAuthConfiguredCondition(
247+
mcpv1alpha1.ConditionReasonAuthValid,
248+
"Authentication configuration is valid",
249+
metav1.ConditionTrue,
250+
)
251+
239252
// Ensure RBAC resources
240253
if err := r.ensureRBACResources(ctx, vmcp); err != nil {
241254
ctxLogger.Error(err, "Failed to ensure RBAC resources")

cmd/thv-operator/controllers/virtualmcpserver_controller_test.go

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -696,3 +696,95 @@ func TestVirtualMCPServerNaming(t *testing.T) {
696696
url := createVmcpServiceURL(vmcpName, "default", 8080)
697697
assert.Equal(t, "http://vmcp-my-vmcp.default.svc.cluster.local:8080", url)
698698
}
699+
700+
// TestVirtualMCPServerAuthConfiguredCondition tests AuthConfigured condition setting
701+
func TestVirtualMCPServerAuthConfiguredCondition(t *testing.T) {
702+
t.Parallel()
703+
704+
scheme := runtime.NewScheme()
705+
_ = mcpv1alpha1.AddToScheme(scheme)
706+
_ = corev1.AddToScheme(scheme)
707+
708+
tests := []struct {
709+
name string
710+
vmcp *mcpv1alpha1.VirtualMCPServer
711+
secrets []client.Object
712+
expectAuthCondition bool
713+
expectedAuthStatus metav1.ConditionStatus
714+
expectedAuthReason string
715+
expectError bool
716+
}{
717+
{
718+
name: "valid auth with no secrets required",
719+
vmcp: &mcpv1alpha1.VirtualMCPServer{
720+
ObjectMeta: metav1.ObjectMeta{
721+
Name: "test-vmcp",
722+
Namespace: "default",
723+
},
724+
Spec: mcpv1alpha1.VirtualMCPServerSpec{
725+
GroupRef: mcpv1alpha1.GroupRef{
726+
Name: "test-group",
727+
},
728+
IncomingAuth: &mcpv1alpha1.IncomingAuthConfig{
729+
Type: "anonymous",
730+
},
731+
},
732+
},
733+
secrets: []client.Object{},
734+
expectAuthCondition: true,
735+
expectedAuthStatus: metav1.ConditionTrue,
736+
expectedAuthReason: mcpv1alpha1.ConditionReasonAuthValid,
737+
expectError: false,
738+
},
739+
// Note: We can't easily test OIDC secret validation without creating the full
740+
// OIDCConfigRef structure with ConfigMaps, which is complex for a unit test.
741+
// The secret validation is tested implicitly through the ensureAllResources flow.
742+
}
743+
744+
for _, tt := range tests {
745+
t.Run(tt.name, func(t *testing.T) {
746+
t.Parallel()
747+
748+
objs := append([]client.Object{tt.vmcp}, tt.secrets...)
749+
750+
fakeClient := fake.NewClientBuilder().
751+
WithScheme(scheme).
752+
WithObjects(objs...).
753+
WithStatusSubresource(&mcpv1alpha1.VirtualMCPServer{}).
754+
Build()
755+
756+
r := &VirtualMCPServerReconciler{
757+
Client: fakeClient,
758+
Scheme: scheme,
759+
PlatformDetector: ctrlutil.NewSharedPlatformDetector(),
760+
}
761+
762+
statusManager := virtualmcpserverstatus.NewStatusManager(tt.vmcp)
763+
err := r.ensureAllResources(context.Background(), tt.vmcp, statusManager)
764+
765+
if tt.expectError {
766+
assert.Error(t, err)
767+
}
768+
// ensureAllResources may return errors for missing resources like MCPGroup
769+
// We're only testing the auth condition setting
770+
771+
// Apply status updates to check condition
772+
_ = statusManager.UpdateStatus(context.Background(), &tt.vmcp.Status)
773+
774+
if tt.expectAuthCondition {
775+
// Find AuthConfigured condition
776+
var authCondition *metav1.Condition
777+
for i := range tt.vmcp.Status.Conditions {
778+
if tt.vmcp.Status.Conditions[i].Type == mcpv1alpha1.ConditionTypeAuthConfigured {
779+
authCondition = &tt.vmcp.Status.Conditions[i]
780+
break
781+
}
782+
}
783+
784+
require.NotNil(t, authCondition, "AuthConfigured condition should be set")
785+
assert.Equal(t, tt.expectedAuthStatus, authCondition.Status)
786+
assert.Equal(t, tt.expectedAuthReason, authCondition.Reason)
787+
}
788+
})
789+
}
790+
}

cmd/thv-operator/pkg/virtualmcpserverstatus/collector.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,11 @@ func (s *StatusCollector) SetReadyCondition(reason, message string, status metav
7777
s.SetCondition(mcpv1alpha1.ConditionTypeVirtualMCPServerReady, reason, message, status)
7878
}
7979

80+
// SetAuthConfiguredCondition sets the AuthConfigured condition.
81+
func (s *StatusCollector) SetAuthConfiguredCondition(reason, message string, status metav1.ConditionStatus) {
82+
s.SetCondition(mcpv1alpha1.ConditionTypeAuthConfigured, reason, message, status)
83+
}
84+
8085
// UpdateStatus applies all collected status changes in a single batch update.
8186
// Expects vmcpStatus to be freshly fetched from the cluster to ensure the update operates on the latest resource version.
8287
func (s *StatusCollector) UpdateStatus(ctx context.Context, vmcpStatus *mcpv1alpha1.VirtualMCPServerStatus) bool {

cmd/thv-operator/pkg/virtualmcpserverstatus/collector_test.go

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,25 @@ func TestStatusCollector_NoChanges(t *testing.T) {
161161
assert.False(t, hasUpdates)
162162
}
163163

164+
func TestStatusCollector_SetAuthConfiguredCondition(t *testing.T) {
165+
t.Parallel()
166+
167+
vmcp := &mcpv1alpha1.VirtualMCPServer{}
168+
collector := NewStatusManager(vmcp)
169+
170+
collector.SetAuthConfiguredCondition("AuthValid", "auth is configured", metav1.ConditionTrue)
171+
172+
status := &mcpv1alpha1.VirtualMCPServerStatus{}
173+
hasUpdates := collector.UpdateStatus(context.Background(), status)
174+
175+
assert.True(t, hasUpdates)
176+
assert.Len(t, status.Conditions, 1)
177+
assert.Equal(t, mcpv1alpha1.ConditionTypeAuthConfigured, status.Conditions[0].Type)
178+
assert.Equal(t, metav1.ConditionTrue, status.Conditions[0].Status)
179+
assert.Equal(t, "AuthValid", status.Conditions[0].Reason)
180+
assert.Equal(t, "auth is configured", status.Conditions[0].Message)
181+
}
182+
164183
func TestStatusCollector_MultipleConditions(t *testing.T) {
165184
t.Parallel()
166185

@@ -169,18 +188,20 @@ func TestStatusCollector_MultipleConditions(t *testing.T) {
169188

170189
collector.SetGroupRefValidatedCondition("GroupValid", "group is valid", metav1.ConditionTrue)
171190
collector.SetReadyCondition("DeploymentReady", "deployment is ready", metav1.ConditionTrue)
191+
collector.SetAuthConfiguredCondition("AuthValid", "auth is configured", metav1.ConditionTrue)
172192

173193
status := &mcpv1alpha1.VirtualMCPServerStatus{}
174194
hasUpdates := collector.UpdateStatus(context.Background(), status)
175195

176196
assert.True(t, hasUpdates)
177-
assert.Len(t, status.Conditions, 2)
197+
assert.Len(t, status.Conditions, 3)
178198

179-
// Verify both conditions are present
199+
// Verify all conditions are present
180200
conditionTypes := make(map[string]bool)
181201
for _, cond := range status.Conditions {
182202
conditionTypes[cond.Type] = true
183203
}
184204
assert.True(t, conditionTypes[mcpv1alpha1.ConditionTypeVirtualMCPServerGroupRefValidated])
185205
assert.True(t, conditionTypes[mcpv1alpha1.ConditionTypeVirtualMCPServerReady])
206+
assert.True(t, conditionTypes[mcpv1alpha1.ConditionTypeAuthConfigured])
186207
}

cmd/thv-operator/pkg/virtualmcpserverstatus/types.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,9 @@ type StatusManager interface {
3535
// SetReadyCondition sets the Ready condition
3636
SetReadyCondition(reason, message string, status metav1.ConditionStatus)
3737

38+
// SetAuthConfiguredCondition sets the AuthConfigured condition
39+
SetAuthConfiguredCondition(reason, message string, status metav1.ConditionStatus)
40+
3841
// UpdateStatus applies all collected status changes in a single batch update.
3942
// Returns true if updates were applied, false if no changes were collected.
4043
UpdateStatus(ctx context.Context, vmcpStatus *mcpv1alpha1.VirtualMCPServerStatus) bool

0 commit comments

Comments
 (0)