public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[v10.1.x] Search v1: Remove unnecessary subqueries (#72710)

Browse Source 
Search v1: Remove unnecessary subqueries (#72388)

* Add feature flag

* Introduce interface and dummy implementation

* Add tests for the new filter

* accessControlDashboardPermissionFilterNoFolderSubquery implementation

* join only if it's necessary

* force ordering for tests

* Temporarily enable new query for benchmarks

(cherry picked from commit 2c26a02b82)

Co-authored-by: Sofia Papagiannaki <1632407+papagian@users.noreply.github.com>
This commit is contained in:
grafana-delivery-bot[bot]
2023-08-03 11:12:14 +03:00
committed by GitHub
co-authored by Sofia Papagiannaki
parent d7a65cb7ca
commit 5769d0be49
15 changed files with 516 additions and 157 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/services/sqlstore/permissions/dashboard_test.go
+159 -105
View File
@@ -3,6 +3,7 @@ package permissions_test
import (
"context"
"strconv"
"strings"
"testing"
"time"
@@ -162,26 +163,39 @@ func TestIntegration_DashboardPermissionFilter(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) {
store := setupTest(t, 10, 100, tt.permissions)
recursiveQueriesAreSupported, err := store.RecursiveQueriesAreSupported()
require.NoError(t, err)
store := setupTest(t, 10, 100, tt.permissions)
recursiveQueriesAreSupported, err := store.RecursiveQueriesAreSupported()
require.NoError(t, err)
usr := &user.SignedInUser{OrgID: 1, OrgRole: org.RoleViewer, Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(tt.permissions)}}
filter := permissions.NewAccessControlDashboardPermissionFilter(usr, tt.permission, tt.queryType, featuremgmt.WithFeatures(), recursiveQueriesAreSupported)
usr := &user.SignedInUser{OrgID: 1, OrgRole: org.RoleViewer, Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(tt.permissions)}}
var result int
err = store.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error {
q, params := filter.Where()
recQry, recQryParams := filter.With()
params = append(recQryParams, params...)
_, err := sess.SQL(recQry+"\nSELECT COUNT(*) FROM dashboard WHERE "+q, params...).Get(&result)
return err
for _, features := range []*featuremgmt.FeatureManager{featuremgmt.WithFeatures(), featuremgmt.WithFeatures(featuremgmt.FlagPermissionsFilterRemoveSubquery)} {
m := features.GetEnabled(context.Background())
keys := make([]string, 0, len(m))
for k := range m {
keys = append(keys, k)
}
t.Run(tt.desc+" with features "+strings.Join(keys, ","), func(t *testing.T) {
filter := permissions.NewAccessControlDashboardPermissionFilter(usr, tt.permission, tt.queryType, features, recursiveQueriesAreSupported)
var result int
err = store.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error {
q, params := filter.Where()
recQry, recQryParams := filter.With()
params = append(recQryParams, params...)
leftJoin := filter.LeftJoin()
s := recQry + "\nSELECT COUNT(*) FROM dashboard WHERE " + q
if leftJoin != "" {
s = recQry + "\nSELECT COUNT(*) FROM dashboard LEFT OUTER JOIN " + leftJoin + " WHERE " + q
}
_, err := sess.SQL(s, params...).Get(&result)
return err
})
require.NoError(t, err)
assert.Equal(t, tt.expectedResult, result)
})
require.NoError(t, err)
assert.Equal(t, tt.expectedResult, result)
})
}
}
}
@@ -323,26 +337,39 @@ func TestIntegration_DashboardPermissionFilter_WithSelfContainedPermissions(t *t
}
for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) {
store := setupTest(t, 10, 100, []accesscontrol.Permission{})
recursiveQueriesAreSupported, err := store.RecursiveQueriesAreSupported()
require.NoError(t, err)
store := setupTest(t, 10, 100, []accesscontrol.Permission{})
recursiveQueriesAreSupported, err := store.RecursiveQueriesAreSupported()
require.NoError(t, err)
usr := &user.SignedInUser{OrgID: 1, OrgRole: org.RoleViewer, AuthenticatedBy: login.ExtendedJWTModule, Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(tt.signedInUserPermissions)}}
filter := permissions.NewAccessControlDashboardPermissionFilter(usr, tt.permission, tt.queryType, featuremgmt.WithFeatures(), recursiveQueriesAreSupported)
usr := &user.SignedInUser{OrgID: 1, OrgRole: org.RoleViewer, AuthenticatedBy: login.ExtendedJWTModule, Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(tt.signedInUserPermissions)}}
var result int
err = store.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error {
q, params := filter.Where()
recQry, recQryParams := filter.With()
params = append(recQryParams, params...)
_, err := sess.SQL(recQry+"\nSELECT COUNT(*) FROM dashboard WHERE "+q, params...).Get(&result)
return err
for _, features := range []*featuremgmt.FeatureManager{featuremgmt.WithFeatures(), featuremgmt.WithFeatures(featuremgmt.FlagPermissionsFilterRemoveSubquery)} {
m := features.GetEnabled(context.Background())
keys := make([]string, 0, len(m))
for k := range m {
keys = append(keys, k)
}
t.Run(tt.desc+" with features "+strings.Join(keys, ","), func(t *testing.T) {
filter := permissions.NewAccessControlDashboardPermissionFilter(usr, tt.permission, tt.queryType, features, recursiveQueriesAreSupported)
var result int
err = store.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error {
q, params := filter.Where()
recQry, recQryParams := filter.With()
params = append(recQryParams, params...)
s := recQry + "\nSELECT COUNT(*) FROM dashboard WHERE " + q
leftJoin := filter.LeftJoin()
if leftJoin != "" {
s = recQry + "\nSELECT COUNT(*) FROM dashboard LEFT OUTER JOIN " + leftJoin + " WHERE " + q
}
_, err := sess.SQL(s, params...).Get(&result)
return err
})
require.NoError(t, err)
assert.Equal(t, tt.expectedResult, result)
})
require.NoError(t, err)
assert.Equal(t, tt.expectedResult, result)
})
}
}
}
@@ -353,14 +380,14 @@ func TestIntegration_DashboardNestedPermissionFilter(t *testing.T) {
permission dashboards.PermissionType
permissions []accesscontrol.Permission
expectedResult []string
features featuremgmt.FeatureToggles
features []interface{}
}{
{
desc: "Should not be able to view dashboards under inherited folders with no permissions if nested folders are enabled",
queryType: searchstore.TypeDashboard,
permission: dashboards.PERMISSION_VIEW,
permissions: nil,
features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders),
features: []interface{}{featuremgmt.FlagNestedFolders},
expectedResult: nil,
},
{
@@ -368,14 +395,14 @@ func TestIntegration_DashboardNestedPermissionFilter(t *testing.T) {
queryType: searchstore.TypeFolder,
permission: dashboards.PERMISSION_VIEW,
permissions: nil,
features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders),
features: []interface{}{featuremgmt.FlagNestedFolders},
expectedResult: nil,
},
{
desc: "Should not be able to view inherited dashboards and folders with no permissions if nested folders are enabled",
permission: dashboards.PERMISSION_VIEW,
permissions: nil,
features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders),
features: []interface{}{featuremgmt.FlagNestedFolders},
expectedResult: nil,
},
{
@@ -385,7 +412,7 @@ func TestIntegration_DashboardNestedPermissionFilter(t *testing.T) {
permissions: []accesscontrol.Permission{
{Action: dashboards.ActionDashboardsRead, Scope: dashboards.ScopeFoldersAll},
},
features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders),
features: []interface{}{featuremgmt.FlagNestedFolders},
expectedResult: []string{"dashboard under parent folder", "dashboard under subfolder"},
},
{
@@ -395,7 +422,7 @@ func TestIntegration_DashboardNestedPermissionFilter(t *testing.T) {
permissions: []accesscontrol.Permission{
{Action: dashboards.ActionDashboardsRead, Scope: "folders:uid:parent"},
},
features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders),
features: []interface{}{featuremgmt.FlagNestedFolders},
expectedResult: []string{"dashboard under parent folder", "dashboard under subfolder"},
},
{
@@ -405,7 +432,7 @@ func TestIntegration_DashboardNestedPermissionFilter(t *testing.T) {
permissions: []accesscontrol.Permission{
{Action: dashboards.ActionDashboardsRead, Scope: "folders:uid:parent"},
},
features: featuremgmt.WithFeatures(),
features: []interface{}{},
expectedResult: []string{"dashboard under parent folder"},
},
{
@@ -415,7 +442,7 @@ func TestIntegration_DashboardNestedPermissionFilter(t *testing.T) {
permissions: []accesscontrol.Permission{
{Action: dashboards.ActionFoldersRead, Scope: "folders:uid:parent"},
},
features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders),
features: []interface{}{featuremgmt.FlagNestedFolders},
expectedResult: []string{"parent", "subfolder"},
},
{
@@ -425,7 +452,7 @@ func TestIntegration_DashboardNestedPermissionFilter(t *testing.T) {
permissions: []accesscontrol.Permission{
{Action: dashboards.ActionFoldersRead, Scope: "folders:uid:parent"},
},
features: featuremgmt.WithFeatures(),
features: []interface{}{},
expectedResult: []string{"parent"},
},
{
@@ -435,7 +462,7 @@ func TestIntegration_DashboardNestedPermissionFilter(t *testing.T) {
{Action: dashboards.ActionFoldersRead, Scope: "folders:uid:parent"},
{Action: dashboards.ActionDashboardsRead, Scope: "folders:uid:parent"},
},
features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders),
features: []interface{}{featuremgmt.FlagNestedFolders},
expectedResult: []string{"parent", "subfolder", "dashboard under parent folder", "dashboard under subfolder"},
},
{
@@ -445,7 +472,7 @@ func TestIntegration_DashboardNestedPermissionFilter(t *testing.T) {
{Action: dashboards.ActionFoldersRead, Scope: "folders:uid:parent"},
{Action: dashboards.ActionDashboardsRead, Scope: "folders:uid:parent"},
},
features: featuremgmt.WithFeatures(),
features: []interface{}{},
expectedResult: []string{"parent", "dashboard under parent folder"},
},
}
@@ -459,29 +486,43 @@ func TestIntegration_DashboardNestedPermissionFilter(t *testing.T) {
var orgID int64 = 1
for _, tc := range testCases {
t.Run(tc.desc, func(t *testing.T) {
tc.permissions = append(tc.permissions, accesscontrol.Permission{
Action: dashboards.ActionFoldersCreate,
}, accesscontrol.Permission{
Action: dashboards.ActionFoldersWrite,
Scope: dashboards.ScopeFoldersAll,
})
usr := &user.SignedInUser{OrgID: orgID, OrgRole: org.RoleViewer, Permissions: map[int64]map[string][]string{orgID: accesscontrol.GroupScopesByAction(tc.permissions)}}
db := setupNestedTest(t, usr, tc.permissions, orgID, tc.features)
recursiveQueriesAreSupported, err := db.RecursiveQueriesAreSupported()
require.NoError(t, err)
filter := permissions.NewAccessControlDashboardPermissionFilter(usr, tc.permission, tc.queryType, tc.features, recursiveQueriesAreSupported)
var result []string
err = db.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error {
q, params := filter.Where()
recQry, recQryParams := filter.With()
params = append(recQryParams, params...)
err := sess.SQL(recQry+"\nSELECT title FROM dashboard WHERE "+q, params...).Find(&result)
return err
})
require.NoError(t, err)
assert.Equal(t, tc.expectedResult, result)
tc.permissions = append(tc.permissions, accesscontrol.Permission{
Action: dashboards.ActionFoldersCreate,
}, accesscontrol.Permission{
Action: dashboards.ActionFoldersWrite,
Scope: dashboards.ScopeFoldersAll,
})
usr := &user.SignedInUser{OrgID: orgID, OrgRole: org.RoleViewer, Permissions: map[int64]map[string][]string{orgID: accesscontrol.GroupScopesByAction(tc.permissions)}}
for _, features := range []*featuremgmt.FeatureManager{featuremgmt.WithFeatures(tc.features...), featuremgmt.WithFeatures(append(tc.features, featuremgmt.FlagPermissionsFilterRemoveSubquery)...)} {
m := features.GetEnabled(context.Background())
keys := make([]string, 0, len(m))
for k := range m {
keys = append(keys, k)
}
t.Run(tc.desc+" with features "+strings.Join(keys, ","), func(t *testing.T) {
db := setupNestedTest(t, usr, tc.permissions, orgID, features)
recursiveQueriesAreSupported, err := db.RecursiveQueriesAreSupported()
require.NoError(t, err)
filter := permissions.NewAccessControlDashboardPermissionFilter(usr, tc.permission, tc.queryType, features, recursiveQueriesAreSupported)
var result []string
err = db.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error {
q, params := filter.Where()
recQry, recQryParams := filter.With()
params = append(recQryParams, params...)
s := recQry + "\nSELECT dashboard.title FROM dashboard WHERE " + q
leftJoin := filter.LeftJoin()
if leftJoin != "" {
s = recQry + "\nSELECT dashboard.title FROM dashboard LEFT OUTER JOIN " + leftJoin + " WHERE " + q + "ORDER BY dashboard.id ASC"
}
err := sess.SQL(s, params...).Find(&result)
return err
})
require.NoError(t, err)
assert.Equal(t, tc.expectedResult, result)
})
}
}
}
@@ -492,14 +533,14 @@ func TestIntegration_DashboardNestedPermissionFilter_WithSelfContainedPermission
permission dashboards.PermissionType
signedInUserPermissions []accesscontrol.Permission
expectedResult []string
features featuremgmt.FeatureToggles
features []interface{}
}{
{
desc: "Should not be able to view dashboards under inherited folders with no permissions if nested folders are enabled",
queryType: searchstore.TypeDashboard,
permission: dashboards.PERMISSION_VIEW,
signedInUserPermissions: nil,
features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders),
features: []interface{}{featuremgmt.FlagNestedFolders},
expectedResult: nil,
},
{
@@ -507,14 +548,14 @@ func TestIntegration_DashboardNestedPermissionFilter_WithSelfContainedPermission
queryType: searchstore.TypeFolder,
permission: dashboards.PERMISSION_VIEW,
signedInUserPermissions: nil,
features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders),
features: []interface{}{featuremgmt.FlagNestedFolders},
expectedResult: nil,
},
{
desc: "Should not be able to view inherited dashboards and folders with no permissions if nested folders are enabled",
permission: dashboards.PERMISSION_VIEW,
signedInUserPermissions: nil,
features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders),
features: []interface{}{featuremgmt.FlagNestedFolders},
expectedResult: nil,
},
{
@@ -524,7 +565,7 @@ func TestIntegration_DashboardNestedPermissionFilter_WithSelfContainedPermission
signedInUserPermissions: []accesscontrol.Permission{
{Action: dashboards.ActionDashboardsRead, Scope: dashboards.ScopeFoldersAll},
},
features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders),
features: []interface{}{featuremgmt.FlagNestedFolders},
expectedResult: []string{"dashboard under parent folder", "dashboard under subfolder"},
},
{
@@ -534,7 +575,7 @@ func TestIntegration_DashboardNestedPermissionFilter_WithSelfContainedPermission
signedInUserPermissions: []accesscontrol.Permission{
{Action: dashboards.ActionDashboardsRead, Scope: "folders:uid:parent"},
},
features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders),
features: []interface{}{featuremgmt.FlagNestedFolders},
expectedResult: []string{"dashboard under parent folder", "dashboard under subfolder"},
},
{
@@ -544,7 +585,7 @@ func TestIntegration_DashboardNestedPermissionFilter_WithSelfContainedPermission
signedInUserPermissions: []accesscontrol.Permission{
{Action: dashboards.ActionDashboardsRead, Scope: "folders:uid:parent"},
},
features: featuremgmt.WithFeatures(),
features: []interface{}{},
expectedResult: []string{"dashboard under parent folder"},
},
{
@@ -554,7 +595,7 @@ func TestIntegration_DashboardNestedPermissionFilter_WithSelfContainedPermission
signedInUserPermissions: []accesscontrol.Permission{
{Action: dashboards.ActionFoldersRead, Scope: "folders:uid:parent"},
},
features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders),
features: []interface{}{featuremgmt.FlagNestedFolders},
expectedResult: []string{"parent", "subfolder"},
},
{
@@ -564,7 +605,7 @@ func TestIntegration_DashboardNestedPermissionFilter_WithSelfContainedPermission
signedInUserPermissions: []accesscontrol.Permission{
{Action: dashboards.ActionFoldersRead, Scope: "folders:uid:parent"},
},
features: featuremgmt.WithFeatures(),
features: []interface{}{},
expectedResult: []string{"parent"},
},
{
@@ -574,7 +615,7 @@ func TestIntegration_DashboardNestedPermissionFilter_WithSelfContainedPermission
{Action: dashboards.ActionFoldersRead, Scope: "folders:uid:parent"},
{Action: dashboards.ActionDashboardsRead, Scope: "folders:uid:parent"},
},
features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders),
features: []interface{}{featuremgmt.FlagNestedFolders},
expectedResult: []string{"parent", "subfolder", "dashboard under parent folder", "dashboard under subfolder"},
},
{
@@ -584,7 +625,7 @@ func TestIntegration_DashboardNestedPermissionFilter_WithSelfContainedPermission
{Action: dashboards.ActionFoldersRead, Scope: "folders:uid:parent"},
{Action: dashboards.ActionDashboardsRead, Scope: "folders:uid:parent"},
},
features: featuremgmt.WithFeatures(),
features: []interface{}{},
expectedResult: []string{"parent", "dashboard under parent folder"},
},
{
@@ -598,7 +639,7 @@ func TestIntegration_DashboardNestedPermissionFilter_WithSelfContainedPermission
{Action: dashboards.ActionDashboardsRead, Scope: "folders:uid:parent"},
{Action: dashboards.ActionDashboardsWrite, Scope: "folders:uid:parent"},
},
features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders),
features: []interface{}{featuremgmt.FlagNestedFolders},
expectedResult: []string{"subfolder", "dashboard under parent folder", "dashboard under subfolder"},
},
}
@@ -612,35 +653,48 @@ func TestIntegration_DashboardNestedPermissionFilter_WithSelfContainedPermission
var orgID int64 = 1
for _, tc := range testCases {
t.Run(tc.desc, func(t *testing.T) {
helperUser := &user.SignedInUser{OrgID: orgID, OrgRole: org.RoleViewer, AuthenticatedBy: login.ExtendedJWTModule,
Permissions: map[int64]map[string][]string{orgID: accesscontrol.GroupScopesByAction([]accesscontrol.Permission{
{
Action: dashboards.ActionFoldersCreate,
},
{
Action: dashboards.ActionFoldersWrite,
Scope: dashboards.ScopeFoldersAll,
},
}),
helperUser := &user.SignedInUser{OrgID: orgID, OrgRole: org.RoleViewer, AuthenticatedBy: login.ExtendedJWTModule,
Permissions: map[int64]map[string][]string{orgID: accesscontrol.GroupScopesByAction([]accesscontrol.Permission{
{
Action: dashboards.ActionFoldersCreate,
},
{
Action: dashboards.ActionFoldersWrite,
Scope: dashboards.ScopeFoldersAll,
},
}),
},
}
for _, features := range []*featuremgmt.FeatureManager{featuremgmt.WithFeatures(tc.features...), featuremgmt.WithFeatures(append(tc.features, featuremgmt.FlagPermissionsFilterRemoveSubquery)...)} {
m := features.GetEnabled(context.Background())
keys := make([]string, 0, len(m))
for k := range m {
keys = append(keys, k)
}
usr := &user.SignedInUser{OrgID: orgID, OrgRole: org.RoleViewer, AuthenticatedBy: login.ExtendedJWTModule, Permissions: map[int64]map[string][]string{orgID: accesscontrol.GroupScopesByAction(tc.signedInUserPermissions)}}
db := setupNestedTest(t, helperUser, []accesscontrol.Permission{}, orgID, tc.features)
recursiveQueriesAreSupported, err := db.RecursiveQueriesAreSupported()
require.NoError(t, err)
filter := permissions.NewAccessControlDashboardPermissionFilter(usr, tc.permission, tc.queryType, tc.features, recursiveQueriesAreSupported)
var result []string
err = db.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error {
q, params := filter.Where()
recQry, recQryParams := filter.With()
params = append(recQryParams, params...)
err := sess.SQL(recQry+"\nSELECT title FROM dashboard WHERE "+q, params...).Find(&result)
return err
t.Run(tc.desc+" with features "+strings.Join(keys, ","), func(t *testing.T) {
usr := &user.SignedInUser{OrgID: orgID, OrgRole: org.RoleViewer, AuthenticatedBy: login.ExtendedJWTModule, Permissions: map[int64]map[string][]string{orgID: accesscontrol.GroupScopesByAction(tc.signedInUserPermissions)}}
db := setupNestedTest(t, helperUser, []accesscontrol.Permission{}, orgID, features)
recursiveQueriesAreSupported, err := db.RecursiveQueriesAreSupported()
require.NoError(t, err)
filter := permissions.NewAccessControlDashboardPermissionFilter(usr, tc.permission, tc.queryType, features, recursiveQueriesAreSupported)
var result []string
err = db.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error {
q, params := filter.Where()
recQry, recQryParams := filter.With()
params = append(recQryParams, params...)
s := recQry + "\nSELECT dashboard.title FROM dashboard WHERE " + q
leftJoin := filter.LeftJoin()
if leftJoin != "" {
s = recQry + "\nSELECT dashboard.title FROM dashboard LEFT OUTER JOIN " + leftJoin + " WHERE " + q + " ORDER BY dashboard.id ASC"
}
err := sess.SQL(s, params...).Find(&result)
return err
})
require.NoError(t, err)
assert.Equal(t, tc.expectedResult, result)
})
require.NoError(t, err)
assert.Equal(t, tc.expectedResult, result)
})
}
}
}