You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by "YuZhangLarry (via GitHub)" <gi...@apache.org> on 2026/04/22 09:04:12 UTC
[PR] feat: add Model Context Protocol (MCP) server implementation [dubbo-admin]
YuZhangLarry opened a new pull request, #1463:
URL: https://github.com/apache/dubbo-admin/pull/1463
Add Model Context Protocol (MCP) server implementation for Dubbo Admin AI integration.
- Modular MCP architecture (core, registry, tools, transport, types)
- Tool registration for Dubbo resources (cluster, search, service discovery, metrics, application/instance details)
- Stdio transport for MCP communication
- Integration tests and live test utilities
Review areas:
- Core Component
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] feat: add Model Context Protocol (MCP) server implementation [dubbo-admin]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #1463:
URL: https://github.com/apache/dubbo-admin/pull/1463#issuecomment-4294934807
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-admin&pullRequest=1463) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-admin&pullRequest=1463&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-admin&pullRequest=1463&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-admin&pullRequest=1463&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-admin&pullRequest=1463&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-admin&pullRequest=1463&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-admin&pullRequest=1463)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] feat: add Model Context Protocol (MCP) server implementation [dubbo-admin]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #1463:
URL: https://github.com/apache/dubbo-admin/pull/1463#issuecomment-4312402188
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-admin&pullRequest=1463) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-admin&pullRequest=1463&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-admin&pullRequest=1463&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-admin&pullRequest=1463&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-admin&pullRequest=1463&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-admin&pullRequest=1463&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-admin&pullRequest=1463)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] feat: add Model Context Protocol (MCP) server implementation [dubbo-admin]
Posted by "YuZhangLarry (via GitHub)" <gi...@apache.org>.
YuZhangLarry closed pull request #1463: feat: add Model Context Protocol (MCP) server implementation
URL: https://github.com/apache/dubbo-admin/pull/1463
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] feat: add Model Context Protocol (MCP) server implementation [dubbo-admin]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #1463:
URL: https://github.com/apache/dubbo-admin/pull/1463#issuecomment-4400508284
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-admin&pullRequest=1463) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-admin&pullRequest=1463&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-admin&pullRequest=1463&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-admin&pullRequest=1463&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-admin&pullRequest=1463&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-admin&pullRequest=1463&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-admin&pullRequest=1463)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] feat: add Model Context Protocol (MCP) server implementation [dubbo-admin]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #1463:
URL: https://github.com/apache/dubbo-admin/pull/1463#discussion_r3143422736
##########
pkg/console/model/application.go:
##########
@@ -88,68 +88,49 @@ func NewApplicationDetail() *ApplicationDetail {
Workloads: NewSet(),
}
}
-
-func (a *ApplicationDetail) MergeMetaData(metadata *mesh.MetaDataResource) {
- a.mergeServiceInfo(metadata)
-}
-
-func (a *ApplicationDetail) mergeServiceInfo(metadata *mesh.MetaDataResource) {
- for _, serviceInfo := range metadata.Spec.Services {
- a.DubboVersions.Add(fmt.Sprintf("dubbo %s", serviceInfo.Params[constants.ReleaseKey]))
- a.RPCProtocols.Add(serviceInfo.Protocol)
- a.SerialProtocols.Add(serviceInfo.Params[constants.SerializationKey])
-
- }
-}
-
-func (a *ApplicationDetail) MergeDataplane(dataplane *mesh.DataplaneResource) {
- if work, ok := dataplane.Spec.Extensions[constants.WorkLoadKey]; ok &&
- regexp.MustCompile(`^.*-\d+$`).MatchString(work) {
+func (a *ApplicationDetail) MergeInstance(instanceRes *meshresource.InstanceResource, cfg app.AdminConfig) {
+ instance := instanceRes.Spec
+ if instance.WorkloadType == constants.StatefulSet {
a.AppTypes.Add(constants.Stateful)
} else {
a.AppTypes.Add(constants.Stateless)
}
-
- inbounds := dataplane.Spec.Networking.Inbound
- for _, inbound := range inbounds {
- a.mergeInbound(inbound)
+ a.DubboPorts.Add(strconv.FormatInt(instance.RpcPort, 10))
+ a.DubboVersions.Add(instance.ReleaseVersion)
+ a.Images.Add(instance.Image)
+ if d := cfg.FindDiscovery(instanceRes.Mesh); d != nil {
+ a.RegisterClusters.Add(instanceRes.Mesh)
Review Comment:
This condition checks that a discovery exists, but then adds `instanceRes.Mesh` to `RegisterClusters` instead of using discovery attributes (e.g., `d.Name`) or consistently using the discovery ID. As written, `d` is only used as a nil-check, and the collected 'clusters' may be IDs rather than user-facing names. Use a consistent value (either `d.Name` or the mesh ID) and remove the redundant lookup if you intend to store IDs.
```suggestion
a.RegisterClusters.Add(d.Name)
```
##########
pkg/console/handler/configurator_rule.go:
##########
@@ -20,142 +20,134 @@ package handler
import (
"fmt"
"net/http"
- "strconv"
"strings"
+ "github.com/duke-git/lancet/v2/strutil"
"github.com/gin-gonic/gin"
- meshproto "github.com/apache/dubbo-admin/api/mesh/v1alpha1"
+ "github.com/apache/dubbo-admin/pkg/common/bizerror"
+ "github.com/apache/dubbo-admin/pkg/common/constants"
consolectx "github.com/apache/dubbo-admin/pkg/console/context"
"github.com/apache/dubbo-admin/pkg/console/model"
"github.com/apache/dubbo-admin/pkg/console/service"
- "github.com/apache/dubbo-admin/pkg/core/consts"
- "github.com/apache/dubbo-admin/pkg/core/store"
+ "github.com/apache/dubbo-admin/pkg/console/util"
+ meshresource "github.com/apache/dubbo-admin/pkg/core/resource/apis/mesh/v1alpha1"
)
func ConfiguratorSearch(ctx consolectx.Context) gin.HandlerFunc {
return func(c *gin.Context) {
- req := model.NewSearchConfiguratorReq()
+ req := model.NewSearchReq()
if err := c.ShouldBindQuery(req); err != nil {
c.JSON(http.StatusBadRequest, model.NewErrorResp(err.Error()))
return
}
- ruleList := &mesh.DynamicConfigResourceList{}
- var respList []model.ConfiguratorSearchResp
- if req.Keywords == "" {
- if err := ctx.ResourceManager().List(ctx.AppContext(), ruleList, store.ListByPage(req.PageSize, strconv.Itoa(req.PageOffset))); err != nil {
- c.JSON(http.StatusBadRequest, model.NewErrorResp(err.Error()))
- return
- }
+ var searchResult *model.SearchPaginationResult
+ var err error
+ if strutil.IsBlank(req.Keywords) {
+ searchResult, err = service.PageListConfiguratorRule(ctx, req)
} else {
- if err := ctx.ResourceManager().List(ctx.AppContext(), ruleList, store.ListByNameContains(req.Keywords), store.ListByPage(req.PageSize, strconv.Itoa(req.PageOffset))); err != nil {
- c.JSON(http.StatusBadRequest, model.NewErrorResp(err.Error()))
- return
- }
- }
- for _, item := range ruleList.Items {
- respList = append(respList, model.ConfiguratorSearchResp{
- RuleName: item.Meta.GetName(),
- Scope: item.Spec.GetScope(),
- CreateTime: item.Meta.GetCreationTime().String(),
- Enabled: item.Spec.GetEnabled(),
- })
- }
- result := model.NewSearchPaginationResult()
- result.List = respList
- result.PageInfo = &ruleList.Pagination
- c.JSON(http.StatusOK, model.NewSuccessResp(result))
+ searchResult, err = service.SearchConfiguratorRuleByKeywords(ctx, req)
+ }
+ if err != nil {
+ util.HandleServiceError(c, err)
+ return
+ }
+ c.JSON(http.StatusOK, model.NewSuccessResp(searchResult))
}
}
func GetConfiguratorWithRuleName(ctx consolectx.Context) gin.HandlerFunc {
return func(c *gin.Context) {
- var name string
ruleName := c.Param("ruleName")
- if strings.HasSuffix(ruleName, consts.ConfiguratorRuleSuffix) {
- name = ruleName[:len(ruleName)-len(consts.ConfiguratorRuleSuffix)]
- } else {
- c.JSON(http.StatusBadRequest, model.NewErrorResp(fmt.Sprintf("ruleName must end with %s", consts.ConfiguratorRuleSuffix)))
+ mesh := c.Query("mesh")
+ if strutil.IsBlank(ruleName) {
+ c.JSON(http.StatusBadRequest, model.NewErrorResp("ruleName cannot be empty"))
+ return
+ }
+ if strutil.IsBlank(mesh) {
+ c.JSON(http.StatusBadRequest, model.NewErrorResp("mesh cannot be empty"))
return
}
- res, err := service.GetConfigurator(ctx, name)
+ res, err := service.GetConfigurator(ctx, ruleName, mesh)
if err != nil {
- c.JSON(http.StatusBadRequest, model.NewErrorResp(err.Error()))
+ util.HandleServiceError(c, err)
return
}
+ if res == nil {
+ c.JSON(http.StatusOK, model.NewBizErrorResp(
+ bizerror.New(bizerror.NotFoundError, fmt.Sprintf("%s not found", ruleName))))
Review Comment:
After returning a NotFound response when `res == nil`, the handler continues and dereferences `res.Spec`, which will panic. Add a `return` after the NotFound JSON response to stop execution.
```suggestion
bizerror.New(bizerror.NotFoundError, fmt.Sprintf("%s not found", ruleName))))
return
```
##########
pkg/console/handler/tag_rule.go:
##########
@@ -20,95 +20,91 @@ package handler
import (
"fmt"
"net/http"
- "strconv"
"strings"
+ "github.com/duke-git/lancet/v2/strutil"
"github.com/gin-gonic/gin"
- meshproto "github.com/apache/dubbo-admin/api/mesh/v1alpha1"
+ "github.com/apache/dubbo-admin/pkg/common/bizerror"
+ "github.com/apache/dubbo-admin/pkg/common/constants"
consolectx "github.com/apache/dubbo-admin/pkg/console/context"
"github.com/apache/dubbo-admin/pkg/console/model"
"github.com/apache/dubbo-admin/pkg/console/service"
- "github.com/apache/dubbo-admin/pkg/core/consts"
- "github.com/apache/dubbo-admin/pkg/core/store"
+ "github.com/apache/dubbo-admin/pkg/console/util"
+ meshresource "github.com/apache/dubbo-admin/pkg/core/resource/apis/mesh/v1alpha1"
)
func TagRuleSearch(ctx consolectx.Context) gin.HandlerFunc {
return func(c *gin.Context) {
+
req := model.NewSearchReq()
if err := c.ShouldBindQuery(req); err != nil {
c.JSON(http.StatusBadRequest, model.NewErrorResp(err.Error()))
return
}
- resList := &mesh.TagRouteResourceList{}
- if req.Keywords == "" {
- if err := ctx.ResourceManager().List(ctx.AppContext(), resList, store.ListByPage(req.PageSize, strconv.Itoa(req.PageOffset))); err != nil {
- c.JSON(http.StatusBadRequest, model.NewErrorResp(err.Error()))
- return
- }
+ var searchResult *model.SearchPaginationResult
+ var err error
+ if strutil.IsBlank(req.Keywords) {
+ searchResult, err = service.PageListTagRule(ctx, req)
} else {
- if err := ctx.ResourceManager().List(ctx.AppContext(), resList, store.ListByNameContains(req.Keywords), store.ListByPage(req.PageSize, strconv.Itoa(req.PageOffset))); err != nil {
- c.JSON(http.StatusBadRequest, model.NewErrorResp(err.Error()))
- return
- }
- }
- var respList []model.TagRuleSearchResp
- for _, item := range resList.Items {
- time := item.Meta.GetCreationTime().String()
- name := item.Meta.GetName()
- respList = append(respList, model.TagRuleSearchResp{
- CreateTime: &time,
- Enabled: &item.Spec.Enabled,
- RuleName: &name,
- })
- }
- result := model.NewSearchPaginationResult()
- result.List = respList
- result.PageInfo = &resList.Pagination
- c.JSON(http.StatusOK, model.NewSuccessResp(result))
+ searchResult, err = service.SearchTagRuleByKeywords(ctx, req)
+ }
+ if err != nil {
+ util.HandleServiceError(c, err)
+ return
+ }
+ c.JSON(http.StatusOK, model.NewSuccessResp(searchResult))
}
}
func GetTagRuleWithRuleName(ctx consolectx.Context) gin.HandlerFunc {
return func(c *gin.Context) {
- var name string
ruleName := c.Param("ruleName")
- if strings.HasSuffix(ruleName, consts.TagRuleSuffix) {
- name = ruleName[:len(ruleName)-len(consts.TagRuleSuffix)]
- } else {
- c.JSON(http.StatusBadRequest, model.NewErrorResp(fmt.Sprintf("ruleName must end with %s", consts.TagRuleSuffix)))
+ mesh := c.Query("mesh")
+ if !strings.HasSuffix(ruleName, constants.TagRuleDotSuffix) {
+ err := bizerror.New(bizerror.InvalidArgument, fmt.Sprintf("ruleName must end with %s", constants.TagRuleDotSuffix))
+ c.JSON(http.StatusBadRequest, model.NewBizErrorResp(err))
return
}
- if res, err := service.GetTagRule(ctx, name); err != nil {
- c.JSON(http.StatusBadRequest, model.NewErrorResp(err.Error()))
+ res, err := service.GetTagRule(ctx, ruleName, mesh)
+ if err != nil {
+ util.HandleServiceError(c, err)
return
- } else {
- c.JSON(http.StatusOK, model.GenTagRouteResp(res.Spec))
}
+ if res == nil {
+ util.HandleNotFoundError(c, ruleName)
+ return
+ }
+ c.JSON(http.StatusOK, model.GenTagRouteResp(res.Spec))
}
}
func PutTagRuleWithRuleName(ctx consolectx.Context) gin.HandlerFunc {
return func(c *gin.Context) {
- var name string
ruleName := c.Param("ruleName")
- if strings.HasSuffix(ruleName, consts.TagRuleSuffix) {
- name = ruleName[:len(ruleName)-len(consts.TagRuleSuffix)]
- } else {
- c.JSON(http.StatusBadRequest, model.NewErrorResp(fmt.Sprintf("ruleName must end with %s", consts.TagRuleSuffix)))
+ mesh := c.Query("mesh")
+ if !strings.HasSuffix(ruleName, constants.TagRuleDotSuffix) {
+ err := bizerror.New(bizerror.InvalidArgument, fmt.Sprintf("ruleName must end with %s", constants.TagRuleDotSuffix))
+ c.JSON(http.StatusBadRequest, model.NewBizErrorResp(err))
return
}
- res := &mesh.TagRouteResource{
- Meta: nil,
- Spec: &meshproto.TagRoute{},
+ tagRuleRes, err := service.GetTagRule(ctx, ruleName, mesh)
+ if err != nil {
+ util.HandleServiceError(c, err)
+ return
}
- err := c.Bind(res.Spec)
+ if tagRuleRes == nil {
+ util.HandleNotFoundError(c, ruleName)
+ return
+ }
+ res := meshresource.NewTagRouteResourceWithAttributes(ruleName, mesh)
+ err = c.Bind(res.Spec)
if err != nil {
- c.JSON(http.StatusBadRequest, model.NewErrorResp(err.Error()))
+ c.JSON(http.StatusOK, model.NewErrorResp(err.Error()))
return
}
- if err = service.UpdateTagRule(ctx, name, res); err != nil {
- c.JSON(http.StatusBadRequest, model.NewErrorResp(err.Error()))
+ if err = service.UpdateTagRule(ctx, res); err != nil {
+ c.JSON(http.StatusOK, model.NewErrorResp(err.Error()))
Review Comment:
Binding and update failures are returned with `http.StatusOK`, while other handlers in this PR use `http.StatusBadRequest` (binding) and `util.HandleServiceError` (service failures). Returning 200 for invalid input makes client-side error handling harder and is inconsistent within the same API surface. Consider returning `http.StatusBadRequest` for bind errors and using the shared `util.HandleServiceError` for service-layer errors.
##########
pkg/console/model/observability.go:
##########
@@ -17,37 +17,26 @@
package model
-type DashboardReq interface {
- GetKeyVariable() string
-}
-
type AppDashboardReq struct {
- Application string `form:"application"`
-}
-
-func (req *AppDashboardReq) GetKeyVariable() string {
- return req.Application
+ AppName string `form:"appName"`
+ Mesh string `form:"appName"`
Review Comment:
`AppDashboardReq.Mesh` is tagged as `form:\"appName\"`, so `mesh` will never bind from query parameters and will overwrite/duplicate appName binding semantics. This should be tagged as `form:\"mesh\"` (and similarly validated/used downstream) to avoid generating incorrect Grafana URLs and mesh-scoped queries.
```suggestion
Mesh string `form:"mesh"`
```
##########
pkg/config/diagnostics/config.go:
##########
@@ -18,19 +18,23 @@
package diagnostics
import (
+ "github.com/apache/dubbo-admin/pkg/common/bizerror"
"github.com/apache/dubbo-admin/pkg/config"
)
type Config struct {
config.BaseConfig
// Port of Diagnostic Server for checking health and readiness of the Control Plane
- ServerPort uint32 `json:"serverPort" envconfig:"DUBBO_DIAGNOSTICS_SERVER_PORT"`
+ ServerPort uint32 `json:"serverPort"`
}
var _ = &Config{}
-func (d *Config) Validate() error {
+func (c *Config) Validate() error {
+ if c.ServerPort < 1 || c.ServerPort > 65535 {
+ return bizerror.New(bizerror.ConfigError, "server port for diagnotics must between 1 to 65535")
Review Comment:
Typo in error message: 'diagnotics' should be 'diagnostics' (and the phrasing is ungrammatical). Consider updating to a clearer message such as 'server port for diagnostics must be between 1 and 65535'.
```suggestion
return bizerror.New(bizerror.ConfigError, "server port for diagnostics must be between 1 and 65535")
```
##########
pkg/console/handler/search.go:
##########
@@ -25,44 +25,42 @@ import (
consolectx "github.com/apache/dubbo-admin/pkg/console/context"
"github.com/apache/dubbo-admin/pkg/console/model"
"github.com/apache/dubbo-admin/pkg/console/service"
+ coremodel "github.com/apache/dubbo-admin/pkg/core/resource/model"
)
func BannerGlobalSearch(ctx consolectx.Context) gin.HandlerFunc {
return func(c *gin.Context) {
- // 参考 API 定义 request 参数
req := model.NewSearchReq()
if err := c.ShouldBindQuery(req); err != nil {
c.JSON(http.StatusBadRequest, model.NewErrorResp(err.Error()))
return
}
- // 根据 request 分流调用，如服务未实现继续实现
-
var res *model.SearchRes
switch req.SearchType {
case "ip":
- instances, _ := service.BannerSearchIp(ctx, req)
+ instances, _ := service.SearchInstanceByIp(ctx, req)
res = convertInstancesToSearchRes(instances)
case "instanceName":
- instances, _ := service.BannerSearchInstances(ctx, req)
+ instances, _ := service.SearchInstanceByName(ctx, req)
res = convertInstancesToSearchRes(instances)
case "appName":
- applications, _ := service.BannerSearchApplications(ctx, req)
+ applications, _ := service.SearchApplicationsByKeywords(ctx, req)
res = convertApplicationsToSearchRes(applications)
Review Comment:
Errors are ignored (`_ := ...`) and the conversion helpers assume non-nil results; if the service returns an error (and/or nil), this can lead to panics or misleading empty responses. Handle the returned error and validate `instances/applications` are non-nil before converting; route failures through the existing error response mechanism.
##########
pkg/console/handler/configurator_rule.go:
##########
@@ -20,142 +20,134 @@ package handler
import (
"fmt"
"net/http"
- "strconv"
"strings"
+ "github.com/duke-git/lancet/v2/strutil"
"github.com/gin-gonic/gin"
- meshproto "github.com/apache/dubbo-admin/api/mesh/v1alpha1"
+ "github.com/apache/dubbo-admin/pkg/common/bizerror"
+ "github.com/apache/dubbo-admin/pkg/common/constants"
consolectx "github.com/apache/dubbo-admin/pkg/console/context"
"github.com/apache/dubbo-admin/pkg/console/model"
"github.com/apache/dubbo-admin/pkg/console/service"
- "github.com/apache/dubbo-admin/pkg/core/consts"
- "github.com/apache/dubbo-admin/pkg/core/store"
+ "github.com/apache/dubbo-admin/pkg/console/util"
+ meshresource "github.com/apache/dubbo-admin/pkg/core/resource/apis/mesh/v1alpha1"
)
func ConfiguratorSearch(ctx consolectx.Context) gin.HandlerFunc {
return func(c *gin.Context) {
- req := model.NewSearchConfiguratorReq()
+ req := model.NewSearchReq()
if err := c.ShouldBindQuery(req); err != nil {
c.JSON(http.StatusBadRequest, model.NewErrorResp(err.Error()))
return
}
- ruleList := &mesh.DynamicConfigResourceList{}
- var respList []model.ConfiguratorSearchResp
- if req.Keywords == "" {
- if err := ctx.ResourceManager().List(ctx.AppContext(), ruleList, store.ListByPage(req.PageSize, strconv.Itoa(req.PageOffset))); err != nil {
- c.JSON(http.StatusBadRequest, model.NewErrorResp(err.Error()))
- return
- }
+ var searchResult *model.SearchPaginationResult
+ var err error
+ if strutil.IsBlank(req.Keywords) {
+ searchResult, err = service.PageListConfiguratorRule(ctx, req)
} else {
- if err := ctx.ResourceManager().List(ctx.AppContext(), ruleList, store.ListByNameContains(req.Keywords), store.ListByPage(req.PageSize, strconv.Itoa(req.PageOffset))); err != nil {
- c.JSON(http.StatusBadRequest, model.NewErrorResp(err.Error()))
- return
- }
- }
- for _, item := range ruleList.Items {
- respList = append(respList, model.ConfiguratorSearchResp{
- RuleName: item.Meta.GetName(),
- Scope: item.Spec.GetScope(),
- CreateTime: item.Meta.GetCreationTime().String(),
- Enabled: item.Spec.GetEnabled(),
- })
- }
- result := model.NewSearchPaginationResult()
- result.List = respList
- result.PageInfo = &ruleList.Pagination
- c.JSON(http.StatusOK, model.NewSuccessResp(result))
+ searchResult, err = service.SearchConfiguratorRuleByKeywords(ctx, req)
+ }
+ if err != nil {
+ util.HandleServiceError(c, err)
+ return
+ }
+ c.JSON(http.StatusOK, model.NewSuccessResp(searchResult))
}
}
func GetConfiguratorWithRuleName(ctx consolectx.Context) gin.HandlerFunc {
return func(c *gin.Context) {
- var name string
ruleName := c.Param("ruleName")
- if strings.HasSuffix(ruleName, consts.ConfiguratorRuleSuffix) {
- name = ruleName[:len(ruleName)-len(consts.ConfiguratorRuleSuffix)]
- } else {
- c.JSON(http.StatusBadRequest, model.NewErrorResp(fmt.Sprintf("ruleName must end with %s", consts.ConfiguratorRuleSuffix)))
+ mesh := c.Query("mesh")
+ if strutil.IsBlank(ruleName) {
+ c.JSON(http.StatusBadRequest, model.NewErrorResp("ruleName cannot be empty"))
+ return
+ }
+ if strutil.IsBlank(mesh) {
+ c.JSON(http.StatusBadRequest, model.NewErrorResp("mesh cannot be empty"))
return
}
- res, err := service.GetConfigurator(ctx, name)
+ res, err := service.GetConfigurator(ctx, ruleName, mesh)
if err != nil {
- c.JSON(http.StatusBadRequest, model.NewErrorResp(err.Error()))
+ util.HandleServiceError(c, err)
return
}
+ if res == nil {
+ c.JSON(http.StatusOK, model.NewBizErrorResp(
+ bizerror.New(bizerror.NotFoundError, fmt.Sprintf("%s not found", ruleName))))
+ }
c.JSON(http.StatusOK, model.GenDynamicConfigToResp(res.Spec))
}
}
func PutConfiguratorWithRuleName(ctx consolectx.Context) gin.HandlerFunc {
return func(c *gin.Context) {
- var name string
ruleName := c.Param("ruleName")
- if strings.HasSuffix(ruleName, consts.ConfiguratorRuleSuffix) {
- name = ruleName[:len(ruleName)-len(consts.ConfiguratorRuleSuffix)]
- } else {
- c.JSON(http.StatusBadRequest, model.NewErrorResp(fmt.Sprintf("ruleName must end with %s", consts.ConfiguratorRuleSuffix)))
+ mesh := c.Query("mesh")
+ if !strings.HasSuffix(ruleName, constants.ConfiguratorRuleDotSuffix) {
+ c.JSON(http.StatusOK, model.NewBizErrorResp(bizerror.New(bizerror.InvalidArgument,
+ fmt.Sprintf("dynamic config name must end with %s", constants.ConfiguratorRuleDotSuffix))))
return
}
- res := &mesh.DynamicConfigResource{
- Meta: nil,
- Spec: &meshproto.DynamicConfig{},
- }
+ res := meshresource.NewDynamicConfigResourceWithAttributes(ruleName, mesh)
err := c.Bind(res.Spec)
if err != nil {
- c.JSON(http.StatusBadRequest, model.NewErrorResp(err.Error()))
+ util.HandleArgumentError(c, err)
return
}
- if err = service.UpdateConfigurator(ctx, name, res); err != nil {
- c.JSON(http.StatusBadRequest, model.NewErrorResp(err.Error()))
+ configurator, err := service.GetConfigurator(ctx, ruleName, mesh)
+ if err != nil {
+ util.HandleServiceError(c, err)
+ return
+ }
+ if configurator == nil {
+ c.JSON(http.StatusOK, model.NewBizErrorResp(
+ bizerror.New(bizerror.NotFoundError, fmt.Sprintf("%s not found", ruleName))))
+ }
+ if err = service.UpdateConfigurator(ctx, res); err != nil {
+ c.JSON(http.StatusOK, model.NewErrorResp(err.Error()))
return
Review Comment:
Same control-flow issue as the GET handler: when `configurator == nil`, the handler writes a NotFound response but then continues to update anyway. Add `return` after the NotFound response, otherwise the API can incorrectly report success/error after already responding.
##########
pkg/console/service/configurator_rule.go:
##########
@@ -18,50 +18,154 @@
package service
import (
- meshproto "github.com/apache/dubbo-admin/api/mesh/v1alpha1"
+ "github.com/apache/dubbo-admin/pkg/common/constants"
+ "github.com/apache/dubbo-admin/pkg/core/lock"
+ "github.com/duke-git/lancet/v2/slice"
+
+ "github.com/apache/dubbo-admin/pkg/common/bizerror"
consolectx "github.com/apache/dubbo-admin/pkg/console/context"
- "github.com/apache/dubbo-admin/pkg/core/consts"
+ "github.com/apache/dubbo-admin/pkg/console/model"
"github.com/apache/dubbo-admin/pkg/core/logger"
- coreresource "github.com/apache/dubbo-admin/pkg/core/resource/apis/mesh/v1alpha1"
+ "github.com/apache/dubbo-admin/pkg/core/manager"
+ meshresource "github.com/apache/dubbo-admin/pkg/core/resource/apis/mesh/v1alpha1"
coremodel "github.com/apache/dubbo-admin/pkg/core/resource/model"
- "github.com/apache/dubbo-admin/pkg/core/store"
+ "github.com/apache/dubbo-admin/pkg/core/store/index"
)
-func GetConfigurator(ctx consolectx.Context, name string) (*meshproto.DynamicConfig, error) {
- res := &coreresource.DynamicConfig{Spec: &meshproto.DynamicConfig{}}
- if err := ctx.ResourceManager().Get(ctx.AppContext(), res,
- // here `name` may be service-name or app-name, set *ByApplication(`name`) is ok.
- store.GetByApplication(name), store.GetByKey(name+consts.ConfiguratorRuleSuffix, coremodel.DefaultMesh)); err != nil {
- logger.Warnf("get %s configurator failed with error: %s", name, err.Error())
+func PageListConfiguratorRule(ctx consolectx.Context, req *model.SearchReq) (*model.SearchPaginationResult, error) {
+ pageData, err := manager.PageListByIndexes[*meshresource.DynamicConfigResource](
+ ctx.ResourceManager(),
+ meshresource.DynamicConfigKind,
+ map[string]string{
+ index.ByMeshIndex: req.Mesh,
+ },
+ req.PageReq)
+ if err != nil {
+ logger.Errorf("search dynamic config rule error: %v", err)
+ return nil, bizerror.New(bizerror.InternalError, "search dynamic config rule failed, please try again")
+ }
+ if pageData.Data == nil || len(pageData.Data) == 0 {
+ return &model.SearchPaginationResult{
+ List: nil,
+ PageInfo: coremodel.Pagination{
+ Total: 0,
+ PageSize: req.PageReq.PageSize,
+ PageOffset: req.PageReq.PageOffset,
+ },
+ }, nil
+ }
+ respList := slice.Map(pageData.Data, func(_ int, item *meshresource.DynamicConfigResource) *model.ConfiguratorSearchResp {
+ return &model.ConfiguratorSearchResp{
+ Scope: item.Spec.Scope,
+ CreateTime: "",
+ Enabled: item.Spec.Enabled,
+ RuleName: item.Name,
+ }
Review Comment:
`CreateTime` is always returned as an empty string for configurator rules, unlike condition rules where creation timestamp is populated. If the underlying resource provides timestamps (e.g., `item.CreationTimestamp`), populate it here to avoid regressions in list UX/API consumers expecting meaningful `createTime`.
##########
pkg/config/loader.go:
##########
@@ -18,39 +18,38 @@
package config
import (
+ "fmt"
"os"
- "github.com/kelseyhightower/envconfig"
"github.com/pkg/errors"
"sigs.k8s.io/yaml"
- "github.com/apache/dubbo-admin/pkg/core"
+ "github.com/apache/dubbo-admin/pkg/common/bizerror"
)
func Load(file string, cfg Config) error {
- return LoadWithOption(file, cfg, false, true, true)
+ return LoadWithOption(file, cfg, false, true)
}
-func LoadWithOption(file string, cfg Config, strict bool, includeEnv bool, validate bool) error {
+func LoadWithOption(file string, cfg Config, strict bool, validate bool) error {
if file == "" {
- core.Log.WithName("config").Info("skipping reading config from file")
- } else if err := loadFromFile(file, cfg, strict); err != nil {
- return err
+ return bizerror.New(bizerror.ConfigError, "config file is needed")
+ }
+ if err := loadFromFile(file, cfg, strict); err != nil {
+ return fmt.Errorf("configuration loading failed, %w", err)
}
Review Comment:
This changes config loading behavior to require a config file and removes environment-variable overlay support (previously supported via `envconfig.Process`). That’s a breaking behavior change for deployments relying on env-only configuration. If backward compatibility is required, consider keeping env overlay optional (or at least preserving the previous `LoadWithOption(... includeEnv ...)` path) and documenting the new requirement.
##########
pkg/console/handler/prometheus.go:
##########
@@ -17,30 +17,44 @@
package handler
-// proxy for prometheus
-
import (
+ "io"
"net/http"
- "net/http/httputil"
- "net/url"
"github.com/gin-gonic/gin"
+
+ "github.com/apache/dubbo-admin/pkg/common/bizerror"
+ consolectx "github.com/apache/dubbo-admin/pkg/console/context"
+ "github.com/apache/dubbo-admin/pkg/console/model"
)
func PromQL(ctx consolectx.Context) gin.HandlerFunc {
return func(c *gin.Context) {
- query := c.Request.URL.Query().Get("query")
- values := url.Values{}
- values.Add("query", query)
- promUrl := ctx.Config().Console.Prometheus + "/api/v1/query?" + values.Encode()
- proxyUrl, _ := url.Parse(promUrl)
- director := func(req *http.Request) {
- req.URL.Scheme = proxyUrl.Scheme
- req.URL.Host = proxyUrl.Host
- req.Host = proxyUrl.Host
- req.URL.Path = proxyUrl.Path
+ promBaseUrl := ctx.Config().Observability.PrometheusBaseURL
+ if promBaseUrl == nil {
+ c.JSON(http.StatusOK, model.NewBizErrorResp(
+ bizerror.New(bizerror.ConfigError, "Please configure prometheus url to retrieve metrics")))
+ return
+ }
+
+ u := *promBaseUrl
+ u.RawQuery = c.Request.URL.RawQuery
+ u.Path = "/api/v1/query"
+ s := u.String()
+ resp, err := http.Get(s)
+ if err != nil {
+ c.JSON(http.StatusOK, model.NewBizErrorResp(
+ bizerror.New(bizerror.NetWorkError, err.Error())))
+ return
+ }
+ defer resp.Body.Close()
+
+ body, err := io.ReadAll(resp.Body)
+ if err != nil {
+ c.JSON(http.StatusOK, model.NewBizErrorResp(
+ bizerror.New(bizerror.NetWorkError, err.Error())))
+ return
}
- proxy := &httputil.ReverseProxy{Director: director}
- proxy.ServeHTTP(c.Writer, c.Request)
+ c.Data(http.StatusOK, resp.Header.Get("Content-Type"), body)
Review Comment:
The handler uses `http.Get` without any timeout and ignores Prometheus response status codes (always returns `http.StatusOK` to the client). This can hang requests under network issues and can mask upstream 4xx/5xx as successful responses. Use an `http.Client` with a reasonable timeout (and preferably `http.NewRequestWithContext` using `c.Request.Context()`), and propagate `resp.StatusCode` when returning `c.Data(...)` (or map it appropriately).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] feat: add Model Context Protocol (MCP) server implementation [dubbo-admin]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #1463:
URL: https://github.com/apache/dubbo-admin/pull/1463#issuecomment-4296639782
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-admin&pullRequest=1463) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-admin&pullRequest=1463&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-admin&pullRequest=1463&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-admin&pullRequest=1463&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-admin&pullRequest=1463&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-admin&pullRequest=1463&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-admin&pullRequest=1463)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org