-
Notifications
You must be signed in to change notification settings - Fork 168
Feat(MCP): add MCP server with nacos #757
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
base: develop
Are you sure you want to change the base?
Conversation
…configuration synchronization
# Conflicts: # go.mod # go.sum
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.
need to write docs about how to use this adapter, u can refer to #751
dataId := ph[0] | ||
group := ph[1] |
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.
does access to ph have out of range problem?
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.
placeholders = append(placeholders, [2]string{dataId, group})
Because the parameter passed to this function is an array of two strings, it should not exceed the range
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.
are there any mechanism from upper content to guarantee ph must have two elements? If not here should check whether ph have two elements.
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.
// parseTemplatePlaceholders parses template placeholders and returns replaced content and required (dataId,group) list
func parseTemplatePlaceholders(content string) (string, [][2]string) {
all := templateRegex.FindAllStringSubmatch(content, -1)
if len(all) == 0 {
return content, nil
}
placeholders := make([][2]string, 0, len(all))
newContent := content
seen := make(map[string]bool)
for _, m := range all {
if len(m) < 2 {
continue
}
p := m[1] // dataId/group
parts := strings.Split(p, "/")
if len(parts) != 2 {
continue
}
dataId := strings.TrimSpace(parts[0])
group := strings.TrimSpace(parts[1])
key := group + "::" + dataId
if seen[key] {
// replace duplicates as well
newContent = strings.ReplaceAll(newContent, "${nacos."+p+"}", ".config.credentials."+group+"_"+dataId)
continue
}
seen[key] = true
placeholders = append(placeholders, [2]string{dataId, group})
newContent = strings.ReplaceAll(newContent, "${nacos."+p+"}", ".config.credentials."+group+"_"+dataId)
}
return newContent, placeholders
}
In this function, len(parts) != 2
has been excluded wrong content.
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.
cool
👌, I will do it later. |
…and DynamicConsumer
…g credential configurations
sample is here apache/dubbo-go-pixiu-samples#97 |
This pull request introduces a new provider-agnostic registry controller framework for MCP servers, with an initial implementation for Nacos. It refactors the registry integration to support pluggable providers, adds utility functions for parsing registry addresses, and updates dependencies in MCP Registry Controller Framework
Nacos Registry Provider Implementation
Utility Functions
Dependency Updates |
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.
Pull Request Overview
This PR introduces a new provider-agnostic MCP (Model Context Protocol) registry controller framework with Nacos-specific implementation for dynamic configuration discovery and management. It also restructures the MCP server configuration data model from a local package to a shared model package to enable reuse across components.
- Registry controller framework with provider-agnostic interfaces and factory pattern
- Nacos registry integration for MCP server discovery and dynamic configuration updates
- Configuration data model moved from
pkg/filter/mcp/mcpserver/config.go
topkg/model/mcpserver.go
Reviewed Changes
Copilot reviewed 21 out of 22 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
pkg/model/mcpserver.go | Moved and consolidated MCP server configuration data structures from filter package |
pkg/filter/mcp/mcpserver/*.go | Updated to use shared model package and added dynamic configuration capabilities |
pkg/adapter/mcpserver/*.go | New adapter for MCP registry integration with provider-agnostic controller pattern |
pkg/adapter/mcpserver/registry/*.go | Registry controller framework with Nacos implementation |
pkg/adapter/mcpserver/common/url.go | URL parsing utility for extracting host and port from various URL formats |
go.mod | Updated dependencies for Nacos v2 and Alibaba Cloud SDK |
Comments suppressed due to low confidence (2)
pkg/filter/mcp/mcpserver/handlers.go:1
- The code on line 456 executes unconditionally after the switch statement, causing all arguments to be added to bodyParams regardless of their intended location. This overwrites the proper handling done in the switch statement.
/*
pkg/filter/mcp/mcpserver/handlers.go:443
- Potential nil pointer dereference if argConfig is nil when the argument name is not found in toolConfig.Args. The code should check if argConfig is nil before accessing argConfig.In.
switch argConfig.In {
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
… and registry components
…laceGoTemplateArgsInPath functions
…n configuration updates
…n configuration updates
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #757 +/- ##
============================================
- Coverage 54.67% 16.21% -38.46%
============================================
Files 671 224 -447
Lines 78658 16723 -61935
============================================
- Hits 43006 2712 -40294
+ Misses 31981 13750 -18231
+ Partials 3671 261 -3410
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
# Conflicts: # go.mod # go.sum # pkg/filter/mcp/mcpserver/handlers.go
pls fix the CI |
# Conflicts: # go.mod # go.sum
|
What this PR does:
This pull request introduces a new provider-agnostic MCP registry controller framework and a Nacos-specific implementation, along with several utility and conversion functions for handling registry configuration. It also updates the
go.mod
dependencies, adding new packages for Nacos v2 and Alibaba Cloud, while cleaning up unused or outdated dependencies. The changes are grouped into three main themes: registry controller framework, Nacos registry integration, and dependency updates.Registry controller framework:
Controller
), a builder function type (BuildFunc
), and a factory for provider registration and controller instantiation inpkg/adapter/mcpserver/registry/controller.go
andpkg/adapter/mcpserver/registry/factory.go
. [1] [2]Nacos registry integration:
pkg/adapter/mcpserver/registry/nacos/adapter.go
andpkg/adapter/mcpserver/registry/nacos/controller.go
. [1] [2]pkg/adapter/mcpserver/registry/nacos/converter.go
.pkg/adapter/mcpserver/registry/nacos/types.go
.pkg/adapter/mcpserver/common/url.go
.