[WIP] MCP Server and Route API Specs #250
Conversation
github-actions
bot
added
bug
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @YouNeedCryDear, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request introduces native support for the Model Context Protocol (MCP) in OME, enabling Large Language Models (LLMs) to integrate with external tools and services. It establishes a standardized interface for LLM-tool interaction by introducing new Kubernetes Custom Resource Definitions (CRDs) for MCPServer and ClusterMCPServer. This foundational work allows for secure, scalable, and independently managed deployment of tool capabilities within the OME ecosystem, separate from inference services.
Highlights
- New CRDs for MCP Servers: Introduces
MCPServer(namespaced) andClusterMCPServer(cluster-scoped) Custom Resource Definitions to manage MCP server deployments within OME. - Standardized LLM Tool Integration: Enables LLMs to interact with external systems (databases, APIs, file systems) through the open Model Context Protocol (MCP), standardizing the interface between LLMs and external services.
- Flexible Transport Protocols: Supports multiple communication protocols for MCP servers, including
stdio,streamable-http, andServer-Sent Events (SSE). - Comprehensive Security Model: Implements a robust security framework with OIDC-based authentication (including Kubernetes service account integration), Cedar policy-based authorization, and fine-grained control over network and file system permissions.
- Standalone Service Architecture: Designs MCP servers as independent resources from InferenceServices, promoting modularity, reusability, and independent scaling of tool capabilities.
- Detailed OEP Documentation: Adds a comprehensive OEP (OME Enhancement Proposal) document (OEP-0005) outlining the motivation, goals, design details, API specifications, architecture, security model, deployment patterns, and test plan for MCP support.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces new APIs (MCPServer and ClusterMCPServer) and a detailed design document for adding Model Context Protocol (MCP) support to OME. The changes are well-structured, and the design document is comprehensive. My review focuses on improving API consistency, documentation, and ensuring the validity of the new Custom Resource Definitions (CRDs). I've identified a critical issue with the CRD definitions that could prevent them from being applied to a Kubernetes cluster. I've also found some high-severity issues related to API consistency and potential runtime errors due to missing defaults. Additionally, there are some medium-severity suggestions to improve the documentation and readability of the API and design document.
oeps/0005-mcp-support/README.md
Outdated
| ```go | ||
| // MCPTransportType defines the transport method for MCP communication | ||
| // +kubebuilder:validation:Enum=stdio;streamable-http;sse | ||
| type MCPTransportType string |
There was a problem hiding this comment.
Nit
we can use kubeBuilder build in validation instead of having another struct
oeps/0005-mcp-support/README.md
Outdated
|
|
||
| ### Security Model | ||
|
|
||
| The MCP support implementation follows a defense-in-depth security model with multiple layers of protection: |
There was a problem hiding this comment.
for example
If original MCP server already supports all those auth
does this mean platform engineers still have to configure those in OME? Just for the record sake?
Or do we wanna have something natively supporting those auth given a bare bones MCP?
There was a problem hiding this comment.
My thought is we need to provide native Auth support even for a bare bones MCP. AFAIK, most of the MCP servers still do not include native auth by default.
YouNeedCryDear
force-pushed
the
feature/mcp-specs
branch
from
3981671 to
bf1bd21
Compare
YouNeedCryDear
changed the title
YouNeedCryDear
force-pushed
the
feature/mcp-specs
branch
from
c25a1bd to
0963ebb
Compare
|
|
||
| const ( | ||
| AuthMethodNone AuthMethod = "None" | ||
| AuthMethodBearer AuthMethod = "Bearer" |
There was a problem hiding this comment.
bear token and api key are the same thing?
| AuthMethodNone AuthMethod = "None" | ||
| AuthMethodBearer AuthMethod = "Bearer" | ||
| AuthMethodApiKey AuthMethod = "ApiKey" | ||
| AuthMethodBasic AuthMethod = "Basic" |
There was a problem hiding this comment.
basic means user credential right?
| AuthMethodApiKey AuthMethod = "ApiKey" | ||
| AuthMethodBasic AuthMethod = "Basic" | ||
| AuthMethodJWT AuthMethod = "JWT" | ||
| AuthMethodClientCertificate AuthMethod = "ClientCertificate" |
| // Timeout defines the authentication request timeout. | ||
| // +kubebuilder:default="30s" | ||
| // +optional | ||
| Timeout *metav1.Duration `json:"timeout,omitempty"` |
There was a problem hiding this comment.
why do we have timeout in auth section?
| } | ||
|
|
||
| // ClientCertCredentials defines client certificate authentication. | ||
| type ClientCertCredentials struct { |
There was a problem hiding this comment.
how does it integrate with things such as cert manager and other cloud cert management
| ) | ||
|
|
||
| // GatewayPolicyConfig defines unified security, authentication, authorization, and traffic policies. | ||
| type MCPGatewayPolicyConfig struct { |
There was a problem hiding this comment.
all of those features for mcp gateway makes sense
now
i think what we really need is
- we provide a global mcp gateway software
- customer can customize them, probably through configmap to start with, or we write them in CRD, but unless this is very common to update them
- customer should write resources such as mcp-ingress, inside of this ingress resource, it should define selector, target ports, routing policy etc (refer to kong or istio ingress gateway CRD)
YouNeedCryDear
changed the title
What type of PR is this?
/kind feature
What this PR does / why we need it:
Add MCP Support to OME
This OEP introduces native support for the Model Context Protocol (MCP) in OME through MCPServer and MCPGateway CRDs that enable Large Language Models to integrate with external tools and services. MCP is an open protocol developed by Anthropic that standardizes the interface between LLMs and external systems, allowing models to perform actions like database queries, file operations, API calls, and infrastructure management in a secure and controlled manner.
Details in OEP 0005-mcp-support
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?