K8s Dashboards: Fix creating a dashboard inside a folder (#98982)

pkg/api/dashboard.go

@@ -25,6 +25,7 @@ import (
 	"github.com/grafana/grafana/pkg/services/dashboards"
 	dashver "github.com/grafana/grafana/pkg/services/dashboardversion"
 	"github.com/grafana/grafana/pkg/services/featuremgmt"
+	"github.com/grafana/grafana/pkg/services/folder"
 	"github.com/grafana/grafana/pkg/services/guardian"
 	"github.com/grafana/grafana/pkg/services/org"
 	pref "github.com/grafana/grafana/pkg/services/preference"
@@ -84,6 +85,8 @@ func dashboardGuardianResponse(err error) response.Response {
 // 403: forbiddenError
 // 404: notFoundError
 // 500: internalServerError
+//
+//nolint:gocyclo
 func (hs *HTTPServer) GetDashboard(c *contextmodel.ReqContext) response.Response {
 	ctx, span := tracer.Start(c.Req.Context(), "api.GetDashboard")
 	defer span.End()
@@ -187,11 +190,23 @@ func (hs *HTTPServer) GetDashboard(c *contextmodel.ReqContext) response.Response
 		PublicDashboardEnabled: publicDashboardEnabled,
 	}
 	metrics.MFolderIDsAPICount.WithLabelValues(metrics.GetDashboard).Inc()
-	// lookup folder title
+	// lookup folder title & url
-	// nolint:staticcheck
+	if dash.FolderUID != "" && hs.Features.IsEnabledGlobally(featuremgmt.FlagKubernetesFoldersServiceV2) {
-	if dash.FolderID > 0 {
+		queryResult, err := hs.folderService.Get(ctx, &folder.GetFolderQuery{
-		// nolint:staticcheck
+			OrgID:        c.SignedInUser.GetOrgID(),
-		query := dashboards.GetDashboardQuery{ID: dash.FolderID, OrgID: c.SignedInUser.GetOrgID()}
+			UID:          &dash.FolderUID,
+			SignedInUser: c.SignedInUser,
+		})
+		if errors.Is(err, dashboards.ErrFolderNotFound) {
+			return response.Error(http.StatusNotFound, "Folder not found", err)
+		}
+		meta.FolderUid = queryResult.UID
+		meta.FolderTitle = queryResult.Title
+		meta.FolderId = queryResult.ID // nolint:staticcheck
+		queryResult = queryResult.WithURL()
+		meta.FolderUrl = queryResult.URL
+	} else if dash.FolderID > 0 { // nolint:staticcheck
+		query := dashboards.GetDashboardQuery{ID: dash.FolderID, OrgID: c.SignedInUser.GetOrgID()} // nolint:staticcheck
 		metrics.MFolderIDsAPICount.WithLabelValues(metrics.GetDashboard).Inc()
 		queryResult, err := hs.DashboardService.GetDashboard(ctx, &query)
 		if err != nil {

pkg/services/dashboards/service/dashboard_service.go

@@ -369,16 +369,28 @@ func (dr *DashboardServiceImpl) BuildSaveDashboardCommand(ctx context.Context, d
 	}
 
 	// Validate folder
-	if dash.FolderUID != "" {
+	if dr.features.IsEnabledGlobally(featuremgmt.FlagKubernetesFoldersServiceV2) {
-		if !dr.features.IsEnabledGlobally(featuremgmt.FlagKubernetesFoldersServiceV2) {
+		folder, err := dr.folderService.Get(ctx, &folder.GetFolderQuery{
-			folder, err := dr.folderStore.GetFolderByUID(ctx, dash.OrgID, dash.FolderUID)
+			OrgID:        dash.OrgID,
-			if err != nil {
+			UID:          &dash.FolderUID,
-				return nil, err
+			ID:           &dash.FolderID, // nolint:staticcheck
-			}
+			SignedInUser: dto.User,
-			metrics.MFolderIDsServiceCount.WithLabelValues(metrics.Dashboard).Inc()
+		})
-			// nolint:staticcheck
+		if err != nil {
-			dash.FolderID = folder.ID
+			return nil, err
 		}
+		metrics.MFolderIDsServiceCount.WithLabelValues(metrics.Dashboard).Inc()
+		// nolint:staticcheck
+		dash.FolderID = folder.ID
+		dash.FolderUID = folder.UID
+	} else if dash.FolderUID != "" {
+		folder, err := dr.folderStore.GetFolderByUID(ctx, dash.OrgID, dash.FolderUID)
+		if err != nil {
+			return nil, err
+		}
+		metrics.MFolderIDsServiceCount.WithLabelValues(metrics.Dashboard).Inc()
+		// nolint:staticcheck
+		dash.FolderID = folder.ID
 	} else if dash.FolderID != 0 { // nolint:staticcheck
 		metrics.MFolderIDsServiceCount.WithLabelValues(metrics.Dashboard).Inc()
 		// nolint:staticcheck
@@ -2171,6 +2183,15 @@ func LegacySaveCommandToUnstructured(cmd *dashboards.SaveDashboardCommand, names
 	finalObj.SetNamespace(namespace)
 	finalObj.SetGroupVersionKind(v0alpha1.DashboardResourceInfo.GroupVersionKind())
 
+	if cmd.FolderUID != "" {
+		meta, err := utils.MetaAccessor(&finalObj)
+		if err != nil {
+			return finalObj, err
+		}
+
+		meta.SetFolder(cmd.FolderUID)
+	}
+
 	return finalObj, nil
 }
 

pkg/services/dashboards/service/dashboard_service_integration_test.go

@@ -9,8 +9,10 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/grafana/grafana/pkg/apimachinery/identity"
+	"github.com/grafana/grafana/pkg/bus"
 	"github.com/grafana/grafana/pkg/components/simplejson"
 	"github.com/grafana/grafana/pkg/infra/db"
+	"github.com/grafana/grafana/pkg/infra/tracing"
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
 	"github.com/grafana/grafana/pkg/services/accesscontrol/actest"
 	accesscontrolmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock"
@@ -19,10 +21,10 @@ import (
 	"github.com/grafana/grafana/pkg/services/featuremgmt"
 	"github.com/grafana/grafana/pkg/services/folder"
 	"github.com/grafana/grafana/pkg/services/folder/folderimpl"
-	"github.com/grafana/grafana/pkg/services/folder/foldertest"
 	"github.com/grafana/grafana/pkg/services/guardian"
 	"github.com/grafana/grafana/pkg/services/org"
 	"github.com/grafana/grafana/pkg/services/quota/quotatest"
+	"github.com/grafana/grafana/pkg/services/supportbundles/supportbundlestest"
 	"github.com/grafana/grafana/pkg/services/tag/tagimpl"
 	"github.com/grafana/grafana/pkg/services/user"
 	"github.com/grafana/grafana/pkg/setting"
@@ -860,6 +862,7 @@ func permissionScenario(t *testing.T, desc string, canSave bool, fn permissionSc
 
 	guardianMock := &guardian.FakeDashboardGuardian{
 		CanSaveValue: canSave,
+		CanViewValue: true,
 	}
 
 	t.Run(desc, func(t *testing.T) {
@@ -873,6 +876,9 @@ func permissionScenario(t *testing.T, desc string, canSave bool, fn permissionSc
 		folderStore := folderimpl.ProvideDashboardFolderStore(sqlStore)
 		folderPermissions := accesscontrolmock.NewMockedPermissionsService()
 		folderPermissions.On("SetPermissions", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return([]accesscontrol.ResourcePermission{}, nil)
+		tracer := tracing.InitializeTracerForTest()
+		folderStore2 := folderimpl.ProvideStore(sqlStore)
+		folderService := folderimpl.ProvideService(folderStore2, actest.FakeAccessControl{ExpectedEvaluate: true}, bus.ProvideBus(tracer), dashboardStore, folderStore, sqlStore, features, supportbundlestest.NewFakeBundleService(), cfg, nil, tracer)
 		dashboardPermissions := accesscontrolmock.NewMockedPermissionsService()
 		dashboardService, err := ProvideDashboardServiceImpl(
 			cfg, dashboardStore, folderStore,
@@ -880,7 +886,7 @@ func permissionScenario(t *testing.T, desc string, canSave bool, fn permissionSc
 			folderPermissions,
 			dashboardPermissions,
 			ac,
-			foldertest.NewFakeService(),
+			folderService,
 			folder.NewFakeStore(),
 			nil,
 			nil,
@@ -942,7 +948,9 @@ func callSaveWithResult(t *testing.T, cmd dashboards.SaveDashboardCommand, sqlSt
 	folderStore := folderimpl.ProvideDashboardFolderStore(sqlStore)
 	folderPermissions := accesscontrolmock.NewMockedPermissionsService()
 	folderPermissions.On("SetPermissions", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return([]accesscontrol.ResourcePermission{}, nil)
-
+	tracer := tracing.InitializeTracerForTest()
+	folderStore2 := folderimpl.ProvideStore(sqlStore)
+	folderService := folderimpl.ProvideService(folderStore2, actest.FakeAccessControl{ExpectedEvaluate: true}, bus.ProvideBus(tracer), dashboardStore, folderStore, sqlStore, features, supportbundlestest.NewFakeBundleService(), cfg, nil, tracer)
 	dashboardPermissions := accesscontrolmock.NewMockedPermissionsService()
 	dashboardPermissions.On("SetPermissions", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return([]accesscontrol.ResourcePermission{}, nil)
 	service, err := ProvideDashboardServiceImpl(
@@ -951,7 +959,7 @@ func callSaveWithResult(t *testing.T, cmd dashboards.SaveDashboardCommand, sqlSt
 		folderPermissions,
 		dashboardPermissions,
 		actest.FakeAccessControl{},
-		foldertest.NewFakeService(),
+		folderService,
 		folder.NewFakeStore(),
 		nil,
 		nil,
@@ -975,13 +983,16 @@ func callSaveWithError(t *testing.T, cmd dashboards.SaveDashboardCommand, sqlSto
 	dashboardStore, err := database.ProvideDashboardStore(sqlStore, cfg, features, tagimpl.ProvideService(sqlStore))
 	require.NoError(t, err)
 	folderStore := folderimpl.ProvideDashboardFolderStore(sqlStore)
+	tracer := tracing.InitializeTracerForTest()
+	folderStore2 := folderimpl.ProvideStore(sqlStore)
+	folderService := folderimpl.ProvideService(folderStore2, actest.FakeAccessControl{ExpectedEvaluate: true}, bus.ProvideBus(tracer), dashboardStore, folderStore, sqlStore, features, supportbundlestest.NewFakeBundleService(), cfg, nil, tracer)
 	service, err := ProvideDashboardServiceImpl(
 		cfg, dashboardStore, folderStore,
 		featuremgmt.WithFeatures(),
 		accesscontrolmock.NewMockedPermissionsService(),
 		accesscontrolmock.NewMockedPermissionsService(),
 		actest.FakeAccessControl{},
-		foldertest.NewFakeService(),
+		folderService,
 		folder.NewFakeStore(),
 		nil,
 		nil,
@@ -1024,13 +1035,16 @@ func saveTestDashboard(t *testing.T, title string, orgID int64, folderUID string
 	folderStore := folderimpl.ProvideDashboardFolderStore(sqlStore)
 	dashboardPermissions := accesscontrolmock.NewMockedPermissionsService()
 	dashboardPermissions.On("SetPermissions", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return([]accesscontrol.ResourcePermission{}, nil)
+	tracer := tracing.InitializeTracerForTest()
+	folderStore2 := folderimpl.ProvideStore(sqlStore)
+	folderService := folderimpl.ProvideService(folderStore2, actest.FakeAccessControl{ExpectedEvaluate: true}, bus.ProvideBus(tracer), dashboardStore, folderStore, sqlStore, features, supportbundlestest.NewFakeBundleService(), cfg, nil, tracer)
 	service, err := ProvideDashboardServiceImpl(
 		cfg, dashboardStore, folderStore,
 		features,
 		accesscontrolmock.NewMockedPermissionsService(),
 		dashboardPermissions,
 		actest.FakeAccessControl{},
-		foldertest.NewFakeService(),
+		folderService,
 		folder.NewFakeStore(),
 		nil,
 		nil,
@@ -1079,6 +1093,9 @@ func saveTestFolder(t *testing.T, title string, orgID int64, sqlStore db.DB) *da
 	require.NoError(t, err)
 	folderStore := folderimpl.ProvideDashboardFolderStore(sqlStore)
 	folderPermissions := accesscontrolmock.NewMockedPermissionsService()
+	tracer := tracing.InitializeTracerForTest()
+	folderStore2 := folderimpl.ProvideStore(sqlStore)
+	folderService := folderimpl.ProvideService(folderStore2, actest.FakeAccessControl{ExpectedEvaluate: true}, bus.ProvideBus(tracer), dashboardStore, folderStore, sqlStore, features, supportbundlestest.NewFakeBundleService(), cfg, nil, tracer)
 	folderPermissions.On("SetPermissions", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return([]accesscontrol.ResourcePermission{}, nil)
 	service, err := ProvideDashboardServiceImpl(
 		cfg, dashboardStore, folderStore,
@@ -1086,7 +1103,7 @@ func saveTestFolder(t *testing.T, title string, orgID int64, sqlStore db.DB) *da
 		folderPermissions,
 		accesscontrolmock.NewMockedPermissionsService(),
 		actest.FakeAccessControl{},
-		foldertest.NewFakeService(),
+		folderService,
 		folder.NewFakeStore(),
 		nil,
 		nil,

pkg/services/dashboards/service/dashboard_service_test.go

@@ -40,7 +40,6 @@ func TestDashboardService(t *testing.T) {
 		defer fakeStore.AssertExpectations(t)
 
 		folderSvc := foldertest.NewFakeService()
-
 		service := &DashboardServiceImpl{
 			cfg:            setting.NewCfg(),
 			log:            log.New("test.logger"),
@@ -48,6 +47,9 @@ func TestDashboardService(t *testing.T) {
 			folderService:  folderSvc,
 			features:       featuremgmt.WithFeatures(),
 		}
+		folderStore := foldertest.FakeFolderStore{}
+		folderStore.On("GetFolderByUID", mock.Anything, mock.AnythingOfType("int64"), mock.AnythingOfType("string")).Return(nil, dashboards.ErrFolderNotFound).Once()
+		service.folderStore = &folderStore
 
 		origNewDashboardGuardian := guardian.New
 		defer func() { guardian.New = origNewDashboardGuardian }()
@@ -102,9 +104,8 @@ func TestDashboardService(t *testing.T) {
 			t.Run("Should return validation error if a folder that is specified can't be found", func(t *testing.T) {
 				dto.Dashboard = dashboards.NewDashboard("Dash")
 				dto.Dashboard.FolderUID = "non-existing-folder"
-				folderStore := foldertest.FakeFolderStore{}
+				folderSvc := foldertest.FakeService{ExpectedError: dashboards.ErrFolderNotFound}
-				folderStore.On("GetFolderByUID", mock.Anything, mock.AnythingOfType("int64"), mock.AnythingOfType("string")).Return(nil, dashboards.ErrFolderNotFound).Once()
+				service.folderService = &folderSvc
-				service.folderStore = &folderStore
 				_, err := service.SaveDashboard(context.Background(), dto, false)
 				require.Equal(t, err, dashboards.ErrFolderNotFound)
 			})
@@ -1835,6 +1836,7 @@ func TestLegacySaveCommandToUnstructured(t *testing.T) {
 	namespace := "test-namespace"
 	t.Run("successfully converts save command to unstructured", func(t *testing.T) {
 		cmd := &dashboards.SaveDashboardCommand{
+			FolderUID: "folder-uid",
 			Dashboard: simplejson.NewFromAny(map[string]any{"test": "test", "title": "testing slugify", "uid": "test-uid"}),
 		}
 
@@ -1845,6 +1847,7 @@ func TestLegacySaveCommandToUnstructured(t *testing.T) {
 		assert.Equal(t, "test-namespace", result.GetNamespace())
 		spec := result.Object["spec"].(map[string]any)
 		assert.Equal(t, spec["version"], 1)
+		assert.Equal(t, result.GetAnnotations(), map[string]string{utils.AnnoKeyFolder: "folder-uid"})
 	})
 
 	t.Run("should increase version when called", func(t *testing.T) {
@@ -1856,6 +1859,8 @@ func TestLegacySaveCommandToUnstructured(t *testing.T) {
 		assert.NotNil(t, result)
 		spec := result.Object["spec"].(map[string]any)
 		assert.Equal(t, spec["version"], float64(2))
+		// folder annotation should not be set if not inside a folder
+		assert.Equal(t, result.GetAnnotations(), map[string]string(nil))
 	})
 }
 

pkg/services/libraryelements/libraryelements_test.go

@@ -303,10 +303,14 @@ func createDashboard(t *testing.T, sqlStore db.DB, user user.SignedInUser, dash
 	dashboardPermissions := acmock.NewMockedPermissionsService()
 	dashboardPermissions.On("SetPermissions", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return([]accesscontrol.ResourcePermission{}, nil)
 	folderStore := folderimpl.ProvideDashboardFolderStore(sqlStore)
+	var expectedFolder *folder.Folder
+	if dash.FolderUID != "" || dash.FolderID != 0 { // nolint:staticcheck
+		expectedFolder = &folder.Folder{ID: folderID, UID: folderUID}
+	}
 	service, err := dashboardservice.ProvideDashboardServiceImpl(
 		cfg, dashboardStore, folderStore,
 		features, folderPermissions, dashboardPermissions, ac,
-		foldertest.NewFakeService(),
+		&foldertest.FakeService{ExpectedFolder: expectedFolder},
 		folder.NewFakeStore(),
 		nil,
 		nil,