Skip to content
Merged
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
106 changes: 66 additions & 40 deletions cmd/thv-operator/controllers/mcpserver_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,13 +71,21 @@ var defaultRBACRules = []rbacv1.PolicyRule{
Resources: []string{"pods/attach"},
Verbs: []string{"create", "get"},
},
{
APIGroups: []string{""},
Resources: []string{"configmaps"},
Verbs: []string{"get", "list", "watch"},
},
}

var ctxLogger = log.FromContext(context.Background())

// mcpContainerName is the name of the mcp container used in pod templates
const mcpContainerName = "mcp"

// trueValue is the string value "true" used for environment variable comparisons
const trueValue = "true"

// Authorization ConfigMap label constants
const (
// authzLabelKey is the label key for authorization configuration type
Expand Down Expand Up @@ -463,38 +471,51 @@ func (r *MCPServerReconciler) deploymentForMCPServer(ctx context.Context, m *mcp

// Prepare container args
args := []string{"run", "--foreground=true"}
args = append(args, fmt.Sprintf("--proxy-port=%d", m.Spec.Port))
args = append(args, fmt.Sprintf("--name=%s", m.Name))
args = append(args, fmt.Sprintf("--transport=%s", m.Spec.Transport))
args = append(args, fmt.Sprintf("--host=%s", getProxyHost()))

if m.Spec.TargetPort != 0 {
args = append(args, fmt.Sprintf("--target-port=%d", m.Spec.TargetPort))
}

// Generate pod template patch for secrets and merge with user-provided patch

finalPodTemplateSpec := NewMCPServerPodTemplateSpecBuilder(m.Spec.PodTemplateSpec).
WithServiceAccount(m.Spec.ServiceAccount).
WithSecrets(m.Spec.Secrets).
Build()
// Add pod template patch if we have one
if finalPodTemplateSpec != nil {
podTemplatePatch, err := json.Marshal(finalPodTemplateSpec)
if err != nil {
logger.Errorf("Failed to marshal pod template spec: %v", err)
} else {
args = append(args, fmt.Sprintf("--k8s-pod-patch=%s", string(podTemplatePatch)))
// Check if global ConfigMap mode is enabled via environment variable
useConfigMap := os.Getenv("TOOLHIVE_USE_CONFIGMAP") == trueValue
if useConfigMap {
// Use the operator-created ConfigMap (format: {name}-runconfig)
configMapName := fmt.Sprintf("%s-runconfig", m.Name)
configMapRef := fmt.Sprintf("%s/%s", m.Namespace, configMapName)
args = append(args, fmt.Sprintf("--from-configmap=%s", configMapRef))
} else {
// Use individual configuration flags (existing behavior)
args = append(args, fmt.Sprintf("--proxy-port=%d", m.Spec.Port))
args = append(args, fmt.Sprintf("--name=%s", m.Name))
args = append(args, fmt.Sprintf("--transport=%s", m.Spec.Transport))
args = append(args, fmt.Sprintf("--host=%s", getProxyHost()))
if m.Spec.TargetPort != 0 {
args = append(args, fmt.Sprintf("--target-port=%d", m.Spec.TargetPort))
}
}

// Add pod template patch and permission profile only if not using ConfigMap
// When using ConfigMap, these are included in the runconfig.json
if !useConfigMap {
// Generate pod template patch for secrets and merge with user-provided patch
finalPodTemplateSpec := NewMCPServerPodTemplateSpecBuilder(m.Spec.PodTemplateSpec).
WithServiceAccount(m.Spec.ServiceAccount).
WithSecrets(m.Spec.Secrets).
Build()
// Add pod template patch if we have one
if finalPodTemplateSpec != nil {
podTemplatePatch, err := json.Marshal(finalPodTemplateSpec)
if err != nil {
logger.Errorf("Failed to marshal pod template spec: %v", err)
} else {
args = append(args, fmt.Sprintf("--k8s-pod-patch=%s", string(podTemplatePatch)))
}
}
}

// Add permission profile args
if m.Spec.PermissionProfile != nil {
switch m.Spec.PermissionProfile.Type {
case mcpv1alpha1.PermissionProfileTypeBuiltin:
args = append(args, fmt.Sprintf("--permission-profile=%s", m.Spec.PermissionProfile.Name))
case mcpv1alpha1.PermissionProfileTypeConfigMap:
args = append(args, fmt.Sprintf("--permission-profile-path=/etc/toolhive/profiles/%s", m.Spec.PermissionProfile.Key))
// Add permission profile args
if m.Spec.PermissionProfile != nil {
switch m.Spec.PermissionProfile.Type {
case mcpv1alpha1.PermissionProfileTypeBuiltin:
args = append(args, fmt.Sprintf("--permission-profile=%s", m.Spec.PermissionProfile.Name))
case mcpv1alpha1.PermissionProfileTypeConfigMap:
args = append(args, fmt.Sprintf("--permission-profile-path=/etc/toolhive/profiles/%s", m.Spec.PermissionProfile.Key))
}
}
}

Expand Down Expand Up @@ -526,15 +547,18 @@ func (r *MCPServerReconciler) deploymentForMCPServer(ctx context.Context, m *mcp
args = append(args, "--enable-audit")
}

// Add environment variables as --env flags for the MCP server
for _, e := range m.Spec.Env {
args = append(args, fmt.Sprintf("--env=%s=%s", e.Name, e.Value))
}
// Add environment variables and tools filter only if not using ConfigMap
if !useConfigMap {
// Add environment variables as --env flags for the MCP server
for _, e := range m.Spec.Env {
args = append(args, fmt.Sprintf("--env=%s=%s", e.Name, e.Value))
}

// Add tools filter args
if len(m.Spec.ToolsFilter) > 0 {
slices.Sort(m.Spec.ToolsFilter)
args = append(args, fmt.Sprintf("--tools=%s", strings.Join(m.Spec.ToolsFilter, ",")))
// Add tools filter args
if len(m.Spec.ToolsFilter) > 0 {
slices.Sort(m.Spec.ToolsFilter)
args = append(args, fmt.Sprintf("--tools=%s", strings.Join(m.Spec.ToolsFilter, ",")))
}
}

// Add OpenTelemetry configuration args
Expand All @@ -550,11 +574,13 @@ func (r *MCPServerReconciler) deploymentForMCPServer(ctx context.Context, m *mcp
}
}

// Add the image
// Always add the image as it's required by proxy runner command signature
// When using ConfigMap, the image from ConfigMap takes precedence, but we still need
// to provide this as a positional argument to satisfy the command requirements
args = append(args, m.Spec.Image)

// Add additional args
if len(m.Spec.Args) > 0 {
// Add additional args only if not using ConfigMap
if !useConfigMap && len(m.Spec.Args) > 0 {
args = append(args, "--")
args = append(args, m.Spec.Args...)
}
Expand Down
20 changes: 16 additions & 4 deletions cmd/thv-operator/controllers/mcpserver_runconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,8 +216,7 @@ func (*MCPServerReconciler) createRunConfigFromMCPServer(m *mcpv1alpha1.MCPServe
}
}

// Use the RunConfigBuilder for operator context with full builder pattern
config, err := runner.NewOperatorRunConfigBuilder().
builder := runner.NewOperatorRunConfigBuilder().
WithName(m.Name).
WithImage(m.Spec.Image).
WithCmdArgs(m.Spec.Args).
Expand All @@ -227,8 +226,21 @@ func (*MCPServerReconciler) createRunConfigFromMCPServer(m *mcpv1alpha1.MCPServe
WithEnvVars(envVars).
WithVolumes(volumes).
WithSecrets(secrets).
WithK8sPodPatch(k8sPodPatch).
BuildForOperator()
WithK8sPodPatch(k8sPodPatch)

// Add permission profile if specified
if m.Spec.PermissionProfile != nil {
switch m.Spec.PermissionProfile.Type {
case mcpv1alpha1.PermissionProfileTypeBuiltin:
builder = builder.WithPermissionProfileNameOrPath(m.Spec.PermissionProfile.Name)
case mcpv1alpha1.PermissionProfileTypeConfigMap:
// For ConfigMap-based permission profiles, we store the path
builder = builder.WithPermissionProfileNameOrPath(fmt.Sprintf("/etc/toolhive/profiles/%s", m.Spec.PermissionProfile.Key))
}
}

// Use the RunConfigBuilder for operator context with full builder pattern
config, err := builder.BuildForOperator()

if err != nil {
return nil, err
Expand Down
146 changes: 146 additions & 0 deletions cmd/thv-proxyrunner/app/run.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,18 @@
package app

import (
"context"
"encoding/json"
"fmt"
"net"
"os"
"strings"

"github.com/spf13/cobra"
"github.com/spf13/pflag"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes"
"k8s.io/client-go/rest"

"github.com/stacklok/toolhive/pkg/container"
"github.com/stacklok/toolhive/pkg/environment"
Expand All @@ -17,6 +24,11 @@ import (
"github.com/stacklok/toolhive/pkg/workloads"
)

// NewRunCmd creates a new run command for testing
func NewRunCmd() *cobra.Command {
return runCmd
}

var runCmd = &cobra.Command{
Use: "run [flags] SERVER_OR_IMAGE_OR_PROTOCOL [-- ARGS...]",
Short: "Run an MCP server",
Expand Down Expand Up @@ -90,6 +102,9 @@ var (

// Environment file processing
runEnvFileDir string

// ConfigMap reference flag (for identification only)
runFromConfigMap string
)

func init() {
Expand Down Expand Up @@ -219,11 +234,76 @@ func init() {
"",
"Load environment variables from all files in a directory",
)
runCmd.Flags().StringVar(
&runFromConfigMap,
"from-configmap",
"",
"[Experimental] Load configuration from a Kubernetes ConfigMap (format: namespace/configmap-name). "+
"This flag is mutually exclusive with other configuration flags.",
)
}

// validateConfigMapOnlyMode validates that no conflicting flags are used with --from-configmap
// It dynamically finds all flags that could conflict by checking which ones are changed
func validateConfigMapOnlyMode(cmd *cobra.Command) error {
// If --from-configmap is not set, no validation needed
fromConfigMapFlag := cmd.Flag("from-configmap")
if fromConfigMapFlag == nil || !fromConfigMapFlag.Changed {
return nil
}

var conflictingFlags []string

// Check all flags that are explicitly set by the user
cmd.Flags().VisitAll(func(flag *pflag.Flag) {
// Skip the from-configmap flag itself and flags that weren't changed
if flag.Name == "from-configmap" || !flag.Changed {
return
}

// Skip flags that are safe to use with ConfigMap (like help, debug, etc.)
if isSafeFlagWithConfigMap(flag.Name) {
return
}

// All other changed flags are considered conflicting
conflictingFlags = append(conflictingFlags, "--"+flag.Name)
})

if len(conflictingFlags) > 0 {
return fmt.Errorf("cannot use --from-configmap with the following flags (configuration should be provided by ConfigMap): %s",
strings.Join(conflictingFlags, ", "))
}

return nil
}

// isSafeFlagWithConfigMap returns true for flags that are safe to use with --from-configmap
// These are typically operational flags that don't affect the RunConfig
func isSafeFlagWithConfigMap(flagName string) bool {
safeFlagsWithConfigMap := map[string]bool{
"help": true,
"debug": true,
// Add other safe flags here if needed
}
return safeFlagsWithConfigMap[flagName]
}

func runCmdFunc(cmd *cobra.Command, args []string) error {
ctx := cmd.Context()

// Handle ConfigMap identification if specified
if runFromConfigMap != "" {
// Validate that conflicting flags are not used with --from-configmap first
if err := validateConfigMapOnlyMode(cmd); err != nil {
return err
}

if err := identifyAndReadConfigMap(ctx, runFromConfigMap); err != nil {
return fmt.Errorf("failed to identify ConfigMap: %w", err)
}
}

// Validate the host flag and default resolving to IP in case hostname is provided
validatedHost, err := ValidateAndNormaliseHostFlag(runHost)
if err != nil {
Expand Down Expand Up @@ -353,3 +433,69 @@ func ValidateAndNormaliseHostFlag(host string) (string, error) {

return "", fmt.Errorf("could not resolve host: %s", host)
}

// parseConfigMapRef parses a ConfigMap reference in the format "namespace/configmap-name"
func parseConfigMapRef(ref string) (namespace, name string, err error) {
parts := strings.SplitN(ref, "/", 2)
if len(parts) != 2 {
return "", "", fmt.Errorf("expected format 'namespace/configmap-name', got '%s'", ref)
}

namespace = strings.TrimSpace(parts[0])
name = strings.TrimSpace(parts[1])

if namespace == "" {
return "", "", fmt.Errorf("namespace cannot be empty")
}
if name == "" {
return "", "", fmt.Errorf("configmap name cannot be empty")
}

return namespace, name, nil
}

// identifyAndReadConfigMap identifies and reads a ConfigMap to validate its existence and contents
// This function only validates the ConfigMap but does not use it for configuration
func identifyAndReadConfigMap(ctx context.Context, configMapRef string) error {
// Parse namespace and ConfigMap name
namespace, configMapName, err := parseConfigMapRef(configMapRef)
if err != nil {
return fmt.Errorf("invalid --from-configmap format: %w", err)
}

logger.Infof("Identifying ConfigMap '%s/%s'", namespace, configMapName)

// Create in-cluster Kubernetes client
config, err := rest.InClusterConfig()
if err != nil {
return fmt.Errorf("failed to create in-cluster config: %w", err)
}

clientset, err := kubernetes.NewForConfig(config)
if err != nil {
return fmt.Errorf("failed to create Kubernetes client: %w", err)
}

// Get the ConfigMap
configMap, err := clientset.CoreV1().ConfigMaps(namespace).Get(ctx, configMapName, metav1.GetOptions{})
if err != nil {
return fmt.Errorf("failed to get ConfigMap '%s/%s': %w", namespace, configMapName, err)
}

// Validate that runconfig.json exists in the ConfigMap
runConfigJSON, ok := configMap.Data["runconfig.json"]
if !ok {
return fmt.Errorf("ConfigMap '%s/%s' does not contain 'runconfig.json' key", namespace, configMapName)
}

// Validate that the JSON is parseable (but don't use it)
var runConfig runner.RunConfig
if err := json.Unmarshal([]byte(runConfigJSON), &runConfig); err != nil {
return fmt.Errorf("ConfigMap '%s/%s' contains invalid runconfig.json: %w", namespace, configMapName, err)
}

logger.Infof("Successfully identified and validated ConfigMap '%s/%s' (contains %d bytes of runconfig.json)",
namespace, configMapName, len(runConfigJSON))

return nil
}
Loading
Loading