Dashboard: Improve search error response (#113617)

* Dashboard: Improve search error response

* improve errors
This commit is contained in:
Ezequiel Victorero
2025-11-08 12:58:51 +01:00
committed by GitHub
parent aa75cc5bbe
commit 24e4e0946d
2 changed files with 65 additions and 16 deletions
@@ -8,6 +8,7 @@ import (
"strings" "strings"
"google.golang.org/grpc" "google.golang.org/grpc"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/selection" "k8s.io/apimachinery/pkg/selection"
claims "github.com/grafana/authlib/types" claims "github.com/grafana/authlib/types"
@@ -62,7 +63,7 @@ func ParseSortName(sortName string) (string, bool, error) {
} }
} }
return "", false, fmt.Errorf("no matching sort field found for: %s", sortName) return "", false, apierrors.NewBadRequest(fmt.Sprintf("no matching sort field found for: %s", sortName))
} }
// nolint:gocyclo // nolint:gocyclo
@@ -97,11 +98,11 @@ func (c *DashboardSearchClient) Search(ctx context.Context, req *resourcepb.Reso
case folders.RESOURCE: case folders.RESOURCE:
queryType = searchstore.TypeFolder queryType = searchstore.TypeFolder
default: default:
return nil, fmt.Errorf("bad type request") return nil, apierrors.NewBadRequest("bad type request")
} }
if len(req.Federated) > 1 { if len(req.Federated) > 1 {
return nil, fmt.Errorf("bad type request") return nil, apierrors.NewBadRequest("bad type request")
} }
if len(req.Federated) == 1 && if len(req.Federated) == 1 &&
@@ -117,7 +118,7 @@ func (c *DashboardSearchClient) Search(ctx context.Context, req *resourcepb.Reso
sortByField := "" sortByField := ""
if len(req.SortBy) != 0 { if len(req.SortBy) != 0 {
if len(req.SortBy) > 1 { if len(req.SortBy) > 1 {
return nil, fmt.Errorf("only one sort field is supported") return nil, apierrors.NewBadRequest("only one sort field is supported")
} }
sort := req.SortBy[0] sort := req.SortBy[0]
sortByField = strings.TrimPrefix(sort.Field, resource.SEARCH_FIELD_PREFIX) sortByField = strings.TrimPrefix(sort.Field, resource.SEARCH_FIELD_PREFIX)
@@ -208,13 +209,13 @@ func (c *DashboardSearchClient) Search(ctx context.Context, req *resourcepb.Reso
case resource.SEARCH_FIELD_SOURCE_PATH: case resource.SEARCH_FIELD_SOURCE_PATH:
// only one value is supported in legacy search // only one value is supported in legacy search
if len(vals) != 1 { if len(vals) != 1 {
return nil, fmt.Errorf("only one repo path query is supported") return nil, apierrors.NewBadRequest("only one repo path query is supported")
} }
query.SourcePath = vals[0] query.SourcePath = vals[0]
case resource.SEARCH_FIELD_MANAGER_KIND: case resource.SEARCH_FIELD_MANAGER_KIND:
if len(vals) != 1 { if len(vals) != 1 {
return nil, fmt.Errorf("only one manager kind supported") return nil, apierrors.NewBadRequest("only one manager kind supported")
} }
query.ManagedBy = utils.ManagerKind(vals[0]) query.ManagedBy = utils.ManagerKind(vals[0])
@@ -226,20 +227,20 @@ func (c *DashboardSearchClient) Search(ctx context.Context, req *resourcepb.Reso
// only one value is supported in legacy search // only one value is supported in legacy search
if len(vals) != 1 { if len(vals) != 1 {
return nil, fmt.Errorf("only one repo name is supported") return nil, apierrors.NewBadRequest("only one repo name is supported")
} }
query.ManagerIdentity = vals[0] query.ManagerIdentity = vals[0]
case unisearch.DASHBOARD_LIBRARY_PANEL_REFERENCE: case unisearch.DASHBOARD_LIBRARY_PANEL_REFERENCE:
if len(vals) != 1 { if len(vals) != 1 {
return nil, fmt.Errorf("only one library panel uid is supported") return nil, apierrors.NewBadRequest("only one library panel uid is supported")
} }
// Make sure the query does not include incompatible combinations // Make sure the query does not include incompatible combinations
for _, f := range req.Options.Fields { for _, f := range req.Options.Fields {
switch f.Key { switch f.Key {
case resource.SEARCH_FIELD_NAME: case resource.SEARCH_FIELD_NAME:
return nil, fmt.Errorf("libraryPanel query must not include explicit names") return nil, apierrors.NewBadRequest("libraryPanel query must not include explicit names")
} }
} }
@@ -257,7 +258,7 @@ func (c *DashboardSearchClient) Search(ctx context.Context, req *resourcepb.Reso
case resource.SEARCH_FIELD_TITLE_PHRASE: case resource.SEARCH_FIELD_TITLE_PHRASE:
if len(vals) != 1 { if len(vals) != 1 {
return nil, fmt.Errorf("only one title supported") return nil, apierrors.NewBadRequest("only one title supported")
} }
query.Title = vals[0] query.Title = vals[0]
@@ -276,7 +277,7 @@ func (c *DashboardSearchClient) Search(ctx context.Context, req *resourcepb.Reso
// legacy sql query, since legacy search does not support this // legacy sql query, since legacy search does not support this
if query.ManagerIdentity != "" || len(query.ManagerIdentityNotIn) > 0 || query.ManagedBy != "" { if query.ManagerIdentity != "" || len(query.ManagerIdentityNotIn) > 0 || query.ManagedBy != "" {
if query.ManagedBy == utils.ManagerKindUnknown { if query.ManagedBy == utils.ManagerKindUnknown {
return nil, fmt.Errorf("query by manager identity also requires manager.kind parameter") return nil, apierrors.NewBadRequest("query by manager identity also requires manager.kind parameter")
} }
// for plugin and orphaned dashboards, we will only return the manager kind alongside the regular search response // for plugin and orphaned dashboards, we will only return the manager kind alongside the regular search response
@@ -399,19 +400,19 @@ func (c *DashboardSearchClient) getLibraryPanelConnections(ctx context.Context,
func (c *DashboardSearchClient) GetStats(ctx context.Context, req *resourcepb.ResourceStatsRequest, _ ...grpc.CallOption) (*resourcepb.ResourceStatsResponse, error) { func (c *DashboardSearchClient) GetStats(ctx context.Context, req *resourcepb.ResourceStatsRequest, _ ...grpc.CallOption) (*resourcepb.ResourceStatsResponse, error) {
info, err := claims.ParseNamespace(req.Namespace) info, err := claims.ParseNamespace(req.Namespace)
if err != nil { if err != nil {
return nil, fmt.Errorf("unable to read namespace") return nil, apierrors.NewInternalError(fmt.Errorf("unable to read namespace: %w", err))
} }
if info.OrgID == 0 { if info.OrgID == 0 {
return nil, fmt.Errorf("invalid OrgID found in namespace") return nil, apierrors.NewInternalError(fmt.Errorf("invalid OrgID found in namespace"))
} }
if len(req.Kinds) != 1 { if len(req.Kinds) != 1 {
return nil, fmt.Errorf("only can query for dashboard kind in legacy fallback") return nil, apierrors.NewBadRequest("only can query for dashboard kind in legacy fallback")
} }
parts := strings.SplitN(req.Kinds[0], "/", 2) parts := strings.SplitN(req.Kinds[0], "/", 2)
if len(parts) != 2 { if len(parts) != 2 {
return nil, fmt.Errorf("invalid kind") return nil, apierrors.NewBadRequest("invalid kind")
} }
var count int64 var count int64
@@ -421,7 +422,7 @@ func (c *DashboardSearchClient) GetStats(ctx context.Context, req *resourcepb.Re
case folders.GROUP: case folders.GROUP:
count, err = c.dashboardStore.CountInOrg(ctx, info.OrgID, true) count, err = c.dashboardStore.CountInOrg(ctx, info.OrgID, true)
default: default:
return nil, fmt.Errorf("invalid group") return nil, apierrors.NewBadRequest("invalid group")
} }
if err != nil { if err != nil {
return nil, err return nil, err
@@ -645,6 +645,54 @@ func TestDashboardSearchClient_Search(t *testing.T) {
require.Contains(t, err.Error(), "only one library panel uid is supported") require.Contains(t, err.Error(), "only one library panel uid is supported")
require.Nil(t, resp) require.Nil(t, resp)
}) })
t.Run("Should reject library panel query when combined with explicit dashboard names", func(t *testing.T) {
req := &resourcepb.ResourceSearchRequest{
Options: &resourcepb.ListOptions{
Key: dashboardKey,
Fields: []*resourcepb.Requirement{
{
Key: unisearch.DASHBOARD_LIBRARY_PANEL_REFERENCE,
Operator: "=",
Values: []string{"test-library-panel"},
},
{
Key: resource.SEARCH_FIELD_NAME,
Operator: "=",
Values: []string{"dashboard-uid"},
},
},
},
}
resp, err := client.Search(ctx, req)
require.Error(t, err)
require.Contains(t, err.Error(), "libraryPanel query must not include explicit names")
require.Nil(t, resp)
// Note: mockStore should NOT be called since validation happens before any store calls
})
t.Run("Should return empty results when library panel has no connected dashboards", func(t *testing.T) {
mockStore.On("GetDashboardsByLibraryPanelUID", mock.Anything, "unused-library-panel", int64(2)).Return([]*dashboards.DashboardRef{}, nil).Once()
req := &resourcepb.ResourceSearchRequest{
Options: &resourcepb.ListOptions{
Key: dashboardKey,
Fields: []*resourcepb.Requirement{
{
Key: unisearch.DASHBOARD_LIBRARY_PANEL_REFERENCE,
Operator: "=",
Values: []string{"unused-library-panel"},
},
},
},
}
resp, err := client.Search(ctx, req)
require.NoError(t, err)
require.NotNil(t, resp)
require.Equal(t, int64(0), resp.TotalHits)
require.Empty(t, resp.Results.Rows)
mockStore.AssertExpectations(t)
})
} }
func TestParseSortName(t *testing.T) { func TestParseSortName(t *testing.T) {