Merge pull request #423 from slimslenderslacks/import-json-profile

fix catalog server filtering and implement bm25 search

Author: slimslenderslacks

pkg/gateway/activateprofile.go

@@ -2,46 +2,105 @@ package gateway
 
 import (
 	"context"
+	"database/sql"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"slices"
 	"strings"
 
 	"github.com/google/jsonschema-go/jsonschema"
 	"github.com/modelcontextprotocol/go-sdk/mcp"
 
+	"github.com/docker/mcp-gateway/pkg/catalog"
+	"github.com/docker/mcp-gateway/pkg/config"
 	"github.com/docker/mcp-gateway/pkg/db"
+	"github.com/docker/mcp-gateway/pkg/gateway/project"
 	"github.com/docker/mcp-gateway/pkg/log"
 	"github.com/docker/mcp-gateway/pkg/oci"
+	"github.com/docker/mcp-gateway/pkg/workingset"
 )
 
-// ActivateProfileResult contains the result of profile activation
-type ActivateProfileResult struct {
-	ActivatedServers []string
-	SkippedServers   []string
-	ErrorMessage     string
-}
+var errProfileNotFound = errors.New("profile not found")
 
-// ActivateProfile activates a profile by name, loading its servers into the gateway
-func (g *Gateway) ActivateProfile(ctx context.Context, profileName string) error {
-	// Create database connection
-	dao, err := db.New()
+// loadProfileFromProject attempts to load a profile from the project's profiles.json
+// Returns the WorkingSet if found, or errProfileNotFound if not found
+func loadProfileFromProject(ctx context.Context, profileName string) (*workingset.WorkingSet, error) {
+	profiles, err := project.LoadProfiles(ctx)
 	if err != nil {
-		return fmt.Errorf("failed to create database client: %w", err)
+		return nil, fmt.Errorf("failed to load profiles.json: %w", err)
+	}
+
+	if profile, found := profiles[profileName]; found {
+		log.Log(fmt.Sprintf("- Found profile '%s' in project's profiles.json", profileName))
+		return &profile, nil
+	}
+
+	return nil, errProfileNotFound
+}
+
+// convertWorkingSetToConfiguration converts a WorkingSet to a Configuration object
+func (g *Gateway) convertWorkingSetToConfiguration(ctx context.Context, ws workingset.WorkingSet) (Configuration, error) {
+	// Ensure snapshots are resolved
+	ociService := oci.NewService()
+	if err := ws.EnsureSnapshotsResolved(ctx, ociService); err != nil {
+		return Configuration{}, fmt.Errorf("failed to resolve snapshots: %w", err)
 	}
-	defer dao.Close()
 
-	// Create a temporary WorkingSetConfiguration to load the profile
-	wsConfig := NewWorkingSetConfiguration(
-		Config{WorkingSet: profileName},
-		oci.NewService(),
-		g.docker,
-	)
+	// Build configuration similar to WorkingSetConfiguration.readOnce
+	cfg := make(map[string]map[string]any)
+	configs := make([]ServerSecretConfig, 0, len(ws.Servers))
+	toolsConfig := config.ToolsConfig{ServerTools: make(map[string][]string)}
+	serverNames := make([]string, 0)
+	servers := make(map[string]catalog.Server)
+
+	for _, server := range ws.Servers {
+		// Skip non-image/remote/registry servers
+		if server.Type != workingset.ServerTypeImage &&
+			server.Type != workingset.ServerTypeRemote &&
+			server.Type != workingset.ServerTypeRegistry {
+			continue
+		}
+
+		serverName := server.Snapshot.Server.Name
+		servers[serverName] = server.Snapshot.Server
+		serverNames = append(serverNames, serverName)
+		cfg[serverName] = server.Config
+
+		// Build secrets configs
+		namespace := ""
+		configs = append(configs, ServerSecretConfig{
+			Secrets:   server.Snapshot.Server.Secrets,
+			OAuth:     server.Snapshot.Server.OAuth,
+			Namespace: namespace,
+		})
+
+		// Add tools
+		if server.Tools != nil {
+			toolsConfig.ServerTools[serverName] = server.Tools
+		}
+	}
 
-	// Load the full profile configuration using the existing readOnce method
-	profileConfig, err := wsConfig.readOnce(ctx, dao)
+	secrets := BuildSecretsURIs(ctx, configs)
+
+	return Configuration{
+		serverNames: serverNames,
+		servers:     servers,
+		config:      cfg,
+		tools:       toolsConfig,
+		secrets:     secrets,
+	}, nil
+}
+
+// ActivateProfile activates a profile by merging its servers into the gateway
+// The WorkingSet should be loaded by the caller before calling this method
+func (g *Gateway) ActivateProfile(ctx context.Context, ws workingset.WorkingSet) error {
+	log.Log(fmt.Sprintf("- Activating profile '%s'", ws.Name))
+
+	// Convert WorkingSet to Configuration
+	profileConfig, err := g.convertWorkingSetToConfiguration(ctx, ws)
 	if err != nil {
-		return fmt.Errorf("failed to load profile '%s': %w", profileName, err)
+		return fmt.Errorf("failed to convert profile '%s': %w", ws.Name, err)
 	}
 
 	// Filter servers: only activate servers that are not already active
@@ -59,14 +118,16 @@ func (g *Gateway) ActivateProfile(ctx context.Context, profileName string) error
 	// If no servers to activate, return early
 	if len(serversToActivate) == 0 {
 		if len(skippedServers) > 0 {
-			log.Log(fmt.Sprintf("- All servers from profile '%s' are already active: %s", profileName, strings.Join(skippedServers, ", ")))
+			log.Log(fmt.Sprintf("- All servers from profile '%s' are already active: %s", ws.Name, strings.Join(skippedServers, ", ")))
 		} else {
-			log.Log(fmt.Sprintf("- No new servers to activate from profile '%s'", profileName))
+			log.Log(fmt.Sprintf("- No new servers to activate from profile '%s'", ws.Name))
 		}
 		return nil
 	}
 
-	// Validate ALL servers before activating any (all-or-nothing)
+	// Validate ALL servers before activating any
+	// Note: Validation ensures prerequisites (secrets, config, images) are met.
+	// Actual capability loading happens during activation and may partially succeed.
 	var validationErrors []serverValidation
 
 	for _, serverName := range serversToActivate {
@@ -86,54 +147,75 @@ func (g *Gateway) ActivateProfile(ctx context.Context, profileName string) error
 			serverConfigMap := profileConfig.config[serverName]
 
 			for _, configItem := range serverConfig.Config {
-				// Config items should be schema objects with a "name" property
+				// Config items are object schemas with a "properties" map.
+				// The "name" field is just an identifier, not a key in serverConfigMap.
 				schemaMap, ok := configItem.(map[string]any)
 				if !ok {
 					continue
 				}
 
-				// Get the name field - this identifies which config to validate
-				configName, ok := schemaMap["name"].(string)
-				if !ok || configName == "" {
+				properties, ok := schemaMap["properties"].(map[string]any)
+				if !ok {
 					continue
 				}
 
-				// Get the actual config value to validate
-				if serverConfigMap == nil {
-					validation.missingConfig = append(validation.missingConfig, fmt.Sprintf("%s (missing)", configName))
-					continue
+				// Build a set of required property names
+				requiredProps := make(map[string]bool)
+				if requiredList, ok := schemaMap["required"].([]any); ok {
+					for _, r := range requiredList {
+						if s, ok := r.(string); ok {
+							requiredProps[s] = true
+						}
+					}
 				}
 
-				configValue := serverConfigMap
+				// Validate each property individually
+				for propName, propSchema := range properties {
+					propSchemaMap, ok := propSchema.(map[string]any)
+					if !ok {
+						continue
+					}
 
-				// Convert the schema map to a jsonschema.Schema for validation
-				schemaBytes, err := json.Marshal(schemaMap)
-				if err != nil {
-					validation.missingConfig = append(validation.missingConfig, fmt.Sprintf("%s (invalid schema)", configName))
-					continue
-				}
+					// Get the value from the user-provided config
+					configValue, exists := serverConfigMap[propName]
+					if !exists {
+						// If the property has a default, the server will use it
+						if _, hasDefault := propSchemaMap["default"]; hasDefault {
+							continue
+						}
+						// Only flag as missing if explicitly required
+						if requiredProps[propName] {
+							validation.missingConfig = append(validation.missingConfig, fmt.Sprintf("%s (missing)", propName))
+						}
+						continue
+					}
 
-				var schema jsonschema.Schema
-				if err := json.Unmarshal(schemaBytes, &schema); err != nil {
-					validation.missingConfig = append(validation.missingConfig, fmt.Sprintf("%s (invalid schema)", configName))
-					continue
-				}
+					// Validate the value against the property schema
+					schemaBytes, err := json.Marshal(propSchemaMap)
+					if err != nil {
+						validation.missingConfig = append(validation.missingConfig, fmt.Sprintf("%s (invalid schema)", propName))
+						continue
+					}
 
-				// Resolve the schema
-				resolved, err := schema.Resolve(nil)
-				if err != nil {
-					validation.missingConfig = append(validation.missingConfig, fmt.Sprintf("%s (schema resolution failed)", configName))
-					continue
-				}
+					var propSchemaObj jsonschema.Schema
+					if err := json.Unmarshal(schemaBytes, &propSchemaObj); err != nil {
+						validation.missingConfig = append(validation.missingConfig, fmt.Sprintf("%s (invalid schema)", propName))
+						continue
+					}
 
-				// Validate the config value against the schema
-				if err := resolved.Validate(configValue); err != nil {
-					// Extract a helpful error message
-					errMsg := err.Error()
-					if len(errMsg) > 100 {
-						errMsg = errMsg[:97] + "..."
+					resolved, err := propSchemaObj.Resolve(nil)
+					if err != nil {
+						validation.missingConfig = append(validation.missingConfig, fmt.Sprintf("%s (schema resolution failed)", propName))
+						continue
+					}
+
+					if err := resolved.Validate(configValue); err != nil {
+						errMsg := err.Error()
+						if len(errMsg) > 100 {
+							errMsg = errMsg[:97] + "..."
+						}
+						validation.missingConfig = append(validation.missingConfig, fmt.Sprintf("%s (%s)", propName, errMsg))
 					}
-					validation.missingConfig = append(validation.missingConfig, fmt.Sprintf("%s (%s)", configName, errMsg))
 				}
 			}
 		}
@@ -155,7 +237,7 @@ func (g *Gateway) ActivateProfile(ctx context.Context, profileName string) error
 	// If any validation errors, return detailed error message
 	if len(validationErrors) > 0 {
 		var errorMessages []string
-		errorMessages = append(errorMessages, fmt.Sprintf("Cannot activate profile '%s'. Validation failed for %d server(s):", profileName, len(validationErrors)))
+		errorMessages = append(errorMessages, fmt.Sprintf("Cannot activate profile '%s'. Validation failed for %d server(s):", ws.Name, len(validationErrors)))
 
 		for _, validation := range validationErrors {
 			errorMessages = append(errorMessages, fmt.Sprintf("\nServer '%s':", validation.serverName))
@@ -177,7 +259,12 @@ func (g *Gateway) ActivateProfile(ctx context.Context, profileName string) error
 	}
 
 	// All validations passed - merge configuration into current gateway
+	// Acquire configuration mutex to ensure atomic updates
+	g.configurationMu.Lock()
+	defer g.configurationMu.Unlock()
+
 	var activatedServers []string
+	var failedServers []string
 
 	// Merge secrets once (they're already namespaced in profileConfig)
 	for secretName, secretValue := range profileConfig.secrets {
@@ -211,6 +298,7 @@ func (g *Gateway) ActivateProfile(ctx context.Context, profileName string) error
 		oldCaps, err := g.reloadServerCapabilities(ctx, serverName, nil)
 		if err != nil {
 			log.Log(fmt.Sprintf("Warning: Failed to reload capabilities for server '%s': %v", serverName, err))
+			failedServers = append(failedServers, serverName)
 			// Continue with other servers even if this one fails
 			continue
 		}
@@ -221,6 +309,7 @@ func (g *Gateway) ActivateProfile(ctx context.Context, profileName string) error
 		if err := g.updateServerCapabilities(serverName, oldCaps, newCaps, nil); err != nil {
 			g.capabilitiesMu.Unlock()
 			log.Log(fmt.Sprintf("Warning: Failed to update server capabilities for '%s': %v", serverName, err))
+			failedServers = append(failedServers, serverName)
 			// Continue with other servers even if this one fails
 			continue
 		}
@@ -229,10 +318,20 @@ func (g *Gateway) ActivateProfile(ctx context.Context, profileName string) error
 		activatedServers = append(activatedServers, serverName)
 	}
 
-	log.Log(fmt.Sprintf("- Successfully activated profile '%s' with %d server(s): %s", profileName, len(activatedServers), strings.Join(activatedServers, ", ")))
+	// Log results
+	if len(activatedServers) > 0 {
+		log.Log(fmt.Sprintf("- Successfully activated profile '%s' with %d server(s): %s", ws.Name, len(activatedServers), strings.Join(activatedServers, ", ")))
+	}
 	if len(skippedServers) > 0 {
 		log.Log(fmt.Sprintf("- Skipped %d already-active server(s): %s", len(skippedServers), strings.Join(skippedServers, ", ")))
 	}
+	if len(failedServers) > 0 {
+		log.Log(fmt.Sprintf("- Failed to activate %d server(s): %s", len(failedServers), strings.Join(failedServers, ", ")))
+		// Return error if all servers failed to activate
+		if len(activatedServers) == 0 {
+			return fmt.Errorf("failed to activate any servers from profile '%s'", ws.Name)
+		}
+	}
 
 	return nil
 }
@@ -271,8 +370,49 @@ func activateProfileHandler(g *Gateway, _ *clientConfig) mcp.ToolHandler {
 
 		profileName := strings.TrimSpace(params.Name)
 
-		// Use the ActivateProfile method
-		err = g.ActivateProfile(ctx, profileName)
+		// Load the profile from either profiles.json or database
+		var ws *workingset.WorkingSet
+
+		// First, try to load from project's profiles.json
+		projectProfile, err := loadProfileFromProject(ctx, profileName)
+		if err != nil && !errors.Is(err, errProfileNotFound) {
+			log.Log(fmt.Sprintf("Warning: Failed to check project profiles: %v", err))
+		}
+
+		if projectProfile != nil {
+			// Found in project's profiles.json
+			log.Log(fmt.Sprintf("- Found profile '%s' in project's profiles.json", profileName))
+			ws = projectProfile
+		} else {
+			// Not found in project, try database
+			log.Log(fmt.Sprintf("- Profile '%s' not found in project's profiles.json, checking database", profileName))
+
+			dao, err := db.New()
+			if err != nil {
+				return nil, fmt.Errorf("failed to create database client: %w", err)
+			}
+			defer dao.Close()
+
+			dbProfile, err := dao.GetWorkingSet(ctx, profileName)
+			if err != nil {
+				if errors.Is(err, sql.ErrNoRows) {
+					return &mcp.CallToolResult{
+						Content: []mcp.Content{&mcp.TextContent{
+							Text: fmt.Sprintf("Error: Profile '%s' not found in project or database", profileName),
+						}},
+						IsError: true,
+					}, nil
+				}
+				return nil, fmt.Errorf("failed to load profile from database: %w", err)
+			}
+
+			log.Log(fmt.Sprintf("- Found profile '%s' in database", profileName))
+			wsFromDb := workingset.NewFromDb(dbProfile)
+			ws = &wsFromDb
+		}
+
+		// Activate the profile
+		err = g.ActivateProfile(ctx, *ws)
 		if err != nil {
 			return &mcp.CallToolResult{
 				Content: []mcp.Content{&mcp.TextContent{
@@ -284,7 +424,7 @@ func activateProfileHandler(g *Gateway, _ *clientConfig) mcp.ToolHandler {
 
 		return &mcp.CallToolResult{
 			Content: []mcp.Content{&mcp.TextContent{
-				Text: fmt.Sprintf("Successfully activated profile '%s'", profileName),
+				Text: fmt.Sprintf("Successfully activated profile '%s'", ws.Name),
 			}},
 		}, nil
 	}

pkg/gateway/configuration.go

@@ -209,7 +209,12 @@ func (c *Configuration) FilterByPolicy(ctx context.Context, pc policy.Client) er
 	}
 
 	// Apply filtering based on batch results.
-	filteredServers := make(map[string]catalog.Server)
+	// Start with all existing servers (to preserve catalog servers for mcp-find)
+	filteredServers := make(map[string]catalog.Server, len(c.servers))
+	for name, server := range c.servers {
+		filteredServers[name] = server
+	}
+
 	filteredServerNames := make([]string, 0, len(c.serverNames))
 	filteredConfig := make(map[string]map[string]any)
 	filteredTools := config.ToolsConfig{
@@ -218,6 +223,8 @@ func (c *Configuration) FilterByPolicy(ctx context.Context, pc policy.Client) er
 
 	for _, name := range c.serverNames {
 		if !allowedServers[name] {
+			// Remove denied enabled servers from the servers map
+			delete(filteredServers, name)
 			continue
 		}