K8s/Folders: Improve k8s client implementation of get (#96434)

* Enable getting folders with kubernetes client
* Add TestIntegrationFolderGetPermissions
* Set full path as part of legacy get
* Replace implementation for setting fullpath
* Add folder get test
* Escape forward slash in parent titles
* Replace test for access control metadata
* Add test case to TestIntegrationFolderGetPermissions
* Improve fetching of access control

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.Get("/", authorize(accesscontrol.EvalPermission(dashboards.ActionFoldersRead, uidScope)), routing.Wrap(hs.GetFolderByUID))
 			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))
@@ -66,19 +65,20 @@ func (hs *HTTPServer) registerFolderAPI(apiRoute routing.RouteRegister, authoriz
 			folderRoute.Get("/", handler.getFolders)
 			folderRoute.Group("/:uid", func(folderUidRoute routing.RouteRegister) {
 				folderUidRoute.Delete("/", handler.deleteFolder)
+				folderUidRoute.Get("/", handler.getFolder)
 			})
 		} else {
 			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.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.Get("/", handler.getFolder)
 				folderUidRoute.Put("/:uid", handler.updateFolder)
 			})
 		}
@@ -994,19 +994,9 @@ func (fk8s *folderK8sHandler) getFolderACMetadata(c *contextmodel.ReqContext, f
 		return nil, nil
 	}
 
-	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 _, uid := range parentsFullPathUIDs[1:] {
-		folderIDs[uid] = true
+	folderIDs, err := fk8s.getParents(f)
+	if err != nil {
+		return nil, err
 	}
 
 	allMetadata := getMultiAccessControlMetadata(c, dashboards.ScopeFoldersPrefix, folderIDs)
@@ -1019,3 +1009,22 @@ func (fk8s *folderK8sHandler) getFolderACMetadata(c *contextmodel.ReqContext, f
 	}
 	return metadata, nil
 }
+
+func (fk8s *folderK8sHandler) getParents(f *folder.Folder) (map[string]bool, error) {
+	folderIDs := map[string]bool{f.UID: true}
+	if (f.UID == accesscontrol.GeneralFolderUID) || (f.UID == folder.SharedWithMeFolderUID) {
+		return folderIDs, 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 folderIDs, nil
+	}
+
+	for _, uid := range parentsFullPathUIDs[1:] {
+		folderIDs[uid] = true
+	}
+
+	return folderIDs, nil
+}

pkg/api/folder_test.go

@@ -5,20 +5,17 @@ import (
 	"encoding/json"
 	"fmt"
 	"net/http"
-	"net/http/httptest"
 	"strings"
 	"testing"
 
 	"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"
 	"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"
@@ -513,105 +510,3 @@ 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 TestHTTPServer_FolderMetadataK8s(t *testing.T) {
-	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		//nolint:errcheck
-		fmt.Fprintln(w,
-			`{
-				"kind": "Folder",
-				"apiVersion": "folder.grafana.app/v0alpha1",
-				"metadata": {
-					"name": "ady4yobv315a8e",
-					"namespace": "default",
-					"uid": "28f306ee-ada1-40f4-8011-b2d1df462aad",
-					"creationTimestamp": "2024-09-17T04:16:35Z",
-					"annotations": {
-						"grafana.app/createdBy": "user:fdxsqt7t5ryf4a",
-						"grafana.app/repoName": "SQL",
-						"grafana.app/repoPath": "3"
-					}
-				},
-				"spec": {
-					"title": "Example folder 226"
-				}
-			}`)
-	}))
-	defer ts.Close()
-
-	mockClientConfigProvider := mockClientConfigProvider{
-		host: ts.URL,
-	}
-
-	setUpRBACGuardian(t)
-	folderService := &foldertest.FakeService{}
-	features := featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders, featuremgmt.FlagKubernetesFolders)
-	server := SetupAPITestServer(t, func(hs *HTTPServer) {
-		hs.Cfg = setting.NewCfg()
-		hs.folderService = folderService
-		hs.QuotaService = quotatest.New(false, nil)
-		hs.SearchService = &mockSearchService{
-			ExpectedResult: model.HitList{},
-		}
-		hs.Features = features
-		hs.clientConfigProvider = mockClientConfigProvider
-	})
-
-	t.Run("Should attach access control metadata to folder response", func(t *testing.T) {
-		folderService.ExpectedFolder = &folder.Folder{UID: "ady4yobv315a8e"}
-
-		req := server.NewGetRequest("/api/folders/ady4yobv315a8e?accesscontrol=true")
-		webtest.RequestWithSignedInUser(req, &user.SignedInUser{UserID: 1, OrgID: 1, Permissions: map[int64]map[string][]string{
-			1: accesscontrol.GroupScopesByActionContext(context.Background(), []accesscontrol.Permission{
-				{Action: dashboards.ActionFoldersRead, Scope: dashboards.ScopeFoldersAll},
-				{Action: dashboards.ActionFoldersWrite, Scope: dashboards.ScopeFoldersProvider.GetResourceScopeUID("ady4yobv315a8e")},
-			}),
-		}})
-
-		res, err := server.Send(req)
-		require.NoError(t, err)
-		assert.Equal(t, http.StatusOK, res.StatusCode)
-		defer func() { require.NoError(t, res.Body.Close()) }()
-
-		body := dtos.Folder{}
-		require.NoError(t, json.NewDecoder(res.Body).Decode(&body))
-
-		assert.True(t, body.AccessControl[dashboards.ActionFoldersRead])
-		assert.True(t, body.AccessControl[dashboards.ActionFoldersWrite])
-	})
-
-	t.Run("Should not attach access control metadata to folder response", func(t *testing.T) {
-		folderService.ExpectedFolder = &folder.Folder{UID: "ady4yobv315a8e"}
-
-		req := server.NewGetRequest("/api/folders/ady4yobv315a8e")
-		webtest.RequestWithSignedInUser(req, &user.SignedInUser{UserID: 1, OrgID: 1, Permissions: map[int64]map[string][]string{
-			1: accesscontrol.GroupScopesByActionContext(context.Background(), []accesscontrol.Permission{
-				{Action: dashboards.ActionFoldersRead, Scope: dashboards.ScopeFoldersAll},
-				{Action: dashboards.ActionFoldersWrite, Scope: dashboards.ScopeFoldersProvider.GetResourceScopeUID("ady4yobv315a8e")},
-			}),
-		}})
-
-		res, err := server.Send(req)
-		require.NoError(t, err)
-		assert.Equal(t, http.StatusOK, res.StatusCode)
-		defer func() { require.NoError(t, res.Body.Close()) }()
-
-		body := dtos.Folder{}
-		require.NoError(t, json.NewDecoder(res.Body).Decode(&body))
-
-		assert.False(t, body.AccessControl[dashboards.ActionFoldersRead])
-		assert.False(t, body.AccessControl[dashboards.ActionFoldersWrite])
-	})
-}

pkg/services/folder/folderimpl/folder.go

@@ -286,9 +286,47 @@ func (s *Service) Get(ctx context.Context, q *folder.GetFolderQuery) (*folder.Fo
 		f.FullpathUIDs = f.UID // set full path to the folder UID
 	}
 
+	if s.features.IsEnabled(ctx, featuremgmt.FlagKubernetesFolders) {
+		f, err = s.setFullpath(ctx, f, q.SignedInUser)
+	}
+
 	return f, err
 }
 
+func (s *Service) setFullpath(ctx context.Context, f *folder.Folder, user identity.Requester) (*folder.Folder, error) {
+	// #TODO is some kind of intermediate conversion required as is the case with user id where
+	// it gets parsed using UserIdentifier(). Also is there some kind of validation taking place as
+	// part of the parsing?
+	f.CreatedByUID = user.GetUID()
+	f.UpdatedByUID = user.GetUID()
+
+	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
+	parents, err := s.GetParents(ctx, folder.GetParentsQuery{
+		UID:   f.UID,
+		OrgID: f.OrgID,
+	})
+	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
+	escapedSlash := "\\/"
+	title := strings.Replace(f.Title, "/", escapedSlash, -1)
+	f.Fullpath = title
+	f.FullpathUIDs = f.UID
+	for _, p := range parents {
+		pt := strings.Replace(p.Title, "/", escapedSlash, -1)
+		f.Fullpath = f.Fullpath + "/" + pt
+		f.FullpathUIDs = f.FullpathUIDs + "/" + p.UID
+	}
+	return f, nil
+}
+
 func (s *Service) GetChildren(ctx context.Context, q *folder.GetChildrenQuery) ([]*folder.Folder, error) {
 	defer func(t time.Time) {
 		parent := q.UID
@@ -673,33 +711,7 @@ func (s *Service) Create(ctx context.Context, cmd *folder.CreateFolderCommand) (
 	}
 
 	if s.features.IsEnabled(ctx, featuremgmt.FlagKubernetesFolders) {
-		// #TODO is some kind of intermediate conversion required as is the case with user id where
-		// it gets parsed using UserIdentifier(). Also is there some kind of validation taking place as
-		// part of the parsing?
-		f.CreatedByUID = user.GetUID()
-		f.UpdatedByUID = user.GetUID()
-
-		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
+		f, err = s.setFullpath(ctx, f, user)
 	}
 
 	return f, nil

pkg/tests/apis/folder/folders_test.go

@@ -16,6 +16,7 @@ import (
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 
+	"github.com/grafana/grafana/pkg/api"
 	"github.com/grafana/grafana/pkg/api/dtos"
 	folderv0alpha1 "github.com/grafana/grafana/pkg/apis/folder/v0alpha1"
 	grafanarest "github.com/grafana/grafana/pkg/apiserver/rest"
@@ -767,6 +768,156 @@ func TestIntegrationFolderCreatePermissions(t *testing.T) {
 	}
 }
 
+func TestIntegrationFolderGetPermissions(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
+
+	type testCase struct {
+		description          string
+		permissions          []resourcepermissions.SetResourcePermissionCommand
+		expectedCode         int
+		expectedParentUIDs   []string
+		expectedParentTitles []string
+		checkAccessControl   bool
+	}
+	tcs := []testCase{
+		{
+			description:          "get folder by UID should return parent folders if nested folder are enabled",
+			expectedCode:         http.StatusOK,
+			expectedParentUIDs:   []string{"parentuid"},
+			expectedParentTitles: []string{"testparent"},
+			permissions: []resourcepermissions.SetResourcePermissionCommand{
+				{
+					Actions:           []string{dashboards.ActionFoldersRead},
+					Resource:          "folders",
+					ResourceAttribute: "uid",
+					ResourceID:        "*",
+				},
+			},
+			checkAccessControl: true,
+		},
+		{
+			description:          "get folder by UID should return parent folders redacted if nested folder are enabled and user does not have read access to parent folders",
+			expectedCode:         http.StatusOK,
+			expectedParentUIDs:   []string{api.REDACTED},
+			expectedParentTitles: []string{api.REDACTED},
+			permissions: []resourcepermissions.SetResourcePermissionCommand{
+				{
+					Actions:           []string{dashboards.ActionFoldersRead},
+					Resource:          "folders",
+					ResourceAttribute: "uid",
+					ResourceID:        "descuid",
+				},
+			},
+		},
+		{
+			description:          "get folder by UID should not succeed if user doesn't have permissions for the folder",
+			expectedCode:         http.StatusForbidden,
+			expectedParentUIDs:   []string{},
+			expectedParentTitles: []string{},
+			permissions:          []resourcepermissions.SetResourcePermissionCommand{},
+		},
+	}
+
+	for _, tc := range tcs {
+		t.Run(tc.description, func(t *testing.T) {
+			helper := 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,
+				},
+			})
+
+			// Create parent folder
+			parentPayload := `{
+				"title": "testparent",
+				"uid": "parentuid"
+				}`
+			parentCreate := apis.DoRequest(helper, apis.RequestParams{
+				User:   helper.Org1.Admin,
+				Method: http.MethodPost,
+				Path:   "/api/folders",
+				Body:   []byte(parentPayload),
+			}, &folder.Folder{})
+			require.NotNil(t, parentCreate.Result)
+			parentUID := parentCreate.Result.UID
+			require.NotEmpty(t, parentUID)
+
+			// Create descendant folder
+			payload := "{ \"uid\": \"descuid\", \"title\": \"Folder\", \"parentUid\": \"parentuid\"}"
+			resp := apis.DoRequest(helper, apis.RequestParams{
+				User:   helper.Org1.Admin,
+				Method: http.MethodPost,
+				Path:   "/api/folders",
+				Body:   []byte(payload),
+			}, &dtos.Folder{})
+			require.Equal(t, http.StatusOK, resp.Response.StatusCode)
+
+			user := helper.CreateUser("user", apis.Org1, org.RoleNone, tc.permissions)
+
+			// Get with accesscontrol disabled
+			getResp := apis.DoRequest(helper, apis.RequestParams{
+				User:   user,
+				Method: http.MethodGet,
+				Path:   "/api/folders/descuid",
+			}, &dtos.Folder{})
+			require.Equal(t, tc.expectedCode, getResp.Response.StatusCode)
+			require.NotNil(t, getResp.Result)
+
+			require.False(t, getResp.Result.AccessControl[dashboards.ActionFoldersRead])
+			require.False(t, getResp.Result.AccessControl[dashboards.ActionFoldersWrite])
+
+			parents := getResp.Result.Parents
+			require.Equal(t, len(tc.expectedParentUIDs), len(parents))
+			require.Equal(t, len(tc.expectedParentTitles), len(parents))
+			for i := 0; i < len(tc.expectedParentUIDs); i++ {
+				require.Equal(t, tc.expectedParentUIDs[i], parents[i].UID)
+				require.Equal(t, tc.expectedParentTitles[i], parents[i].Title)
+			}
+
+			// Get with accesscontrol enabled
+			if tc.checkAccessControl {
+				acPerms := []resourcepermissions.SetResourcePermissionCommand{
+					{
+						Actions:           []string{dashboards.ActionFoldersRead},
+						Resource:          "folders",
+						ResourceAttribute: "uid",
+						ResourceID:        "*",
+					},
+					{
+						Actions:           []string{dashboards.ActionFoldersWrite},
+						Resource:          "folders",
+						ResourceAttribute: "uid",
+						ResourceID:        "parentuid",
+					},
+				}
+				acUser := helper.CreateUser("acuser", apis.Org1, org.RoleNone, acPerms)
+
+				getWithAC := apis.DoRequest(helper, apis.RequestParams{
+					User:   acUser,
+					Method: http.MethodGet,
+					Path:   "/api/folders/descuid?accesscontrol=true",
+				}, &dtos.Folder{})
+				require.Equal(t, tc.expectedCode, getWithAC.Response.StatusCode)
+				require.NotNil(t, getWithAC.Result)
+
+				require.True(t, getWithAC.Result.AccessControl[dashboards.ActionFoldersRead])
+				require.True(t, getWithAC.Result.AccessControl[dashboards.ActionFoldersWrite])
+			}
+		})
+	}
+}
+
 // TestFoldersCreateAPIEndpointK8S is the counterpart of pkg/api/folder_test.go TestFoldersCreateAPIEndpoint
 func TestFoldersCreateAPIEndpointK8S(t *testing.T) {
 	if testing.Short() {