Folders: Replace sql query with folder service call when collecting folder tree (#98443)

* Replace sql query with folder service call when collecting folder tree
* Update provider for folder service implementation for wire
* Refactor provisioning of oss service in folder permissions test util
pull/98571/head
Arati R. 6 months ago committed by GitHub
parent de9aec8e56
commit 6957e1f7b7
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
  1. 8
      pkg/api/folder_bench_test.go
  2. 1
      pkg/server/wire.go
  3. 8
      pkg/services/accesscontrol/acimpl/service.go
  4. 1
      pkg/services/accesscontrol/acimpl/service_test.go
  5. 29
      pkg/services/accesscontrol/dualwrite/collectors.go
  6. 5
      pkg/services/accesscontrol/dualwrite/reconciler.go
  7. 10
      pkg/services/accesscontrol/ossaccesscontrol/testutil/testutil.go
  8. 18
      pkg/services/dashboards/service/zanzana_integration_test.go
  9. 2
      pkg/services/folder/folderimpl/folder.go
  10. 2
      pkg/services/serviceaccounts/extsvcaccounts/service_test.go

@ -459,13 +459,13 @@ func setupServer(b testing.TB, sc benchScenario, features featuremgmt.FeatureTog
ac := acimpl.ProvideAccessControl(featuremgmt.WithFeatures(), zanzana.NewNoopClient()) ac := acimpl.ProvideAccessControl(featuremgmt.WithFeatures(), zanzana.NewNoopClient())
cfg := setting.NewCfg() cfg := setting.NewCfg()
actionSets := resourcepermissions.NewActionSetService(features) actionSets := resourcepermissions.NewActionSetService(features)
acSvc := acimpl.ProvideOSSService(
sc.cfg, acdb.ProvideService(sc.db), actionSets, localcache.ProvideService(),
features, tracing.InitializeTracerForTest(), zanzana.NewNoopClient(), sc.db, permreg.ProvidePermissionRegistry(), nil,
)
fStore := folderimpl.ProvideStore(sc.db) fStore := folderimpl.ProvideStore(sc.db)
folderServiceWithFlagOn := folderimpl.ProvideService(fStore, ac, bus.ProvideBus(tracing.InitializeTracerForTest()), dashStore, folderServiceWithFlagOn := folderimpl.ProvideService(fStore, ac, bus.ProvideBus(tracing.InitializeTracerForTest()), dashStore,
folderStore, sc.db, features, supportbundlestest.NewFakeBundleService(), nil, tracing.InitializeTracerForTest()) folderStore, sc.db, features, supportbundlestest.NewFakeBundleService(), nil, tracing.InitializeTracerForTest())
acSvc := acimpl.ProvideOSSService(
sc.cfg, acdb.ProvideService(sc.db), actionSets, localcache.ProvideService(),
features, tracing.InitializeTracerForTest(), zanzana.NewNoopClient(), sc.db, permreg.ProvidePermissionRegistry(), nil, folderServiceWithFlagOn,
)
folderPermissions, err := ossaccesscontrol.ProvideFolderPermissions( folderPermissions, err := ossaccesscontrol.ProvideFolderPermissions(
cfg, features, routing.NewRouteRegister(), sc.db, ac, license, &dashboards.FakeDashboardStore{}, folderServiceWithFlagOn, acSvc, sc.teamSvc, sc.userSvc, actionSets) cfg, features, routing.NewRouteRegister(), sc.db, ac, license, &dashboards.FakeDashboardStore{}, folderServiceWithFlagOn, acSvc, sc.teamSvc, sc.userSvc, actionSets)
require.NoError(b, err) require.NoError(b, err)

@ -300,6 +300,7 @@ var wireBasicSet = wire.NewSet(
dashboardservice.ProvideDashboardPluginService, dashboardservice.ProvideDashboardPluginService,
dashboardstore.ProvideDashboardStore, dashboardstore.ProvideDashboardStore,
folderimpl.ProvideService, folderimpl.ProvideService,
wire.Bind(new(folder.Service), new(*folderimpl.Service)),
folderimpl.ProvideStore, folderimpl.ProvideStore,
wire.Bind(new(folder.Store), new(*folderimpl.FolderStoreImpl)), wire.Bind(new(folder.Store), new(*folderimpl.FolderStoreImpl)),
folderimpl.ProvideDashboardFolderStore, folderimpl.ProvideDashboardFolderStore,

@ -55,7 +55,7 @@ func ProvideService(
cfg *setting.Cfg, db db.DB, routeRegister routing.RouteRegister, cache *localcache.CacheService, cfg *setting.Cfg, db db.DB, routeRegister routing.RouteRegister, cache *localcache.CacheService,
accessControl accesscontrol.AccessControl, userService user.Service, actionResolver accesscontrol.ActionResolver, accessControl accesscontrol.AccessControl, userService user.Service, actionResolver accesscontrol.ActionResolver,
features featuremgmt.FeatureToggles, tracer tracing.Tracer, zclient zanzana.Client, permRegistry permreg.PermissionRegistry, features featuremgmt.FeatureToggles, tracer tracing.Tracer, zclient zanzana.Client, permRegistry permreg.PermissionRegistry,
lock *serverlock.ServerLockService, lock *serverlock.ServerLockService, folderService folder.Service,
) (*Service, error) { ) (*Service, error) {
service := ProvideOSSService( service := ProvideOSSService(
cfg, cfg,
@ -68,6 +68,7 @@ func ProvideService(
db, db,
permRegistry, permRegistry,
lock, lock,
folderService,
) )
api.NewAccessControlAPI(routeRegister, accessControl, service, userService, features).RegisterAPIEndpoints() api.NewAccessControlAPI(routeRegister, accessControl, service, userService, features).RegisterAPIEndpoints()
@ -89,7 +90,8 @@ func ProvideService(
func ProvideOSSService( func ProvideOSSService(
cfg *setting.Cfg, store accesscontrol.Store, actionResolver accesscontrol.ActionResolver, cfg *setting.Cfg, store accesscontrol.Store, actionResolver accesscontrol.ActionResolver,
cache *localcache.CacheService, features featuremgmt.FeatureToggles, tracer tracing.Tracer, cache *localcache.CacheService, features featuremgmt.FeatureToggles, tracer tracing.Tracer,
zclient zanzana.Client, db db.DB, permRegistry permreg.PermissionRegistry, lock *serverlock.ServerLockService, zclient zanzana.Client, db db.DB, permRegistry permreg.PermissionRegistry,
lock *serverlock.ServerLockService, folderService folder.Service,
) *Service { ) *Service {
s := &Service{ s := &Service{
actionResolver: actionResolver, actionResolver: actionResolver,
@ -99,7 +101,7 @@ func ProvideOSSService(
log: log.New("accesscontrol.service"), log: log.New("accesscontrol.service"),
roles: accesscontrol.BuildBasicRoleDefinitions(), roles: accesscontrol.BuildBasicRoleDefinitions(),
store: store, store: store,
reconciler: dualwrite.NewZanzanaReconciler(cfg, zclient, db, lock), reconciler: dualwrite.NewZanzanaReconciler(cfg, zclient, db, lock, folderService),
permRegistry: permRegistry, permRegistry: permRegistry,
} }

@ -74,6 +74,7 @@ func TestUsageMetrics(t *testing.T) {
nil, nil,
permreg.ProvidePermissionRegistry(), permreg.ProvidePermissionRegistry(),
nil, nil,
nil,
) )
assert.Equal(t, tt.expectedValue, s.GetUsageStats(context.Background())["stats.oss.accesscontrol.enabled.count"]) assert.Equal(t, tt.expectedValue, s.GetUsageStats(context.Background())["stats.oss.accesscontrol.enabled.count"])
}) })

@ -8,6 +8,9 @@ import (
"github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/infra/db"
authzextv1 "github.com/grafana/grafana/pkg/services/authz/proto/v1" authzextv1 "github.com/grafana/grafana/pkg/services/authz/proto/v1"
"github.com/grafana/grafana/pkg/services/authz/zanzana" "github.com/grafana/grafana/pkg/services/authz/zanzana"
"github.com/grafana/grafana/pkg/services/dashboards"
"github.com/grafana/grafana/pkg/services/folder"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/setting"
) )
@ -63,24 +66,26 @@ func teamMembershipCollector(store db.DB) legacyTupleCollector {
} }
// folderTreeCollector collects folder tree structure and writes it as relation tuples // folderTreeCollector collects folder tree structure and writes it as relation tuples
func folderTreeCollector(store db.DB) legacyTupleCollector { func folderTreeCollector(folderService folder.Service) legacyTupleCollector {
return func(ctx context.Context, orgID int64) (map[string]map[string]*openfgav1.TupleKey, error) { return func(ctx context.Context, orgID int64) (map[string]map[string]*openfgav1.TupleKey, error) {
ctx, span := tracer.Start(ctx, "accesscontrol.migrator.folderTreeCollector") ctx, span := tracer.Start(ctx, "accesscontrol.migrator.folderTreeCollector")
defer span.End() defer span.End()
const query = ` user := &user.SignedInUser{
SELECT uid, parent_uid, org_id FROM folder WHERE org_id = ? Login: "folder-tree-collector",
` OrgRole: "Admin",
type folder struct { IsGrafanaAdmin: true,
FolderUID string `xorm:"uid"` IsServiceAccount: true,
ParentUID string `xorm:"parent_uid"` Permissions: map[int64]map[string][]string{orgID: {dashboards.ActionFoldersRead: {dashboards.ScopeFoldersAll}}},
OrgID: orgID,
} }
var folders []folder q := folder.GetFoldersQuery{
err := store.WithDbSession(ctx, func(sess *db.Session) error { OrgID: orgID,
return sess.SQL(query, orgID).Find(&folders) SignedInUser: user,
}) }
folders, err := folderService.GetFolders(ctx, q)
if err != nil { if err != nil {
return nil, err return nil, err
} }
@ -94,7 +99,7 @@ func folderTreeCollector(store db.DB) legacyTupleCollector {
} }
tuple = &openfgav1.TupleKey{ tuple = &openfgav1.TupleKey{
Object: zanzana.NewTupleEntry(zanzana.TypeFolder, f.FolderUID, ""), Object: zanzana.NewTupleEntry(zanzana.TypeFolder, f.UID, ""),
Relation: zanzana.RelationParent, Relation: zanzana.RelationParent,
User: zanzana.NewTupleEntry(zanzana.TypeFolder, f.ParentUID, ""), User: zanzana.NewTupleEntry(zanzana.TypeFolder, f.ParentUID, ""),
} }

@ -13,6 +13,7 @@ import (
"github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/infra/serverlock" "github.com/grafana/grafana/pkg/infra/serverlock"
"github.com/grafana/grafana/pkg/services/authz/zanzana" "github.com/grafana/grafana/pkg/services/authz/zanzana"
"github.com/grafana/grafana/pkg/services/folder"
"github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/setting"
) )
@ -34,7 +35,7 @@ type ZanzanaReconciler struct {
reconcilers []resourceReconciler reconcilers []resourceReconciler
} }
func NewZanzanaReconciler(cfg *setting.Cfg, client zanzana.Client, store db.DB, lock *serverlock.ServerLockService) *ZanzanaReconciler { func NewZanzanaReconciler(cfg *setting.Cfg, client zanzana.Client, store db.DB, lock *serverlock.ServerLockService, folderService folder.Service) *ZanzanaReconciler {
zanzanaReconciler := &ZanzanaReconciler{ zanzanaReconciler := &ZanzanaReconciler{
cfg: cfg, cfg: cfg,
log: log.New("zanzana.reconciler"), log: log.New("zanzana.reconciler"),
@ -50,7 +51,7 @@ func NewZanzanaReconciler(cfg *setting.Cfg, client zanzana.Client, store db.DB,
), ),
newResourceReconciler( newResourceReconciler(
"folder tree", "folder tree",
folderTreeCollector(store), folderTreeCollector(folderService),
zanzanaCollector([]string{zanzana.RelationParent}), zanzanaCollector([]string{zanzana.RelationParent}),
client, client,
), ),

@ -33,10 +33,6 @@ func ProvideFolderPermissions(
sqlStore *sqlstore.SQLStore, sqlStore *sqlstore.SQLStore,
) (*ossaccesscontrol.FolderPermissionsService, error) { ) (*ossaccesscontrol.FolderPermissionsService, error) {
actionSets := resourcepermissions.NewActionSetService(features) actionSets := resourcepermissions.NewActionSetService(features)
acSvc := acimpl.ProvideOSSService(
cfg, acdb.ProvideService(sqlStore), actionSets, localcache.ProvideService(),
features, tracing.InitializeTracerForTest(), zanzana.NewNoopClient(), sqlStore, permreg.ProvidePermissionRegistry(), nil,
)
license := licensingtest.NewFakeLicensing() license := licensingtest.NewFakeLicensing()
license.On("FeatureEnabled", "accesscontrol.enforcement").Return(true).Maybe() license.On("FeatureEnabled", "accesscontrol.enforcement").Return(true).Maybe()
@ -55,6 +51,12 @@ func ProvideFolderPermissions(
dashboardStore, folderStore, sqlStore, features, dashboardStore, folderStore, sqlStore, features,
supportbundlestest.NewFakeBundleService(), nil, tracing.InitializeTracerForTest()) supportbundlestest.NewFakeBundleService(), nil, tracing.InitializeTracerForTest())
acSvc := acimpl.ProvideOSSService(
cfg, acdb.ProvideService(sqlStore), actionSets, localcache.ProvideService(),
features, tracing.InitializeTracerForTest(), zanzana.NewNoopClient(), sqlStore, permreg.ProvidePermissionRegistry(),
nil, fService,
)
orgService, err := orgimpl.ProvideService(sqlStore, cfg, quotaService) orgService, err := orgimpl.ProvideService(sqlStore, cfg, quotaService)
if err != nil { if err != nil {
return nil, err return nil, err

@ -8,6 +8,7 @@ import (
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
"github.com/grafana/grafana/pkg/bus"
"github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/infra/db"
"github.com/grafana/grafana/pkg/infra/serverlock" "github.com/grafana/grafana/pkg/infra/serverlock"
"github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/infra/tracing"
@ -23,6 +24,7 @@ import (
"github.com/grafana/grafana/pkg/services/guardian" "github.com/grafana/grafana/pkg/services/guardian"
"github.com/grafana/grafana/pkg/services/quota/quotatest" "github.com/grafana/grafana/pkg/services/quota/quotatest"
"github.com/grafana/grafana/pkg/services/sqlstore/migrator" "github.com/grafana/grafana/pkg/services/sqlstore/migrator"
"github.com/grafana/grafana/pkg/services/supportbundles/supportbundlestest"
"github.com/grafana/grafana/pkg/services/tag/tagimpl" "github.com/grafana/grafana/pkg/services/tag/tagimpl"
"github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/setting"
@ -87,10 +89,24 @@ func TestIntegrationDashboardServiceZanzana(t *testing.T) {
createDashboards(t, service, 100, "test-a") createDashboards(t, service, 100, "test-a")
createDashboards(t, service, 100, "test-b") createDashboards(t, service, 100, "test-b")
folderImplStore := folderimpl.ProvideStore(db)
folderService := folderimpl.ProvideService(
folderImplStore,
ac,
bus.ProvideBus(tracing.InitializeTracerForTest()),
dashboardStore,
folderStore,
db,
featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders),
supportbundlestest.NewFakeBundleService(),
nil,
tracing.InitializeTracerForTest(),
)
// Sync Grafana DB with zanzana (migrate data) // Sync Grafana DB with zanzana (migrate data)
tracer := tracing.InitializeTracerForTest() tracer := tracing.InitializeTracerForTest()
lock := serverlock.ProvideService(db, tracer) lock := serverlock.ProvideService(db, tracer)
zanzanaSyncronizer := dualwrite.NewZanzanaReconciler(cfg, zclient, db, lock) zanzanaSyncronizer := dualwrite.NewZanzanaReconciler(cfg, zclient, db, lock, folderService)
err = zanzanaSyncronizer.ReconcileSync(context.Background()) err = zanzanaSyncronizer.ReconcileSync(context.Background())
require.NoError(t, err) require.NoError(t, err)

@ -68,7 +68,7 @@ func ProvideService(
supportBundles supportbundles.Service, supportBundles supportbundles.Service,
r prometheus.Registerer, r prometheus.Registerer,
tracer tracing.Tracer, tracer tracing.Tracer,
) folder.Service { ) *Service {
srv := &Service{ srv := &Service{
log: slog.Default().With("logger", "folder-service"), log: slog.Default().With("logger", "folder-service"),
dashboardStore: dashboardStore, dashboardStore: dashboardStore,

@ -55,7 +55,7 @@ func setupTestEnv(t *testing.T) *TestEnv {
acSvc: acimpl.ProvideOSSService( acSvc: acimpl.ProvideOSSService(
cfg, env.AcStore, &resourcepermissions.FakeActionSetSvc{}, cfg, env.AcStore, &resourcepermissions.FakeActionSetSvc{},
localcache.New(0, 0), fmgt, tracing.InitializeTracerForTest(), nil, nil, localcache.New(0, 0), fmgt, tracing.InitializeTracerForTest(), nil, nil,
permreg.ProvidePermissionRegistry(), nil, permreg.ProvidePermissionRegistry(), nil, nil,
), ),
defaultOrgID: autoAssignOrgID, defaultOrgID: autoAssignOrgID,
logger: logger, logger: logger,

Loading…
Cancel
Save