K8s: Folders: Fix not found errors (#101585)

This commit is contained in:
Stephanie Hingtgen
2025-03-04 15:31:41 -07:00
committed by GitHub
parent 8832aa2aa2
commit 67221fb328
3 changed files with 87 additions and 17 deletions

View File

@ -2,7 +2,6 @@ package folders
import (
"context"
"errors"
"fmt"
"strings"
@ -138,13 +137,14 @@ func (s *legacyStorage) Get(ctx context.Context, name string, options *metav1.Ge
UID: &name,
OrgID: info.OrgID,
})
if err != nil || dto == nil {
if errors.Is(err, dashboards.ErrFolderNotFound) || err == nil {
err = resourceInfo.NewNotFound(name)
}
if err != nil {
statusErr := apierrors.ToFolderStatusError(err)
return nil, &statusErr
}
if dto == nil {
statusErr := apierrors.ToFolderStatusError(dashboards.ErrFolderNotFound)
return nil, &statusErr
}
r, err := convertToK8sResource(dto, s.namespacer)
if err != nil {

View File

@ -77,6 +77,10 @@ func (ss *FolderUnifiedStoreImpl) Delete(ctx context.Context, UIDs []string, org
func (ss *FolderUnifiedStoreImpl) Update(ctx context.Context, cmd folder.UpdateFolderCommand) (*folder.Folder, error) {
obj, err := ss.k8sclient.Get(ctx, cmd.UID, cmd.OrgID, v1.GetOptions{})
if err != nil {
if apierrors.IsNotFound(err) {
return nil, dashboards.ErrFolderNotFound
}
return nil, err
}
updated := obj.DeepCopy()
@ -144,7 +148,7 @@ func (ss *FolderUnifiedStoreImpl) GetParents(ctx context.Context, q folder.GetPa
parentUid := q.UID
for parentUid != "" {
out, err := ss.k8sclient.Get(ctx, parentUid, q.OrgID, v1.GetOptions{})
folder, err := ss.Get(ctx, folder.GetFolderQuery{UID: &parentUid, OrgID: q.OrgID})
if err != nil {
var statusError *apierrors.StatusError
if errors.As(err, &statusError) && statusError.ErrStatus.Code == http.StatusForbidden {
@ -155,11 +159,6 @@ func (ss *FolderUnifiedStoreImpl) GetParents(ctx context.Context, q folder.GetPa
return nil, err
}
folder, err := ss.UnstructuredToLegacyFolder(ctx, out)
if err != nil {
return nil, err
}
parentUid = folder.ParentUID
hits = append(hits, folder)
}
@ -183,6 +182,15 @@ func (ss *FolderUnifiedStoreImpl) GetChildren(ctx context.Context, q folder.GetC
q.Page = 1
}
if q.UID != "" {
// the original get children query fails if the parent folder does not exist
// check that the parent exists first
_, err := ss.Get(ctx, folder.GetFolderQuery{UID: &q.UID, OrgID: q.OrgID})
if err != nil {
return nil, err
}
}
req := &resource.ResourceSearchRequest{
Options: &resource.ListOptions{
Fields: []*resource.Requirement{
@ -229,12 +237,7 @@ func (ss *FolderUnifiedStoreImpl) GetChildren(ctx context.Context, q folder.GetC
}
// search only returns a subset of info, get all info of the folder
item, err := ss.k8sclient.Get(ctx, item.Name, q.OrgID, v1.GetOptions{})
if err != nil {
return nil, err
}
f, err := ss.UnstructuredToLegacyFolder(ctx, item)
f, err := ss.Get(ctx, folder.GetFolderQuery{UID: &item.Name, OrgID: q.OrgID})
if err != nil {
return nil, err
}
@ -411,6 +414,10 @@ func getDescendants(nodes map[string]*folder.Folder, tree map[string]map[string]
func (ss *FolderUnifiedStoreImpl) CountFolderContent(ctx context.Context, orgID int64, ancestor_uid string) (folder.DescendantCounts, error) {
counts, err := ss.k8sclient.Get(ctx, ancestor_uid, orgID, v1.GetOptions{}, "counts")
if err != nil {
if apierrors.IsNotFound(err) {
return nil, dashboards.ErrFolderNotFound
}
return nil, err
}

View File

@ -9,6 +9,7 @@ import (
"github.com/grafana/grafana/pkg/apimachinery/identity"
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/apiserver/client"
"github.com/grafana/grafana/pkg/services/dashboards"
"github.com/grafana/grafana/pkg/services/folder"
"github.com/grafana/grafana/pkg/storage/unified/resource"
"github.com/stretchr/testify/mock"
@ -16,6 +17,7 @@ import (
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/selection"
)
@ -170,6 +172,16 @@ func TestGetParents(t *testing.T) {
require.Len(t, result, 1)
require.Equal(t, "parenttwo", result[0].UID)
})
t.Run("should stop if parent folder is not found", func(t *testing.T) {
mockCli.On("Get", mock.Anything, "parentone", orgID, mock.Anything, mock.Anything).Return(nil, apierrors.NewNotFound(schema.GroupResource{Group: "folders.folder.grafana.app", Resource: "folder"}, "parentone")).Once()
_, err := store.GetParents(ctx, folder.GetParentsQuery{
UID: "parentone",
OrgID: orgID,
})
require.ErrorIs(t, err, dashboards.ErrFolderNotFound)
})
}
func TestGetChildren(t *testing.T) {
@ -213,6 +225,11 @@ func TestGetChildren(t *testing.T) {
},
TotalHits: 1,
}, nil).Once()
mockCli.On("Get", mock.Anything, "folder1", orgID, mock.Anything, mock.Anything).Return(&unstructured.Unstructured{
Object: map[string]interface{}{
"metadata": map[string]interface{}{"name": "folder1"},
},
}, nil).Once()
mockCli.On("Get", mock.Anything, "folder2", orgID, mock.Anything, mock.Anything).Return(&unstructured.Unstructured{
Object: map[string]interface{}{
"metadata": map[string]interface{}{"name": "folder2"},
@ -235,6 +252,47 @@ func TestGetChildren(t *testing.T) {
require.Equal(t, "folder3", result[1].UID)
})
t.Run("should return an error if the folder is not found", func(t *testing.T) {
mockCli.On("Search", mock.Anything, orgID, &resource.ResourceSearchRequest{
Options: &resource.ListOptions{
Fields: []*resource.Requirement{
{
Key: resource.SEARCH_FIELD_FOLDER,
Operator: string(selection.In),
Values: []string{"folder1"},
},
},
},
Limit: folderSearchLimit, // should default to folderSearchLimit
Offset: 0, // should be set as limit * (page - 1)
Page: 1, // should be set to 1 by default
}).Return(&resource.ResourceSearchResponse{
Results: &resource.ResourceTable{
Columns: []*resource.ResourceTableColumnDefinition{
{Name: "folder", Type: resource.ResourceTableColumnDefinition_STRING},
},
Rows: []*resource.ResourceTableRow{
{
Key: &resource.ResourceKey{Name: "folder2", Resource: "folder"},
Cells: [][]byte{[]byte("folder1")},
},
{
Key: &resource.ResourceKey{Name: "folder3", Resource: "folder"},
Cells: [][]byte{[]byte("folder1")},
},
},
},
TotalHits: 1,
}, nil).Once()
mockCli.On("Get", mock.Anything, "folder1", orgID, mock.Anything, mock.Anything).Return(nil, apierrors.NewNotFound(schema.GroupResource{Group: "folders.folder.grafana.app", Resource: "folder"}, "folder1")).Once()
_, err := store.GetChildren(ctx, folder.GetChildrenQuery{
UID: "folder1",
OrgID: orgID,
})
require.ErrorIs(t, err, dashboards.ErrFolderNotFound)
})
t.Run("pages should be able to be set, general folder should be turned to empty string, and folder uids should be passed in", func(t *testing.T) {
mockCli.On("Search", mock.Anything, orgID, &resource.ResourceSearchRequest{
Options: &resource.ListOptions{
@ -301,6 +359,11 @@ func TestGetChildren(t *testing.T) {
},
TotalHits: 1,
}, nil)
mockCli.On("Get", mock.Anything, "folder", orgID, mock.Anything, mock.Anything).Return(&unstructured.Unstructured{
Object: map[string]interface{}{
"metadata": map[string]interface{}{"name": "folder"},
},
}, nil)
mockCli.On("Get", mock.Anything, accesscontrol.K6FolderUID, orgID, mock.Anything, mock.Anything).Return(&unstructured.Unstructured{
Object: map[string]interface{}{
"metadata": map[string]interface{}{"name": accesscontrol.K6FolderUID},