thread safe role cache'

Emyrk · Emyrk · commit 2b0cba6e2e25 · 2024-05-15T22:06:29.000-05:00
diff --git a/coderd/database/migrations/000209_custom_roles.up.sql b/coderd/database/migrations/000209_custom_roles.up.sql
@@ -1,5 +1,5 @@
 CREATE TABLE custom_roles (
-	-- name is globally unique. Org scoped roles have their orgid prepended
+	-- name is globally unique. Org scoped roles have their orgid appended
 	-- like:  "name":"organization-admin:bbe8c156-c61e-4d36-b91e-697c6b1477e8"
 	name text primary key,
 	-- display_name is the actual name of the role displayed to the user.
diff --git a/coderd/rbac/roles.go b/coderd/rbac/roles.go
@@ -380,6 +380,11 @@ type Permission struct {
 }
 
 func (perm Permission) Valid() error {
+	if perm.ResourceType == policy.WildcardSymbol {
+		// Wildcard is tricky to check. Just allow it.
+		return nil
+	}
+
 	resource, ok := policy.RBACPermissions[perm.ResourceType]
 	if !ok {
 		return fmt.Errorf("invalid resource type %q", perm.ResourceType)
diff --git a/coderd/rbac/rolestore/rolestore.go b/coderd/rbac/rolestore/rolestore.go
@@ -9,13 +9,13 @@ import (
 
 	"github.com/coder/coder/v2/coderd/database"
 	"github.com/coder/coder/v2/coderd/rbac"
+	"github.com/coder/coder/v2/coderd/util/syncmap"
 )
 
-type (
-	customRoleCtxKey struct{}
-	customRoleCache  map[string]rbac.Role
-)
+type customRoleCtxKey struct{}
 
+// CustomRoleMW adds a custom role cache on the ctx to prevent duplicate
+// db fetches.
 func CustomRoleMW(next http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		r = r.WithContext(CustomRoleCacheContext(r.Context()))
@@ -27,33 +27,25 @@ func CustomRoleMW(next http.Handler) http.Handler {
 // same request lifecycle. Optimizing this to span requests should be done
 // in the future.
 func CustomRoleCacheContext(ctx context.Context) context.Context {
-	return context.WithValue(ctx, customRoleCtxKey{}, customRoleCache{})
+	return context.WithValue(ctx, customRoleCtxKey{}, syncmap.New[string, rbac.Role]())
 }
 
-func roleCache(ctx context.Context) customRoleCache {
-	c, ok := ctx.Value(customRoleCtxKey{}).(customRoleCache)
+func roleCache(ctx context.Context) *syncmap.Map[string, rbac.Role] {
+	c, ok := ctx.Value(customRoleCtxKey{}).(*syncmap.Map[string, rbac.Role])
 	if !ok {
-		return customRoleCache{}
+		return syncmap.New[string, rbac.Role]()
 	}
 	return c
 }
 
-func store(ctx context.Context, name string, role rbac.Role) {
-	roleCache(ctx)[name] = role
-}
-
-func load(ctx context.Context, name string) (rbac.Role, bool) {
-	r, ok := roleCache(ctx)[name]
-	return r, ok
-}
-
 // Expand will expand built in roles, and fetch custom roles from the database.
 func Expand(ctx context.Context, db database.Store, names []string) (rbac.Roles, error) {
 	if len(names) == 0 {
 		// That was easy
 		return []rbac.Role{}, nil
 	}
 
+	cache := roleCache(ctx)
 	lookup := make([]string, 0)
 	roles := make([]rbac.Role, 0, len(names))
 
@@ -66,7 +58,7 @@ func Expand(ctx context.Context, db database.Store, names []string) (rbac.Roles,
 		}
 
 		// Check custom role cache
-		customRole, ok := load(ctx, name)
+		customRole, ok := cache.Load(name)
 		if ok {
 			roles = append(roles, customRole)
 			continue
@@ -92,7 +84,7 @@ func Expand(ctx context.Context, db database.Store, names []string) (rbac.Roles,
 				return nil, xerrors.Errorf("convert db role %q: %w", dbrole, err)
 			}
 			roles = append(roles, converted)
-			store(ctx, dbrole.Name, converted)
+			cache.Store(dbrole.Name, converted)
 		}
 	}
 
diff --git a/enterprise/coderd/roles_test.go b/enterprise/coderd/roles_test.go
@@ -34,6 +34,7 @@ func TestCustomRole(t *testing.T) {
 
 		ctx := testutil.Context(t, testutil.WaitMedium)
 
+		//nolint:gocritic -- owner is required for this
 		role, err := owner.UpsertCustomSiteRole(ctx, codersdk.Role{
 			Name:        "test-role",
 			DisplayName: "Testing Purposes",
diff --git a/site/src/pages/UsersPage/UsersTable/UserRoleCell.tsx b/site/src/pages/UsersPage/UsersTable/UserRoleCell.tsx
@@ -160,7 +160,9 @@ const roleNamesByAccessLevel: readonly string[] = [
   "auditor",
 ];
 
-function sortRolesByAccessLevel(roles: readonly SlimRole[]): readonly SlimRole[] {
+function sortRolesByAccessLevel(
+  roles: readonly SlimRole[],
+): readonly SlimRole[] {
   if (roles.length === 0) {
     return roles;
   }
