From 86e8e3b75dd74fdabd7c0c5037c39c51297ac929 Mon Sep 17 00:00:00 2001 From: Aaron Godin Date: Wed, 28 Aug 2024 03:44:38 -0500 Subject: [PATCH] Revert "RBAC: Optimize permissions caching" (#92548) Revert "RBAC: Optimize permissions caching (#92412)" This reverts commit 488e994d378363fd6a866d3aa62e299462582d24. Co-authored-by: gamab --- pkg/services/accesscontrol/acimpl/service.go | 16 +--------------- pkg/services/accesscontrol/cacheutils.go | 13 +------------ pkg/services/accesscontrol/cacheutils_test.go | 3 +-- 3 files changed, 3 insertions(+), 29 deletions(-) diff --git a/pkg/services/accesscontrol/acimpl/service.go b/pkg/services/accesscontrol/acimpl/service.go index 877fdaa1abc..77e44244bf0 100644 --- a/pkg/services/accesscontrol/acimpl/service.go +++ b/pkg/services/accesscontrol/acimpl/service.go @@ -12,7 +12,6 @@ import ( "go.opentelemetry.io/otel/attribute" "github.com/grafana/authlib/claims" - "github.com/grafana/grafana/pkg/api/routing" "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/infra/db" @@ -243,11 +242,6 @@ func (s *Service) getCachedUserPermissions(ctx context.Context, user identity.Re defer span.End() permissions := []accesscontrol.Permission{} - cacheKey := accesscontrol.GetUserPermissionCacheKey(user) - if cachedPermissions, ok := s.cache.Get(cacheKey); ok { - return cachedPermissions.([]accesscontrol.Permission), nil - } - permissions, err := s.getCachedBasicRolesPermissions(ctx, user, options, permissions) if err != nil { return nil, err @@ -264,7 +258,6 @@ func (s *Service) getCachedUserPermissions(ctx context.Context, user identity.Re } permissions = append(permissions, userPermissions...) - s.cache.Set(cacheKey, permissions, cacheTTL) span.SetAttributes(attribute.Int("num_permissions", len(permissions))) return permissions, nil @@ -344,14 +337,8 @@ func (s *Service) getCachedTeamsPermissions(ctx context.Context, user identity.R teams := user.GetTeams() orgID := user.GetOrgID() miss := teams - compositeKey := accesscontrol.GetTeamPermissionCompositeCacheKey(teams, orgID) if !options.ReloadCache { - teamsPermissions, ok := s.cache.Get(compositeKey) - if ok { - return teamsPermissions.([]accesscontrol.Permission), nil - } - miss = make([]int64, 0) for _, teamID := range teams { key := accesscontrol.GetTeamPermissionCacheKey(teamID, orgID) @@ -381,13 +368,12 @@ func (s *Service) getCachedTeamsPermissions(ctx context.Context, user identity.R permissions = append(permissions, teamPermissions...) } } - s.cache.Set(compositeKey, permissions, cacheTTL) return permissions, nil } func (s *Service) ClearUserPermissionCache(user identity.Requester) { - s.cache.Delete(accesscontrol.GetUserPermissionCacheKey(user)) + s.cache.Delete(accesscontrol.GetPermissionCacheKey(user)) s.cache.Delete(accesscontrol.GetUserDirectPermissionCacheKey(user)) } diff --git a/pkg/services/accesscontrol/cacheutils.go b/pkg/services/accesscontrol/cacheutils.go index fe139662d95..9326d9d88c8 100644 --- a/pkg/services/accesscontrol/cacheutils.go +++ b/pkg/services/accesscontrol/cacheutils.go @@ -2,14 +2,12 @@ package accesscontrol import ( "fmt" - "slices" - "strconv" "strings" "github.com/grafana/grafana/pkg/apimachinery/identity" ) -func GetUserPermissionCacheKey(user identity.Requester) string { +func GetPermissionCacheKey(user identity.Requester) string { return fmt.Sprintf("rbac-permissions-%s", user.GetCacheKey()) } @@ -43,12 +41,3 @@ func GetBasicRolePermissionCacheKey(role string, orgID int64) string { func GetTeamPermissionCacheKey(teamID int64, orgID int64) string { return fmt.Sprintf("rbac-permissions-team-%d-%d", orgID, teamID) } - -func GetTeamPermissionCompositeCacheKey(teamIds []int64, orgID int64) string { - teams := make([]string, 0) - for _, id := range teamIds { - teams = append(teams, strconv.FormatInt(id, 10)) - } - slices.Sort(teams) - return fmt.Sprintf("rbac-permissions-team-%d-%s", orgID, strings.Join(teams, "-")) -} diff --git a/pkg/services/accesscontrol/cacheutils_test.go b/pkg/services/accesscontrol/cacheutils_test.go index c5bf747b60b..7aff7941586 100644 --- a/pkg/services/accesscontrol/cacheutils_test.go +++ b/pkg/services/accesscontrol/cacheutils_test.go @@ -6,7 +6,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/grafana/authlib/claims" - "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/user" ) @@ -69,7 +68,7 @@ func TestPermissionCacheKey(t *testing.T) { for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { - assert.Equal(t, tc.expected, GetUserPermissionCacheKey(tc.signedInUser)) + assert.Equal(t, tc.expected, GetPermissionCacheKey(tc.signedInUser)) }) } }