Nested folders: Remove feature flag (#109212)

2025-08-06 10:07:23 +03:00
parent fc13a1ae0c
commit 6b86277ecf
45 changed files with 511 additions and 1101 deletions
@@ -220,36 +220,23 @@ func (f *accessControlDashboardPermissionFilter) buildClauses(dialect migrator.D
 			}
 			permSelector.WriteRune(')')

-			switch f.features.IsEnabledGlobally(featuremgmt.FlagNestedFolders) {
-			case true:
-				if len(permSelectorArgs) > 0 {
-					switch f.recursiveQueriesAreSupported {
-					case true:
-						builder.WriteString("(dashboard.folder_id IN (SELECT d.id FROM dashboard as d ")
-						recQueryName := fmt.Sprintf("RecQry%d", len(f.recQueries))
-						f.addRecQry(recQueryName, permSelector.String(), permSelectorArgs, orgID)
-						builder.WriteString(fmt.Sprintf("WHERE d.org_id = ? AND d.uid IN (SELECT uid FROM %s)", recQueryName))
-						args = append(args, orgID)
-					default:
-						nestedFoldersSelectors, nestedFoldersArgs := f.nestedFoldersSelectors(permSelector.String(), permSelectorArgs, "dashboard", "folder_id", "d.id", orgID)
-						builder.WriteRune('(')
-						builder.WriteString(nestedFoldersSelectors)
-						args = append(args, nestedFoldersArgs...)
-					}
-				} else {
+			if len(permSelectorArgs) > 0 {
+				switch f.recursiveQueriesAreSupported {
+				case true:
 					builder.WriteString("(dashboard.folder_id IN (SELECT d.id FROM dashboard as d ")
-					builder.WriteString("WHERE 1 = 0")
-				}
-			default:
-				builder.WriteString("(dashboard.folder_id IN (SELECT d.id FROM dashboard as d ")
-				if len(permSelectorArgs) > 0 {
-					builder.WriteString("WHERE d.org_id = ? AND d.uid IN ")
+					recQueryName := fmt.Sprintf("RecQry%d", len(f.recQueries))
+					f.addRecQry(recQueryName, permSelector.String(), permSelectorArgs, orgID)
+					builder.WriteString(fmt.Sprintf("WHERE d.org_id = ? AND d.uid IN (SELECT uid FROM %s)", recQueryName))
 					args = append(args, orgID)
-					builder.WriteString(permSelector.String())
-					args = append(args, permSelectorArgs...)
-				} else {
-					builder.WriteString("WHERE 1 = 0")
+				default:
+					nestedFoldersSelectors, nestedFoldersArgs := f.nestedFoldersSelectors(permSelector.String(), permSelectorArgs, "dashboard", "folder_id", "d.id", orgID)
+					builder.WriteRune('(')
+					builder.WriteString(nestedFoldersSelectors)
+					args = append(args, nestedFoldersArgs...)
 				}
+			} else {
+				builder.WriteString("(dashboard.folder_id IN (SELECT d.id FROM dashboard as d ")
+				builder.WriteString("WHERE 1 = 0")
 			}
 			builder.WriteString(") AND NOT dashboard.is_folder)")

@@ -297,33 +284,22 @@ func (f *accessControlDashboardPermissionFilter) buildClauses(dialect migrator.D

 			permSelector.WriteRune(')')

-			switch f.features.IsEnabledGlobally(featuremgmt.FlagNestedFolders) {
-			case true:
-				if len(permSelectorArgs) > 0 {
-					switch f.recursiveQueriesAreSupported {
-					case true:
-						recQueryName := fmt.Sprintf("RecQry%d", len(f.recQueries))
-						f.addRecQry(recQueryName, permSelector.String(), permSelectorArgs, orgID)
-						builder.WriteString("(dashboard.uid IN ")
-						builder.WriteString(fmt.Sprintf("(SELECT uid FROM %s)", recQueryName))
-					default:
-						nestedFoldersSelectors, nestedFoldersArgs := f.nestedFoldersSelectors(permSelector.String(), permSelectorArgs, "dashboard", "uid", "d.uid", orgID)
-						builder.WriteRune('(')
-						builder.WriteString(nestedFoldersSelectors)
-						builder.WriteRune(')')
-						args = append(args, nestedFoldersArgs...)
-					}
-				} else {
-					builder.WriteString("(1 = 0")
-				}
-			default:
-				if len(permSelectorArgs) > 0 {
+			if len(permSelectorArgs) > 0 {
+				switch f.recursiveQueriesAreSupported {
+				case true:
+					recQueryName := fmt.Sprintf("RecQry%d", len(f.recQueries))
+					f.addRecQry(recQueryName, permSelector.String(), permSelectorArgs, orgID)
 					builder.WriteString("(dashboard.uid IN ")
-					builder.WriteString(permSelector.String())
-					args = append(args, permSelectorArgs...)
-				} else {
-					builder.WriteString("(1 = 0")
+					builder.WriteString(fmt.Sprintf("(SELECT uid FROM %s)", recQueryName))
+				default:
+					nestedFoldersSelectors, nestedFoldersArgs := f.nestedFoldersSelectors(permSelector.String(), permSelectorArgs, "dashboard", "uid", "d.uid", orgID)
+					builder.WriteRune('(')
+					builder.WriteString(nestedFoldersSelectors)
+					builder.WriteRune(')')
+					args = append(args, nestedFoldersArgs...)
 				}
+			} else {
+				builder.WriteString("(1 = 0")
 			}
 			builder.WriteString(" AND dashboard.is_folder)")
 		} else {
@@ -7,7 +7,6 @@ import (
 	"github.com/grafana/grafana/pkg/apimachinery/identity"
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
 	"github.com/grafana/grafana/pkg/services/dashboards"
-	"github.com/grafana/grafana/pkg/services/featuremgmt"
 	"github.com/grafana/grafana/pkg/services/folder"
 	"github.com/grafana/grafana/pkg/services/login"
 	"github.com/grafana/grafana/pkg/services/sqlstore/migrator"
@@ -107,36 +106,22 @@ func (f *accessControlDashboardPermissionFilterNoFolderSubquery) buildClauses(di

 			permSelector.WriteRune(')')

-			switch f.features.IsEnabledGlobally(featuremgmt.FlagNestedFolders) {
-			case true:
-				if len(permSelectorArgs) > 0 {
-					switch f.recursiveQueriesAreSupported {
-					case true:
-						recQueryName := fmt.Sprintf("RecQry%d", len(f.recQueries))
-						f.addRecQry(recQueryName, permSelector.String(), permSelectorArgs, orgID)
-						builder.WriteString("(folder.uid IN (SELECT uid FROM " + recQueryName)
-					default:
-						nestedFoldersSelectors, nestedFoldersArgs := f.nestedFoldersSelectors(permSelector.String(), permSelectorArgs, "folder", "uid", "", orgID)
-						builder.WriteRune('(')
-						builder.WriteString(nestedFoldersSelectors)
-						args = append(args, nestedFoldersArgs...)
-					}
-					f.folderIsRequired = true
-					builder.WriteString(") AND NOT dashboard.is_folder)")
-				} else {
-					builder.WriteString("( 1 = 0 AND NOT dashboard.is_folder)")
+			if len(permSelectorArgs) > 0 {
+				switch f.recursiveQueriesAreSupported {
+				case true:
+					recQueryName := fmt.Sprintf("RecQry%d", len(f.recQueries))
+					f.addRecQry(recQueryName, permSelector.String(), permSelectorArgs, orgID)
+					builder.WriteString("(folder.uid IN (SELECT uid FROM " + recQueryName)
+				default:
+					nestedFoldersSelectors, nestedFoldersArgs := f.nestedFoldersSelectors(permSelector.String(), permSelectorArgs, "folder", "uid", "", orgID)
+					builder.WriteRune('(')
+					builder.WriteString(nestedFoldersSelectors)
+					args = append(args, nestedFoldersArgs...)
 				}
-			default:
-				builder.WriteString("(")
-				if len(permSelectorArgs) > 0 {
-					builder.WriteString("folder.uid IN ")
-					builder.WriteString(permSelector.String())
-					args = append(args, permSelectorArgs...)
-					f.folderIsRequired = true
-				} else {
-					builder.WriteString("1 = 0 ")
-				}
-				builder.WriteString(" AND NOT dashboard.is_folder)")
+				f.folderIsRequired = true
+				builder.WriteString(") AND NOT dashboard.is_folder)")
+			} else {
+				builder.WriteString("( 1 = 0 AND NOT dashboard.is_folder)")
 			}

 			// Include all the dashboards under the root if the user has the required permissions on the root (used to be the General folder)
@@ -181,33 +166,22 @@ func (f *accessControlDashboardPermissionFilterNoFolderSubquery) buildClauses(di
 			}
 			permSelector.WriteRune(')')

-			switch f.features.IsEnabledGlobally(featuremgmt.FlagNestedFolders) {
-			case true:
-				if len(permSelectorArgs) > 0 {
-					switch f.recursiveQueriesAreSupported {
-					case true:
-						recQueryName := fmt.Sprintf("RecQry%d", len(f.recQueries))
-						f.addRecQry(recQueryName, permSelector.String(), permSelectorArgs, orgID)
-						builder.WriteString("(dashboard.uid IN ")
-						builder.WriteString(fmt.Sprintf("(SELECT uid FROM %s)", recQueryName))
-					default:
-						nestedFoldersSelectors, nestedFoldersArgs := f.nestedFoldersSelectors(permSelector.String(), permSelectorArgs, "dashboard", "uid", "", orgID)
-						builder.WriteRune('(')
-						builder.WriteString(nestedFoldersSelectors)
-						builder.WriteRune(')')
-						args = append(args, nestedFoldersArgs...)
-					}
-				} else {
-					builder.WriteString("(1 = 0")
-				}
-			default:
-				if len(permSelectorArgs) > 0 {
+			if len(permSelectorArgs) > 0 {
+				switch f.recursiveQueriesAreSupported {
+				case true:
+					recQueryName := fmt.Sprintf("RecQry%d", len(f.recQueries))
+					f.addRecQry(recQueryName, permSelector.String(), permSelectorArgs, orgID)
 					builder.WriteString("(dashboard.uid IN ")
-					builder.WriteString(permSelector.String())
-					args = append(args, permSelectorArgs...)
-				} else {
-					builder.WriteString("(1 = 0")
+					builder.WriteString(fmt.Sprintf("(SELECT uid FROM %s)", recQueryName))
+				default:
+					nestedFoldersSelectors, nestedFoldersArgs := f.nestedFoldersSelectors(permSelector.String(), permSelectorArgs, "dashboard", "uid", "", orgID)
+					builder.WriteRune('(')
+					builder.WriteString(nestedFoldersSelectors)
+					builder.WriteRune(')')
+					args = append(args, nestedFoldersArgs...)
 				}
+			} else {
+				builder.WriteString("(1 = 0")
 			}
 			builder.WriteString(" AND dashboard.is_folder)")
 		} else {
@@ -384,14 +384,12 @@ func TestIntegration_DashboardNestedPermissionFilter(t *testing.T) {
 		permission     dashboardaccess.PermissionType
 		permissions    []accesscontrol.Permission
 		expectedResult []string
-		features       []any
 	}{
 		{
 			desc:           "Should not be able to view dashboards under inherited folders with no permissions if nested folders are enabled",
 			queryType:      searchstore.TypeDashboard,
 			permission:     dashboardaccess.PERMISSION_VIEW,
 			permissions:    nil,
-			features:       []any{featuremgmt.FlagNestedFolders},
 			expectedResult: nil,
 		},
 		{
@@ -399,14 +397,12 @@ func TestIntegration_DashboardNestedPermissionFilter(t *testing.T) {
 			queryType:      searchstore.TypeFolder,
 			permission:     dashboardaccess.PERMISSION_VIEW,
 			permissions:    nil,
-			features:       []any{featuremgmt.FlagNestedFolders},
 			expectedResult: nil,
 		},
 		{
 			desc:           "Should not be able to view inherited dashboards and folders with no permissions if nested folders are enabled",
 			permission:     dashboardaccess.PERMISSION_VIEW,
 			permissions:    nil,
-			features:       []any{featuremgmt.FlagNestedFolders},
 			expectedResult: nil,
 		},
 		{
@@ -416,7 +412,6 @@ func TestIntegration_DashboardNestedPermissionFilter(t *testing.T) {
 			permissions: []accesscontrol.Permission{
 				{Action: dashboards.ActionDashboardsRead, Scope: dashboards.ScopeFoldersAll},
 			},
-			features:       []any{featuremgmt.FlagNestedFolders},
 			expectedResult: []string{"dashboard under the root", "dashboard under parent folder", "dashboard under subfolder"},
 		},
 		{
@@ -426,19 +421,8 @@ func TestIntegration_DashboardNestedPermissionFilter(t *testing.T) {
 			permissions: []accesscontrol.Permission{
 				{Action: dashboards.ActionFoldersRead, Scope: "folders:uid:parent", Kind: "folders", Identifier: "parent"},
 			},
-			features:       []any{featuremgmt.FlagNestedFolders},
 			expectedResult: []string{"parent", "subfolder"},
 		},
-		{
-			desc:       "Should not be able to view inherited folders if nested folders are not enabled",
-			queryType:  searchstore.TypeFolder,
-			permission: dashboardaccess.PERMISSION_VIEW,
-			permissions: []accesscontrol.Permission{
-				{Action: dashboards.ActionFoldersRead, Scope: "folders:uid:parent", Kind: "folders", Identifier: "parent"},
-			},
-			features:       []any{},
-			expectedResult: []string{"parent"},
-		},
 	}

 	var orgID int64 = 1
@@ -452,7 +436,7 @@ func TestIntegration_DashboardNestedPermissionFilter(t *testing.T) {
 		})
 		usr := &user.SignedInUser{OrgID: orgID, OrgRole: org.RoleViewer, Permissions: map[int64]map[string][]string{orgID: accesscontrol.GroupScopesByActionContext(context.Background(), tc.permissions)}}

-		for _, features := range []featuremgmt.FeatureToggles{featuremgmt.WithFeatures(tc.features...), featuremgmt.WithFeatures(append(tc.features, featuremgmt.FlagPermissionsFilterRemoveSubquery)...)} {
+		for _, features := range []featuremgmt.FeatureToggles{featuremgmt.WithFeatures(), featuremgmt.WithFeatures(featuremgmt.FlagPermissionsFilterRemoveSubquery)} {
 			m := features.GetEnabled(context.Background())
 			keys := make([]string, 0, len(m))
 			for k := range m {
@@ -494,14 +478,12 @@ func TestIntegration_DashboardNestedPermissionFilter_WithSelfContainedPermission
 		permission              dashboardaccess.PermissionType
 		signedInUserPermissions []accesscontrol.Permission
 		expectedResult          []string
-		features                []any
 	}{
 		{
 			desc:                    "Should not be able to view dashboards under inherited folders with no permissions if nested folders are enabled",
 			queryType:               searchstore.TypeDashboard,
 			permission:              dashboardaccess.PERMISSION_VIEW,
 			signedInUserPermissions: nil,
-			features:                []any{featuremgmt.FlagNestedFolders},
 			expectedResult:          nil,
 		},
 		{
@@ -509,14 +491,12 @@ func TestIntegration_DashboardNestedPermissionFilter_WithSelfContainedPermission
 			queryType:               searchstore.TypeFolder,
 			permission:              dashboardaccess.PERMISSION_VIEW,
 			signedInUserPermissions: nil,
-			features:                []any{featuremgmt.FlagNestedFolders},
 			expectedResult:          nil,
 		},
 		{
 			desc:                    "Should not be able to view inherited dashboards and folders with no permissions if nested folders are enabled",
 			permission:              dashboardaccess.PERMISSION_VIEW,
 			signedInUserPermissions: nil,
-			features:                []any{featuremgmt.FlagNestedFolders},
 			expectedResult:          nil,
 		},
 		{
@@ -526,7 +506,6 @@ func TestIntegration_DashboardNestedPermissionFilter_WithSelfContainedPermission
 			signedInUserPermissions: []accesscontrol.Permission{
 				{Action: dashboards.ActionDashboardsRead, Scope: dashboards.ScopeFoldersAll},
 			},
-			features:       []any{featuremgmt.FlagNestedFolders},
 			expectedResult: []string{"dashboard under the root", "dashboard under parent folder", "dashboard under subfolder"},
 		},
 		{
@@ -536,19 +515,8 @@ func TestIntegration_DashboardNestedPermissionFilter_WithSelfContainedPermission
 			signedInUserPermissions: []accesscontrol.Permission{
 				{Action: dashboards.ActionFoldersRead, Scope: "folders:uid:parent"},
 			},
-			features:       []any{featuremgmt.FlagNestedFolders},
 			expectedResult: []string{"parent", "subfolder"},
 		},
-		{
-			desc:       "Should not be able to view inherited folders if nested folders are not enabled",
-			queryType:  searchstore.TypeFolder,
-			permission: dashboardaccess.PERMISSION_VIEW,
-			signedInUserPermissions: []accesscontrol.Permission{
-				{Action: dashboards.ActionFoldersRead, Scope: "folders:uid:parent"},
-			},
-			features:       []any{},
-			expectedResult: []string{"parent"},
-		},
 	}

 	var orgID int64 = 1
@@ -566,7 +534,7 @@ func TestIntegration_DashboardNestedPermissionFilter_WithSelfContainedPermission
 			}),
 			},
 		}
-		for _, features := range []featuremgmt.FeatureToggles{featuremgmt.WithFeatures(tc.features...), featuremgmt.WithFeatures(append(tc.features, featuremgmt.FlagPermissionsFilterRemoveSubquery)...)} {
+		for _, features := range []featuremgmt.FeatureToggles{featuremgmt.WithFeatures(), featuremgmt.WithFeatures(featuremgmt.FlagPermissionsFilterRemoveSubquery)} {
 			m := features.GetEnabled(context.Background())
 			keys := make([]string, 0, len(m))
 			for k := range m {
@@ -676,7 +644,7 @@ func TestIntegration_DashboardNestedPermissionFilter_WithActionSets(t *testing.T
 			Scope:  "folders:uid:unrelated"})
 		usr := &user.SignedInUser{OrgID: orgID, OrgRole: org.RoleViewer, Permissions: map[int64]map[string][]string{orgID: accesscontrol.GroupScopesByActionContext(context.Background(), tc.signedInUserPermissions)}}

-		for _, features := range []featuremgmt.FeatureToggles{featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders), featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders, featuremgmt.FlagPermissionsFilterRemoveSubquery)} {
+		for _, features := range []featuremgmt.FeatureToggles{featuremgmt.WithFeatures(), featuremgmt.WithFeatures(featuremgmt.FlagPermissionsFilterRemoveSubquery)} {
 			m := features.GetEnabled(context.Background())
 			keys := make([]string, 0, len(m))
 			for k := range m {
@@ -711,56 +679,65 @@ func TestIntegration_DashboardNestedPermissionFilter_WithActionSets(t *testing.T
 func setupTest(t *testing.T, numFolders, numDashboards int, permissions []accesscontrol.Permission) db.DB {
 	t.Helper()

-	store := db.InitTestDB(t)
-	err := store.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error {
-		dashes := make([]dashboards.Dashboard, 0, numFolders+numDashboards)
-		for i := 1; i <= numFolders; i++ {
-			str := strconv.Itoa(i)
-			dashes = append(dashes, dashboards.Dashboard{
-				OrgID:    1,
-				Slug:     str,
-				UID:      str,
-				Title:    str,
-				IsFolder: true,
-				Data:     simplejson.New(),
-				Created:  time.Now(),
-				Updated:  time.Now(),
-			})
-		}
-		// Seed dashboards
-		for i := numFolders + 1; i <= numFolders+numDashboards; i++ {
-			str := strconv.Itoa(i)
-			folderID := 0
-			if i%(numFolders+1) != 0 {
-				folderID = i % (numFolders + 1)
-			}
-			dashes = append(dashes, dashboards.Dashboard{
-				OrgID:     1,
-				IsFolder:  false,
-				FolderUID: strconv.Itoa(folderID),
-				UID:       str,
-				Slug:      str,
-				Title:     str,
-				Data:      simplejson.New(),
-				Created:   time.Now(),
-				Updated:   time.Now(),
-			})
+	db, cfg := db.InitTestDBWithCfg(t)
+	dashStore, err := database.ProvideDashboardStore(db, cfg, featuremgmt.WithFeatures(), tagimpl.ProvideService(db))
+	require.NoError(t, err)
+	fStore := folderimpl.ProvideStore(db)
+
+	// Create a signed-in user for folder creation
+	usr := &user.SignedInUser{
+		UserID: 1,
+		OrgID:  1,
+		Login:  "test",
+	}
+
+	// folders need to be created in both the folder and dashboard table
+	for i := 1; i <= numFolders; i++ {
+		str := strconv.Itoa(i)
+		folder, err := fStore.Create(context.Background(), folder.CreateFolderCommand{
+			Title:        str,
+			OrgID:        1,
+			UID:          str,
+			SignedInUser: usr,
+		})
+		require.NoError(t, err)
+		_, err = dashStore.SaveDashboard(context.Background(), dashboards.SaveDashboardCommand{
+			OrgID: 1,
+			Dashboard: simplejson.NewFromAny(map[string]any{
+				"title": str,
+				"uid":   folder.UID,
+			}),
+			IsFolder: true,
+		})
+		require.NoError(t, err)
+	}
+
+	// now create dashboards
+	for i := numFolders + 1; i <= numFolders+numDashboards; i++ {
+		str := strconv.Itoa(i)
+		folderID := 0
+		if i%(numFolders+1) != 0 {
+			folderID = i % (numFolders + 1)
 		}

-		// Insert dashboards in batches
-		batchSize := 500
-		for i := 0; i < len(dashes); i += batchSize {
-			end := i + batchSize
-			if end > len(dashes) {
-				end = len(dashes)
-			}
-
-			_, err := sess.InsertMulti(dashes[i:end])
-			if err != nil {
-				return err
-			}
+		cmd := dashboards.SaveDashboardCommand{
+			OrgID: 1,
+			Dashboard: simplejson.NewFromAny(map[string]any{
+				"title": str,
+				"uid":   str,
+			}),
+			IsFolder: false,
+		}
+		if folderID != 0 {
+			cmd.FolderUID = strconv.Itoa(folderID)
 		}

+		_, err := dashStore.SaveDashboard(context.Background(), cmd)
+		require.NoError(t, err)
+	}
+
+	// insert permissions
+	err = db.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error {
 		role := &accesscontrol.Role{
 			OrgID:   0,
 			UID:     "basic_viewer",
@@ -792,6 +769,7 @@ func setupTest(t *testing.T, numFolders, numDashboards int, permissions []access
 		}

 		if len(permissions) > 0 {
+			batchSize := 500
 			for i := 0; i < len(permissions); i += batchSize {
 				end := i + batchSize
 				if end > len(permissions) {
@@ -808,7 +786,7 @@ func setupTest(t *testing.T, numFolders, numDashboards int, permissions []access
 		return nil
 	})
 	require.NoError(t, err)
-	return store
+	return db
 }

 func setupNestedTest(t *testing.T, usr *user.SignedInUser, perms []accesscontrol.Permission, orgID int64, features featuremgmt.FeatureToggles) db.DB {
@@ -44,11 +44,6 @@ func benchmarkDashboardPermissionFilter(b *testing.B, numUsers, numDashboards, n
 	}}

 	features := featuremgmt.WithFeatures()
-	// if nestingLevel > 0 enable nested folders
-	if nestingLevel > 0 {
-		features = featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders)
-	}
-
 	store := setupBenchMark(b, usr, features, numUsers, numDashboards, numFolders, nestingLevel)

 	recursiveQueriesAreSupported, err := store.RecursiveQueriesAreSupported()