diff --git a/pkg/api/folder.go b/pkg/api/folder.go index ba95523964f..047923e7950 100644 --- a/pkg/api/folder.go +++ b/pkg/api/folder.go @@ -49,7 +49,6 @@ func (hs *HTTPServer) registerFolderAPI(apiRoute routing.RouteRegister, authoriz folderRoute.Get("/id/:id", authorize(accesscontrol.EvalPermission(dashboards.ActionFoldersRead, idScope)), routing.Wrap(hs.GetFolderByID)) folderRoute.Group("/:uid", func(folderUidRoute routing.RouteRegister) { - folderUidRoute.Put("/", authorize(accesscontrol.EvalPermission(dashboards.ActionFoldersWrite, uidScope)), routing.Wrap(hs.UpdateFolder)) folderUidRoute.Post("/move", authorize(accesscontrol.EvalPermission(dashboards.ActionFoldersWrite, uidScope)), routing.Wrap(hs.MoveFolder)) folderUidRoute.Get("/counts", authorize(accesscontrol.EvalPermission(dashboards.ActionFoldersRead, uidScope)), routing.Wrap(hs.GetFolderDescendantCounts)) @@ -64,6 +63,7 @@ func (hs *HTTPServer) registerFolderAPI(apiRoute routing.RouteRegister, authoriz folderRoute.Post("/", handler.createFolder) folderRoute.Get("/", handler.getFolders) folderRoute.Group("/:uid", func(folderUidRoute routing.RouteRegister) { + folderUidRoute.Put("/", handler.updateFolder) folderUidRoute.Delete("/", handler.deleteFolder) folderUidRoute.Get("/", handler.getFolder) }) @@ -71,17 +71,11 @@ func (hs *HTTPServer) registerFolderAPI(apiRoute routing.RouteRegister, authoriz folderRoute.Post("/", authorize(accesscontrol.EvalPermission(dashboards.ActionFoldersCreate)), routing.Wrap(hs.CreateFolder)) folderRoute.Get("/", authorize(accesscontrol.EvalPermission(dashboards.ActionFoldersRead)), routing.Wrap(hs.GetFolders)) folderRoute.Group("/:uid", func(folderUidRoute routing.RouteRegister) { + folderUidRoute.Put("/", authorize(accesscontrol.EvalPermission(dashboards.ActionFoldersWrite, uidScope)), routing.Wrap(hs.UpdateFolder)) folderUidRoute.Delete("/", authorize(accesscontrol.EvalPermission(dashboards.ActionFoldersDelete, uidScope)), routing.Wrap(hs.DeleteFolder)) folderUidRoute.Get("/", authorize(accesscontrol.EvalPermission(dashboards.ActionFoldersRead, uidScope)), routing.Wrap(hs.GetFolderByUID)) }) } - // Only adding support for some routes with the k8s handler for now. Include the rest here. - if false { - handler := newFolderK8sHandler(hs) - folderRoute.Group("/:uid", func(folderUidRoute routing.RouteRegister) { - folderUidRoute.Put("/:uid", handler.updateFolder) - }) - } }) } @@ -969,7 +963,7 @@ func (fk8s *folderK8sHandler) toDTO(c *contextmodel.ReqContext, fold *folder.Fol UpdatedBy: updater, Updated: fold.Updated, // #TODO version doesn't seem to be used--confirm or set it properly - Version: 1, + Version: fold.Version, AccessControl: acMetadata, ParentUID: fold.ParentUID, }, nil diff --git a/pkg/api/folder_test.go b/pkg/api/folder_test.go index 6a45068ea23..58f4ac2a54a 100644 --- a/pkg/api/folder_test.go +++ b/pkg/api/folder_test.go @@ -5,17 +5,24 @@ import ( "encoding/json" "fmt" "net/http" + "net/http/httptest" "strings" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" + clientrest "k8s.io/client-go/rest" "github.com/grafana/grafana/pkg/api/dtos" + folderv0alpha1 "github.com/grafana/grafana/pkg/apis/folder/v0alpha1" + grafanarest "github.com/grafana/grafana/pkg/apiserver/rest" + conversions "github.com/grafana/grafana/pkg/registry/apis/folders" "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/accesscontrol/actest" acmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock" + contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/folder" @@ -24,6 +31,7 @@ import ( "github.com/grafana/grafana/pkg/services/quota/quotatest" "github.com/grafana/grafana/pkg/services/search/model" "github.com/grafana/grafana/pkg/services/user" + "github.com/grafana/grafana/pkg/services/user/usertest" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/web/webtest" ) @@ -510,3 +518,232 @@ func TestFolderGetAPIEndpoint(t *testing.T) { }) } } + +type mockClientConfigProvider struct { + host string +} + +func (m mockClientConfigProvider) GetDirectRestConfig(c *contextmodel.ReqContext) *clientrest.Config { + return &clientrest.Config{ + Host: m.host, + } +} + +func (m mockClientConfigProvider) DirectlyServeHTTP(w http.ResponseWriter, r *http.Request) {} + +func TestUpdateFolderLegacyAndUnifiedStorage(t *testing.T) { + testuser := &user.User{ID: 99, UID: "fdxsqt7t5ryf4a", Login: "testuser"} + + legacyFolder := folder.Folder{ + UID: "ady4yobv315a8e", + Title: "Example folder 226", + URL: "/dashboards/f/ady4yobv315a8e/example-folder-226", + CreatedBy: 99, + CreatedByUID: "fdxsqt7t5ryf4a", + Created: time.Date(2024, time.November, 29, 0, 42, 34, 0, time.UTC), + UpdatedBy: 99, + UpdatedByUID: "fdxsqt7t5ryf4a", + Updated: time.Date(2024, time.November, 29, 0, 42, 34, 0, time.UTC), + Version: 3, + } + + namespacer := func(_ int64) string { return "1" } + unifiedStorageFolder, err := conversions.LegacyFolderToUnstructured(&legacyFolder, namespacer) + require.NoError(t, err) + + expectedFolder := dtos.Folder{ + UID: legacyFolder.UID, + OrgID: 0, + Title: legacyFolder.Title, + URL: legacyFolder.URL, + HasACL: false, + CanSave: false, + CanEdit: true, + CanAdmin: false, + CanDelete: false, + CreatedBy: "testuser", + Created: legacyFolder.Created, + UpdatedBy: "testuser", + Updated: legacyFolder.Updated, + Version: legacyFolder.Version, + } + + mux := http.NewServeMux() + mux.HandleFunc("PUT /apis/folder.grafana.app/v0alpha1/namespaces/default/folders/ady4yobv315a8e", func(w http.ResponseWriter, req *http.Request) { + err := json.NewEncoder(w).Encode(unifiedStorageFolder) + require.NoError(t, err) + }) + + folderApiServerMock := httptest.NewServer(mux) + defer folderApiServerMock.Close() + + t.Run("happy path", func(t *testing.T) { + type testCase struct { + description string + folderUID string + legacyFolder folder.Folder + expectedFolder dtos.Folder + expectedFolderServiceError error + unifiedStorageEnabled bool + unifiedStorageMode grafanarest.DualWriterMode + expectedCode int + } + + tcs := []testCase{ + { + description: "Happy Path - Legacy", + expectedCode: http.StatusOK, + legacyFolder: legacyFolder, + folderUID: legacyFolder.UID, + expectedFolder: expectedFolder, + unifiedStorageEnabled: false, + }, + { + description: "Happy Path - Unified storage, mode 1", + expectedCode: http.StatusOK, + legacyFolder: legacyFolder, + folderUID: legacyFolder.UID, + expectedFolder: expectedFolder, + unifiedStorageEnabled: true, + unifiedStorageMode: grafanarest.Mode1, + }, + { + description: "Happy Path - Unified storage, mode 2", + expectedCode: http.StatusOK, + legacyFolder: legacyFolder, + folderUID: legacyFolder.UID, + expectedFolder: expectedFolder, + unifiedStorageEnabled: true, + unifiedStorageMode: grafanarest.Mode2, + }, + { + description: "Happy Path - Unified storage, mode 3", + expectedCode: http.StatusOK, + legacyFolder: legacyFolder, + folderUID: legacyFolder.UID, + expectedFolder: expectedFolder, + unifiedStorageEnabled: true, + unifiedStorageMode: grafanarest.Mode3, + }, + { + description: "Happy Path - Unified storage, mode 4", + expectedCode: http.StatusOK, + legacyFolder: legacyFolder, + folderUID: legacyFolder.UID, + expectedFolder: expectedFolder, + unifiedStorageEnabled: true, + unifiedStorageMode: grafanarest.Mode4, + }, + { + description: "Folder Not Found - Legacy", + expectedCode: http.StatusNotFound, + legacyFolder: legacyFolder, + folderUID: "notfound", + expectedFolder: expectedFolder, + unifiedStorageEnabled: false, + expectedFolderServiceError: dashboards.ErrFolderNotFound, + }, + { + description: "Folder Not Found - Unified storage, mode 1", + expectedCode: http.StatusNotFound, + legacyFolder: legacyFolder, + folderUID: "notfound", + expectedFolder: expectedFolder, + unifiedStorageEnabled: true, + unifiedStorageMode: grafanarest.Mode1, + }, + { + description: "Folder Not Found - Unified storage, mode 2", + expectedCode: http.StatusNotFound, + legacyFolder: legacyFolder, + folderUID: "notfound", + expectedFolder: expectedFolder, + unifiedStorageEnabled: true, + unifiedStorageMode: grafanarest.Mode2, + }, + { + description: "Folder Not Found - Unified storage, mode 3", + expectedCode: http.StatusNotFound, + legacyFolder: legacyFolder, + folderUID: "notfound", + expectedFolder: expectedFolder, + unifiedStorageEnabled: true, + unifiedStorageMode: grafanarest.Mode3, + }, + { + description: "Folder Not Found - Unified storage, mode 4", + expectedCode: http.StatusNotFound, + legacyFolder: legacyFolder, + folderUID: "notfound", + expectedFolder: expectedFolder, + unifiedStorageEnabled: true, + unifiedStorageMode: grafanarest.Mode4, + }, + } + + for _, tc := range tcs { + t.Run(tc.description, func(t *testing.T) { + setUpRBACGuardian(t) + + cfg := setting.NewCfg() + cfg.UnifiedStorage = map[string]setting.UnifiedStorageConfig{ + folderv0alpha1.RESOURCEGROUP: { + DualWriterMode: tc.unifiedStorageMode, + }, + } + + featuresArr := []any{featuremgmt.FlagNestedFolders} + if tc.unifiedStorageEnabled { + featuresArr = append(featuresArr, featuremgmt.FlagKubernetesFolders) + } + + server := SetupAPITestServer(t, func(hs *HTTPServer) { + hs.Cfg = cfg + hs.folderService = &foldertest.FakeService{ + ExpectedFolder: &tc.legacyFolder, + ExpectedError: tc.expectedFolderServiceError, + } + hs.QuotaService = quotatest.New(false, nil) + hs.SearchService = &mockSearchService{ + ExpectedResult: model.HitList{}, + } + hs.userService = &usertest.FakeUserService{ + ExpectedUser: testuser, + } + hs.Features = featuremgmt.WithFeatures( + featuresArr..., + ) + hs.clientConfigProvider = mockClientConfigProvider{ + host: folderApiServerMock.URL, + } + }) + + req := server.NewRequest(http.MethodPut, fmt.Sprintf("/api/folders/%s", tc.folderUID), strings.NewReader(`{"title":"new title"}`)) + req.Header.Set("Content-Type", "application/json") + webtest.RequestWithSignedInUser(req, &user.SignedInUser{UserID: 1, OrgID: 1, Permissions: map[int64]map[string][]string{ + 1: accesscontrol.GroupScopesByActionContext(context.Background(), []accesscontrol.Permission{ + {Action: dashboards.ActionFoldersWrite, Scope: dashboards.ScopeFoldersAll}, + {Action: dashboards.ActionFoldersWrite, Scope: dashboards.ScopeFoldersProvider.GetResourceScopeUID("ady4yobv315a8e")}, + }), + }}) + + res, err := server.Send(req) + require.NoError(t, err) + + require.Equal(t, tc.expectedCode, res.StatusCode) + defer func() { require.NoError(t, res.Body.Close()) }() + + if tc.expectedCode == http.StatusOK { + body := dtos.Folder{} + require.NoError(t, json.NewDecoder(res.Body).Decode(&body)) + + //nolint:staticcheck + body.ID = 0 + body.Version = 0 + tc.expectedFolder.Version = 0 + require.Equal(t, tc.expectedFolder, body) + } + }) + } + }) +} diff --git a/pkg/registry/apis/folders/conversions.go b/pkg/registry/apis/folders/conversions.go index 2d0c2520884..e76a51801f4 100644 --- a/pkg/registry/apis/folders/conversions.go +++ b/pkg/registry/apis/folders/conversions.go @@ -89,7 +89,7 @@ func UnstructuredToLegacyFolder(item unstructured.Unstructured, orgID int64) (*f if created != nil { // #TODO Fix this time format. The legacy time format seems to be along the lines of time.Now() // which includes a part that represents a fraction of a second. Format should be "2024-09-12T15:37:41.09466+02:00" - createdTime = created.Local() + createdTime = (*created).UTC() } f := &folder.Folder{ @@ -122,6 +122,10 @@ func UnstructuredToLegacyFolder(item unstructured.Unstructured, orgID int64) (*f // #TODO figure out about adding version, parents, orgID fields } +func LegacyFolderToUnstructured(v *folder.Folder, namespacer request.NamespaceMapper) (*v0alpha1.Folder, error) { + return convertToK8sResource(v, namespacer) +} + func convertToK8sResource(v *folder.Folder, namespacer request.NamespaceMapper) (*v0alpha1.Folder, error) { f := &v0alpha1.Folder{ TypeMeta: v0alpha1.FolderResourceInfo.TypeMeta(), @@ -208,11 +212,8 @@ func getURL(meta utils.GrafanaMetaAccessor, title string) string { } func getCreated(meta utils.GrafanaMetaAccessor) (*time.Time, error) { - created, err := meta.GetRepositoryTimestamp() - if err != nil { - return nil, err - } - return created, nil + created := meta.GetCreationTimestamp().Time + return &created, nil } func GetParentTitles(fullPath string) ([]string, error) { diff --git a/pkg/registry/apis/folders/register.go b/pkg/registry/apis/folders/register.go index 7b8628ef9a1..b1fe170b3d1 100644 --- a/pkg/registry/apis/folders/register.go +++ b/pkg/registry/apis/folders/register.go @@ -3,6 +3,7 @@ package folders import ( "context" "errors" + "fmt" "slices" "github.com/prometheus/client_golang/prometheus" @@ -246,6 +247,15 @@ func (b *FolderAPIBuilder) Validate(ctx context.Context, a admission.Attributes, obj := a.GetObject() + f, ok := obj.(*v0alpha1.Folder) + if !ok { + return fmt.Errorf("obj is not v0alpha1.Folder") + } + + if f.Spec.Title == "" { + return dashboards.ErrFolderTitleEmpty + } + for i := 1; i <= folderValidationRules.maxDepth; i++ { parent := getParent(obj) if parent == "" { diff --git a/pkg/registry/apis/folders/register_test.go b/pkg/registry/apis/folders/register_test.go index 6113a60516d..b3ff66b2393 100644 --- a/pkg/registry/apis/folders/register_test.go +++ b/pkg/registry/apis/folders/register_test.go @@ -17,7 +17,6 @@ import ( "github.com/grafana/grafana/pkg/services/user" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apiserver/pkg/admission" "k8s.io/apiserver/pkg/authorization/authorizer" ) @@ -149,6 +148,42 @@ func TestFolderAPIBuilder_getAuthorizerFunc(t *testing.T) { allow: false, }, }, + { + name: "user with write permissions should be able to update a folder", + input: input{ + user: &user.SignedInUser{ + UserID: 1, + OrgID: orgID, + Name: "123", + Permissions: map[int64]map[string][]string{ + orgID: {dashboards.ActionFoldersWrite: {dashboards.ScopeFoldersAll}}, + }, + }, + verb: string(utils.VerbUpdate), + }, + expect: expect{ + eval: "folders:write", + allow: true, + }, + }, + { + name: "user without write permissions should NOT be able to update a folder", + input: input{ + user: &user.SignedInUser{ + UserID: 1, + OrgID: orgID, + Name: "123", + Permissions: map[int64]map[string][]string{ + orgID: {}, + }, + }, + verb: string(utils.VerbUpdate), + }, + expect: expect{ + eval: "folders:write", + allow: false, + }, + }, } b := &FolderAPIBuilder{ @@ -177,9 +212,19 @@ func TestFolderAPIBuilder_getAuthorizerFunc(t *testing.T) { func TestFolderAPIBuilder_Validate(t *testing.T) { type input struct { - obj *unstructured.Unstructured - name string + obj *v0alpha1.Folder + annotations map[string]string + name string } + + circularObj := &v0alpha1.Folder{ + Spec: v0alpha1.Spec{ + Title: "foo", + }, + } + circularObj.Name = "valid-name" + circularObj.Annotations = map[string]string{"grafana.app/folder": "valid-name"} + tests := []struct { name string input input @@ -189,9 +234,9 @@ func TestFolderAPIBuilder_Validate(t *testing.T) { { name: "should return error when name is invalid", input: input{ - obj: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "meta": map[string]interface{}{"name": folderValidationRules.invalidNames[0]}, + obj: &v0alpha1.Folder{ + Spec: v0alpha1.Spec{ + Title: "foo", }, }, name: folderValidationRules.invalidNames[0], @@ -201,9 +246,9 @@ func TestFolderAPIBuilder_Validate(t *testing.T) { { name: "should return no error if every validation passes", input: input{ - obj: &unstructured.Unstructured{ - Object: map[string]interface{}{ - "meta": map[string]interface{}{"name": "valid-name"}, + obj: &v0alpha1.Folder{ + Spec: v0alpha1.Spec{ + Title: "foo", }, }, name: "valid-name", @@ -212,23 +257,33 @@ func TestFolderAPIBuilder_Validate(t *testing.T) { { name: "should return error when creating a nested folder higher than max depth", input: input{ - obj: &unstructured.Unstructured{ - Object: map[string]any{ - "metadata": map[string]any{"name": "valid-name", "annotations": map[string]any{"grafana.app/folder": "valid-name"}}, + obj: &v0alpha1.Folder{ + Spec: v0alpha1.Spec{ + Title: "foo", }, }, - name: "valid-name", + annotations: map[string]string{"grafana.app/folder": "valid-name"}, + name: "valid-name", }, setupFn: func(m *mock.Mock) { m.On("Get", mock.Anything, "valid-name", mock.Anything).Return( - &unstructured.Unstructured{ - Object: map[string]any{ - "metadata": map[string]any{"name": "valid-name", "annotations": map[string]any{"grafana.app/folder": "valid-name"}}, - }, - }, nil) + circularObj, + nil) }, err: folder.ErrMaximumDepthReached, }, + { + name: "should return error when title is empty", + input: input{ + obj: &v0alpha1.Folder{ + Spec: v0alpha1.Spec{ + Title: "", + }, + }, + name: "foo", + }, + err: dashboards.ErrFolderTitleEmpty, + }, } s := (grafanarest.Storage)(nil) @@ -246,6 +301,9 @@ func TestFolderAPIBuilder_Validate(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + tt.input.obj.Name = tt.input.name + tt.input.obj.Annotations = tt.input.annotations + if tt.setupFn != nil { tt.setupFn(m) } @@ -264,7 +322,9 @@ func TestFolderAPIBuilder_Validate(t *testing.T) { &user.SignedInUser{}, ), nil) - if tt.err != nil { + if tt.err == nil { + require.NoError(t, err) + } else { require.ErrorIs(t, err, tt.err) return }