-
Notifications
You must be signed in to change notification settings - Fork 167
Add Virtual MCP Server architecture documentation #3272
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Add comprehensive architecture documentation for the Virtual MCP Server (vMCP) component, which was previously described as an "upcoming feature" but is now fully implemented. Changes: - Add new docs/arch/10-virtual-mcp-architecture.md covering vMCP architecture including aggregation pipeline, two-boundary authentication, conflict resolution, and composite tools - Update README.md to include vMCP in documentation index and architecture map, and fix outdated "upcoming" references - Update 07-groups.md to reflect that vMCP is implemented (was listed as "proposed, under consideration") - Update 00-overview.md to add vmcp binary as a key component - Update 09-operator-architecture.md to add VirtualMCPCompositeToolDefinition CRD documentation - Update 02-core-concepts.md to link to the new vMCP architecture doc Signed-off-by: Juan Antonio Osorio <ozz@stacklok.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3272 +/- ##
=======================================
Coverage 63.65% 63.65%
=======================================
Files 357 357
Lines 35156 35156
=======================================
Hits 22379 22379
Misses 10974 10974
Partials 1803 1803 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@claude please review this and cross reference with the code |
|
Claude finished @JAORMX's task in 2m 26s —— View job Documentation Review Complete ✅I've thoroughly reviewed the Virtual MCP Server architecture documentation and cross-referenced it with the actual implementation. Here's my comprehensive feedback: Overall AssessmentExcellent work! The documentation is comprehensive, well-structured, and accurately reflects the implementation. The vMCP architecture is clearly explained with appropriate diagrams and implementation references. ✅ Strengths
✅ Internal Links VerificationAll internal documentation links are valid:
📝 Minor Observations (Not Issues)
🎯 Technical Accuracy VerificationCross-referenced key claims with implementation:
🔍 Key Documentation Features
✨ Recommendations for Future Enhancements (Optional)These are suggestions for potential future additions, not issues with the current PR:
Final VerdictAPPROVED ✅ The documentation accurately represents the Virtual MCP Server implementation, all file paths are correct, internal links work, and the content successfully transitions vMCP from "proposed" to "fully implemented" status. This is production-ready documentation. The PR successfully achieves its stated goals:
Todo List
|
| Manages MCP servers in Kubernetes clusters using custom resources. | ||
|
|
||
| The operator watches for `MCPServer`, `MCPRegistry`, `MCPToolConfig`, and `MCPExternalAuthConfig` CRDs, reconciling them into Kubernetes resources (Deployments, StatefulSets, Services). | ||
| The operator watches for `MCPServer`, `MCPRegistry`, `MCPToolConfig`, `MCPExternalAuthConfig`, `MCPGroup`, and `VirtualMCPServer` CRDs, reconciling them into Kubernetes resources (Deployments, StatefulSets, Services). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is true as of now, but work is being done to move the watching of the resources to the vmcp binary and make the operator reconcile just vmcp resources (this might be too much of a nitpick and maybe it's OK to leave as-is in the overview and dive into the details in the architecture)
| - **Conflict resolution** - Handle duplicate tool names automatically | ||
| - **Composite workflows** - Create new tools that orchestrate multiple backends | ||
| - **Centralized security** - Single authentication and authorization point | ||
| - **Token management** - Exchange and cache tokens for backend access |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's explicitly say access token management or authnz management, token is a loaded word in the LLM space
| 3. For each backend, URL, transport type, and auth config are extracted | ||
| 4. vMCP queries each backend for available tools, resources, and prompts | ||
|
|
||
| **Implementation**: `pkg/vmcp/aggregator/` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure we should mention implementation, this is going to drift over time, but if we do let's also mention pkg/vmcp/discovery.
|
|
||
| ## Health Monitoring | ||
|
|
||
| vMCP monitors backend health with configurable intervals. Health status (healthy, degraded, unhealthy, unknown) affects routing decisions and is reported in VirtualMCPServer status. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: we also have unauthenticated
Address review findings from PR #3272 to align documentation with actual implementation. docs/arch/10-virtual-mcp-architecture.md: - Fix prefix format example to use underscore separator (github_create_issue) instead of dot, matching the default format in prefix_resolver.go - Add 'unauthenticated' to health status list, which is tracked by the health monitor but was missing from documentation - Add explicit references to pkg/vmcp/discovery/ and pkg/vmcp/router/ packages shown in architecture diagram but not referenced docs/arch/02-core-concepts.md: - Document CLI deployment mode alongside Kubernetes to resolve inconsistency with 10-virtual-mcp-architecture.md which mentioned both deployment options docs/arch/09-operator-architecture.md: - Replace non-existent "Capabilities summary" status field with actual "Backend count" field from VirtualMCPServerStatus struct - Fix reference direction: VirtualMCPServer references MCPGroup (not the reverse), and correct field path to spec.config.groupRef
Address review findings from PR #3272 to align documentation with actual implementation. docs/arch/10-virtual-mcp-architecture.md: - Fix prefix format example to use underscore separator (github_create_issue) instead of dot, matching the default format in prefix_resolver.go - Add 'unauthenticated' to health status list, which is tracked by the health monitor but was missing from documentation - Add explicit references to pkg/vmcp/discovery/ and pkg/vmcp/router/ packages shown in architecture diagram but not referenced docs/arch/02-core-concepts.md: - Document CLI deployment mode alongside Kubernetes to resolve inconsistency with 10-virtual-mcp-architecture.md which mentioned both deployment options docs/arch/09-operator-architecture.md: - Replace non-existent "Capabilities summary" status field with actual "Backend count" field from VirtualMCPServerStatus struct - Fix reference direction: VirtualMCPServer references MCPGroup (not the reverse), and correct field path to spec.config.groupRef
Summary
docs/arch/10-virtual-mcp-architecture.mdwith comprehensive vMCP architecture documentationDetails
The Virtual MCP Server (vMCP) was previously described as "proposed" or "upcoming" in the architecture documentation, but it is now fully implemented. This PR brings the documentation up to date.
New document (
10-virtual-mcp-architecture.md) covers:Updated documents:
README.md- Added vMCP to index and architecture map07-groups.md- Changed "Future Features" to "Virtual MCP Integration"00-overview.md- Added vmcp binary as key component Implement secret injection #509-operator-architecture.md- Added VirtualMCPCompositeToolDefinition CRD02-core-concepts.md- Added cross-reference to new docTest plan
🤖 Generated with Claude Code