Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -229,28 +229,45 @@ func (*VirtualMCPCompositeToolDefinition) validateStepType(index int, step Workf

// validateStepTemplates validates all template fields in a step
func (*VirtualMCPCompositeToolDefinition) validateStepTemplates(index int, step WorkflowStep) error {
for argName, argValue := range step.Arguments {
if err := validateTemplate(argValue); err != nil {
return fmt.Errorf("spec.steps[%d].arguments[%s]: invalid template: %v", index, argName, err)
return validateWorkflowStepTemplates("spec.steps", index, step)
}

// validateWorkflowStepTemplates validates all template fields in a workflow step.
// This is a shared validation function used by both VirtualMCPServer and VirtualMCPCompositeToolDefinition webhooks.
// The pathPrefix parameter allows customizing error message paths (e.g., "spec.steps" or "spec.compositeTools[0].steps").
func validateWorkflowStepTemplates(pathPrefix string, index int, step WorkflowStep) error {
// Validate template syntax in arguments (only for string values)
if step.Arguments != nil && len(step.Arguments.Raw) > 0 {
var args map[string]any
if err := json.Unmarshal(step.Arguments.Raw, &args); err != nil {
return fmt.Errorf("%s[%d].arguments: invalid JSON: %v", pathPrefix, index, err)
}
for argName, argValue := range args {
// Only validate template syntax for string values
if strValue, ok := argValue.(string); ok {
if err := validateTemplate(strValue); err != nil {
return fmt.Errorf("%s[%d].arguments[%s]: invalid template: %v", pathPrefix, index, argName, err)
}
}
}
}

if step.Condition != "" {
if err := validateTemplate(step.Condition); err != nil {
return fmt.Errorf("spec.steps[%d].condition: invalid template: %v", index, err)
return fmt.Errorf("%s[%d].condition: invalid template: %v", pathPrefix, index, err)
}
}

if step.Message != "" {
if err := validateTemplate(step.Message); err != nil {
return fmt.Errorf("spec.steps[%d].message: invalid template: %v", index, err)
return fmt.Errorf("%s[%d].message: invalid template: %v", pathPrefix, index, err)
}
}

// Validate JSON Schema for elicitation steps
if step.Schema != nil {
if err := validateJSONSchema(step.Schema.Raw); err != nil {
return fmt.Errorf("spec.steps[%d].schema: invalid JSON Schema: %v", index, err)
return fmt.Errorf("%s[%d].schema: invalid JSON Schema: %v", pathPrefix, index, err)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,9 +126,8 @@ func TestVirtualMCPCompositeToolDefinitionValidate(t *testing.T) {
{
ID: "deploy",
Tool: "kubectl.apply",
Arguments: map[string]string{
"namespace": "{{.params.environment}}",
"replicas": "{{.params.replicas}}",
Arguments: &runtime.RawExtension{
Raw: []byte(`{"namespace": "{{.params.environment}}", "replicas": "{{.params.replicas}}"}`),
},
},
},
Expand Down Expand Up @@ -515,8 +514,8 @@ func TestVirtualMCPCompositeToolDefinitionValidate(t *testing.T) {
{
ID: "deploy",
Tool: "kubectl.apply",
Arguments: map[string]string{
"namespace": "{{.params.env",
Arguments: &runtime.RawExtension{
Raw: []byte(`{"namespace": "{{.params.env"}`),
},
},
},
Expand Down
10 changes: 7 additions & 3 deletions cmd/thv-operator/api/v1alpha1/virtualmcpserver_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -280,10 +280,14 @@ type WorkflowStep struct {
// +optional
Tool string `json:"tool,omitempty"`

// Arguments is a map of argument templates
// Supports Go template syntax with .params and .steps
// Arguments is a map of argument values with template expansion support.
// Supports Go template syntax with .params and .steps for string values.
// Non-string values (integers, booleans, arrays, objects) are passed as-is.
// Note: the templating is only supported on the first level of the key-value pairs.
// +optional
Arguments map[string]string `json:"arguments,omitempty"`
// +kubebuilder:pruning:PreserveUnknownFields
// +kubebuilder:validation:Type=object
Arguments *runtime.RawExtension `json:"arguments,omitempty"`

// Message is the elicitation message
// Only used when Type is "elicitation"
Expand Down
8 changes: 7 additions & 1 deletion cmd/thv-operator/api/v1alpha1/virtualmcpserver_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,13 @@ func validateCompositeToolStep(
}

// Validate error handling
return validateStepErrorHandling(toolIndex, stepIndex, step)
if err := validateStepErrorHandling(toolIndex, stepIndex, step); err != nil {
return err
}

// Validate templates in arguments and other fields
pathPrefix := fmt.Sprintf("spec.compositeTools[%d].steps", toolIndex)
return validateWorkflowStepTemplates(pathPrefix, stepIndex, step)
}

// validateStepType validates the step type and type-specific requirements
Expand Down
6 changes: 2 additions & 4 deletions cmd/thv-operator/api/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -703,8 +703,8 @@ func TestVirtualMCPServerReconciler_CompositeToolRefs_EndToEnd(t *testing.T) {
ID: "step1",
Type: "tool",
Tool: "backend.echo",
Arguments: map[string]string{
"input": "{{ .params.message }}",
Arguments: &runtime.RawExtension{
Raw: []byte(`{"input": "{{ .params.message }}"}`),
},
},
},
Expand Down
61 changes: 42 additions & 19 deletions cmd/thv-operator/pkg/vmcpconfig/converter.go
Original file line number Diff line number Diff line change
Expand Up @@ -518,17 +518,20 @@ func (c *Converter) resolveMCPToolConfig(
func (c *Converter) convertCompositeTools(
ctx context.Context,
vmcp *mcpv1alpha1.VirtualMCPServer,
) []*vmcpconfig.CompositeToolConfig {
) ([]*vmcpconfig.CompositeToolConfig, error) {
compositeTools := make([]*vmcpconfig.CompositeToolConfig, 0, len(vmcp.Spec.CompositeTools))

for _, crdTool := range vmcp.Spec.CompositeTools {
tool := c.convertCompositeToolSpec(
tool, err := c.convertCompositeToolSpec(
ctx, crdTool.Name, crdTool.Description, crdTool.Timeout,
crdTool.Parameters, crdTool.Steps, crdTool.Output, crdTool.Name)
if err != nil {
return nil, err
}
compositeTools = append(compositeTools, tool)
}

return compositeTools
return compositeTools, nil
}

// convertAllCompositeTools converts both inline CompositeTools and referenced CompositeToolRefs,
Expand All @@ -538,12 +541,14 @@ func (c *Converter) convertAllCompositeTools(
vmcp *mcpv1alpha1.VirtualMCPServer,
) ([]*vmcpconfig.CompositeToolConfig, error) {
// Convert inline composite tools
inlineTools := c.convertCompositeTools(ctx, vmcp)
inlineTools, err := c.convertCompositeTools(ctx, vmcp)
if err != nil {
return nil, fmt.Errorf("failed to convert inline composite tools: %w", err)
}

// Convert referenced composite tools
var referencedTools []*vmcpconfig.CompositeToolConfig
if len(vmcp.Spec.CompositeToolRefs) > 0 {
var err error
referencedTools, err = c.convertReferencedCompositeTools(ctx, vmcp)
if err != nil {
return nil, fmt.Errorf("failed to convert referenced composite tools: %w", err)
Expand Down Expand Up @@ -587,7 +592,10 @@ func (c *Converter) convertReferencedCompositeTools(
}

// Convert the referenced definition to CompositeToolConfig
tool := c.convertCompositeToolDefinition(ctx, compositeToolDef)
tool, err := c.convertCompositeToolDefinition(ctx, compositeToolDef)
if err != nil {
return nil, fmt.Errorf("failed to convert referenced tool %q: %w", ref.Name, err)
}
referencedTools = append(referencedTools, tool)
}

Expand All @@ -598,7 +606,7 @@ func (c *Converter) convertReferencedCompositeTools(
func (c *Converter) convertCompositeToolDefinition(
ctx context.Context,
def *mcpv1alpha1.VirtualMCPCompositeToolDefinition,
) *vmcpconfig.CompositeToolConfig {
) (*vmcpconfig.CompositeToolConfig, error) {
return c.convertCompositeToolSpec(
ctx, def.Spec.Name, def.Spec.Description, def.Spec.Timeout,
def.Spec.Parameters, def.Spec.Steps, def.Spec.Output, def.Name)
Expand All @@ -613,7 +621,7 @@ func (c *Converter) convertCompositeToolSpec(
steps []mcpv1alpha1.WorkflowStep,
output *mcpv1alpha1.OutputSpec,
toolNameForLogging string,
) *vmcpconfig.CompositeToolConfig {
) (*vmcpconfig.CompositeToolConfig, error) {
tool := &vmcpconfig.CompositeToolConfig{
Name: name,
Description: description,
Expand Down Expand Up @@ -650,30 +658,39 @@ func (c *Converter) convertCompositeToolSpec(
}

// Convert steps
tool.Steps = c.convertWorkflowSteps(ctx, steps, toolNameForLogging)
workflowSteps, err := c.convertWorkflowSteps(ctx, steps, toolNameForLogging)
if err != nil {
return nil, fmt.Errorf("failed to convert steps for tool %q: %w", toolNameForLogging, err)
}
tool.Steps = workflowSteps

// Convert output configuration
if output != nil {
tool.Output = convertOutputSpec(ctx, output)
}

return tool
return tool, nil
}

// convertWorkflowSteps converts a slice of WorkflowStep CRD objects to WorkflowStepConfig.
func (*Converter) convertWorkflowSteps(
ctx context.Context,
steps []mcpv1alpha1.WorkflowStep,
toolNameForLogging string,
) []*vmcpconfig.WorkflowStepConfig {
) ([]*vmcpconfig.WorkflowStepConfig, error) {
workflowSteps := make([]*vmcpconfig.WorkflowStepConfig, 0, len(steps))

for _, crdStep := range steps {
args, err := convertArguments(crdStep.Arguments)
if err != nil {
return nil, fmt.Errorf("step %q: %w", crdStep.ID, err)
}

step := &vmcpconfig.WorkflowStepConfig{
ID: crdStep.ID,
Type: crdStep.Type,
Tool: crdStep.Tool,
Arguments: convertArguments(crdStep.Arguments),
Arguments: args,
Message: crdStep.Message,
Condition: crdStep.Condition,
DependsOn: crdStep.DependsOn,
Expand Down Expand Up @@ -726,7 +743,7 @@ func (*Converter) convertWorkflowSteps(
workflowSteps = append(workflowSteps, step)
}

return workflowSteps
return workflowSteps, nil
}

// validateCompositeToolNames checks for duplicate tool names across all composite tools.
Expand All @@ -741,13 +758,19 @@ func validateCompositeToolNames(tools []*vmcpconfig.CompositeToolConfig) error {
return nil
}

// convertArguments converts string arguments to any type for template expansion
func convertArguments(args map[string]string) map[string]any {
result := make(map[string]any, len(args))
for k, v := range args {
result[k] = v
// convertArguments converts arguments from runtime.RawExtension to map[string]any.
// This preserves the original types (integers, booleans, arrays, objects) from the CRD.
// Returns an empty map if no arguments are specified.
// Returns an error if the JSON fails to unmarshal.
func convertArguments(args *runtime.RawExtension) (map[string]any, error) {
if args == nil || len(args.Raw) == 0 {
return make(map[string]any), nil
}
var result map[string]any
if err := json.Unmarshal(args.Raw, &result); err != nil {
return nil, fmt.Errorf("failed to unmarshal arguments: %w", err)
}
return result
return result, nil
}

// convertOutputSpec converts OutputSpec from CRD to vmcp config OutputConfig
Expand Down
Loading
Loading