Users: Use Remote Cache for storing signed in users [v9.3.x] (#59883) (#59934)

* FeatureToggle: for storing signed in user object in a Remote Cache (#59883)

Co-authored-by: ievaVasiljeva <ieva.vasiljeva@grafana.com>
This commit is contained in:
Gabriel MABILLE
2022-12-07 11:04:41 +01:00
committed by GitHub
co-authored by ievaVasiljeva
parent a32d25bbe3
commit 3adad3c21a
12 changed files with 107 additions and 16 deletions
@@ -78,6 +78,7 @@ export interface FeatureToggles {
queryLibrary?: boolean;
showDashboardValidationWarnings?: boolean;
mysqlAnsiQuotes?: boolean;
userRemoteCache?: boolean;
datasourceLogger?: boolean;
accessControlOnCall?: boolean;
nestedFolders?: boolean;
+5 -1
View File
@@ -403,7 +403,11 @@ func setupHTTPServerWithCfgDb(
acService, err = acimpl.ProvideService(cfg, db, routeRegister, localcache.ProvideService(), featuremgmt.WithFeatures())
require.NoError(t, err)
ac = acimpl.ProvideAccessControl(cfg)
userSvc, err = userimpl.ProvideService(db, nil, cfg, teamimpl.ProvideService(db, cfg), localcache.ProvideService(), quotatest.New(false, nil))
userSvc, err = userimpl.ProvideService(
db, nil, cfg, teamimpl.ProvideService(db, cfg),
localcache.ProvideService(), quotatest.New(false, nil),
nil, featuremgmt.WithFeatures(),
)
require.NoError(t, err)
}
teamPermissionService, err := ossaccesscontrol.ProvideTeamPermissions(cfg, routeRegister, db, ac, license, acService, teamService, userSvc)
+24 -6
View File
@@ -393,7 +393,10 @@ func TestGetOrgUsersAPIEndpoint_AccessControlMetadata(t *testing.T) {
var err error
sc := setupHTTPServerWithCfg(t, false, cfg, func(hs *HTTPServer) {
hs.userService, err = userimpl.ProvideService(
hs.SQLStore, nil, cfg, teamimpl.ProvideService(hs.SQLStore.(*sqlstore.SQLStore), cfg), localcache.ProvideService(), quotatest.New(false, nil))
hs.SQLStore, nil, cfg, teamimpl.ProvideService(hs.SQLStore.(*sqlstore.SQLStore), cfg),
localcache.ProvideService(), quotatest.New(false, nil),
nil, featuremgmt.WithFeatures(),
)
require.NoError(t, err)
hs.orgService, err = orgimpl.ProvideService(hs.SQLStore, cfg, quotatest.New(false, nil))
require.NoError(t, err)
@@ -500,7 +503,10 @@ func TestGetOrgUsersAPIEndpoint_AccessControl(t *testing.T) {
sc := setupHTTPServerWithCfg(t, false, cfg, func(hs *HTTPServer) {
quotaService := quotatest.New(false, nil)
hs.userService, err = userimpl.ProvideService(
hs.SQLStore, nil, cfg, teamimpl.ProvideService(hs.SQLStore.(*sqlstore.SQLStore), cfg), localcache.ProvideService(), quotaService)
hs.SQLStore, nil, cfg, teamimpl.ProvideService(hs.SQLStore.(*sqlstore.SQLStore), cfg),
localcache.ProvideService(), quotaService,
nil, featuremgmt.WithFeatures(),
)
require.NoError(t, err)
hs.orgService, err = orgimpl.ProvideService(hs.SQLStore, cfg, quotaService)
require.NoError(t, err)
@@ -607,7 +613,10 @@ func TestPostOrgUsersAPIEndpoint_AccessControl(t *testing.T) {
var err error
sc := setupHTTPServerWithCfg(t, false, cfg, func(hs *HTTPServer) {
hs.userService, err = userimpl.ProvideService(
hs.SQLStore, nil, cfg, teamimpl.ProvideService(hs.SQLStore.(*sqlstore.SQLStore), cfg), localcache.ProvideService(), quotatest.New(false, nil))
hs.SQLStore, nil, cfg, teamimpl.ProvideService(hs.SQLStore.(*sqlstore.SQLStore), cfg),
localcache.ProvideService(), quotatest.New(false, nil),
nil, featuremgmt.WithFeatures(),
)
require.NoError(t, err)
})
@@ -727,7 +736,10 @@ func TestOrgUsersAPIEndpointWithSetPerms_AccessControl(t *testing.T) {
sc := setupHTTPServer(t, true, func(hs *HTTPServer) {
hs.tempUserService = tempuserimpl.ProvideService(hs.SQLStore)
hs.userService, err = userimpl.ProvideService(
hs.SQLStore, nil, setting.NewCfg(), teamimpl.ProvideService(hs.SQLStore.(*sqlstore.SQLStore), setting.NewCfg()), localcache.ProvideService(), quotatest.New(false, nil))
hs.SQLStore, nil, setting.NewCfg(), teamimpl.ProvideService(hs.SQLStore.(*sqlstore.SQLStore),
setting.NewCfg()), localcache.ProvideService(), quotatest.New(false, nil),
nil, featuremgmt.WithFeatures(),
)
require.NoError(t, err)
})
setInitCtxSignedInViewer(sc.initCtx)
@@ -847,7 +859,10 @@ func TestPatchOrgUsersAPIEndpoint_AccessControl(t *testing.T) {
sc := setupHTTPServerWithCfg(t, false, cfg, func(hs *HTTPServer) {
quotaService := quotatest.New(false, nil)
hs.userService, err = userimpl.ProvideService(
hs.SQLStore, nil, cfg, teamimpl.ProvideService(hs.SQLStore.(*sqlstore.SQLStore), cfg), localcache.ProvideService(), quotaService)
hs.SQLStore, nil, cfg, teamimpl.ProvideService(hs.SQLStore.(*sqlstore.SQLStore), cfg),
localcache.ProvideService(), quotaService,
nil, featuremgmt.WithFeatures(),
)
require.NoError(t, err)
hs.orgService, err = orgimpl.ProvideService(hs.SQLStore, cfg, quotaService)
require.NoError(t, err)
@@ -977,7 +992,10 @@ func TestDeleteOrgUsersAPIEndpoint_AccessControl(t *testing.T) {
sc := setupHTTPServerWithCfg(t, false, cfg, func(hs *HTTPServer) {
quotaService := quotatest.New(false, nil)
hs.userService, err = userimpl.ProvideService(
hs.SQLStore, nil, cfg, teamimpl.ProvideService(hs.SQLStore.(*sqlstore.SQLStore), cfg), localcache.ProvideService(), quotaService)
hs.SQLStore, nil, cfg, teamimpl.ProvideService(hs.SQLStore.(*sqlstore.SQLStore), cfg),
localcache.ProvideService(), quotaService,
nil, featuremgmt.WithFeatures(),
)
require.NoError(t, err)
hs.orgService, err = orgimpl.ProvideService(hs.SQLStore, cfg, quotaService)
require.NoError(t, err)
+5 -1
View File
@@ -20,6 +20,7 @@ import (
"github.com/grafana/grafana/pkg/infra/usagestats"
"github.com/grafana/grafana/pkg/models"
acmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/services/login/authinfoservice"
authinfostore "github.com/grafana/grafana/pkg/services/login/authinfoservice/database"
"github.com/grafana/grafana/pkg/services/login/logintest"
@@ -72,7 +73,10 @@ func TestUserAPIEndpoint_userLoggedIn(t *testing.T) {
}
user, err := sqlStore.CreateUser(context.Background(), createUserCmd)
require.Nil(t, err)
hs.userService, err = userimpl.ProvideService(sqlStore, nil, sc.cfg, nil, nil, quotatest.New(false, nil))
hs.userService, err = userimpl.ProvideService(
sqlStore, nil, sc.cfg, nil, nil, quotatest.New(false, nil),
nil, featuremgmt.WithFeatures(),
)
require.NoError(t, err)
sc.handlerFunc = hs.GetUserByID
@@ -11,6 +11,7 @@ import (
"github.com/grafana/grafana/pkg/infra/db"
"github.com/grafana/grafana/pkg/services/accesscontrol"
accesscontrolmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/services/licensing/licensingtest"
"github.com/grafana/grafana/pkg/services/quota/quotatest"
"github.com/grafana/grafana/pkg/services/sqlstore"
@@ -226,7 +227,8 @@ func setupTestEnvironment(t *testing.T, permissions []accesscontrol.Permission,
sql := db.InitTestDB(t)
cfg := setting.NewCfg()
teamSvc := teamimpl.ProvideService(sql, cfg)
userSvc, err := userimpl.ProvideService(sql, nil, cfg, teamimpl.ProvideService(sql, cfg), nil, quotatest.New(false, nil))
userSvc, err := userimpl.ProvideService(sql, nil, cfg, teamimpl.ProvideService(sql, cfg),
nil, quotatest.New(false, nil), nil, featuremgmt.WithFeatures())
require.NoError(t, err)
license := licensingtest.NewFakeLicensing()
license.On("FeatureEnabled", "accesscontrol.enforcement").Return(true).Maybe()
+5
View File
@@ -346,6 +346,11 @@ var (
Description: "Use double quote to escape keyword in Mysql query",
State: FeatureStateAlpha,
},
{
Name: "userRemoteCache",
Description: "Enable using remote cache for users",
State: FeatureStateAlpha,
},
{
Name: "datasourceLogger",
Description: "Logs all datasource requests",
+4
View File
@@ -255,6 +255,10 @@ const (
// Use double quote to escape keyword in Mysql query
FlagMysqlAnsiQuotes = "mysqlAnsiQuotes"
// FlagUserRemoteCache
// Enable using remote cache for users
FlagUserRemoteCache = "userRemoteCache"
// FlagDatasourceLogger
// Logs all datasource requests
FlagDatasourceLogger = "datasourceLogger"
@@ -606,7 +606,8 @@ func setupAccessControlGuardianTest(t *testing.T, uid string, permissions []acce
license := licensingtest.NewFakeLicensing()
license.On("FeatureEnabled", "accesscontrol.enforcement").Return(true).Maybe()
teamSvc := teamimpl.ProvideService(store, store.Cfg)
userSvc, err := userimpl.ProvideService(store, nil, store.Cfg, nil, nil, quotatest.New(false, nil))
userSvc, err := userimpl.ProvideService(store, nil, store.Cfg, nil, nil,
quotatest.New(false, nil), nil, featuremgmt.WithFeatures())
require.NoError(t, err)
folderPermissions, err := ossaccesscontrol.ProvideFolderPermissions(
+2 -1
View File
@@ -88,7 +88,8 @@ func TestIntegrationQuotaCommandsAndQueries(t *testing.T) {
quotaService := ProvideService(sqlStore, sqlStore.Cfg)
orgService, err := orgimpl.ProvideService(sqlStore, sqlStore.Cfg, quotaService)
require.NoError(t, err)
userService, err := userimpl.ProvideService(sqlStore, orgService, sqlStore.Cfg, nil, nil, quotaService)
userService, err := userimpl.ProvideService(sqlStore, orgService,
sqlStore.Cfg, nil, nil, quotaService, nil, featuremgmt.WithFeatures())
require.NoError(t, err)
setupEnv(t, sqlStore, b, quotaService)
+3 -1
View File
@@ -25,6 +25,7 @@ import (
"github.com/grafana/grafana/pkg/services/accesscontrol/ossaccesscontrol"
"github.com/grafana/grafana/pkg/services/apikey/apikeyimpl"
"github.com/grafana/grafana/pkg/services/contexthandler/ctxkey"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/services/licensing"
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/services/org/orgimpl"
@@ -292,7 +293,8 @@ func setupTestServer(t *testing.T, svc *tests.ServiceAccountMock,
cfg := setting.NewCfg()
teamSvc := teamimpl.ProvideService(sqlStore, cfg)
userSvc, err := userimpl.ProvideService(sqlStore, nil, cfg, teamimpl.ProvideService(sqlStore, cfg), nil, quotatest.New(false, nil))
userSvc, err := userimpl.ProvideService(sqlStore, nil, cfg, teamimpl.ProvideService(sqlStore, cfg),
nil, quotatest.New(false, nil), nil, featuremgmt.WithFeatures())
require.NoError(t, err)
saPermissionService, err := ossaccesscontrol.ProvideServiceAccountPermissions(
cfg, routing.NewRouteRegister(), sqlStore, acmock, &licensing.OSSLicensingService{}, saStore, acmock, teamSvc, userSvc)
+50 -4
View File
@@ -8,9 +8,12 @@ import (
"github.com/grafana/grafana/pkg/infra/db"
"github.com/grafana/grafana/pkg/infra/localcache"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/infra/remotecache"
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/models/roletype"
ac "github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/services/quota"
"github.com/grafana/grafana/pkg/services/team"
@@ -24,6 +27,9 @@ type Service struct {
orgService org.Service
teamService team.Service
cacheService *localcache.CacheService
remoteCache *remotecache.RemoteCache
logger log.Logger
features *featuremgmt.FeatureManager
cfg *setting.Cfg
}
@@ -34,6 +40,8 @@ func ProvideService(
teamService team.Service,
cacheService *localcache.CacheService,
quotaService quota.Service,
remoteCache *remotecache.RemoteCache,
features *featuremgmt.FeatureManager,
) (user.Service, error) {
store := ProvideStore(db, cfg)
s := &Service{
@@ -42,6 +50,13 @@ func ProvideService(
cfg: cfg,
teamService: teamService,
cacheService: cacheService,
remoteCache: remoteCache,
features: features,
logger: log.New("user.service"),
}
if features.IsEnabled(featuremgmt.FlagUserRemoteCache) {
remotecache.Register(user.SignedInUser{})
}
defaultLimits, err := readQuotaConfig(cfg)
@@ -231,15 +246,35 @@ func (s *Service) SetUsingOrg(ctx context.Context, cmd *user.SetUsingOrgCommand)
}
func (s *Service) GetSignedInUserWithCacheCtx(ctx context.Context, query *user.GetSignedInUserQuery) (*user.SignedInUser, error) {
var signedInUser *user.SignedInUser
// only check cache if we have a user ID and an org ID in query
if query.OrgID > 0 && query.UserID > 0 {
cacheKey := newSignedInUserCacheKey(query.OrgID, query.UserID)
// Fetching from remote cache first
if s.features.IsEnabled(featuremgmt.FlagUserRemoteCache) {
res, errCache := s.remoteCache.Get(ctx, cacheKey)
if errCache == nil {
if cachedUser, ok := res.(user.SignedInUser); ok {
s.logger.Debug("got user from remote cache",
"cacheKey", cacheKey)
return &cachedUser, nil
}
} else {
if errors.Is(errCache, remotecache.ErrCacheItemNotFound) {
s.logger.Debug("user not found in cache",
"cacheKey", cacheKey)
} else {
s.logger.Warn("failed to get user from cache",
"cacheKey", cacheKey, "error", errCache)
}
}
}
// Fallback to in memory cache
if cached, found := s.cacheService.Get(cacheKey); found {
cachedUser := cached.(user.SignedInUser)
signedInUser = &cachedUser
return signedInUser, nil
s.logger.Debug("got user from local cache", "cachekey", cacheKey)
return &cachedUser, nil
}
}
@@ -249,7 +284,18 @@ func (s *Service) GetSignedInUserWithCacheCtx(ctx context.Context, query *user.G
}
cacheKey := newSignedInUserCacheKey(result.OrgID, result.UserID)
// Remember user in remote cache
if s.features.IsEnabled(featuremgmt.FlagUserRemoteCache) {
errCache := s.remoteCache.Set(ctx, cacheKey, *(result), time.Second*5)
if errCache != nil {
s.logger.Warn("could not cache user in remote cache",
"cacheKey", cacheKey, "error", errCache)
}
}
// Remember user in memory cache
s.cacheService.Set(cacheKey, *result, time.Second*5)
return result, nil
}
+3
View File
@@ -7,6 +7,7 @@ import (
"github.com/grafana/grafana/pkg/infra/localcache"
"github.com/grafana/grafana/pkg/models/roletype"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/services/org/orgtest"
"github.com/grafana/grafana/pkg/services/team/teamtest"
@@ -24,6 +25,7 @@ func TestUserService(t *testing.T) {
store: userStore,
orgService: orgService,
cacheService: localcache.ProvideService(),
features: featuremgmt.WithFeatures(),
}
t.Run("create user", func(t *testing.T) {
@@ -102,6 +104,7 @@ func TestUserService(t *testing.T) {
orgService: orgService,
cacheService: localcache.ProvideService(),
teamService: teamtest.NewFakeService(),
features: featuremgmt.WithFeatures(),
}
usr := &user.SignedInUser{
OrgID: 1,