Skip to content

Commit e2c67bb

Browse files
yroblataskbot
andauthored
Design and implement the VirtualMCPServer CRD (#2371)
Maximizing reuse of existing ToolHive CRDs for a unified experience. Related-to: stacklok/stacklok-epics#151 Co-authored-by: taskbot <[email protected]>
1 parent 65f7a4e commit e2c67bb

File tree

12 files changed

+4811
-2
lines changed

12 files changed

+4811
-2
lines changed

cmd/thv-operator/api/v1alpha1/virtualmcpserver_types.go

Lines changed: 629 additions & 0 deletions
Large diffs are not rendered by default.

cmd/thv-operator/api/v1alpha1/virtualmcpserver_types_test.go

Lines changed: 609 additions & 0 deletions
Large diffs are not rendered by default.
Lines changed: 339 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,339 @@
1+
package v1alpha1
2+
3+
import (
4+
"fmt"
5+
)
6+
7+
// Validate performs validation for VirtualMCPServer
8+
// This method can be called by the controller during reconciliation
9+
func (r *VirtualMCPServer) Validate() error {
10+
// Validate GroupRef is set (required field)
11+
if r.Spec.GroupRef.Name == "" {
12+
return fmt.Errorf("spec.groupRef.name is required")
13+
}
14+
15+
// Validate OutgoingAuth configuration
16+
if r.Spec.OutgoingAuth != nil {
17+
if err := r.validateOutgoingAuth(); err != nil {
18+
return err
19+
}
20+
}
21+
22+
// Validate Aggregation configuration
23+
if r.Spec.Aggregation != nil {
24+
if err := r.validateAggregation(); err != nil {
25+
return err
26+
}
27+
}
28+
29+
// Validate CompositeTools
30+
if len(r.Spec.CompositeTools) > 0 {
31+
if err := r.validateCompositeTools(); err != nil {
32+
return err
33+
}
34+
}
35+
36+
// Validate TokenCache configuration
37+
if r.Spec.TokenCache != nil {
38+
if err := r.validateTokenCache(); err != nil {
39+
return err
40+
}
41+
}
42+
43+
return nil
44+
}
45+
46+
// validateOutgoingAuth validates OutgoingAuth configuration
47+
func (r *VirtualMCPServer) validateOutgoingAuth() error {
48+
auth := r.Spec.OutgoingAuth
49+
50+
// Validate source enum values (already validated by kubebuilder but being explicit)
51+
validSources := map[string]bool{
52+
"discovered": true,
53+
"inline": true,
54+
"mixed": true,
55+
}
56+
if auth.Source != "" && !validSources[auth.Source] {
57+
return fmt.Errorf("spec.outgoingAuth.source must be one of: discovered, inline, mixed")
58+
}
59+
60+
// Validate backend configurations
61+
for backendName, backendAuth := range auth.Backends {
62+
if err := r.validateBackendAuth(backendName, backendAuth); err != nil {
63+
return err
64+
}
65+
}
66+
67+
return nil
68+
}
69+
70+
// validateBackendAuth validates a single backend auth configuration
71+
func (*VirtualMCPServer) validateBackendAuth(backendName string, auth BackendAuthConfig) error {
72+
// Validate type is set
73+
if auth.Type == "" {
74+
return fmt.Errorf("spec.outgoingAuth.backends[%s].type is required", backendName)
75+
}
76+
77+
// Validate type-specific configurations
78+
switch auth.Type {
79+
case BackendAuthTypeServiceAccount:
80+
if auth.ServiceAccount == nil {
81+
return fmt.Errorf("spec.outgoingAuth.backends[%s].serviceAccount is required when type is service_account", backendName)
82+
}
83+
if auth.ServiceAccount.CredentialsRef.Name == "" {
84+
return fmt.Errorf("spec.outgoingAuth.backends[%s].serviceAccount.credentialsRef.name is required", backendName)
85+
}
86+
if auth.ServiceAccount.CredentialsRef.Key == "" {
87+
return fmt.Errorf("spec.outgoingAuth.backends[%s].serviceAccount.credentialsRef.key is required", backendName)
88+
}
89+
90+
case BackendAuthTypeExternalAuthConfigRef:
91+
if auth.ExternalAuthConfigRef == nil {
92+
return fmt.Errorf(
93+
"spec.outgoingAuth.backends[%s].externalAuthConfigRef is required when type is external_auth_config_ref",
94+
backendName)
95+
}
96+
if auth.ExternalAuthConfigRef.Name == "" {
97+
return fmt.Errorf("spec.outgoingAuth.backends[%s].externalAuthConfigRef.name is required", backendName)
98+
}
99+
100+
case BackendAuthTypeDiscovered, BackendAuthTypePassThrough:
101+
// No additional validation needed
102+
103+
default:
104+
return fmt.Errorf(
105+
"spec.outgoingAuth.backends[%s].type must be one of: discovered, pass_through, service_account, external_auth_config_ref",
106+
backendName)
107+
}
108+
109+
return nil
110+
}
111+
112+
// validateAggregation validates Aggregation configuration
113+
func (r *VirtualMCPServer) validateAggregation() error {
114+
agg := r.Spec.Aggregation
115+
116+
// Validate conflict resolution strategy
117+
if agg.ConflictResolution != "" {
118+
validStrategies := map[string]bool{
119+
ConflictResolutionPrefix: true,
120+
ConflictResolutionPriority: true,
121+
ConflictResolutionManual: true,
122+
}
123+
if !validStrategies[agg.ConflictResolution] {
124+
return fmt.Errorf("spec.aggregation.conflictResolution must be one of: prefix, priority, manual")
125+
}
126+
}
127+
128+
// Validate conflict resolution config based on strategy
129+
if agg.ConflictResolutionConfig != nil {
130+
config := agg.ConflictResolutionConfig
131+
132+
switch agg.ConflictResolution {
133+
case ConflictResolutionPriority:
134+
if len(config.PriorityOrder) == 0 {
135+
return fmt.Errorf("spec.aggregation.conflictResolutionConfig.priorityOrder is required when conflictResolution is priority")
136+
}
137+
138+
case ConflictResolutionManual:
139+
// For manual resolution, tools must define explicit overrides
140+
// This will be validated at runtime when conflicts are detected
141+
}
142+
}
143+
144+
// Validate per-workload tool configurations
145+
for i, toolConfig := range agg.Tools {
146+
if toolConfig.Workload == "" {
147+
return fmt.Errorf("spec.aggregation.tools[%d].workload is required", i)
148+
}
149+
150+
// If ToolConfigRef is specified, ensure it has a name
151+
if toolConfig.ToolConfigRef != nil && toolConfig.ToolConfigRef.Name == "" {
152+
return fmt.Errorf("spec.aggregation.tools[%d].toolConfigRef.name is required when toolConfigRef is specified", i)
153+
}
154+
}
155+
156+
return nil
157+
}
158+
159+
// validateCompositeTools validates composite tool definitions
160+
func (r *VirtualMCPServer) validateCompositeTools() error {
161+
toolNames := make(map[string]bool)
162+
163+
for i, tool := range r.Spec.CompositeTools {
164+
if err := r.validateCompositeTool(i, tool, toolNames); err != nil {
165+
return err
166+
}
167+
}
168+
169+
return nil
170+
}
171+
172+
// validateCompositeTool validates a single composite tool
173+
func (*VirtualMCPServer) validateCompositeTool(index int, tool CompositeToolSpec, toolNames map[string]bool) error {
174+
// Check for required fields
175+
if tool.Name == "" {
176+
return fmt.Errorf("spec.compositeTools[%d].name is required", index)
177+
}
178+
if tool.Description == "" {
179+
return fmt.Errorf("spec.compositeTools[%d].description is required", index)
180+
}
181+
if len(tool.Steps) == 0 {
182+
return fmt.Errorf("spec.compositeTools[%d].steps must have at least one step", index)
183+
}
184+
185+
// Check for duplicate tool names
186+
if toolNames[tool.Name] {
187+
return fmt.Errorf("spec.compositeTools[%d].name %q is duplicated", index, tool.Name)
188+
}
189+
toolNames[tool.Name] = true
190+
191+
// Validate steps
192+
return validateCompositeToolSteps(index, tool.Steps)
193+
}
194+
195+
// validateCompositeToolSteps validates all steps in a composite tool
196+
func validateCompositeToolSteps(toolIndex int, steps []WorkflowStep) error {
197+
stepIDs := make(map[string]bool)
198+
199+
for j, step := range steps {
200+
if err := validateCompositeToolStep(toolIndex, j, step, steps, stepIDs); err != nil {
201+
return err
202+
}
203+
}
204+
205+
return nil
206+
}
207+
208+
// validateCompositeToolStep validates a single workflow step
209+
func validateCompositeToolStep(
210+
toolIndex, stepIndex int, step WorkflowStep, allSteps []WorkflowStep, stepIDs map[string]bool,
211+
) error {
212+
if step.ID == "" {
213+
return fmt.Errorf("spec.compositeTools[%d].steps[%d].id is required", toolIndex, stepIndex)
214+
}
215+
216+
// Check for duplicate step IDs
217+
if stepIDs[step.ID] {
218+
return fmt.Errorf("spec.compositeTools[%d].steps[%d].id %q is duplicated", toolIndex, stepIndex, step.ID)
219+
}
220+
stepIDs[step.ID] = true
221+
222+
// Validate step type
223+
if err := validateStepType(toolIndex, stepIndex, step); err != nil {
224+
return err
225+
}
226+
227+
// Validate dependsOn references
228+
if err := validateStepDependencies(toolIndex, stepIndex, step, allSteps, stepIDs); err != nil {
229+
return err
230+
}
231+
232+
// Validate error handling
233+
return validateStepErrorHandling(toolIndex, stepIndex, step)
234+
}
235+
236+
// validateStepType validates the step type and type-specific requirements
237+
func validateStepType(toolIndex, stepIndex int, step WorkflowStep) error {
238+
if step.Type != "" && step.Type != WorkflowStepTypeToolCall && step.Type != WorkflowStepTypeElicitation {
239+
return fmt.Errorf("spec.compositeTools[%d].steps[%d].type must be tool_call or elicitation", toolIndex, stepIndex)
240+
}
241+
242+
stepType := step.Type
243+
if stepType == "" {
244+
stepType = WorkflowStepTypeToolCall // default
245+
}
246+
247+
if stepType == WorkflowStepTypeToolCall && step.Tool == "" {
248+
return fmt.Errorf("spec.compositeTools[%d].steps[%d].tool is required when type is tool_call", toolIndex, stepIndex)
249+
}
250+
251+
if stepType == WorkflowStepTypeElicitation && step.Message == "" {
252+
return fmt.Errorf("spec.compositeTools[%d].steps[%d].message is required when type is elicitation", toolIndex, stepIndex)
253+
}
254+
255+
return nil
256+
}
257+
258+
// validateStepDependencies validates that dependsOn references exist
259+
func validateStepDependencies(
260+
toolIndex, stepIndex int, step WorkflowStep, allSteps []WorkflowStep, stepIDs map[string]bool,
261+
) error {
262+
for _, depID := range step.DependsOn {
263+
if !stepIDs[depID] {
264+
// Check if it's a forward reference
265+
found := false
266+
for k := stepIndex + 1; k < len(allSteps); k++ {
267+
if allSteps[k].ID == depID {
268+
found = true
269+
break
270+
}
271+
}
272+
if !found {
273+
return fmt.Errorf("spec.compositeTools[%d].steps[%d].dependsOn references unknown step %q",
274+
toolIndex, stepIndex, depID)
275+
}
276+
}
277+
}
278+
return nil
279+
}
280+
281+
// validateStepErrorHandling validates error handling configuration for a step
282+
func validateStepErrorHandling(toolIndex, stepIndex int, step WorkflowStep) error {
283+
if step.OnError == nil || step.OnError.Action == "" {
284+
return nil
285+
}
286+
287+
validActions := map[string]bool{
288+
"abort": true,
289+
"continue": true,
290+
"retry": true,
291+
}
292+
if !validActions[step.OnError.Action] {
293+
return fmt.Errorf("spec.compositeTools[%d].steps[%d].onError.action must be abort, continue, or retry",
294+
toolIndex, stepIndex)
295+
}
296+
297+
if step.OnError.Action == "retry" && step.OnError.MaxRetries < 1 {
298+
return fmt.Errorf("spec.compositeTools[%d].steps[%d].onError.maxRetries must be at least 1 when action is retry",
299+
toolIndex, stepIndex)
300+
}
301+
302+
return nil
303+
}
304+
305+
// validateTokenCache validates token cache configuration
306+
func (r *VirtualMCPServer) validateTokenCache() error {
307+
cache := r.Spec.TokenCache
308+
309+
// Validate provider
310+
if cache.Provider != "" {
311+
validProviders := map[string]bool{
312+
"memory": true,
313+
"redis": true,
314+
}
315+
if !validProviders[cache.Provider] {
316+
return fmt.Errorf("spec.tokenCache.provider must be memory or redis")
317+
}
318+
}
319+
320+
// Validate provider-specific configuration
321+
if cache.Provider == "redis" || (cache.Provider == "" && cache.Redis != nil) {
322+
if cache.Redis == nil {
323+
return fmt.Errorf("spec.tokenCache.redis is required when provider is redis")
324+
}
325+
if cache.Redis.Address == "" {
326+
return fmt.Errorf("spec.tokenCache.redis.address is required")
327+
}
328+
if cache.Redis.PasswordRef != nil {
329+
if cache.Redis.PasswordRef.Name == "" {
330+
return fmt.Errorf("spec.tokenCache.redis.passwordRef.name is required when passwordRef is specified")
331+
}
332+
if cache.Redis.PasswordRef.Key == "" {
333+
return fmt.Errorf("spec.tokenCache.redis.passwordRef.key is required when passwordRef is specified")
334+
}
335+
}
336+
}
337+
338+
return nil
339+
}

0 commit comments

Comments
 (0)