feat(mcp): integrate tools with functions.Client API instead of CLI shell-out

cmd/mcp.go

@@ -6,11 +6,13 @@ import (
 	"strconv"
 
 	"github.com/spf13/cobra"
-	"knative.dev/func/pkg/mcp"
-
 	fn "knative.dev/func/pkg/functions"
+	"knative.dev/func/pkg/mcp"
 )
 
+// testMCPOptions is set by tests to inject MCP server options (e.g. in-memory transport).
+var testMCPOptions []mcp.Option
+
 func NewMCPCmd(newClient ClientFactory) *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "mcp",
@@ -89,14 +91,10 @@ DESCRIPTION
 			return runMCPStart(cmd, args, newClient)
 		},
 	}
-	// no flags at this time; future enhancements may be to allow configuring
-	// HTTP Stream vs stdio, single vs multiuser modes, etc.  For now
-	// we just use a simple gathering of options in runMCPStart.
 	return cmd
 }
 
 func runMCPStart(cmd *cobra.Command, args []string, newClient ClientFactory) error {
-	// Configure write mode
 	writeEnabled := false
 	if val := os.Getenv("FUNC_ENABLE_MCP_WRITE"); val != "" {
 		parsed, err := strconv.ParseBool(val)
@@ -106,18 +104,23 @@ func runMCPStart(cmd *cobra.Command, args []string, newClient ClientFactory) err
 		writeEnabled = parsed
 	}
 
-	// Configure 'func' or 'kn func'?
-	rootCmd := cmd.Root()
-	cmdPrefix := rootCmd.Use
+	cmdPrefix := cmd.Root().Use
 
-	// Instantiate
-	client, done := newClient(ClientConfig{},
-		fn.WithMCPServer(mcp.New(
-			mcp.WithPrefix(cmdPrefix),
-			mcp.WithReadonly(!writeEnabled))))
-	defer done()
+	factory := func(cfg mcp.ClientConfig, options ...fn.Option) (*fn.Client, func()) {
+		return newClient(ClientConfig{
+			Verbose:            cfg.Verbose,
+			InsecureSkipVerify: cfg.InsecureSkipVerify,
+		}, options...)
+	}
+
+	opts := []mcp.Option{
+		mcp.WithPrefix(cmdPrefix),
+		mcp.WithReadonly(!writeEnabled),
+		mcp.WithClientFactory(factory),
+	}
+	opts = append(opts, testMCPOptions...)
 
-	// Start
-	return client.StartMCPServer(cmd.Context())
+	server := mcp.New(opts...)
 
+	return server.Start(cmd.Context())
 }

cmd/mcp_test.go

@@ -1,60 +1,77 @@
 package cmd
 
 import (
+	"context"
 	"testing"
 
-	fn "knative.dev/func/pkg/functions"
-	"knative.dev/func/pkg/mock"
+	"github.com/modelcontextprotocol/go-sdk/mcp"
+	mcpkg "knative.dev/func/pkg/mcp"
 	. "knative.dev/func/pkg/testing"
 )
 
-// TestMCP_Start ensures the "mcp start" command starts the MCP server.
 func TestMCP_Start(t *testing.T) {
 	_ = FromTempDirectory(t)
 
-	server := mock.NewMCPServer()
+	serverTpt, clientTpt := mcp.NewInMemoryTransports()
+	testMCPOptions = []mcpkg.Option{mcpkg.WithTransport(serverTpt)}
+	t.Cleanup(func() { testMCPOptions = nil })
 
-	cmd := NewMCPCmd(NewTestClient(fn.WithMCPServer(server)))
+	cmd := NewMCPCmd(NewTestClient())
 	cmd.SetArgs([]string{"start"})
-	if err := cmd.Execute(); err != nil {
-		t.Fatal(err)
-	}
 
-	if !server.StartInvoked {
-		// Indicates a failure of the command to correctly map the request
-		// for "mcp start" to an actual invocation of the client's
-		// StartMCPServer method, or something more fundamental like failure
-		// to register the subcommand, etc.
-		t.Fatal("MCP server's start method not invoked")
+	ctx, cancel := context.WithCancel(t.Context())
+	defer cancel()
+
+	go func() { _ = cmd.ExecuteContext(ctx) }()
+
+	client := mcp.NewClient(&mcp.Implementation{Name: "test", Version: "1.0"}, nil)
+	session, err := client.Connect(t.Context(), clientTpt, nil)
+	if err != nil {
+		cancel()
+		t.Fatal(err)
 	}
+	session.Close()
+	cancel()
 }
 
-// TestMCP_StartWriteable ensures that the FUNC_ENABLE_MCP_WRITE environment
-// variable is correctly parsed and the server starts in both default
-// (readonly) and write-enabled modes.
 func TestMCP_StartWriteable(t *testing.T) {
 	_ = FromTempDirectory(t)
 
-	// Ensure it defaults to readonly (no env var set).
-	server := mock.NewMCPServer()
-	cmd := NewMCPCmd(NewTestClient(fn.WithMCPServer(server)))
+	// Readonly mode
+	serverTpt, clientTpt := mcp.NewInMemoryTransports()
+	testMCPOptions = []mcpkg.Option{mcpkg.WithTransport(serverTpt)}
+	t.Cleanup(func() { testMCPOptions = nil })
+
+	cmd := NewMCPCmd(NewTestClient())
 	cmd.SetArgs([]string{"start"})
-	if err := cmd.Execute(); err != nil {
+	ctx, cancel := context.WithCancel(t.Context())
+	go func() { _ = cmd.ExecuteContext(ctx) }()
+
+	client := mcp.NewClient(&mcp.Implementation{Name: "test", Version: "1.0"}, nil)
+	session, err := client.Connect(t.Context(), clientTpt, nil)
+	if err != nil {
+		cancel()
 		t.Fatal(err)
 	}
-	if !server.StartInvoked {
-		t.Fatal("MCP server was not started in default mode")
-	}
+	session.Close()
+	cancel()
 
-	// Ensure it starts successfully with write mode enabled.
+	// Write mode
 	t.Setenv("FUNC_ENABLE_MCP_WRITE", "true")
-	server = mock.NewMCPServer()
-	cmd = NewMCPCmd(NewTestClient(fn.WithMCPServer(server)))
+	serverTpt2, clientTpt2 := mcp.NewInMemoryTransports()
+	testMCPOptions = []mcpkg.Option{mcpkg.WithTransport(serverTpt2)}
+
+	cmd = NewMCPCmd(NewTestClient())
 	cmd.SetArgs([]string{"start"})
-	if err := cmd.Execute(); err != nil {
+	ctx2, cancel2 := context.WithCancel(t.Context())
+	go func() { _ = cmd.ExecuteContext(ctx2) }()
+
+	client2 := mcp.NewClient(&mcp.Implementation{Name: "test", Version: "1.0"}, nil)
+	session2, err := client2.Connect(t.Context(), clientTpt2, nil)
+	if err != nil {
+		cancel2()
 		t.Fatal(err)
 	}
-	if !server.StartInvoked {
-		t.Fatal("MCP server was not started with write mode enabled")
-	}
+	session2.Close()
+	cancel2()
 }

pkg/mcp/client_options.go

@@ -0,0 +1,203 @@
+package mcp
+
+import (
+	"fmt"
+	"strings"
+
+	"github.com/google/go-containerregistry/pkg/name"
+	"knative.dev/func/pkg/builders"
+	"knative.dev/func/pkg/buildpacks"
+	"knative.dev/func/pkg/config"
+	"knative.dev/func/pkg/creds"
+	"knative.dev/func/pkg/docker"
+	fn "knative.dev/func/pkg/functions"
+	fnhttp "knative.dev/func/pkg/http"
+	"knative.dev/func/pkg/k8s"
+	"knative.dev/func/pkg/keda"
+	"knative.dev/func/pkg/knative"
+	"knative.dev/func/pkg/oci"
+	"knative.dev/func/pkg/pipelines/tekton"
+	"knative.dev/func/pkg/s2i"
+)
+
+// builderClientOptions returns client options for the given builder name.
+func builderClientOptions(builder string, cfg ClientConfig, withTimestamp bool, registryAuthfile string) ([]fn.Option, error) {
+	if builder == "" {
+		builder = builders.Pack
+	}
+
+	o := []fn.Option{
+		fn.WithVerbose(cfg.Verbose),
+	}
+
+	t := fnhttp.NewRoundTripper(
+		fnhttp.WithInsecureSkipVerify(cfg.InsecureSkipVerify),
+		fnhttp.WithOpenShiftServiceCA(),
+	)
+	credsProvider := newCredentialsProvider(config.Dir(), t, registryAuthfile, cfg.InsecureSkipVerify)
+
+	switch builder {
+	case builders.Host:
+		o = append(o,
+			fn.WithScaffolder(oci.NewScaffolder(cfg.Verbose)),
+			fn.WithBuilder(oci.NewBuilder(builders.Host, cfg.Verbose)),
+			fn.WithPusher(oci.NewPusher(cfg.InsecureSkipVerify, false, cfg.Verbose,
+				oci.WithTransport(fnhttp.NewRoundTripper(fnhttp.WithInsecureSkipVerify(cfg.InsecureSkipVerify), fnhttp.WithOpenShiftServiceCA())),
+				oci.WithCredentialsProvider(credsProvider),
+				oci.WithVerbose(cfg.Verbose))),
+		)
+	case builders.Pack:
+		o = append(o,
+			fn.WithScaffolder(buildpacks.NewScaffolder(cfg.Verbose)),
+			fn.WithBuilder(buildpacks.NewBuilder(
+				buildpacks.WithName(builders.Pack),
+				buildpacks.WithTimestamp(withTimestamp),
+				buildpacks.WithVerbose(cfg.Verbose))),
+			fn.WithPusher(docker.NewPusher(
+				docker.WithCredentialsProvider(credsProvider),
+				docker.WithTransport(t),
+				docker.WithVerbose(cfg.Verbose),
+				docker.WithInsecure(cfg.InsecureSkipVerify))),
+		)
+	case builders.S2I:
+		o = append(o,
+			fn.WithScaffolder(s2i.NewScaffolder(cfg.Verbose)),
+			fn.WithBuilder(s2i.NewBuilder(
+				s2i.WithName(builders.S2I),
+				s2i.WithVerbose(cfg.Verbose))),
+			fn.WithPusher(docker.NewPusher(
+				docker.WithCredentialsProvider(credsProvider),
+				docker.WithTransport(t),
+				docker.WithVerbose(cfg.Verbose),
+				docker.WithInsecure(cfg.InsecureSkipVerify))),
+		)
+	default:
+		return o, builders.ErrUnknownBuilder{Name: builder, Known: builders.All()}
+	}
+	return o, nil
+}
+
+// deployClientOptions returns client options for a deploy operation.
+func deployClientOptions(builder, deployer string, cfg ClientConfig, withTimestamp bool) ([]fn.Option, error) {
+	o, err := builderClientOptions(builder, cfg, withTimestamp, "")
+	if err != nil {
+		return nil, err
+	}
+
+	t := fnhttp.NewRoundTripper(
+		fnhttp.WithInsecureSkipVerify(cfg.InsecureSkipVerify),
+		fnhttp.WithOpenShiftServiceCA(),
+	)
+	credsProvider := newCredentialsProvider(config.Dir(), t, "", cfg.InsecureSkipVerify)
+
+	o = append(o, fn.WithPipelinesProvider(tekton.NewPipelinesProvider(
+		tekton.WithCredentialsProvider(credsProvider),
+		tekton.WithVerbose(cfg.Verbose),
+		tekton.WithPipelineDecorator(deployDecorator{}),
+		tekton.WithTransport(t),
+	)))
+
+	if deployer == "" {
+		deployer = knative.KnativeDeployerName
+	}
+	switch deployer {
+	case knative.KnativeDeployerName:
+		o = append(o, fn.WithDeployer(knative.NewDeployer(
+			knative.WithDeployerVerbose(cfg.Verbose),
+			knative.WithDeployerDecorator(deployDecorator{}),
+		)))
+	case k8s.KubernetesDeployerName:
+		o = append(o, fn.WithDeployer(k8s.NewDeployer(
+			k8s.WithDeployerVerbose(cfg.Verbose),
+			k8s.WithDeployerDecorator(deployDecorator{}),
+		)))
+	case keda.KedaDeployerName:
+		o = append(o, fn.WithDeployer(keda.NewDeployer(
+			keda.WithDeployerVerbose(cfg.Verbose),
+			keda.WithDeployerDecorator(deployDecorator{}),
+		)))
+	default:
+		return nil, fmt.Errorf("unsupported deployer: %s (supported: %s, %s, %s)",
+			deployer, knative.KnativeDeployerName, k8s.KubernetesDeployerName, keda.KedaDeployerName)
+	}
+	return o, nil
+}
+
+func platformBuildOptions(platform string) ([]fn.BuildOption, error) {
+	if platform == "" {
+		return nil, nil
+	}
+	parts := strings.Split(platform, "/")
+	if len(parts) != 2 {
+		return nil, fmt.Errorf("platform must be in the form OS/Architecture (e.g. linux/amd64)")
+	}
+	return []fn.BuildOption{fn.BuildWithPlatforms([]fn.Platform{{OS: parts[0], Architecture: parts[1]}})}, nil
+}
+
+func shouldBuild(buildFlag string, f fn.Function) (bool, error) {
+	if buildFlag == "" || buildFlag == "auto" {
+		if f.Built() {
+			return false, nil
+		}
+		return true, nil
+	}
+	build, err := parseBoolFlag(buildFlag)
+	if err != nil {
+		return false, fmt.Errorf("invalid build flag %q: must be 'auto' or a boolean", buildFlag)
+	}
+	return build, nil
+}
+
+func parseBoolFlag(v string) (bool, error) {
+	switch strings.ToLower(v) {
+	case "true", "1", "t", "yes":
+		return true, nil
+	case "false", "0", "f", "no":
+		return false, nil
+	default:
+		return false, fmt.Errorf("invalid boolean value %q", v)
+	}
+}
+
+func isDigestedImage(v string) (bool, error) {
+	ref, err := name.ParseReference(v)
+	if err != nil {
+		return false, err
+	}
+	_, ok := ref.(name.Digest)
+	return ok, nil
+}
+
+func newCredentialsProvider(configPath string, t fnhttp.RoundTripCloser, authFilePath string, insecure bool) oci.CredentialsProvider {
+	additionalLoaders := append(k8s.GetOpenShiftDockerCredentialLoaders(), k8s.GetGoogleCredentialLoader()...)
+	additionalLoaders = append(additionalLoaders, k8s.GetECRCredentialLoader()...)
+	additionalLoaders = append(additionalLoaders, k8s.GetACRCredentialLoader()...)
+
+	options := []creds.Opt{
+		creds.WithTransport(t),
+		creds.WithInsecure(insecure),
+		creds.WithAdditionalCredentialLoaders(additionalLoaders...),
+	}
+	if authFilePath != "" {
+		options = append(options, creds.WithAuthFilePath(authFilePath))
+	}
+	return creds.NewCredentialsProvider(configPath, options...)
+}
+
+type deployDecorator struct {
+	oshDec k8s.OpenshiftMetadataDecorator
+}
+
+func (d deployDecorator) UpdateAnnotations(function fn.Function, annotations map[string]string) map[string]string {
+	if k8s.IsOpenShift() {
+		return d.oshDec.UpdateAnnotations(function, annotations)
+	}
+	return annotations
+}
+
+func (d deployDecorator) UpdateLabels(function fn.Function, labels map[string]string) map[string]string {
+	if k8s.IsOpenShift() {
+		return d.oshDec.UpdateLabels(function, labels)
+	}
+	return labels
+}