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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
