Unistore: Update Folders Endpoint (#97317)

* Add route for update folders via apiserver

Signed-off-by: Maicon Costa <maiconscosta@gmail.com>

---------

Signed-off-by: Maicon Costa <maiconscosta@gmail.com>

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

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)
+				}
+			})
+		}
+	})
+}

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()
+	created := meta.GetCreationTimestamp().Time
-	if err != nil {
+	return &created, nil
-		return nil, err
-	}
-	return created, nil
 }
 
 func GetParentTitles(fullPath string) ([]string, error) {

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 == "" {

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
+		obj         *v0alpha1.Folder
-		name string
+		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{
+				obj: &v0alpha1.Folder{
-					Object: map[string]interface{}{
+					Spec: v0alpha1.Spec{
-						"meta": map[string]interface{}{"name": folderValidationRules.invalidNames[0]},
+						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{
+				obj: &v0alpha1.Folder{
-					Object: map[string]interface{}{
+					Spec: v0alpha1.Spec{
-						"meta": map[string]interface{}{"name": "valid-name"},
+						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{
+				obj: &v0alpha1.Folder{
-					Object: map[string]any{
+					Spec: v0alpha1.Spec{
-						"metadata": map[string]any{"name": "valid-name", "annotations": map[string]any{"grafana.app/folder": "valid-name"}},
+						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{
+					circularObj,
-						Object: map[string]any{
+					nil)
-							"metadata": map[string]any{"name": "valid-name", "annotations": map[string]any{"grafana.app/folder": "valid-name"}},
-						},
-					}, 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
 			}