diff --git a/pkg/mcp/tools_config_envs.go b/pkg/mcp/tools_config_envs.go index 397b8c1eef..3c6a642fd4 100644 --- a/pkg/mcp/tools_config_envs.go +++ b/pkg/mcp/tools_config_envs.go @@ -3,8 +3,10 @@ package mcp import ( "context" "fmt" + "strings" "github.com/modelcontextprotocol/go-sdk/mcp" + fn "knative.dev/func/pkg/functions" ) // config_envs_list @@ -21,30 +23,56 @@ var configEnvsListTool = &mcp.Tool{ } type ConfigEnvsListInput struct { - Path string `json:"path" jsonschema:"required,Path to the function project directory"` - Verbose *bool `json:"verbose,omitempty" jsonschema:"Enable verbose logging output"` + Path string `json:"path" jsonschema:"required,Path to the function project directory"` } -func (i ConfigEnvsListInput) Args() []string { - args := []string{"envs", "--path", i.Path} - args = appendBoolFlag(args, "--verbose", i.Verbose) - return args +// EnvVar is the MCP-facing representation of a configured environment +// variable. It mirrors fn.Env without the invopop/jsonschema struct tags, +// which the MCP SDK's output-schema generator cannot parse. +type EnvVar struct { + Name *string `json:"name,omitempty" jsonschema:"Environment variable name"` + Value *string `json:"value,omitempty" jsonschema:"Literal value or template expression"` } +// ConfigEnvsListOutput exposes the configured environment variables as typed +// structured data; Message is a human-readable summary for fallback display. type ConfigEnvsListOutput struct { - Message string `json:"message" jsonschema:"Output message"` + Envs []EnvVar `json:"envs" jsonschema:"Configured environment variables"` + Message string `json:"message" jsonschema:"Human-readable summary"` } -func (s *Server) configEnvsListHandler(ctx context.Context, r *mcp.CallToolRequest, input ConfigEnvsListInput) (result *mcp.CallToolResult, output ConfigEnvsListOutput, err error) { - out, err := s.executor.Execute(ctx, "config", input.Args()...) +// configEnvsListHandler loads the function at the given path and returns its +// configured environment variables directly from pkg/functions rather than +// shelling out to `func config envs`. Part of the migration tracked in +// https://github.com/knative/func/issues/3771. +func (s *Server) configEnvsListHandler(_ context.Context, _ *mcp.CallToolRequest, input ConfigEnvsListInput) (result *mcp.CallToolResult, output ConfigEnvsListOutput, err error) { + f, err := fn.NewFunction(input.Path) if err != nil { - err = fmt.Errorf("%w\n%s", err, string(out)) return } - output = ConfigEnvsListOutput{Message: string(out)} + envs := make([]EnvVar, len(f.Run.Envs)) + for i, e := range f.Run.Envs { + envs[i] = EnvVar{Name: e.Name, Value: e.Value} + } + output = ConfigEnvsListOutput{ + Envs: envs, + Message: formatEnvs(f.Run.Envs), + } return } +func formatEnvs(envs fn.Envs) string { + if len(envs) == 0 { + return "There aren't any configured Environment variables" + } + var b strings.Builder + b.WriteString("Configured Environment variables:\n") + for _, e := range envs { + fmt.Fprintf(&b, " - %s\n", e.String()) + } + return b.String() +} + // config_envs_add var configEnvsAddTool = &mcp.Tool{ diff --git a/pkg/mcp/tools_config_envs_test.go b/pkg/mcp/tools_config_envs_test.go index 327ab06f9a..8b1b45b06f 100644 --- a/pkg/mcp/tools_config_envs_test.go +++ b/pkg/mcp/tools_config_envs_test.go @@ -2,13 +2,29 @@ package mcp import ( "context" + "encoding/json" "errors" + "path/filepath" "testing" "github.com/modelcontextprotocol/go-sdk/mcp" + fn "knative.dev/func/pkg/functions" "knative.dev/func/pkg/mcp/mock" ) +// writeTestFunction writes a minimal valid function to root, applying modify +// to set envs/labels/volumes for the test. Shared by the config *_list tests. +func writeTestFunction(t *testing.T, root string, modify func(*fn.Function)) { + t.Helper() + f := fn.NewFunctionWith(fn.Function{Root: root, Name: "test-fn", Runtime: "go"}) + if modify != nil { + modify(&f) + } + if err := f.Write(); err != nil { + t.Fatalf("writing test function: %v", err) + } +} + // TestTool_ConfigEnvsAdd ensures the config_envs_add tool executes with all arguments. func TestTool_ConfigEnvsAdd(t *testing.T) { stringFlags := map[string]struct { @@ -456,38 +472,23 @@ func TestTool_ConfigEnvsAdd_ConfigMapKeyWithoutConfigMapName(t *testing.T) { } } -// TestTool_ConfigEnvsList ensures the config_envs_list tool lists environment variables. +// TestTool_ConfigEnvsList verifies the config_envs_list tool reads envs +// directly from the function on disk via pkg/functions (no subprocess) and +// returns them as structured output. func TestTool_ConfigEnvsList(t *testing.T) { - executor := mock.NewExecutor() - executor.ExecuteFn = func(ctx context.Context, subcommand string, args ...string) ([]byte, error) { - if subcommand != "config" { - t.Fatalf("expected subcommand 'config', got %q", subcommand) - } - - // "envs" + "--path" + "." = 3 args - if len(args) != 3 { - t.Fatalf("expected 3 args, got %d: %v", len(args), args) - } - if args[0] != "envs" { - t.Fatalf("expected args[0]='envs', got %q", args[0]) - } - - argsMap := argsToMap(args[1:]) - if val, ok := argsMap["--path"]; !ok || val != "." { - t.Fatalf("expected --path='.', got %q", val) - } - - return []byte("DATABASE_URL=postgres://localhost\nAPI_KEY=secret\n"), nil - } + root := t.TempDir() + writeTestFunction(t, root, func(f *fn.Function) { + f.Run.Envs = fn.Envs{{Name: ptr("API_KEY"), Value: ptr("secret")}} + }) - client, _, err := newTestPair(t, WithExecutor(executor)) + client, _, err := newTestPair(t) if err != nil { t.Fatal(err) } result, err := client.CallTool(t.Context(), &mcp.CallToolParams{ Name: "config_envs_list", - Arguments: map[string]any{"path": "."}, + Arguments: map[string]any{"path": root}, }) if err != nil { t.Fatal(err) @@ -495,8 +496,20 @@ func TestTool_ConfigEnvsList(t *testing.T) { if result.IsError { t.Fatalf("unexpected error result: %v", result) } - if !executor.ExecuteInvoked { - t.Fatal("executor was not invoked") + if result.StructuredContent == nil { + t.Fatal("expected StructuredContent to be populated") + } + + raw, err := json.Marshal(result.StructuredContent) + if err != nil { + t.Fatalf("marshal structured content: %v", err) + } + var out ConfigEnvsListOutput + if err := json.Unmarshal(raw, &out); err != nil { + t.Fatalf("unmarshal output: %v", err) + } + if len(out.Envs) != 1 || out.Envs[0].Name == nil || *out.Envs[0].Name != "API_KEY" { + t.Fatalf("unexpected envs in output: %+v", out.Envs) } } @@ -556,27 +569,23 @@ func TestTool_ConfigEnvsRemove(t *testing.T) { } } -// TestTool_ConfigEnvsList_Error ensures the config_envs_list tool propagates executor errors. +// TestTool_ConfigEnvsList_Error ensures the config_envs_list tool returns an +// error result when the function path does not exist. func TestTool_ConfigEnvsList_Error(t *testing.T) { - executor := mock.NewExecutor() - executor.ExecuteFn = func(ctx context.Context, subcommand string, args ...string) ([]byte, error) { - return []byte("list failed"), errors.New("executor error") - } - - client, _, err := newTestPair(t, WithExecutor(executor)) + client, _, err := newTestPair(t) if err != nil { t.Fatal(err) } result, err := client.CallTool(t.Context(), &mcp.CallToolParams{ Name: "config_envs_list", - Arguments: map[string]any{"path": "."}, + Arguments: map[string]any{"path": filepath.Join(t.TempDir(), "does-not-exist")}, }) if err != nil { t.Fatal(err) } if !result.IsError { - t.Fatal("expected error result, got success") + t.Fatal("expected error result for nonexistent path") } } diff --git a/pkg/mcp/tools_config_labels.go b/pkg/mcp/tools_config_labels.go index 0c62e87290..36993ecadf 100644 --- a/pkg/mcp/tools_config_labels.go +++ b/pkg/mcp/tools_config_labels.go @@ -3,8 +3,10 @@ package mcp import ( "context" "fmt" + "strings" "github.com/modelcontextprotocol/go-sdk/mcp" + fn "knative.dev/func/pkg/functions" ) // config_labels_list @@ -21,30 +23,56 @@ var configLabelsListTool = &mcp.Tool{ } type ConfigLabelsListInput struct { - Path string `json:"path" jsonschema:"required,Path to the function project directory"` - Verbose *bool `json:"verbose,omitempty" jsonschema:"Enable verbose logging output"` + Path string `json:"path" jsonschema:"required,Path to the function project directory"` } -func (i ConfigLabelsListInput) Args() []string { - args := []string{"labels", "--path", i.Path} - args = appendBoolFlag(args, "--verbose", i.Verbose) - return args +// LabelPair is the MCP-facing representation of a configured label. It +// mirrors fn.Label without the invopop/jsonschema struct tags, which the MCP +// SDK's output-schema generator cannot parse. +type LabelPair struct { + Key *string `json:"key,omitempty" jsonschema:"Label key"` + Value *string `json:"value,omitempty" jsonschema:"Label value or template expression"` } +// ConfigLabelsListOutput exposes the configured labels as typed structured +// data; Message is a human-readable summary for fallback display. type ConfigLabelsListOutput struct { - Message string `json:"message" jsonschema:"Output message"` + Labels []LabelPair `json:"labels" jsonschema:"Configured labels"` + Message string `json:"message" jsonschema:"Human-readable summary"` } -func (s *Server) configLabelsListHandler(ctx context.Context, r *mcp.CallToolRequest, input ConfigLabelsListInput) (result *mcp.CallToolResult, output ConfigLabelsListOutput, err error) { - out, err := s.executor.Execute(ctx, "config", input.Args()...) +// configLabelsListHandler loads the function at the given path and returns its +// configured labels directly from pkg/functions rather than shelling out to +// `func config labels`. Part of the migration tracked in +// https://github.com/knative/func/issues/3771. +func (s *Server) configLabelsListHandler(_ context.Context, _ *mcp.CallToolRequest, input ConfigLabelsListInput) (result *mcp.CallToolResult, output ConfigLabelsListOutput, err error) { + f, err := fn.NewFunction(input.Path) if err != nil { - err = fmt.Errorf("%w\n%s", err, string(out)) return } - output = ConfigLabelsListOutput{Message: string(out)} + labels := make([]LabelPair, len(f.Deploy.Labels)) + for i, l := range f.Deploy.Labels { + labels[i] = LabelPair{Key: l.Key, Value: l.Value} + } + output = ConfigLabelsListOutput{ + Labels: labels, + Message: formatLabels(f.Deploy.Labels), + } return } +func formatLabels(labels []fn.Label) string { + if len(labels) == 0 { + return "No labels defined" + } + var b strings.Builder + b.WriteString("Labels:\n") + for _, l := range labels { + fmt.Fprintf(&b, " - %s\n", l.String()) + } + return b.String() +} + // config_labels_add var configLabelsAddTool = &mcp.Tool{ diff --git a/pkg/mcp/tools_config_labels_test.go b/pkg/mcp/tools_config_labels_test.go index e3d2a42c83..02911b3537 100644 --- a/pkg/mcp/tools_config_labels_test.go +++ b/pkg/mcp/tools_config_labels_test.go @@ -2,10 +2,13 @@ package mcp import ( "context" + "encoding/json" "errors" + "path/filepath" "testing" "github.com/modelcontextprotocol/go-sdk/mcp" + fn "knative.dev/func/pkg/functions" "knative.dev/func/pkg/mcp/mock" ) @@ -71,38 +74,23 @@ func TestTool_ConfigLabelsAdd(t *testing.T) { } } -// TestTool_ConfigLabelsList ensures the config_labels_list tool lists labels. +// TestTool_ConfigLabelsList verifies the config_labels_list tool reads labels +// directly from the function on disk via pkg/functions (no subprocess) and +// returns them as structured output. func TestTool_ConfigLabelsList(t *testing.T) { - executor := mock.NewExecutor() - executor.ExecuteFn = func(ctx context.Context, subcommand string, args ...string) ([]byte, error) { - if subcommand != "config" { - t.Fatalf("expected subcommand 'config', got %q", subcommand) - } - - // "labels" + "--path" + "." = 3 args - if len(args) != 3 { - t.Fatalf("expected 3 args, got %d: %v", len(args), args) - } - if args[0] != "labels" { - t.Fatalf("expected args[0]='labels', got %q", args[0]) - } - - argsMap := argsToMap(args[1:]) - if val, ok := argsMap["--path"]; !ok || val != "." { - t.Fatalf("expected --path='.', got %q", val) - } - - return []byte("app=my-function\nenvironment=prod\n"), nil - } + root := t.TempDir() + writeTestFunction(t, root, func(f *fn.Function) { + f.Deploy.Labels = []fn.Label{{Key: ptr("environment"), Value: ptr("prod")}} + }) - client, _, err := newTestPair(t, WithExecutor(executor)) + client, _, err := newTestPair(t) if err != nil { t.Fatal(err) } result, err := client.CallTool(t.Context(), &mcp.CallToolParams{ Name: "config_labels_list", - Arguments: map[string]any{"path": "."}, + Arguments: map[string]any{"path": root}, }) if err != nil { t.Fatal(err) @@ -110,8 +98,20 @@ func TestTool_ConfigLabelsList(t *testing.T) { if result.IsError { t.Fatalf("unexpected error result: %v", result) } - if !executor.ExecuteInvoked { - t.Fatal("executor was not invoked") + if result.StructuredContent == nil { + t.Fatal("expected StructuredContent to be populated") + } + + raw, err := json.Marshal(result.StructuredContent) + if err != nil { + t.Fatalf("marshal structured content: %v", err) + } + var out ConfigLabelsListOutput + if err := json.Unmarshal(raw, &out); err != nil { + t.Fatalf("unmarshal output: %v", err) + } + if len(out.Labels) != 1 || out.Labels[0].Key == nil || *out.Labels[0].Key != "environment" { + t.Fatalf("unexpected labels in output: %+v", out.Labels) } } @@ -171,27 +171,23 @@ func TestTool_ConfigLabelsRemove(t *testing.T) { } } -// TestTool_ConfigLabelsList_Error ensures the config_labels_list tool propagates executor errors. +// TestTool_ConfigLabelsList_Error ensures the config_labels_list tool returns +// an error result when the function path does not exist. func TestTool_ConfigLabelsList_Error(t *testing.T) { - executor := mock.NewExecutor() - executor.ExecuteFn = func(ctx context.Context, subcommand string, args ...string) ([]byte, error) { - return []byte("list failed"), errors.New("executor error") - } - - client, _, err := newTestPair(t, WithExecutor(executor)) + client, _, err := newTestPair(t) if err != nil { t.Fatal(err) } result, err := client.CallTool(t.Context(), &mcp.CallToolParams{ Name: "config_labels_list", - Arguments: map[string]any{"path": "."}, + Arguments: map[string]any{"path": filepath.Join(t.TempDir(), "does-not-exist")}, }) if err != nil { t.Fatal(err) } if !result.IsError { - t.Fatal("expected error result, got success") + t.Fatal("expected error result for nonexistent path") } } diff --git a/pkg/mcp/tools_config_volumes.go b/pkg/mcp/tools_config_volumes.go index 232d866d30..73b61445c1 100644 --- a/pkg/mcp/tools_config_volumes.go +++ b/pkg/mcp/tools_config_volumes.go @@ -3,8 +3,10 @@ package mcp import ( "context" "fmt" + "strings" "github.com/modelcontextprotocol/go-sdk/mcp" + fn "knative.dev/func/pkg/functions" ) // config_volumes_list @@ -21,30 +23,88 @@ var configVolumesListTool = &mcp.Tool{ } type ConfigVolumesListInput struct { - Path string `json:"path" jsonschema:"required,Path to the function project directory"` - Verbose *bool `json:"verbose,omitempty" jsonschema:"Enable verbose logging output"` + Path string `json:"path" jsonschema:"required,Path to the function project directory"` } -func (i ConfigVolumesListInput) Args() []string { - args := []string{"volumes", "--path", i.Path} - args = appendBoolFlag(args, "--verbose", i.Verbose) - return args +// VolumeMount is the MCP-facing representation of a configured volume mount. +// It mirrors fn.Volume without the invopop/jsonschema struct tags, which the +// MCP SDK's output-schema generator cannot parse. +type VolumeMount struct { + Secret *string `json:"secret,omitempty" jsonschema:"Name of the Secret to mount"` + ConfigMap *string `json:"configMap,omitempty" jsonschema:"Name of the ConfigMap to mount"` + PersistentVolumeClaim *VolumePVC `json:"persistentVolumeClaim,omitempty" jsonschema:"PersistentVolumeClaim mount"` + EmptyDir *VolumeEmptyDir `json:"emptyDir,omitempty" jsonschema:"EmptyDir mount"` + Path *string `json:"path,omitempty" jsonschema:"Mount path in the container"` +} + +// VolumePVC mirrors fn.PersistentVolumeClaim for MCP output. +type VolumePVC struct { + ClaimName *string `json:"claimName,omitempty" jsonschema:"Name of the PersistentVolumeClaim"` + ReadOnly bool `json:"readOnly,omitempty" jsonschema:"Mount the volume read-only"` } +// VolumeEmptyDir mirrors fn.EmptyDir for MCP output. +type VolumeEmptyDir struct { + Medium string `json:"medium,omitempty" jsonschema:"Storage medium (empty or 'Memory')"` + SizeLimit *string `json:"sizeLimit,omitempty" jsonschema:"Maximum size limit"` +} + +// ConfigVolumesListOutput exposes the configured volume mounts as typed +// structured data; Message is a human-readable summary for fallback display. type ConfigVolumesListOutput struct { - Message string `json:"message" jsonschema:"Output message"` + Volumes []VolumeMount `json:"volumes" jsonschema:"Configured volume mounts"` + Message string `json:"message" jsonschema:"Human-readable summary"` } -func (s *Server) configVolumesListHandler(ctx context.Context, r *mcp.CallToolRequest, input ConfigVolumesListInput) (result *mcp.CallToolResult, output ConfigVolumesListOutput, err error) { - out, err := s.executor.Execute(ctx, "config", input.Args()...) +// configVolumesListHandler loads the function at the given path and returns its +// configured volume mounts directly from pkg/functions rather than shelling +// out to `func config volumes`. Part of the migration tracked in +// https://github.com/knative/func/issues/3771. +func (s *Server) configVolumesListHandler(_ context.Context, _ *mcp.CallToolRequest, input ConfigVolumesListInput) (result *mcp.CallToolResult, output ConfigVolumesListOutput, err error) { + f, err := fn.NewFunction(input.Path) if err != nil { - err = fmt.Errorf("%w\n%s", err, string(out)) return } - output = ConfigVolumesListOutput{Message: string(out)} + volumes := make([]VolumeMount, len(f.Run.Volumes)) + for i, v := range f.Run.Volumes { + vm := VolumeMount{ + Secret: v.Secret, + ConfigMap: v.ConfigMap, + Path: v.Path, + } + if v.PersistentVolumeClaim != nil { + vm.PersistentVolumeClaim = &VolumePVC{ + ClaimName: v.PersistentVolumeClaim.ClaimName, + ReadOnly: v.PersistentVolumeClaim.ReadOnly, + } + } + if v.EmptyDir != nil { + vm.EmptyDir = &VolumeEmptyDir{ + Medium: v.EmptyDir.Medium, + SizeLimit: v.EmptyDir.SizeLimit, + } + } + volumes[i] = vm + } + output = ConfigVolumesListOutput{ + Volumes: volumes, + Message: formatVolumes(f.Run.Volumes), + } return } +func formatVolumes(volumes []fn.Volume) string { + if len(volumes) == 0 { + return "There aren't any configured Volume mounts" + } + var b strings.Builder + b.WriteString("Configured Volumes mounts:\n") + for _, v := range volumes { + fmt.Fprintf(&b, " - %s\n", v.String()) + } + return b.String() +} + // config_volumes_add var configVolumesAddTool = &mcp.Tool{ diff --git a/pkg/mcp/tools_config_volumes_test.go b/pkg/mcp/tools_config_volumes_test.go index bc10965554..196ab89daa 100644 --- a/pkg/mcp/tools_config_volumes_test.go +++ b/pkg/mcp/tools_config_volumes_test.go @@ -2,10 +2,13 @@ package mcp import ( "context" + "encoding/json" "errors" + "path/filepath" "testing" "github.com/modelcontextprotocol/go-sdk/mcp" + fn "knative.dev/func/pkg/functions" "knative.dev/func/pkg/mcp/mock" ) @@ -75,38 +78,26 @@ func TestTool_ConfigVolumesAdd(t *testing.T) { } } -// TestTool_ConfigVolumesList ensures the config_volumes_list tool lists volumes. +// TestTool_ConfigVolumesList verifies the config_volumes_list tool reads +// volume mounts directly from the function on disk via pkg/functions (no +// subprocess) and returns them as structured output. func TestTool_ConfigVolumesList(t *testing.T) { - executor := mock.NewExecutor() - executor.ExecuteFn = func(ctx context.Context, subcommand string, args ...string) ([]byte, error) { - if subcommand != "config" { - t.Fatalf("expected subcommand 'config', got %q", subcommand) - } - - // "volumes" + "--path" + "." = 3 args - if len(args) != 3 { - t.Fatalf("expected 3 args, got %d: %v", len(args), args) - } - if args[0] != "volumes" { - t.Fatalf("expected args[0]='volumes', got %q", args[0]) - } - - argsMap := argsToMap(args[1:]) - if val, ok := argsMap["--path"]; !ok || val != "." { - t.Fatalf("expected --path='.', got %q", val) - } - - return []byte("secret:my-secret:/workspace/secret\n"), nil - } + root := t.TempDir() + writeTestFunction(t, root, func(f *fn.Function) { + f.Run.Volumes = []fn.Volume{{ + Secret: ptr("my-secret"), + Path: ptr("/workspace/secret"), + }} + }) - client, _, err := newTestPair(t, WithExecutor(executor)) + client, _, err := newTestPair(t) if err != nil { t.Fatal(err) } result, err := client.CallTool(t.Context(), &mcp.CallToolParams{ Name: "config_volumes_list", - Arguments: map[string]any{"path": "."}, + Arguments: map[string]any{"path": root}, }) if err != nil { t.Fatal(err) @@ -114,8 +105,20 @@ func TestTool_ConfigVolumesList(t *testing.T) { if result.IsError { t.Fatalf("unexpected error result: %v", result) } - if !executor.ExecuteInvoked { - t.Fatal("executor was not invoked") + if result.StructuredContent == nil { + t.Fatal("expected StructuredContent to be populated") + } + + raw, err := json.Marshal(result.StructuredContent) + if err != nil { + t.Fatalf("marshal structured content: %v", err) + } + var out ConfigVolumesListOutput + if err := json.Unmarshal(raw, &out); err != nil { + t.Fatalf("unmarshal output: %v", err) + } + if len(out.Volumes) != 1 || out.Volumes[0].Secret == nil || *out.Volumes[0].Secret != "my-secret" { + t.Fatalf("unexpected volumes in output: %+v", out.Volumes) } } @@ -175,27 +178,23 @@ func TestTool_ConfigVolumesRemove(t *testing.T) { } } -// TestTool_ConfigVolumesList_Error ensures the config_volumes_list tool propagates executor errors. +// TestTool_ConfigVolumesList_Error ensures the config_volumes_list tool +// returns an error result when the function path does not exist. func TestTool_ConfigVolumesList_Error(t *testing.T) { - executor := mock.NewExecutor() - executor.ExecuteFn = func(ctx context.Context, subcommand string, args ...string) ([]byte, error) { - return []byte("list failed"), errors.New("executor error") - } - - client, _, err := newTestPair(t, WithExecutor(executor)) + client, _, err := newTestPair(t) if err != nil { t.Fatal(err) } result, err := client.CallTool(t.Context(), &mcp.CallToolParams{ Name: "config_volumes_list", - Arguments: map[string]any{"path": "."}, + Arguments: map[string]any{"path": filepath.Join(t.TempDir(), "does-not-exist")}, }) if err != nil { t.Fatal(err) } if !result.IsError { - t.Fatal("expected error result, got success") + t.Fatal("expected error result for nonexistent path") } }