From 0f04c88f1ac92f89836124b5ee973eb02f49cc9a Mon Sep 17 00:00:00 2001 From: Sundeep Gottipati Date: Mon, 22 Jun 2026 23:01:46 -0700 Subject: [PATCH 1/4] Validate profile config before gateway startup --- pkg/gateway/activateprofile.go | 106 +-------------- pkg/gateway/config_validation.go | 166 ++++++++++++++++++++++++ pkg/gateway/config_validation_test.go | 83 ++++++++++++ pkg/gateway/configuration_workingset.go | 3 + 4 files changed, 254 insertions(+), 104 deletions(-) create mode 100644 pkg/gateway/config_validation.go create mode 100644 pkg/gateway/config_validation_test.go diff --git a/pkg/gateway/activateprofile.go b/pkg/gateway/activateprofile.go index 17d83055d..a64c2d324 100644 --- a/pkg/gateway/activateprofile.go +++ b/pkg/gateway/activateprofile.go @@ -9,7 +9,6 @@ import ( "slices" "strings" - "github.com/google/jsonschema-go/jsonschema" "github.com/modelcontextprotocol/go-sdk/mcp" "github.com/docker/mcp-gateway/pkg/catalog" @@ -143,81 +142,7 @@ func (g *Gateway) ActivateProfile(ctx context.Context, ws workingset.WorkingSet) // Check if all required config values are set and validate against schema if len(serverConfig.Config) > 0 { - // Get config from profile - serverConfigMap := profileConfig.config[serverName] - - for _, configItem := range serverConfig.Config { - // 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 - } - - properties, ok := schemaMap["properties"].(map[string]any) - if !ok { - 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 - } - } - } - - // Validate each property individually - for propName, propSchema := range properties { - propSchemaMap, ok := propSchema.(map[string]any) - if !ok { - 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 - } - - // 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 - } - - var propSchemaObj jsonschema.Schema - if err := json.Unmarshal(schemaBytes, &propSchemaObj); err != nil { - validation.missingConfig = append(validation.missingConfig, fmt.Sprintf("%s (invalid schema)", propName)) - continue - } - - 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, validateServerConfig(serverConfig, profileConfig.config[serverName])...) } // Validate that Docker image can be pulled @@ -236,26 +161,7 @@ func (g *Gateway) ActivateProfile(ctx context.Context, ws workingset.WorkingSet) // 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):", ws.Name, len(validationErrors))) - - for _, validation := range validationErrors { - errorMessages = append(errorMessages, fmt.Sprintf("\nServer '%s':", validation.serverName)) - - if len(validation.missingSecrets) > 0 { - errorMessages = append(errorMessages, fmt.Sprintf(" Missing secrets: %s", strings.Join(validation.missingSecrets, ", "))) - } - - if len(validation.missingConfig) > 0 { - errorMessages = append(errorMessages, fmt.Sprintf(" Missing/invalid config: %s", strings.Join(validation.missingConfig, ", "))) - } - - if validation.imagePullError != nil { - errorMessages = append(errorMessages, fmt.Sprintf(" Image pull failed: %v", validation.imagePullError)) - } - } - - return fmt.Errorf("%s", strings.Join(errorMessages, "\n")) + return formatProfileValidationError(ws.Name, validationErrors) } // All validations passed - merge configuration into current gateway @@ -336,14 +242,6 @@ func (g *Gateway) ActivateProfile(ctx context.Context, ws workingset.WorkingSet) return nil } -// serverValidation holds validation results for a single server -type serverValidation struct { - serverName string - missingSecrets []string - missingConfig []string - imagePullError error -} - func activateProfileHandler(g *Gateway, _ *clientConfig) mcp.ToolHandler { return func(ctx context.Context, req *mcp.CallToolRequest) (*mcp.CallToolResult, error) { // Parse profile-id parameter diff --git a/pkg/gateway/config_validation.go b/pkg/gateway/config_validation.go new file mode 100644 index 000000000..1d35e9e7e --- /dev/null +++ b/pkg/gateway/config_validation.go @@ -0,0 +1,166 @@ +package gateway + +import ( + "encoding/json" + "fmt" + "strings" + + "github.com/google/jsonschema-go/jsonschema" + + "github.com/docker/mcp-gateway/pkg/catalog" + "github.com/docker/mcp-gateway/pkg/workingset" +) + +// serverValidation holds validation results for a single server. +type serverValidation struct { + serverName string + missingSecrets []string + missingConfig []string + imagePullError error +} + +func validateWorkingSetServerConfigs(ws workingset.WorkingSet) []serverValidation { + var validationErrors []serverValidation + + for _, server := range ws.Servers { + if server.Snapshot == nil { + continue + } + + serverName := server.Snapshot.Server.Name + missingConfig := validateServerConfig(server.Snapshot.Server, server.Config) + if len(missingConfig) == 0 { + continue + } + + validationErrors = append(validationErrors, serverValidation{ + serverName: serverName, + missingConfig: missingConfig, + }) + } + + return validationErrors +} + +func validateServerConfig(server catalog.Server, serverConfigMap map[string]any) []string { + var missingConfig []string + + for _, configItem := range server.Config { + schemaMap, ok := configItem.(map[string]any) + if !ok { + continue + } + + properties, ok := schemaMap["properties"].(map[string]any) + if !ok { + continue + } + + requiredProps := requiredConfigProperties(schemaMap) + + for propName, propSchema := range properties { + propSchemaMap, ok := propSchema.(map[string]any) + if !ok { + continue + } + + configValue, exists := serverConfigMap[propName] + if !exists { + if _, hasDefault := propSchemaMap["default"]; hasDefault { + continue + } + if requiredProps[propName] { + missingConfig = append(missingConfig, fmt.Sprintf("%s (missing)", propName)) + } + continue + } + if isEmptyConfigValue(configValue) { + if requiredProps[propName] { + missingConfig = append(missingConfig, fmt.Sprintf("%s (missing)", propName)) + } + continue + } + + schemaBytes, err := json.Marshal(propSchemaMap) + if err != nil { + missingConfig = append(missingConfig, fmt.Sprintf("%s (invalid schema)", propName)) + continue + } + + var propSchemaObj jsonschema.Schema + if err := json.Unmarshal(schemaBytes, &propSchemaObj); err != nil { + missingConfig = append(missingConfig, fmt.Sprintf("%s (invalid schema)", propName)) + continue + } + + resolved, err := propSchemaObj.Resolve(nil) + if err != nil { + missingConfig = append(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] + "..." + } + missingConfig = append(missingConfig, fmt.Sprintf("%s (%s)", propName, errMsg)) + } + } + } + + return missingConfig +} + +func requiredConfigProperties(schemaMap map[string]any) map[string]bool { + requiredProps := make(map[string]bool) + switch requiredList := schemaMap["required"].(type) { + case []any: + for _, r := range requiredList { + if s, ok := r.(string); ok { + requiredProps[s] = true + } + } + case []string: + for _, s := range requiredList { + requiredProps[s] = true + } + } + return requiredProps +} + +func isEmptyConfigValue(v any) bool { + if v == nil { + return true + } + if s, ok := v.(string); ok { + return s == "" + } + if m, ok := v.(map[string]any); ok { + return len(m) == 0 + } + return false +} + +func formatProfileValidationError(profileName string, validationErrors []serverValidation) error { + var errorMessages []string + errorMessages = append(errorMessages, fmt.Sprintf("Cannot activate profile '%s'. Validation failed for %d server(s):", profileName, len(validationErrors))) + + for _, validation := range validationErrors { + errorMessages = append(errorMessages, fmt.Sprintf("\nServer '%s':", validation.serverName)) + + if len(validation.missingSecrets) > 0 { + errorMessages = append(errorMessages, fmt.Sprintf(" Missing secrets: %s", strings.Join(validation.missingSecrets, ", "))) + } + + if len(validation.missingConfig) > 0 { + errorMessages = append(errorMessages, fmt.Sprintf(" Missing/invalid config: %s", strings.Join(validation.missingConfig, ", "))) + } + + if validation.imagePullError != nil { + errorMessages = append(errorMessages, fmt.Sprintf(" Image pull failed: %v", validation.imagePullError)) + } + } + + return fmt.Errorf("%s", strings.Join(errorMessages, "\n")) +} diff --git a/pkg/gateway/config_validation_test.go b/pkg/gateway/config_validation_test.go new file mode 100644 index 000000000..656d114ba --- /dev/null +++ b/pkg/gateway/config_validation_test.go @@ -0,0 +1,83 @@ +package gateway + +import ( + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/docker/mcp-gateway/pkg/catalog" + "github.com/docker/mcp-gateway/pkg/db" + "github.com/docker/mcp-gateway/pkg/workingset" + "github.com/docker/mcp-gateway/test/mocks" +) + +func TestValidateServerConfigReportsMissingRequiredConfig(t *testing.T) { + server := catalog.Server{ + Config: []any{ + map[string]any{ + "properties": map[string]any{ + "config_path": map[string]any{ + "type": "string", + }, + }, + "required": []any{"config_path"}, + }, + }, + } + + require.Equal(t, []string{"config_path (missing)"}, validateServerConfig(server, nil)) + require.Equal(t, []string{"config_path (missing)"}, validateServerConfig(server, map[string]any{"config_path": ""})) + require.Empty(t, validateServerConfig(server, map[string]any{"config_path": "/home/user/.kube/config"})) +} + +func TestWorkingSetConfigurationReadRejectsMissingRequiredConfig(t *testing.T) { + dao, err := db.New(db.WithDatabaseFile(filepath.Join(t.TempDir(), "test.db"))) + require.NoError(t, err) + t.Cleanup(func() { + require.NoError(t, dao.Close()) + }) + + kubernetesServer := catalog.Server{ + Name: "kubernetes", + Type: "server", + Image: "mcp/kubernetes@sha256:ad0316b4ddcc61356d45abce4abf5b090c85cd20ffbff5dd0b6e743db11cb788", + Volumes: []string{ + "{{kubernetes.config_path}}:/home/appuser/.kube/config", + }, + Config: []any{ + map[string]any{ + "name": "kubernetes", + "properties": map[string]any{ + "config_path": map[string]any{ + "type": "string", + }, + }, + "required": []any{"config_path"}, + "type": "object", + }, + }, + } + + err = dao.CreateWorkingSet(t.Context(), db.WorkingSet{ + ID: "test", + Name: "test", + Servers: db.ServerList{ + { + Type: string(workingset.ServerTypeImage), + Image: kubernetesServer.Image, + Snapshot: &db.ServerSnapshot{ + Server: kubernetesServer, + }, + }, + }, + Secrets: db.SecretMap{}, + }) + require.NoError(t, err) + + cfg := NewWorkingSetConfiguration(Config{WorkingSet: "test"}, mocks.NewMockOCIService(), nil) + _, err = cfg.readOnce(t.Context(), dao) + require.ErrorContains(t, err, "Cannot activate profile 'test'") + require.ErrorContains(t, err, "Server 'kubernetes'") + require.ErrorContains(t, err, "Missing/invalid config: config_path (missing)") +} diff --git a/pkg/gateway/configuration_workingset.go b/pkg/gateway/configuration_workingset.go index b940d9efd..ab2048bad 100644 --- a/pkg/gateway/configuration_workingset.go +++ b/pkg/gateway/configuration_workingset.go @@ -74,6 +74,9 @@ func (c *WorkingSetConfiguration) readOnce(ctx context.Context, dao db.DAO) (Con if err := workingSet.EnsureSnapshotsResolved(ctx, c.ociService); err != nil { return Configuration{}, fmt.Errorf("failed to resolve snapshots: %w", err) } + if validationErrors := validateWorkingSetServerConfigs(workingSet); len(validationErrors) > 0 { + return Configuration{}, formatProfileValidationError(workingSet.Name, validationErrors) + } cfg := make(map[string]map[string]any) From e8bbf861ed689a9dfcd67cdfd032463c2c296643 Mon Sep 17 00:00:00 2001 From: Sundeep Gottipati Date: Tue, 23 Jun 2026 01:13:25 -0700 Subject: [PATCH 2/4] Allow empty object profile config values --- pkg/gateway/config_validation.go | 3 --- pkg/gateway/config_validation_test.go | 17 +++++++++++++++++ 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/pkg/gateway/config_validation.go b/pkg/gateway/config_validation.go index 1d35e9e7e..847294616 100644 --- a/pkg/gateway/config_validation.go +++ b/pkg/gateway/config_validation.go @@ -136,9 +136,6 @@ func isEmptyConfigValue(v any) bool { if s, ok := v.(string); ok { return s == "" } - if m, ok := v.(map[string]any); ok { - return len(m) == 0 - } return false } diff --git a/pkg/gateway/config_validation_test.go b/pkg/gateway/config_validation_test.go index 656d114ba..501d9983f 100644 --- a/pkg/gateway/config_validation_test.go +++ b/pkg/gateway/config_validation_test.go @@ -31,6 +31,23 @@ func TestValidateServerConfigReportsMissingRequiredConfig(t *testing.T) { require.Empty(t, validateServerConfig(server, map[string]any{"config_path": "/home/user/.kube/config"})) } +func TestValidateServerConfigAllowsEmptyObjectValue(t *testing.T) { + server := catalog.Server{ + Config: []any{ + map[string]any{ + "properties": map[string]any{ + "headers": map[string]any{ + "type": "object", + }, + }, + "required": []any{"headers"}, + }, + }, + } + + require.Empty(t, validateServerConfig(server, map[string]any{"headers": map[string]any{}})) +} + func TestWorkingSetConfigurationReadRejectsMissingRequiredConfig(t *testing.T) { dao, err := db.New(db.WithDatabaseFile(filepath.Join(t.TempDir(), "test.db"))) require.NoError(t, err) From 60153709b36fcbd5569f9297dcf1d4e2ac7fb075 Mon Sep 17 00:00:00 2001 From: Sundeep Gottipati Date: Wed, 24 Jun 2026 10:04:47 -0700 Subject: [PATCH 3/4] Add profile config validation regression tests --- pkg/gateway/config_validation_test.go | 112 ++++++++++++++++++++++++++ 1 file changed, 112 insertions(+) diff --git a/pkg/gateway/config_validation_test.go b/pkg/gateway/config_validation_test.go index 501d9983f..4ad410033 100644 --- a/pkg/gateway/config_validation_test.go +++ b/pkg/gateway/config_validation_test.go @@ -1,6 +1,10 @@ package gateway import ( + "context" + "net" + "net/http" + "os" "path/filepath" "testing" @@ -48,6 +52,79 @@ func TestValidateServerConfigAllowsEmptyObjectValue(t *testing.T) { require.Empty(t, validateServerConfig(server, map[string]any{"headers": map[string]any{}})) } +func TestValidateServerConfigRejectsWrongTypeForRequiredString(t *testing.T) { + server := catalog.Server{ + Config: []any{ + map[string]any{ + "properties": map[string]any{ + "config_path": map[string]any{ + "type": "string", + }, + }, + "required": []any{"config_path"}, + }, + }, + } + + for _, value := range []any{map[string]any{}, []any{}} { + missingConfig := validateServerConfig(server, map[string]any{"config_path": value}) + require.Len(t, missingConfig, 1) + require.Contains(t, missingConfig[0], "config_path (") + require.NotContains(t, missingConfig[0], "missing") + } +} + +func TestActivateProfileRejectsMissingRequiredConfig(t *testing.T) { + serveEmptySecretsEngine(t) + + server := catalog.Server{ + Name: "kubernetes", + Type: "remote", + Remote: catalog.Remote{ + URL: "https://mcp.example.com/mcp", + }, + Secrets: []catalog.Secret{ + { + Name: "KUBECONFIG_TOKEN", + Env: "KUBECONFIG_TOKEN", + }, + }, + Config: []any{ + map[string]any{ + "name": "kubernetes", + "properties": map[string]any{ + "config_path": map[string]any{ + "type": "string", + }, + }, + "required": []any{"config_path"}, + "type": "object", + }, + }, + } + + g := &Gateway{} + err := g.ActivateProfile(t.Context(), workingset.WorkingSet{ + ID: "test", + Name: "test", + Servers: []workingset.Server{ + { + Type: workingset.ServerTypeRemote, + Endpoint: server.Remote.URL, + Config: map[string]any{}, + Snapshot: &workingset.ServerSnapshot{ + Server: server, + }, + }, + }, + }) + + require.ErrorContains(t, err, "Cannot activate profile 'test'") + require.ErrorContains(t, err, "Server 'kubernetes'") + require.ErrorContains(t, err, "Missing secrets: KUBECONFIG_TOKEN") + require.ErrorContains(t, err, "Missing/invalid config: config_path (missing)") +} + func TestWorkingSetConfigurationReadRejectsMissingRequiredConfig(t *testing.T) { dao, err := db.New(db.WithDatabaseFile(filepath.Join(t.TempDir(), "test.db"))) require.NoError(t, err) @@ -98,3 +175,38 @@ func TestWorkingSetConfigurationReadRejectsMissingRequiredConfig(t *testing.T) { require.ErrorContains(t, err, "Server 'kubernetes'") require.ErrorContains(t, err, "Missing/invalid config: config_path (missing)") } + +func serveEmptySecretsEngine(t *testing.T) { + t.Helper() + + home, err := os.MkdirTemp("/tmp", "mcp-gateway-test-home-") + require.NoError(t, err) + t.Cleanup(func() { + require.NoError(t, os.RemoveAll(home)) + }) + + t.Setenv("HOME", home) + t.Setenv("XDG_CACHE_HOME", filepath.Join(home, ".cache")) + + cacheDir, err := os.UserCacheDir() + require.NoError(t, err) + socketPath := filepath.Join(cacheDir, "docker-secrets-engine", "engine.sock") + require.NoError(t, os.MkdirAll(filepath.Dir(socketPath), 0o755)) + + listener, err := net.Listen("unix", socketPath) + require.NoError(t, err) + + server := &http.Server{ + Handler: http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) + }), + } + + go func() { + _ = server.Serve(listener) + }() + + t.Cleanup(func() { + require.NoError(t, server.Shutdown(context.Background())) + }) +} From 6b6d502f981bb90ae87b8c01a3668cb1fe1d0c23 Mon Sep 17 00:00:00 2001 From: Sundeep Gottipati Date: Wed, 24 Jun 2026 10:14:47 -0700 Subject: [PATCH 4/4] Fix profile validation test lint --- pkg/gateway/config_validation_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/gateway/config_validation_test.go b/pkg/gateway/config_validation_test.go index 4ad410033..0dec5f3ac 100644 --- a/pkg/gateway/config_validation_test.go +++ b/pkg/gateway/config_validation_test.go @@ -193,7 +193,7 @@ func serveEmptySecretsEngine(t *testing.T) { socketPath := filepath.Join(cacheDir, "docker-secrets-engine", "engine.sock") require.NoError(t, os.MkdirAll(filepath.Dir(socketPath), 0o755)) - listener, err := net.Listen("unix", socketPath) + listener, err := (&net.ListenConfig{}).Listen(t.Context(), "unix", socketPath) require.NoError(t, err) server := &http.Server{