K8s/Folders: Remove folder service from client (#94450)

* Support getting full path of UIDs
* Use full path to set parents field
* Update get folder test
* Add folder store test for getting with full path UIDs
* Add test for parsing parent titles
* Test nested folder create payload
This commit is contained in:
Arati R.
2024-10-10 13:22:57 +02:00
committed by GitHub
parent bf9e5ae056
commit 011978e81b
10 changed files with 262 additions and 84 deletions
+36 -59
View File
@@ -20,6 +20,7 @@ import (
"github.com/grafana/grafana/pkg/api/routing"
folderalpha1 "github.com/grafana/grafana/pkg/apis/folder/v0alpha1"
"github.com/grafana/grafana/pkg/infra/metrics"
"github.com/grafana/grafana/pkg/infra/slugify"
internalfolders "github.com/grafana/grafana/pkg/registry/apis/folders"
"github.com/grafana/grafana/pkg/services/accesscontrol"
grafanaapiserver "github.com/grafana/grafana/pkg/services/apiserver"
@@ -635,8 +636,6 @@ type folderK8sHandler struct {
// #TODO check if it makes more sense to move this to FolderAPIBuilder
accesscontrolService accesscontrol.Service
userService user.Service
// #TODO remove after we handle the nested folder case
folderService folder.Service
}
//-----------------------------------------------------------------------------------------
@@ -650,7 +649,6 @@ func newFolderK8sHandler(hs *HTTPServer) *folderK8sHandler {
clientConfigProvider: hs.clientConfigProvider,
accesscontrolService: hs.accesscontrolService,
userService: hs.userService,
folderService: hs.folderService,
}
}
@@ -884,55 +882,38 @@ func (fk8s *folderK8sHandler) newToFolderDto(c *contextmodel.ReqContext, item un
return dtos.Folder{}, err
}
parents := []*folder.Folder{}
if folderDTO.ParentUID != "" {
parents, err = fk8s.folderService.GetParents(
c.Req.Context(),
folder.GetParentsQuery{
UID: folderDTO.UID,
OrgID: folderDTO.OrgID,
})
if err != nil {
return dtos.Folder{}, err
}
if len(f.Fullpath) == 0 || len(f.FullpathUIDs) == 0 {
return folderDTO, nil
}
// #TODO refactor so that we have just one function for converting to folder DTO
toParentDTO := func(fold *folder.Folder, checkCanView bool) (dtos.Folder, error) {
g, err := guardian.NewByFolder(c.Req.Context(), fold, c.SignedInUser.GetOrgID(), c.SignedInUser)
if err != nil {
return dtos.Folder{}, err
}
parentsFullPath, err := internalfolders.GetParentTitles(f.Fullpath)
if err != nil {
return dtos.Folder{}, err
}
parentsFullPathUIDs := strings.Split(f.FullpathUIDs, "/")
if checkCanView {
canView, _ := g.CanView()
if !canView {
return dtos.Folder{
UID: REDACTED,
Title: REDACTED,
}, nil
}
}
metrics.MFolderIDsAPICount.WithLabelValues(metrics.NewToFolderDTO).Inc()
return dtos.Folder{
UID: fold.UID,
Title: fold.Title,
URL: fold.URL,
}, nil
// The first part of the path is the newly created folder which we don't need to include
// in the parents field
if len(parentsFullPath) < 2 || len(parentsFullPathUIDs) < 2 {
return folderDTO, nil
}
folderDTO.Parents = make([]dtos.Folder, 0, len(parents))
for _, f := range parents {
DTO, err := toParentDTO(f, true)
if err != nil {
// #TODO add logging
// fk8s.log.Error("failed to convert folder to DTO", "folder", f.UID, "org", f.OrgID, "error", err)
continue
}
folderDTO.Parents = append(folderDTO.Parents, DTO)
parents := []dtos.Folder{}
for i, v := range parentsFullPath[1:] {
slug := slugify.Slugify(v)
uid := parentsFullPathUIDs[1:][i]
url := dashboards.GetFolderURL(uid, slug)
parents = append(parents, dtos.Folder{
UID: uid,
OrgID: c.SignedInUser.GetOrgID(),
Title: v,
URL: url,
})
}
folderDTO.Parents = parents
return folderDTO, nil
}
@@ -953,23 +934,19 @@ func (fk8s *folderK8sHandler) getFolderACMetadata(c *contextmodel.ReqContext, f
return nil, nil
}
var err error
parents := []*folder.Folder{}
if f.ParentUID != "" {
parents, err = fk8s.folderService.GetParents(
c.Req.Context(),
folder.GetParentsQuery{
UID: f.UID,
OrgID: c.SignedInUser.GetOrgID(),
})
if err != nil {
return nil, err
}
if len(f.FullpathUIDs) == 0 {
return map[string]bool{}, nil
}
parentsFullPathUIDs := strings.Split(f.FullpathUIDs, "/")
// The first part of the path is the newly created folder which we don't need to check here
if len(parentsFullPathUIDs) < 2 {
return map[string]bool{}, nil
}
folderIDs := map[string]bool{f.UID: true}
for _, p := range parents {
folderIDs[p.UID] = true
for _, uid := range parentsFullPathUIDs[1:] {
folderIDs[uid] = true
}
allMetadata := getMultiAccessControlMetadata(c, dashboards.ScopeFoldersPrefix, folderIDs)
+26
View File
@@ -34,6 +34,11 @@ const AnnoKeyOriginPath = "grafana.app/originPath"
const AnnoKeyOriginHash = "grafana.app/originHash"
const AnnoKeyOriginTimestamp = "grafana.app/originTimestamp"
// #TODO revisit keeping these folder-specific annotations once we have complete support for mode 1
const AnnoKeyFullPath = "grafana.app/fullPath"
const AnnoKeyFullPathUIDs = "grafana.app/fullPathUIDs"
// ResourceOriginInfo is saved in annotations. This is used to identify where the resource came from
// This object can model the same data as our existing provisioning table or a more general git sync
type ResourceOriginInfo struct {
@@ -101,6 +106,11 @@ type GrafanaMetaAccessor interface {
// NOTE the type must match the existing value, or an error will be thrown
SetStatus(any) error
GetFullPath() string
SetFullPath(path string)
GetFullPathUIDs() string
SetFullPathUIDs(path string)
// Find a title in the object
// This will reflect the object and try to get:
// * spec.title
@@ -598,6 +608,22 @@ func (m *grafanaMetaAccessor) SetStatus(s any) (err error) {
return
}
func (m *grafanaMetaAccessor) GetFullPath() string {
return m.get(AnnoKeyFullPath)
}
func (m *grafanaMetaAccessor) SetFullPath(path string) {
m.SetAnnotation(AnnoKeyFullPath, path)
}
func (m *grafanaMetaAccessor) GetFullPathUIDs() string {
return m.get(AnnoKeyFullPathUIDs)
}
func (m *grafanaMetaAccessor) SetFullPathUIDs(path string) {
m.SetAnnotation(AnnoKeyFullPathUIDs, path)
}
func (m *grafanaMetaAccessor) FindTitle(defaultTitle string) string {
// look for Spec.Title or Spec.Name
spec := m.r.FieldByName("Spec")
+32 -4
View File
@@ -2,6 +2,7 @@ package folders
import (
"fmt"
"regexp"
"strconv"
"time"
@@ -85,10 +86,12 @@ func UnstructuredToLegacyFolder(item unstructured.Unstructured, orgID int64) *fo
// #TODO add created by field if necessary
// CreatedBy: meta.GetCreatedBy(),
// UpdatedBy: meta.GetCreatedBy(),
URL: getURL(meta, title),
Created: createdTime,
Updated: createdTime,
OrgID: orgID,
URL: getURL(meta, title),
Created: createdTime,
Updated: createdTime,
OrgID: orgID,
Fullpath: meta.GetFullPath(),
FullpathUIDs: meta.GetFullPathUIDs(),
}
return f
}
@@ -183,6 +186,12 @@ func convertToK8sResource(v *folder.Folder, namespacer request.NamespaceMapper)
if v.ParentUID != "" {
meta.SetFolder(v.ParentUID)
}
if v.Fullpath != "" {
meta.SetFullPath(v.Fullpath)
}
if v.FullpathUIDs != "" {
meta.SetFullPathUIDs(v.FullpathUIDs)
}
f.UID = gapiutil.CalculateClusterWideUID(f)
return f, nil
}
@@ -226,3 +235,22 @@ func getCreated(meta utils.GrafanaMetaAccessor) (*time.Time, error) {
}
return created, nil
}
func GetParentTitles(fullPath string) ([]string, error) {
// Find all forward slashes which aren't escaped
r, err := regexp.Compile(`[^\\](/)`)
if err != nil {
return nil, err
}
indices := r.FindAllStringIndex(fullPath, -1)
var start int
titles := []string{}
for _, i := range indices {
titles = append(titles, fullPath[start:i[0]+1])
start = i[0] + 2
}
titles = append(titles, fullPath[start:])
return titles, nil
}
@@ -0,0 +1,18 @@
package folders
import (
"testing"
"github.com/stretchr/testify/require"
)
func TestGetParentTitles(t *testing.T) {
path := "get\\/folder-folder-0/get\\/folder-folder-1/another"
titles, err := GetParentTitles(path)
require.Nil(t, err)
require.Equal(t, 3, len(titles))
require.Equal(t, "get\\/folder-folder-0", titles[0])
require.Equal(t, "get\\/folder-folder-1", titles[1])
require.Equal(t, "another", titles[2])
}
+27 -1
View File
@@ -260,6 +260,7 @@ func (s *Service) Get(ctx context.Context, q *folder.GetFolderQuery) (*folder.Fo
if !s.features.IsEnabled(ctx, featuremgmt.FlagNestedFolders) {
dashFolder.Fullpath = dashFolder.Title
dashFolder.FullpathUIDs = dashFolder.UID
return dashFolder, nil
}
@@ -282,7 +283,8 @@ func (s *Service) Get(ctx context.Context, q *folder.GetFolderQuery) (*folder.Fo
f.Version = dashFolder.Version
if !s.features.IsEnabled(ctx, featuremgmt.FlagNestedFolders) {
f.Fullpath = f.Title // set full path to the folder title (unescaped)
f.Fullpath = f.Title // set full path to the folder title (unescaped)
f.FullpathUIDs = f.UID // set full path to the folder UID
}
return f, err
@@ -671,6 +673,30 @@ func (s *Service) Create(ctx context.Context, cmd *folder.CreateFolderCommand) (
return nil, err
}
if s.features.IsEnabled(ctx, featuremgmt.FlagKubernetesFolders) {
if f.ParentUID == "" {
return f, nil
}
// Fetch the parent since the permissions for fetching the newly created folder
// are not yet present for the user--this requires a call to ClearUserPermissionCache
parent, err := s.Get(ctx, &folder.GetFolderQuery{
UID: &f.ParentUID,
OrgID: f.OrgID,
WithFullpath: true,
WithFullpathUIDs: true,
SignedInUser: user,
})
if err != nil {
return nil, err
}
// #TODO revisit setting permissions so that we can centralise the logic for escaping slashes in titles
// Escape forward slashes in the title
title := strings.Replace(f.Title, "/", "\\/", -1)
f.Fullpath = title + "/" + parent.Fullpath
f.FullpathUIDs = f.UID + "/" + parent.FullpathUIDs
}
return f, nil
}
+31 -14
View File
@@ -501,7 +501,7 @@ func TestIntegrationNestedFolderService(t *testing.T) {
lps, err := librarypanels.ProvideService(cfg, db, routeRegister, elementService, serviceWithFlagOn)
require.NoError(t, err)
ancestors := CreateSubtreeInStore(t, nestedFolderStore, serviceWithFlagOn, depth, "getDescendantCountsOn", createCmd)
ancestors := CreateSubtreeInStore(t, nestedFolderStore, serviceWithFlagOn, depth, "getDescendantCountsOn", createCmd, true)
parent, err := serviceWithFlagOn.dashboardFolderStore.GetFolderByUID(context.Background(), orgID, ancestors[0].UID)
require.NoError(t, err)
@@ -584,7 +584,7 @@ func TestIntegrationNestedFolderService(t *testing.T) {
lps, err := librarypanels.ProvideService(cfg, db, routeRegister, elementService, serviceWithFlagOff)
require.NoError(t, err)
ancestors := CreateSubtreeInStore(t, nestedFolderStore, serviceWithFlagOn, depth, "getDescendantCountsOff", createCmd)
ancestors := CreateSubtreeInStore(t, nestedFolderStore, serviceWithFlagOn, depth, "getDescendantCountsOff", createCmd, true)
parent, err := serviceWithFlagOn.dashboardFolderStore.GetFolderByUID(context.Background(), orgID, ancestors[0].UID)
require.NoError(t, err)
@@ -725,7 +725,7 @@ func TestIntegrationNestedFolderService(t *testing.T) {
alertStore, err := ngstore.ProvideDBStore(cfg, tc.featuresFlag, db, tc.service, dashSrv, ac)
require.NoError(t, err)
ancestors := CreateSubtreeInStore(t, nestedFolderStore, serviceWithFlagOn, tc.depth, tc.prefix, createCmd)
ancestors := CreateSubtreeInStore(t, nestedFolderStore, serviceWithFlagOn, tc.depth, tc.prefix, createCmd, true)
parent, err := serviceWithFlagOn.dashboardFolderStore.GetFolderByUID(context.Background(), orgID, ancestors[0].UID)
require.NoError(t, err)
@@ -1536,8 +1536,8 @@ func TestIntegrationNestedFolderSharedWithMe(t *testing.T) {
t.Run("Should get folders shared with given user", func(t *testing.T) {
depth := 3
ancestorFoldersWithPermissions := CreateSubtreeInStore(t, nestedFolderStore, serviceWithFlagOn, depth, "withPermissions", createCmd)
ancestorFoldersWithoutPermissions := CreateSubtreeInStore(t, nestedFolderStore, serviceWithFlagOn, depth, "withoutPermissions", createCmd)
ancestorFoldersWithPermissions := CreateSubtreeInStore(t, nestedFolderStore, serviceWithFlagOn, depth, "withPermissions", createCmd, true)
ancestorFoldersWithoutPermissions := CreateSubtreeInStore(t, nestedFolderStore, serviceWithFlagOn, depth, "withoutPermissions", createCmd, true)
parent, err := serviceWithFlagOn.dashboardFolderStore.GetFolderByUID(context.Background(), orgID, ancestorFoldersWithoutPermissions[0].UID)
require.NoError(t, err)
@@ -1661,8 +1661,8 @@ func TestIntegrationNestedFolderSharedWithMe(t *testing.T) {
// tree2-folder-0
// └──tree2-folder-1
// └──tree2-folder-2
tree1 := CreateSubtreeInStore(t, nestedFolderStore, serviceWithFlagOn, depth, "tree1-", createCmd)
tree2 := CreateSubtreeInStore(t, nestedFolderStore, serviceWithFlagOn, depth, "tree2-", createCmd)
tree1 := CreateSubtreeInStore(t, nestedFolderStore, serviceWithFlagOn, depth, "tree1-", createCmd, true)
tree2 := CreateSubtreeInStore(t, nestedFolderStore, serviceWithFlagOn, depth, "tree2-", createCmd, true)
signedInUser.Permissions[orgID][dashboards.ActionFoldersRead] = []string{
// Add permission to tree1-folder-0
@@ -1928,14 +1928,16 @@ func TestFolderServiceGetFolder(t *testing.T) {
}
depth := 3
folders := CreateSubtreeInStore(t, folderSvcOn.store, &folderSvcOn, depth, "get/folder-", createCmd)
folders := CreateSubtreeInStore(t, folderSvcOn.store, &folderSvcOn, depth, "get/folder-", createCmd, false)
f := folders[1]
testCases := []struct {
name string
svc *Service
WithFullpath bool
expectedFullpath string
name string
svc *Service
WithFullpath bool
WithFullpathUIDs bool
expectedFullpath string
expectedFullpathUIDs string
}{
{
name: "when flag is off",
@@ -1954,6 +1956,18 @@ func TestFolderServiceGetFolder(t *testing.T) {
WithFullpath: true,
expectedFullpath: "get\\/folder-folder-0/get\\/folder-folder-1",
},
{
name: "when flag is on and WithFullpathUIDs is false",
svc: &folderSvcOn,
WithFullpathUIDs: false,
expectedFullpathUIDs: "",
},
{
name: "when flag is on and WithFullpathUIDs is true",
svc: &folderSvcOn,
WithFullpathUIDs: true,
expectedFullpathUIDs: "uidfor-0/uidfor-1",
},
}
for _, tc := range testCases {
@@ -2021,7 +2035,7 @@ func TestFolderServiceGetFolders(t *testing.T) {
})
prefix := "getfolders/ff/off"
folders := CreateSubtreeInStore(t, nestedFolderStore, serviceWithFlagOff, 5, prefix, createCmd)
folders := CreateSubtreeInStore(t, nestedFolderStore, serviceWithFlagOff, 5, prefix, createCmd, true)
f := folders[rand.Intn(len(folders))]
t.Run("when flag is off", func(t *testing.T) {
@@ -2510,7 +2524,7 @@ func TestSupportBundle(t *testing.T) {
}
}
func CreateSubtreeInStore(t *testing.T, store folder.Store, service *Service, depth int, prefix string, cmd folder.CreateFolderCommand) []*folder.Folder {
func CreateSubtreeInStore(t *testing.T, store folder.Store, service *Service, depth int, prefix string, cmd folder.CreateFolderCommand, randomUID bool) []*folder.Folder {
t.Helper()
folders := make([]*folder.Folder, 0, depth)
@@ -2518,6 +2532,9 @@ func CreateSubtreeInStore(t *testing.T, store folder.Store, service *Service, de
title := fmt.Sprintf("%sfolder-%d", prefix, i)
cmd.Title = title
cmd.UID = util.GenerateShortUID()
if !randomUID {
cmd.UID = fmt.Sprintf("uidfor-%d", i)
}
cmd.OrgID = orgID
cmd.SignedInUser = &user.SignedInUser{OrgID: orgID, Permissions: map[int64]map[string][]string{orgID: {dashboards.ActionFoldersCreate: {dashboards.ScopeFoldersAll}}}}
+5 -1
View File
@@ -201,8 +201,11 @@ func (ss *FolderStoreImpl) Get(ctx context.Context, q folder.GetFolderQuery) (*f
if q.WithFullpath {
s.WriteString(fmt.Sprintf(`, %s AS fullpath`, getFullpathSQL(ss.db.GetDialect())))
}
if q.WithFullpathUIDs {
s.WriteString(fmt.Sprintf(`, %s AS fullpath_uids`, getFullapathUIDsSQL(ss.db.GetDialect())))
}
s.WriteString(" FROM folder f0")
if q.WithFullpath {
if q.WithFullpath || q.WithFullpathUIDs {
s.WriteString(getFullpathJoinsSQL())
}
switch {
@@ -241,6 +244,7 @@ func (ss *FolderStoreImpl) Get(ctx context.Context, q folder.GetFolderQuery) (*f
})
foldr.Fullpath = strings.TrimLeft(foldr.Fullpath, "/")
foldr.FullpathUIDs = strings.TrimLeft(foldr.FullpathUIDs, "/")
return foldr.WithURL(), err
}
@@ -485,6 +485,24 @@ func TestIntegrationGet(t *testing.T) {
assert.NotEmpty(t, ff.Updated)
assert.NotEmpty(t, ff.URL)
})
t.Run("get folder withFullpathUIDs should set fullpathUIDs as expected", func(t *testing.T) {
ff, err := folderStore.Get(context.Background(), folder.GetFolderQuery{
UID: &subfolderWithSameName.UID,
OrgID: orgID,
WithFullpathUIDs: true,
})
require.NoError(t, err)
assert.Equal(t, subfolderWithSameName.UID, ff.UID)
assert.Equal(t, subfolderWithSameName.OrgID, ff.OrgID)
assert.Equal(t, subfolderWithSameName.Title, ff.Title)
assert.Equal(t, subfolderWithSameName.Description, ff.Description)
assert.Equal(t, path.Join(f.UID, subfolderWithSameName.UID), ff.FullpathUIDs)
assert.Equal(t, f.UID, ff.ParentUID)
assert.NotEmpty(t, ff.Created)
assert.NotEmpty(t, ff.Updated)
assert.NotEmpty(t, ff.URL)
})
}
func TestIntegrationGetParents(t *testing.T) {
+6 -5
View File
@@ -148,11 +148,12 @@ type DeleteFolderCommand struct {
type GetFolderQuery struct {
UID *string
// Deprecated: use FolderUID instead
ID *int64
Title *string
ParentUID *string
OrgID int64
WithFullpath bool
ID *int64
Title *string
ParentUID *string
OrgID int64
WithFullpath bool
WithFullpathUIDs bool
SignedInUser identity.Requester `json:"-"`
}
+63
View File
@@ -3,6 +3,7 @@ package playlist
import (
"context"
"encoding/json"
"fmt"
"net/http"
"slices"
@@ -13,6 +14,7 @@ import (
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime/schema"
"github.com/grafana/grafana/pkg/api/dtos"
folderv0alpha1 "github.com/grafana/grafana/pkg/apis/folder/v0alpha1"
grafanarest "github.com/grafana/grafana/pkg/apiserver/rest"
"github.com/grafana/grafana/pkg/services/featuremgmt"
@@ -286,6 +288,24 @@ func TestIntegrationFoldersApp(t *testing.T) {
doFolderTests(t, helper)
})
t.Run("with dual write (unified storage, mode 1, nested folders)", func(t *testing.T) {
checkNestedCreate(t, apis.NewK8sTestHelper(t, testinfra.GrafanaOpts{
AppModeProduction: true,
DisableAnonymous: true,
APIServerStorageType: "unified",
UnifiedStorageConfig: map[string]setting.UnifiedStorageConfig{
folderv0alpha1.RESOURCEGROUP: {
DualWriterMode: grafanarest.Mode1,
},
},
EnableFeatureToggles: []string{
featuremgmt.FlagGrafanaAPIServerTestingWithExperimentalAPIs,
featuremgmt.FlagNestedFolders,
featuremgmt.FlagKubernetesFolders,
},
}))
})
}
func doFolderTests(t *testing.T, helper *apis.K8sTestHelper) *apis.K8sTestHelper {
@@ -368,6 +388,49 @@ func doFolderTests(t *testing.T, helper *apis.K8sTestHelper) *apis.K8sTestHelper
return helper
}
func checkNestedCreate(t *testing.T, helper *apis.K8sTestHelper) {
client := helper.GetResourceClient(apis.ResourceClientArgs{
User: helper.Org1.Admin,
GVR: gvr,
})
parentPayload := `{
"title": "Test/parent",
"uid": ""
}`
parentCreate := apis.DoRequest(helper, apis.RequestParams{
User: client.Args.User,
Method: http.MethodPost,
Path: "/api/folders",
Body: []byte(parentPayload),
}, &folder.Folder{})
require.NotNil(t, parentCreate.Result)
parentUID := parentCreate.Result.UID
require.NotEmpty(t, parentUID)
childPayload := fmt.Sprintf(`{
"title": "Test/child",
"uid": "",
"parentUid": "%s"
}`, parentUID)
childCreate := apis.DoRequest(helper, apis.RequestParams{
User: client.Args.User,
Method: http.MethodPost,
Path: "/api/folders",
Body: []byte(childPayload),
}, &dtos.Folder{})
require.NotNil(t, childCreate.Result)
childUID := childCreate.Result.UID
require.NotEmpty(t, childUID)
require.Equal(t, "Test/child", childCreate.Result.Title)
require.Equal(t, 1, len(childCreate.Result.Parents))
parent := childCreate.Result.Parents[0]
require.Equal(t, parentUID, parent.UID)
require.Equal(t, "Test\\/parent", parent.Title)
require.Equal(t, parentCreate.Result.URL, parent.URL)
}
// This does a get with both k8s and legacy API, and verifies the results are the same
func getFromBothAPIs(t *testing.T,
helper *apis.K8sTestHelper,